Remove all usages of BlazeDirectories#(getExecRoot()|getOutputPath()) from Bazel
Bazel's execution root path looks as follows: output_base/execroot/workspace_name. That is one needs to know the workspace name when constructing it. Bazel only knows the workspace name after/during the loading phase however BlazeDirectories is constructed before that. So BlazeDirectories.getExecRoot() returns a path to a folder that's not the exec root (it takes the name of the directory that contains the WORKSPACE file as the workspace name) and this leads to all kinds of bugs as one can imagine.
Thus I removed all calls to BlazeDirectories.(getExecRoot()|getOutputPath()) in Bazel and replaced them with calls to BlazeDirectories.getExecRoot(workspaceName).
What this change does:
- All artifacts in Bazel agree on one exec root.
- Bazel will no longer create two execroot directories on disk.
- Renamed getExecRoot to getBlazeExecRoot() and updated the code to return null when called in Bazel in order to prevent future wrong use of this method.
- Resolved a TODO in BlazeDirectories where we had code to support the workspace path being null.
RELNOTES: None.
PiperOrigin-RevId: 253790673
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java b/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java
index dcc4ccd..649471f 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java
@@ -41,7 +41,6 @@
private static final int CONCURRENCY_LEVEL = Runtime.getRuntime().availableProcessors();
private static final Striped<Lock> STRIPED_LOCK = Striped.lock(CONCURRENCY_LEVEL);
- private final Path execRoot;
private final Path execRootParent;
private final PathFragment derivedPathPrefix;
private ImmutableMap<Root, ArtifactRoot> sourceArtifactRoots;
@@ -133,12 +132,10 @@
/**
* Constructs a new artifact factory that will use a given execution root when creating artifacts.
*
- * @param execRoot the execution root Path to use. This will be
- * [output_base]/execroot/[workspace].
+ * @param execRootParent the execution root's parent path. This will be [output_base]/execroot.
*/
- public ArtifactFactory(Path execRoot, String derivedPathPrefix) {
- this.execRoot = execRoot;
- this.execRootParent = execRoot.getParentDirectory();
+ public ArtifactFactory(Path execRootParent, String derivedPathPrefix) {
+ this.execRootParent = execRootParent;
this.derivedPathPrefix = PathFragment.create(derivedPathPrefix);
}
@@ -474,7 +471,7 @@
}
@Override
- public Path getPathFromSourceExecPath(PathFragment execPath) {
+ public Path getPathFromSourceExecPath(Path execRoot, PathFragment execPath) {
Preconditions.checkState(
!execPath.startsWith(derivedPathPrefix), "%s is derived: %s", execPath, derivedPathPrefix);
Root sourceRoot =
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ArtifactResolver.java b/src/main/java/com/google/devtools/build/lib/actions/ArtifactResolver.java
index 25af4fe..0b02f9c 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ArtifactResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ArtifactResolver.java
@@ -80,7 +80,7 @@
Map<PathFragment, Artifact> resolveSourceArtifacts(
Iterable<PathFragment> execPaths, PackageRootResolver resolver) throws InterruptedException;
- Path getPathFromSourceExecPath(PathFragment execPath);
+ Path getPathFromSourceExecPath(Path execRoot, PathFragment execPath);
/**
* Supplies an {@link ArtifactFactory}. We define a custom interface because parameterized types
diff --git a/src/main/java/com/google/devtools/build/lib/actions/CompletionContext.java b/src/main/java/com/google/devtools/build/lib/actions/CompletionContext.java
index ae63c83..a4f9848 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/CompletionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/CompletionContext.java
@@ -40,12 +40,13 @@
Map<Artifact, Collection<Artifact>> expandedArtifacts,
Map<Artifact, ImmutableList<FilesetOutputSymlink>> expandedFilesets,
ActionInputMap inputMap,
- PathResolverFactory pathResolverFactory) {
+ PathResolverFactory pathResolverFactory,
+ String workspaceName) {
ArtifactExpander expander = new ArtifactExpanderImpl(expandedArtifacts, expandedFilesets);
ArtifactPathResolver pathResolver =
pathResolverFactory.shouldCreatePathResolverForArtifactValues()
? pathResolverFactory.createPathResolverForArtifactValues(
- inputMap, expandedArtifacts, expandedFilesets.keySet())
+ inputMap, expandedArtifacts, expandedFilesets.keySet(), workspaceName)
: ArtifactPathResolver.IDENTITY;
return new AutoValue_CompletionContext(expander, pathResolver);
}
@@ -83,7 +84,8 @@
ArtifactPathResolver createPathResolverForArtifactValues(
ActionInputMap actionInputMap,
Map<Artifact, Collection<Artifact>> expandedArtifacts,
- Iterable<Artifact> filesets);
+ Iterable<Artifact> filesets,
+ String workspaceName);
boolean shouldCreatePathResolverForArtifactValues();
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BlazeDirectories.java b/src/main/java/com/google/devtools/build/lib/analysis/BlazeDirectories.java
index 27fce36..9267455 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BlazeDirectories.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BlazeDirectories.java
@@ -15,6 +15,7 @@
package com.google.devtools.build.lib.analysis;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Ascii;
import com.google.common.hash.HashCode;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
@@ -23,6 +24,7 @@
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.Objects;
+import javax.annotation.Nullable;
/**
* Encapsulates the directories related to a workspace.
@@ -59,11 +61,11 @@
*/
private final Path defaultSystemJavabase;
/** The root of all build actions. */
- private final Path execRoot;
+ private final Path blazeExecRoot;
// These two are kept to avoid creating new objects every time they are accessed. This showed up
// in a profiler.
- private final Path outputPath;
+ private final Path blazeOutputPath;
private final Path localOutputPath;
private final String productName;
@@ -78,18 +80,22 @@
this.defaultSystemJavabase = defaultSystemJavabase;
this.productName = productName;
Path outputBase = serverDirectories.getOutputBase();
- boolean useDefaultExecRootName =
- this.workspace == null || this.workspace.getParentDirectory() == null;
- if (useDefaultExecRootName) {
- // TODO(bazel-team): if workspace is null execRoot should be null, but at the moment there is
- // a lot of code that depends on it being non-null.
- this.execRoot = serverDirectories.getExecRootBase().getChild(DEFAULT_EXEC_ROOT);
+ if (Ascii.equalsIgnoreCase(productName, "blaze")) {
+ boolean useDefaultExecRootName =
+ this.workspace == null || this.workspace.getParentDirectory() == null;
+ if (useDefaultExecRootName) {
+ // TODO(bazel-team): if workspace is null execRoot should be null, but at the moment there
+ // is a lot of code that depends on it being non-null.
+ this.blazeExecRoot = serverDirectories.getExecRootBase().getChild(DEFAULT_EXEC_ROOT);
+ } else {
+ this.blazeExecRoot = serverDirectories.getExecRootBase().getChild(workspace.getBaseName());
+ }
+ this.blazeOutputPath = blazeExecRoot.getRelative(getRelativeOutputPath());
} else {
- this.execRoot = serverDirectories.getExecRootBase().getChild(workspace.getBaseName());
+ this.blazeExecRoot = null;
+ this.blazeOutputPath = null;
}
- String relativeOutputPath = getRelativeOutputPath(productName);
- this.outputPath = execRoot.getRelative(getRelativeOutputPath());
- this.localOutputPath = outputBase.getRelative(relativeOutputPath);
+ this.localOutputPath = outputBase.getRelative(getRelativeOutputPath());
}
public ServerDirectories getServerDirectories() {
@@ -132,12 +138,17 @@
}
/**
- * Returns the execution root for the main package. This is created before the workspace file has
- * been read, so it has an incorrect path. Use {@link #getExecRoot(String)} instead.
+ * Returns the execution root of Blaze.
+ *
+ * @deprecated Avoid using this method as it will only work if your workspace is named like
+ * Google's internal workspace. This method will not work in Bazel. Use {@link
+ * #getExecRoot(String)} instead.
+ * <p><em>AVOID USING THIS METHOD</em>
*/
+ @Nullable
@Deprecated
- public Path getExecRoot() {
- return execRoot;
+ public Path getBlazeExecRoot() {
+ return blazeExecRoot;
}
/**
@@ -150,12 +161,17 @@
}
/**
- * Returns the output path for the main repository using the workspace's directory name. Use
- * {@link #getOutputPath(String)}, instead.
+ * Returns the output path of Blaze.
+ *
+ * @deprecated Avoid using this method as it will only work if your workspace is named like
+ * Google's internal workspace. This method will not work in Bazel. Use {@link
+ * #getOutputPath(String)} instead.
+ * <p><em>AVOID USING THIS METHOD</em>
*/
+ @Nullable
@Deprecated
- public Path getOutputPath() {
- return outputPath;
+ public Path getBlazeOutputPath() {
+ return blazeOutputPath;
}
/** Returns the output path used by this Blaze instance. */
@@ -216,8 +232,8 @@
@Override
public int hashCode() {
- // execRoot is derivable from other fields, but better safe than sorry.
- return Objects.hash(serverDirectories, workspace, productName, execRoot);
+ // blazeExecRoot is derivable from other fields, but better safe than sorry.
+ return Objects.hash(serverDirectories, workspace, productName);
}
@Override
@@ -231,8 +247,6 @@
BlazeDirectories that = (BlazeDirectories) obj;
return this.serverDirectories.equals(that.serverDirectories)
&& this.workspace.equals(that.workspace)
- && this.productName.equals(that.productName)
- // execRoot is derivable from other fields, but better safe than sorry.
- && this.execRoot.equals(that.execRoot);
+ && this.productName.equals(that.productName);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/PathExistenceCache.java b/src/main/java/com/google/devtools/build/lib/includescanning/PathExistenceCache.java
index 6b308f3..7ce71f1 100644
--- a/src/main/java/com/google/devtools/build/lib/includescanning/PathExistenceCache.java
+++ b/src/main/java/com/google/devtools/build/lib/includescanning/PathExistenceCache.java
@@ -50,7 +50,7 @@
k -> {
Path path =
isSource
- ? artifactFactory.getPathFromSourceExecPath(execPath)
+ ? artifactFactory.getPathFromSourceExecPath(execRoot, execPath)
: execRoot.getRelative(execPath);
return path.isFile();
});
@@ -68,7 +68,7 @@
directoryExistenceCache.computeIfAbsent(
execPath,
k -> {
- Path path = artifactFactory.getPathFromSourceExecPath(execPath);
+ Path path = artifactFactory.getPathFromSourceExecPath(execRoot, execPath);
return path.isDirectory();
});
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java
index 0bfdeb4..8eee474 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java
@@ -209,33 +209,33 @@
}
/**
- * Returns the immutable list of include directories to be added with "-I"
- * (possibly empty but never null). This includes the include dirs from the
- * transitive deps closure of the target. This list does not contain
- * duplicates. All fragments are either absolute or relative to the exec root
- * (see {@link com.google.devtools.build.lib.analysis.BlazeDirectories#getExecRoot}).
+ * Returns the immutable list of include directories to be added with "-I" (possibly empty but
+ * never null). This includes the include dirs from the transitive deps closure of the target.
+ * This list does not contain duplicates. All fragments are either absolute or relative to the
+ * exec root (see {@link
+ * com.google.devtools.build.lib.analysis.BlazeDirectories#getExecRoot(String)}).
*/
public ImmutableList<PathFragment> getIncludeDirs() {
return commandLineCcCompilationContext.includeDirs;
}
/**
- * Returns the immutable list of include directories to be added with
- * "-iquote" (possibly empty but never null). This includes the include dirs
- * from the transitive deps closure of the target. This list does not contain
- * duplicates. All fragments are either absolute or relative to the exec root
- * (see {@link com.google.devtools.build.lib.analysis.BlazeDirectories#getExecRoot}).
+ * Returns the immutable list of include directories to be added with "-iquote" (possibly empty
+ * but never null). This includes the include dirs from the transitive deps closure of the target.
+ * This list does not contain duplicates. All fragments are either absolute or relative to the
+ * exec root (see {@link
+ * com.google.devtools.build.lib.analysis.BlazeDirectories#getExecRoot(String)}).
*/
public ImmutableList<PathFragment> getQuoteIncludeDirs() {
return commandLineCcCompilationContext.quoteIncludeDirs;
}
/**
- * Returns the immutable list of include directories to be added with
- * "-isystem" (possibly empty but never null). This includes the include dirs
- * from the transitive deps closure of the target. This list does not contain
- * duplicates. All fragments are either absolute or relative to the exec root
- * (see {@link com.google.devtools.build.lib.analysis.BlazeDirectories#getExecRoot}).
+ * Returns the immutable list of include directories to be added with "-isystem" (possibly empty
+ * but never null). This includes the include dirs from the transitive deps closure of the target.
+ * This list does not contain duplicates. All fragments are either absolute or relative to the
+ * exec root (see {@link
+ * com.google.devtools.build.lib.analysis.BlazeDirectories#getExecRoot(String)}).
*/
public ImmutableList<PathFragment> getSystemIncludeDirs() {
return commandLineCcCompilationContext.systemIncludeDirs;
@@ -619,10 +619,10 @@
}
/**
- * Add a single include directory to be added with "-I". It can be either
- * relative to the exec root (see
- * {@link com.google.devtools.build.lib.analysis.BlazeDirectories#getExecRoot}) or
- * absolute. Before it is stored, the include directory is normalized.
+ * Add a single include directory to be added with "-I". It can be either relative to the exec
+ * root (see {@link
+ * com.google.devtools.build.lib.analysis.BlazeDirectories#getExecRoot(String)}) or absolute.
+ * Before it is stored, the include directory is normalized.
*/
public Builder addIncludeDir(PathFragment includeDir) {
includeDirs.add(includeDir);
@@ -636,10 +636,10 @@
}
/**
- * Add a single include directory to be added with "-iquote". It can be
- * either relative to the exec root (see {@link
- * com.google.devtools.build.lib.analysis.BlazeDirectories#getExecRoot}) or absolute. Before it
- * is stored, the include directory is normalized.
+ * Add a single include directory to be added with "-iquote". It can be either relative to the
+ * exec root (see {@link
+ * com.google.devtools.build.lib.analysis.BlazeDirectories#getExecRoot(String)}) or absolute.
+ * Before it is stored, the include directory is normalized.
*/
public Builder addQuoteIncludeDir(PathFragment quoteIncludeDir) {
quoteIncludeDirs.add(quoteIncludeDir);
@@ -654,8 +654,9 @@
/**
* Add include directories to be added with "-isystem". It can be either relative to the exec
- * root (see {@link com.google.devtools.build.lib.analysis.BlazeDirectories#getExecRoot}) or
- * absolute. Before it is stored, the include directory is normalized.
+ * root (see {@link
+ * com.google.devtools.build.lib.analysis.BlazeDirectories#getExecRoot(String)}) or absolute.
+ * Before it is stored, the include directory is normalized.
*/
public Builder addSystemIncludeDirs(Iterable<PathFragment> systemIncludeDirs) {
Iterables.addAll(this.systemIncludeDirs, systemIncludeDirs);
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 7a5f770..af3414e 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
@@ -90,7 +90,7 @@
writeOutputBaseReadmeFile();
writeDoNotBuildHereFile();
}
- setupExecRoot();
+
// Here we use outputBase instead of outputPath because we need a file system to create the
// latter.
this.outputBaseFilesystemTypeName = FileSystemUtils.getFileSystem(getOutputBase());
@@ -147,13 +147,6 @@
return outputBaseFilesystemTypeName;
}
- /**
- * Returns the output path associated with this Blaze server process..
- */
- public Path getOutputPath() {
- return directories.getOutputPath();
- }
-
public Path getInstallBase() {
return directories.getInstallBase();
}
@@ -309,18 +302,6 @@
getOutputBase().getRelative("execroot").getRelative(DO_NOT_BUILD_FILE_NAME));
}
- /**
- * Creates the execRoot dir under outputBase.
- */
- private void setupExecRoot() {
- try {
- FileSystemUtils.createDirectoryAndParents(directories.getExecRoot());
- } catch (IOException e) {
- logger.warning(
- "failed to create execution root '" + directories.getExecRoot() + "': " + e.getMessage());
- }
- }
-
@Nullable
public AllocationTracker getAllocationTracker() {
return allocationTracker;
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/mobileinstall/MobileInstallCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/mobileinstall/MobileInstallCommand.java
index 7c72f55..732aff6 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/mobileinstall/MobileInstallCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/mobileinstall/MobileInstallCommand.java
@@ -277,7 +277,8 @@
}
}
- Path workingDir = env.getBlazeWorkspace().getOutputPath().getParentDirectory();
+ Path workingDir =
+ env.getDirectories().getOutputPath(env.getWorkspaceName()).getParentDirectory();
com.google.devtools.build.lib.shell.Command command =
new CommandBuilder()
.addArgs(cmdLine)
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
index 2d5ab17..bde506d 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
@@ -343,6 +343,12 @@
@Override
public SkyValue compute(SkyKey skyKey, Environment env)
throws CompletionFunctionException, InterruptedException {
+ WorkspaceNameValue workspaceNameValue =
+ (WorkspaceNameValue) env.getValue(WorkspaceNameValue.key());
+ if (workspaceNameValue == null) {
+ return null;
+ }
+
TValue value = completor.getValueFromSkyKey(skyKey, env);
TopLevelArtifactContext topLevelContext = completor.getTopLevelArtifactContext(skyKey);
if (env.valuesMissing()) {
@@ -430,7 +436,11 @@
CompletionContext ctx =
CompletionContext.create(
- expandedArtifacts, expandedFilesets, inputMap, pathResolverFactory);
+ expandedArtifacts,
+ expandedFilesets,
+ inputMap,
+ pathResolverFactory,
+ workspaceNameValue.getName());
ExtendedEventHandler.Postable postable =
completor.createSucceeded(skyKey, value, ctx, topLevelContext, env);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java
index 7a5760b..d1315db 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java
@@ -26,6 +26,8 @@
import com.google.devtools.build.lib.skyframe.RecursiveFilesystemTraversalFunction.DanglingSymlinkException;
import com.google.devtools.build.lib.skyframe.RecursiveFilesystemTraversalFunction.RecursiveFilesystemTraversalException;
import com.google.devtools.build.lib.skyframe.RecursiveFilesystemTraversalValue.ResolvedFile;
+import com.google.devtools.build.lib.skyframe.RecursiveFilesystemTraversalValue.TraversalRequest;
+import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionException;
@@ -47,15 +49,21 @@
}
}
- private final PathFragment execRoot;
+ private final Function<String, Path> getExecRoot;
- public FilesetEntryFunction(PathFragment execRoot) {
- this.execRoot = execRoot;
+ public FilesetEntryFunction(Function<String, Path> getExecRoot) {
+ this.getExecRoot = getExecRoot;
}
@Override
public SkyValue compute(SkyKey key, Environment env)
throws FilesetEntryFunctionException, InterruptedException {
+ WorkspaceNameValue workspaceNameValue =
+ (WorkspaceNameValue) env.getValue(WorkspaceNameValue.key());
+ if (workspaceNameValue == null) {
+ return null;
+ }
+
FilesetTraversalParams t = (FilesetTraversalParams) key.argument();
Preconditions.checkState(
t.getDirectTraversal().isPresent() && t.getNestedArtifact() == null,
@@ -179,7 +187,8 @@
f.getMetadata(),
t.getDestPath(),
direct.isGenerated(),
- outputSymlinks);
+ outputSymlinks,
+ getExecRoot.apply(workspaceNameValue.getName()));
}
return FilesetEntryValue.of(ImmutableSet.copyOf(outputSymlinks.values()));
@@ -192,12 +201,14 @@
Object metadata,
PathFragment destPath,
boolean isGenerated,
- Map<PathFragment, FilesetOutputSymlink> result) {
+ Map<PathFragment, FilesetOutputSymlink> result,
+ Path execRoot) {
linkName = destPath.getRelative(linkName);
if (!result.containsKey(linkName)) {
result.put(
linkName,
- FilesetOutputSymlink.create(linkName, linkTarget, metadata, isGenerated, execRoot));
+ FilesetOutputSymlink.create(
+ linkName, linkTarget, metadata, isGenerated, execRoot.asFragment()));
}
}
@@ -207,12 +218,11 @@
}
/**
- * Returns the {@link RecursiveFilesystemTraversalValue.TraversalRequest} node used to compute the
- * Skyframe value for {@code filesetEntryKey}. Should only be called to determine which nodes need
- * to be rewound, and only when {@code filesetEntryKey.isGenerated()}.
+ * Returns the {@link TraversalRequest} node used to compute the Skyframe value for {@code
+ * filesetEntryKey}. Should only be called to determine which nodes need to be rewound, and only
+ * when {@code filesetEntryKey.isGenerated()}.
*/
- public static RecursiveFilesystemTraversalValue.TraversalRequest getDependencyForRewinding(
- FilesetEntryKey filesetEntryKey) {
+ public static TraversalRequest getDependencyForRewinding(FilesetEntryKey filesetEntryKey) {
FilesetTraversalParams t = filesetEntryKey.argument();
Preconditions.checkState(
t.getDirectTraversal().isPresent() && t.getNestedArtifact() == null,
@@ -227,9 +237,9 @@
return createTraversalRequestKey(createErrorInfo(t), t.getDirectTraversal().get());
}
- private static RecursiveFilesystemTraversalValue.TraversalRequest createTraversalRequestKey(
+ private static TraversalRequest createTraversalRequestKey(
String errorInfo, DirectTraversal traversal) {
- return RecursiveFilesystemTraversalValue.TraversalRequest.create(
+ return TraversalRequest.create(
traversal.getRoot(),
traversal.isGenerated(),
traversal.getPackageBoundaryMode(),
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
index bc740eb..8d86812 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
@@ -155,7 +155,9 @@
this.skyframeActionExecutor = skyframeActionExecutor;
this.factory = new ConfiguredTargetFactory(ruleClassProvider);
this.artifactFactory =
- new ArtifactFactory(directories.getExecRoot(), directories.getRelativeOutputPath());
+ new ArtifactFactory(
+ /* execRootParent= */ directories.getExecRootBase(),
+ directories.getRelativeOutputPath());
this.skyframeExecutor = skyframeExecutor;
this.ruleClassProvider = ruleClassProvider;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index 1d11f8f..6da1f32 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -364,10 +364,11 @@
public ArtifactPathResolver createPathResolverForArtifactValues(
ActionInputMap actionInputMap,
Map<Artifact, Collection<Artifact>> expandedArtifacts,
- Iterable<Artifact> filesets) {
+ Iterable<Artifact> filesets,
+ String workspaceName) {
Preconditions.checkState(shouldCreatePathResolverForArtifactValues());
return outputService.createPathResolverForArtifactValues(
- directories.getExecRoot().asFragment(),
+ directories.getExecRoot(workspaceName).asFragment(),
directories.getRelativeOutputPath(),
fileSystem,
getPathEntries(),
@@ -601,9 +602,7 @@
this.actionExecutionFunction = actionExecutionFunction;
map.put(
SkyFunctions.RECURSIVE_FILESYSTEM_TRAVERSAL, new RecursiveFilesystemTraversalFunction());
- map.put(
- SkyFunctions.FILESET_ENTRY,
- new FilesetEntryFunction(directories.getExecRoot().asFragment()));
+ map.put(SkyFunctions.FILESET_ENTRY, new FilesetEntryFunction(directories::getExecRoot));
map.put(
SkyFunctions.ACTION_TEMPLATE_EXPANSION,
new ActionTemplateExpansionFunction(actionKeyContext));
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils/SerializationDepsUtils.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils/SerializationDepsUtils.java
index f80b78d..8f50010 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils/SerializationDepsUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils/SerializationDepsUtils.java
@@ -68,7 +68,7 @@
}
@Override
- public Path getPathFromSourceExecPath(PathFragment execPath) {
+ public Path getPathFromSourceExecPath(Path execRoot, PathFragment execPath) {
throw new UnsupportedOperationException();
}
};
diff --git a/src/test/java/com/google/devtools/build/lib/actions/ArtifactFactoryTest.java b/src/test/java/com/google/devtools/build/lib/actions/ArtifactFactoryTest.java
index 5c0532b..a9af0f4 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/ArtifactFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/ArtifactFactoryTest.java
@@ -89,7 +89,7 @@
alienPackage = PackageIdentifier.create("@alien", alienPath);
alienRelative = alienPath.getRelative("alien.txt");
- artifactFactory = new ArtifactFactory(execRoot, "bazel-out");
+ artifactFactory = new ArtifactFactory(execRoot.getParentDirectory(), "bazel-out");
setupRoots();
}
diff --git a/src/test/java/com/google/devtools/build/lib/actions/ArtifactTest.java b/src/test/java/com/google/devtools/build/lib/actions/ArtifactTest.java
index 6e05f61..ef9cd43b 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/ArtifactTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/ArtifactTest.java
@@ -353,7 +353,8 @@
public void testCodecRecyclesSourceArtifactInstances() throws Exception {
Root root = Root.fromPath(scratch.dir("/"));
ArtifactRoot artifactRoot = ArtifactRoot.asSourceRoot(root);
- ArtifactFactory artifactFactory = new ArtifactFactory(execDir, "blaze-out");
+ ArtifactFactory artifactFactory =
+ new ArtifactFactory(execDir.getParentDirectory(), "blaze-out");
artifactFactory.setSourceArtifactRoots(ImmutableMap.of(root, artifactRoot));
ArtifactResolverSupplier artifactResolverSupplierForTest = () -> artifactFactory;
diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
index 347badc..a5451f8 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
@@ -754,7 +754,7 @@
}
@Override
- public Path getPathFromSourceExecPath(PathFragment execPath) {
+ public Path getPathFromSourceExecPath(Path execRoot, PathFragment execPath) {
throw new UnsupportedOperationException();
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BlazeDirectoriesTest.java b/src/test/java/com/google/devtools/build/lib/analysis/BlazeDirectoriesTest.java
index ecb0883..adbd086 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/BlazeDirectoriesTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/BlazeDirectoriesTest.java
@@ -13,13 +13,14 @@
// limitations under the License.
package com.google.devtools.build.lib.analysis;
+
import static com.google.common.truth.Truth.assertThat;
import com.google.devtools.build.lib.skyframe.serialization.testutils.FsUtils;
import com.google.devtools.build.lib.skyframe.serialization.testutils.SerializationTester;
import com.google.devtools.build.lib.testutil.FoundationTestCase;
+import com.google.devtools.build.lib.testutil.TestConstants;
import com.google.devtools.build.lib.vfs.FileSystem;
-import com.google.devtools.build.lib.vfs.Path;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -29,42 +30,6 @@
public class BlazeDirectoriesTest extends FoundationTestCase {
@Test
- public void testCreatingDirectories() {
- FileSystem fs = scratch.getFileSystem();
- Path installBase = fs.getPath("/my/install");
- Path outputBase = fs.getPath("/my/output");
- Path userRoot = fs.getPath("/home/user/root");
- Path workspace = fs.getPath("/my/ws");
- BlazeDirectories directories =
- new BlazeDirectories(
- new ServerDirectories(installBase, outputBase, userRoot),
- workspace,
- /* defaultSystemJavabase= */ null,
- "foo");
- assertThat(outputBase.getRelative("execroot/ws")).isEqualTo(directories.getExecRoot());
-
- workspace = null;
- directories =
- new BlazeDirectories(
- new ServerDirectories(installBase, outputBase, userRoot),
- workspace,
- /* defaultSystemJavabase= */ null,
- "foo");
- assertThat(outputBase.getRelative("execroot/" + BlazeDirectories.DEFAULT_EXEC_ROOT))
- .isEqualTo(directories.getExecRoot());
-
- workspace = fs.getPath("/");
- directories =
- new BlazeDirectories(
- new ServerDirectories(installBase, outputBase, userRoot),
- workspace,
- /* defaultSystemJavabase= */ null,
- "foo");
- assertThat(outputBase.getRelative("execroot/" + BlazeDirectories.DEFAULT_EXEC_ROOT))
- .isEqualTo(directories.getExecRoot());
- }
-
- @Test
public void testCodec() throws Exception {
new SerializationTester(
new BlazeDirectories(
@@ -74,7 +39,7 @@
FsUtils.TEST_FILESYSTEM.getPath("/user_root")),
FsUtils.TEST_FILESYSTEM.getPath("/workspace"),
/* defaultSystemJavabase= */ null,
- "Blaze"),
+ TestConstants.PRODUCT_NAME),
new BlazeDirectories(
new ServerDirectories(
FsUtils.TEST_FILESYSTEM.getPath("/install_base"),
@@ -84,7 +49,7 @@
"1234abcd1234abcd1234abcd1234abcd"),
FsUtils.TEST_FILESYSTEM.getPath("/workspace"),
/* defaultSystemJavabase= */ null,
- "Blaze"),
+ TestConstants.PRODUCT_NAME),
new BlazeDirectories(
new ServerDirectories(
FsUtils.TEST_FILESYSTEM.getPath("/install_base"),
@@ -92,8 +57,23 @@
FsUtils.TEST_FILESYSTEM.getPath("/user_root")),
FsUtils.TEST_FILESYSTEM.getPath("/workspace"),
/* defaultSystemJavabase= */ null,
- "Bazel"))
+ TestConstants.PRODUCT_NAME))
.addDependency(FileSystem.class, FsUtils.TEST_FILESYSTEM)
.runTests();
}
+
+ @Test
+ public void testBlazeExecIsNullInBazel() {
+ BlazeDirectories directories =
+ new BlazeDirectories(
+ new ServerDirectories(
+ FsUtils.TEST_FILESYSTEM.getPath("/install_base"),
+ FsUtils.TEST_FILESYSTEM.getPath("/output_base"),
+ FsUtils.TEST_FILESYSTEM.getPath("/user_root")),
+ FsUtils.TEST_FILESYSTEM.getPath("/workspace"),
+ /* defaultSystemJavabase= */ null,
+ /* productName = */ "bazel");
+ assertThat(directories.getBlazeExecRoot()).isNull();
+ assertThat(directories.getBlazeOutputPath()).isNull();
+ }
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index aa7d269..15d0010 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -233,6 +233,7 @@
rootDirectory,
/* defaultSystemJavabase= */ null,
analysisMock.getProductName());
+
actionKeyContext = new ActionKeyContext();
mockToolsConfig = new MockToolsConfig(rootDirectory, false);
mockToolsConfig.create("/bazel_tools_workspace/WORKSPACE", "workspace(name = 'bazel_tools')");
@@ -256,7 +257,7 @@
workspaceStatusActionFactory = new AnalysisTestUtil.DummyWorkspaceStatusActionFactory();
mutableActionGraph = new MapBasedActionGraph(actionKeyContext);
ruleClassProvider = getRuleClassProvider();
-
+ getOutputPath().createDirectoryAndParents();
ImmutableList<PrecomputedValue.Injected> extraPrecomputedValues =
ImmutableList.of(
PrecomputedValue.injected(PrecomputedValue.REPO_ENV, ImmutableMap.<String, String>of()),
@@ -1502,7 +1503,7 @@
}
protected Path getOutputPath() {
- return directories.getOutputPath();
+ return directories.getOutputPath(ruleClassProvider.getRunfilesPrefix());
}
/**
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/HeaderThinningTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/HeaderThinningTest.java
index 8004914..8436ac9 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/HeaderThinningTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/HeaderThinningTest.java
@@ -212,7 +212,8 @@
getDerivedArtifact(
PathFragment.create(name),
ArtifactRoot.asDerivedRoot(
- directories.getExecRoot(), directories.getExecRoot().getChild("out")),
+ directories.getExecRoot("workspace"),
+ directories.getExecRoot("workspace").getChild("out")),
ActionsTestUtil.NULL_ARTIFACT_OWNER);
return new SpecialArtifact(
treeArtifactBase.getRoot(),
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java
index 166403d..c45f999 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java
@@ -59,6 +59,7 @@
import com.google.devtools.build.skyframe.SequencedRecordingDifferencer;
import com.google.devtools.build.skyframe.SequentialBuildDriver;
import com.google.devtools.build.skyframe.SkyFunction;
+import com.google.devtools.build.skyframe.SkyFunctionException;
import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
@@ -129,7 +130,8 @@
/*hardcodedBlacklistedPackagePrefixes=*/ ImmutableSet.of(),
/*additionalBlacklistedPackagePrefixesFile=*/ PathFragment.EMPTY_FRAGMENT));
skyFunctions.put(
- SkyFunctions.FILESET_ENTRY, new FilesetEntryFunction(rootDirectory.asFragment()));
+ SkyFunctions.FILESET_ENTRY, new FilesetEntryFunction((unused) -> rootDirectory));
+ skyFunctions.put(SkyFunctions.WORKSPACE_NAME, new TestWorkspaceNameFunction());
skyFunctions.put(SkyFunctions.LOCAL_REPOSITORY_LOOKUP, new LocalRepositoryLookupFunction());
differencer = new SequencedRecordingDifferencer();
@@ -959,4 +961,20 @@
}
}.doTest();
}
+
+ private static class TestWorkspaceNameFunction implements SkyFunction {
+
+ @Nullable
+ @Override
+ public SkyValue compute(SkyKey skyKey, Environment env)
+ throws SkyFunctionException, InterruptedException {
+ return WorkspaceNameValue.withName("workspace");
+ }
+
+ @Nullable
+ @Override
+ public String extractTag(SkyKey skyKey) {
+ return null;
+ }
+ }
}
