Skip to content

Commit dd3991d

Browse files
justinhorvitzcopybara-github
authored andcommitted
Yank out the virtual exec root.
After this, only the virtual source root remains. Most `devirtualize()` calls can probably be removed at this point, but to be safe, I'll leave them until the virtual source root is also removed, so I don't have to distinguish between which calls are on output paths vs source paths. PiperOrigin-RevId: 759670248 Change-Id: Ic8ee500e2b50fa5da0fcbbe0d080cdf4bd069060
1 parent eea0882 commit dd3991d

File tree

7 files changed

+14
-42
lines changed

7 files changed

+14
-42
lines changed

src/main/java/com/google/devtools/build/lib/analysis/BlazeDirectories.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,13 +163,13 @@ public Path getOutputBase() {
163163
return serverDirectories.getOutputBase();
164164
}
165165

166-
/** Returns the effective execution root, which may be virtualized. */
166+
/** Returns the execution root base path with no workspace name fragment. */
167167
public Path getExecRootBase() {
168168
return serverDirectories.getExecRootBase();
169169
}
170170

171171
/**
172-
* Returns the local execution root of Google-internal Blaze. Virtualization is not respected.
172+
* Returns the local execution root of Google-internal Blaze.
173173
*
174174
* <p>This method throws {@link NullPointerException} in Bazel. Use {@link #getExecRoot} instead.
175175
*/
@@ -187,7 +187,7 @@ public Path getExecRoot(String workspaceName) {
187187
}
188188

189189
/**
190-
* Returns the local output path of Google-internal Blaze. Virtualization is not respected.
190+
* Returns the local output path of Google-internal Blaze.
191191
*
192192
* <p>This method throws {@link NullPointerException} in Bazel. Use {@link #getOutputPath}
193193
* instead.

src/main/java/com/google/devtools/build/lib/analysis/ServerDirectories.java

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -121,11 +121,7 @@ public Path getOutputUserRoot() {
121121
* Parent of all execution roots.
122122
*
123123
* <p>By default, this is a folder called {@linkplain #EXECROOT execroot} in {@link
124-
* #getOutputBase}. However, some {@link com.google.devtools.build.lib.vfs.FileSystem}
125-
* implementations may choose to virtualize the execroot (in other words, it is not a real on-disk
126-
* path, but one that the {@link com.google.devtools.build.lib.vfs.FileSystem} recognizes).
127-
*
128-
* <p>This is virtual if and only if {@link #getVirtualSourceRoot} is present.
124+
* #getOutputBase}.
129125
*/
130126
public Path getExecRootBase() {
131127
return execRootBase;
@@ -138,8 +134,6 @@ public Path getExecRootBase() {
138134
* <p>If present, the server's {@link com.google.devtools.build.lib.vfs.FileSystem} is responsible
139135
* for translating paths under this root to the actual requested {@code --package_path} for a
140136
* given command.
141-
*
142-
* <p>Present if and only if {@link #getExecRootBase} is virtualized.
143137
*/
144138
@Nullable
145139
public Root getVirtualSourceRoot() {

src/main/java/com/google/devtools/build/lib/bazel/BazelFileSystemModule.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import com.google.devtools.build.lib.vfs.DigestHashFunction.DigestFunctionConverter;
3131
import com.google.devtools.build.lib.vfs.FileSystem;
3232
import com.google.devtools.build.lib.vfs.JavaIoFileSystem;
33-
import com.google.devtools.build.lib.vfs.PathFragment;
3433
import com.google.devtools.build.lib.vfs.bazel.BazelHashFunctions;
3534
import com.google.devtools.build.lib.windows.WindowsFileSystem;
3635
import com.google.devtools.common.options.OptionsParsingException;
@@ -50,8 +49,7 @@ public class BazelFileSystemModule extends BlazeModule {
5049
}
5150

5251
@Override
53-
public ModuleFileSystem getFileSystem(
54-
OptionsParsingResult startupOptions, PathFragment realExecRootBase)
52+
public ModuleFileSystem getFileSystem(OptionsParsingResult startupOptions)
5553
throws AbruptExitException {
5654
BlazeServerStartupOptions options =
5755
checkNotNull(startupOptions.getOptions(BlazeServerStartupOptions.class));

src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,9 @@ public void globalInit(OptionsParsingResult startupOptions) throws AbruptExitExc
9292
* and {@link #blazeStartup}).
9393
*
9494
* @param startupOptions the server's startup options
95-
* @param realExecRootBase absolute path fragment of the actual, underlying execution root
9695
*/
9796
@Nullable
98-
public ModuleFileSystem getFileSystem(
99-
OptionsParsingResult startupOptions, PathFragment realExecRootBase)
97+
public ModuleFileSystem getFileSystem(OptionsParsingResult startupOptions)
10098
throws AbruptExitException {
10199
return null;
102100
}
@@ -130,25 +128,15 @@ public abstract static class ModuleFileSystem {
130128
*/
131129
abstract Optional<Root> virtualSourceRoot();
132130

133-
/**
134-
* Present if this filesystem virtualizes the execroot folder. See {@link
135-
* ServerDirectories#getExecRootBase}.
136-
*/
137-
abstract Optional<Path> virtualExecRootBase();
138-
139131
public static ModuleFileSystem createWithVirtualization(
140-
FileSystem fileSystem, PathFragment virtualSourceRoot, PathFragment virtualExecRootBase) {
132+
FileSystem fileSystem, PathFragment virtualSourceRoot) {
141133
return new AutoValue_BlazeModule_ModuleFileSystem(
142-
fileSystem,
143-
Optional.of(Root.fromPath(fileSystem.getPath(virtualSourceRoot))),
144-
Optional.of(fileSystem.getPath(virtualExecRootBase)));
134+
fileSystem, Optional.of(Root.fromPath(fileSystem.getPath(virtualSourceRoot))));
145135
}
146136

147137
public static ModuleFileSystem create(FileSystem fileSystem) {
148138
return new AutoValue_BlazeModule_ModuleFileSystem(
149-
fileSystem,
150-
/* virtualSourceRoot= */ Optional.empty(),
151-
/* virtualExecRootBase= */ Optional.empty());
139+
fileSystem, /* virtualSourceRoot= */ Optional.empty());
152140
}
153141
}
154142

src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1290,7 +1290,7 @@ private static BlazeRuntime newRuntime(
12901290
PathFragment outputUserRoot = startupOptions.outputUserRoot;
12911291
PathFragment installBase = startupOptions.installBase;
12921292
PathFragment outputBase = startupOptions.outputBase;
1293-
PathFragment realExecRootBase = outputBase.getRelative(ServerDirectories.EXECROOT);
1293+
PathFragment execRootBase = outputBase.getRelative(ServerDirectories.EXECROOT);
12941294

12951295
// Force JNI linking before the first real use of JNI to emit a helpful error message now that
12961296
// we have the install base path handy.
@@ -1314,14 +1314,12 @@ private static BlazeRuntime newRuntime(
13141314

13151315
FileSystem nativeFs = null;
13161316
Optional<Root> virtualSourceRoot = Optional.empty();
1317-
Optional<Path> virtualExecRootBase = Optional.empty();
13181317
for (BlazeModule module : blazeModules) {
1319-
ModuleFileSystem moduleFs = module.getFileSystem(options, realExecRootBase);
1318+
ModuleFileSystem moduleFs = module.getFileSystem(options);
13201319
if (moduleFs != null) {
13211320
Preconditions.checkState(nativeFs == null, "more than one module returns a file system");
13221321
nativeFs = moduleFs.fileSystem();
13231322
virtualSourceRoot = moduleFs.virtualSourceRoot();
1324-
virtualExecRootBase = moduleFs.virtualExecRootBase();
13251323
}
13261324
}
13271325

@@ -1415,7 +1413,7 @@ public void uncaughtException(Thread thread, Throwable throwable) {
14151413
Path outputUserRootPath = fs.getPath(outputUserRoot);
14161414
Path installBasePath = fs.getPath(installBase);
14171415
Path outputBasePath = fs.getPath(outputBase);
1418-
Path execRootBasePath = virtualExecRootBase.orElseGet(() -> fs.getPath(realExecRootBase));
1416+
Path execRootBasePath = fs.getPath(execRootBase);
14191417
Path workspaceDirectoryPath = null;
14201418
if (!workspaceDirectory.equals(PathFragment.EMPTY_FRAGMENT)) {
14211419
workspaceDirectoryPath = nativeFs.getPath(workspaceDirectory);

src/test/java/com/google/devtools/build/lib/buildtool/util/BuildIntegrationTestCase.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ public final void createFilesAndMocks() throws Exception {
254254
/* installBase= */ outputBase,
255255
/* outputBase= */ outputBase,
256256
/* outputUserRoot= */ outputBase,
257-
/* execRootBase= */ getExecRootBase(),
257+
/* execRootBase= */ outputBase.getRelative(ServerDirectories.EXECROOT),
258258
/* virtualSourceRoot= */ getVirtualSourceRoot(),
259259
// Arbitrary install base hash.
260260
/* installMD5= */ "83bc4458738962b9b77480bac76164a9");
@@ -332,10 +332,6 @@ protected Root getVirtualSourceRoot() {
332332
return null;
333333
}
334334

335-
protected Path getExecRootBase() {
336-
return outputBase.getRelative(ServerDirectories.EXECROOT);
337-
}
338-
339335
protected void createRuntimeWrapper() throws Exception {
340336
if (runtimeWrapper != null) {
341337
cleanupInterningPools();

src/test/java/com/google/devtools/build/lib/skyframe/rewinding/RewindingTest.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,7 @@ public void onTargetAnalyzed(TargetConfiguredEvent event) throws IOException {
102102
// Necessary due to the RepositoryHelpersHolder nulling above, simulates the effect of
103103
// TopLevelTargetReadyForSymlinkPlanting.
104104
FileSystemUtils.ensureSymbolicLink(
105-
getExecRootBase()
106-
.getChild(TestConstants.WORKSPACE_NAME)
107-
.getRelative("external/bazel_tools"),
105+
directories.getExecRoot(TestConstants.WORKSPACE_NAME).getRelative("external/bazel_tools"),
108106
getOutputBase().getRelative("external/bazel_tools"));
109107
}
110108
}

0 commit comments

Comments
 (0)