search

keith / rules_multirun

Bazel rules for running multiple commands in parallel in a single bazel invocation
Apache License 2.0
65 stars 13 forks source link

Consider dropping Python dependency #43

Open alexeagle opened 4 months ago

alexeagle commented 4 months ago

rules_python isn't a great dependency. It's still pre-1.0 and not very stable.

If rules_multirun is used beneath some ruleset, the rules_python dependency that ruleset takes can easily replace the user's, leading to breaking changes. This is true under both WORKSPACE and bzlmod.

https://www.gnu.org/software/parallel/ is a good alternative that can be installed hermetically and doesn't require any runtime.

fyi @thesayyn who might be able to contribute it, if you like the idea :)

keith commented 3 months ago

I'm open to something, but it seems tough because it's all about the tradeoffs. The original version of this tool was using golang, but I didn't want to force that toolchain on folks either. And if we use more shell we might have that same kind-of issue with windows (I'm still not sure how big of an issue that is for windows support). Given how widely used rules_python is I'm not particularly phased by the version number, but I can't blame people if they don't otherwise use python and now depend on that through this

alexeagle commented 3 months ago

FWIW we use Golang for tools in our bazel repos, but include pre-built binaries on releases so users don't have any toolchain dependency. It's pretty conveniently tucked into our release automation so it doesn't cost us much. https://blog.aspect.dev/releasing-bazel-rulesets-rust

keith commented 3 months ago

yea i think doing something like that could be a great solution

peakschris commented 3 weeks ago

I also have issues with python tools, rules_python is extremely slow on windows when run in hermetic mode (it unpacks 1000s of files for each python invocation), which forces me to either avoid python or install a system python.

I'm skeptical about gnu parallel. It's got a lovely UI for command line, but is written in perl, and is not supported on windows (although it is said to work in git bash or msys2)

As you can see in my other issue, we are facing issues with rules_multirun not supporting windows, and the fix seems complicated. The combination of python binaries and ctx.runfiles makes my mind fry a little (I am beginning to understand why, we need to collect runfiles for all the tools being invoked).

I love the idea of a well written go program that can be packaged and consumed as a prebuilt binary, and that takes a job manifest and just gets everything done, with a very lightweight integration in bazel rules.

alexeagle commented 3 weeks ago

Note we also have a nice pattern for distributing Rust tools, if the eventual contributor here prefers

peakschris commented 3 weeks ago

@alexeagle are you familiar with the go multirun that this repo was derived from? It's still under development although the owner doesn't make releases. I have been wondering if combining the best of both, ending up with a go solution that can be released as binary, would be a nice idea. It would need to have a formal release mechanism for us to depend on it.

https://github.com/ash2k/bazel-tools

keith commented 3 weeks ago

yea i think that, avoiding depending on the entire go world, could be ideal. alternatively we could just use go for building and vendor the binaries for releases as some repos do

alexeagle commented 3 weeks ago

I can contribute the bits to use rules_go here as a devDependency. I wonder if that Go implementation is still compatible? And if we switch to it, maybe we should just go back to that fork

peakschris commented 3 weeks ago

I've been doing some trial work on the go multirun. It's looking promising, most test cases are now passing on windows with runfiles on and the underlying functionality also works with runfiles off. I think the API is largely the same as here.

I like the idea of keeping this rules_multitool repo, it's lean. I could contribute a PR that added windows support to the go implementation and could help out in to the port effort. If desired, I could also do a draft PR that added the go code to this repo.

I can't be an ongoing maintainer, I'm in the middle of porting a huge monorepo to bazel and won't have much time once I've got rules_lint working for us (that's my main driver right now)

Which CI / binary build infra would be used?

alexeagle commented 3 weeks ago

https://blog.aspect.build/releasing-bazel-rulesets-rust describes the build infra that has the desired properties:

It's minimal effort for the rules maintainer and doesn't leak your tooling into users repos.

peakschris commented 3 weeks ago

That looks good, Alex.

I've spent some time working on the go version's code. I was trying to update the go repository test cases to support windows and norunfiles, and it's got too complex; the combination of a lot of different ways to pass arguments and quote them has made the complexity skyrocket to a point where I think it would be unmaintainable.

My conclusion is that it would be better to stick with Keith's implementations; the API is tidier, and smaller, and so the test cases and usage patterns out in the wild are likely to be simpler.

I think it would be good to:

peakschris commented 3 weeks ago

@keith @alexeagle Please see https://github.com/keith/rules_multirun/pull/57, I'm keen to have your feedback and ideas. It's a big change - needs careful discussion.

alexeagle commented 2 weeks ago

Sorry there's no time! Hair on fire! but I really appreciate your work on that and making Bazel better on Windows. Please keep pushing on us :)

peakschris commented 2 weeks ago

Alex, no worries - I know the feeling, and I'll keep reminding for when you have a chance!

Just to update, there is a chain of PRs that I need to get in to fix various windows issues we are facing, and to enable use of rules_lint format command:

@keith could you let us know your thoughts?

keith commented 2 weeks ago

I'm generally supportive here, I just need some time to review that one since it's pretty big