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

[Bug]: bzlmod lockfile is not platform-invariant via npm extension's dependency on platform-specific yq #1880

Open
willjschmitt opened this issue Aug 7, 2024 · 0 comments
Labels
bug Something isn't working

Comments

@willjschmitt
Copy link

What happened?

After trying to flip --lockfile_mode=error in our CI environment to try to enforce that the lockfile was appropriately updated within a PR, we found our Windows build was failing because the lockfile needed an update due to a platform-specific update for the yq toolchain dependency from the npm extension in rules_js. This means, in-general, commits submitted from windows devs and linux devs using --lockfile_mode=update will spuriously update the lockfile for unrelated changes, since each OS resolves to different yq toolchains.

After building on Windows with --lockfile_mode=update, the moduleDepGraph."@@aspect_rules_js~//npm:extensions.bzl%npm".general.recordedFileInputs and moduleDepGraph."@@aspect_rules_js~//npm:extensions.bzl%npm".general.recordedRepoMappingEntries entries in the lockfile updated with the following diff:

git diff
diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock
index 45c5978..8b3c0e6 100644
--- a/MODULE.bazel.lock
+++ b/MODULE.bazel.lock
@@ -4753,7 +4753,7 @@
         "bzlTransitiveDigest": "95DPKNQTVRvKlYPcSf3TzPnzragr/FfG+tvoW80mBdA=",
         "recordedFileInputs": {
           "@@//pnpm-lock.yaml": "1e09e0ba3534328641caf9eac94634636e3354fbf7f7ae8e1260470bb34f5049",
-          "@@aspect_bazel_lib~~toolchains~yq_linux_amd64//yq": "042f7462ec8378f8b0d3bac85d1b1a063b63beef5d8e3826bb2377294116ae90"
+          "@@aspect_bazel_lib~~toolchains~yq_windows_amd64//yq.exe": "7d391921bf5481db063e2b0f043aab0ef2ddaa854ae0e03a4d6d9a1112d18fa8"
         },
         "recordedDirentsInputs": {},
         "envVariables": {},
@@ -84777,8 +84777,8 @@
           ],
           [
             "aspect_rules_js~",
-            "yq_linux_amd64",
-            "aspect_bazel_lib~~toolchains~yq_linux_amd64"
+            "yq_windows_amd64",
+            "aspect_bazel_lib~~toolchains~yq_windows_amd64"
           ],
           [
             "bazel_features~",

This might be an issue better suited for https://github.com/aspect-build/bazel-lib, but the extension itself is defined here, which has the dependency, so it might be something to be handled in the extension definition. I suspect this also might show up in different ways other extensions depend on any of the pre-built toolchains in bazel-lib

A side-note, but somewhat related: I also had a diff on the moduleDepGraph."@@aspect_rules_js~//npm:extensions.bzl%npm".general.recordedFileInputs."@@//pnpm-lock.yaml" entry on Windows, which was from line-ending differences. I can understand line endings changing the hash of the file, so I updated .gitattributes to force unix line-endings for the pnpm lock file. What I was surprised by, however, was that this lock file was the only instance I had of line-endings causing a diff in my lockfile, suggesting maybe there's a difference in how this file is handled from other bzlmod modules like rules_python and their requirements.txt files

Version

Development (host) and target OS/architectures:

Output of bazel --version: 7.1.1

Version of the Aspect rules, or other relevant rules from your
WORKSPACE or MODULE.bazel file:

  • aspect_rules_js - 1.40.0

Language(s) and/or frameworks involved:

  • bzlmod

How to reproduce

No response

Any other information?

For now, we are going to make the linux-version of the MODULE.bazel.lock canonical and disable the --lockfile_mode=error flag on Windows CI.

It looks like rules_python had similar challenges with platform-specific inputs (specifically due to varying requirements.txt files across platforms, which kicked off support for os_dependent and arch_dependent to be introduced to bazel extension definitions (design doc and relevant bazelbuild/bazel issue discussing. rules_python initially added these arguments to quickly solve the issue in bazelbuild/rules_python#1433, but later after cleanup removed them in the PRs solving bazelbuild/rules_python#1643, so they didn't have the need any longer. Perhaps, a solve inspired by rules_python could be a path

@willjschmitt willjschmitt added the bug Something isn't working label Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant