Skip to content

Conversation

@aganea
Copy link
Member

@aganea aganea commented Aug 13, 2024

If any of the printed paths by llvm-config contain quotes, spaces, backslashes or dollar sign characters, these paths will be quoted and escaped, but only if using --quote-paths. The previous behavior is retained for compatibility and --quote-paths is there to acknowledge the migration to the new behavior.

Following discussion in #76304

Fixes #28117

Superseeds #97305

I could also do what @tothambrus11 suggests in #97305 (comment) but that makes all Windows paths quoted & escaped since they all contain backslashes.

@aganea aganea self-assigned this Aug 13, 2024
@aganea aganea removed their assignment Aug 13, 2024
Comment on lines 512 to 518
bool QuotePaths = false;
for (int i = 1; i != argc; ++i) {
if (StringRef(argv[i]) == "--quote-paths") {
QuotePaths = true;
break;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Could we do something like:

Suggested change
bool QuotePaths = false;
for (int i = 1; i != argc; ++i) {
if (StringRef(argv[i]) == "--quote-paths") {
QuotePaths = true;
break;
}
}
bool QuotePaths = std::any_of(&argv[0], &argv[argc], [](const char *arg) {
return StringRef(arg) == "--quote-paths";
});

Copy link
Member Author

Choose a reason for hiding this comment

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

As suggested.

@aganea
Copy link
Member Author

aganea commented Sep 1, 2025

I am getting back on some of my older PRs. Any objections in landing this? +@rnk

Copy link
Collaborator

@rnk rnk left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the ping!

@tothambrus11
Copy link

tothambrus11 commented Sep 4, 2025

Hi, thanks for the work, looks great!

As a food for thought regarding \-es: my go-to practice with paths on windows was to replace all \ characters in paths with /. It's so much less hassle, and it works perfectly with all tools I've tried. Unfortunately, not all external tools may take this into account, but regular Windows functionality should work. So probably escaping is the more robust choice in general. https://retrocomputing.stackexchange.com/a/28348

For us at Hylo, it works great though, see my script for generating a pkg-config file for LLVM: https://github.com/hylo-lang/get-llvm/blob/main/src/get-llvm.ts#L305

aganea and others added 3 commits November 1, 2025 11:12
If any of the printed paths by llvm-config contains quotes, spaces,
backslashes or dollar sign characters, these paths can be quoted if
using --quote-paths and the corresponding characters will be escaped.

Following discussion in llvm#76304

Fixes llvm#28117
@aganea aganea force-pushed the quote_llvm_config_paths branch from 49cef79 to 159409b Compare November 1, 2025 16:33
@aganea aganea enabled auto-merge (squash) November 1, 2025 16:34
@github-actions
Copy link

github-actions bot commented Nov 1, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@aganea aganea merged commit 421ba7f into llvm:main Nov 1, 2025
10 of 11 checks passed
aganea added a commit that referenced this pull request Nov 1, 2025
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Nov 1, 2025
DEBADRIBASAK pushed a commit to DEBADRIBASAK/llvm-project that referenced this pull request Nov 3, 2025
…cape paths (llvm#103397)

If any of the printed paths by llvm-config contain quotes, spaces,
backslashes or dollar sign characters, these paths will be quoted and
escaped, but only if using `--quote-paths`. The previous behavior is
retained for compatibility and `--quote-paths` is there to acknowledge
the migration to the new behavior.

Following discussion in llvm#76304

Fixes llvm#28117

Superseeds llvm#97305

I could also do what @tothambrus11 suggests in
llvm#97305 (comment)
but that makes all Windows paths quoted & escaped since they all contain
backslashes.
DEBADRIBASAK pushed a commit to DEBADRIBASAK/llvm-project that referenced this pull request Nov 3, 2025
ckoparkar pushed a commit to ckoparkar/llvm-project that referenced this pull request Nov 6, 2025
…cape paths (llvm#103397)

If any of the printed paths by llvm-config contain quotes, spaces,
backslashes or dollar sign characters, these paths will be quoted and
escaped, but only if using `--quote-paths`. The previous behavior is
retained for compatibility and `--quote-paths` is there to acknowledge
the migration to the new behavior.

Following discussion in llvm#76304

Fixes llvm#28117

Superseeds llvm#97305

I could also do what @tothambrus11 suggests in
llvm#97305 (comment)
but that makes all Windows paths quoted & escaped since they all contain
backslashes.
ckoparkar pushed a commit to ckoparkar/llvm-project that referenced this pull request Nov 6, 2025
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.

Spaces in paths from llvm-config should be quoted

5 participants