Skip to content

Commit

Permalink
Stage repository mapping manifest as a root symlink (#16733)
Browse files Browse the repository at this point in the history
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

Closes #16652.

PiperOrigin-RevId: 487532254
Change-Id: I9774b8930337c5967fce92a861cc0db71dea2f0f
  • Loading branch information
fmeum authored Nov 10, 2022
1 parent 38c5019 commit 5929cb7
Show file tree
Hide file tree
Showing 16 changed files with 161 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -386,11 +386,14 @@ static Map<PathFragment, Artifact> filterListForObscuringSymlinks(
* normal source tree entries, or runfile conflicts. May be null, in which case obscuring
* symlinks are silently discarded, and conflicts are overwritten.
* @param location Location for eventHandler warnings. Ignored if eventHandler is null.
* @param repoMappingManifest repository mapping manifest to add as a root symlink. This manifest
* has to be added automatically for every executable and is thus not part of the Runfiles
* advertised by a configured target.
* @return Map<PathFragment, Artifact> path fragment to artifact, of normal source tree entries
* and elements that live outside the source tree. Null values represent empty input files.
*/
public Map<PathFragment, Artifact> getRunfilesInputs(
EventHandler eventHandler, Location location) {
EventHandler eventHandler, Location location, @Nullable Artifact repoMappingManifest) {
ConflictChecker checker = new ConflictChecker(conflictPolicy, eventHandler, location);
Map<PathFragment, Artifact> manifest = getSymlinksAsMap(checker);
// Add artifacts (committed to inclusion on construction of runfiles).
Expand All @@ -417,6 +420,9 @@ public Map<PathFragment, Artifact> getRunfilesInputs(
checker = new ConflictChecker(ConflictPolicy.WARN, eventHandler, location);
}
builder.add(getRootSymlinksAsMap(checker), checker);
if (repoMappingManifest != null) {
checker.put(builder.manifest, PathFragment.create("_repo_mapping"), repoMappingManifest);
}
return builder.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,18 +132,20 @@ private static RunfilesSupport create(
}
Preconditions.checkState(!runfiles.isEmpty());

Artifact repoMappingManifest =
createRepoMappingManifestAction(ruleContext, runfiles, owningExecutable);

Artifact runfilesInputManifest;
Artifact runfilesManifest;
if (createManifest) {
runfilesInputManifest = createRunfilesInputManifestArtifact(ruleContext, owningExecutable);
runfilesManifest =
createRunfilesAction(ruleContext, runfiles, buildRunfileLinks, runfilesInputManifest);
createRunfilesAction(
ruleContext, runfiles, buildRunfileLinks, runfilesInputManifest, repoMappingManifest);
} else {
runfilesInputManifest = null;
runfilesManifest = null;
}
Artifact repoMappingManifest =
createRepoMappingManifestAction(ruleContext, runfiles, owningExecutable);
Artifact runfilesMiddleman =
createRunfilesMiddleman(
ruleContext, owningExecutable, runfiles, runfilesManifest, repoMappingManifest);
Expand Down Expand Up @@ -387,7 +389,8 @@ private static Artifact createRunfilesAction(
ActionConstructionContext context,
Runfiles runfiles,
boolean createSymlinks,
Artifact inputManifest) {
Artifact inputManifest,
@Nullable Artifact repoMappingManifest) {
// Compute the names of the runfiles directory and its MANIFEST file.
context
.getAnalysisEnvironment()
Expand All @@ -397,6 +400,7 @@ private static Artifact createRunfilesAction(
context.getActionOwner(),
inputManifest,
runfiles,
repoMappingManifest,
context.getConfiguration().remotableSourceManifestActions()));

if (!createSymlinks) {
Expand All @@ -423,6 +427,7 @@ private static Artifact createRunfilesAction(
inputManifest,
runfiles,
outputManifest,
repoMappingManifest,
/*filesetRoot=*/ null));
return outputManifest;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,12 @@
/** {@link RunfilesSupplier} implementation wrapping a single {@link Runfiles} directory mapping. */
@AutoCodec
public final class SingleRunfilesSupplier implements RunfilesSupplier {

private final PathFragment runfilesDir;
private final Runfiles runfiles;
private final Supplier<Map<PathFragment, Artifact>> runfilesInputs;
@Nullable private final Artifact manifest;
@Nullable private final Artifact repoMappingManifest;
private final boolean buildRunfileLinks;
private final boolean runfileLinksEnabled;

Expand All @@ -50,28 +52,31 @@ public static SingleRunfilesSupplier create(RunfilesSupport runfilesSupport) {
runfilesSupport.getRunfiles(),
/*runfilesCachingEnabled=*/ false,
/*manifest=*/ null,
runfilesSupport.getRepoMappingManifest(),
runfilesSupport.isBuildRunfileLinks(),
runfilesSupport.isRunfilesEnabled());
}

/**
* Same as {@link SingleRunfilesSupplier#SingleRunfilesSupplier(PathFragment, Runfiles, Artifact,
* boolean, boolean)}, except adds caching for {@linkplain Runfiles#getRunfilesInputs runfiles
* inputs}.
* Artifact, boolean, boolean)}, except adds caching for {@linkplain Runfiles#getRunfilesInputs
* runfiles inputs}.
*
* <p>The runfiles inputs are computed lazily and softly cached. Caching is shared across
* instances created via {@link #withOverriddenRunfilesDir}.
*/
public static SingleRunfilesSupplier createCaching(
PathFragment runfilesDir,
Runfiles runfiles,
@Nullable Artifact repoMappingManifest,
boolean buildRunfileLinks,
boolean runfileLinksEnabled) {
return new SingleRunfilesSupplier(
runfilesDir,
runfiles,
/*runfilesCachingEnabled=*/ true,
/*manifest=*/ null,
repoMappingManifest,
buildRunfileLinks,
runfileLinksEnabled);
}
Expand All @@ -92,13 +97,15 @@ public SingleRunfilesSupplier(
PathFragment runfilesDir,
Runfiles runfiles,
@Nullable Artifact manifest,
@Nullable Artifact repoMappingManifest,
boolean buildRunfileLinks,
boolean runfileLinksEnabled) {
this(
runfilesDir,
runfiles,
/*runfilesCachingEnabled=*/ false,
manifest,
repoMappingManifest,
buildRunfileLinks,
runfileLinksEnabled);
}
Expand All @@ -108,15 +115,19 @@ private SingleRunfilesSupplier(
Runfiles runfiles,
boolean runfilesCachingEnabled,
@Nullable Artifact manifest,
@Nullable Artifact repoMappingManifest,
boolean buildRunfileLinks,
boolean runfileLinksEnabled) {
this(
runfilesDir,
runfiles,
runfilesCachingEnabled
? new RunfilesCacher(runfiles)
: () -> runfiles.getRunfilesInputs(/*eventHandler=*/ null, /*location=*/ null),
? new RunfilesCacher(runfiles, repoMappingManifest)
: () ->
runfiles.getRunfilesInputs(
/*eventHandler=*/ null, /*location=*/ null, repoMappingManifest),
manifest,
repoMappingManifest,
buildRunfileLinks,
runfileLinksEnabled);
}
Expand All @@ -126,13 +137,15 @@ private SingleRunfilesSupplier(
Runfiles runfiles,
Supplier<Map<PathFragment, Artifact>> runfilesInputs,
@Nullable Artifact manifest,
@Nullable Artifact repoMappingManifest,
boolean buildRunfileLinks,
boolean runfileLinksEnabled) {
checkArgument(!runfilesDir.isAbsolute());
this.runfilesDir = checkNotNull(runfilesDir);
this.runfiles = checkNotNull(runfiles);
this.runfilesInputs = checkNotNull(runfilesInputs);
this.manifest = manifest;
this.repoMappingManifest = repoMappingManifest;
this.buildRunfileLinks = buildRunfileLinks;
this.runfileLinksEnabled = runfileLinksEnabled;
}
Expand Down Expand Up @@ -199,17 +212,21 @@ public SingleRunfilesSupplier withOverriddenRunfilesDir(PathFragment newRunfiles
runfiles,
runfilesInputs,
manifest,
repoMappingManifest,
buildRunfileLinks,
runfileLinksEnabled);
}

/** Softly caches the result of {@link Runfiles#getRunfilesInputs}. */
private static final class RunfilesCacher implements Supplier<Map<PathFragment, Artifact>> {

private final Runfiles runfiles;
@Nullable private final Artifact repoMappingManifest;
private volatile SoftReference<Map<PathFragment, Artifact>> ref = new SoftReference<>(null);

RunfilesCacher(Runfiles runfiles) {
RunfilesCacher(Runfiles runfiles, @Nullable Artifact repoMappingManifest) {
this.runfiles = runfiles;
this.repoMappingManifest = repoMappingManifest;
}

@Override
Expand All @@ -221,7 +238,9 @@ public Map<PathFragment, Artifact> get() {
synchronized (this) {
result = ref.get();
if (result == null) {
result = runfiles.getRunfilesInputs(/*eventHandler=*/ null, /*location=*/ null);
result =
runfiles.getRunfilesInputs(
/*eventHandler=*/ null, /*location=*/ null, repoMappingManifest);
ref = new SoftReference<>(result);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public final class SourceManifestAction extends AbstractFileWriteAction {

private static final Comparator<Map.Entry<PathFragment, Artifact>> ENTRY_COMPARATOR =
(path1, path2) -> path1.getKey().getPathString().compareTo(path2.getKey().getPathString());

private final Artifact repoMappingManifest;
/**
* Interface for defining manifest formatting and reporting specifics. Implementations must be
* immutable.
Expand Down Expand Up @@ -118,7 +118,7 @@ void writeEntry(
@VisibleForTesting
SourceManifestAction(
ManifestWriter manifestWriter, ActionOwner owner, Artifact primaryOutput, Runfiles runfiles) {
this(manifestWriter, owner, primaryOutput, runfiles, /*remotableSourceManifestActions=*/ false);
this(manifestWriter, owner, primaryOutput, runfiles, null, false);
}

/**
Expand All @@ -129,17 +129,20 @@ void writeEntry(
* @param owner the action owner
* @param primaryOutput the file to which to write the manifest
* @param runfiles runfiles
* @param repoMappingManifest the repository mapping manifest for runfiles
*/
public SourceManifestAction(
ManifestWriter manifestWriter,
ActionOwner owner,
Artifact primaryOutput,
Runfiles runfiles,
@Nullable Artifact repoMappingManifest,
boolean remotableSourceManifestActions) {
// The real set of inputs is computed in #getInputs().
super(owner, NestedSetBuilder.emptySet(Order.STABLE_ORDER), primaryOutput, false);
this.manifestWriter = manifestWriter;
this.runfiles = runfiles;
this.repoMappingManifest = repoMappingManifest;
this.remotableSourceManifestActions = remotableSourceManifestActions;
}

Expand Down Expand Up @@ -180,7 +183,9 @@ public synchronized NestedSet<Artifact> getInputs() {
@VisibleForTesting
public void writeOutputFile(OutputStream out, @Nullable EventHandler eventHandler)
throws IOException {
writeFile(out, runfiles.getRunfilesInputs(eventHandler, getOwner().getLocation()));
writeFile(
out,
runfiles.getRunfilesInputs(eventHandler, getOwner().getLocation(), repoMappingManifest));
}

/**
Expand All @@ -202,7 +207,8 @@ public String getStarlarkContent() throws IOException {
@Override
public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) {
final Map<PathFragment, Artifact> runfilesInputs =
runfiles.getRunfilesInputs(ctx.getEventHandler(), getOwner().getLocation());
runfiles.getRunfilesInputs(
ctx.getEventHandler(), getOwner().getLocation(), repoMappingManifest);
return out -> writeFile(out, runfilesInputs);
}

Expand Down Expand Up @@ -247,6 +253,10 @@ protected void computeKey(
fp.addString(GUID);
fp.addBoolean(remotableSourceManifestActions);
runfiles.fingerprint(fp);
fp.addBoolean(repoMappingManifest != null);
if (repoMappingManifest != null) {
fp.addPath(repoMappingManifest.getExecPath());
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public final class SymlinkTreeAction extends AbstractAction {
private final boolean enableRunfiles;
private final boolean inprocessSymlinkCreation;
private final boolean skipRunfilesManifests;
private final Artifact repoMappingManifest;

/**
* Creates SymlinkTreeAction instance.
Expand All @@ -59,6 +60,7 @@ public final class SymlinkTreeAction extends AbstractAction {
* @param runfiles the input runfiles
* @param outputManifest the generated symlink tree manifest (must have "MANIFEST" base name).
* Symlink tree root will be set to the artifact's parent directory.
* @param repoMappingManifest the repository mapping manifest
* @param filesetRoot non-null if this is a fileset symlink tree
*/
public SymlinkTreeAction(
Expand All @@ -67,12 +69,14 @@ public SymlinkTreeAction(
Artifact inputManifest,
@Nullable Runfiles runfiles,
Artifact outputManifest,
@Nullable Artifact repoMappingManifest,
String filesetRoot) {
this(
owner,
inputManifest,
runfiles,
outputManifest,
repoMappingManifest,
filesetRoot,
config.getActionEnvironment(),
config.runfilesEnabled(),
Expand All @@ -90,21 +94,24 @@ public SymlinkTreeAction(
* @param runfiles the input runfiles
* @param outputManifest the generated symlink tree manifest (must have "MANIFEST" base name).
* Symlink tree root will be set to the artifact's parent directory.
* @param repoMappingManifest the repository mapping manifest
* @param filesetRoot non-null if this is a fileset symlink tree,
*/
public SymlinkTreeAction(
ActionOwner owner,
Artifact inputManifest,
@Nullable Runfiles runfiles,
Artifact outputManifest,
@Nullable Artifact repoMappingManifest,
@Nullable String filesetRoot,
ActionEnvironment env,
boolean enableRunfiles,
boolean inprocessSymlinkCreation,
boolean skipRunfilesManifests) {
super(
owner,
computeInputs(enableRunfiles, skipRunfilesManifests, runfiles, inputManifest),
computeInputs(
enableRunfiles, skipRunfilesManifests, runfiles, inputManifest, repoMappingManifest),
ImmutableSet.of(outputManifest),
env);
Preconditions.checkArgument(outputManifest.getPath().getBaseName().equals("MANIFEST"));
Expand All @@ -118,13 +125,15 @@ public SymlinkTreeAction(
this.inprocessSymlinkCreation = inprocessSymlinkCreation;
this.skipRunfilesManifests = skipRunfilesManifests && enableRunfiles && (filesetRoot == null);
this.inputManifest = this.skipRunfilesManifests ? null : inputManifest;
this.repoMappingManifest = repoMappingManifest;
}

private static NestedSet<Artifact> computeInputs(
boolean enableRunfiles,
boolean skipRunfilesManifests,
Runfiles runfiles,
Artifact inputManifest) {
Artifact inputManifest,
@Nullable Artifact repoMappingManifest) {
NestedSetBuilder<Artifact> inputs = NestedSetBuilder.<Artifact>stableOrder();
if (!skipRunfilesManifests || !enableRunfiles || runfiles == null) {
inputs.add(inputManifest);
Expand All @@ -134,6 +143,9 @@ private static NestedSet<Artifact> computeInputs(
// existing, so directory or file links can be made as appropriate.
if (enableRunfiles && runfiles != null && OS.getCurrent() == OS.WINDOWS) {
inputs.addTransitive(runfiles.getAllArtifacts());
if (repoMappingManifest != null) {
inputs.add(repoMappingManifest);
}
}
return inputs.build();
}
Expand All @@ -151,6 +163,11 @@ public Artifact getOutputManifest() {
return outputManifest;
}

@Nullable
public Artifact getRepoMappingManifest() {
return repoMappingManifest;
}

public boolean isFilesetTree() {
return filesetRoot != null;
}
Expand Down Expand Up @@ -201,6 +218,10 @@ protected void computeKey(
if (runfiles != null) {
runfiles.fingerprint(fp);
}
fp.addBoolean(repoMappingManifest != null);
if (repoMappingManifest != null) {
fp.addPath(repoMappingManifest.getExecPath());
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,7 @@ private TestParams createTestAction(int shards)
SingleRunfilesSupplier.createCaching(
runfilesSupport.getRunfilesDirectoryExecPath(),
runfilesSupport.getRunfiles(),
runfilesSupport.getRepoMappingManifest(),
runfilesSupport.isBuildRunfileLinks(),
runfilesSupport.isRunfilesEnabled());
} else {
Expand Down
Loading

0 comments on commit 5929cb7

Please sign in to comment.