Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jonahgraham
Copy link
Member

@jonahgraham jonahgraham commented Feb 2, 2025

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

For example, when building with a quoted argument with a space it works now, and it also quotes the output correctly in the Console so it can be copied and pasted if needed:

image

Note that make, and other arguments that don't need quoting are always output quoted. A separate PR will address that.

…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

Fixes eclipse-cdt#1072
jonahgraham added a commit to jonahgraham/cdt that referenced this pull request Feb 2, 2025
Initially CommandLineUtil.argumentsToString was used to provide properly
quoted string when interacting with GDB. But it is also useful to
print a string that can be copied + pasted to a terminal for the user.

When doing this the always quote every argument looks less nice, so
this change updates the code to only quote argument if needed.

Tests have also been added for the quoting.

Improves look and feel of changes in eclipse-cdt#1073
@@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ewaterlander I have gone back and forth a little on straight out removing this API in favour of my new code. Please let me know if you are depending on the array version of setBuildCommand and setCleanCommand.

@jonahgraham jonahgraham marked this pull request as ready for review February 2, 2025 18:18
@jonahgraham
Copy link
Member Author

Note that make, and other arguments that don't need quoting are always output quoted. A separate PR will address that.

See #1074

@jonahgraham jonahgraham added this to the 12.0.0 M3 milestone Feb 2, 2025
Copy link

github-actions bot commented Feb 2, 2025

Test Results

   600 files  ± 0     600 suites  ±0   13m 28s ⏱️ +7s
10 206 tests ± 0  10 183 ✅ ± 0  23 💤 ±0  0 ❌ ±0 
10 224 runs   - 20  10 201 ✅  - 20  23 💤 ±0  0 ❌ ±0 

Results for commit c55db63. ± Comparison against base commit c8e47b3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CBS Makefile builder does not parse build / clean commands fully
1 participant