From 36a6c17629e76872a876cd313623246d4d0aa082 Mon Sep 17 00:00:00 2001 From: Kristina Chodorow Date: Fri, 22 Jul 2016 15:18:07 +0000 Subject: Create a symlink with the right workspace name under the execroot The execution root currently uses the basename of the workspace directory for the workspace name, not the name in the WORKSPACE file. (For example, if our sources were in /path/to/foo and our WORKSPACE file had workspace(name = "bar"), our execution root would look like execroot/foo.) This creates a symlink bar -> foo, so that accessing ../repo_name actually works for the main repository. RELNOTES[INC]: The main repository's execution root is under the main repository's workspace name, not the source directory's basename. This shouldn't have any effect on most builds, but it's possible it could break someone doing weird things with paths in actions. -- MOS_MIGRATED_REVID=128175455 --- .../devtools/build/lib/analysis/BuildView.java | 15 +++++++++++--- .../build/lib/buildtool/ExecutionTool.java | 16 +++++++-------- .../build/lib/buildtool/SymlinkForest.java | 22 ++++++++++++++++++++- .../lib/pkgcache/LegacyLoadingPhaseRunner.java | 20 ++++++++++++++----- .../devtools/build/lib/pkgcache/LoadingResult.java | 9 ++++++++- .../lib/skyframe/TargetPatternPhaseFunction.java | 23 ++++++++++++++++++---- .../lib/skyframe/TargetPatternPhaseValue.java | 11 +++++++++-- 7 files changed, 92 insertions(+), 24 deletions(-) (limited to 'src/main') diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java index 2f5f70f01d..e7c4ac468c 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java @@ -283,7 +283,8 @@ public class BuildView { ImmutableList.of(), ImmutableList.of(), null, - ImmutableMap.of()); + ImmutableMap.of(), + ""); private final ImmutableList targetsToBuild; @Nullable private final ImmutableList targetsToTest; @@ -295,6 +296,7 @@ public class BuildView { @Nullable private final TopLevelArtifactContext topLevelContext; private final ImmutableList aspects; private final ImmutableMap packageRoots; + private final String workspaceName; private AnalysisResult( Collection targetsToBuild, @@ -306,7 +308,8 @@ public class BuildView { Collection parallelTests, Collection exclusiveTests, TopLevelArtifactContext topLevelContext, - ImmutableMap packageRoots) { + ImmutableMap packageRoots, + String workspaceName) { this.targetsToBuild = ImmutableList.copyOf(targetsToBuild); this.aspects = ImmutableList.copyOf(aspects); this.targetsToTest = targetsToTest == null ? null : ImmutableList.copyOf(targetsToTest); @@ -317,6 +320,7 @@ public class BuildView { this.exclusiveTests = ImmutableSet.copyOf(exclusiveTests); this.topLevelContext = topLevelContext; this.packageRoots = packageRoots; + this.workspaceName = workspaceName; } /** @@ -386,6 +390,10 @@ public class BuildView { public TopLevelArtifactContext getTopLevelContext() { return topLevelContext; } + + public String getWorkspaceName() { + return workspaceName; + } } @@ -602,7 +610,8 @@ public class BuildView { parallelTests, exclusiveTests, topLevelOptions, - skyframeAnalysisResult.getPackageRoots()); + skyframeAnalysisResult.getPackageRoots(), + loadingResult.getWorkspaceName()); } private static NestedSet getBaselineCoverageArtifacts( diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java index 1f54bcc0b5..cf838bce49 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java @@ -51,7 +51,6 @@ import com.google.devtools.build.lib.actions.SimpleActionContextProvider; import com.google.devtools.build.lib.actions.SpawnActionContext; import com.google.devtools.build.lib.actions.TestExecException; import com.google.devtools.build.lib.actions.cache.ActionCache; -import com.google.devtools.build.lib.analysis.BuildView; import com.google.devtools.build.lib.analysis.BuildView.AnalysisResult; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.SymlinkTreeActionContext; @@ -94,7 +93,6 @@ import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.ModifiedFileSet; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; - import java.io.IOException; import java.io.OutputStream; import java.io.PrintWriter; @@ -341,7 +339,7 @@ public class ExecutionTool { TopLevelArtifactContext topLevelArtifactContext) throws BuildFailedException, InterruptedException, TestExecException, AbruptExitException { Stopwatch timer = Stopwatch.createStarted(); - prepare(packageRoots); + prepare(packageRoots, analysisResult.getWorkspaceName()); ActionGraph actionGraph = analysisResult.getActionGraph(); @@ -363,9 +361,10 @@ public class ExecutionTool { if (targetConfigurations.size() == 1) { String productName = runtime.getProductName(); OutputDirectoryLinksUtils.createOutputDirectoryLinks( - env.getWorkspaceName(), env.getWorkspace(), getExecRoot(), - env.getOutputPath(), getReporter(), targetConfiguration, - request.getBuildOptions().getSymlinkPrefix(productName), productName); + env.getWorkspaceName(), env.getWorkspace(), + getExecRoot(), env.getOutputPath(), getReporter(), + targetConfiguration, request.getBuildOptions().getSymlinkPrefix(productName), + productName); } ActionCache actionCache = getActionCache(); @@ -499,7 +498,7 @@ public class ExecutionTool { } } - private void prepare(ImmutableMap packageRoots) + private void prepare(ImmutableMap packageRoots, String workspaceName) throws ExecutorInitException { // Prepare for build. Profiler.instance().markPhase(ProfilePhase.PREPARE); @@ -510,7 +509,8 @@ public class ExecutionTool { // Plant the symlink forest. try { new SymlinkForest( - packageRoots, getExecRoot(), runtime.getProductName()).plantSymlinkForest(); + packageRoots, getExecRoot(), runtime.getProductName(), workspaceName) + .plantSymlinkForest(); } catch (IOException e) { throw new ExecutorInitException("Source forest creation failed", e); } diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java b/src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java index 5c8ff90565..9d57a44466 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java @@ -43,13 +43,16 @@ class SymlinkForest { private final ImmutableMap packageRoots; private final Path workspace; + private final String workspaceName; private final String productName; private final String[] prefixes; SymlinkForest( - ImmutableMap packageRoots, Path workspace, String productName) { + ImmutableMap packageRoots, Path workspace, String productName, + String workspaceName) { this.packageRoots = packageRoots; this.workspace = workspace; + this.workspaceName = workspaceName; this.productName = productName; this.prefixes = new String[] { ".", "_", productName + "-"}; } @@ -216,6 +219,23 @@ class SymlinkForest { } } } + + symlinkCorrectWorkspaceName(); + } + + /** + * Right now, the execution root is under the basename of the source directory, not the name + * defined in the WORKSPACE file. Thus, this adds a symlink with the WORKSPACE's workspace name + * to the old-style execution root. + * TODO(kchodorow): get rid of this once exec root is always under the WORKSPACE's workspace + * name. + * @throws IOException + */ + private void symlinkCorrectWorkspaceName() throws IOException { + Path correctDirectory = workspace.getParentDirectory().getRelative(workspaceName); + if (!correctDirectory.exists()) { + correctDirectory.createSymbolicLink(workspace); + } } private static PackageIdentifier getParent(PackageIdentifier packageIdentifier) { diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/LegacyLoadingPhaseRunner.java b/src/main/java/com/google/devtools/build/lib/pkgcache/LegacyLoadingPhaseRunner.java index 09698e6176..741e1be26a 100644 --- a/src/main/java/com/google/devtools/build/lib/pkgcache/LegacyLoadingPhaseRunner.java +++ b/src/main/java/com/google/devtools/build/lib/pkgcache/LegacyLoadingPhaseRunner.java @@ -27,6 +27,7 @@ import com.google.devtools.build.lib.cmdline.TargetParsingException; import com.google.devtools.build.lib.events.DelegatingEventHandler; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; +import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.packages.NoSuchThingException; import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper; import com.google.devtools.build.lib.packages.Rule; @@ -213,7 +214,7 @@ public final class LegacyLoadingPhaseRunner extends LoadingPhaseRunner { */ private LoadingResult doSimpleLoadingPhase(EventHandler eventHandler, EventBus eventBus, ResolvedTargets targets, ImmutableSet testsToRun, boolean keepGoing) - throws LoadingFailedException { + throws InterruptedException, LoadingFailedException { Stopwatch timer = preLoadingLogging(eventHandler); ImmutableSet targetsToLoad = targets.getTargets(); @@ -226,7 +227,7 @@ public final class LegacyLoadingPhaseRunner extends LoadingPhaseRunner { postLoadingLogging(eventBus, targetsToLoad, expandedResult.getTargets(), timer); return new LoadingResult(targets.hasError(), expandedResult.hasError(), - expandedResult.getTargets(), testsToRun); + expandedResult.getTargets(), testsToRun, getWorkspaceName(eventHandler)); } /** @@ -254,10 +255,9 @@ public final class LegacyLoadingPhaseRunner extends LoadingPhaseRunner { freeMemoryAfterLoading(callback, pkgLoader.getVisitedPackageNames()); postLoadingLogging(eventBus, baseResult.getTargets(), expandedResult.getTargets(), timer); - LoadingResult loadingResult = new LoadingResult(targets.hasError(), + return new LoadingResult(targets.hasError(), !baseResult.isSuccesful() || expandedResult.hasError(), - expandedResult.getTargets(), testsToRun); - return loadingResult; + expandedResult.getTargets(), testsToRun, getWorkspaceName(eventHandler)); } private Stopwatch preLoadingLogging(EventHandler eventHandler) { @@ -445,4 +445,14 @@ public final class LegacyLoadingPhaseRunner extends LoadingPhaseRunner { } } } + + private String getWorkspaceName(EventHandler eventHandler) + throws InterruptedException, LoadingFailedException { + try { + return packageManager.getPackage(eventHandler, Label.EXTERNAL_PACKAGE_IDENTIFIER) + .getWorkspaceName(); + } catch (NoSuchPackageException e) { + throw new LoadingFailedException("Failed to load //external package", e); + } + } } diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingResult.java b/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingResult.java index 94f597649e..f78e0afb75 100644 --- a/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingResult.java +++ b/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingResult.java @@ -27,14 +27,16 @@ public final class LoadingResult { private final boolean hasLoadingError; private final ImmutableSet targetsToAnalyze; private final ImmutableSet testsToRun; + private final String workspaceName; public LoadingResult(boolean hasTargetPatternError, boolean hasLoadingError, - Collection targetsToAnalyze, Collection testsToRun) { + Collection targetsToAnalyze, Collection testsToRun, String workspaceName) { this.hasTargetPatternError = hasTargetPatternError; this.hasLoadingError = hasLoadingError; this.targetsToAnalyze = targetsToAnalyze == null ? null : ImmutableSet.copyOf(targetsToAnalyze); this.testsToRun = testsToRun == null ? null : ImmutableSet.copyOf(testsToRun); + this.workspaceName = workspaceName; } /** Whether there were errors during target pattern evaluation. */ @@ -56,4 +58,9 @@ public final class LoadingResult { public Collection getTestsToRun() { return testsToRun; } + + /** The name of the local workspace. */ + public String getWorkspaceName() { + return workspaceName; + } } \ No newline at end of file diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java index 9608878f34..5215b7370a 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java @@ -20,6 +20,7 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.ResolvedTargets; import com.google.devtools.build.lib.cmdline.TargetParsingException; import com.google.devtools.build.lib.events.Event; +import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.packages.TargetUtils; import com.google.devtools.build.lib.pkgcache.CompileOneDependencyTransformer; @@ -37,14 +38,12 @@ import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; import com.google.devtools.build.skyframe.ValueOrException; - import java.util.ArrayList; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; - import javax.annotation.Nullable; /** @@ -56,6 +55,22 @@ final class TargetPatternPhaseFunction implements SkyFunction { @Override public TargetPatternPhaseValue compute(SkyKey key, Environment env) { TargetPatternList options = (TargetPatternList) key.argument(); + PackageValue packageValue = null; + boolean workspaceError = false; + try { + packageValue = (PackageValue) env.getValueOrThrow( + PackageValue.key(Label.EXTERNAL_PACKAGE_IDENTIFIER), NoSuchPackageException.class); + } catch (NoSuchPackageException e) { + env.getListener().handle(Event.error(e.getMessage())); + workspaceError = true; + } + if (env.valuesMissing()) { + return null; + } + String workspaceName = ""; + if (!workspaceError) { + workspaceName = packageValue.getPackage().getWorkspaceName(); + } // Determine targets to build: ResolvedTargets targets = getTargetsToBuild(env, @@ -163,8 +178,8 @@ final class TargetPatternPhaseFunction implements SkyFunction { Set testSuiteTargets = Sets.difference(targets.getTargets(), expandedTargets.getTargets()); return new TargetPatternPhaseValue(expandedTargets.getTargets(), testsToRun, preExpansionError, - expandedTargets.hasError(), filteredTargets, testFilteredTargets, - targets.getTargets(), ImmutableSet.copyOf(testSuiteTargets)); + expandedTargets.hasError() || workspaceError, filteredTargets, testFilteredTargets, + targets.getTargets(), ImmutableSet.copyOf(testSuiteTargets), workspaceName); } /** diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseValue.java index 0e43191dbc..ca8881386f 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseValue.java @@ -53,11 +53,12 @@ public final class TargetPatternPhaseValue implements SkyValue { // TODO(ulfjack): Support EventBus event posting in Skyframe, and remove this code again. private final ImmutableSet originalTargets; private final ImmutableSet testSuiteTargets; + private final String workspaceName; TargetPatternPhaseValue(ImmutableSet targets, @Nullable ImmutableSet testsToRun, boolean hasError, boolean hasPostExpansionError, ImmutableSet filteredTargets, ImmutableSet testFilteredTargets, ImmutableSet originalTargets, - ImmutableSet testSuiteTargets) { + ImmutableSet testSuiteTargets, String workspaceName) { this.targets = Preconditions.checkNotNull(targets); this.testsToRun = testsToRun; this.hasError = hasError; @@ -66,6 +67,7 @@ public final class TargetPatternPhaseValue implements SkyValue { this.testFilteredTargets = Preconditions.checkNotNull(testFilteredTargets); this.originalTargets = Preconditions.checkNotNull(originalTargets); this.testSuiteTargets = Preconditions.checkNotNull(testSuiteTargets); + this.workspaceName = workspaceName; } public ImmutableSet getTargets() { @@ -101,8 +103,13 @@ public final class TargetPatternPhaseValue implements SkyValue { return testSuiteTargets; } + public String getWorkspaceName() { + return workspaceName; + } + public LoadingResult toLoadingResult() { - return new LoadingResult(hasError(), hasPostExpansionError(), getTargets(), getTestsToRun()); + return new LoadingResult( + hasError(), hasPostExpansionError(), getTargets(), getTestsToRun(), getWorkspaceName()); } @SuppressWarnings("unused") -- cgit v1.2.3