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

extractArgumentsForDynamicLinkParamFile causes arguments that start with @rpath to be interpreted as a param file #14316

Closed
brentleyjones opened this issue Nov 24, 2021 · 5 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) platform: apple team-Rules-CPP Issues for C++ rules type: feature request

Comments

@brentleyjones
Copy link
Contributor

Description of the problem / feature request:

While updating rules_apple we couldn't use @rpath arguments as it would result in a build failure. Instead we had to pass the arguments in a different way:

https://github.com/bazelbuild/rules_apple/pull/1207/files#diff-d84498974951b9a579baeeaf19d52ffb0a42a15b520c4359dac92448e2b806ceR670

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

This code needs to be updated to support arguments that start with @rpath:

What operating system are you running Bazel on?

macOS 11.6.1

What's the output of bazel info release?

5.0.0rc2

@keith
Copy link
Member

keith commented Nov 29, 2021

FWIW this was a bit of an intentional tradeoff when we added params files support. In the shell scripts we check if the file exists, and assume that if it doesn't it's an intentional arg otherwise, so we could do the same here to fix it probably.

@sventiffe
Copy link
Contributor

So is this working as intended?

@keith
Copy link
Member

keith commented Jan 14, 2022

We should fix anywhere we can easily, but most of the time the recommendation is to change whoever is passing the flag to use the -Wl,@rpath format to avoid this.

keith added a commit to keith/bazel that referenced this issue Jan 26, 2022
Most programs that accept params files use the `@file` syntax. For Apple
platform builds `@` can be the start of non-params file arguments as
well, such as `-rpath @executable_path/Frameworks`. There is a small
list of options where this is the case, so this new behavior no longer
assumes params files if args start with `@`, they also have to not start
with one of the 3 keywords used with this (from `man dyld` on macOS).
This should always hold since params files generated by bazel should
always start with `bazel-out`, if someone renames the symlinks to one of
the keywords, they're on their own.

Previously the workaround was to always make sure to pass the
`-Wl,-rpath,@executable_path` form of these arguments, but this makes
users not have to worry about this.

In a few other places we check this by checking if the file exists,
which is likely more accurate, but feels excessive and potentially
dangerous in this context.

Related: bazelbuild#13148
Fixes: bazelbuild#14316
@keith
Copy link
Member

keith commented Jan 26, 2022

#14650

@gregestren gregestren added team-Rules-CPP Issues for C++ rules untriaged labels Jan 31, 2022
@oquenchil oquenchil added P3 We're not considering working on this, but happy to review a PR. (No assignee) platform: apple type: feature request and removed untriaged labels Feb 14, 2022
@brentleyjones
Copy link
Contributor Author

@bazel-io fork 5.1

brentleyjones pushed a commit to brentleyjones/bazel that referenced this issue Mar 2, 2022
Most programs that accept params files use the `@file` syntax. For Apple
platform builds `@` can be the start of non-params file arguments as
well, such as `-rpath @executable_path/Frameworks`. There is a small
list of options where this is the case, so this new behavior no longer
assumes params files if args start with `@`, they also have to not start
with one of the 3 keywords used with this (from `man dyld` on macOS).
This should always hold since params files generated by bazel should
always start with `bazel-out`, if someone renames the symlinks to one of
the keywords, they're on their own.

Previously the workaround was to always make sure to pass the
`-Wl,-rpath,@executable_path` form of these arguments, but this makes
users not have to worry about this.

In a few other places we check this by checking if the file exists,
which is likely more accurate, but feels excessive and potentially
dangerous in this context.

Related: bazelbuild#13148
Fixes: bazelbuild#14316

Closes bazelbuild#14650.

PiperOrigin-RevId: 430195929
(cherry picked from commit 24e8242)
Wyverald pushed a commit that referenced this issue Mar 2, 2022
Most programs that accept params files use the `@file` syntax. For Apple
platform builds `@` can be the start of non-params file arguments as
well, such as `-rpath @executable_path/Frameworks`. There is a small
list of options where this is the case, so this new behavior no longer
assumes params files if args start with `@`, they also have to not start
with one of the 3 keywords used with this (from `man dyld` on macOS).
This should always hold since params files generated by bazel should
always start with `bazel-out`, if someone renames the symlinks to one of
the keywords, they're on their own.

Previously the workaround was to always make sure to pass the
`-Wl,-rpath,@executable_path` form of these arguments, but this makes
users not have to worry about this.

In a few other places we check this by checking if the file exists,
which is likely more accurate, but feels excessive and potentially
dangerous in this context.

Related: #13148
Fixes: #14316

Closes #14650.

PiperOrigin-RevId: 430195929
(cherry picked from commit 24e8242)

Co-authored-by: Keith Smiley <keithbsmiley@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) platform: apple team-Rules-CPP Issues for C++ rules type: feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants