Add repository_ctx.original_name

[fmeum]

This new attribute contains the original value of the name attribute at the instantiation site of the repo rule (e.g., rctx.original_name would be foo if rctx.name is +ext+foo).

Fixes #24467

[fmeum]

Are these still a thing at HEAD? What's our strategy for backporting this, should I add new tests to the backports?

[fmeum]

@Wyverald @meteorcloudy Since rulesets still have to support WORKSPACE at this point, in which rctx.attr.name is a valid apparent repo name, I wonder whether we should instead make it so that:

• rctx.name returns the canonical repo name (current behavior)
• rctx.attr.name returns the original value of the name attribute (which equals ctx.name for WORKSPACE repos)

We wouldn't be able to cherry-pick the change, but for backwards compatibility users already need to resort to canonical repo name parsing where necessary (e.g. repo rules meant to be used with use_repo_rule). Advantages are that:

• It's pretty self-explanatory that rctx.attr.name returns the value of the attribute unmodified.
• We avoid introducing a third way to get a name for a repo rule.

Implementing this wouldn't be difficult, we could just monkey patch the StructImpl returned by rctx.attr to return the value of rctx.attr.$original_name for rctx.attr.name if set.

[meteorcloudy]

I don't have strong preference on either way, I trust you and @Wyverald to make the best decision.

But we should definitely document the best practices somewhere, I can follow up on that.

[Wyverald]

Since rulesets still have to support WORKSPACE at this point, in which rctx.attr.name is a valid apparent repo name, [...]

I don't quite follow this logic -- currently, in either Bzlmod or WORKSPACE, it's always true that rctx.name == rctx.attr.name. Changing the behavior of either one at this point would be breaking, and for unclear benefits.

Although TBH I agree that if we were to start from scratch, having the rctx.name vs rctx.attr.name dichotomy would've been nice.

[fmeum]

I don't quite follow this logic -- currently, in either Bzlmod or WORKSPACE, it's always true that rctx.name == rctx.attr.name

They are equal today, but I doubt anyone relies on that property. What I (badly) described above is that a repo rule today must be prepared to receive both foo (with WORKSPACE) and +ext+foo (with Bzlmod) in whichever of the two it uses. Changing one of them to be of the form foo all the time thus shouldn't be that breaking in practice.

If you don't think that's doable, I'm also happy to go with the approach that's currently implemented by this PR.

[Wyverald]

What I meant is that, technically, a repo rule could be relying on the semantics that rctx.name (and rctx.attr.name) refer to the canonical name of the repo (which has always been true in Bzlmod or WORKSPACE). For such a repo rule, it doesn't matter whether rctx.name is foo or +repo_rules3_foo or 8dwfu91e; it just needs @@${rctx.name}//:blah to be a valid label referring to itself.

With that said, I am struggling to think of a realistic example of such a repo rule... but IMO introducing an explicit API could avoid some headache in the future. Not a super strong opinion.

[fmeum]

We aren't locking us in, we could still change the behavior of ctx.attr.name in the future. I'm okay with the PR as is.

[fmeum]

@bazel-io fork 8.1.0

[iancha1992]

@fmeum can you please resolve the conflicts? Thanks

[fmeum]

@iancha1992 Done!

[satyanandak]

@fmeum could you please take a look at the presubmit failures?

[fmeum]

@satyanandak Fixed