bug 384520: New Project Wizard ignores $PATH from preferences, won't show toolchain

4 files changed, 87 insertions, 53 deletions

diff --git a/build/org.eclipse.cdt.managedbuilder.gnu.ui/src/org/eclipse/cdt/managedbuilder/gnu/cygwin/CygwinPathResolver.java b/build/org.eclipse.cdt.managedbuilder.gnu.ui/src/org/eclipse/cdt/managedbuilder/gnu/cygwin/CygwinPathResolver.java
index 4460a4d7c76..8f2b5b7e781 100644
--- a/build/org.eclipse.cdt.managedbuilder.gnu.ui/src/org/eclipse/cdt/managedbuilder/gnu/cygwin/CygwinPathResolver.java
+++ b/build/org.eclipse.cdt.managedbuilder.gnu.ui/src/org/eclipse/cdt/managedbuilder/gnu/cygwin/CygwinPathResolver.java
@@ -19,7 +19,9 @@ import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.util.ArrayList;
 
+import org.eclipse.cdt.core.CCorePlugin;
 import org.eclipse.cdt.core.envvar.IEnvironmentVariable;
+import org.eclipse.cdt.core.settings.model.util.CDataUtil;
 import org.eclipse.cdt.managedbuilder.core.IBuildPathResolver;
 import org.eclipse.cdt.managedbuilder.core.IConfiguration;
 import org.eclipse.cdt.managedbuilder.core.ManagedBuildManager;
@@ -29,7 +31,6 @@ import org.eclipse.cdt.utils.WindowsRegistry;
 import org.eclipse.cdt.utils.spawner.ProcessFactory;
 import org.eclipse.core.runtime.IPath;
 
-
 /**
  * @noextend This class is not intended to be subclassed by clients.
  */
@@ -58,15 +59,13 @@ public class CygwinPathResolver implements IBuildPathResolver {
 	private static final String MINGW_SPECIAL = "mingw ";    //$NON-NLS-1$
 	private static final String CYGWIN_SPECIAL = "cygwin ";    //$NON-NLS-1$
 
-	private static boolean checked = false;
+	private static String envPathValueCached = null;
 	private static String binCygwin  = null;
 	private static String rootCygwin = null;
 	private static String etcCygwin = null;
 
 	@Override
-	public String[] resolveBuildPaths(int pathType, String variableName,
-			String variableValue, IConfiguration configuration) {
-
+	public String[] resolveBuildPaths(int pathType, String variableName, String variableValue, IConfiguration configuration) {
 		if(!isWindows()) {
 			return variableValue.split(DELIMITER_UNIX);
 		} else if(isMinGW(configuration)) {
@@ -92,25 +91,34 @@ public class CygwinPathResolver implements IBuildPathResolver {
 
 	/**
 	 * returns "/etc" path in Windows format
+	 *
+	 * If you use this do not cache results to ensure user preferences are accounted for.
+	 * Please rely on internal caching.
 	 */
 	public static String getEtcPath() {
-		if (!checked) findPaths();
+		findPaths();
 		return etcCygwin;
 	}
 
 	/**
 	 * returns "/usr/bin" path in Windows format
+	 *
+	 * If you use this do not cache results to ensure user preferences are accounted for.
+	 * Please rely on internal caching.
 	 */
 	public static String getBinPath() {
-		if (!checked) findPaths();
+		findPaths();
 		return binCygwin;
 	}
 
 	/**
 	 * returns Cygwin root ("/") path in Windows format
+	 *
+	 * If you use this do not cache results to ensure user preferences are accounted for.
+	 * Please rely on internal caching.
 	 */
 	public static String getRootPath() {
-		if (!checked) findPaths();
+		findPaths();
 		return rootCygwin;
 	}
 
@@ -164,11 +172,11 @@ public class CygwinPathResolver implements IBuildPathResolver {
 	 *
 	 * @return The absolute path to cygwin's root or null if not found
 	 */
-	private static String findRoot() {
+	private static String findRoot(String paths) {
 		String rootValue = null;
 
 		// 1. Look in PATH values. Look for bin\cygwin1.dll
-		IPath location = PathUtil.findProgramLocation("cygwin1.dll"); //$NON-NLS-1$
+		IPath location = PathUtil.findProgramLocation("cygwin1.dll", paths); //$NON-NLS-1$
 		if (location!=null) {
 			rootValue = location.removeLastSegments(2).toOSString();
 		}
@@ -202,16 +210,25 @@ public class CygwinPathResolver implements IBuildPathResolver {
 	}
 
 	/**
-	 * Finds Cygwin's paths and sets corresponding properties
+	 * Finds Cygwin's paths and sets corresponding properties.
 	 */
 	private static synchronized void findPaths() {
-		if (checked) return;
+		if (!isWindows()) {
+			return;
+		}
+
+		IEnvironmentVariable varPath = CCorePlugin.getDefault().getBuildEnvironmentManager().getVariable("PATH", null, true); //$NON-NLS-1$
+		String envPathValue = varPath != null ? varPath.getValue() : null;
+
+		if (CDataUtil.objectsEqual(envPathValue, envPathValueCached)) {
+			return;
+		}
+
 		etcCygwin  = null;
 		binCygwin  = null;
 		rootCygwin = null;
-		if (!isWindows()) return;
 
-		rootCygwin = findRoot();
+		rootCygwin = findRoot(envPathValue);
 
 		// 1. Try to find the paths by appending the patterns to the root dir
 		etcCygwin = getValueFromRoot(ETCPATTERN);
@@ -225,7 +242,7 @@ public class CygwinPathResolver implements IBuildPathResolver {
 		if(binCygwin == null)
 			binCygwin = readValueFromRegistry(REGISTRY_KEY_MOUNTS + BINPATTERN, PATH_NAME);
 
-		checked = true;
+		envPathValueCached = envPathValue;
 	}
 
 	private static String[] exec(String cmd, IConfiguration cfg) {
diff --git a/build/org.eclipse.cdt.managedbuilder.gnu.ui/src/org/eclipse/cdt/managedbuilder/gnu/cygwin/IsGnuCygwinToolChainSupported.java b/build/org.eclipse.cdt.managedbuilder.gnu.ui/src/org/eclipse/cdt/managedbuilder/gnu/cygwin/IsGnuCygwinToolChainSupported.java
index cf7d2816c2c..a2d2050338a 100644
--- a/build/org.eclipse.cdt.managedbuilder.gnu.ui/src/org/eclipse/cdt/managedbuilder/gnu/cygwin/IsGnuCygwinToolChainSupported.java
+++ b/build/org.eclipse.cdt.managedbuilder.gnu.ui/src/org/eclipse/cdt/managedbuilder/gnu/cygwin/IsGnuCygwinToolChainSupported.java
@@ -16,6 +16,7 @@ import java.io.FileNotFoundException;
 import java.io.FileReader;
 import java.io.IOException;
 
+import org.eclipse.cdt.core.settings.model.util.CDataUtil;
 import org.eclipse.cdt.managedbuilder.core.IManagedIsToolChainSupported;
 import org.eclipse.cdt.managedbuilder.core.IToolChain;
 import org.osgi.framework.Version;
@@ -33,24 +34,23 @@ import org.osgi.framework.Version;
  * @noextend This class is not intended to be subclassed by clients.
  */
 public class IsGnuCygwinToolChainSupported implements IManagedIsToolChainSupported {
-	static final String[] CHECKED_NAMES = {"gcc", "binutils", "make"};  //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
+	private static final String[] CHECKED_NAMES = {"gcc", "binutils", "make"};  //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
 
-	static boolean suppChecked = false;
-	static boolean toolchainIsSupported = false;
+	private static String etcCygwinCached = null;
+	private static boolean toolchainIsSupported = false;
 
 	/**
 	 * @since 8.0
 	 */
 	@Override
 	public boolean isSupported(IToolChain toolChain, Version version, String instance) {
-		if (suppChecked) return toolchainIsSupported;
-
 		String etcCygwin = CygwinPathResolver.getEtcPath();
-		if (etcCygwin != null) {
-			toolchainIsSupported = arePackagesInstalled(etcCygwin);
+		if (CDataUtil.objectsEqual(etcCygwin, etcCygwinCached)) {
+			return toolchainIsSupported;
 		}
 
-		suppChecked = true;
+		toolchainIsSupported = etcCygwin != null && arePackagesInstalled(etcCygwin);
+		etcCygwinCached = etcCygwin;
 
 		return toolchainIsSupported;
 	}
diff --git a/build/org.eclipse.cdt.managedbuilder.gnu.ui/src/org/eclipse/cdt/managedbuilder/gnu/mingw/MingwEnvironmentVariableSupplier.java b/build/org.eclipse.cdt.managedbuilder.gnu.ui/src/org/eclipse/cdt/managedbuilder/gnu/mingw/MingwEnvironmentVariableSupplier.java
index 65313653e39..859cce141e7 100644
--- a/build/org.eclipse.cdt.managedbuilder.gnu.ui/src/org/eclipse/cdt/managedbuilder/gnu/mingw/MingwEnvironmentVariableSupplier.java
+++ b/build/org.eclipse.cdt.managedbuilder.gnu.ui/src/org/eclipse/cdt/managedbuilder/gnu/mingw/MingwEnvironmentVariableSupplier.java
@@ -11,6 +11,9 @@
 
 package org.eclipse.cdt.managedbuilder.gnu.mingw;
 
+import org.eclipse.cdt.core.CCorePlugin;
+import org.eclipse.cdt.core.envvar.IEnvironmentVariable;
+import org.eclipse.cdt.core.settings.model.util.CDataUtil;
 import org.eclipse.cdt.managedbuilder.core.IConfiguration;
 import org.eclipse.cdt.managedbuilder.envvar.IBuildEnvironmentVariable;
 import org.eclipse.cdt.managedbuilder.envvar.IConfigurationEnvironmentVariableSupplier;
@@ -26,7 +29,8 @@ import org.eclipse.core.runtime.Platform;
  * @noextend This class is not intended to be subclassed by clients.
  */
 public class MingwEnvironmentVariableSupplier implements IConfigurationEnvironmentVariableSupplier {
-	private static boolean checked = false;
+	private static String envPathValueCached = null;
+	private static String envMingwHomeValueCached = null;
 	private static IPath binDir = null;
 
 	private static class MingwBuildEnvironmentVariable implements IBuildEnvironmentVariable {
@@ -63,41 +67,61 @@ public class MingwEnvironmentVariableSupplier implements IConfigurationEnvironme
 
 	private IBuildEnvironmentVariable path;
 
+	/**
+	 * @return location of $MINGW_HOME/bin folder on the file-system.
+	 *
+	 * If you use this do not cache results to ensure user preferences are accounted for.
+	 * Please rely on internal caching.
+	 */
 	public static IPath getBinDir() {
-		if (!checked) {
-			binDir = findBinDir();
-			checked = true;
+		IEnvironmentVariable varPath = CCorePlugin.getDefault().getBuildEnvironmentManager().getVariable("PATH", null, true); //$NON-NLS-1$
+		String envPathValue = varPath != null ? varPath.getValue() : null;
+		IEnvironmentVariable varMingwHome = CCorePlugin.getDefault().getBuildEnvironmentManager().getVariable("MINGW_HOME", null, true); //$NON-NLS-1$
+		String envMingwHomeValue = varMingwHome != null ? varMingwHome.getValue() : null;
+
+		if (CDataUtil.objectsEqual(envPathValue, envPathValueCached) && CDataUtil.objectsEqual(envMingwHomeValue, envMingwHomeValueCached)) {
+			return binDir;
 		}
-		return binDir;
-	}
 
-	private static IPath findBinDir() {
-		// Try in MinGW home
-		String mingwHome = System.getenv("MINGW_HOME"); //$NON-NLS-1$
-		IPath mingwBinDir = new Path(mingwHome + "\\bin"); //$NON-NLS-1$
-		if (mingwBinDir.toFile().isDirectory())
-			return mingwBinDir;
+		envPathValueCached = envPathValue;
+		envMingwHomeValueCached = envMingwHomeValue;
+		binDir = null;
+
+		// Check $MINGW_HOME
+		IPath mingwBinDir = new Path(envMingwHomeValue + "\\bin"); //$NON-NLS-1$
+		if (mingwBinDir.toFile().isDirectory()) {
+			binDir = mingwBinDir;
+			return binDir;
+		}
 
 		// Try the mingw directory in the platform install directory
 		// CDT distributions like Wascana may distribute MinGW like that
 		IPath installPath = new Path(Platform.getInstallLocation().getURL().getFile());
 		mingwBinDir = installPath.append("mingw\\bin"); //$NON-NLS-1$
-		if (mingwBinDir.toFile().isDirectory())
-			return mingwBinDir;
+		if (mingwBinDir.toFile().isDirectory()) {
+			binDir = mingwBinDir;
+			return binDir;
+		}
 
 		// Look in PATH values. Look for mingw32-gcc.exe
 		// TODO: Since this dir is already in the PATH, why are we adding it here?
 		// This is really only to support isToolchainAvail. Must be a better way.
-		IPath gccLoc = PathUtil.findProgramLocation("mingw32-gcc.exe"); //$NON-NLS-1$
-		if (gccLoc != null)
-			return gccLoc.removeLastSegments(1);
+		// AG: Because otherwise the toolchain won't be shown in the list of "supported" toolchains in UI
+		// when MinGW installed in custom location even if it is in the PATH
+		IPath gccLoc = PathUtil.findProgramLocation("mingw32-gcc.exe", envPathValue); //$NON-NLS-1$
+		if (gccLoc != null) {
+			binDir = gccLoc.removeLastSegments(1);
+			return binDir;
+		}
 
 		// Try the default MinGW install dir
 		mingwBinDir = new Path("C:\\MinGW\\bin"); //$NON-NLS-1$
-		if (mingwBinDir.toFile().isDirectory())
-			return mingwBinDir;
+		if (mingwBinDir.toFile().isDirectory()) {
+			binDir = mingwBinDir;
+			return binDir;
+		}
 
-		return null;
+		return binDir;
 	}
 
 	public static IPath getMsysBinDir() {
@@ -107,7 +131,7 @@ public class MingwEnvironmentVariableSupplier implements IConfigurationEnvironme
 		if (msysBinPath.toFile().isDirectory())
 			return msysBinPath;
 
-		String mingwHome = System.getenv("MINGW_HOME"); //$NON-NLS-1$
+		String mingwHome = envMingwHomeValueCached;
 		if (mingwHome != null) {
 			msysBinPath = new Path(mingwHome + "\\msys\\1.0\\bin"); //$NON-NLS-1$
 			if (msysBinPath.toFile().isDirectory())
diff --git a/build/org.eclipse.cdt.managedbuilder.gnu.ui/src/org/eclipse/cdt/managedbuilder/gnu/mingw/MingwIsToolChainSupported.java b/build/org.eclipse.cdt.managedbuilder.gnu.ui/src/org/eclipse/cdt/managedbuilder/gnu/mingw/MingwIsToolChainSupported.java
index a92364c870d..da6312958aa 100644
--- a/build/org.eclipse.cdt.managedbuilder.gnu.ui/src/org/eclipse/cdt/managedbuilder/gnu/mingw/MingwIsToolChainSupported.java
+++ b/build/org.eclipse.cdt.managedbuilder.gnu.ui/src/org/eclipse/cdt/managedbuilder/gnu/mingw/MingwIsToolChainSupported.java
@@ -21,17 +21,10 @@ import org.osgi.framework.Version;
  * @noextend This class is not intended to be subclassed by clients.
  */
 public class MingwIsToolChainSupported implements IManagedIsToolChainSupported {
-
-	private final boolean supported;
-
-	public MingwIsToolChainSupported() {
-		// Only supported if we can find the mingw bin dir to run the compiler
-		supported = MingwEnvironmentVariableSupplier.getBinDir() != null;
-	}
-
 	@Override
 	public boolean isSupported(IToolChain toolChain, Version version, String instance) {
-		return supported;
+		// Only supported if we can find the mingw bin dir to run the compiler
+		return MingwEnvironmentVariableSupplier.getBinDir() != null;
 	}
 
 }