Expose "human usable" workspace names for labels

[shs96c]

Description of the feature request:

In rules_jvm_external we stamp jars so that buildifier can give accurate instructions. To do this, we need to know the workspace name that a human will use, rather than the canonical workspace name.

@fmeum has suggested opening up Label#getDisplayForm to Starlark to enable this.

Which category does this issue belong to?

Core, Documentation, External Dependency

What underlying problem are you trying to solve with this feature?

I need to be able to supply the workspace name that a user will use in dialogues and embedded information in artifacts.

Which operating system are you running Bazel on?

macOS

What is the output of bazel info release?

release 7.0.0rc7

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

Have you found anything relevant by searching the web?

This conversation on Slack

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

No response

[fmeum]

This is relevant even for the Java rules. Cc @hvadehra

[aiuto]

cc: @aiuto

[fmeum]

I could implement this as Label#to_display_form(). As @Wyverald described in the linked slack thread, making this a field rather than a method isn't feasible at this time.

It should be noted that this introduces the capability to sniff (and more crucially, branch on) whether something is a direct dependency of the root module, which is probably not something we want to encourage. But I don't see a way to prevent this and consider it acceptable.

[fmeum]

@justinhorvitz I initially resolved this issue by adding a to_display_form() method that returns the label as a string with an apparent repo name (if available). In particular, this matches the output of Label#toString() for labels pointing into the main repo.

I am now thinking that this isn't the best design: The current main use case for this label form is to pass an argument to a spawn that can be used for strict deps error messages, where labels should be safe to include in BUILD files, which canonical labels aren't.

However, doing that in a memory-efficient way With Args requires quite a bit of boilerplate. Ideally, all Label arguments to structured command lines would automatically end up formatted in this way, just like for main repo labels.

Do you see a way to get the required RepositoryMapping of the main repo into StarlarkCustomCommandline? A new field would probably result in a regression.

[justinhorvitz]

Can you give me an example of a label getting included in a command line via Args?

I am not too familiar with repository mappings because they don't exist internally. Is there anything in the ctx passed to the rule implementation function that would give you the repository mapping? If not, can we add it, then maybe use map_each to format labels?

[fmeum]

A Starlark example: https://cs.opensource.google/bazel/bazel/+/2ae85c16c7c7f6cc69a8412a44db8061d4126bf3:src/main/starlark/builtins_bzl/common/java/java_common_internal_for_builtins.bzl;l=378

A native example:
https://cs.opensource.google/bazel/bazel/+/2ae85c16c7c7f6cc69a8412a44db8061d4126bf3:src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileActionBuilder.java;l=318

In both cases, the label ends up in buildozer fixup commands and should thus use user-specified apparent repository names when available.

The BazelModuleContext has access to the repository mapping of a .bzl file's repository, which powers the Label() constructor. But for the case above, we always need the mapping of the main repository since that has the BUILD files that buildozer can edit. Repository mappings aren't objects that users should have arbitrary access to, so ideally they would just be able to turn a label into a string of this form - this is what Label.to_display_form() does.

However, while this function can be used with map_each, this (at least the native equivalent) appears to cause a memory regression at Google and is in particular wasteful for main repo labels. Thus, correct usage would look something like this:

def _to_display_form(label):
    return label.to_display_form()

def _my_rule_impl(ctx);
    ...
    if some_label.repo_name:
        args.add_all([some_label], map_each = _to_display_form)
    else:
        args.add(some_label)

This is very verbose and makes me think that to_display_form just isn't the right API. Since I don't know of any use case for adding a collection of labels to a command line, my current favorite solution would be to make args.add(some_label) work by type checking the argument and prepending the RepositoryMapping in StarlarkCustomCommandline if it is a label pointing into an external repo. But of course having all labels be subject to this, not just those in scalar args, would be cleaner. What do you think about that?

[justinhorvitz]

this (at least the native equivalent) appears to cause a memory regression at Google and is in particular wasteful for main repo labels

Are you referring to #21180? The significant memory cost for that was the skyframe dep from every configured target on the repository mapping, not the command line. In fact, the benchmark build did not show any significant regression when run with --notrack_incremental_state in which edges are not stored.

We could instead try an approach like what CachingAnalysisEnvironment does for the stamp artifacts - request them from skyframe when the getter method is called, and use MissingDepException if not yet available (although I imagine the main repo mapping would always be available at that point).

my current favorite solution would be to make Args.add(some_label) work by type checking the argument and prepending the RepositoryMapping

I'm concerned about how this would change the behavior of any actions that currently pass a label in such a manner. Someone more familiar would have to weigh in about whether this concern is valid, but if so, you could create a specialized Args.add_display_label method.

Another option is something like Label.to_display_form(ctx) in which the implementation can avoid a skyframe dep altogether if the label is in the main repo, or else request the main repo mapping as a skyframe dep from the AnalysisEnvironment as discussed above.

[shs96c]

What I'm considering as an alternative is to override StarlarkValue#debugPrint for Label and have that return the display form. Since the return value isn't available to Starlark, the dependency wouldn't need to be tracked.

This would be sufficient for my needs, yes.