From c55db63bad4a7c3fa572bf8a57d7ee36fde5a109 Mon Sep 17 00:00:00 2001 From: Jonah Graham Date: Sat, 1 Feb 2025 23:45:06 -0500 Subject: [PATCH] Use CommandLineUtil to split and join command lines in CBS Makefile support 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 --- NewAndNoteworthy/CHANGELOG-API.md | 5 ++ .../internal/ui/MakeBuildSettingsTab.java | 15 +--- .../build/StandardBuildConfiguration.java | 69 +++++++++++-------- .../eclipse/cdt/utils/CommandLineUtil.java | 21 ++++++ 4 files changed, 70 insertions(+), 40 deletions(-) diff --git a/NewAndNoteworthy/CHANGELOG-API.md b/NewAndNoteworthy/CHANGELOG-API.md index 0a520f8d9f0..e11d8ac869e 100644 --- a/NewAndNoteworthy/CHANGELOG-API.md +++ b/NewAndNoteworthy/CHANGELOG-API.md @@ -24,6 +24,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. diff --git a/build/org.eclipse.cdt.make.ui/src/org/eclipse/cdt/make/internal/ui/MakeBuildSettingsTab.java b/build/org.eclipse.cdt.make.ui/src/org/eclipse/cdt/make/internal/ui/MakeBuildSettingsTab.java index c3700801eae..e2e1a6b8b41 100644 --- a/build/org.eclipse.cdt.make.ui/src/org/eclipse/cdt/make/internal/ui/MakeBuildSettingsTab.java +++ b/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()); diff --git a/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/build/StandardBuildConfiguration.java b/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/build/StandardBuildConfiguration.java index 179cbdda0a4..abd355694e6 100644 --- a/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/build/StandardBuildConfiguration.java +++ b/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; @@ -61,11 +62,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; @@ -97,18 +98,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; } } @@ -161,20 +162,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); @@ -212,9 +225,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; @@ -242,8 +255,9 @@ public IProject[] build(int kind, Map args, IConsole console, IP infoStream.write(String.format(Messages.StandardBuildConfiguration_0, buildDir.toString())); + String[] parsedBuildCommand = CommandLineUtil.argumentsToArray(buildCommand); List 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$ @@ -252,15 +266,16 @@ public IProject[] build(int kind, Map 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()); @@ -301,9 +316,9 @@ public void clean(IConsole console, IProgressMonitor monitor) throws CoreExcepti List command = new ArrayList<>(); List 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)); @@ -321,7 +336,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()); diff --git a/core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/CommandLineUtil.java b/core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/CommandLineUtil.java index 5d91d7faa10..8b956efdf14 100644 --- a/core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/CommandLineUtil.java +++ b/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 + * true if newline (\r or + * \n) 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 args, boolean encodeNewline) { + return argumentsToString(args.toArray(String[]::new), encodeNewline); + } + /** * Converts argument array to a string suitable for passing to Bash like: *