Skip to content

Create sanitized response file instead of expanding arguments #480

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

Merged
merged 3 commits into from
Apr 10, 2025

Conversation

dfreese
Copy link
Contributor

@dfreese dfreese commented Apr 9, 2025

The cc_wrapper.sh scripts inspects response files and sanitizes each line within them. How the logic is structured currently causes all of these arguments to the be directly used later in the script. This can run into argument overflows. It can also run into problems where quoting in response files and from args is treated differently.

This addresses some of the comments in #430, namely cleaning up files and not overwriting the input files. I have not tried to apply a threshold here as that is best left up to the tooling that previously decided to use response files, which can cause difficult to debug changes in behavior.

This conflicts with #479, but is trying to address the same underlying problem of quoted arguments in response files as the result of golang's link actions.

Fixes #421

The cc_wrapper.sh scripts inspects response files and sanitizes each
line within them.  How the logic is structured currently causes all of
these arguments to the be directly used later in the script.  This can
run into argument overflows.  It can also run into problems where
quoting in response files and from args is treated differently.

This addresses some of the comments in bazel-contrib#430, namely cleaning up files
and not overwriting the input files.  I have not tried to apply a
threshold here as that is best left up to the tooling that previously
decided to use response files, which can cause difficult to debug
changes in behavior.

This conflicts with bazel-contrib#479, but is trying to address the same underlying
problem of quoted arguments in response files as the result of golang's
link actions.

Fixes bazel-contrib#421
@fmeum fmeum enabled auto-merge (squash) April 9, 2025 21:46
@dfreese
Copy link
Contributor Author

dfreese commented Apr 9, 2025

added a commit to address formatting errors and the unused variable. For me that isn't showing up on the PR due to github "Processing Updates"

auto-merge was automatically disabled April 9, 2025 22:46

Head branch was pushed to by a user without write access

@dfreese
Copy link
Contributor Author

dfreese commented Apr 9, 2025

Force-pushed to get retry the "Processing updates". Should be good to go now.

@fmeum fmeum enabled auto-merge (squash) April 10, 2025 09:29
@mering
Copy link
Contributor

mering commented Apr 10, 2025

This conflicts with #479, but is trying to address the same underlying problem of quoted arguments in response files as the result of golang's link actions.

I think we still want both (this to avoid too long argument lines and the other one to correctly sanitize options within a response file).

@fmeum
Copy link
Member

fmeum commented Apr 10, 2025

@dfreese Please fix the remaining linter findings, then this should be good to go in.

auto-merge was automatically disabled April 10, 2025 11:35

Head branch was pushed to by a user without write access

@dfreese
Copy link
Contributor Author

dfreese commented Apr 10, 2025

@dfreese Please fix the remaining linter findings, then this should be good to go in.

Done.

@fmeum fmeum enabled auto-merge (squash) April 10, 2025 11:37
@fmeum fmeum merged commit 4c3d6cf into bazel-contrib:master Apr 10, 2025
35 checks passed
@dfreese dfreese deleted the sanitize_files branch April 10, 2025 11:53
@ParkMyCar
Copy link
Contributor

Thanks @dfreese!

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.

Why are response files expanded in osx_cc_wrapper?
4 participants