Skip to content

Commit

Permalink
Use CommandLineUtil to split and join command lines in CBS Makefile s…
Browse files Browse the repository at this point in the history 
…upport

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

TODO:

- [ ] Add missing tests for joining array back into string
- [ ] Implement optional quoting of arguments on argumentsToStringWindowsCreateProcesswind
- [ ] Add quotes around empty parameters for argumentsToString*
  • Loading branch information
@jonahgraham
jonahgraham committed Feb 2, 2025
1 parent c8e47b3 commit 82277b6
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 16 deletions.
5 changes: 3 additions & 2 deletions build/org.eclipse.cdt.make.ui/src/org/eclipse/cdt/make/internal/ui/MakeBuildSettingsTab.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.eclipse.cdt.core.build.StandardBuildConfiguration;
import org.eclipse.cdt.launch.ui.corebuild.CommonBuildTab;
import org.eclipse.cdt.make.core.MakefileBuildConfigurationProvider;
import org.eclipse.cdt.utils.CommandLineUtil;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.Path;
Expand Down Expand Up @@ -200,14 +201,14 @@ public void performApply(ILaunchConfigurationWorkingCopy configuration) {

String buildCommand = buildCmdText.getText().trim();
if (!buildCommand.isEmpty()) {
stdConfig.setBuildCommand(buildCommand.split(" ")); //$NON-NLS-1$
stdConfig.setBuildCommand(CommandLineUtil.argumentsToArray(buildCommand));
} else {
stdConfig.setBuildCommand(null);
}

String cleanCommand = cleanCmdText.getText().trim();
if (!cleanCommand.isEmpty()) {
stdConfig.setCleanCommand(cleanCommand.split(" ")); //$NON-NLS-1$
stdConfig.setCleanCommand(CommandLineUtil.argumentsToArray(cleanCommand));
} else {
stdConfig.setCleanCommand(null);
}
Expand Down
18 changes: 10 additions & 8 deletions core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/build/StandardBuildConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -99,14 +100,14 @@ private void applyProperties() {

String buildCmd = getProperty(BUILD_COMMAND);
if (buildCmd != null && !buildCmd.trim().isEmpty()) {
buildCommand = buildCmd.split(" "); //$NON-NLS-1$
buildCommand = CommandLineUtil.argumentsToArray(buildCmd);
} else {
buildCommand = DEFAULT_BUILD_COMMAND;
}

String cleanCmd = getProperty(CLEAN_COMMAND);
if (cleanCmd != null && !cleanCmd.trim().isEmpty()) {
cleanCommand = cleanCmd.split(" "); //$NON-NLS-1$
cleanCommand = CommandLineUtil.argumentsToArray(cleanCmd);
} else {
cleanCommand = DEFAULT_CLEAN_COMMAND;
}
Expand Down Expand Up @@ -164,7 +165,7 @@ public void setBuildContainer(IContainer buildContainer) {
public void setBuildCommand(String[] buildCommand) {
if (buildCommand != null) {
this.buildCommand = buildCommand;
setProperty(BUILD_COMMAND, String.join(" ", buildCommand)); //$NON-NLS-1$
setProperty(BUILD_COMMAND, CommandLineUtil.argumentsToString(buildCommand, false));
} else {
this.buildCommand = DEFAULT_BUILD_COMMAND;
removeProperty(BUILD_COMMAND);
Expand All @@ -174,7 +175,7 @@ public void setBuildCommand(String[] buildCommand) {
public void setCleanCommand(String[] cleanCommand) {
if (cleanCommand != null) {
this.cleanCommand = cleanCommand;
setProperty(CLEAN_COMMAND, String.join(" ", cleanCommand)); //$NON-NLS-1$
setProperty(CLEAN_COMMAND, CommandLineUtil.argumentsToString(cleanCommand, false));
} else {
this.cleanCommand = DEFAULT_CLEAN_COMMAND;
removeProperty(CLEAN_COMMAND);
Expand Down Expand Up @@ -212,9 +213,9 @@ public String getProperty(String name) {
return null;
}
case BUILD_COMMAND:
return String.join(" ", buildCommand); //$NON-NLS-1$
return CommandLineUtil.argumentsToString(buildCommand, false);
case CLEAN_COMMAND:
return String.join(" ", cleanCommand); //$NON-NLS-1$
return CommandLineUtil.argumentsToString(cleanCommand, false);
}

return null;
Expand Down Expand Up @@ -260,7 +261,8 @@ public IProject[] build(int kind, Map<String, String> args, IConsole console, IP
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());
Expand Down Expand Up @@ -321,7 +323,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());
Expand Down
46 changes: 40 additions & 6 deletions core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/CommandLineUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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:
*
Expand Down Expand Up @@ -346,20 +367,33 @@ public static String argumentsToStringBash(String[] args, boolean encodeNewline)
builder.append(' ');
}

builder.append('\'');
StringBuilder argSb = new StringBuilder();
boolean argNeedsQuotes = false;
for (int j = 0; j < arg.length(); j++) {
char c = arg.charAt(j);
if (c == '\'') {
builder.append("'\"'\"'"); //$NON-NLS-1$
argNeedsQuotes = true;
argSb.append("'\"'\"'"); //$NON-NLS-1$
} else if (c == '\r' && encodeNewline) {
builder.append("'$'\\r''"); //$NON-NLS-1$
argNeedsQuotes = true;
argSb.append("'$'\\r''"); //$NON-NLS-1$
} else if (c == '\n' && encodeNewline) {
builder.append("'$'\\n''"); //$NON-NLS-1$
argNeedsQuotes = true;
argSb.append("'$'\\n''"); //$NON-NLS-1$
} else {
builder.append(c);
if (Character.isWhitespace(c)) {
argNeedsQuotes = true;
}
argSb.append(c);
}
}
builder.append('\'');
if (argNeedsQuotes) {
builder.append('\'');
}
builder.append(argSb);
if (argNeedsQuotes) {
builder.append('\'');
}
}

return builder.toString();
Expand Down

0 comments on commit 82277b6

Please sign in to comment.