-
Notifications
You must be signed in to change notification settings - Fork 38
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
Remove passing lists of args to genrule #67
Conversation
@@ -446,16 +446,6 @@ def _run_shell( | |||
actions = actions, | |||
) | |||
|
|||
# TODO(b/77637734) remove "workaround" once the bazel issue is resolved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have insight into if this is closed yet
incompatiblity with Bazel at HEAD. Disable this for now because apple_support currently relies on a workaround that needs to be able to pass a sequence of strings to actions.run_shell's 'command'.
@@ -80,7 +80,7 @@ def _apple_genrule_impl(ctx): | |||
inputs = resolved_srcs.to_list() + resolved_inputs, | |||
outputs = files_to_build.to_list(), | |||
env = ctx.configuration.default_shell_env, | |||
command = argv, | |||
command = " ".join(argv), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we use the shell escape method in skylib here? https://github.com/bazelbuild/bazel-skylib/blob/182046f090d6ecae264c7e5d520da096370287e2/lib/shell.bzl#L37-L49
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like that overquotes
This is required for this migration bazelbuild/bazel#5903
066a737
to
b7047fa
Compare
This is required for this migration bazelbuild/bazel#5903