Skip to content

Fix bash issues in osx_cc_wrapper.sh.tpl#685

Merged
fmeum merged 2 commits intobazel-contrib:masterfrom
elliottt:trevor/fix-bash-osx-wrapper-issues
Feb 25, 2026
Merged

Fix bash issues in osx_cc_wrapper.sh.tpl#685
fmeum merged 2 commits intobazel-contrib:masterfrom
elliottt:trevor/fix-bash-osx-wrapper-issues

Conversation

@elliottt
Copy link
Contributor

@elliottt elliottt commented Feb 24, 2026

This PR fixes two issues with the osx_cc_wrapper.sh.tpl and cc_wrapper.sh.tpl templates:

  1. (Both files) The condition for entering the loop to process parameter files included -r ${i:1}, which was checking the loop index value, not the argument that corresponds to the name of the parameter file. This was changed to -r ${!i:1}.
  2. (Only osx_cc_wrapper.sh.tpl) The temporary file was being appended to with the result of parse_option, which doesn't return any value, resulting in the tempfile being empty. This was changed to run parse_option "${opt}" separately from appending the option to the file, ensuring that the temporary file is being populated with the sanitized commands.

The first fix addresses the issue of arguments in the param files not being sanitized, while the second addresses the issue of the sanitized arguments in the parameter file not being passed through.

Copy link
Collaborator

@helly25 helly25 left a comment

Choose a reason for hiding this comment

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

The linter is unhappy. Just follow the instructions of the failed PR check.

@fmeum
Copy link
Member

fmeum commented Feb 24, 2026

Please fix the lint finding

@elliottt elliottt force-pushed the trevor/fix-bash-osx-wrapper-issues branch from f45c36a to ba8e4fe Compare February 24, 2026 19:27
@elliottt elliottt requested a review from helly25 February 24, 2026 19:42
@elliottt
Copy link
Contributor Author

@helly25, I believe I've fixed the formatting issue.

@fmeum fmeum force-pushed the trevor/fix-bash-osx-wrapper-issues branch from ba8e4fe to 1c6b1c2 Compare February 25, 2026 08:12
@fmeum fmeum enabled auto-merge (squash) February 25, 2026 08:13
@fmeum fmeum merged commit 8fa06ea into bazel-contrib:master Feb 25, 2026
61 checks passed
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.

4 participants