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

Create a way for macros to "reify" string labels into label objects #15172

Closed
Wyverald opened this issue Apr 4, 2022 · 3 comments
Closed

Create a way for macros to "reify" string labels into label objects #15172

Wyverald opened this issue Apr 4, 2022 · 3 comments
Labels
area-Bzlmod Bzlmod-specific PRs, issues, and feature requests P2 We'll consider working on this in future. (Assignee optional) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request

Comments

@Wyverald
Copy link
Member

Wyverald commented Apr 4, 2022

The immediately obvious answer seems to be "just call Label(...)", except that would anchor the string label at the current .bzl file's context. For a macro that receives string labels from BUILD files, it would probably want to convert string labels with the BUILD file's context. Example:

# @myproject//pkg/BUILD
load("@myrule//myrule:defs.bzl", "mymacro")
mymacro(
    name = "mytarget",
    deps = [":hello"],  # Intended to be @myproject//pkg:hello
)
filegroup(name = "hello")


# @myrule//myrule/defs.bzl
DEFAULT_DEP = Label("//myrule:compiler")  # Always points to @myrule//myrule:compiler, regardless of repo mappings
def mymacro(name, deps):
    if DEFAULT_DEP not in deps:  # THIS LINE DOES NOT WORK
        deps += [DEFAULT_DEP]
    myrule(name, deps)

Here mymacro simply includes an extra dep before forwarding the call to myrule. But the check is almost never true because deps almost always contains only strings! We can't do [Label(dep) for dep in deps] because that would mean ":hello" gets turned into @myrule//myrule:hello instead of @myproject//pkg:hello. We also can't check if str(DEFAULT_DEP) not in deps because deps can contain non-absolute labels, whereas str(DEFAULT_DEP) is an absolute label.

We need a way to turn a string into a Label using the calling package's context. This is actually exactly the deprecated Label(..., relative_to_caller_package=True) constructor that we removed, thinking nobody would ever use it. It turns out that's not true (see protocolbuffers/protobuf#9688). We should maybe add it back, and maybe think of a better API for it.

cc @brandjon @meteorcloudy

@Wyverald Wyverald added type: feature request P2 We'll consider working on this in future. (Assignee optional) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. area-Bzlmod Bzlmod-specific PRs, issues, and feature requests labels Apr 4, 2022
@pcjanzen
Copy link
Contributor

pcjanzen commented Apr 4, 2022

does
Label(native.repository_name() + "//" + native.package_name()).relative(dep)
solve this problem?
(since native.repository_name() and native.package_name() are relative to the caller package?)

@Wyverald
Copy link
Member Author

Wyverald commented Apr 4, 2022

That's a nice trick! Unfortunately it breaks when repo mappings are involved. Label(...) resolves the text using the current .bzl file's repo mapping, but native.repository_name() is the "canonical" repo name (post-mapping).

However, this idea could still be valuable if we decide to fix #14580 by allowing some sort of "canonical-repo-name absolute label literals".

@Wyverald
Copy link
Member Author

Duping against #15593

@Wyverald Wyverald closed this as not planned Won't fix, can't repro, duplicate, stale May 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Bzlmod Bzlmod-specific PRs, issues, and feature requests P2 We'll consider working on this in future. (Assignee optional) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request
Projects
None yet
Development

No branches or pull requests

2 participants