Automated g4 rollback of commit c4134802dd15d6ef5cca6521f6bf6aac395ee2ad. *** Reason for rollback *** Roll forward of directory name change *** Original change description *** Automated g4 rollback of commit 1d9e1ac90197b1d3d7b137ba3c1ada67bb9ba31b. *** Reason for rollback *** Breaks //src/test/shell/integration:force_delete_output_test *** Original change description *** Symlink output directories to the correct directory name If the workspace directory is /path/to/my/proj and the name in the WORKSPACE file is "floop", this will symlink the output directories to output_base/execroot/floop instead of output_base/execroot/proj. More prep for #1262, fixes #1681. PiperOrigin-RevId: 156892980
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Root.java b/src/main/java/com/google/devtools/build/lib/actions/Root.java index dfbfa74..afd5a62 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Root.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Root.java
@@ -142,6 +142,7 @@ private final boolean isMainRepo; private final PathFragment execPath; + private Root(@Nullable Path execRoot, Path path, boolean isMiddlemanRoot, boolean isMainRepo) { this.execRoot = execRoot; this.path = Preconditions.checkNotNull(path);
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisPhaseStartedEvent.java b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisPhaseStartedEvent.java index 48b746c..25e9594 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisPhaseStartedEvent.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisPhaseStartedEvent.java
@@ -15,6 +15,7 @@ package com.google.devtools.build.lib.analysis; import com.google.common.base.Function; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.Target; @@ -26,19 +27,14 @@ */ public class AnalysisPhaseStartedEvent { - private final Iterable<Label> labels; + private final ImmutableSet<Target> targets; /** * Construct the event. * @param targets The set of active targets that remain. */ public AnalysisPhaseStartedEvent(Collection<Target> targets) { - this.labels = Iterables.transform(targets, new Function<Target, Label>() { - @Override - public Label apply(Target input) { - return input.getLabel(); - } - }); + this.targets = ImmutableSet.copyOf(targets); } /** @@ -46,6 +42,15 @@ * of the targets we attempted to load. */ public Iterable<Label> getLabels() { - return labels; + return Iterables.transform(targets, new Function<Target, Label>() { + @Override + public Label apply(Target input) { + return input.getLabel(); + } + }); + } + + public ImmutableSet<Target> getTargets() { + return targets; } }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfigurationCollectionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfigurationCollectionFactory.java index 75ac3a0..ae209c9 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfigurationCollectionFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfigurationCollectionFactory.java
@@ -39,6 +39,7 @@ * @param loadedPackageProvider the package provider * @param buildOptions top-level build options representing the command-line * @param errorEventListener the event listener for errors + * @param mainRepositoryName the workspace name of the main repository * @return the top-level configuration * @throws InvalidConfigurationException */ @@ -48,7 +49,8 @@ Cache<String, BuildConfiguration> cache, PackageProviderForConfigurations loadedPackageProvider, BuildOptions buildOptions, - EventHandler errorEventListener) + EventHandler errorEventListener, + String mainRepositoryName) throws InvalidConfigurationException, InterruptedException; /**
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java index f1e1db2..5989b62 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -257,7 +257,7 @@ * Returns the workspace name for the rule. */ public String getWorkspaceName() { - return rule.getPackage().getWorkspaceName(); + return rule.getRepository().strippedName(); } /**
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BinTools.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BinTools.java index 2dd779f..8c50b36 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BinTools.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BinTools.java
@@ -37,12 +37,14 @@ */ public final class BinTools { private final BlazeDirectories directories; - private final Path binDir; // the working bin directory under execRoot + private final Path execrootParent; private final ImmutableList<String> embeddedTools; + private Path binDir; // the working bin directory under execRoot + private BinTools(BlazeDirectories directories, ImmutableList<String> tools) { this.directories = directories; - this.binDir = directories.getExecRoot().getRelative("_bin"); + this.execrootParent = directories.getExecRoot().getParentDirectory(); ImmutableList.Builder<String> builder = ImmutableList.builder(); // Files under embedded_tools shouldn't be copied to under _bin dir // They won't be used during action execution time. @@ -52,6 +54,7 @@ } } this.embeddedTools = builder.build(); + this.binDir = null; } /** @@ -69,7 +72,8 @@ */ @VisibleForTesting public static BinTools empty(BlazeDirectories directories) { - return new BinTools(directories, ImmutableList.<String>of()); + return new BinTools(directories, ImmutableList.<String>of()).setBinDir( + directories.getWorkspace().getBaseName()); } /** @@ -79,7 +83,8 @@ */ @VisibleForTesting public static BinTools forUnitTesting(BlazeDirectories directories, Iterable<String> tools) { - return new BinTools(directories, ImmutableList.copyOf(tools)); + return new BinTools(directories, ImmutableList.copyOf(tools)).setBinDir( + directories.getWorkspace().getBaseName()); } /** @@ -88,7 +93,7 @@ */ @VisibleForTesting public static BinTools forIntegrationTesting( - BlazeDirectories directories, String srcDir, Iterable<String> tools) + BlazeDirectories directories, String srcDir, Iterable<String> tools, String repositoryName) throws IOException { Path srcPath = directories.getOutputBase().getFileSystem().getPath(srcDir); for (String embedded : tools) { @@ -101,7 +106,7 @@ // much point in creating a symlink to a non-existent binary here. continue; } - Path outputPath = directories.getExecRoot().getChild("_bin").getChild(embedded); + Path outputPath = directories.getExecRoot(repositoryName).getChild("_bin").getChild(embedded); if (outputPath.exists()) { outputPath.delete(); } @@ -109,7 +114,7 @@ outputPath.createSymbolicLink(runfilesPath); } - return new BinTools(directories, ImmutableList.copyOf(tools)); + return new BinTools(directories, ImmutableList.copyOf(tools)).setBinDir(repositoryName); } private static void scanDirectoryRecursively( @@ -143,6 +148,7 @@ } public Artifact getEmbeddedArtifact(String embedPath, ArtifactFactory artifactFactory) { + Preconditions.checkNotNull(binDir); PathFragment path = getExecPath(embedPath); Preconditions.checkNotNull(path, embedPath + " not found in embedded tools"); return artifactFactory.getDerivedArtifact(path, binDir.getParentDirectory()); @@ -156,11 +162,17 @@ return builder.build(); } + private BinTools setBinDir(String workspaceName) { + binDir = execrootParent.getRelative(workspaceName).getRelative("_bin"); + return this; + } + /** * Initializes the build tools not available at absolute paths. Note that * these must be constant across all configurations. */ - public void setupBuildTools() throws ExecException { + public void setupBuildTools(String workspaceName) throws ExecException { + setBinDir(workspaceName); try { FileSystemUtils.createDirectoryAndParents(binDir); } catch (IOException e) { @@ -173,6 +185,7 @@ } private void setupTool(String embeddedPath) throws ExecException { + Preconditions.checkNotNull(binDir); Path sourcePath = directories.getEmbeddedBinariesRoot().getRelative(embeddedPath); Path linkPath = binDir.getRelative(PathFragment.create(embeddedPath).getBaseName()); linkTool(sourcePath, linkPath);
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java index 170c9c6..1098e2a 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -1137,6 +1137,7 @@ private final ImmutableMap<Class<? extends Fragment>, Fragment> fragments; private final ImmutableMap<String, Class<? extends Fragment>> skylarkVisibleFragments; + private final RepositoryName mainRepositoryName; /** @@ -1207,21 +1208,21 @@ } Root getRoot( - RepositoryName repositoryName, String outputDirName, BlazeDirectories directories) { + RepositoryName repositoryName, String outputDirName, BlazeDirectories directories, + RepositoryName mainRepositoryName) { // e.g., execroot/repo1 - Path execRoot = directories.getExecRoot(); + Path execRoot = directories.getExecRoot(mainRepositoryName.strippedName()); // e.g., execroot/repo1/bazel-out/config/bin Path outputDir = execRoot.getRelative(directories.getRelativeOutputPath()) .getRelative(outputDirName); if (middleman) { - return INTERNER.intern(Root.middlemanRoot(execRoot, outputDir, repositoryName.isMain())); + return INTERNER.intern(Root.middlemanRoot(execRoot, outputDir, + repositoryName.equals(mainRepositoryName))); } // e.g., [[execroot/repo1]/bazel-out/config/bin] return INTERNER.intern( - Root.asDerivedRoot( - execRoot, - outputDir.getRelative(nameFragment), - repositoryName.isMain())); + Root.asDerivedRoot(execRoot, outputDir.getRelative(nameFragment), + repositoryName.equals(mainRepositoryName))); } } @@ -1446,7 +1447,8 @@ */ public BuildConfiguration(BlazeDirectories directories, Map<Class<? extends Fragment>, Fragment> fragmentsMap, - BuildOptions buildOptions) { + BuildOptions buildOptions, + String repositoryName) { this.directories = directories; this.fragments = ImmutableSortedMap.copyOf(fragmentsMap, lexicalFragmentSorter); @@ -1455,6 +1457,7 @@ this.buildOptions = buildOptions.clone(); this.actionsEnabled = buildOptions.enableActions(); this.options = buildOptions.get(Options.class); + this.mainRepositoryName = RepositoryName.createFromValidStrippedName(repositoryName); Map<String, String> testEnv = new TreeMap<>(); for (Map.Entry<String, String> entry : this.options.testEnvironment) { @@ -1479,19 +1482,26 @@ ? options.outputDirectoryName : mnemonic; this.outputDirectoryForMainRepository = - OutputDirectory.OUTPUT.getRoot(RepositoryName.MAIN, outputDirName, directories); + OutputDirectory.OUTPUT.getRoot( + RepositoryName.MAIN, outputDirName, directories, mainRepositoryName); this.binDirectoryForMainRepository = - OutputDirectory.BIN.getRoot(RepositoryName.MAIN, outputDirName, directories); + OutputDirectory.BIN.getRoot( + RepositoryName.MAIN, outputDirName, directories, mainRepositoryName); this.includeDirectoryForMainRepository = - OutputDirectory.INCLUDE.getRoot(RepositoryName.MAIN, outputDirName, directories); + OutputDirectory.INCLUDE.getRoot( + RepositoryName.MAIN, outputDirName, directories, mainRepositoryName); this.genfilesDirectoryForMainRepository = - OutputDirectory.GENFILES.getRoot(RepositoryName.MAIN, outputDirName, directories); + OutputDirectory.GENFILES.getRoot( + RepositoryName.MAIN, outputDirName, directories, mainRepositoryName); this.coverageDirectoryForMainRepository = - OutputDirectory.COVERAGE.getRoot(RepositoryName.MAIN, outputDirName, directories); + OutputDirectory.COVERAGE.getRoot( + RepositoryName.MAIN, outputDirName, directories, mainRepositoryName); this.testlogsDirectoryForMainRepository = - OutputDirectory.TESTLOGS.getRoot(RepositoryName.MAIN, outputDirName, directories); + OutputDirectory.TESTLOGS.getRoot( + RepositoryName.MAIN, outputDirName, directories, mainRepositoryName); this.middlemanDirectoryForMainRepository = - OutputDirectory.MIDDLEMAN.getRoot(RepositoryName.MAIN, outputDirName, directories); + OutputDirectory.MIDDLEMAN.getRoot( + RepositoryName.MAIN, outputDirName, directories, mainRepositoryName); this.platformName = buildPlatformName(); @@ -1540,8 +1550,8 @@ } BuildOptions options = buildOptions.trim( getOptionsClasses(fragmentsMap.keySet(), ruleClassProvider)); - BuildConfiguration newConfig = - new BuildConfiguration(directories, fragmentsMap, options); + BuildConfiguration newConfig = new BuildConfiguration( + directories, fragmentsMap, options, mainRepositoryName.strippedName()); newConfig.setConfigurationTransitions(this.transitions); return newConfig; } @@ -2117,9 +2127,10 @@ * Returns the output directory for this build configuration. */ public Root getOutputDirectory(RepositoryName repositoryName) { - return repositoryName.equals(RepositoryName.MAIN) + return repositoryName.isMain() || repositoryName.equals(mainRepositoryName) ? outputDirectoryForMainRepository - : OutputDirectory.OUTPUT.getRoot(repositoryName, outputDirName, directories); + : OutputDirectory.OUTPUT.getRoot( + repositoryName, outputDirName, directories, mainRepositoryName); } /** @@ -2138,9 +2149,10 @@ * repositories (external) but will need to be fixed. */ public Root getBinDirectory(RepositoryName repositoryName) { - return repositoryName.equals(RepositoryName.MAIN) + return repositoryName.isMain() || repositoryName.equals(mainRepositoryName) ? binDirectoryForMainRepository - : OutputDirectory.BIN.getRoot(repositoryName, outputDirName, directories); + : OutputDirectory.BIN.getRoot( + repositoryName, outputDirName, directories, mainRepositoryName); } /** @@ -2154,9 +2166,10 @@ * Returns the include directory for this build configuration. */ public Root getIncludeDirectory(RepositoryName repositoryName) { - return repositoryName.equals(RepositoryName.MAIN) + return repositoryName.isMain() || repositoryName.equals(mainRepositoryName) ? includeDirectoryForMainRepository - : OutputDirectory.INCLUDE.getRoot(repositoryName, outputDirName, directories); + : OutputDirectory.INCLUDE.getRoot( + repositoryName, outputDirName, directories, mainRepositoryName); } /** @@ -2169,9 +2182,10 @@ } public Root getGenfilesDirectory(RepositoryName repositoryName) { - return repositoryName.equals(RepositoryName.MAIN) + return repositoryName.isMain() || repositoryName.equals(mainRepositoryName) ? genfilesDirectoryForMainRepository - : OutputDirectory.GENFILES.getRoot(repositoryName, outputDirName, directories); + : OutputDirectory.GENFILES.getRoot( + repositoryName, outputDirName, directories, mainRepositoryName); } /** @@ -2180,18 +2194,20 @@ * needed for Jacoco's coverage reporting tools. */ public Root getCoverageMetadataDirectory(RepositoryName repositoryName) { - return repositoryName.equals(RepositoryName.MAIN) + return repositoryName.isMain() || repositoryName.equals(mainRepositoryName) ? coverageDirectoryForMainRepository - : OutputDirectory.COVERAGE.getRoot(repositoryName, outputDirName, directories); + : OutputDirectory.COVERAGE.getRoot( + repositoryName, outputDirName, directories, mainRepositoryName); } /** * Returns the testlogs directory for this build configuration. */ public Root getTestLogsDirectory(RepositoryName repositoryName) { - return repositoryName.equals(RepositoryName.MAIN) + return repositoryName.isMain() || repositoryName.equals(mainRepositoryName) ? testlogsDirectoryForMainRepository - : OutputDirectory.TESTLOGS.getRoot(repositoryName, outputDirName, directories); + : OutputDirectory.TESTLOGS.getRoot( + repositoryName, outputDirName, directories, mainRepositoryName); } /** @@ -2218,9 +2234,10 @@ * Returns the internal directory (used for middlemen) for this build configuration. */ public Root getMiddlemanDirectory(RepositoryName repositoryName) { - return repositoryName.equals(RepositoryName.MAIN) + return repositoryName.isMain() || repositoryName.equals(mainRepositoryName) ? middlemanDirectoryForMainRepository - : OutputDirectory.MIDDLEMAN.getRoot(repositoryName, outputDirName, directories); + : OutputDirectory.MIDDLEMAN.getRoot( + repositoryName, outputDirName, directories, mainRepositoryName); } public boolean getAllowRuntimeDepsOnNeverLink() { @@ -2235,6 +2252,10 @@ return options.pluginList; } + public String getMainRepositoryName() { + return mainRepositoryName.strippedName(); + } + /** * Returns the configuration-dependent string for this configuration. This is also the name of the * configuration's base output directory unless {@link Options#outputDirectoryName} overrides it.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationFactory.java index 9301784..3a95a4b 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationFactory.java
@@ -69,10 +69,11 @@ Cache<String, BuildConfiguration> cache, PackageProviderForConfigurations loadedPackageProvider, BuildOptions buildOptions, - EventHandler errorEventListener) + EventHandler errorEventListener, + String mainRepositoryName) throws InvalidConfigurationException, InterruptedException { return configurationCollectionFactory.createConfigurations(this, cache, - loadedPackageProvider, buildOptions, errorEventListener); + loadedPackageProvider, buildOptions, errorEventListener, mainRepositoryName); } /** @@ -85,7 +86,8 @@ public BuildConfiguration getConfiguration( PackageProviderForConfigurations loadedPackageProvider, BuildOptions buildOptions, - Cache<String, BuildConfiguration> cache) + Cache<String, BuildConfiguration> cache, + String repositoryName) throws InvalidConfigurationException, InterruptedException { String cacheKey = buildOptions.computeCacheKey(); @@ -108,7 +110,7 @@ return null; } - result = new BuildConfiguration(directories, fragments, buildOptions); + result = new BuildConfiguration(directories, fragments, buildOptions, repositoryName); cache.put(cacheKey, result); return result; }
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java index 4cf089b..1459eec 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java
@@ -51,11 +51,12 @@ Cache<String, BuildConfiguration> cache, PackageProviderForConfigurations packageProvider, BuildOptions buildOptions, - EventHandler eventHandler) + EventHandler eventHandler, + String mainRepositoryName) throws InvalidConfigurationException, InterruptedException { // Target configuration BuildConfiguration targetConfiguration = configurationFactory.getConfiguration( - packageProvider, buildOptions, cache); + packageProvider, buildOptions, cache, mainRepositoryName); if (targetConfiguration == null) { return null; } @@ -66,7 +67,7 @@ // Note that this passes in the dataConfiguration, not the target // configuration. This is intentional. BuildConfiguration hostConfiguration = getHostConfigurationFromRequest(configurationFactory, - packageProvider, dataConfiguration, buildOptions, cache); + packageProvider, dataConfiguration, buildOptions, cache, mainRepositoryName); if (hostConfiguration == null) { return null; } @@ -76,7 +77,7 @@ for (SplitTransition<BuildOptions> transition : buildOptions.getPotentialSplitTransitions()) { for (BuildOptions splitOptions : transition.split(buildOptions)) { BuildConfiguration splitConfig = configurationFactory.getConfiguration( - packageProvider, splitOptions, cache); + packageProvider, splitOptions, cache, mainRepositoryName); splitTransitionsTable.put(transition, splitConfig); } } @@ -133,14 +134,15 @@ PackageProviderForConfigurations loadedPackageProvider, BuildConfiguration requestConfig, BuildOptions buildOptions, - Cache<String, BuildConfiguration> cache) + Cache<String, BuildConfiguration> cache, + String repositoryName) throws InvalidConfigurationException, InterruptedException { BuildConfiguration.Options commonOptions = buildOptions.get(BuildConfiguration.Options.class); if (!commonOptions.useDistinctHostConfiguration) { return requestConfig; } else { BuildConfiguration hostConfig = configurationFactory.getConfiguration( - loadedPackageProvider, buildOptions.createHostOptions(false), cache); + loadedPackageProvider, buildOptions.createHostOptions(false), cache, repositoryName); if (hostConfig == null) { return null; }
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java index 29ccba9..135ca86 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java
@@ -217,7 +217,7 @@ zipRunfilesPath = workspaceName.getRelative(path).normalize().toString(); } else { // If the file is in external package, strip "external" - zipRunfilesPath = path.relativeTo(Label.EXTERNAL_PACKAGE_NAME).normalize().toString(); + zipRunfilesPath = path.relativeTo(Label.EXTERNAL_PATH_PREFIX).normalize().toString(); } // We put the whole runfiles tree under the ZIP_RUNFILES_DIRECTORY_NAME directory, by doing this // , we avoid the conflict between default workspace name "__main__" and __main__.py file.
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java index 9c1db17..6692a9e 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
@@ -143,12 +143,6 @@ try { env.getEventBus().post(new BuildStartingEvent(env, request)); LOG.info("Build identifier: " + request.getId()); - executionTool = new ExecutionTool(env, request); - if (needsExecutionPhase(request.getBuildOptions())) { - // Initialize the execution tool early if we need it. This hides the latency of setting up - // the execution backends. - executionTool.init(); - } // Error out early if multi_cpus is set, but we're not in build or test command. if (!request.getMultiCpus().isEmpty()) { @@ -168,6 +162,11 @@ // Target pattern evaluation. LoadingResult loadingResult = evaluateTargetPatterns(request, validator); + env.setWorkspaceName(loadingResult.getWorkspaceName()); + executionTool = new ExecutionTool(env, request); + if (needsExecutionPhase(request.getBuildOptions())) { + executionTool.init(); + } // Exit if there are any pending exceptions from modules. env.throwPendingException();
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 0d435f2..c706046 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
@@ -184,7 +184,6 @@ this.runtime = env.getRuntime(); this.request = request; - // Create tools before getting the strategies from the modules as some of them need tools to // determine whether the host actually supports certain strategies (e.g. sandboxing). createToolsSymlinks(); @@ -333,7 +332,7 @@ TopLevelArtifactContext topLevelArtifactContext) throws BuildFailedException, InterruptedException, TestExecException, AbruptExitException { Stopwatch timer = Stopwatch.createStarted(); - prepare(packageRoots, analysisResult.getWorkspaceName()); + prepare(packageRoots); ActionGraph actionGraph = analysisResult.getActionGraph(); @@ -347,7 +346,7 @@ request.getBuildOptions().finalizeActions); } else { // TODO(bazel-team): this could be just another OutputService - startLocalOutputBuild(analysisResult.getWorkspaceName()); + startLocalOutputBuild(); } // Must be created after the output path is created above. @@ -358,10 +357,10 @@ ? targetConfigurations.get(0) : null; if (targetConfigurations.size() == 1) { String productName = runtime.getProductName(); - String dirName = env.getWorkspaceName(); + String workspaceName = env.getWorkspaceName(); OutputDirectoryLinksUtils.createOutputDirectoryLinks( - dirName, env.getWorkspace(), env.getDirectories().getExecRoot(), - env.getDirectories().getOutputPath(), getReporter(), targetConfiguration, + workspaceName, env.getWorkspace(), env.getDirectories().getExecRoot(workspaceName), + env.getDirectories().getOutputPath(workspaceName), getReporter(), targetConfiguration, request.getBuildOptions().getSymlinkPrefix(productName), productName); } @@ -493,7 +492,7 @@ } } - private void prepare(ImmutableMap<PackageIdentifier, Path> packageRoots, String workspaceName) + private void prepare(ImmutableMap<PackageIdentifier, Path> packageRoots) throws ExecutorInitException { // Prepare for build. Profiler.instance().markPhase(ProfilePhase.PREPARE); @@ -501,7 +500,7 @@ // Plant the symlink forest. try { new SymlinkForest( - packageRoots, getExecRoot(), runtime.getProductName(), workspaceName) + packageRoots, getExecRoot(), runtime.getProductName(), env.getWorkspaceName()) .plantSymlinkForest(); } catch (IOException e) { throw new ExecutorInitException("Source forest creation failed", e); @@ -510,7 +509,7 @@ private void createToolsSymlinks() throws ExecutorInitException { try { - env.getBlazeWorkspace().getBinTools().setupBuildTools(); + env.getBlazeWorkspace().getBinTools().setupBuildTools(env.getWorkspaceName()); } catch (ExecException e) { throw new ExecutorInitException("Tools symlink creation failed", e); } @@ -531,9 +530,9 @@ /** * Prepare for a local output build. */ - private void startLocalOutputBuild(String workspaceName) throws ExecutorInitException { + private void startLocalOutputBuild() throws ExecutorInitException { try (AutoProfiler p = AutoProfiler.profiled("Starting local output build", ProfilerTask.INFO)) { - Path outputPath = env.getDirectories().getOutputPath(workspaceName); + Path outputPath = env.getDirectories().getOutputPath(env.getWorkspaceName()); Path localOutputPath = env.getDirectories().getLocalOutputPath(); if (outputPath.isSymbolicLink()) {
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/OutputDirectoryLinksUtils.java b/src/main/java/com/google/devtools/build/lib/buildtool/OutputDirectoryLinksUtils.java index 40d8d0f..fd1fae5 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/OutputDirectoryLinksUtils.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/OutputDirectoryLinksUtils.java
@@ -78,15 +78,16 @@ } // Points to execroot - createLink(workspace, execRootSymlink(symlinkPrefix, workspaceName), execRoot, failures); - + createLink(workspace, execRootSymlink( + symlinkPrefix, workspace.getBaseName()), execRoot, failures); + RepositoryName repositoryName = RepositoryName.createFromValidStrippedName(workspaceName); if (targetConfig != null) { createLink(workspace, symlinkPrefix + "bin", - targetConfig.getBinDirectory(RepositoryName.MAIN).getPath(), failures); + targetConfig.getBinDirectory(repositoryName).getPath(), failures); createLink(workspace, symlinkPrefix + "testlogs", - targetConfig.getTestLogsDirectory(RepositoryName.MAIN).getPath(), failures); + targetConfig.getTestLogsDirectory(repositoryName).getPath(), failures); createLink(workspace, symlinkPrefix + "genfiles", - targetConfig.getGenfilesDirectory(RepositoryName.MAIN).getPath(), failures); + targetConfig.getGenfilesDirectory(repositoryName).getPath(), failures); } if (!failures.isEmpty()) { @@ -168,6 +169,7 @@ removeLink(workspace, outputSymlinkName, failures); } removeLink(workspace, execRootSymlink(symlinkPrefix, workspaceName), failures); + removeLink(workspace, execRootSymlink(symlinkPrefix, workspace.getBaseName()), failures); removeLink(workspace, symlinkPrefix + "bin", failures); removeLink(workspace, symlinkPrefix + "testlogs", failures); removeLink(workspace, symlinkPrefix + "genfiles", failures);
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Rule.java b/src/main/java/com/google/devtools/build/lib/packages/Rule.java index da411c1..1a7cc36d 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Rule.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Rule.java
@@ -728,7 +728,7 @@ * @return The repository name. */ public RepositoryName getRepository() { - return getLabel().getPackageIdentifier().getRepository(); + return RepositoryName.createFromValidStrippedName(pkg.getWorkspaceName()); } /** Returns the suffix of target kind for all rules. */
diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java index d26ede3..3166957 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
@@ -141,8 +141,6 @@ /** * Parses the given WORKSPACE file without resolving skylark imports. - * - * <p>Called by com.google.devtools.build.workspace.Resolver from //src/tools/generate_workspace. */ public void parse(ParserInputSource source) throws BuildFileContainsErrorsException, InterruptedException {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportFunction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportFunction.java index 4a1cbe2..d4c8389 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportFunction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportFunction.java
@@ -16,6 +16,7 @@ import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.rules.cpp.FdoSupport.FdoException; import com.google.devtools.build.lib.skyframe.PrecomputedValue; +import com.google.devtools.build.lib.skyframe.WorkspaceNameValue; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionException; @@ -46,12 +47,13 @@ public SkyValue compute(SkyKey skyKey, Environment env) throws FdoSkyException, InterruptedException { BlazeDirectories blazeDirectories = PrecomputedValue.BLAZE_DIRECTORIES.get(env); + WorkspaceNameValue workspaceNameValue = (WorkspaceNameValue) env.getValue( + WorkspaceNameValue.key()); if (env.valuesMissing()) { return null; } - // TODO(kchodorow): create a SkyFunction to get the main repository name and pass it in here. - Path execRoot = blazeDirectories.getExecRoot(); + Path execRoot = blazeDirectories.getExecRoot(workspaceNameValue.getName()); FdoSupportValue.Key key = (FdoSupportValue.Key) skyKey.argument(); FdoSupport fdoSupport; try {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/TestResult.java b/src/main/java/com/google/devtools/build/lib/rules/test/TestResult.java index df5abb4..966a6af 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/test/TestResult.java +++ b/src/main/java/com/google/devtools/build/lib/rules/test/TestResult.java
@@ -23,7 +23,6 @@ import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.Path; -import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.view.test.TestStatus.BlazeTestStatus; import com.google.devtools.build.lib.view.test.TestStatus.TestResultData; import java.util.Collection; @@ -93,9 +92,9 @@ /** * @return Coverage data artifact, if available and null otherwise. */ - public PathFragment getCoverageData() { + public Path getCoverageData() { if (data.getHasCoverage()) { - return testAction.getCoverageData().getExecPath(); + return testAction.getCoverageData().getPath(); } return null; }
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java index 5be28a2..c0eda33 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java
@@ -150,15 +150,6 @@ } /** - * Returns the execution root directory associated with this Blaze server - * process. This is where all input and output files visible to the actual - * build reside. - */ - public Path getExecRoot() { - return directories.getExecRoot(); - } - - /** * Returns path to the cache directory. Path must be inside output base to * ensure that users can run concurrent instances of blaze in different * clients without attempting to concurrently write to the same action cache
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java index 85be56c..7b093af 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java
@@ -93,6 +93,7 @@ private long commandStartTime; private OutputService outputService; private Path workingDirectory; + private String workspaceName; private String commandName; private OptionsProvider options; @@ -145,6 +146,7 @@ // TODO(ulfjack): We don't call beforeCommand() in tests, but rely on workingDirectory being set // in setupPackageCache(). This leads to NPE if we don't set it here. this.workingDirectory = directories.getWorkspace(); + this.workspaceName = null; workspace.getSkyframeExecutor().setEventBus(eventBus); } @@ -312,13 +314,14 @@ } public String getWorkspaceName() { - Path workspace = getDirectories().getWorkspace(); - if (workspace == null) { - return ""; - } - return workspace.getBaseName(); + Preconditions.checkNotNull(workspaceName); + return workspaceName; } + public void setWorkspaceName(String workspaceName) { + Preconditions.checkState(this.workspaceName == null, "workspace name can only be set once"); + this.workspaceName = workspaceName; + } /** * Returns if the client passed a valid workspace to be used for the build. */ @@ -341,7 +344,8 @@ * build reside. */ public Path getExecRoot() { - return getDirectories().getExecRoot(); + Preconditions.checkNotNull(workspaceName); + return getDirectories().getExecRoot(workspaceName); } /**
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/TestResultAnalyzer.java b/src/main/java/com/google/devtools/build/lib/runtime/TestResultAnalyzer.java index 8206527..d23b7e5 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/TestResultAnalyzer.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/TestResultAnalyzer.java
@@ -30,9 +30,7 @@ import com.google.devtools.build.lib.runtime.TerminalTestResultNotifier.TestSummaryOptions; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.Path; -import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.view.test.TestStatus.BlazeTestStatus; - import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -45,7 +43,6 @@ */ @ThreadCompatible public class TestResultAnalyzer { - private final Path execRoot; private final TestSummaryOptions summaryOptions; private final ExecutionOptions executionOptions; private final EventBus eventBus; @@ -59,11 +56,9 @@ * @param executionOptions Parsed build/test execution options. * @param eventBus For reporting failed to build and cached tests. */ - public TestResultAnalyzer(Path execRoot, - TestSummaryOptions summaryOptions, + public TestResultAnalyzer(TestSummaryOptions summaryOptions, ExecutionOptions executionOptions, EventBus eventBus) { - this.execRoot = execRoot; this.summaryOptions = summaryOptions; this.executionOptions = executionOptions; this.eventBus = eventBus; @@ -211,10 +206,9 @@ numLocalActionCached++; } - PathFragment coverageData = result.getCoverageData(); + Path coverageData = result.getCoverageData(); if (coverageData != null) { - summaryBuilder.addCoverageFiles( - Collections.singletonList(execRoot.getRelative(coverageData))); + summaryBuilder.addCoverageFiles(Collections.singletonList(coverageData)); } if (!executionOptions.runsPerTestDetectsFlakes) {
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java index 466c3f9..8dd171b 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java
@@ -219,6 +219,7 @@ private void actuallyClean(CommandEnvironment env, Path outputBase, Options cleanOptions, String symlinkPrefix) throws IOException, ShutdownBlazeServerException, CommandException, ExecException, InterruptedException { + String workspaceDirectory = env.getWorkspace().getBaseName(); if (env.getOutputService() != null) { env.getOutputService().clean(); } @@ -241,8 +242,7 @@ env.getBlazeWorkspace().clearCaches(); // In order to be sure that we delete everything, delete the workspace directory both for // --deep_execroot and for --nodeep_execroot. - for (String directory : new String[] { - env.getWorkspaceName(), "execroot/" + env.getWorkspaceName() }) { + for (String directory : new String[] {workspaceDirectory, "execroot"}) { Path child = outputBase.getRelative(directory); if (child.exists()) { LOG.finest("Cleaning " + child + (cleanOptions.async ? " asynchronously..." : "")); @@ -256,8 +256,8 @@ } // remove convenience links OutputDirectoryLinksUtils.removeOutputDirectoryLinks( - env.getWorkspaceName(), env.getWorkspace(), env.getReporter(), symlinkPrefix, - env.getRuntime().getProductName()); + workspaceDirectory, env.getWorkspace(), env.getReporter(), + symlinkPrefix, env.getRuntime().getProductName()); // shutdown on expunge cleans if (cleanOptions.expunge || cleanOptions.expunge_async) { throw new ShutdownBlazeServerException(0);
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/InfoItem.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/InfoItem.java index 3f0ba62..bf47b14 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/InfoItem.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/InfoItem.java
@@ -181,7 +181,7 @@ public byte[] get(Supplier<BuildConfiguration> configurationSupplier, CommandEnvironment env) throws AbruptExitException { checkNotNull(env); - return print(env.getRuntime().getWorkspace().getExecRoot()); + return print(env.getDirectories().getExecRoot()); } }
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/ProfileCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/ProfileCommand.java index 970cb5c..bbc7a02 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/ProfileCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/ProfileCommand.java
@@ -192,7 +192,7 @@ MultiProfileStatistics statistics = new MultiProfileStatistics( env.getWorkingDirectory(), - env.getWorkspaceName(), + env.getWorkspace().getBaseName(), options.getResidue(), getInfoListener(env), opts.vfsStatsLimit > 0); @@ -248,7 +248,7 @@ phaseStatistics.put( phase, new PhaseStatistics( - phase, info, env.getWorkspaceName(), opts.vfsStatsLimit > 0)); + phase, info, env.getWorkspace().getBaseName(), opts.vfsStatsLimit > 0)); } CriticalPathStatistics critPathStats = new CriticalPathStatistics(info);
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java index cd31c7d..94b71d8 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java
@@ -85,7 +85,6 @@ @Override public ExitCode exec(CommandEnvironment env, OptionsProvider options) { TestResultAnalyzer resultAnalyzer = new TestResultAnalyzer( - env.getDirectories().getExecRoot(), options.getOptions(TestSummaryOptions.class), options.getOptions(ExecutionOptions.class), env.getEventBus());
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java index fb09a67..16a4efd 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java
@@ -74,7 +74,7 @@ buildRequest.getOptions(SandboxOptions.class)); this.sandboxOptions = buildRequest.getOptions(SandboxOptions.class); this.blazeDirs = cmdEnv.getDirectories(); - this.execRoot = blazeDirs.getExecRoot(); + this.execRoot = cmdEnv.getExecRoot(); this.verboseFailures = verboseFailures; this.spawnInputExpander = new SpawnInputExpander(false); this.inaccessibleHelperFile = inaccessibleHelperFile;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationFunction.java index 6044583..b515fd7 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationFunction.java
@@ -53,6 +53,12 @@ @Override public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException, BuildConfigurationFunctionException { + WorkspaceNameValue workspaceNameValue = (WorkspaceNameValue) env + .getValue(WorkspaceNameValue.key()); + if (workspaceNameValue == null) { + return null; + } + BuildConfigurationValue.Key key = (BuildConfigurationValue.Key) skyKey.argument(); Set<Fragment> fragments; try { @@ -70,7 +76,7 @@ } BuildConfiguration config = new BuildConfiguration(directories, fragmentsMap, - key.getBuildOptions()); + key.getBuildOptions(), workspaceNameValue.getName()); // Unlike static configurations, dynamic configurations don't need to embed transition logic // within the configuration itself. However we still use this interface to provide a mapping // between Transition types (e.g. HOST) and the dynamic transitions that apply those
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionFunction.java index 4e91031..79011a8 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionFunction.java
@@ -64,7 +64,7 @@ return null; } RepositoryName repositoryName = RepositoryName.createFromValidStrippedName( - nameValue.maybeGetName()); + nameValue.getName()); final ArtifactFactory factory = artifactFactory.get(); BuildInfoContext context = new BuildInfoContext() {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationCollectionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationCollectionFunction.java index d69da90..f782e15 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationCollectionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationCollectionFunction.java
@@ -56,11 +56,17 @@ @Override public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException, ConfigurationCollectionFunctionException { + WorkspaceNameValue workspaceNameValue = (WorkspaceNameValue) env + .getValue(WorkspaceNameValue.key()); + if (workspaceNameValue == null) { + return null; + } ConfigurationCollectionKey collectionKey = (ConfigurationCollectionKey) skyKey.argument(); try { BuildConfigurationCollection result = getConfigurations(env, new SkyframePackageLoaderWithValueEnvironment(env, ruleClassProvider), - collectionKey.getBuildOptions(), collectionKey.getMultiCpu()); + collectionKey.getBuildOptions(), collectionKey.getMultiCpu(), + workspaceNameValue.getName()); // BuildConfigurationCollection can be created, but dependencies to some files might be // missing. In that case we need to build configurationCollection again. @@ -79,7 +85,8 @@ Environment env, PackageProviderForConfigurations loadedPackageProvider, BuildOptions buildOptions, - ImmutableSet<String> multiCpu) + ImmutableSet<String> multiCpu, + String repositoryName) throws InvalidConfigurationException, InterruptedException { // We cache all the related configurations for this target configuration in a cache that is // dropped at the end of this method call. We instead rely on the cache for entire collections @@ -92,7 +99,7 @@ if (!multiCpu.isEmpty()) { for (String cpu : multiCpu) { BuildConfiguration targetConfiguration = createConfiguration( - cache, env.getListener(), loadedPackageProvider, buildOptions, cpu); + cache, env.getListener(), loadedPackageProvider, buildOptions, cpu, repositoryName); if (targetConfiguration == null || targetConfigurations.contains(targetConfiguration)) { continue; } @@ -103,7 +110,7 @@ } } else { BuildConfiguration targetConfiguration = createConfiguration( - cache, env.getListener(), loadedPackageProvider, buildOptions, null); + cache, env.getListener(), loadedPackageProvider, buildOptions, null, repositoryName); if (targetConfiguration == null) { return null; } @@ -168,7 +175,8 @@ ExtendedEventHandler originalEventListener, PackageProviderForConfigurations loadedPackageProvider, BuildOptions buildOptions, - String cpuOverride) + String cpuOverride, + String repositoryName) throws InvalidConfigurationException, InterruptedException { ErrorSensingEventHandler eventHandler = new ErrorSensingEventHandler(originalEventListener); if (cpuOverride != null) { @@ -180,7 +188,7 @@ } BuildConfiguration targetConfig = configurationFactory.get().createConfigurations( - cache, loadedPackageProvider, buildOptions, eventHandler); + cache, loadedPackageProvider, buildOptions, eventHandler, repositoryName); if (targetConfig == null) { return null; }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java index ce2ec24..a2868b2 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
@@ -454,13 +454,7 @@ if (workspaceNameValue == null) { return null; } - String workspaceName = workspaceNameValue.maybeGetName(); - if (workspaceName == null) { - throw new PackageFunctionException( - new BuildFileContainsErrorsException(Label.EXTERNAL_PACKAGE_IDENTIFIER), - Transience.PERSISTENT); - } - + String workspaceName = workspaceNameValue.getName(); RootedPath buildFileRootedPath = packageLookupValue.getRootedPath(packageId); FileValue buildFileValue = null; Path buildFilePath = buildFileRootedPath.asPath();
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameFunction.java index f6dc151..9284f5c 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameFunction.java
@@ -14,9 +14,11 @@ package com.google.devtools.build.lib.skyframe; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException; import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.skyframe.SkyFunction; +import com.google.devtools.build.skyframe.SkyFunctionException; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; import javax.annotation.Nullable; @@ -24,22 +26,24 @@ /** * {@link SkyFunction} for {@link WorkspaceNameValue}s. * - * <p>All transitive errors (e.g. a symlink cycle encountered when consuming the WORKSPACE file) - * result in a {@link NoSuchPackageException}. + * <p>All errors (e.g. parsing errors or a symlink cycle encountered when consuming the WORKSPACE + * file) result in a {@link NoSuchPackageException}. */ public class WorkspaceNameFunction implements SkyFunction { @Override @Nullable - public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException { + public SkyValue compute(SkyKey skyKey, Environment env) + throws InterruptedException, WorkspaceNameFunctionException { SkyKey externalPackageKey = PackageValue.key(Label.EXTERNAL_PACKAGE_IDENTIFIER); PackageValue externalPackageValue = (PackageValue) env.getValue(externalPackageKey); if (externalPackageValue == null) { return null; } Package externalPackage = externalPackageValue.getPackage(); - return externalPackage.containsErrors() - ? WorkspaceNameValue.withError() - : WorkspaceNameValue.withName(externalPackage.getWorkspaceName()); + if (externalPackage.containsErrors()) { + throw new WorkspaceNameFunctionException(); + } + return WorkspaceNameValue.withName(externalPackage.getWorkspaceName()); } @Override @@ -47,4 +51,11 @@ public String extractTag(SkyKey skyKey) { return null; } + + private class WorkspaceNameFunctionException extends SkyFunctionException { + WorkspaceNameFunctionException() { + super(new BuildFileContainsErrorsException(Label.EXTERNAL_PACKAGE_IDENTIFIER), + Transience.PERSISTENT); + } + } }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameValue.java index 668c636..4da943b 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameValue.java
@@ -18,7 +18,6 @@ import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; import java.util.Objects; -import javax.annotation.Nullable; /** * A value that solely represents the 'name' of a Bazel workspace, as defined in the WORKSPACE file. @@ -31,20 +30,17 @@ public class WorkspaceNameValue implements SkyValue { private static final SkyKey KEY = LegacySkyKey.create(SkyFunctions.WORKSPACE_NAME, DummyArgument.INSTANCE); - private static final WorkspaceNameValue ERROR = new WorkspaceNameValue(null); - @Nullable private final String workspaceName; - private WorkspaceNameValue(@Nullable String workspaceName) { + private WorkspaceNameValue(String workspaceName) { this.workspaceName = workspaceName; } /** - * Returns the name of the workspace, or {@code null} if there was an error in the WORKSPACE file. + * Returns the name of the workspace. */ - @Nullable - public String maybeGetName() { + public String getName() { return workspaceName; } @@ -53,11 +49,6 @@ return KEY; } - /** Returns a {@link WorkspaceNameValue} for a workspace whose WORKSPACE file is in error. */ - public static WorkspaceNameValue withError() { - return WorkspaceNameValue.ERROR; - } - /** Returns a {@link WorkspaceNameValue} for a workspace with the given name. */ public static WorkspaceNameValue withName(String workspaceName) { return new WorkspaceNameValue(Preconditions.checkNotNull(workspaceName)); @@ -84,7 +75,7 @@ /** Singleton class used as the {@link SkyKey#argument} for {@link WorkspaceNameValue#key}. */ public static final class DummyArgument { - public static final int HASHCODE = DummyArgument.class.getCanonicalName().hashCode(); + static final int HASHCODE = DummyArgument.class.getCanonicalName().hashCode(); public static final DummyArgument INSTANCE = new DummyArgument(); private DummyArgument() {
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerActionContextProvider.java index bf28e66..f480117 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerActionContextProvider.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerActionContextProvider.java
@@ -35,7 +35,7 @@ WorkerSpawnStrategy workerSpawnStrategy = new WorkerSpawnStrategy( - env.getDirectories(), + env.getExecRoot(), workers, buildRequest.getOptions(ExecutionOptions.class).verboseFailures, extraFlags.build());
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java index 2bd4fb1..dbe8bde 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java
@@ -37,7 +37,6 @@ import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnActionContext; import com.google.devtools.build.lib.actions.UserExecException; -import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.exec.SpawnInputExpander; import com.google.devtools.build.lib.sandbox.SandboxHelpers; @@ -88,13 +87,13 @@ private final SpawnInputExpander spawnInputExpander; public WorkerSpawnStrategy( - BlazeDirectories blazeDirs, + Path execRoot, WorkerPool workers, boolean verboseFailures, Multimap<String, String> extraFlags) { Preconditions.checkNotNull(workers); this.workers = Preconditions.checkNotNull(workers); - this.execRoot = blazeDirs.getExecRoot(); + this.execRoot = execRoot; this.verboseFailures = verboseFailures; this.extraFlags = extraFlags; this.spawnInputExpander = new SpawnInputExpander(false);