Fix `RemoteExternalOverlayFileSystem#resolveSymbolicLinks`
Ensures that the returned `Path` is still in the overlay file system.
Also make the error message emitted by `Path#checkSameFileSystem` more informative. This is motivated by and helped discover the above as the fix for the following crash observed when using the remote repo contents cache with an explicit `--sandbox_base`:
```
Caused by: java.lang.IllegalArgumentException: Files are on different filesystems: /dev/shm/bazel-sandbox.b10976335efa519b0184f3091ac8e21f7beefb92142303f9ab2c3341f45a2f28/linux-sandbox/18/execroot/_main/external/c-ares+/configs/ares_build.h (on com.google.devtools.build.lib.unix.UnixFileSystem@5e0a8154), /home/ubuntu/.cache/bazel/_bazel_ubuntu/123/execroot/_main/external/c-ares+/configs/ares_build.h (on com.google.devtools.build.lib.remote.RemoteExternalOverlayFileSystem@6cd9bfda)
at com.google.devtools.build.lib.vfs.Path.checkSameFileSystem(Path.java:964)
at com.google.devtools.build.lib.vfs.Path.createSymbolicLink(Path.java:523)
at com.google.devtools.build.lib.vfs.Path.createSymbolicLink(Path.java:535)
at com.google.devtools.build.lib.sandbox.SymlinkedSandboxedSpawn.copyFile(SymlinkedSandboxedSpawn.java:129)
```
Alternative to https://github.com/bazelbuild/bazel/pull/27721
Closes #27802.
PiperOrigin-RevId: 837832265
Change-Id: I3b73167496b011aef66954d59ca3804b4b64996f
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExternalOverlayFileSystem.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExternalOverlayFileSystem.java
index 99204b8..55bbc76 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteExternalOverlayFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteExternalOverlayFileSystem.java
@@ -542,7 +542,8 @@
@Override
public Path resolveSymbolicLinks(PathFragment path) throws IOException {
- return fsForPath(path).resolveSymbolicLinks(path);
+ // Ensure that the return value doesn't leave the overlay file system.
+ return getPath(fsForPath(path).resolveSymbolicLinks(path).asFragment());
}
@Nullable
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/Path.java b/src/main/java/com/google/devtools/build/lib/vfs/Path.java
index 03a078d..d6f8f8f 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/Path.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/Path.java
@@ -895,7 +895,8 @@
private void checkSameFileSystem(Path that) {
if (this.fileSystem != that.fileSystem) {
throw new IllegalArgumentException(
- "Files are on different filesystems: " + this + ", " + that);
+ "Files are on different filesystems: %s (on %s), %s (on %s)"
+ .formatted(this, this.fileSystem, that, that.fileSystem));
}
}
}
diff --git a/src/test/py/bazel/bzlmod/remote_repo_contents_cache_test.py b/src/test/py/bazel/bzlmod/remote_repo_contents_cache_test.py
index 12bf9e2..c901e9b 100644
--- a/src/test/py/bazel/bzlmod/remote_repo_contents_cache_test.py
+++ b/src/test/py/bazel/bzlmod/remote_repo_contents_cache_test.py
@@ -482,7 +482,11 @@
with open(self.Path('bazel-bin/main/out.txt')) as f:
self.assertEqual(f.read(), 'hello')
- def testUseRepoFileInBuildRule_actionDoesNotUseCache(self):
+ def do_testUseRepoFileInBuildRule_actionDoesNotUseCache(
+ self, extra_flags=None
+ ):
+ if extra_flags is None:
+ extra_flags = []
self.ScratchFile(
'MODULE.bazel',
[
@@ -518,7 +522,7 @@
repo_dir = self.RepoDir('my_repo')
# First fetch: not cached
- _, _, stderr = self.RunBazel(['build', '//main:use_data'])
+ _, _, stderr = self.RunBazel(['build', '//main:use_data'] + extra_flags)
self.assertIn('JUST FETCHED', '\n'.join(stderr))
self.assertTrue(os.path.exists(os.path.join(repo_dir, 'BUILD')))
self.assertTrue(os.path.exists(os.path.join(repo_dir, 'data.txt')))
@@ -528,7 +532,7 @@
# After expunging: repo and build action cached
self.RunBazel(['clean', '--expunge'])
- _, _, stderr = self.RunBazel(['build', '//main:use_data'])
+ _, _, stderr = self.RunBazel(['build', '//main:use_data'] + extra_flags)
self.assertNotIn('JUST FETCHED', '\n'.join(stderr))
self.assertFalse(os.path.exists(os.path.join(repo_dir, 'BUILD')))
self.assertTrue(os.path.exists(os.path.join(repo_dir, 'data.txt')))
@@ -536,6 +540,17 @@
with open(self.Path('bazel-bin/main/out.txt')) as f:
self.assertEqual(f.read(), 'hello')
+ def testUseRepoFileInBuildRule_actionDoesNotUseCache(self):
+ self.do_testUseRepoFileInBuildRule_actionDoesNotUseCache()
+
+ def testUseRepoFileInBuildRule_actionDoesNotUseCache_withExplicitSandboxBase(
+ self,
+ ):
+ tmpdir = self.ScratchDir('sandbox_base')
+ self.do_testUseRepoFileInBuildRule_actionDoesNotUseCache(
+ extra_flags=['--sandbox_base=' + tmpdir]
+ )
+
def testLostRemoteFile_build(self):
# Create a repo with two BUILD files (one in a subpackage), build a target
# from one to cause it to be cached, then build that target again after
