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

crates_vendor produces a defs.bzl file that fails when included as part of a bzlmod submodule #2661

Open
tel opened this issue May 17, 2024 · 1 comment

Comments

@tel
Copy link

tel commented May 17, 2024

When generating a crates universe using crates_vendor a defs.bzl file is created to define all of the downloaded http_archive targets. Here's an example target that's generated.

maybe(
    http_archive,
    name = "vendor__once_cell-1.19.0",
    sha256 = "3fdb12b2476b595f9358c5161aa467c2438859caa136dec86c26fdd2efe17b92",
    type = "tar.gz",
    urls = ["https://static.crates.io/crates/once_cell/1.19.0/download"],
    strip_prefix = "once_cell-1.19.0",
    build_file = Label("@//third-party/bazel:BUILD.once_cell-1.19.0.bazel"),
)

This works fine as long as the path @//third-party/bazel is correct. This path becomes incorrect, however, if the module that makes use of this crates_vendor target happens to get included via bzlmod. Now, the absolute path @// is unknown from within the included module.

In the top-level module, this leads to the creation of an unparseable MODULE.bazel.lock file which includes labels like

"vendor__windows_x86_64_msvc-0.52.5": {
  "bzlFile": "@@bazel_tools//tools/build_defs/repo:http.bzl",
  "ruleClassName": "http_archive",
  "attributes": {
    "sha256": "bec47e5bfd1bff0eeaf6d8b485cc1074891a197ab4225d504cb7a1ab88b02bf0",
    "type": "tar.gz",
    "urls": [
      "https://static.crates.io/crates/windows_x86_64_msvc/0.52.5/download"
    ],
    "strip_prefix": "windows_x86_64_msvc-0.52.5",
    "build_file": "@@[unknown repo '' requested from @@cxx.rs~]//third-party/bazel:BUILD.windows_x86_64_msvc-0.52.5.bazel"
  }
},

The fix is simple, if manual, we can replace those labels in the defs.bzl file with ones that reference the local top-level repo, i.e. replace the @// with // instead. However, it's unclear if this is correct. Why are these @// paths being generated? Is it valid to make this substitution?

Here's a quick reproduction of the error.

https://github.com/tel/cxx_bzlmod_repro

@AmeliasCode
Copy link
Contributor

That file is generated by this template:

build_file = Label("{{ crates_module_label(file="BUILD.bazel") }}"),

This uses the crates_module_label function defined here:

tera.register_function(
"crates_module_label",
module_label_fn_generator(render_config.crates_module_template.clone()),

This uses the crates_module_template to determine the output. That is defined here:

"crates_module_template": "@{}//{}:{{file}}".format(

@UebelAndre would simply updating that line be enough? Label() should give the absolute path to the object as seen from crates_vendor.bzl, so @ doesn't seem like it should be there?

github-merge-queue bot pushed a commit that referenced this issue Jul 29, 2024
Addresses #2661

If the workspace name is empty, the crate_module_template will use a
label like `Label(//<stuff>)` instead of `Label(@//<stuff>)`

Most of the changes in this are just regenerating the vendor'ed crates
in the examples. I ran the tests in all the relevant examples to ensure
they still worked.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants