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

Repository rules aren't refetched when their repo mappings change #20722

Closed
fmeum opened this issue Jan 3, 2024 · 6 comments
Closed

Repository rules aren't refetched when their repo mappings change #20722

fmeum opened this issue Jan 3, 2024 · 6 comments
Labels
P2 We'll consider working on this in future. (Assignee optional) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: bug

Comments

@fmeum
Copy link
Collaborator

fmeum commented Jan 3, 2024

Description of the bug:

This is similar to #20721, but 1) about repository rules rather than module extensions and 2) not a regression in 7.0.0.

A repository rule isn't refetched when its repo mapping changes, which can result in Label(...) calls made in the implementation function to return stale results when canonical repo names change, e.g. during bazel_dep version bumps.

Which category does this issue belong to?

External Dependency

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

# MODULE.bazel
bazel_dep(name = "rules_go", version = "0.40.0")

ext = use_extension("//:ext.bzl", "ext")
ext.tag(label_str = "@rules_go//go")
use_repo(ext, "repo")

# BUILD
load("@repo//:defs.bzl", "LABEL")
print(LABEL)

# ext.bzl
def _repo_impl(ctx):
    ctx.file("WORKSPACE")
    ctx.file("BUILD")
    ctx.file("defs.bzl", "LABEL = Label({})".format(repr(str(Label(ctx.attr.label_str)))))

_repo = repository_rule(
    implementation=_repo_impl,
    attrs={"label_str": attr.string(mandatory=True)},
)

_tag = tag_class(
    attrs={"label_str": attr.string(mandatory=True)},
)

def _ext_impl(ctx):
    for module in ctx.modules:
        for tag in module.tags.tag:
            _repo(name = "repo", label_str = tag.label_str)

ext = module_extension(
    implementation=_ext_impl,
    tag_classes={"tag": _tag},
)
$ bazel build //... --lockfile_mode=off
DEBUG: /home/fhenneke/tmp/repo-mapping-lock/BUILD:2:6: @@rules_go~0.40.0//go:go
$ buildozer 'set version 0.41.0' MODULE.bazel:rules_go
$ bazel build //... --lockfile_mode=off
<no output since the old warning has already been omitted and hasn't changed>
$ bazel shutdown
$ bazel build //... --lockfile_mode=off
DEBUG: /home/fhenneke/tmp/repo-mapping-lock/BUILD:2:6: @@rules_go~0.40.0//go:go

Which operating system are you running Bazel on?

No response

What is the output of bazel info release?

7.0.0 and HEAD

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

@fmeum
Copy link
Collaborator Author

fmeum commented Jan 9, 2024

@bazel-io fork 7.0.1

@fmeum
Copy link
Collaborator Author

fmeum commented Jan 9, 2024

@Wyverald @meteorcloudy This, combined with f4be0b2#diff-ef4da5cc2da76cded220009a0dcc95373700d704629a1a2e9ebe84a9be076854, actually breaks jvm_maven_import_external and http_jar when rules_java is updated, only workaround is to manually refetch the maven repo.

@fmeum
Copy link
Collaborator Author

fmeum commented Jan 9, 2024

If you don't plan to solve this for 7.0.1, we could alternatively fix the more immediate issue by getting rid of the rules_java loads in all Bazel-provided repo rules, instead relying on the native symbols.

fmeum added a commit to fmeum/bazel that referenced this issue Jan 9, 2024
As long as bazelbuild#20722 isn't resolved, the canonical name for the given
apparent name can change without the repo rule being refetched.
@fmeum
Copy link
Collaborator Author

fmeum commented Jan 9, 2024

Feel free to downgrade this to 7.1.0 if you think that #20810 is reasonable in the meantime.

fmeum added a commit to fmeum/bazel that referenced this issue Jan 9, 2024
As long as bazelbuild#20722 isn't resolved, the canonical name for the given
apparent name can change without the repo rule being refetched.
fmeum added a commit to fmeum/bazel that referenced this issue Jan 9, 2024
As long as bazelbuild#20722 isn't resolved, the canonical name for the given
apparent name can change without the repo rule being refetched.
@iancha1992
Copy link
Member

@bazel-io fork 7.1.0

copybara-service bot pushed a commit that referenced this issue Jan 9, 2024
As long as #20722 isn't resolved, the canonical name for the given apparent name can change without the repo rule being refetched.

Closes #20810.

PiperOrigin-RevId: 597048244
Change-Id: I225424cc32e572b26c6d6e76e2c09c4d2e6a4ba6
fmeum added a commit to fmeum/bazel that referenced this issue Jan 10, 2024
As long as bazelbuild#20722 isn't resolved, the canonical name for the given apparent name can change without the repo rule being refetched.

Closes bazelbuild#20810.

PiperOrigin-RevId: 597048244
Change-Id: I225424cc32e572b26c6d6e76e2c09c4d2e6a4ba6

Closes bazelbuild#20825
fmeum added a commit to fmeum/bazel that referenced this issue Jan 10, 2024
As long as bazelbuild#20722 isn't resolved, the canonical name for the given apparent name can change without the repo rule being refetched.

Closes bazelbuild#20810.

PiperOrigin-RevId: 597048244
Change-Id: I225424cc32e572b26c6d6e76e2c09c4d2e6a4ba6

Closes bazelbuild#20825
iancha1992 pushed a commit that referenced this issue Jan 10, 2024
As long as #20722 isn't resolved, the canonical name for the given
apparent name can change without the repo rule being refetched.

Closes #20810.

PiperOrigin-RevId: 597048244
Change-Id: I225424cc32e572b26c6d6e76e2c09c4d2e6a4ba6

Closes #20825
@Wyverald Wyverald added P2 We'll consider working on this in future. (Assignee optional) and removed untriaged labels Jan 10, 2024
github-merge-queue bot pushed a commit that referenced this issue Jan 17, 2024
As long as #20722 isn't resolved, the canonical name for the given
apparent name can change without the repo rule being refetched.

Closes #20810.
Commit
1c45bc5

PiperOrigin-RevId: 597048244
Change-Id: I225424cc32e572b26c6d6e76e2c09c4d2e6a4ba6

---------

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
Wyverald added a commit that referenced this issue Jan 29, 2024
Similar to the fix for #20721, we write recorded repo mapping entries into the marker file so that repo fetching is sensitive to any relevant repo mapping changes.

Fixes #20722.
bazel-io pushed a commit to bazel-io/bazel that referenced this issue Feb 1, 2024
Similar to the fix for bazelbuild#20721, we write recorded repo mapping entries into the marker file so that repo fetching is sensitive to any relevant repo mapping changes.

Fixes bazelbuild#20722.

Closes bazelbuild#21131.

PiperOrigin-RevId: 603310262
Change-Id: I806f383e8579fed3533fac9b181efd8b75e76fcd
github-merge-queue bot pushed a commit that referenced this issue Feb 1, 2024
)

Similar to the fix for #20721,
we write recorded repo mapping entries into the marker file so that repo
fetching is sensitive to any relevant repo mapping changes.

Fixes #20722.

Closes #21131.

Commit
9edaddd

PiperOrigin-RevId: 603310262
Change-Id: I806f383e8579fed3533fac9b181efd8b75e76fcd

Co-authored-by: Xdng Yng <wyverald@gmail.com>
@iancha1992
Copy link
Member

A fix for this issue has been included in Bazel 7.1.0 RC1. Please test out the release candidate and report any issues as soon as possible. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants