bug 392416: Rebuild builtin GCC compiler settings when GCC version changes

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 {
+			waitForProviderToFinish();
-				Job.getJobManager().join(AbstractBuiltinSpecsDetector.JOB_FAMILY_BUILTIN_SPECS_DETECTOR, null);
-			} catch (Exception e) {
-			}
 		}
 		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 {
+		waitForProviderToFinish();
-			Job.getJobManager().join(AbstractBuiltinSpecsDetector.JOB_FAMILY_BUILTIN_SPECS_DETECTOR, null);
-		} catch (Exception e) {
-		}
 		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 {
+		waitForProviderToFinish();
-			Job.getJobManager().join(AbstractBuiltinSpecsDetector.JOB_FAMILY_BUILTIN_SPECS_DETECTOR, null);
-		} catch (Exception e) {
-		}
 		// 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 {
+		waitForProviderToFinish();
-			Job.getJobManager().join(AbstractBuiltinSpecsDetector.JOB_FAMILY_BUILTIN_SPECS_DETECTOR, null);
-		} catch (Exception e) {
-		}
 		// 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 {
+			waitForProviderToFinish();
-				Job.getJobManager().join(AbstractBuiltinSpecsDetector.JOB_FAMILY_BUILTIN_SPECS_DETECTOR, null);
-			} catch (Exception e) {
-			}
 			// 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 {
+		waitForProviderToFinish();
-			Job.getJobManager().join(AbstractBuiltinSpecsDetector.JOB_FAMILY_BUILTIN_SPECS_DETECTOR, null);
-		} catch (Exception e) {
-		}
 
 		// 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 {
+		waitForProviderToFinish();
-			Job.getJobManager().join(AbstractBuiltinSpecsDetector.JOB_FAMILY_BUILTIN_SPECS_DETECTOR, null);
-		} catch (Exception e) {
-		}
 
 		// 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 {
+		waitForProviderToFinish();
-			Job.getJobManager().join(AbstractBuiltinSpecsDetector.JOB_FAMILY_BUILTIN_SPECS_DETECTOR, null);
-		} catch (Exception e) {
-		}
 		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 {
+		waitForProviderToFinish();
-			Job.getJobManager().join(AbstractBuiltinSpecsDetector.JOB_FAMILY_BUILTIN_SPECS_DETECTOR, null);
-		} catch (Exception e) {
-		}
 
 		// 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 {
+		waitForProviderToFinish();
-			Job.getJobManager().join(AbstractBuiltinSpecsDetector.JOB_FAMILY_BUILTIN_SPECS_DETECTOR, null);
-		} catch (Exception e) {
-		}
 		// 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.
 	 */

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<String> languageIds = getLanguageScope();
+		if (languageIds == null) {
+			languageIds = new ArrayList<String>(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:
 	 * <br>
@@ -415,12 +458,12 @@ public abstract class AbstractBuiltinSpecsDetector extends AbstractLanguageSetti
 	 * @since 8.2
 	 */
 	protected boolean validateEnvironment() {
-		String envPathValue = environmentMap.get(ENV_PATH);
+		long envHashNew = calculateEnvHash();
-		int envPathValueHash = envPathValue != null ? envPathValue.hashCode() : 0;
+		if (envHashNew != envPathHash) {
-		if (envPathValueHash != envPathHash) {
+			envPathHash = envHashNew;
-			envPathHash = envPathValueHash;
 			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;
 	}
 

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;