diff options
author | Michajlo Matijkiw <michajlo@google.com> | 2015-05-12 15:05:58 +0000 |
---|---|---|
committer | Damien Martin-Guillerez <dmarting@google.com> | 2015-05-15 09:35:19 +0000 |
commit | 68ac06d203bd7e9abbd79a2b6dfc42b80d121311 (patch) | |
tree | 74a9a857cba905cb11ab6fa37da0fa9684de8dd4 /src/main/java | |
parent | 0d56b6cdc0d2177231a2c14ab4c33be40a3ae234 (diff) |
Make local TestStrategy bits rely less on runfiles manifests
TestStrategy#getLocalRunfilesDirectory(...) was using manifest names
to infer if the runfiles symlinks were created by comparing the name
of manifests. This is a bit confusing and adds unnecessary reliance
on manifests which we'd like to reduce. Instead get whether or not
we created the symlinks from the boolean that determines it.
--
MOS_MIGRATED_REVID=93414150
Diffstat (limited to 'src/main/java')
3 files changed, 33 insertions, 27 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java index 0be417d26c..dd91cfb975 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java @@ -158,6 +158,11 @@ public class RunfilesSupport { executablePath.getBaseName() + RUNFILES_DIR_EXT); } + /** @return whether or not runfiles symlinks should be created */ + public boolean getCreateSymlinks() { + return createSymlinks; + } + public Runfiles getRunfiles() { return runfiles; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/TestStrategy.java b/src/main/java/com/google/devtools/build/lib/rules/test/TestStrategy.java index 98a2692331..93ce649aa2 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/test/TestStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/rules/test/TestStrategy.java @@ -296,16 +296,15 @@ public abstract class TestStrategy implements TestActionContext { InterruptedException { TestTargetExecutionSettings execSettings = testAction.getExecutionSettings(); - // --nobuild_runfile_links disables runfiles generation only for C++ rules. - // In that case, getManifest returns the .runfiles_manifest (input) file, - // not the MANIFEST output file of the build-runfiles action. So the - // extension ".runfiles_manifest" indicates no runfiles tree. - if (!execSettings.getManifest().equals(execSettings.getInputManifest())) { - return execSettings.getManifest().getPath().getParentDirectory(); + // If the symlink farm is already created then return the existing directory. If not we + // need to explicitly build it. This can happen when --nobuild_runfile_links is supplied + // as a flag to the build. + if (execSettings.getRunfilesSymlinksCreated()) { + return execSettings.getRunfilesDir(); } - // We might need to build runfiles tree now, since it was not created yet - // local testing is needed. + // TODO(bazel-team): Should we be using TestTargetExecutionSettings#getRunfilesDir() here over + // generating the directory ourselves? Path program = execSettings.getExecutable().getPath(); Path runfilesDir = program.getParentDirectory().getChild(program.getBaseName() + ".runfiles"); @@ -314,7 +313,7 @@ public abstract class TestStrategy implements TestActionContext { // trying to generate same runfiles tree in case of --runs_per_test > 1 or // local test sharding. long startTime = Profiler.nanoTimeMaybe(); - synchronized (execSettings.getManifest()) { + synchronized (execSettings.getInputManifest()) { Profiler.instance().logSimpleTask(startTime, ProfilerTask.WAIT, testAction); updateLocalRunfilesDirectory(testAction, runfilesDir, actionExecutionContext, binTools); } @@ -335,8 +334,10 @@ public abstract class TestStrategy implements TestActionContext { TestTargetExecutionSettings execSettings = testAction.getExecutionSettings(); try { + // Avoid rebuilding the runfiles directory if the manifest in it matches the input manifest, + // implying the symlinks exist and are already up to date. if (Arrays.equals(runfilesDir.getRelative("MANIFEST").getMD5Digest(), - execSettings.getManifest().getPath().getMD5Digest())) { + execSettings.getInputManifest().getPath().getMD5Digest())) { return; } } catch (IOException e1) { @@ -346,7 +347,7 @@ public abstract class TestStrategy implements TestActionContext { executor.getEventHandler().handle(Event.progress( "Building runfiles directory for '" + execSettings.getExecutable().prettyPrint() + "'.")); - new SymlinkTreeHelper(execSettings.getManifest().getExecPath(), + new SymlinkTreeHelper(execSettings.getInputManifest().getExecPath(), runfilesDir.relativeTo(executor.getExecRoot()), /* filesetTree= */ false) .createSymlinks(testAction, actionExecutionContext, binTools); diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/TestTargetExecutionSettings.java b/src/main/java/com/google/devtools/build/lib/rules/test/TestTargetExecutionSettings.java index 20ad8af5ec..1948a3b330 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/test/TestTargetExecutionSettings.java +++ b/src/main/java/com/google/devtools/build/lib/rules/test/TestTargetExecutionSettings.java @@ -26,6 +26,7 @@ import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.RunUnder; import com.google.devtools.build.lib.packages.TargetUtils; +import com.google.devtools.build.lib.vfs.Path; import java.util.List; @@ -41,17 +42,18 @@ public final class TestTargetExecutionSettings { private final RunUnder runUnder; private final Artifact runUnderExecutable; private final Artifact executable; - private final Artifact runfilesManifest; + private final boolean runfilesSymlinksCreated; + private final Path runfilesDir; private final Artifact runfilesInputManifest; private final Artifact instrumentedFileManifest; - TestTargetExecutionSettings(RuleContext ruleContext, RunfilesSupport runfiles, + TestTargetExecutionSettings(RuleContext ruleContext, RunfilesSupport runfilesSupport, Artifact executable, Artifact instrumentedFileManifest, int shards) { Preconditions.checkArgument(TargetUtils.isTestRule(ruleContext.getRule())); Preconditions.checkArgument(shards >= 0); BuildConfiguration config = ruleContext.getConfiguration(); - List<String> targetArgs = runfiles.getArgs(); + List<String> targetArgs = runfilesSupport.getArgs(); testArguments = targetArgs.isEmpty() ? config.getTestArguments() : ImmutableList.copyOf(Iterables.concat(targetArgs, config.getTestArguments())); @@ -62,8 +64,9 @@ public final class TestTargetExecutionSettings { this.testFilter = config.getTestFilter(); this.executable = executable; - this.runfilesManifest = runfiles.getRunfilesManifest(); - this.runfilesInputManifest = runfiles.getRunfilesInputManifest(); + this.runfilesSymlinksCreated = runfilesSupport.getCreateSymlinks(); + this.runfilesDir = runfilesSupport.getRunfilesDirectory(); + this.runfilesInputManifest = runfilesSupport.getRunfilesInputManifest(); this.instrumentedFileManifest = instrumentedFileManifest; } @@ -99,17 +102,14 @@ public final class TestTargetExecutionSettings { return executable; } - /** - * Returns the runfiles manifest for this test. - * - * <p>This returns either the input manifest outside of the runfiles tree, - * if blaze is run with --nobuild_runfile_links or the manifest inside the - * runfiles tree, if blaze is run with --build_runfile_links. - * - * @see com.google.devtools.build.lib.analysis.RunfilesSupport#getRunfilesManifest() - */ - public Artifact getManifest() { - return runfilesManifest; + /** @return whether or not the runfiles symlinks were created */ + public boolean getRunfilesSymlinksCreated() { + return runfilesSymlinksCreated; + } + + /** @return the directory of the runfiles */ + public Path getRunfilesDir() { + return runfilesDir; } /** |