Use CommandLineUtil to split and join command lines in CBS Makefile support (#1073)

Without this the build and clean command entered in the CBS
settings could be parsed and re-assembled incorrectly.

A simple example is that an extra space (e.g. "make  clean") would
lead to an error when running make like:

```
make: *** empty string invalid as file name.  Stop.
```

This happened because the code used trivial split on " " and join
with " " to handle parsing that command line string into array of
arguments. This change fixes that by using the functionality already
available in CommandLineUtil

Fixes #1072

Author: jonahgraham

NewAndNoteworthy/CHANGELOG-API.md

@@ -179,6 +179,11 @@ The following classes have been removed or modified in API breaking ways:
   - spelling corrected for methods with Uninitialized in the name
   - setWarnUnused renamed to setWarnUnusedVars and isWarnUnused renamed to isWarnUnusedVars
 
+### StandardBuildConfiguration.setBuildCommand(String[]) and StandardBuildConfiguration.setCleanCommand(String[]) removed
+
+These methods (in `org.eclipse.cdt.core.build.StandardBuildConfiguration`) made it difficult to save and load users build and clean command without modifying it.
+They have been replaced with methods that take only a `String` for consistent parsing of command lines.
+See [#1072](https://github.com/eclipse-cdt/cdt/issues/1072) for more details on motivation for this change.
 
 ## API Changes in CDT 11.5.
 

build/org.eclipse.cdt.make.ui/src/org/eclipse/cdt/make/internal/ui/MakeBuildSettingsTab.java

@@ -198,19 +198,8 @@ public void performApply(ILaunchConfigurationWorkingCopy configuration) {
 					stdConfig.setBuildContainer(stdConfig.getProject());
 				}
 
-				String buildCommand = buildCmdText.getText().trim();
-				if (!buildCommand.isEmpty()) {
-					stdConfig.setBuildCommand(buildCommand.split(" ")); //$NON-NLS-1$
-				} else {
-					stdConfig.setBuildCommand(null);
-				}
-
-				String cleanCommand = cleanCmdText.getText().trim();
-				if (!cleanCommand.isEmpty()) {
-					stdConfig.setCleanCommand(cleanCommand.split(" ")); //$NON-NLS-1$
-				} else {
-					stdConfig.setCleanCommand(null);
-				}
+				stdConfig.setBuildCommand(buildCmdText.getText());
+				stdConfig.setCleanCommand(cleanCmdText.getText());
 			}
 		} catch (CoreException e) {
 			MakeUIPlugin.log(e.getStatus());

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

@@ -29,6 +29,7 @@
 import org.eclipse.cdt.core.model.ICModelMarker;
 import org.eclipse.cdt.core.resources.IConsole;
 import org.eclipse.cdt.internal.core.build.Messages;
+import org.eclipse.cdt.utils.CommandLineUtil;
 import org.eclipse.core.resources.IBuildConfiguration;
 import org.eclipse.core.resources.IContainer;
 import org.eclipse.core.resources.IProject;
@@ -62,11 +63,11 @@ public class StandardBuildConfiguration extends CBuildConfiguration {
 	 */
 	public static final String CLEAN_COMMAND = "stdbuild.clean.command"; //$NON-NLS-1$
 
-	private static final String[] DEFAULT_BUILD_COMMAND = new String[] { "make" }; //$NON-NLS-1$
-	private static final String[] DEFAULT_CLEAN_COMMAND = new String[] { "make", "clean" }; //$NON-NLS-1$ //$NON-NLS-2$
+	private static final String DEFAULT_BUILD_COMMAND = "make"; //$NON-NLS-1$
+	private static final String DEFAULT_CLEAN_COMMAND = "make clean"; //$NON-NLS-1$
 
-	private String[] buildCommand = DEFAULT_BUILD_COMMAND;
-	private String[] cleanCommand = DEFAULT_CLEAN_COMMAND;
+	private String buildCommand = DEFAULT_BUILD_COMMAND;
+	private String cleanCommand = DEFAULT_CLEAN_COMMAND;
 	private IContainer buildContainer;
 	private IEnvironmentVariable[] envVars;
 
@@ -101,18 +102,18 @@ private IContainer getContainer(String containerPath) {
 	private void applyProperties() {
 		setBuildContainer(getProperty(BUILD_CONTAINER));
 
-		String buildCmd = getProperty(BUILD_COMMAND);
-		if (buildCmd != null && !buildCmd.trim().isEmpty()) {
-			buildCommand = buildCmd.split(" "); //$NON-NLS-1$
+		String buildCommand = getProperty(BUILD_COMMAND);
+		if (buildCommand != null && !buildCommand.isBlank()) {
+			this.buildCommand = buildCommand;
 		} else {
-			buildCommand = DEFAULT_BUILD_COMMAND;
+			this.buildCommand = DEFAULT_BUILD_COMMAND;
 		}
 
-		String cleanCmd = getProperty(CLEAN_COMMAND);
-		if (cleanCmd != null && !cleanCmd.trim().isEmpty()) {
-			cleanCommand = cleanCmd.split(" "); //$NON-NLS-1$
+		String cleanCommand = getProperty(CLEAN_COMMAND);
+		if (cleanCommand != null && !cleanCommand.isBlank()) {
+			this.cleanCommand = cleanCommand;
 		} else {
-			cleanCommand = DEFAULT_CLEAN_COMMAND;
+			this.cleanCommand = DEFAULT_CLEAN_COMMAND;
 		}
 	}
 
@@ -165,20 +166,32 @@ public void setBuildContainer(IContainer buildContainer) {
 		}
 	}
 
-	public void setBuildCommand(String[] buildCommand) {
-		if (buildCommand != null) {
+	/**
+	 * Set the build command to use. The string will be converted to
+	 * command line arguments using {@link CommandLineUtil}
+	 *
+	 * @since 9.0
+	 */
+	public void setBuildCommand(String buildCommand) {
+		if (buildCommand != null && !buildCommand.isBlank()) {
 			this.buildCommand = buildCommand;
-			setProperty(BUILD_COMMAND, String.join(" ", buildCommand)); //$NON-NLS-1$
+			setProperty(BUILD_COMMAND, buildCommand);
 		} else {
 			this.buildCommand = DEFAULT_BUILD_COMMAND;
 			removeProperty(BUILD_COMMAND);
 		}
 	}
 
-	public void setCleanCommand(String[] cleanCommand) {
-		if (cleanCommand != null) {
+	/**
+	 * Set the build command to use. The string will be converted to
+	 * command line arguments using {@link CommandLineUtil}
+	 *
+	 * @since 9.0
+	 */
+	public void setCleanCommand(String cleanCommand) {
+		if (cleanCommand != null && !cleanCommand.isBlank()) {
 			this.cleanCommand = cleanCommand;
-			setProperty(CLEAN_COMMAND, String.join(" ", cleanCommand)); //$NON-NLS-1$
+			setProperty(CLEAN_COMMAND, cleanCommand);
 		} else {
 			this.cleanCommand = DEFAULT_CLEAN_COMMAND;
 			removeProperty(CLEAN_COMMAND);
@@ -216,9 +229,9 @@ public String getProperty(String name) {
 				return null;
 			}
 		case BUILD_COMMAND:
-			return String.join(" ", buildCommand); //$NON-NLS-1$
+			return buildCommand;
 		case CLEAN_COMMAND:
-			return String.join(" ", cleanCommand); //$NON-NLS-1$
+			return cleanCommand;
 		}
 
 		return null;
@@ -246,8 +259,9 @@ public IProject[] build(int kind, Map<String, String> args, IConsole console, IP
 
 			infoStream.write(String.format(Messages.StandardBuildConfiguration_0, buildDir.toString()));
 
+			String[] parsedBuildCommand = CommandLineUtil.argumentsToArray(buildCommand);
 			List<String> command = new ArrayList<>();
-			command.add(buildCommand[0]);
+			command.add(parsedBuildCommand[0]);
 
 			if (!getBuildContainer().equals(getProject())) {
 				Path makefile = Paths.get(getProject().getFile("Makefile").getLocationURI()); //$NON-NLS-1$
@@ -256,15 +270,16 @@ public IProject[] build(int kind, Map<String, String> args, IConsole console, IP
 				command.add(relative.toString());
 			}
 
-			for (int i = 1; i < buildCommand.length; i++) {
-				command.add(buildCommand[i]);
+			for (int i = 1; i < parsedBuildCommand.length; i++) {
+				command.add(parsedBuildCommand[i]);
 			}
 
 			try (ErrorParserManager epm = new ErrorParserManager(project, getProject().getLocationURI(), this,
 					getToolChain().getErrorParserIds())) {
 				epm.setOutputStream(console.getOutputStream());
 				// run make
-				console.getOutputStream().write(String.format("%s\n", String.join(" ", command))); //$NON-NLS-1$ //$NON-NLS-2$
+				console.getOutputStream()
+						.write(String.format("%s\n", CommandLineUtil.argumentsToString(command, false))); //$NON-NLS-1$
 
 				org.eclipse.core.runtime.Path workingDir = new org.eclipse.core.runtime.Path(
 						getBuildDirectory().toString());
@@ -305,9 +320,9 @@ public void clean(IConsole console, IProgressMonitor monitor) throws CoreExcepti
 			List<String> command = new ArrayList<>();
 			List<String> buildCommand;
 			if (cleanCommand != null) {
-				buildCommand = Arrays.asList(cleanCommand);
+				buildCommand = Arrays.asList(CommandLineUtil.argumentsToArray(cleanCommand));
 			} else {
-				buildCommand = Arrays.asList(DEFAULT_CLEAN_COMMAND);
+				buildCommand = Arrays.asList(CommandLineUtil.argumentsToArray(DEFAULT_CLEAN_COMMAND));
 			}
 
 			command.add(buildCommand.get(0));
@@ -325,7 +340,7 @@ public void clean(IConsole console, IProgressMonitor monitor) throws CoreExcepti
 			}
 
 			// run make
-			infoStream.write(String.format("%s\n", String.join(" ", command))); //$NON-NLS-1$ //$NON-NLS-2$
+			infoStream.write(String.format("%s\n", CommandLineUtil.argumentsToString(command, false))); //$NON-NLS-1$
 
 			org.eclipse.core.runtime.Path workingDir = new org.eclipse.core.runtime.Path(
 					getBuildDirectory().toString());

core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/CommandLineUtil.java

@@ -15,6 +15,7 @@
 package org.eclipse.cdt.utils;
 
 import java.util.ArrayList;
+import java.util.Collection;
 
 import org.eclipse.core.runtime.Platform;
 import org.eclipse.osgi.service.environment.Constants;
@@ -269,6 +270,26 @@ public static String[] argumentsToArrayWindowsStyle(String line) {
 		return aList.toArray(new String[aList.size()]);
 	}
 
+	/**
+	 * Converts argument array to a string suitable for passing to Bash like:
+	 *
+	 * This process reverses {@link #argumentsToArray(String)}, but does not
+	 * restore the exact same results.
+	 *
+	 * @param args
+	 *            the arguments to convert and escape
+	 * @param encodeNewline
+	 *            <code>true</code> if newline (<code>\r</code> or
+	 *            <code>\n</code>) should be encoded
+	 *
+	 * @return args suitable for passing to some process that decodes the string
+	 *         into an argument array
+	 * @since 9.0
+	 */
+	public static String argumentsToString(Collection<String> args, boolean encodeNewline) {
+		return argumentsToString(args.toArray(String[]::new), encodeNewline);
+	}
+
 	/**
 	 * Converts argument array to a string suitable for passing to Bash like:
 	 *