--
You received this message because you are subscribed to the Google Groups "bazel-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to bazel-dev+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-dev/CADMn-5aNVqOMx82vP%2BMAoLvVeUfths1YuU0OQ3obJziROUxJqw%40mail.gmail.com.
--
On Wed, May 18, 2022 at 12:00 AM Gregg Reynolds <d...@mobileink.com> wrote:
>
> IMO the Label function should construct labels from strings, period. If you want to then obtain the mapping from that label to (the label of) a canonical repo name, that's a separate operation.
With the proposal, the only way to get your hands on such a new label
literal in the first place would be to call `str` on a `Label` object,
which has been a relatively uncommon operation so far. For all label
literals that are valid today, the result of applying `Label` to them
wouldn't change. Would you prefer a new function or struct field (e.g.
`to_canonical_label()`) to be added to `Label` objects instead of
modifying their stringification behavior?
> But maybe you could get the same result by adding another level of scoping. Repository mapping already in fact does this - a `repo_mapping` is scoped to the repo in which it occurs, no? Can you just make this explicit? Maybe something as simple as @foo@bar//pkg:target, meaning @bar//pkg:target as mapped by @foo. Then you could also have e.g. @baz@bar//pkg:target, mapping to a different version of @bar.
It's important to keep in mind that, without any further context, any
unambiguous reference to a target has to include at least one
canonical repository name: "@foo@bar//pkg:target" is meaningless on
its own if "foo" is just an apparent repository name (borrowing
language from the proposal). To be unambiguous in general, only
"@foo.1.0.2@bar//pkg:target" has enough information to 1. look up the
repo mapping to apply from "foo.1.0.2"'s perspective and 2. apply it
to the apparent repo name "bar" to get the canonical repo name, say
"bar.2.0.4". But then the result is just a more complicated version of
the syntax proposed by Xudong, with which this label could instead be
written as "@@bar.2.0.4//pkg:target".
> More generally, the canonical label is an implementation detail that should not be exposed to Starlark code (IMO).
To be clear, I don't intend to pick on the particular label syntax
"@foo@bar//pkg:target", but rather point out the more general
complexity of the situation: At the moment, `Label` objects carry
additional information beyond what is reflected in their
stringification. If we want to allow repository rules to generate
BUILD files that reference targets passed in via labels (we most
certainly do, since this is a common use case), we need *some* way to
serialize all the information attached to a `Label` object into a
string that can later be reified into a `Label` object. Such a
serialization format would probably only be good for this purpose, but
it definitely has to be exposed to Starlark (in the form of BUILD
files) to fulfill it. Whether that is in the form of
"@@foo.1.0.3//pkg:target" (new syntax) or
"@_never_even_think_of_writing_this_yourself___foo.1.0.3//pkg:target"
(a long prefix to prevent this from colliding with existing labels) or
`labels.from_canonical_only_use_in_generated_code("@foo.1.0.3//pkg:target")`
(new function instead of new syntax) is something to be discussed and,
ultimately, shouldn't matter too much.
Fabian
>
> -Gregg
>
>
>
> On Mon, May 16, 2022 at 9:36 AM 'Xudong Yang' via bazel-dev <baze...@googlegroups.com> wrote:
>>
>> Hi all,
>>
>> I'm proposing to add a new label syntax to Bazel ("@@foo//:bar" -- note the extra "@"), which allows the label to bypass repo mapping. This will solve some pain points with the upcoming Bzlmod. Please feel free to leave comments!
>>
>> Thanks,
>> Xudong
>>
>> --
>> You received this message because you are subscribed to the Google Groups "bazel-dev" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to bazel-dev+...@googlegroups.com.
>> To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-dev/CADMn-5aNVqOMx82vP%2BMAoLvVeUfths1YuU0OQ3obJziROUxJqw%40mail.gmail.com.
>
> --
> You received this message because you are subscribed to the Google Groups "bazel-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to bazel-dev+...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-dev/CAO40MinFLK8NWGDNa0S6Ehhi1VQzhPBo6EaqV9xQNCPe5e9ouQ%40mail.gmail.com.
> Adding bzlmod should not change the meaning of the Label function. IMO the Label function should construct labels from strings, period.This is not new behavior introduced by Bzlmod; today, the Label function already passes the argument through the current repo's repo mapping. Imagine that you say repo A should map "foo" to "bar", and A creates a label using `Label("@foo//:target")`. IMO it would be completely surprising if that didn't get mapped to "@bar//:target"; what would A having a repo mapping even do, then?
You received this message because you are subscribed to the Google Groups "bazel-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discus...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/CADMn-5YeVD251gz-SiiO%2BEZ_%2BLOCgZH5Zqexfr0hsr%3DFL3E%3Diw%40mail.gmail.com.
> my gut feeling is that we are better off if e.g. "bazel query" reported the form of Label before repository remapping.What does this mean exactly? Say you have a dependency on @foo, and @foo has a dependency on @bar. (You have no visibility into @bar.) You do `bazel query deps(@foo//:target)`, and it spits out::other_target//my_pkg:target@bar//:somethingBecause these are the strings written in the BUILD file of @foo's root package, for the target called "target".Is that what you'd want? To me, that seems incredibly confusing and *very* error prone. I'd much rather it output something like the following:@foo//:other_target@foo//my_okg:target@@bar.1.0.2//:somethingSo that I know the first two deps I'm getting are actually in @foo, not in my main repo, and that the third thing is something that I don't normally have access to.> All of the above sounds like an argument for Fabian's proposed "Label.to_canonical()" function, doesn't it? (if it is not impossible to implement)It's definitely not impossible to implement, but again, what do you expect `str(Label("@bar//:something"))` to return when it's used from within @foo? Right now, it returns "@bar.1.0.2//:something", which is very much not ideal because this string cannot actually be used anywhere. The proposal changes it to "@@bar.1.0.2//:something". Is your expectation that it returns "@bar//:something" (which would be impossible to implement without storing an "original string" with every Label object -- and IMO a bad idea to implement in the first place), or something else?
On May 19, 2022, at 05:35, 'Xudong Yang' via bazel-dev <baze...@googlegroups.com> wrote:> I *personally* use it a lot to expand macros and the like, for which use case being as close to the original string the label was parsed from is unquestionably the best approachCould you give an example for "expand macros"? I feel like resolving the label is never *not* a good approach. Also, I can't stress this enough -- even today, the label is already always "resolved", even in `query` -- if you write "//pkg:target" in @foo, it's always understood as "@foo//pkg:target".
Is this such an example?-Gunnar
On May 19, 2022, at 05:35, 'Xudong Yang' via bazel-dev <baze...@googlegroups.com> wrote:> I *personally* use it a lot to expand macros and the like, for which use case being as close to the original string the label was parsed from is unquestionably the best approachCould you give an example for "expand macros"? I feel like resolving the label is never *not* a good approach. Also, I can't stress this enough -- even today, the label is already always "resolved", even in `query` -- if you write "//pkg:target" in @foo, it's always understood as "@foo//pkg:target".
In the context of the repo @foo, what does `Label.parse("@bar//pkg:target")` return? What's its type? And what does `Label.resolve("@bar//pkg:target")` return? What's its type?
I feel like resolving the label is never *not* a good approach.
Hi all,I'm proposing to add a new label syntax to Bazel ("@@foo//:bar" -- note the extra "@"), which allows the label to bypass repo mapping. This will solve some pain points with the upcoming Bzlmod. Please feel free to leave comments!
> I *personally* use it a lot to expand macros and the like, for which use case being as close to the original string the label was parsed from is unquestionably the best approachCould you give an example for "expand macros"? I feel like resolving the label is never *not* a good approach. Also, I can't stress this enough -- even today, the label is already always "resolved", even in `query` -- if you write "//pkg:target" in @foo, it's always understood as "@foo//pkg:target".
On Wed, May 18, 2022 at 12:00 AM Gregg Reynolds <d...@mobileink.com> wrote:
>
> IMO the Label function should construct labels from strings, period. If you want to then obtain the mapping from that label to (the label of) a canonical repo name, that's a separate operation.
With the proposal, the only way to get your hands on such a new label
literal in the first place would be to call `str` on a `Label` object,
which has been a relatively uncommon operation so far. For all label
literals that are valid today, the result of applying `Label` to them
wouldn't change. Would you prefer a new function or struct field (e.g.
`to_canonical_label()`) to be added to `Label` objects instead of
modifying their stringification behavior?
> But maybe you could get the same result by adding another level of scoping. Repository mapping already in fact does this - a `repo_mapping` is scoped to the repo in which it occurs, no? Can you just make this explicit? Maybe something as simple as @foo@bar//pkg:target, meaning @bar//pkg:target as mapped by @foo. Then you could also have e.g. @baz@bar//pkg:target, mapping to a different version of @bar.
It's important to keep in mind that, without any further context, any
unambiguous reference to a target has to include at least one
canonical repository name: "@foo@bar//pkg:target" is meaningless on
its own if "foo" is just an apparent repository name (borrowing
language from the proposal).
BUILD files that reference targets passed in via labels (we most
certainly do, since this is a common use case), we need *some* way to
serialize all the information attached to a `Label` object into a
string that can later be reified into a `Label` object. Such a
serialization format would probably only be good for this purpose, but
it definitely has to be exposed to Starlark (in the form of BUILD
files) to fulfill it. Whether that is in the form of
"@@foo.1.0.3//pkg:target" (new syntax) or
"@_never_even_think_of_writing_this_yourself___foo.1.0.3//pkg:target"
(a long prefix to prevent this from colliding with existing labels) or
`labels.from_canonical_only_use_in_generated_code("@foo.1.0.3//pkg:target")`
(new function instead of new syntax) is something to be discussed and,
ultimately, shouldn't matter too much.
> I *personally* use it a lot to expand macros and the like, for which use case being as close to the original string the label was parsed from is unquestionably the best approachCould you give an example for "expand macros"? I feel like resolving the label is never *not* a good approach. Also, I can't stress this enough -- even today, the label is already always "resolved", even in `query` -- if you write "//pkg:target" in @foo, it's always understood as "@foo//pkg:target".
> Maybe we can call new Label() a legacy API and provide e.g. Label.parse() and Label.resolve() to Starlark?
Sorry for being insistently pedantic, but what do you envision these to actually do? I have a hard time evaluating many of these arguments since they all appear very vague to me.In the context of the repo @foo, what does `Label.parse("@bar//pkg:target")` return? What's its type? And what does `Label.resolve("@bar//pkg:target")` return? What's its type?
It's a pretty big change and as such, I'm worried that some issue will slip through, regardless of how many smart people read the design doc, however carefully. Do you have any alternatives in mind that are less intrusive?
--
You received this message because you are subscribed to the Google Groups "bazel-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to bazel-dev+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-dev/CADMn-5aNVqOMx82vP%2BMAoLvVeUfths1YuU0OQ3obJziROUxJqw%40mail.gmail.com.