Bug 533888 - Simplify Meson Core Build to use startBuildProcess

- fix MesonBuildConfiguration to use startBuildProcess instead
  of CommandLauncherManager
- change CBuildConfiguration startBuildProcess() method to accept
  a build directory and to replace environment variables based
  on input parameter
- fix CBuildConfiguration watchProcess routines to ensure that the
  ReaderThreads have completed processing output to the console
- change StandardBuildConfiguration and CMakeBuildConfiguration to
  pass the working directory to startBuildProcess

Change-Id: Icb4f2d076f0e6bb1513b20f6f198f720eae07e51

build/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/internal/CMakeBuildConfiguration.java

@@ -178,7 +178,8 @@ public class CMakeBuildConfiguration extends CBuildConfiguration {
 
 				outStream.write(String.join(" ", command) + '\n'); //$NON-NLS-1$
 
-				Process p = startBuildProcess(command, new IEnvironmentVariable[0], console, monitor);
+				org.eclipse.core.runtime.Path workingDir = new org.eclipse.core.runtime.Path(getBuildDirectory().toString());
+				Process p = startBuildProcess(command, new IEnvironmentVariable[0], workingDir, console, monitor);
 				if (p == null) {
 					console.getErrorStream().write(String.format(Messages.CMakeBuildConfiguration_Failure, "")); //$NON-NLS-1$
 					return null;
@@ -222,7 +223,8 @@ public class CMakeBuildConfiguration extends CBuildConfiguration {
 
 				outStream.write(String.join(" ", command) + '\n'); //$NON-NLS-1$
 				
-				Process p = startBuildProcess(command, envVars.toArray(new IEnvironmentVariable[0]), console, monitor);
+				org.eclipse.core.runtime.Path workingDir = new org.eclipse.core.runtime.Path(getBuildDirectory().toString());
+				Process p = startBuildProcess(command, envVars.toArray(new IEnvironmentVariable[0]), workingDir, console, monitor);
 				if (p == null) {
 					console.getErrorStream().write(String.format(Messages.CMakeBuildConfiguration_Failure, "")); //$NON-NLS-1$
 					return null;
@@ -280,7 +282,8 @@ public class CMakeBuildConfiguration extends CBuildConfiguration {
 
 			outStream.write(String.join(" ", command) + '\n'); //$NON-NLS-1$
 			
-			Process p = startBuildProcess(command, env, console, monitor);
+			org.eclipse.core.runtime.Path workingDir = new org.eclipse.core.runtime.Path(getBuildDirectory().toString());
+			Process p = startBuildProcess(command, env, workingDir, console, monitor);
 			if (p == null) {
 				console.getErrorStream().write(String.format(Messages.CMakeBuildConfiguration_Failure, "")); //$NON-NLS-1$
 				return;

build/org.eclipse.cdt.cmake.ui/src/org/eclipse/cdt/cmake/ui/internal/messages.properties

@@ -19,8 +19,8 @@ CMakePropertyPage_FailedToStartCMakeGui_Body=Failed to run the CMake GUI:
 CMakePropertyPage_FailedToStartCMakeGui_Title=Failed to run CMake GUI
 CMakePropertyPage_FailedToGetOS_Body=Failed to get target OS for CMake project:
 CMakePropertyPage_FailedToGetOS_Title=Failed to get target OS
-CMakePropertyPage_FailedToFetchCMakeConfiguration_Body=Failed to fetch CMake configuration values
+CMakePropertyPage_FailedToGetCMakeConfiguration_Body=Failed to fetch CMake configuration values
-CMakePropertyPage_FailedToFetchCMakeConfiguration_Title=Failed to fetch CMake configuration
+CMakePropertyPage_FailedToGetCMakeConfiguration_Title=Failed to fetch CMake configuration
 CMakePropertyPage_FailedToConfigure=Failed to reconfigure CMake
 CMakePropertyPage_Terminated=Command terminated with rc={0}\n
 CMakePropertyPage_LaunchCMakeGui=Launch CMake GUI...

build/org.eclipse.cdt.meson.core/src/org/eclipse/cdt/internal/meson/core/MesonBuildConfiguration.java

@@ -17,16 +17,14 @@ import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Map.Entry;
-import java.util.Properties;
 
-import org.eclipse.cdt.core.CommandLauncherManager;
 import org.eclipse.cdt.core.ConsoleOutputStream;
 import org.eclipse.cdt.core.ErrorParserManager;
-import org.eclipse.cdt.core.ICommandLauncher;
+import org.eclipse.cdt.core.IConsoleParser;
 import org.eclipse.cdt.core.build.CBuildConfiguration;
-import org.eclipse.cdt.core.build.ICBuildCommandLauncher;
 import org.eclipse.cdt.core.build.IToolChain;
+import org.eclipse.cdt.core.envvar.EnvironmentVariable;
+import org.eclipse.cdt.core.envvar.IEnvironmentVariable;
 import org.eclipse.cdt.core.model.ICModelMarker;
 import org.eclipse.cdt.core.resources.IConsole;
 import org.eclipse.cdt.meson.core.Activator;
@@ -38,11 +36,9 @@ import org.eclipse.core.resources.IBuildConfiguration;
 import org.eclipse.core.resources.IProject;
 import org.eclipse.core.resources.IResource;
 import org.eclipse.core.runtime.CoreException;
-import org.eclipse.core.runtime.IPath;
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.Status;
-import org.eclipse.core.runtime.SubMonitor;
 import org.eclipse.core.runtime.jobs.Job;
 import org.osgi.service.prefs.BackingStoreException;
 import org.osgi.service.prefs.Preferences;
@@ -140,56 +136,48 @@ public class MesonBuildConfiguration extends CBuildConfiguration {
 
 			boolean runMeson = !Files.exists(buildDir.resolve("build.ninja")); //$NON-NLS-1$
 			if (runMeson) { // $NON-NLS-1$
-				org.eclipse.core.runtime.Path cmdPath = new org.eclipse.core.runtime.Path("/bin/sh"); //$NON-NLS-1$
+				List<String> commandList = new ArrayList<>();
 				
-				List<String> argsList = new ArrayList<>();
-				
-				// if we have env variables, use "env" command with modifications specified after to
-				// add to environment without replacing it (e.g. losing default path)
 				String envStr = getProperty(IMesonConstants.MESON_ENV);
+				List<IEnvironmentVariable> envVars = new ArrayList<>();
 				if (envStr != null) {
-					cmdPath = new org.eclipse.core.runtime.Path("/usr/bin/env"); //$NON-NLS-1$
+					List<String> envList = MesonUtils.stripEnvVars(envStr);
-					argsList.addAll(MesonUtils.stripEnvVars(envStr));
+					for (String s : envList) {
-					argsList.add("/bin/sh"); //$NON-NLS-1$
+						int index = s.indexOf("="); //$NON-NLS-1$
+						if (index == -1) {
+							envVars.add(new EnvironmentVariable(s));
+						} else {
+							envVars.add(new EnvironmentVariable(s.substring(0, index), s.substring(index + 1)));
+						}
+					}
 				}
-				argsList.add("-c"); //$NON-NLS-1$
 				
-				StringBuilder b = new StringBuilder();
+				commandList.add("meson"); //$NON-NLS-1$
-				b.append("meson"); //$NON-NLS-1$
 
 				String userArgs = getProperty(IMesonConstants.MESON_ARGUMENTS);
-				if (userArgs != null) {
+				if (userArgs != null && !userArgs.isEmpty()) {
-					b.append(" "); //$NON-NLS-1$
+					commandList.addAll(Arrays.asList(userArgs.split(" ")));
-					b.append(userArgs);
 				}
 				String projOptions = getProperty(IMesonConstants.MESON_PROJECT_OPTIONS);
-				if (projOptions != null) {
+				if (projOptions != null && !projOptions.isEmpty()) {
-					b.append(" "); //$NON-NLS-1$
+					commandList.addAll(Arrays.asList(projOptions.split(" ")));
-					b.append(projOptions);
 				}
-				b.append(" "); //$NON-NLS-1$
+				commandList.add(getBuildDirectory().toString());
-				b.append(getBuildDirectory().toString());
-				argsList.add(b.toString());
 
-				ICommandLauncher launcher = CommandLauncherManager.getInstance().getCommandLauncher(this);
-				
-				launcher.setProject(getProject());
-				if (launcher instanceof ICBuildCommandLauncher) {
-					((ICBuildCommandLauncher)launcher).setBuildConfiguration(this);
-				}
-				
 				monitor.subTask(Messages.MesonBuildConfiguration_RunningMeson);
 
 				outStream.write(String.join(" ", envStr != null ? ("env " + envStr) : "", //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
 						"sh -c \"meson", userArgs != null ? userArgs : "", projOptions != null ? projOptions : "", //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
 								getBuildDirectory().getParent().getParent().toString() + "\"\n")); //$NON-NLS-1$
+				
 				org.eclipse.core.runtime.Path workingDir = new org.eclipse.core.runtime.Path(getBuildDirectory().getParent().getParent().toString());
-				Process p = launcher.execute(cmdPath, argsList.toArray(new String[0]), new String[0], workingDir, monitor);
+				Process p = startBuildProcess(commandList, envVars.toArray(new IEnvironmentVariable[0]), workingDir, console, monitor);
-				if (p == null || launcher.waitAndRead(outStream, outStream, SubMonitor.convert(monitor)) != ICommandLauncher.OK) {
+				if (p == null) {
-					String errMsg = p == null ? "" : launcher.getErrorMessage(); //$NON-NLS-1$
+					console.getErrorStream().write(String.format(Messages.MesonBuildConfiguration_RunningMesonFailure, "")); //$NON-NLS-1$
-					console.getErrorStream().write(String.format(Messages.MesonBuildConfiguration_RunningMesonFailure, errMsg));
 					return null;
 				}
+				
+				watchProcess(p, console);
 			}
 			
 			if (!Files.exists(buildDir.resolve("build.ninja"))) { //$NON-NLS-1$
@@ -206,49 +194,44 @@ public class MesonBuildConfiguration extends CBuildConfiguration {
 					buildCommand = "ninja"; //$NON-NLS-1$
 				}
 
-				List<String> envList = new ArrayList<>();
-				String[] env = envList.toArray(new String[0]);
-
-				ICommandLauncher launcher = CommandLauncherManager.getInstance().getCommandLauncher(this);
-
-				launcher.setProject(getProject());
-				if (launcher instanceof ICBuildCommandLauncher) {
-					((ICBuildCommandLauncher)launcher).setBuildConfiguration(this);
-				}
 
 				monitor.subTask(Messages.MesonBuildConfiguration_RunningNinja);
 
-				org.eclipse.core.runtime.Path cmdPath = new org.eclipse.core.runtime.Path("/usr/bin/env"); //$NON-NLS-1$
+				List<String> commandList = new ArrayList<>();
-				org.eclipse.core.runtime.Path workingDir = new org.eclipse.core.runtime.Path(getBuildDirectory().toString());
-				
-				List<String> argList = new ArrayList<>();
 				
+				List<IEnvironmentVariable> envList = new ArrayList<>();
 				if (ninjaEnv != null) {
-					for (String envVar : ninjaEnv) {
+					for (String s : ninjaEnv) {
-						argList.addAll(MesonUtils.stripEnvVars(envVar));
+						int index = s.indexOf("="); //$NON-NLS-1$
+						if (index == -1) {
+							envList.add(new EnvironmentVariable(s));
+						} else {
+							envList.add(new EnvironmentVariable(s.substring(0, index), s.substring(index + 1)));
+						}
 					}
 				}
+				IEnvironmentVariable[] env = envList.toArray(new IEnvironmentVariable[0]);
 
-				argList.add("sh"); //$NON-NLS-1$
+				commandList.add(buildCommand);
-				argList.add("-c"); //$NON-NLS-1$
-				StringBuilder b = new StringBuilder();
-				b.append(buildCommand);
 				if (ninjaArgs == null) {
-					b.append(" -v"); //$NON-NLS-1$
+					commandList.add("-v"); //$NON-NLS-1$
 				} else {
 					for (String arg : ninjaArgs) {
-						b.append(" "); //$NON-NLS-1$
+						if (!arg.isEmpty()) {
-						b.append(arg);
+							commandList.add(arg);
+						}
 					}
 				}
-				argList.add(b.toString());
+				
+				org.eclipse.core.runtime.Path workingDir = new org.eclipse.core.runtime.Path(getBuildDirectory().toString());
 
-				Process p = launcher.execute(cmdPath, argList.toArray(new String[0]), env, workingDir, monitor);
+				Process p = startBuildProcess(commandList, env, workingDir, console, monitor);
-				if (p != null && launcher.waitAndRead(epm.getOutputStream(), epm.getOutputStream(), SubMonitor.convert(monitor)) != ICommandLauncher.OK) {
+				if (p == null) {
-					String errMsg = launcher.getErrorMessage();
+					console.getErrorStream().write(String.format(Messages.MesonBuildConfiguration_RunningNinjaFailure, "")); //$NON-NLS-1$
-					console.getErrorStream().write(String.format(Messages.MesonBuildConfiguration_RunningNinjaFailure, errMsg));
 					return null;
 				}
+				
+				watchProcess(p, new IConsoleParser[] { epm });
 			}
 
 			project.refreshLocal(IResource.DEPTH_INFINITE, monitor);
@@ -285,38 +268,28 @@ public class MesonBuildConfiguration extends CBuildConfiguration {
 					getToolChain().getErrorParserIds())) {
 				epm.setOutputStream(console.getOutputStream());
 
-
+				List<String> commandList = new ArrayList<>();
 				String cleanCommand = getProperty(IMesonConstants.CLEAN_COMMAND);
 				if (cleanCommand == null) {
-					cleanCommand = "ninja clean -v"; //$NON-NLS-1$
+					commandList.add("ninja"); //$NON-NLS-1$
-				}
+					commandList.add("clean"); //$NON-NLS-1$
-				String[] command = cleanCommand.split(" "); //$NON-NLS-1$
+					commandList.add("-v"); //$NON-NLS-1$
-
+				} else {
-				IPath cmd = new org.eclipse.core.runtime.Path("/usr/bin/env"); //$NON-NLS-1$
+					commandList.addAll(Arrays.asList(cleanCommand.split(" "))); //$NON-NLS-1$
-				
-				List<String> argList = new ArrayList<>();
-				argList.add("sh"); //$NON-NLS-1$
-				argList.add("-c"); //$NON-NLS-1$
-				argList.add(cleanCommand);
-
-				ICommandLauncher launcher = CommandLauncherManager.getInstance().getCommandLauncher(this);
-
-				launcher.setProject(getProject());
-				if (launcher instanceof ICBuildCommandLauncher) {
-					((ICBuildCommandLauncher)launcher).setBuildConfiguration(this);
 				}
 
 				org.eclipse.core.runtime.Path workingDir = new org.eclipse.core.runtime.Path(buildDir.toString());
 
-				String[] env = new String[0];
+				IEnvironmentVariable[] env = new IEnvironmentVariable[0];
 
-				outStream.write(String.join(" ", command) + '\n'); //$NON-NLS-1$
+				outStream.write(String.join(" ", commandList) + '\n'); //$NON-NLS-1$
-				Process p = launcher.execute(cmd, argList.toArray(new String[0]), env, workingDir, monitor);
+				Process p = startBuildProcess(commandList, env, workingDir, console, monitor);
-				if (p == null || launcher.waitAndRead(epm.getOutputStream(), epm.getOutputStream(), SubMonitor.convert(monitor)) != ICommandLauncher.OK) {
+				if (p == null) {
-					String errMsg = launcher.getErrorMessage();
+					console.getErrorStream().write(String.format(Messages.MesonBuildConfiguration_RunningNinjaFailure, "")); //$NON-NLS-1$
-					console.getErrorStream().write(String.format(Messages.MesonBuildConfiguration_RunningNinjaFailure, errMsg));
 					return;
 				}
+				
+				watchProcess(p, console);
 			}
 			
 			outStream.write(String.format(Messages.MesonBuildConfiguration_BuildingComplete, buildDir.toString()));

build/org.eclipse.cdt.meson.ui.tests/src/org/eclipse/cdt/internal/meson/ui/tests/NewMesonConfigureTest.java

@@ -215,12 +215,15 @@ public class NewMesonConfigureTest {
 		
 		String[] lines = new String[0];
 		
-		while (lines.length < 16) {
+		int counter = 0;
+		while (lines.length < 18 && counter++ < 100) {
 			String output = console.bot().styledText().getText();
 			lines = output.split("\\r?\\n"); //$NON-NLS-1$
 			bot.sleep(2000);
 		}
 		
+		assertTrue(lines.length > 17);
+		
 		bot.sleep(2000);
 		
 		assertEquals("Building in: " + projectPath + "/build/default", lines[0]);

build/org.eclipse.cdt.meson.ui.tests/src/org/eclipse/cdt/internal/meson/ui/tests/NewMesonProjectTest.java

@@ -262,6 +262,13 @@ public class NewMesonProjectTest {
 		
 		String[] lines = output.split("\\r?\\n"); //$NON-NLS-1$
 		
+		int counter = 0;
+		while (lines.length < 5 && counter++ < 100) {
+			output = console.bot().styledText().getText();
+			lines = output.split("\\r?\\n"); //$NON-NLS-1$
+			bot.sleep(2000);
+		}
+	
 		assertEquals("Building in: " + projectPath + "/build/default", lines[0]);
 		assertEquals("ninja clean -v", lines[1]);
 		assertEquals("[1/1] ninja -t clean", lines[2]);

core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/build/CBuildConfiguration.java

@@ -478,11 +478,11 @@ public abstract class CBuildConfiguration extends PlatformObject
 	/**
 	 * @since 6.5
 	 */
-	public Process startBuildProcess(List<String> commands, IEnvironmentVariable[] envVars, IConsole console, IProgressMonitor monitor) throws IOException, CoreException {
+	public Process startBuildProcess(List<String> commands, IEnvironmentVariable[] envVars, IPath buildDirectory, IConsole console, IProgressMonitor monitor) throws IOException, CoreException {
 		Process process = null;
 		IToolChain tc = getToolChain();
 		if (tc instanceof IToolChain2) {
-			process = ((IToolChain2)tc).startBuildProcess(this, commands, getBuildDirectory().toString(), envVars, console, monitor);
+			process = ((IToolChain2)tc).startBuildProcess(this, commands, buildDirectory.toString(), envVars, console, monitor);
 		} else {
 			// verify command can be found locally on path
 			Path commandPath = findCommand(commands.get(0));
@@ -494,8 +494,13 @@ public abstract class CBuildConfiguration extends PlatformObject
 			commands.set(0, commandPath.toString());
 
 			ProcessBuilder processBuilder = new ProcessBuilder(commands)
-					.directory(getBuildDirectory().toFile());
+					.directory(buildDirectory.toFile());
-			setBuildEnvironment(processBuilder.environment());
+			// Override environment variables
+			Map<String, String> environment = processBuilder.environment();
+			for (IEnvironmentVariable envVar : envVars) {
+				environment.put(envVar.getName(), envVar.getValue());
+			}
+			setBuildEnvironment(environment);
 			process = processBuilder.start();
 		}
 		return process;
@@ -515,10 +520,20 @@ public abstract class CBuildConfiguration extends PlatformObject
 	 * @since 6.4
 	 */
 	protected int watchProcess(Process process, IConsole console) throws CoreException {
-		new ReaderThread(process.getInputStream(), console.getOutputStream()).start();
+		Thread t1 = new ReaderThread(process.getInputStream(), console.getOutputStream());
-		new ReaderThread(process.getErrorStream(), console.getErrorStream()).start();
+		t1.start();
+		Thread t2 = new ReaderThread(process.getErrorStream(), console.getErrorStream());
+		t2.start();
 		try {
-			return process.waitFor();
+			int rc = process.waitFor();
+			// Allow reader threads the chance to process all output to console
+			while (t1.isAlive()) {
+				Thread.sleep(100);
+			}
+			while (t2.isAlive()) {
+				Thread.sleep(100);
+			}
+			return rc;
 		} catch (InterruptedException e) {
 			CCorePlugin.log(e);
 			return -1;
@@ -530,10 +545,20 @@ public abstract class CBuildConfiguration extends PlatformObject
 	 */
 	protected int watchProcess(Process process, IConsoleParser[] consoleParsers)
 			throws CoreException {
-		new ReaderThread(this, process.getInputStream(), consoleParsers).start();
+		Thread t1 = new ReaderThread(this, process.getInputStream(), consoleParsers);
-		new ReaderThread(this, process.getErrorStream(), consoleParsers).start();
+		t1.start();
+		Thread t2 = new ReaderThread(this, process.getErrorStream(), consoleParsers);
+		t2.start();
 		try {
-			return process.waitFor();
+			int rc = process.waitFor();
+			// Allow reader threads the chance to process all output to console
+			while (t1.isAlive()) {
+				Thread.sleep(100);
+			}
+			while (t2.isAlive()) {
+				Thread.sleep(100);
+			}
+			return rc;
 		} catch (InterruptedException e) {
 			CCorePlugin.log(e);
 			return -1;

core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/build/StandardBuildConfiguration.java

@@ -255,7 +255,8 @@ public class StandardBuildConfiguration extends CBuildConfiguration {
 				// run make
 				console.getOutputStream().write(String.format("%s\n", String.join(" ", command))); //$NON-NLS-1$ //$NON-NLS-2$
 				
-				Process p = startBuildProcess(command, envVars, console, monitor);
+				org.eclipse.core.runtime.Path workingDir = new org.eclipse.core.runtime.Path(getBuildDirectory().toString());
+				Process p = startBuildProcess(command, envVars, workingDir, console, monitor);
 				
 				if (p == null) {
 					console.getErrorStream().write(String.format(Messages.StandardBuildConfiguration_Failure, "")); //$NON-NLS-1$
@@ -270,7 +271,6 @@ public class StandardBuildConfiguration extends CBuildConfiguration {
 				outStream.write(String.format(Messages.StandardBuildConfiguration_1, epm.getErrorCount(), 
 						epm.getWarningCount(), buildDir.toString()));
 			}
-
 			return new IProject[] { project };
 		} catch (IOException e) {
 			throw new CoreException(
@@ -315,7 +315,8 @@ public class StandardBuildConfiguration extends CBuildConfiguration {
 			// run make
 			outStream.write(String.format("%s\n", String.join(" ", command))); //$NON-NLS-1$ //$NON-NLS-2$
 			
-			Process p = startBuildProcess(command, envVars, console, monitor);
+			org.eclipse.core.runtime.Path workingDir = new org.eclipse.core.runtime.Path(getBuildDirectory().toString());
+			Process p = startBuildProcess(command, envVars, workingDir, console, monitor);
 			if (p == null) {
 				console.getErrorStream().write(String.format(Messages.StandardBuildConfiguration_Failure, "")); //$NON-NLS-1$
 				return;