From c03b1c1796a2ee7e4d11d78dc7f371616e5566a1 Mon Sep 17 00:00:00 2001 From: Andrew Gvozdev Date: Wed, 29 Sep 2010 04:37:52 +0000 Subject: [PATCH] bug 308042: Spawner messages are too cryptic to be useful to a user --- .../core/ExternalBuildRunner.java | 51 +++-- .../internal/core/PluginResources.properties | 3 + .../gnu/cygwin/CygwinPathResolver.java | 20 +- .../MingwEnvironmentVariableSupplier.java | 18 +- .../cdt/utils/FindProgramLocationTest.java | 183 ++++++++++++++++++ .../core/suite/AutomatedIntegrationSuite.java | 2 + core/org.eclipse.cdt.core/plugin.xml | 1 + .../utils/org/eclipse/cdt/utils/PathUtil.java | 70 +++++++ 8 files changed, 299 insertions(+), 49 deletions(-) create mode 100644 core/org.eclipse.cdt.core.tests/misc/org/eclipse/cdt/utils/FindProgramLocationTest.java diff --git a/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/core/ExternalBuildRunner.java b/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/core/ExternalBuildRunner.java index 7b8594c20bd..0da61536ecb 100644 --- a/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/core/ExternalBuildRunner.java +++ b/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/core/ExternalBuildRunner.java @@ -44,6 +44,7 @@ import org.eclipse.cdt.managedbuilder.macros.BuildMacroException; import org.eclipse.cdt.managedbuilder.macros.IBuildMacroProvider; import org.eclipse.cdt.newmake.internal.core.StreamMonitor; import org.eclipse.cdt.utils.CommandLineUtil; +import org.eclipse.cdt.utils.PathUtil; import org.eclipse.core.resources.IMarker; import org.eclipse.core.resources.IProject; import org.eclipse.core.resources.IResource; @@ -68,6 +69,7 @@ public class ExternalBuildRunner implements IBuildRunner { private static final String TYPE_INC = "ManagedMakeBuider.type.incremental"; //$NON-NLS-1$ private static final String CONSOLE_HEADER = "ManagedMakeBuilder.message.console.header"; //$NON-NLS-1$ private static final String WARNING_UNSUPPORTED_CONFIGURATION = "ManagedMakeBuilder.warning.unsupported.configuration"; //$NON-NLS-1$ + private static final String NEWLINE = System.getProperty("line.separator", "\n"); //$NON-NLS-1$ //$NON-NLS-2$ public boolean invokeBuild(int kind, IProject project, IConfiguration configuration, IBuilder builder, IConsole console, IMarkerGenerator markerGenerator, @@ -106,16 +108,15 @@ public class ExternalBuildRunner implements IBuildRunner { consoleHeader[1] = configuration.getName(); consoleHeader[2] = project.getName(); - buf.append(System.getProperty("line.separator", "\n")); //$NON-NLS-1$ //$NON-NLS-2$ - buf.append(ManagedMakeMessages.getFormattedString(CONSOLE_HEADER, consoleHeader)); - buf.append(System.getProperty("line.separator", "\n")); //$NON-NLS-1$ //$NON-NLS-2$ - buf.append(System.getProperty("line.separator", "\n")); //$NON-NLS-1$ //$NON-NLS-2$ + buf.append(NEWLINE); + buf.append(ManagedMakeMessages.getFormattedString(CONSOLE_HEADER, consoleHeader)).append(NEWLINE); + buf.append(NEWLINE); if(!configuration.isSupported()){ - buf.append(ManagedMakeMessages.getFormattedString(WARNING_UNSUPPORTED_CONFIGURATION, - new String[] { configuration.getName(), configuration.getToolChain().getName() })); - buf.append(System.getProperty("line.separator", "\n")); //$NON-NLS-1$ //$NON-NLS-2$ - buf.append(System.getProperty("line.separator", "\n")); //$NON-NLS-1$ //$NON-NLS-2$ + String unsupportedToolchainMsg = ManagedMakeMessages.getFormattedString(WARNING_UNSUPPORTED_CONFIGURATION, + new String[] { configuration.getName(), configuration.getToolChain().getName() }); + buf.append(unsupportedToolchainMsg).append(NEWLINE); + buf.append(NEWLINE); } cos.write(buf.toString().getBytes()); cos.flush(); @@ -140,7 +141,8 @@ public class ExternalBuildRunner implements IBuildRunner { launcher.showCommand(true); // Set the environment - String[] env = getEnvStrings(getEnvironment(builder)); + Map envMap = getEnvironment(builder); + String[] env = getEnvStrings(envMap); String[] buildArguments = targets; String[] newArgs = CommandLineUtil.argumentsToArray(builder.getBuildArguments()); @@ -181,31 +183,38 @@ public class ExternalBuildRunner implements IBuildRunner { try { // Do not allow the cancel of the refresh, since the builder is external // to Eclipse, files may have been created/modified and we will be out-of-sync. - // The caveat is for hugue projects, it may take sometimes at every build. + // The caveat is for huge projects, it may take sometimes at every build. // TODO should only refresh output folders project.refreshLocal(IResource.DEPTH_INFINITE, null); } catch (CoreException e) { } } else { + buf = new StringBuffer(launcher.getCommandLine()).append(NEWLINE); errMsg = launcher.getErrorMessage(); } project.setSessionProperty(qName, !monitor.isCanceled() && !isClean ? new Integer(streamMon.getWorkDone()) : null); if (errMsg != null) { - buf = new StringBuffer(buildCommand.toString() + " "); //$NON-NLS-1$ - for (String arg : buildArguments) { - buf.append(arg); - buf.append(' '); + // Launching failed, trying to figure out possible cause + String errorPrefix = ManagedMakeMessages.getResourceString("ManagedMakeBuilder.error.prefix"); //$NON-NLS-1$ + String buildCommandStr = buildCommand.toString(); + if (PathUtil.findProgramLocation(buildCommandStr)==null) { + buf.append(errMsg).append(NEWLINE); + errMsg = ManagedMakeMessages.getFormattedString("ManagedMakeBuilder.message.program.not.in.path", buildCommandStr); //$NON-NLS-1$ + buf.append(errorPrefix).append(errMsg).append(NEWLINE); + buf.append(NEWLINE); + buf.append("PATH=["+envMap.get("PATH")+"]").append(NEWLINE); //$NON-NLS-1$//$NON-NLS-2$//$NON-NLS-3$ + } else { + buf.append(errorPrefix).append(errMsg).append(NEWLINE); } - - String errorDesc = ManagedMakeMessages.getFormattedString("MakeBuilder.buildError", buf.toString()); //$NON-NLS-1$ - buf = new StringBuffer(errorDesc); - buf.append(System.getProperty("line.separator", "\n")); //$NON-NLS-1$ //$NON-NLS-2$ - buf.append("(").append(errMsg).append(")"); //$NON-NLS-1$ //$NON-NLS-2$ - cos.write(buf.toString().getBytes()); - cos.flush(); + consoleErr.write(buf.toString().getBytes()); + consoleErr.flush(); } + + buf = new StringBuffer(NEWLINE); + buf.append(ManagedMakeMessages.getResourceString("ManagedMakeBuilder.message.build.finished")).append(NEWLINE); //$NON-NLS-1$ + consoleOut.write(buf.toString().getBytes()); stdout.close(); stderr.close(); diff --git a/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/core/PluginResources.properties b/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/core/PluginResources.properties index 551b5a0743e..abadc94b1c7 100644 --- a/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/core/PluginResources.properties +++ b/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/core/PluginResources.properties @@ -32,10 +32,13 @@ ManagedMakeBuilder.message.internal.builder.error = Build failed: Internal build ManagedMakeBuilder.message.stopped.error=Build error occurred, build is stopped ManagedMakeBuilder.message.clean.deleting.output=Removing build artifacts from {0} ManagedMakeBuilder.message.clean.build.clean=Trying a make clean in {0} +ManagedMakeBuilder.message.program.not.in.path=Program "{0}" is not found in PATH +ManagedMakeBuilder.message.build.finished=**** Build Finished **** ManagedMakeBuilder.type.clean = Clean-only build ManagedMakeBuider.type.incremental = Build ManagedMakeBuider.type.rebuild = Rebuild ManagedMakeBuilder.warning.unsupported.configuration=**** WARNING: The "{0}" Configuration may not build ****\n**** because it uses the "{1}" ****\n**** tool-chain that is unsupported on this system. ****\n\n**** Attempting to build... **** +ManagedMakeBuilder.error.prefix=Error: # Option exception messages Option.error.bad_value_type=Bad value for type 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 c866414fcf1..4d97d2ac5a0 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 @@ -24,9 +24,10 @@ import org.eclipse.cdt.managedbuilder.core.IBuildPathResolver; import org.eclipse.cdt.managedbuilder.core.IConfiguration; import org.eclipse.cdt.managedbuilder.core.ManagedBuildManager; import org.eclipse.cdt.managedbuilder.gnu.ui.GnuUIPlugin; +import org.eclipse.cdt.utils.PathUtil; import org.eclipse.cdt.utils.WindowsRegistry; import org.eclipse.cdt.utils.spawner.ProcessFactory; -import org.eclipse.core.runtime.Path; +import org.eclipse.core.runtime.IPath; /** @@ -167,20 +168,11 @@ public class CygwinPathResolver implements IBuildPathResolver { String rootValue = null; // 1. Look in PATH values. Look for bin\cygwin1.dll - String pathVariable = System.getenv("PATH"); //$NON-NLS-1$ - String[] paths = pathVariable.split(";"); //$NON-NLS-1$ - for (String pathStr : paths) { - // If there is a trailing / or \, remove it - if ((pathStr.endsWith("\\") || pathStr.endsWith("/")) && pathStr.length() > 1) //$NON-NLS-1$ //$NON-NLS-2$ - pathStr = pathStr.substring(0, pathStr.length() - 1); - - Path pathFile = new Path(pathStr + "\\cygwin1.dll"); //$NON-NLS-1$ - if (pathFile.toFile().exists()) { - rootValue = pathFile.removeLastSegments(2).toOSString(); - break; - } + IPath location = PathUtil.findProgramLocation("cygwin1.dll"); //$NON-NLS-1$ + if (location!=null) { + rootValue = location.removeLastSegments(2).toOSString(); } - + // 2. Try to find the root dir in SOFTWARE\Cygwin\setup if(rootValue == null) { rootValue = readValueFromRegistry(REGISTRY_KEY_SETUP, "rootdir"); //$NON-NLS-1$ 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 c42d1100706..ad7dd6ebb0a 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,12 +11,11 @@ package org.eclipse.cdt.managedbuilder.gnu.mingw; -import java.io.File; - import org.eclipse.cdt.managedbuilder.core.IConfiguration; import org.eclipse.cdt.managedbuilder.envvar.IBuildEnvironmentVariable; import org.eclipse.cdt.managedbuilder.envvar.IConfigurationEnvironmentVariableSupplier; import org.eclipse.cdt.managedbuilder.envvar.IEnvironmentVariableProvider; +import org.eclipse.cdt.utils.PathUtil; import org.eclipse.cdt.utils.WindowsRegistry; import org.eclipse.core.runtime.IPath; import org.eclipse.core.runtime.Path; @@ -88,18 +87,9 @@ public class MingwEnvironmentVariableSupplier implements // 3. Look in PATH values. Look for mingw32-gcc.exe if (binDir == null) { - String pathVariable = System.getenv("PATH"); //$NON-NLS-1$ - String[] paths = pathVariable.split(";"); //$NON-NLS-1$ - for (String pathStr : paths) { - // If there is a trailing / or \, remove it - if ((pathStr.endsWith("\\") || pathStr.endsWith("/")) && pathStr.length() > 1) //$NON-NLS-1$ //$NON-NLS-2$ - pathStr = pathStr.substring(0, pathStr.length() - 1); - - File pathFile = new File(pathStr + "\\mingw32-gcc.exe"); //$NON-NLS-1$ - if (pathFile.exists()) { - binDir = new Path(pathStr); - break; - } + IPath location = PathUtil.findProgramLocation("mingw32-gcc.exe"); //$NON-NLS-1$ + if (location!=null) { + binDir = location.removeLastSegments(1); } } diff --git a/core/org.eclipse.cdt.core.tests/misc/org/eclipse/cdt/utils/FindProgramLocationTest.java b/core/org.eclipse.cdt.core.tests/misc/org/eclipse/cdt/utils/FindProgramLocationTest.java new file mode 100644 index 00000000000..646a12292a4 --- /dev/null +++ b/core/org.eclipse.cdt.core.tests/misc/org/eclipse/cdt/utils/FindProgramLocationTest.java @@ -0,0 +1,183 @@ +/******************************************************************************* + * Copyright (c) 2010, 2010 Andrew Gvozdev and others. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Andrew Gvozdev (Quoin Inc.) - initial API and implementation + *******************************************************************************/ +package org.eclipse.cdt.utils; + +import java.io.File; +import java.io.IOException; + +import junit.framework.Test; +import junit.framework.TestCase; +import junit.framework.TestSuite; + +import org.eclipse.cdt.core.testplugin.ResourceHelper; +import org.eclipse.core.runtime.CoreException; +import org.eclipse.core.runtime.IPath; +import org.eclipse.core.runtime.Path; +import org.eclipse.core.runtime.Platform; + +public class FindProgramLocationTest extends TestCase { + + private static final String PATH_SEPARATOR = File.pathSeparator; + + public static Test suite() { + return new TestSuite(FindProgramLocationTest.class); + } + + @Override + protected void tearDown() throws Exception { + ResourceHelper.cleanUp(); + } + + public void testCornerCases() throws CoreException, IOException { + assertNull(PathUtil.findProgramLocation("", "")); + assertNull(PathUtil.findProgramLocation("prog", "")); + assertNull(PathUtil.findProgramLocation("prog", PATH_SEPARATOR)); + assertNull(PathUtil.findProgramLocation("prog", "x"+PATH_SEPARATOR)); + assertNull(PathUtil.findProgramLocation("prog", PATH_SEPARATOR+"x")); + assertNull(PathUtil.findProgramLocation("prog", PATH_SEPARATOR+PATH_SEPARATOR)); + assertNull(PathUtil.findProgramLocation("prog", PATH_SEPARATOR+"x"+PATH_SEPARATOR)); + } + + public void testFind() throws CoreException, IOException { + String name1 = "file1"; + String name2 = "file2"; + String name3 = "file3"; + String nameA = "fileA"; + + // Create some folders and files + IPath dir1 = ResourceHelper.createTemporaryFolder(); + IPath dir2 = ResourceHelper.createTemporaryFolder(); + IPath dir3 = ResourceHelper.createTemporaryFolder(); + + IPath filePath1 = new Path(dir1 + File.separator + name1); + IPath filePath2 = new Path(dir2 + File.separator + name2); + IPath filePath3 = new Path(dir3 + File.separator + name3); + + IPath filePath2A = new Path(dir2 + File.separator + nameA); + IPath filePath3A = new Path(dir3 + File.separator + nameA); + + File file1 = filePath1.toFile(); + file1.createNewFile(); + assertTrue(file1.exists()); + + File file2 = filePath2.toFile(); + file2.createNewFile(); + assertTrue(file2.exists()); + + File file3 = filePath3.toFile(); + file3.createNewFile(); + assertTrue(file3.exists()); + + File file2A = filePath2A.toFile(); + file2A.createNewFile(); + assertTrue(file2A.exists()); + + File file3A = filePath3A.toFile(); + file3A.createNewFile(); + assertTrue(file3A.exists()); + + // sample $PATH + String path123 = dir1 + PATH_SEPARATOR + dir2 + PATH_SEPARATOR + dir3; + + { + // non-existing file + IPath actual = PathUtil.findProgramLocation("non-existing", path123); + assertEquals(null, actual); + } + { + // file in the first path + IPath actual = PathUtil.findProgramLocation(name1, path123); + assertEquals(filePath1, actual); + } + { + // file in the middle path + IPath actual = PathUtil.findProgramLocation(name2, path123); + assertEquals(filePath2, actual); + } + { + // file in the last path + IPath actual = PathUtil.findProgramLocation(name3, path123); + assertEquals(filePath3, actual); + } + { + // if two exist return first + IPath actual = PathUtil.findProgramLocation(nameA, path123); + assertEquals(filePath2A, actual); + } + { + // try directory + String nameDir = "subdir"; + IPath subdirPath = new Path(dir1 + File.separator + nameDir); + File subdir = subdirPath.toFile(); + subdir.mkdir(); + assertTrue(file1.exists()); + + // directory should not be found + IPath actual = PathUtil.findProgramLocation(nameDir, path123); + assertEquals(null, actual); + } + } + + public void testWindows() throws CoreException, IOException { + if (!Platform.getOS().equals(Platform.OS_WIN32)) + return; + + String name1 = "file1"; + String name2 = "file2"; + String name3 = "file3"; + + // Create some folders and files + IPath dir1 = ResourceHelper.createTemporaryFolder(); + IPath dir2 = ResourceHelper.createTemporaryFolder(); + + IPath filePath1_com = new Path(dir1 + File.separator + name1 + ".com"); + IPath filePath2_exe = new Path(dir1 + File.separator + name2 + ".exe"); + + IPath filePath3 = new Path(dir1 + File.separator + name3); + IPath filePath3_exe = new Path(dir2 + File.separator + name3 + ".exe"); + + File file1_com = filePath1_com.toFile(); + file1_com.createNewFile(); + assertTrue(file1_com.exists()); + + File file2_exe = filePath2_exe.toFile(); + file2_exe.createNewFile(); + assertTrue(file2_exe.exists()); + + File file3 = filePath3.toFile(); + file3.createNewFile(); + assertTrue(file3.exists()); + + File file3_exe = filePath3_exe.toFile(); + file3_exe.createNewFile(); + assertTrue(file3_exe.exists()); + + String path1 = dir1.toOSString(); + { + // file.com + IPath actual = PathUtil.findProgramLocation(name1, path1); + assertEquals(filePath1_com, actual); + } + { + // file.exe + IPath actual = PathUtil.findProgramLocation(name2, path1); + assertEquals(filePath2_exe, actual); + } + + String path12 = dir1.toOSString() + PATH_SEPARATOR + dir2.toOSString(); + { + // dir2/file.exe is preferred to dir1/file + IPath actual = PathUtil.findProgramLocation(name3, path12); + assertEquals(filePath3_exe, actual); + } + } + +} diff --git a/core/org.eclipse.cdt.core.tests/suite/org/eclipse/cdt/core/suite/AutomatedIntegrationSuite.java b/core/org.eclipse.cdt.core.tests/suite/org/eclipse/cdt/core/suite/AutomatedIntegrationSuite.java index 29502e7e750..ae0241d30ec 100644 --- a/core/org.eclipse.cdt.core.tests/suite/org/eclipse/cdt/core/suite/AutomatedIntegrationSuite.java +++ b/core/org.eclipse.cdt.core.tests/suite/org/eclipse/cdt/core/suite/AutomatedIntegrationSuite.java @@ -34,6 +34,7 @@ import org.eclipse.cdt.internal.index.tests.IndexTests; import org.eclipse.cdt.internal.pdom.tests.PDOMTests; import org.eclipse.cdt.utils.CdtVariableResolverTest; import org.eclipse.cdt.utils.CommandLineUtilTest; +import org.eclipse.cdt.utils.FindProgramLocationTest; /** * @author vhirsl @@ -74,6 +75,7 @@ public class AutomatedIntegrationSuite extends TestSuite { suite.addTest(RewriteTests.suite()); suite.addTest(CdtVariableResolverTest.suite()); suite.addTest(CommandLineUtilTest.suite()); + suite.addTest(FindProgramLocationTest.suite()); suite.addTest(EFSExtensionTests.suite()); // Add in PDOM tests diff --git a/core/org.eclipse.cdt.core/plugin.xml b/core/org.eclipse.cdt.core/plugin.xml index 6381bbf9bc1..46ab3dccdca 100644 --- a/core/org.eclipse.cdt.core/plugin.xml +++ b/core/org.eclipse.cdt.core/plugin.xml @@ -272,6 +272,7 @@ +