Bugzilla – Full Text Bug Listing
| Summary: | Unknown tool option silently breaks all other options => gives broken build | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Tools] CDT | Reporter: | James Blackburn <jamesblackburn+eclipse> | ||||
| Component: | cdt-build-managed | Assignee: | James Blackburn <jamesblackburn+eclipse> | ||||
| Status: | RESOLVED FIXED | QA Contact: | Chris Recoskie <recoskie> | ||||
| Severity: | major | ||||||
| Priority: | P3 | CC: | elaskavaia.cdt | ||||
| Version: | 7.0 | ||||||
| Target Milestone: | 7.0 | ||||||
| Hardware: | PC | ||||||
| OS: | Linux-GTK | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
James Blackburn
Option#getValueType can throw a buildException. This is thrown all the way out to GnuMakefileGenerator:
Option.getValueType() line: 1469
Tool.getToolCommandFlags(IPath, IPath, SupplierBasedCdtVariableSubstitutor, IMacroContextInfoProvider) line: 2545
Tool.getToolCommandFlags(IPath, IPath) line: 2682
GnuMakefileGenerator.addRuleForSource(String, StringBuffer, IResource, IPath, IResourceInfo, boolean, Vector<IPath>, Vector<IPath>) line: 2637
GnuMakefileGenerator.addFragmentMakefileEntriesForSource(LinkedHashMap<String,String>, StringBuffer, IFolder, String, IResource, IPath, IResourceInfo, String, boolean) line: 2080
This is then silently eaten by the GnuMakefileGenerator:
// Get the tool command line options
try {
flags = tool.getToolCommandFlags(sourceLocation, outputLocation);
} catch( BuildException ex ) {
// TODO add some routines to catch this
flags = EMPTY_STRING_ARRAY;
}
This is pretty bad. #getToolCommandFlags iterates over all the tool options. So one bad option breaks all the others...
Chris, Andrew: AFAICS #getToolCommandFlags(IPath, IPath, ...) should never throw BuildException as *all* 21 indirect callers (as viewed by show call hierarchy) _silently_ eat this exception. So for fixing point 2, I'd say that getToolCaommandFlags shouldn't throw a build exception but rather should log the exception and carry on processing other options. Created attachment 170632 [details] patch 1 The attached patch fixes issues 1 & 2. The warning emitted to the error log, looks like: Problem discovering arguments for Tool option: Include files (-include) (gnu.c.compiler.option.include.files.953373612) <backtrace> The patch is straightforward; I'll target 7.0.1. Detailed changes: - ManagedBuildCorePlugin: Incorrectly logs CoreExceptions (see CoreException#getStatus() - Option.java: attempt to preserve superClassId even if superClass == null - Tool.java#getToolCommandFlags(... SupplierBasedCdtVariableSubstitutor ...) + no longer throws BuildException. + Logs exceptions (like above is something goes wrong) The 3rd issue mentioned in comment 0: it seems that options are only emitted in the .cproject if they're ever changed. Unfortunately, resetting them to defaultValue doesn't remove them. This is a less severe issue, so not tackling this. Why not fix it 7.0? it seems safe. People often switch between versions... (In reply to comment #4) > Why not fix it 7.0? it seems safe. People often switch between versions... Ok. I just needed review. This is pretty serious for us, so I've put the fix into our product, will commit to CDT. Fixed in 7.0 *** cdt cvs genie on behalf of jblackburn *** Bug 315187 Unknown tool option silently breaks ManagedBuild tool command line generation [*] PluginResources.properties 1.58 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/core/PluginResources.properties?root=Tools_Project&r1=1.57&r2=1.58 [*] Option.java 1.47 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/core/Option.java?root=Tools_Project&r1=1.46&r2=1.47 [*] Tool.java 1.98 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/core/Tool.java?root=Tools_Project&r1=1.97&r2=1.98 [*] BuildMacroProvider.java 1.10 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/macros/BuildMacroProvider.java?root=Tools_Project&r1=1.9&r2=1.10 [*] ManagedBuilderCorePlugin.java 1.28 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/core/ManagedBuilderCorePlugin.java?root=Tools_Project&r1=1.27&r2=1.28 [*] ToolSettingsPrefStore.java 1.7 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.managedbuilder.ui/src/org/eclipse/cdt/managedbuilder/ui/properties/ToolSettingsPrefStore.java?root=Tools_Project&r1=1.6&r2=1.7 |