Future of --experimental_sibling_repository_layout?

[lberki]

We have an experimental flag that costs quite a bit to maintain: --experimental_sibling_repository_layout.

It makes external repositories appear under ../$REPO instead of external/$REPO in the execroo and runfiles trees, which has two benefits:

1. It frees up the name external/. Otherwise, Bazel doesn't support projects that have this directory (which is kind of embarrassing)
2. It puts external repositories and the main repository on equal footing, which is more sound in principle.

As it stands, "it's a forever experimental" flag: we have it implemented, we pay the maintenance price, but we have not committed to migrating to it, which is the worst of all worlds: our users in general don't derive the above benefits (since they presumably don't opt in) and we pay the maintenance price.

I think the best plan is to flip it in Bazel 8, but this is mostly based on feelings ((1) is quite painful). However, it may be a costly migration, so I could be convinced to delete the flag and give up on the whole things.

@Wyverald @meisterT @comius do you have any thoughts?

[tpudlik]

Putting external repositories and the main repository on an equal footing is not necessarily a benefit. For example, in Pigweed we use the fact that external repos are under external/ to silence compiler warnings from external code using --per_file_copt=external/.*@-w in our bazelrc. This is essential if you want your toolchain to apply warnings of any strictness, since without this feature you'll be seeing warnings from your dependencies.

Incidentally, this should ideally be done through toolchain configuration rather than .bazelrc, but there's no support for that at present (bazelbuild/rules_cc#209).

[tpudlik]

The documentation is not very clear. At the very top is says, "--host_per_file_copt=<a comma-separated list of regex expressions with prefix '-' specifying excluded paths followed by an @ and a comma separated list of options>" (emphasis added). Since it explicitly refers to paths, it sounds like a path regex. But later, the example provided is --host_per_file_copt=//foo/.*\.cc,-//foo/bar\.cc@-O0, which looks like a label regex. Actually, if this is a label regex, I'm surprised --per_file_copt=external/.*@-w works at all (the labels don't include the external, do they?).

If --per_file_copt=^@.+//.*@-w did work, that would address my concern! (But is it certain that files in my own repository are never referred to using a label starting with their canonical repository name?)

[Wyverald]

I've only ever seen it used with execroot paths. Don't think labels would work.

[Wyverald]

Ah, looking at the code, it seems that the regex is used against both the label and the artifact path. The problem is (IIUC) that some artifacts might not have labels at all (like certain generated sources?)

But obviously the other problem is the @ being used as the separator between regex and options.

[Wyverald]

But is it certain that files in my own repository are never referred to using a label starting with their canonical repository name?

This is true as of today -- some file in your own repo would appear as //pkg:file.cc, with no starting @s at all. This is arguably a quirk of the Label.toString() behavior.

[illicitonion]

Reference to issue about this: #19665

[tjgq]

I agree that having two code paths is very unfortunate from a testing surface standpoint. I share the sentiment around the sibling layout being cleaner, but pragmatically speaking, if we can't find the time/energy to do the migration soon™, we should just cut our losses and accept the ugly/embarrassing API.

On that note, it would help to know how many people are actively using the sibling layout and haven't found any bugs in Bazel around it. (Otherwise, this might not be "just" a matter of flipping the flag and fixing downstream projects.)

[jin]

+1. This is a nearly decade-old design decision that's caused delays in Bazel adoptions (most notably in Android, when we had to implement this). The workaround is ugly (Bazel doesn't work in your source tree, so change your source tree), so IMO there's value in getting it over the hump, leveraging on major version changes.

[fmeum]

@tjgq #20500 (comment) is the one example of a bug in the sibling layout I am aware of.

[Wyverald]

It makes external repositories appear under ../$REPO instead of external/$REPO in the execroo and runfiles trees, which has two benefits:

Does this actually affect the runfiles tree? I thought runfiles trees were already organized as $runfilesRoot/$repoName without any nesting under $runfilesRoot/_main/external/$repoName.

do you have any thoughts?

I'm mostly worried about @tpudlik's comment: IMO we definitely need a solution to the --per_file_copts problem before we can even think about flipping this.

[lberki]

When the flag flipped, the paths of files in external repositories starts with ../ instead of external/ so the regex could be changed accordingly (or even to accommodate both alternatives).

So it looks like --per_file_copt isn't the thing this decision should hinge upon.

[fmeum]

I would also welcome the flag flip. Another benefit that hasn't been mentioned yet is that it aligns the directory structure of input files with that of runfiles, which would have helped me a lot when I initially got into rules authoring.

That said, if we want to flip this in Bazel 8, I think that we need to resolve the following issues soon (i.e., in time for Bazel 7.1 so that ruleset authors have ample time to prepare the migration):

1. https://groups.google.com/g/bazel-discuss/c/eyjCTJl3OHA: Even with --experimental_sibling_repository_layout, bazel-out for an external repo is still rooted under the main repo (e.g. __main__/bazel-out/other_repo/k8-fastbuild/bin/pkg/gen.txt). This voids most of the gains in conceptual simplicity and can also break builds if other_repo happens to be called k8-fastbuild.
2. Feature Request: local_includes for cc_library, making an easy path for good cross-platform hygiene #16472: Without a local_includes attribute on cc_library, users currently tend to hardcode external/... in BUILD files. There are unofficial workarounds (see bzlmod external repo name and copts #15553), so an alternative could be to standardize on one and make it available in rules_cc. This would be less discoverable though.

With these issues addressed and with buy-in from the Rules Authors SIG to address ruleset incompatibilities, this migration should be doable.

[lberki]

re (1): it was an explicit decision to put the bazel-out of external repositories under the cwd. If I recall correctly, one of the arguments for it was that it turned out that a single "output directory" was much easier to handle in the code than one for each external repository and it's also convenient that all output files are accessible under a single convenience symlink. It does open us up to name clashes like the way you mention and I'm still of the opinion that if we want to flip this flag, we might revisit this decision.

re (2), I think it's indicative of the long-tail issues we'd be facing if we committed to this flag flip.

[lberki]

I sent out #21785 to disable the flag :(