Emit labels in display form in Java rules

[fmeum]

Buildozer fixes for Java strict deps violations referring to external repositories now contain labels in display form, which avoid canonical repository names that should never be added to BUILD files and aren't understood by buildozer.

The Starlark actions can use Label.to_display_form(). Java compilation actions, which are still implemented in Java, can't access the main repository mapping via BazelModuleContext and instead retrieve it from a new field on AnalysisEnvironment.

[fmeum]

Stacked on #21179, please ignore the first commit during review.

[fmeum]

@hvadehra This PR is no longer stacked.

[hvadehra]

LGTM for java changes.

@meteorcloudy for tests now requiring network.

[meteorcloudy]

Yep, currently there is no way around it. Adding requires-network looks fine.

[fmeum]

@bazel-io fork 7.1.0

[meteorcloudy]

Unfortunately, this change was rolled back at fd048d3 due to a memory regression.

[fmeum]

@hvadehra Do you know whether the Starlark part would have been fine by itself? If so, I could try to pass this argument into the native action from Starlark instead.

[hvadehra]

@fmeum As noted in fd048d3, the issue is we're adding a new skyframe edge for every target/aspect. The exact cost of such a edge is a little hard to calculate in general due to skyframe optimizations. It can be particularly expensive for certain transitions in the number of edges (cc @justinhorvitz to keep me honest about this).

Additionally, we'd like to avoid the edge completely internally (i.e. no external repositories). Even for Bazel, it would be nice to only add the edge when required. Not sure if there's a good way to do that though. Solving the latter should also solve the former though.

[hvadehra]

Do you know whether the Starlark part would have been fine by itself? If so, I could try to pass this argument into the native action from Starlark instead.

I tried out an experimental fix yesterday which dropped the edge in the target/aspect functions when external repos were not in play, and there was still some regression. So I suspect not. But I can try and measure more exactly.

[fmeum]

@hvadehra Thanks for the explanation. I will try to hide this edge in StarlarkBuiltinsValue instead. We won't need it anymore when all rules have been Starlarkified, so moving it there doesn't sound like too much of a hack. Would it be okay to have that single edge also internally or would you like me to make it conditional too?

[fmeum]

I tried out an experimental fix yesterday which dropped the edge in the target/aspect functions when external repos were not in play, and there was still some regression. So I suspect not. But I can try and measure more exactly.

I have already been a bit worried about the memory impact of an additional VectorArg + ImmutableList instance. I could get rid of the list, but also removing the VectorArg wouldn't really be feasible.

I will submit separate PRs for Starlark and native actions.