feat: windows #45

peakschris · 2024-06-17T13:11:02Z

Support windows files and archives in lockfiles.

Limitations:

requires these two flags on windows (these have no effect or are already default on other platforms)
- startup --windows_enable_symlinks
- common --enable_runfiles

Validations:

cd examples{module,workspace}
bazel run @multitool//tools/target-determinator:target-determinator
bazel run @multitool//tools/target-determinator:cwd
bazel test //... passes

peakschris · 2024-06-17T13:22:26Z

examples\module>bazel test //:integration_test --test_output=all

DEBUG: D:/udu/b/eheyrjsw/external/rules_multitool~override/multitool/private/multitool.bzl:122:22: downloading url: https://github.com/bazel-contrib/target-determinator/releases/download/v0.25.0/target-determinator.windows.amd64.exe to: tools/target-determinator/windows_x86_64_executable.exe
DEBUG: D:/udu/b/eheyrjsw/external/rules_multitool~override/multitool/private/multitool.bzl:137:22: downloading url: https://github.com/cli/cli/releases/download/v2.44.1/gh_2.44.1_windows_amd64.zip to: tools/gh/windows_x86_64_archive
DEBUG: D:/udu/b/eheyrjsw/external/rules_multitool~override/multitool/private/multitool.bzl:147:22: symlink: tools/gh/windows_x86_64_archive/gh_2.44.1_windows_amd64/bin, tools/gh/windows_x86_64_executable.exe
DEBUG: D:/udu/b/eheyrjsw/external/rules_multitool~override~multitool~multitool/tools/target-determinator/tool.bzl:10:10: symlink: bazel-out/x64_windows-fastbuild/bin/external/rules_multitool~override~multitool~multitool/tools/target-determinator/target-determinator.exe, external/rules_multitool~override~multitool~multitool.windows_x86_64/tools/target-determinator/windows_x86_64_executable.exe
INFO: Analyzed target //:integration_test (5 packages loaded, 29 targets configured).
FAIL: //:integration_test (see D:/udu/b/eheyrjsw/execroot/_main/bazel-out/x64_windows-fastbuild/testlogs/integration_test/test.log)
INFO: From Testing //:integration_test:
==================== Test output for //:integration_test:
/d/udu/b/eheyrjsw/execroot/_main/bazel-out/x64_windows-fastbuild/bin/integration_test.exe.runfiles/_main
D:\udu\b\eheyrjsw\execroot\_main\bazel-out\x64_windows-fastbuild\bin\integration_test: line 5: external/rules_multitool~override~multitool~multitool/tools/target-determinator/target-determinator.exe: No such file or directory
================================================================================
INFO: Found 1 test target...
Target //:integration_test up-to-date:
  bazel-bin/integration_test
  bazel-bin/integration_test.exe
INFO: Elapsed time: 2.176s, Critical Path: 0.84s
INFO: 8 processes: 6 internal, 2 local.
INFO: Build completed, 1 test FAILED, 8 total actions
//:integration_test                                                      FAILED in 0.7s
  D:/udu/b/eheyrjsw/execroot/_main/bazel-out/x64_windows-fastbuild/testlogs/integration_test/test.log

Executed 1 out of 1 test: 1 fails locally.

D:\udu\b\eheyrjsw\execroot_main\bazel-out\x64_windows-fastbuild\bin\integration_test.exe.runfiles_main is empty

Any clues? Thank you!

peakschris · 2024-06-17T13:24:05Z

(.venv) d:\workdir\rules_multitool\examples\module>bazel run @multitool//tools/target-determinator:cwd

Starting local Bazel server and connecting to it...
DEBUG: D:/udu/b/eheyrjsw/external/rules_multitool~override/multitool/private/multitool.bzl:123:22: downloading url: https://github.com/bazel-contrib/target-determinator/releases/download/v0.25.0/target-determinator.windows.amd64.exe to: tools/target-determinator/windows_x86_64_executable.exe
DEBUG: D:/udu/b/eheyrjsw/external/rules_multitool~override/multitool/private/multitool.bzl:123:22: downloading url: https://github.com/bazel-contrib/target-determinator/releases/download/v0.25.0/target-determinator.windows.amd64.exe to: tools/target-determinator/windows_x86_64_executable.exe
DEBUG: D:/udu/b/eheyrjsw/external/rules_multitool~override/multitool/private/multitool.bzl:139:22: downloading url: https://github.com/cli/cli/releases/download/v2.44.1/gh_2.44.1_windows_amd64.zip to: tools/gh/windows_x86_64_archive
DEBUG: D:/udu/b/eheyrjsw/external/rules_multitool~override/multitool/private/multitool.bzl:150:22: symlink: tools/gh/windows_x86_64_archive/gh_2.44.1_windows_amd64/bin, tools/gh/windows_x86_64_executable.exe
DEBUG: D:/udu/b/eheyrjsw/external/rules_multitool~override/multitool/private/multitool.bzl:123:22: downloading url: https://github.com/bazel-contrib/target-determinator/releases/download/v0.25.0/target-determinator.windows.amd64.exe to: tools/target-determinator/windows_x86_64_executable.exe
DEBUG: D:/udu/b/eheyrjsw/external/rules_multitool~override/multitool/private/multitool.bzl:139:22: downloading url: https://github.com/cli/cli/releases/download/v2.44.1/gh_2.44.1_windows_amd64.zip to: tools/gh/windows_x86_64_archive
DEBUG: D:/udu/b/eheyrjsw/external/rules_multitool~override/multitool/private/multitool.bzl:150:22: symlink: tools/gh/windows_x86_64_archive/gh_2.44.1_windows_amd64/bin, tools/gh/windows_x86_64_executable.exe
DEBUG: D:/udu/b/eheyrjsw/external/rules_multitool~override/multitool/private/multitool.bzl:123:22: downloading url: https://github.com/bazel-contrib/target-determinator/releases/download/v0.25.0/target-determinator.windows.amd64.exe to: tools/target-determinator/windows_x86_64_executable.exe
DEBUG: D:/udu/b/eheyrjsw/external/rules_multitool~override/multitool/private/multitool.bzl:139:22: downloading url: https://github.com/cli/cli/releases/download/v2.44.1/gh_2.44.1_windows_amd64.zip to: tools/gh/windows_x86_64_archive
DEBUG: D:/udu/b/eheyrjsw/external/rules_multitool~override/multitool/private/multitool.bzl:150:22: symlink: tools/gh/windows_x86_64_archive/gh_2.44.1_windows_amd64/bin, tools/gh/windows_x86_64_executable.exe
DEBUG: D:/udu/b/eheyrjsw/external/rules_multitool~override~multitool~multitool/tools/target-determinator/tool.bzl:11:10: symlink: bazel-out/x64_windows-opt-exec-ST-13d3ddad9198/bin/external/rules_multitool~override~multitool~multitool/tools/target-determinator/target-determinator.exe, external/rules_multitool~override~multitool~multitool.windows_x86_64/tools/target-determinator/windows_x86_64_executable.exe
INFO: Analyzed target @@rules_multitool~override~multitool~multitool//tools/target-determinator:cwd (68 packages loaded, 322 targets configured).
INFO: Found 1 target...
Target @@rules_multitool~override~multitool~multitool//tools/target-determinator:cwd up-to-date:
  bazel-bin/external/rules_multitool~override~multitool~multitool/tools/target-determinator/cwd
INFO: Elapsed time: 10.076s, Critical Path: 0.06s
INFO: 6 processes: 6 internal.
INFO: Build completed successfully, 6 total actions
INFO: Running command line: bazel-bin/external/rules_multitool~override~multitool~multitool/tools/target-determinator/cwd
FATAL: ExecuteProgram(D:\udu\b\eheyrjsw\execroot\_main\bazel-out\x64_windows-fastbuild\bin\external\rules_multitool~override~multitool~multitool\tools\target-determinator\cwd) failed: ERROR: src/main/native/windows/process.cc(202): CreateProcessW("D:\udu\b\eheyrjsw\execroot\_main\bazel-out\x64_windows-fastbuild\bin\external\rules_multitool~override~multitool~multitool\tools\target-determinator\cwd"): %1 is not a valid Win32 application.
 (error: 193)

I think we need to pass ext into cwd rule somehow, can this have access to toolchain?

mark-thm · 2024-06-17T13:39:00Z

I think we need to pass ext into cwd rule somehow, can this have access to toolchain?

I’m guessing, but I suspect you need to set some kind of extension for the output filename of the cwd target, right now we declare the output file to be the same name as the target, but you may need to set it to name + “.sh” to have Bazel/Windows decide it’s a bash script — I’ve never run Bazel on Windows though.

peakschris · 2024-06-17T14:36:57Z

cwd and tests are passing now, thanks for the inputs that helped.

examples/module/integration_test.sh

mark-thm

Do we have to do all the stuff with file extensions on Windows? It seems like, for instance, the batch script for cwd didn't need the extension in order to be runnable.

examples/module/multitool.lock.json

multitool/private/cwd.bzl

multitool/private/cwd.template.bat

multitool/private/hub_repo_tool_template/tool.bzl.template

multitool/private/multitool.bzl

peakschris · 2024-06-17T15:37:48Z

Do we have to do all the stuff with file extensions on Windows? It seems like, for instance, the batch script for cwd didn't need the extension in order to be runnable.

Yes, windows requires files to have 'exe' 'bat' or 'cmd' extensions to be executed. There is no 'execute' file bit. The cwd batch script does end up with an extension (cwd.bat)

In support of theoremlp/rules_multitool#45

peakschris · 2024-06-17T16:54:03Z

something was fixed in bazel 7.1.0 (broken in 7.0.2) related to symlinks on windows. It's corrected some of the issues I'm seeing.

peakschris · 2024-06-17T16:55:07Z

I could bump .bazelversion to 7.1.0, or 7.2.0 which I'm using quite happily in my repo; any preferences?

mark-thm · 2024-06-17T17:27:30Z

I'm not sure if there's a way to guard windows support behind a Bazel version, but that'd probably be the ideal setup vs. assuming an incrementally higher minimum version. Otherwise bumping the repo to 7.1.0 is probably fine.

peakschris · 2024-06-17T17:59:38Z

I've implemented a version check that kicks in on all platforms if any windows artifacts are seen in the lockfile. And I've also bumped repo version to 7.1.0 so that check does not fire. The version check is not bulletproof; when bazel runs off a commit hash or latest it doesn't report a version so I can't check.

peakschris · 2024-06-17T18:00:53Z

Could you try a CI run?

mark-thm · 2024-06-17T18:03:57Z

.github/workflows/ci.yml

+      # our CI only supports linux and macos
      exclude_windows: true


actually can you take this away so that the Windows CI runs now that you've added support?

mark-thm · 2024-06-17T18:04:17Z

Could you try a CI run?

Looks like just a buildifier failure.

peakschris · 2024-06-17T18:09:25Z

Is there some way I can see the test log from buildifier? It's passing locally

peakschris · 2024-06-17T18:12:02Z

never mind, it's failing locally on linux, passing on windows ;-). I'll fix the linux failures

mark-thm · 2024-06-17T18:14:48Z

#46 should make the errors visible in CI if you rebase/merge main

Co-authored-by: Mark Elliot <123787712+mark-thm@users.noreply.github.com>

peakschris · 2024-06-17T18:34:48Z

never mind, it's failing locally on linux, passing on windows ;-). I'll fix the linux failures

I think that buildifier_prebuilt has a windows bug that causes it to silently not run. I'll chase that up when I integrate it into our repo

Interestingly, they do have a way to handle invoking tools through a wrapper when runfiles are disabled. Some bash code that parses the manifest. That might be worth looking at in future.

peakschris · 2024-06-17T18:35:09Z

Buildifier is fixed, can you try another CI?

mark-thm · 2024-06-17T18:50:22Z

Interestingly it looks like buildifier.test is failing when common --nolegacy_external_runfiles is present.

mark-thm · 2024-06-17T18:52:39Z

Looks like that's a version-sensitive bug, we can bump buildifier_prebuilt to 6.4.0 and call it a day.

mark-thm · 2024-06-17T18:53:55Z

#47

peakschris · 2024-06-17T19:03:59Z

awesome! thanks for your support 🥳

mark-thm · 2024-06-17T19:08:24Z

Thanks for taking the time to improve this ruleset!

peakschris · 2024-06-17T19:23:25Z

I had a bit of trouble with 7.1.x, specifically in cases where repo rules had errors bazel would hang. I'm seeing it fixed in 7.2.0, so if you get trouble with the current version that's the easy fix.

alexeagle · 2024-07-05T17:38:27Z

Neat! I'm adding multitool fetch ruff in https://github.com/aspect-build/rules_lint/actions/runs/9811753252/job/27094364261?pr=309 where I included Windows. However that repo has a test on Bazel 6. Something seems wrong because I get the new error message even for a repo gofumpt where no windows binaries were attempted.

I guess rules_lint can't offer anyone windows binaries until all users are forced to upgrade Bazel? That's probably not desirable.
I'll back out the windows binary to land that PR...

mark-thm · 2024-07-05T17:42:13Z

It’s possible there’s a version of Bazel 6 with the same patch in 7.1+, happy to expand the range if you want to test that?

mark-thm · 2024-07-05T17:43:21Z

Also will note that you need Bazel 7.2.0 to fetch ruff using http_archive due to a Bazel bug present before that.

peakschris · 2024-07-05T17:46:20Z

Alex - At present the version check produces a failure on all platforms if a windows binary is referenced in the manifest and the version is less than 7.1. Instead, we could change it so that the version check only fails on windows platform, and lower versions are tolerated on other platforms. I can see plusses and minuses of that approach... what do you think?

alexeagle · 2024-07-05T17:56:53Z

thanks @mark-thm - I already have a re-implementation of download_and_extract due to that bug https://github.com/aspect-build/rules_lint/blob/562b84bdb00f076658fd94483a765303f671a07e/lint/ruff.bzl#L208-L220

@peakschris yeah I think we need to relax the fail semantics in some way, otherwise any ruleset that depends on multitool will have the same problem - you can either allow users on Bazel 6, or you can offer windows downloads, but not both. Your proposal to fail only when running on a windows host SGTM.

peakschris · 2024-07-05T18:55:17Z

@alexeagle there is a fix in #50

This was referenced Jun 17, 2024

support windows #44

Closed

[Bug]: Various issues running examples on windows aspect-build/rules_lint#294

Open

peakschris mentioned this pull request Jun 17, 2024

feat: windows theoremlp/multitool#34

Merged

mark-thm reviewed Jun 17, 2024

View reviewed changes

examples/module/integration_test.sh Outdated Show resolved Hide resolved

mark-thm reviewed Jun 17, 2024

View reviewed changes

mark-thm pushed a commit to theoremlp/multitool that referenced this pull request Jun 17, 2024

Add support for windows artifacts in the lockfile (#34)

39d2d5d

In support of theoremlp/rules_multitool#45

mark-thm reviewed Jun 17, 2024

View reviewed changes

peakschris and others added 9 commits June 17, 2024 14:28

feat: windows

8bd0799

fix: typo

79afe45

fix: add debug and sha

64526be

fix: call out temporary debug

55165e0

feat: flags for windows

1202971

fix: cwd works and tests pass

8599aa3

fix: remove debug

2e94bd5

Apply suggestions from code review

cd7e993

Co-authored-by: Mark Elliot <123787712+mark-thm@users.noreply.github.com>

fix: review suggestions

a89cdd7

peakschris added 6 commits June 17, 2024 14:28

debug: restore prints

19c4768

fix: bump bazelversion; final choice TBD

e3e330b

fix: bump bazelversion; final choice TBD

221254a

fix: fixes

d860293

feat: check bazel version; require 7.1+ for windows artifacts

98098a5

buildifier, main .bazelrc, windows ci

e8b7903

peakschris force-pushed the windows branch from 2304024 to e8b7903 Compare June 17, 2024 18:30

mark-thm merged commit 45ad7ae into theoremlp:main Jun 17, 2024
8 of 9 checks passed

peakschris mentioned this pull request Jun 17, 2024

[feat] windows improvements aspect-build/rules_lint#295

Closed

peakschris mentioned this pull request Jul 5, 2024

Windows version check is too strict #49

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: windows #45

feat: windows #45

peakschris commented Jun 17, 2024 •

edited

Loading

peakschris commented Jun 17, 2024

peakschris commented Jun 17, 2024

mark-thm commented Jun 17, 2024

peakschris commented Jun 17, 2024

mark-thm left a comment

peakschris commented Jun 17, 2024

peakschris commented Jun 17, 2024

peakschris commented Jun 17, 2024

mark-thm commented Jun 17, 2024

peakschris commented Jun 17, 2024

peakschris commented Jun 17, 2024

mark-thm Jun 17, 2024

mark-thm commented Jun 17, 2024

peakschris commented Jun 17, 2024

peakschris commented Jun 17, 2024

mark-thm commented Jun 17, 2024

peakschris commented Jun 17, 2024

peakschris commented Jun 17, 2024

mark-thm commented Jun 17, 2024

mark-thm commented Jun 17, 2024

mark-thm commented Jun 17, 2024

peakschris commented Jun 17, 2024

mark-thm commented Jun 17, 2024

peakschris commented Jun 17, 2024

alexeagle commented Jul 5, 2024

mark-thm commented Jul 5, 2024

mark-thm commented Jul 5, 2024

peakschris commented Jul 5, 2024

alexeagle commented Jul 5, 2024

peakschris commented Jul 5, 2024

feat: windows #45

feat: windows #45

Conversation

peakschris commented Jun 17, 2024 • edited Loading

Validations:

peakschris commented Jun 17, 2024

peakschris commented Jun 17, 2024

mark-thm commented Jun 17, 2024

peakschris commented Jun 17, 2024

mark-thm left a comment

Choose a reason for hiding this comment

peakschris commented Jun 17, 2024

peakschris commented Jun 17, 2024

peakschris commented Jun 17, 2024

mark-thm commented Jun 17, 2024

peakschris commented Jun 17, 2024

peakschris commented Jun 17, 2024

mark-thm Jun 17, 2024

Choose a reason for hiding this comment

mark-thm commented Jun 17, 2024

peakschris commented Jun 17, 2024

peakschris commented Jun 17, 2024

mark-thm commented Jun 17, 2024

peakschris commented Jun 17, 2024

peakschris commented Jun 17, 2024

mark-thm commented Jun 17, 2024

mark-thm commented Jun 17, 2024

mark-thm commented Jun 17, 2024

peakschris commented Jun 17, 2024

mark-thm commented Jun 17, 2024

peakschris commented Jun 17, 2024

alexeagle commented Jul 5, 2024

mark-thm commented Jul 5, 2024

mark-thm commented Jul 5, 2024

peakschris commented Jul 5, 2024

alexeagle commented Jul 5, 2024

peakschris commented Jul 5, 2024

peakschris commented Jun 17, 2024 •

edited

Loading