From cc0bb2b375454faf8a0b0c47ff74f6708a5134e4 Mon Sep 17 00:00:00 2001 From: Andrew Gvozdev Date: Sun, 6 Oct 2013 07:04:05 -0400 Subject: [PATCH] bug 392416: Rebuild builtin GCC compiler settings when GCC version changes --- .../tests/BuiltinSpecsDetectorTest.java | 170 +++++++++++++----- .../AbstractBuiltinSpecsDetector.java | 61 ++++++- .../ToolchainBuiltinSpecsDetector.java | 7 +- 3 files changed, 185 insertions(+), 53 deletions(-) diff --git a/build/org.eclipse.cdt.managedbuilder.core.tests/tests/org/eclipse/cdt/managedbuilder/language/settings/providers/tests/BuiltinSpecsDetectorTest.java b/build/org.eclipse.cdt.managedbuilder.core.tests/tests/org/eclipse/cdt/managedbuilder/language/settings/providers/tests/BuiltinSpecsDetectorTest.java index cf6aea5f211..68c897fe425 100644 --- a/build/org.eclipse.cdt.managedbuilder.core.tests/tests/org/eclipse/cdt/managedbuilder/language/settings/providers/tests/BuiltinSpecsDetectorTest.java +++ b/build/org.eclipse.cdt.managedbuilder.core.tests/tests/org/eclipse/cdt/managedbuilder/language/settings/providers/tests/BuiltinSpecsDetectorTest.java @@ -26,6 +26,7 @@ import org.eclipse.cdt.core.envvar.IEnvironmentVariable; import org.eclipse.cdt.core.envvar.IEnvironmentVariableManager; import org.eclipse.cdt.core.language.settings.providers.ILanguageSettingsProvider; import org.eclipse.cdt.core.language.settings.providers.ILanguageSettingsProvidersKeeper; +import org.eclipse.cdt.core.language.settings.providers.IWorkingDirectoryTracker; import org.eclipse.cdt.core.model.CoreModel; import org.eclipse.cdt.core.settings.model.CIncludeFileEntry; import org.eclipse.cdt.core.settings.model.CIncludePathEntry; @@ -112,16 +113,40 @@ public class BuiltinSpecsDetectorTest extends BaseTestCase { @Override protected void execute() { super.execute(); - try { - Job.getJobManager().join(AbstractBuiltinSpecsDetector.JOB_FAMILY_BUILTIN_SPECS_DETECTOR, null); - } catch (Exception e) { - } + waitForProviderToFinish(); } protected boolean isExecuted() { return isExecuted; } } + /** + * Mock built-in specs detector which track how many times it was run. + */ + private class MockBuiltinSpecsDetectorWithRunCount extends DummyBuiltinSpecsDetector { + private int executedCount = 0; + + @Override + public void startup(ICConfigurationDescription cfgDescription, IWorkingDirectoryTracker cwdTracker) throws CoreException { + executedCount++; + super.startup(cfgDescription, cwdTracker); + } + @Override + public MockBuiltinSpecsDetectorEnvironmentChangeListener cloneShallow() throws CloneNotSupportedException { + MockBuiltinSpecsDetectorEnvironmentChangeListener clone = (MockBuiltinSpecsDetectorEnvironmentChangeListener) super.cloneShallow(); + return clone; + } + @Override + public MockBuiltinSpecsDetectorEnvironmentChangeListener clone() throws CloneNotSupportedException { + MockBuiltinSpecsDetectorEnvironmentChangeListener clone = (MockBuiltinSpecsDetectorEnvironmentChangeListener) super.clone(); + return clone; + } + + public int getExecutedCount() { + return executedCount; + } + } + /** * Mock built-in specs detector to test environment change functionality. */ @@ -198,12 +223,19 @@ public class BuiltinSpecsDetectorTest extends BaseTestCase { @Override protected void tearDown() throws Exception { + waitForProviderToFinish(); + super.tearDown(); + } + + /** + * Waits until all AbstractBuiltinSpecsDetector jobs are finished. + */ + private void waitForProviderToFinish() { try { Job.getJobManager().join(AbstractBuiltinSpecsDetector.JOB_FAMILY_BUILTIN_SPECS_DETECTOR, null); } catch (Exception e) { // ignore } - super.tearDown(); } /** @@ -564,10 +596,7 @@ public class BuiltinSpecsDetectorTest extends BaseTestCase { MockBuiltinSpecsDetectorEnvironmentChangeListener provider = new MockBuiltinSpecsDetectorEnvironmentChangeListener(); // register environment listener on configuration - note that provider is not included in the configuration provider.registerListener(cfgDescription); - try { - Job.getJobManager().join(AbstractBuiltinSpecsDetector.JOB_FAMILY_BUILTIN_SPECS_DETECTOR, null); - } catch (Exception e) { - } + waitForProviderToFinish(); assertEquals(true, provider.isExecuted()); assertEquals(null, provider.getSampleEnvVar()); // unset "isExecuted" flag @@ -591,10 +620,7 @@ public class BuiltinSpecsDetectorTest extends BaseTestCase { CoreModel.getDefault().setProjectDescription(project, prjDescriptionWritable); } - try { - Job.getJobManager().join(AbstractBuiltinSpecsDetector.JOB_FAMILY_BUILTIN_SPECS_DETECTOR, null); - } catch (Exception e) { - } + waitForProviderToFinish(); // check if provider got executed with new value assertEquals(true, provider.isExecuted()); assertEquals(ENV_SAMPLE_VALUE_1, provider.getSampleEnvVar()); @@ -622,10 +648,7 @@ public class BuiltinSpecsDetectorTest extends BaseTestCase { CoreModel.getDefault().setProjectDescription(project, prjDescriptionWritable); } - try { - Job.getJobManager().join(AbstractBuiltinSpecsDetector.JOB_FAMILY_BUILTIN_SPECS_DETECTOR, null); - } catch (Exception e) { - } + waitForProviderToFinish(); // check if provider got executed with new value assertEquals(true, provider.isExecuted()); assertEquals(ENV_SAMPLE_VALUE_2, provider.getSampleEnvVar()); @@ -653,10 +676,7 @@ public class BuiltinSpecsDetectorTest extends BaseTestCase { // Write to project description CProjectDescriptionManager.getInstance().setProjectDescription(project, prjDescriptionWritable); - try { - Job.getJobManager().join(AbstractBuiltinSpecsDetector.JOB_FAMILY_BUILTIN_SPECS_DETECTOR, null); - } catch (Exception e) { - } + waitForProviderToFinish(); // Check that provider got executed assertEquals(true, provider.isExecuted()); assertEquals(null, provider.getSampleEnvVar()); @@ -681,13 +701,10 @@ public class BuiltinSpecsDetectorTest extends BaseTestCase { assertEquals(false, provider.isExecuted()); assertEquals(null, provider.getSampleEnvVar()); - // Save project description including saving environment to the configuration + // Save project description including saving environment to the configuration CoreModel.getDefault().setProjectDescription(project, prjDescriptionWritable); } - try { - Job.getJobManager().join(AbstractBuiltinSpecsDetector.JOB_FAMILY_BUILTIN_SPECS_DETECTOR, null); - } catch (Exception e) { - } + waitForProviderToFinish(); // Check if the provider got executed { @@ -726,13 +743,10 @@ public class BuiltinSpecsDetectorTest extends BaseTestCase { assertEquals(false, provider.isExecuted()); assertEquals(ENV_SAMPLE_VALUE_1, provider.getSampleEnvVar()); - // Save project description including saving environment to the configuration + // Save project description including saving environment to the configuration CoreModel.getDefault().setProjectDescription(project, prjDescriptionWritable); } - try { - Job.getJobManager().join(AbstractBuiltinSpecsDetector.JOB_FAMILY_BUILTIN_SPECS_DETECTOR, null); - } catch (Exception e) { - } + waitForProviderToFinish(); // Check if the provider got executed { @@ -760,10 +774,7 @@ public class BuiltinSpecsDetectorTest extends BaseTestCase { MockBuiltinSpecsDetectorEnvironmentChangeListener provider = new MockBuiltinSpecsDetectorEnvironmentChangeListener(); // register environment listener on workspace provider.registerListener(null); - try { - Job.getJobManager().join(AbstractBuiltinSpecsDetector.JOB_FAMILY_BUILTIN_SPECS_DETECTOR, null); - } catch (Exception e) { - } + waitForProviderToFinish(); assertEquals(true, provider.isExecuted()); assertEquals(null, provider.getSampleEnvVar()); // unset "isExecuted" flag @@ -777,10 +788,7 @@ public class BuiltinSpecsDetectorTest extends BaseTestCase { vars.createVariable(ENV_SAMPLE, ENV_SAMPLE_VALUE_1); fUserSupplier.setWorkspaceEnvironment(vars); - try { - Job.getJobManager().join(AbstractBuiltinSpecsDetector.JOB_FAMILY_BUILTIN_SPECS_DETECTOR, null); - } catch (Exception e) { - } + waitForProviderToFinish(); // check if provider got executed with new value assertEquals(true, provider.isExecuted()); @@ -792,10 +800,7 @@ public class BuiltinSpecsDetectorTest extends BaseTestCase { vars.createVariable(ENV_SAMPLE, ENV_SAMPLE_VALUE_2); fUserSupplier.setWorkspaceEnvironment(vars); - try { - Job.getJobManager().join(AbstractBuiltinSpecsDetector.JOB_FAMILY_BUILTIN_SPECS_DETECTOR, null); - } catch (Exception e) { - } + waitForProviderToFinish(); // check if provider got executed with new value assertEquals(true, provider.isExecuted()); assertEquals(ENV_SAMPLE_VALUE_2, provider.getSampleEnvVar()); @@ -804,6 +809,85 @@ public class BuiltinSpecsDetectorTest extends BaseTestCase { provider.unregisterListener(); } + /** + * Test running a provider on compiler upgrades. + */ + public void testAbstractBuiltinSpecsDetector_CompilerUpgrade() throws Exception { + // Create test "compiler" + java.io.File compiler = new java.io.File("compiler"); + compiler.createNewFile(); + assertTrue(compiler.exists()); + String compilerPath = compiler.getAbsolutePath(); + + // Create provider + MockBuiltinSpecsDetectorWithRunCount provider = new MockBuiltinSpecsDetectorWithRunCount(); + provider.setCommand(compilerPath + " arg1"); + // register environment listener on workspace + provider.registerListener(null); + waitForProviderToFinish(); + assertEquals(1, provider.getExecutedCount()); + + // Check that an event doesn't trigger unnecessary rerun + provider.handleEvent(null); + waitForProviderToFinish(); + assertEquals(1, provider.getExecutedCount()); + + // "Upgrade" the "compiler" + compiler.setLastModified(compiler.lastModified() + 1); + + // Check that an event triggers rerun after upgrade + provider.handleEvent(null); + waitForProviderToFinish(); + assertEquals(2, provider.getExecutedCount()); + + // unregister listeners + provider.unregisterListener(); + } + + /** + * Test running a provider on compiler upgrades when the compiler is a symbolic link. + */ + public void testAbstractBuiltinSpecsDetector_CompilerUpgrade_SymbolicLink() throws Exception { + // do not test on systems where symbolic links are not supported + if (!ResourceHelper.isSymbolicLinkSupported()) { + return; + } + + // Create test "compiler" + java.io.File compiler = new java.io.File("compiler"); + compiler.createNewFile(); + assertTrue(compiler.exists()); + // Create symbolic link to the test compiler + ResourceHelper.createSymbolicLink(new Path("compilerLink"), new Path(compiler.getAbsolutePath())); + java.io.File compilerLink = new java.io.File("compilerLink"); + assertTrue(compilerLink.exists()); + String compilerLinkPath = compilerLink.getAbsolutePath(); + + // Create provider + MockBuiltinSpecsDetectorWithRunCount provider = new MockBuiltinSpecsDetectorWithRunCount(); + provider.setCommand(compilerLinkPath + " arg1"); + // register environment listener on workspace + provider.registerListener(null); + waitForProviderToFinish(); + assertEquals(1, provider.getExecutedCount()); + + // Check that an event doesn't trigger unnecessary rerun + provider.handleEvent(null); + waitForProviderToFinish(); + assertEquals(1, provider.getExecutedCount()); + + // "Upgrade" the "compiler" + compiler.setLastModified(compiler.lastModified() + 1); + + // Check that an event triggers rerun after upgrade + provider.handleEvent(null); + waitForProviderToFinish(); + assertEquals(2, provider.getExecutedCount()); + + // unregister listeners + provider.unregisterListener(); + } + /** * Check that entries get grouped by kinds by stock built-in specs detector. */ diff --git a/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/language/settings/providers/AbstractBuiltinSpecsDetector.java b/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/language/settings/providers/AbstractBuiltinSpecsDetector.java index 592e4cc8204..30d6e4fd1f9 100644 --- a/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/language/settings/providers/AbstractBuiltinSpecsDetector.java +++ b/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/language/settings/providers/AbstractBuiltinSpecsDetector.java @@ -51,6 +51,7 @@ import org.eclipse.cdt.internal.core.envvar.EnvironmentVariableManager; import org.eclipse.cdt.managedbuilder.core.ManagedBuilderCorePlugin; import org.eclipse.cdt.managedbuilder.internal.core.ManagedMakeMessages; import org.eclipse.cdt.utils.CommandLineUtil; +import org.eclipse.cdt.utils.PathUtil; import org.eclipse.cdt.utils.envvar.IEnvironmentChangeEvent; import org.eclipse.cdt.utils.envvar.IEnvironmentChangeListener; import org.eclipse.core.resources.IMarker; @@ -134,7 +135,7 @@ public abstract class AbstractBuiltinSpecsDetector extends AbstractLanguageSetti protected volatile boolean isExecuted = false; private static final int HASH_NOT_INITIALIZED = -1; - private int envPathHash = HASH_NOT_INITIALIZED; + private long envPathHash = HASH_NOT_INITIALIZED; private BuildRunnerHelper buildRunnerHelper; private SDMarkerGenerator markerGenerator = new SDMarkerGenerator(); @@ -402,6 +403,48 @@ public abstract class AbstractBuiltinSpecsDetector extends AbstractLanguageSetti super.shutdown(); } + /** + * Calculate hash code for the environment + */ + private long calculateEnvHash() { + String envPathValue = environmentMap.get(ENV_PATH); + long envHashNew = envPathValue != null ? envPathValue.hashCode() : 0; + + List languageIds = getLanguageScope(); + if (languageIds == null) { + languageIds = new ArrayList(1); + // "null" language indicates that the provider provides for any language + languageIds.add(null); + } + for (String languageId : languageIds) { + try { + String command = resolveCommand(languageId); + String[] cmdArray = CommandLineUtil.argumentsToArray(command); + if (cmdArray != null && cmdArray.length > 0) { + IPath location = new Path(cmdArray[0]); + if (!location.isAbsolute()) { + location = PathUtil.findProgramLocation(cmdArray[0], envPathValue); + } + if (location != null) { + java.io.File file = new java.io.File(location.toString()); + try { + // handles symbolic links as java.io.File.getCanonicalPath() resolves symlinks on UNIX + file = file.getCanonicalFile(); + } catch (IOException e) { + ManagedBuilderCorePlugin.log(e); + } + long lastModified = file.lastModified(); + envHashNew = 31*envHashNew + location.hashCode(); + envHashNew = 31*envHashNew + lastModified; + } + } + } catch (CoreException e) { + ManagedBuilderCorePlugin.log(e); + } + } + return envHashNew; + } + /** * This method does 2 related things: *
@@ -415,12 +458,12 @@ public abstract class AbstractBuiltinSpecsDetector extends AbstractLanguageSetti * @since 8.2 */ protected boolean validateEnvironment() { - String envPathValue = environmentMap.get(ENV_PATH); - int envPathValueHash = envPathValue != null ? envPathValue.hashCode() : 0; - if (envPathValueHash != envPathHash) { - envPathHash = envPathValueHash; + long envHashNew = calculateEnvHash(); + if (envHashNew != envPathHash) { + envPathHash = envHashNew; return false; } + return true; } @@ -824,7 +867,7 @@ public abstract class AbstractBuiltinSpecsDetector extends AbstractLanguageSetti /** * Determine additional options to pass to scanner discovery command. * These options are intended to come from the tool-chain. - * + * * @param languageId - language ID. * @return additional options to pass to scanner discovery command. * @@ -839,7 +882,7 @@ public abstract class AbstractBuiltinSpecsDetector extends AbstractLanguageSetti Element elementProvider = super.serializeAttributes(parentElement); elementProvider.setAttribute(ATTR_CONSOLE, Boolean.toString(isConsoleEnabled)); if (envPathHash != HASH_NOT_INITIALIZED) { - elementProvider.setAttribute(ATTR_ENV_HASH, Integer.toString(envPathHash)); + elementProvider.setAttribute(ATTR_ENV_HASH, Long.toString(envPathHash)); } return elementProvider; } @@ -857,7 +900,7 @@ public abstract class AbstractBuiltinSpecsDetector extends AbstractLanguageSetti String envPathHashStr = XmlUtil.determineAttributeValue(providerNode, ATTR_ENV_HASH); if (envPathHashStr != null) { try { - envPathHash = Integer.parseInt(envPathHashStr); + envPathHash = Long.parseLong(envPathHashStr); } catch (Exception e) { ManagedBuilderCorePlugin.log(new Status(IStatus.ERROR, ManagedBuilderCorePlugin.PLUGIN_ID, "Wrong integer format [" + envPathHashStr + "]", e)); //$NON-NLS-1$ //$NON-NLS-2$ } @@ -902,7 +945,7 @@ public abstract class AbstractBuiltinSpecsDetector extends AbstractLanguageSetti int result = super.hashCode(); result = prime * result + (isConsoleEnabled ? 1231 : 1237); result = prime * result + (isExecuted ? 1231 : 1237); - result = prime * result + envPathHash; + result = prime * result + new Long(envPathHash).hashCode(); return result; } diff --git a/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/language/settings/providers/ToolchainBuiltinSpecsDetector.java b/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/language/settings/providers/ToolchainBuiltinSpecsDetector.java index 0fbb79a93c3..906fa0de38b 100644 --- a/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/language/settings/providers/ToolchainBuiltinSpecsDetector.java +++ b/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/language/settings/providers/ToolchainBuiltinSpecsDetector.java @@ -58,6 +58,10 @@ public abstract class ToolchainBuiltinSpecsDetector extends AbstractBuiltinSpecs * This returns the first tool found. */ private ITool getTool(String languageId) { + if (languageId == null) { + return null; + } + if (currentCfgDescription == null) { ITool tool = toolMap.get(languageId); if (tool != null) { @@ -166,8 +170,9 @@ public abstract class ToolchainBuiltinSpecsDetector extends AbstractBuiltinSpecs optionValue = ""; //$NON-NLS-1$ String cmd = option.getCommand(); for (String value : values) { - if(!value.isEmpty() && !value.equals(EMPTY_QUOTED_STRING)) + if(!value.isEmpty() && !value.equals(EMPTY_QUOTED_STRING)) { optionValue = optionValue + cmd + value + ' '; + } } } break;