Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stage repository mapping manifest as a root symlink #16733

Merged
merged 1 commit into from
Nov 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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