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

Use param files to invoke process_target.py #283

Merged
merged 1 commit into from
Sep 4, 2024

Conversation

mrkkrp
Copy link
Contributor

@mrkkrp mrkkrp commented Aug 22, 2024

Without a param file invocations fail with "Argument list too long" when run on larger projects.

Without a param file invocations fail with "Argument list too long" when run
on larger projects.
Copy link
Owner

@martis42 martis42 left a comment

Choose a reason for hiding this comment

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

@mrkkrp
Thanks for this contribution.
One minor question though. Is there a specific reason why you set use_always? I do not exactly have a problem with this, but would like to understand why the default behavior is not sufficient,

@mrkkrp
Copy link
Contributor Author

mrkkrp commented Sep 3, 2024

Well, one reason is testing. Without use_always I think your test suite would not exercise the code that this change introduces. Another reason is that I simply did not trust Bazel to switch between behaviors correctly (which is just me being paranoid) and in general I do not like having 2 possible branches of execution with regard to how arguments are passed. Happy to remove use_always if desired!

@martis42 martis42 merged commit bc5bd28 into martis42:main Sep 4, 2024
11 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.

2 participants