Use py_binary for script execution - #199
Conversation
379462e to
318751b
Compare
nettle
left a comment
nettle
left a comment
There was a problem hiding this comment.
Looks promising! If it really works for all Bazel versions
| "codechecker_script.py", | ||
| "per_file_script.py", | ||
| ], | ||
| py_binary( |
There was a problem hiding this comment.
Right, this is interesting idea!
We just need to make sure it is reliable and stable in all situations
| # Use environment variables instead of expand_template | ||
| environment_variables = { |
There was a problem hiding this comment.
However, using env vars is really creepy :)
I think we should consider arguments instead
Szelethus
left a comment
Szelethus
left a comment
There was a problem hiding this comment.
I understand this is quite a nice patch. Tibor, can you rebase and revisit whether we can remove anything, as we are dropping support for bazel 6?
After rebase and a short overview again, I think this (conceptually) a good patch to land.
Add py_binary rule for per_file rule Format environment variables Remove unused function Use arguments instead of environment variables Remove remains of environment variables
|
It seems we hit the argument list limit. |
Szelethus
left a comment
Szelethus
left a comment
There was a problem hiding this comment.
Needs a rebase and fixes to CI.
3 participants
Why:
We should rely on Bazel to run Python scripts and not hack them together ourselves.
What:
codechecker_script.pyper_file_script.pyexpand_templatewith environment variables in codechecker ruleexpand_templatewith arguments in per_file rule (use_default_shell_env = True,does not allow us to define env variables)Addresses:
none