Community
Participate
Working Groups
Build Identifier: I20090611-1540 The problem occurs for a toolchain that has an option for which the "Name" attribute is not defined. And it happens when a project has a folder specific settings. Reproducible: Always Steps to Reproduce: 1. Create a project with a source folder 2. Select a source folder and enter folder Properities -> C/C++ build settings. 3. Change any checkbox setting to create folder-specific settings 4. Change the same checkbox now for the whole project 5. Try to uncheck the checkbox, the NPE window pops up. The root of the problem is in Tool.java private boolean isAnyOptionModified(ITool t1, ITool t2) { for (IOption op1 : t1.getOptions()) { for (IOption op2 : t2.getOptions()) { // find matching option try { if (op1.getValueType() == op2.getValueType() && op1.getName().equals(op2.getName())) { ....... op1.getName().equals throws NPE.
Tested with CDT 6.0, but this method is the same also in CDT 7.0 HEAD. Matching option check, in this case, should be done by comparing option "ID" and not the "name".
Created attachment 175421 [details] Changed the calls IOption.getName() to IOption.getId()
I don't think that's right. Historically (from the MBS extensibility document) names are used to check option & tool equivalence, not id. In this case what happens for derived options which are of the same type, but have a different ID?
Good point, I'll take a look at it tomorrow, perhaps getBaseId() is the way to go. The problem with the "Name" attribute is that it is neither unique nor required for option definition.
Hmm. Doing some archaeology shows that this change appeared with bug 223059. It's difficult to know why Oleg is comparing the option value type && name rather than just the base Id... > The problem with the "Name" attribute is that it is neither unique nor required for option definition. How do you have an option without a name? What does the user see? Note that the Managed Build System Extensibility document does state an option name is required: 3.13 : "Each option must have a unique id for the build model to properly manage it. A descriptive name that will appear in the UI must be specified. " I suspect this is done to preserve settings when changing toolchains. With your change are settings preserved when you do this?
(In reply to comment #5) > How do you have an option without a name? What does the user see? This option is not visible to the user. I'm using a GNUARM Eclipse plug-in from SF for cross compiling and it has the following definition: <inputType id="org.eclipse.cdt.gnuarm.objcopyflash.input" name="Input" multipleOfType="true" option="org.eclipse.cdt.gnuarm.objcopyflash.InFileName" sources="elf" /> <option id="org.eclipse.cdt.gnuarm.objcopyflash.InFileName" resourceFilter="all" value="${BuildArtifactFileName}" valueType="string" /> Although it's from version 0.4.0, but the latest version has similar definitions. > Note that the Managed Build System Extensibility document does state an option > name is required: > 3.13 : "Each option must have a unique id for the build model to properly > manage it. A descriptive name that will appear in the UI must be specified. " I would agree that it's a plug-in bug, if the schema specified this attribute as required, but it doesn't. At plugin.xml editor, the "Name" attribute is not required (has no *). Also the Managed Build System Extensibility document says it's not required at the schema table 3.13.5. Then, even if we change the schema, the "Name" is not unique, what will happen if different options have the same name? > I suspect this is done to preserve settings when changing toolchains. With your change are settings preserved when you do this? I'm not sure I follow, but both tools compared here are exactly the same, this is ensured at the caller method hasCustomSettings(). Though, there is a problem with the patch using getId() or getBaseId() as is, because the returned value is appended with a random number, which is different for folder config and project config for the same setting. The compare should be done without the number.
(In reply to comment #6) > I would agree that it's a plug-in bug, if the schema specified this attribute > as required, but it doesn't. > ... There are lots of issues with the schema & JavaDoc. It may very well be that this is a real bug, however the code has been this way for more than 2 years, and as the person who wrote this is no longer here, we need to be very careful we don't break anything. > Then, even if we change the schema, the "Name" is not unique, what will happen > if different options have the same name? Tool options in CDT in most cases map to options on real tools in the toolchain. In this case name really is unique and non-null -- i.e. an options valueType & name really does uniquely identify it. > Though, there is a problem with the patch using getId() or getBaseId() as is, Ah, I'm not sure how easy this will be. The model objects are derived from other objects. The root of this will be the object created from the toolchain xml. I guess you need to recurse the superClass hierarchy (until there are no more) and get the base id of that. I'm certainly not against fixing a bug, we should just look carefully at why the code is as it is today, and what changing this could affect.
