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

Go port and windows support #57

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

peakschris
Copy link

As discussed, this is a port of the runner to go, removing python dependency. This PR also introduces full windows support, with runfiles either enabled or disabled. The multirun runner is invoked natively, without any bash or bat wrappers.

Features

  • Clean go port of multirun.py, assisted by Copilot. Only the json parsing function was carried over from bazel-tools/multirun.
  • Multirun is invoked natively without any wrappers, by following the 'native_binary' approach in bazel skylib. This uses symlinks to a single exe so each runner is small and fast to create.
  • Port of runfiles.bash v3 to windows bat, assisted by Copilot. These allow full runfiles lookup via a bat script. Initially these are inside this MR, but I intend to submit to aspect-bazel-lib as a new version of their windows_utils.bzl file.
  • Windows supported without bash. Using the previously mentioned windows_utils.bzl, commands are invoked via either a bash or bat wrapper depending on platform.
  • No API changes. Some behaviour differences (see below)
  • All tests pass on both windows and linux

Behaviour differences

  • As multirun is no longer wrapped in a bash script, and is invoked via a symlink, there is some complexity inside multirun to figure out where it is invoked from, and to set the RUNFILES variables correctly.
  • Multirun filenames are now .exe, instead of .bash
  • v3 runfiles library is used
  • As go is now used instead of python for concurrent execution, there may be some differences in scheduling and ordering of tasks.

Followup help needed

  • CI setup, ideally packaging prebuilt binaries
  • Testing

Copy link
Author

Choose a reason for hiding this comment

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

This file will be removed from PR once bazel-contrib/bazel-lib#872 closes

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.

1 participant