Stage repository mapping manifest as a root symlink

[fmeum]

By adding the repository mapping manifest to runfiles as a root symlink, it is staged as foo.runfiles/_repo_mapping with all execution strategies. This includes sandboxed and remote execution, which previously did not stage the manifest at all.

As a side effect, runfiles libraries can now find the repository mapping manifest via rlocation("_repo_mapping").

Fixes #16643
Work towards #16124

[fmeum]

@lberki @Wyverald

[fmeum]

@Wyverald Had to fix a check on Windows and added another one for good measure.

[lberki]

This looks reasonable:

1. _repo_mapping cannot clash with repository names
2. Runfiles manifests are not currently in sandboxes / remote execution so there is no prior art with which we would be inconstent with
3. I just learned that we do create "symlink trees" on Windows by copying, which also works with this approach

I have one suggestion: this creates an extra Runfiles object, which may be a memory hog. How about adding an extra field to RunfilesSupplier and tweaking it so that it adds that to the runfiles trees it returns instead of creating that extra Runfiles instance?

[Wyverald]

to confirm, does this mean we might in some cases have exactly one of foo.runfiles/_repo_mapping and foo.repo_mapping present? And that's fine because it'll always be found by rlocation("_repo_mapping")?

[fmeum]

I have one suggestion: this creates an extra Runfiles object, which may be a memory hog. How about adding an extra field to RunfilesSupplier and tweaking it so that it adds that to the runfiles trees it returns instead of creating that extra Runfiles instance?

I will try to implement it that way as I share this concern.

to confirm, does this mean we might in some cases have exactly one of foo.runfiles/_repo_mapping and foo.repo_mapping present? And that's fine because it'll always be found by rlocation("_repo_mapping")?

Yes, with --noenable_runfiles, only foo.repo_mapping will be present, but pointed to by an entry in the runfiles manifest. rlocation("_repo_mapping") will always work (the integration test should ensure this).

[fmeum]

@lberki There are number of places that directly consume Runfiles.getRunfilesInputs and don't go through RunfilesSupplier, e.g. SymlinkTreeStrategy and SourceManifestAction. Do you have a suggestion for how I should deal with them?

[fmeum]

@lberki I pushed a commit that wires up the root symlink without allocating a new Runfiles in the most straightforward way I could find. It does seem to require passing the manifest into SourceManifestAction and SymlinkTreeAction, which is not so nice.

[fmeum]

@bazel-io flag

[lberki]

Yeah, this is kind of ugly, but I don't have a better idea, either (other than the previous "create new Runfiles instance" one, which I think is worse on the balance. Maybe add a comment to Runfiles.getRunfilesInputs() that explains why the repo mapping manifest can't be there?

Conceptually, the issue is that Runfiles is now two things:

1. What a configured target propagates upwards (and there is never a repo mapping manifest there)
2. What is put in the symlink tree (there may be a repo mapping manifest here)

So maybe the best would be to create two separate classes for these two use cases, but I won't insist.

[fmeum]

Maybe add a comment to Runfiles.getRunfilesInputs() that explains why the repo mapping manifest can't be there?

I added a comment.

So maybe the best would be to create two separate classes for these two use cases, but I won't insist.

Fully agree, but that would be a bit over my head right now.

[meteorcloudy]

@bazel-io fork 6.0.0