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

Fix aggressive params file assumption #14650

Conversation

keith
Copy link
Member

@keith keith commented 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: #13148
Fixes: #14316

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
Copy link
Contributor

@oquenchil oquenchil left a comment

Choose a reason for hiding this comment

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

LGTM

@bazel-io bazel-io closed this in 24e8242 Feb 22, 2022
brentleyjones pushed a commit to brentleyjones/bazel that referenced this pull request 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 pull request 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
team-Rules-CPP Issues for C++ rules
Projects
None yet
3 participants