Conversation
|
edit: no .bazelrc.user required |
|
thesayyn
left a comment
There was a problem hiding this comment.
This looks great, i am okay with landing this as is, if you could selectively disable tests that doesn't work windows.
|
@thesayyn I now have bsdtar fixed on Windows; thanks to @alexeagle for pushing the patch through all the repos. rules_oci now looks like it is working pretty well.
Thanks! |
|
@thesayyn ping |
|
@thesayyn This is ready to go - except the docs update which I can't run from windows. Please would you do this and advise on next steps? Thank you! cc @alexeagle @fmeum |
|
@thesayyn I fixed the bazel-lib docs macros whilst I was waiting :-) so these docs files are now updated and this is ready to go. |
83252d5 to
2d955fd
Compare
|
@fmeum I made some fixes; could you please trigger CI again? |
|
@thesayyn any chance you can take another look at this PR? :) |
thesayyn
left a comment
There was a problem hiding this comment.
This is looking closer. There seems to be some bad diff from formatters and feature additions which i'd like to address in a separate PR.
4186047 to
041ff88
Compare
041ff88 to
5d2ae2e
Compare
|
@thesayyn Thanks for your review; I've addressed all your comments; moved unrelated features out; rebased to latest |
thesayyn
left a comment
There was a problem hiding this comment.
Looking great, a few small changes.
|
I'll merge this once the windows cfg selection is fixed. |
|
@peakschris it would be nice to get windows CI back. do you mind adding that to the matrix? Also bcr presubmit will need to change to test for windows there. |
I've added to bcr presubmit; happy to make a change to github workflows if you can guide me - I'm not familiar with this and superficially it looks like windows is already configured |
|
@thesayyn trying to get windows CI added & not hanging by following pattern from bazel-lib; can you give me rights to trigger/retrigger the workflow? |
|
@thesayyn I could use some advice. Building case4_transition with Bazel 8.3.1 or 8.4.1: results in ctx.toolchains["@bazel_tools//tools/sh:toolchain_type"].path returning /bin/bash instead of C:\msys64\usr\bin\bash.exe as set in BAZEL_SH envvar. This results in the bat wrappers generated for windows failing with invalid path. Is there any way to get the @bazel_tools//tools/sh:toolchain_type without it being interfered by the transition? This is only an issue with bazel 8; bazel 7.6.1 returns the windows bash.exe Thanks! Chris |
|
@thesayyn @mostynb I'm going to give up on this for now; I'm unable to get all the tests green for the same reasons that this PR struggled: #842 Upgrading to a version of bazel_lib that contains a tar that works on Windows requires us to drop support for modules under bazel 6; as @mostynb argues we could consider dropping bazel 6 completely. If you can get #842 merged then I can have another go with this. |
cc97f2c to
f609908
Compare
f609908 to
87d6e02
Compare
|
Thanks, I'll see how i can help. |
|
|
||
| # Create the host platform repository transitively required by bazel-lib | ||
|
|
||
| maybe( |
There was a problem hiding this comment.
Hmm, why this is needed? i thought we simply dropped this requirement with some release of bazel_lib.
There was a problem hiding this comment.
I'm not sure if it was needed; I was seeing strange errors relate to resolving zstd toolchain so I pasted it in from bazel_lib release notes.
Thank you! |
|
Here's an attempt to upgrade CI to test bazel 7 and 8: #854 |
|
I am trying to get @mostynb PR green. |
|
This needs a rebase now #854 is landed. |

Most features are working
Adds:
Limitations
Fixes:
_maybe_wrap_launcher_for_windowsselects wrapper based on target platform #420@@bazel_tools//src/conditions:host_windows_x64_constraintbeing used byoci_image#819Other relevant PRs
Test results:
bazel test //... --action_env=PATH - 36 pass and 14 skipped
bazel test //... --action_env=PATH --enable_runfiles - 36 pass and 14 skipped