Skip to content

Commit

Permalink
Make runfiles tree creation on Windows depend on the artifacts of the…
Browse files Browse the repository at this point in the history
… actual runfiles.

This is necessary because the plan for Windows calls for knowing if the target of the symlink is a directory or a file, thus, we cannot create runfiles trees before the artifacts in them are built. This probably comes with a performance hit due to the extra scheduling constraints.

This makes almost every test pass save:

- test_tmpdir in bazel_test_test, which I hereby dismiss as a fluke
- test_http_archive_tgz in external_integration_test (Maybe a permissions issue due to copying things?)
- A bunch of test in external_correctness_test, probably related to the fact that since we are copying things, we don't notice changes to the original files.

--
MOS_MIGRATED_REVID=113050025
  • Loading branch information
lberki committed Jan 26, 2016
1 parent 213ae7a commit 41f4456
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@

import java.util.Iterator;

import javax.annotation.Nullable;

/**
* A factory to create middleman objects.
*/
Expand Down Expand Up @@ -82,8 +84,8 @@ public Artifact createAggregatingMiddleman(
* @param middlemanDir the directory in which to place the middleman.
*/
public Artifact createRunfilesMiddleman(
ActionOwner owner, Artifact owningArtifact, Iterable<Artifact> inputs, Root middlemanDir,
String tag) {
ActionOwner owner, @Nullable Artifact owningArtifact, Iterable<Artifact> inputs,
Root middlemanDir, String tag) {
if (hasExactlyOneInput(inputs)) { // Optimization: No middleman for just one input.
return Iterables.getOnlyElement(inputs);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ private RunfilesSupport(RuleContext ruleContext, Artifact executable, Runfiles r

Artifact artifactsMiddleman = createArtifactsMiddleman(ruleContext, runfiles.getAllArtifacts());
runfilesInputManifest = createRunfilesInputManifestArtifact(ruleContext);
this.runfilesManifest = createRunfilesAction(ruleContext, runfiles);
this.runfilesManifest = createRunfilesAction(ruleContext, runfiles, artifactsMiddleman);
this.runfilesMiddleman = createRunfilesMiddleman(
ruleContext, artifactsMiddleman, runfilesManifest);
sourcesManifest = createSourceManifest(ruleContext, runfiles);
Expand Down Expand Up @@ -272,7 +272,8 @@ private Artifact createRunfilesMiddleman(ActionConstructionContext context,
* generated files, etc.) into a single tree, so that programs can access them
* using the workspace-relative name.
*/
private Artifact createRunfilesAction(ActionConstructionContext context, Runfiles runfiles) {
private Artifact createRunfilesAction(ActionConstructionContext context, Runfiles runfiles,
Artifact artifactsMiddleman) {
// Compute the names of the runfiles directory and its MANIFEST file.
Artifact inputManifest = getRunfilesInputManifest();
context.getAnalysisEnvironment().registerAction(
Expand All @@ -292,7 +293,8 @@ private Artifact createRunfilesAction(ActionConstructionContext context, Runfile
Artifact outputManifest = context.getDerivedArtifact(
outputManifestPath, config.getBinDirectory());
context.getAnalysisEnvironment().registerAction(new SymlinkTreeAction(
context.getActionOwner(), inputManifest, outputManifest, /*filesetTree=*/false));
context.getActionOwner(), inputManifest, artifactsMiddleman, outputManifest,
/*filesetTree=*/false));
return outputManifest;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.util.Preconditions;

import javax.annotation.Nullable;

/**
* Action responsible for the symlink tree creation.
* Used to generate runfiles and fileset symlink farms.
Expand All @@ -40,22 +42,36 @@ public class SymlinkTreeAction extends AbstractAction {
* Creates SymlinkTreeAction instance.
*
* @param owner action owner
* @param inputManifest exec path to the input runfiles manifest
* @param outputManifest exec path to the generated symlink tree manifest
* @param inputManifest the input runfiles manifest
* @param artifactMiddleman the middleman artifact representing all the files the symlinks
* point to (on Windows we need to know if the target of a "symlink" is
* a directory or a file so we need to build it before)
* @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 filesetTree true if this is fileset symlink tree,
* false if this is a runfiles symlink tree.
*/
public SymlinkTreeAction(ActionOwner owner, Artifact inputManifest, Artifact outputManifest,
boolean filesetTree) {
super(owner, ImmutableList.of(inputManifest), ImmutableList.of(outputManifest));
public SymlinkTreeAction(ActionOwner owner, Artifact inputManifest,
@Nullable Artifact artifactMiddleman, Artifact outputManifest, boolean filesetTree) {
super(owner, computeInputs(inputManifest, artifactMiddleman), ImmutableList.of(outputManifest));
Preconditions.checkArgument(outputManifest.getPath().getBaseName().equals("MANIFEST"));
this.inputManifest = inputManifest;
this.outputManifest = outputManifest;
this.filesetTree = filesetTree;
}

private static ImmutableList<Artifact> computeInputs(
Artifact inputManifest, Artifact artifactMiddleman) {
ImmutableList.Builder<Artifact> result = ImmutableList.<Artifact>builder()
.add(inputManifest);
if (artifactMiddleman != null
&& !artifactMiddleman.getPath().getFileSystem().supportsSymbolicLinksNatively()) {
result.add(artifactMiddleman);
}
return result.build();
}

public Artifact getInputManifest() {
return inputManifest;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,13 @@ public Artifact createApkBuilderSymlinks(RuleContext ruleContext) {
.addRootSymlinks(symlinks)
.build());
Artifact outputManifest = AndroidBinary.getDxArtifact(ruleContext, "native_symlinks/MANIFEST");
Artifact nativeLibsMiddleman =
ruleContext.getAnalysisEnvironment().getMiddlemanFactory().createRunfilesMiddleman(
ruleContext.getActionOwner(), null, symlinks.values(),
ruleContext.getConfiguration().getMiddlemanDirectory(), "android_native_libs");

ruleContext.registerAction(new SymlinkTreeAction(
ruleContext.getActionOwner(), inputManifest, outputManifest, false));
ruleContext.getActionOwner(), inputManifest, nativeLibsMiddleman, outputManifest, false));
return outputManifest;
}

Expand Down

0 comments on commit 41f4456

Please sign in to comment.