facebook
`//A/B/C` is a valid name for `//A/B/C:C`
Prefer using the short name when referring to an eponymous target (
//xinstead of//x:x). If you are in the same package, prefer the local reference (:xinstead of//x).
See: https://bazel.build/build/style-guide#target-naming
This syntax is supported on the command-line, but not in BUCK files. Unfortunately, the only(?) Starlark formatter (buildifier: https://github.com/bazelbuild/buildtools) automatically abbreviates targets on the assumption that this syntax is valid, which causes errors when using Buck2:
...
4: Error coercing "//src/Foo"
5: Invalid absolute target pattern `//src/Foo` is not allowed
6: Expected a `:`, a trailing `/...` or the literal `...`.
We can adjust the parsing rules to allow this syntax in more places.
Supercedes https://github.com/facebook/buck2/pull/925
@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. (Because this pull request was imported automatically, there will not be any future comments.)
From #925
Discussed this with @wavewave. We would really like to have this feature because of Bazel compatibility and dev familiarity (fewer divergences with the command line and less repeating yourself) and we decided to keep holding on this patch internally as we think it's quite important to UX, so I would appreciate getting it upstreamed.
I think the solution to the short target syntax being objectionable/confusing when reading buck files is to have the formatter rewrite it to the canonical form, if that's an important property to have internally at Meta.
I missed the original patch and to be honest, not sure how much it means for my vote, but I'd quite like this in my codebases too, I think (FTR, I didn't know Bazel did this.) I rely on the shorthand syntax on the command line extensively because it's nice with the autocomplete support, so I agree it'd be a nice bit of uniformity.
Thanks for resurrecting this! I might try it out in my fork for a few days.
Alright, so we chatted about this in our team syncs. We're not super thrilled about this - the general vibe is that we want people to understand what the things they write mean, and this probably hurts that. This applies to the command line too fwiw, I don't think we'd feel great about implementing it today if we hadn't done that already.
That being said though, bazel interop seems like an ok reason to add this and if someone does want to use it that's probably up to them. So we're ok with this, under the requirement that this has to be behind an off-by-default buckconfig
I was looking at this PR recently and wondering how feasible it was to add a buckconfig flag for it. I've experimented with it a little and actually think it's nice, but haven't migrated code to use it yet. Unfortunately, it's so deep inside the code I'm not sure there's an easy way to get the buck context there, without just threading a parameter through everything? So that's an annoying lift.
This applies to the command line too fwiw, I don't think we'd feel great about implementing it today if we hadn't done that already.
Honestly, I find this one to be a useful timesaver feature as a user on the CLI. It makes shell completion much quicker without having to tab through a selection menu on the targets of a given package and pick the "right" one. Normally I'm just autocompleting what are effectively directory paths in the root// cell anyway, so just completing the directory is enough. So I guess this is just to say I think it's useful even if some find it a little ugly. :)
That being said though, bazel interop seems like an ok reason to add this and if someone does want to use it that's probably up to them. So we're ok with this, under the requirement that this has to be behind an off-by-default buckconfig
One problem I realized with this approach is: what happens when you try to use some code that uses the short-syntax, but you want to prohibit the short-syntax in your own code? For example, what happens in this scenario where I depend on an external cell foo//, and foo// needs this feature enabled (because its BUILD files are written assuming it is), but root// does not want it enabled? Can this scenario be handled correctly? There might be others I haven't thought of.
I wonder if there is a better approach in possibly making the use of this feature some kind of toggle-able warning/lint error instead of strictly enabling/disabling it, and instead it was always enabled all the time. Then users can just turn off the warning instead, but at Meta it would remain a hard error, of some sort?
still TODO: address feedback. I need to figure out how to thread the config through.