bazel-skylib
bazel-skylib copied to clipboard
bazelbuild
Add process wrapper
As we are working toward having native rule compatibility with windows, we encountered a lot of run_shell usage to have access to some basic shell functionality like substituting $pwd in command line arguments or environment variables or redirecting the output. Or reading a file that contains arguments generated by another rule (build.rs scripts in rust).
Could you start with a design discussion somewhere?
I think I agree something like this is worthwhile because bash scripts have no place in a portable world, but there are some problems
- this takes a dependency on C++, which I'm not sure we want in skylib
- it's not clear the features implemented work in a general enough way to be useful outside of your use case.
I created a google doc to discuss this! https://docs.google.com/document/d/13M9ArI_Qdj3PuJk2kXSRm6YISp8uceq_lEG8_I7kXcA
+bazel-discuss [email protected]
On Wed, Jul 8, 2020 at 11:09 PM Jin [email protected] wrote:
cc @meteorcloudy https://github.com/meteorcloudy
— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/bazelbuild/bazel-skylib/pull/257#issuecomment-655871004, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXHHHAXZAUW5LIB4MWKOWTR2UYIDANCNFSM4OU4MPTA .
Given that Bazel already has a process-wrapper tool, should this be called something else?
Given that Bazel already has a process-wrapper tool, should this be called something else?
I don't have any strong opinion about the name :) if you have a suggestion I would be happy to change it!
Also, I'm wondering... all this argument rewriting... why cannot it be done from Starlark? I think the design doc should better explain why a separate tool is necessary. A separate tool feels a bit awkward to me, and it risks becoming its own "shell interpreter" over time.
Ideally it would, but looking at some open issues dating back to 2018 like this one https://github.com/bazelbuild/bazel/issues/5511, it might be harder to make these changes to support remote executions platfoms. But I'm not an expert here!