diff --git a/src/main/java/com/google/devtools/build/lib/vfs/FileSystemUtils.java b/src/main/java/com/google/devtools/build/lib/vfs/FileSystemUtils.java index ec30e22d309b07..60158401881ff6 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/FileSystemUtils.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/FileSystemUtils.java @@ -456,6 +456,17 @@ public static MoveResult moveFile(Path from, Path to) throws IOException { try (InputStream in = from.getInputStream(); OutputStream out = to.getOutputStream()) { copyLargeBuffer(in, out); + } catch (FileAccessException e1) { + // Rules can accidentally make output non-readable, let's fix that (b/150963503) + if (!from.isReadable()) { + from.setReadable(true); + try (InputStream in = from.getInputStream(); + OutputStream out = to.getOutputStream()) { + copyLargeBuffer(in, out); + } + } else { + throw e1; + } } to.setLastModifiedTime(stat.getLastModifiedTime()); // Preserve mtime. if (!from.isWritable()) { diff --git a/src/test/java/com/google/devtools/build/lib/vfs/FileSystemUtilsTest.java b/src/test/java/com/google/devtools/build/lib/vfs/FileSystemUtilsTest.java index 88e9a7de077fe7..c934a9e152a943 100644 --- a/src/test/java/com/google/devtools/build/lib/vfs/FileSystemUtilsTest.java +++ b/src/test/java/com/google/devtools/build/lib/vfs/FileSystemUtilsTest.java @@ -364,26 +364,11 @@ public void testMoveFile() throws IOException { @Test public void testMoveFileAcrossDevices() throws Exception { - class MultipleDeviceFS extends InMemoryFileSystem { - MultipleDeviceFS() { - super(DigestHashFunction.SHA256); - } - - @Override - public void renameTo(Path source, Path target) throws IOException { - if (!source.startsWith(target.asFragment().subFragment(0, 1))) { - throw new IOException("EXDEV"); - } - super.renameTo(source, target); - } - } FileSystem fs = new MultipleDeviceFS(); - Path dev1 = fs.getPath("/fs1"); - dev1.createDirectory(); - Path dev2 = fs.getPath("/fs2"); - dev2.createDirectory(); - Path source = dev1.getChild("source"); - Path target = dev2.getChild("target"); + Path source = fs.getPath("/fs1/source"); + source.getParentDirectory().createDirectoryAndParents(); + Path target = fs.getPath("/fs2/target"); + target.getParentDirectory().createDirectoryAndParents(); FileSystemUtils.writeContent(source, UTF_8, "hello, world"); source.setLastModifiedTime(142); @@ -394,12 +379,34 @@ public void renameTo(Path source, Path target) throws IOException { assertThat(target.getLastModifiedTime()).isEqualTo(142); source.createSymbolicLink(PathFragment.create("link-target")); + assertThat(FileSystemUtils.moveFile(source, target)).isEqualTo(MoveResult.FILE_COPIED); + assertThat(source.exists(Symlinks.NOFOLLOW)).isFalse(); assertThat(target.isSymbolicLink()).isTrue(); assertThat(target.readSymbolicLink()).isEqualTo(PathFragment.create("link-target")); } + @Test + public void testMoveFileFixPermissions() throws Exception { + FileSystem fs = new MultipleDeviceFS(); + Path source = fs.getPath("/fs1/source"); + source.getParentDirectory().createDirectoryAndParents(); + Path target = fs.getPath("/fs2/target"); + target.getParentDirectory().createDirectoryAndParents(); + + FileSystemUtils.writeContent(source, UTF_8, "linear-a"); + source.setLastModifiedTime(142); + source.setReadable(false); + + MoveResult moveResult = moveFile(source, target); + + assertThat(moveResult).isEqualTo(MoveResult.FILE_COPIED); + assertThat(source.exists(Symlinks.NOFOLLOW)).isFalse(); + assertThat(target.isFile(Symlinks.NOFOLLOW)).isTrue(); + assertThat(FileSystemUtils.readContent(target, UTF_8)).isEqualTo("linear-a"); + } + @Test public void testReadContentWithLimit() throws IOException { createTestDirectoryTree(); @@ -834,4 +841,18 @@ public void testCreateHardLinkForNonEmptyDirectory_success() throws Exception { assertThat(fileSystem.stat(linkPath3, false).getNodeId()) .isEqualTo(fileSystem.stat(originalPath3, false).getNodeId()); } + + static class MultipleDeviceFS extends InMemoryFileSystem { + MultipleDeviceFS() { + super(DigestHashFunction.SHA256); + } + + @Override + public void renameTo(Path source, Path target) throws IOException { + if (!source.startsWith(target.asFragment().subFragment(0, 1))) { + throw new IOException("EXDEV"); + } + super.renameTo(source, target); + } + } }