From e1cd9509862aef684b4dbbdfd15d0b877fb8fad3 Mon Sep 17 00:00:00 2001 From: Googler Date: Wed, 7 Sep 2016 14:33:29 +0000 Subject: Fixed the issue that hard links are handled improperly when bazel decompresses tarballs. Issue link: https://github.com/bazelbuild/bazel/issues/574 -- MOS_MIGRATED_REVID=132434278 --- .../devtools/build/lib/rules/repository/BUILD | 1 + .../repository/CompressedTarFunctionTest.java | 126 +++++++++++++++++++++ .../repository/test_decompress_archive.tar.gz | Bin 0 -> 216 bytes .../devtools/build/lib/vfs/FileSystemTest.java | 91 +++++++++++++-- .../build/lib/vfs/FileSystemUtilsTest.java | 74 +++++++++++- .../build/lib/vfs/JavaIoFileSystemTest.java | 20 +++- .../lib/vfs/ScopeEscapableFileSystemTest.java | 19 ++-- 7 files changed, 310 insertions(+), 21 deletions(-) create mode 100644 src/test/java/com/google/devtools/build/lib/rules/repository/CompressedTarFunctionTest.java create mode 100644 src/test/java/com/google/devtools/build/lib/rules/repository/test_decompress_archive.tar.gz (limited to 'src/test/java') diff --git a/src/test/java/com/google/devtools/build/lib/rules/repository/BUILD b/src/test/java/com/google/devtools/build/lib/rules/repository/BUILD index c5df5f3661..24be41f2f8 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/repository/BUILD +++ b/src/test/java/com/google/devtools/build/lib/rules/repository/BUILD @@ -7,6 +7,7 @@ filegroup( java_test( name = "RepositoryTests", srcs = glob(["*.java"]), + data = ["test_decompress_archive.tar.gz"], tags = ["rules"], test_class = "com.google.devtools.build.lib.AllTests", deps = [ diff --git a/src/test/java/com/google/devtools/build/lib/rules/repository/CompressedTarFunctionTest.java b/src/test/java/com/google/devtools/build/lib/rules/repository/CompressedTarFunctionTest.java new file mode 100644 index 0000000000..b638b21052 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/rules/repository/CompressedTarFunctionTest.java @@ -0,0 +1,126 @@ +// Copyright 2016 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.rules.repository; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.devtools.build.lib.bazel.repository.CompressedTarFunction; +import com.google.devtools.build.lib.bazel.repository.DecompressorDescriptor; +import com.google.devtools.build.lib.bazel.repository.TarGzFunction; +import com.google.devtools.build.lib.testutil.BlazeTestUtils; +import com.google.devtools.build.lib.testutil.TestConstants; +import com.google.devtools.build.lib.testutil.TestUtils; +import com.google.devtools.build.lib.util.OS; +import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.JavaIoFileSystem; +import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.UnixFileSystem; +import java.io.File; +import java.io.FileInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Files; +import java.util.zip.GZIPInputStream; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests decompressing archives. */ +@RunWith(JUnit4.class) +public class CompressedTarFunctionTest { + + /* Regular file */ + private static final String REGULAR_FILE_NAME = "regularFile"; + + /* Hard link file, created by ln */ + private static final String HARD_LINK_FILE_NAME = "hardLinkFile"; + + /* Symbolic(Soft) link file, created by ln -s */ + private static final String SYMBOLIC_LINK_FILE_NAME = "symbolicLinkFile"; + + private static final String PATH_TO_TEST_ARCHIVE = + "/com/google/devtools/build/lib/rules/repository/"; + + /* Tarball, created by + * tar -czf + */ + private static final String ARCHIVE_NAME = "test_decompress_archive.tar.gz"; + + private FileSystem testFS; + private Path workingDir; + private Path tarballPath; + private Path outDir; + private DecompressorDescriptor.Builder descriptorBuilder; + + @Before + public void setUpFs() throws Exception { + + testFS = OS.getCurrent() == OS.WINDOWS ? new JavaIoFileSystem() : new UnixFileSystem(); + + tarballPath = + testFS + .getPath(BlazeTestUtils.runfilesDir()) + .getRelative(TestConstants.JAVATESTS_ROOT + PATH_TO_TEST_ARCHIVE + ARCHIVE_NAME); + + workingDir = testFS.getPath(new File(TestUtils.tmpDir()).getCanonicalPath()); + outDir = workingDir.getRelative("out"); + + descriptorBuilder = + DecompressorDescriptor.builder() + .setDecompressor(TarGzFunction.INSTANCE) + .setRepositoryPath(outDir) + .setArchivePath(tarballPath); + } + + /** + * Test decompressing a tar.gz file with hard link file and symbolic link file inside + * + * @throws Exception + */ + @Test + public void testDecompress() throws Exception { + + Path outputDir = + new CompressedTarFunction() { + @Override + protected InputStream getDecompressorStream(DecompressorDescriptor descriptor) + throws IOException { + return new GZIPInputStream(new FileInputStream(descriptor.archivePath().getPathFile())); + } + }.decompress(descriptorBuilder.build()); + + assertThat(outputDir.exists()).isTrue(); + assertThat(outputDir.getRelative(REGULAR_FILE_NAME).exists()).isTrue(); + assertThat(outputDir.getRelative(REGULAR_FILE_NAME).getFileSize()).isNotEqualTo(0); + assertThat(outputDir.getRelative(REGULAR_FILE_NAME).isSymbolicLink()).isFalse(); + assertThat(outputDir.getRelative(HARD_LINK_FILE_NAME).exists()).isTrue(); + assertThat(outputDir.getRelative(HARD_LINK_FILE_NAME).getFileSize()).isNotEqualTo(0); + assertThat(outputDir.getRelative(HARD_LINK_FILE_NAME).isSymbolicLink()).isFalse(); + assertThat(outputDir.getRelative(SYMBOLIC_LINK_FILE_NAME).exists()).isTrue(); + assertThat(outputDir.getRelative(SYMBOLIC_LINK_FILE_NAME).getFileSize()).isNotEqualTo(0); + assertThat(outputDir.getRelative(SYMBOLIC_LINK_FILE_NAME).isSymbolicLink()).isTrue(); + assertThat( + Files.isSameFile( + java.nio.file.Paths.get(outputDir.getRelative(REGULAR_FILE_NAME).toString()), + java.nio.file.Paths.get(outputDir.getRelative(HARD_LINK_FILE_NAME).toString()))) + .isTrue(); + assertThat( + Files.isSameFile( + java.nio.file.Paths.get(outputDir.getRelative(REGULAR_FILE_NAME).toString()), + java.nio.file.Paths.get(outputDir.getRelative(SYMBOLIC_LINK_FILE_NAME).toString()))) + .isTrue(); + } +} diff --git a/src/test/java/com/google/devtools/build/lib/rules/repository/test_decompress_archive.tar.gz b/src/test/java/com/google/devtools/build/lib/rules/repository/test_decompress_archive.tar.gz new file mode 100644 index 0000000000..f951e97860 Binary files /dev/null and b/src/test/java/com/google/devtools/build/lib/rules/repository/test_decompress_archive.tar.gz differ diff --git a/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java b/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java index aba86119ab..8b4255b510 100644 --- a/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java +++ b/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.vfs; import static com.google.common.truth.Truth.assertThat; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -26,18 +27,17 @@ import com.google.devtools.build.lib.testutil.TestUtils; import com.google.devtools.build.lib.unix.NativePosixFiles; import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.util.Preconditions; - -import org.junit.After; -import org.junit.Before; -import org.junit.Test; - import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.nio.file.FileAlreadyExistsException; import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; /** * This class handles the generic tests that any filesystem must pass. @@ -50,6 +50,7 @@ public abstract class FileSystemTest { private long savedTime; protected FileSystem testFS; protected boolean supportsSymlinks; + protected boolean supportsHardlinks; protected Path workingDir; // Some useful examples of various kinds of files (mnemonic: "x" = "eXample") @@ -67,6 +68,7 @@ public abstract class FileSystemTest { workingDir = testFS.getPath(getTestTmpDir()); cleanUpWorkingDirectory(workingDir); supportsSymlinks = testFS.supportsSymbolicLinksNatively(); + supportsHardlinks = testFS.supportsHardLinksNatively(); // % ls -lR // -rw-rw-r-- xFile @@ -1203,7 +1205,7 @@ public abstract class FileSystemTest { public void testWritingToReadOnlyFileThrowsException() throws Exception { xFile.setWritable(false); try { - FileSystemUtils.writeContent(xFile, "hello, world!".getBytes()); + FileSystemUtils.writeContent(xFile, "hello, world!".getBytes(UTF_8)); fail("No exception thrown."); } catch (IOException e) { assertThat(e).hasMessage(xFile + " (Permission denied)"); @@ -1212,7 +1214,7 @@ public abstract class FileSystemTest { @Test public void testReadingFromUnreadableFileThrowsException() throws Exception { - FileSystemUtils.writeContent(xFile, "hello, world!".getBytes()); + FileSystemUtils.writeContent(xFile, "hello, world!".getBytes(UTF_8)); xFile.setReadable(false); try { FileSystemUtils.readContent(xFile); @@ -1367,4 +1369,79 @@ public abstract class FileSystemTest { } } + @Test + public void testCreateHardLink_Success() throws Exception { + if (!supportsHardlinks) { + return; + } + xFile.createHardLink(xLink); + assertTrue(xFile.exists()); + assertTrue(xLink.exists()); + assertTrue(xFile.isFile()); + assertTrue(xLink.isFile()); + assertTrue(isHardLinked(xFile, xLink)); + } + + @Test + public void testCreateHardLink_NeitherOriginalNorLinkExists() throws Exception { + if (!supportsHardlinks) { + return; + } + + /* Neither original file nor link file exists */ + xFile.delete(); + try { + xFile.createHardLink(xLink); + fail("expected FileNotFoundException: File \"xFile\" linked from \"xLink\" does not exist"); + } catch (FileNotFoundException expected) { + assertThat(expected).hasMessage("File \"xFile\" linked from \"xLink\" does not exist"); + } + assertFalse(xFile.exists()); + assertFalse(xLink.exists()); + } + + @Test + public void testCreateHardLink_OriginalDoesNotExistAndLinkExists() throws Exception { + + if (!supportsHardlinks) { + return; + } + + /* link file exists and original file does not exist */ + xFile.delete(); + FileSystemUtils.createEmptyFile(xLink); + + try { + xFile.createHardLink(xLink); + fail("expected FileNotFoundException: File \"xFile\" linked from \"xLink\" does not exist"); + } catch (FileNotFoundException expected) { + assertThat(expected).hasMessage("File \"xFile\" linked from \"xLink\" does not exist"); + } + assertFalse(xFile.exists()); + assertTrue(xLink.exists()); + } + + @Test + public void testCreateHardLink_BothOriginalAndLinkExist() throws Exception { + + if (!supportsHardlinks) { + return; + } + /* Both original file and link file exist */ + FileSystemUtils.createEmptyFile(xLink); + + try { + xFile.createHardLink(xLink); + fail("expected FileAlreadyExistsException: New link file \"xLink\" already exists"); + } catch (FileAlreadyExistsException expected) { + assertThat(expected).hasMessage("New link file \"xLink\" already exists"); + } + assertTrue(xFile.exists()); + assertTrue(xLink.exists()); + assertFalse(isHardLinked(xFile, xLink)); + } + + protected boolean isHardLinked(Path a, Path b) throws IOException { + return testFS.stat(a, false).getNodeId() == testFS.stat(b, false).getNodeId(); + } } 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 5c4aaa32d7..bcecada52e 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 @@ -38,17 +38,15 @@ import com.google.common.collect.Lists; import com.google.devtools.build.lib.testutil.BlazeTestUtils; import com.google.devtools.build.lib.testutil.ManualClock; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; - -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - import java.io.FileNotFoundException; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Collection; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; /** * This class tests the file system utilities. @@ -64,6 +62,7 @@ public class FileSystemUtilsTest { clock = new ManualClock(); fileSystem = new InMemoryFileSystem(clock); workingDir = fileSystem.getPath("/workingDir"); + workingDir.createDirectory(); } Path topDir; @@ -769,4 +768,67 @@ public class FileSystemUtilsTest { long timestamp = file.getLastModifiedTime(Symlinks.NOFOLLOW); assertEquals(prevTimeMillis, timestamp); } + + @Test + public void testCreateHardLinkForFile_Success() throws Exception { + + /* Original file exists and link file does not exist */ + Path originalPath = workingDir.getRelative("original"); + Path linkPath = workingDir.getRelative("link"); + FileSystemUtils.createEmptyFile(originalPath); + FileSystemUtils.createHardLink(linkPath, originalPath); + assertTrue(originalPath.exists()); + assertTrue(linkPath.exists()); + assertEquals( + fileSystem.stat(originalPath, false).getNodeId(), + fileSystem.stat(linkPath, false).getNodeId()); + } + + @Test + public void testCreateHardLinkForEmptyDirectory_Success() throws Exception { + + Path originalDir = workingDir.getRelative("originalDir"); + Path linkPath = workingDir.getRelative("link"); + + FileSystemUtils.createDirectoryAndParents(originalDir); + + /* Original directory is empty, no link to be created. */ + FileSystemUtils.createHardLink(linkPath, originalDir); + assertFalse(linkPath.exists()); + } + + @Test + public void testCreateHardLinkForNonEmptyDirectory_Success() throws Exception { + + /* Test when original path is a directory */ + Path originalDir = workingDir.getRelative("originalDir"); + Path linkPath = workingDir.getRelative("link"); + Path originalPath1 = originalDir.getRelative("original1"); + Path originalPath2 = originalDir.getRelative("original2"); + Path originalPath3 = originalDir.getRelative("original3"); + Path linkPath1 = linkPath.getRelative("original1"); + Path linkPath2 = linkPath.getRelative("original2"); + Path linkPath3 = linkPath.getRelative("original3"); + + FileSystemUtils.createDirectoryAndParents(originalDir); + FileSystemUtils.createEmptyFile(originalPath1); + FileSystemUtils.createEmptyFile(originalPath2); + FileSystemUtils.createEmptyFile(originalPath3); + + /* Three link files created under linkPath */ + FileSystemUtils.createHardLink(linkPath, originalDir); + assertTrue(linkPath.exists()); + assertTrue(linkPath1.exists()); + assertTrue(linkPath2.exists()); + assertTrue(linkPath3.exists()); + assertEquals( + fileSystem.stat(originalPath1, false).getNodeId(), + fileSystem.stat(linkPath1, false).getNodeId()); + assertEquals( + fileSystem.stat(originalPath2, false).getNodeId(), + fileSystem.stat(linkPath2, false).getNodeId()); + assertEquals( + fileSystem.stat(originalPath3, false).getNodeId(), + fileSystem.stat(linkPath3, false).getNodeId()); + } } diff --git a/src/test/java/com/google/devtools/build/lib/vfs/JavaIoFileSystemTest.java b/src/test/java/com/google/devtools/build/lib/vfs/JavaIoFileSystemTest.java index efbd5b0dbf..75bbe93571 100644 --- a/src/test/java/com/google/devtools/build/lib/vfs/JavaIoFileSystemTest.java +++ b/src/test/java/com/google/devtools/build/lib/vfs/JavaIoFileSystemTest.java @@ -16,7 +16,10 @@ package com.google.devtools.build.lib.vfs; import static org.junit.Assert.assertEquals; import com.google.devtools.build.lib.testutil.ManualClock; - +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.attribute.BasicFileAttributes; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -59,4 +62,19 @@ public class JavaIoFileSystemTest extends SymlinkAwareFileSystemTest { file.setLastModifiedTime(-1L); assertEquals(42000L, file.getLastModifiedTime()); } + + @Override + protected boolean isHardLinked(Path a, Path b) throws IOException { + return Files.readAttributes( + java.nio.file.Paths.get(a.toString()), + BasicFileAttributes.class, + LinkOption.NOFOLLOW_LINKS) + .fileKey() + .equals( + Files.readAttributes( + java.nio.file.Paths.get(b.toString()), + BasicFileAttributes.class, + LinkOption.NOFOLLOW_LINKS) + .fileKey()); + } } diff --git a/src/test/java/com/google/devtools/build/lib/vfs/ScopeEscapableFileSystemTest.java b/src/test/java/com/google/devtools/build/lib/vfs/ScopeEscapableFileSystemTest.java index f012b75463..f321ab93e3 100644 --- a/src/test/java/com/google/devtools/build/lib/vfs/ScopeEscapableFileSystemTest.java +++ b/src/test/java/com/google/devtools/build/lib/vfs/ScopeEscapableFileSystemTest.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.vfs; import static com.google.common.truth.Truth.assertThat; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotSame; @@ -24,16 +25,14 @@ import static org.junit.Assert.fail; import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.util.Preconditions; - -import org.junit.Before; -import org.junit.Test; - import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.util.Collection; +import org.junit.Before; +import org.junit.Test; /** * Generic tests for any file system that implements {@link ScopeEscapableFileSystem}, @@ -82,6 +81,10 @@ public abstract class ScopeEscapableFileSystemTest extends SymlinkAwareFileSyste return true; } + @Override public boolean supportsHardLinksNatively() { + return true; + } + @Override public boolean isFilePathCaseSensitive() { return true; @@ -113,7 +116,9 @@ public abstract class ScopeEscapableFileSystemTest extends SymlinkAwareFileSyste @Override protected void createSymbolicLink(Path linkPath, PathFragment targetFragment) { throw re(); } - + @Override protected void createFSDependentHardLink(Path linkPath, Path originalPath) { + throw re(); + } @Override protected PathFragment readSymbolicLink(Path path) { throw re(); } @Override protected InputStream getInputStream(Path path) { throw re(); } @Override protected Collection getDirectoryEntries(Path path) { throw re(); } @@ -639,12 +644,12 @@ public abstract class ScopeEscapableFileSystemTest extends SymlinkAwareFileSyste }; scopedFS().setDelegator(delegator); - delegator.setState(new ByteArrayInputStream("blah".getBytes())); + delegator.setState(new ByteArrayInputStream("blah".getBytes(UTF_8))); InputStream is = fileLink.getInputStream(); assertEquals(fileLinkTarget, delegator.lastPath()); assertSame(delegator.objectState(), is); - delegator.setState(new ByteArrayInputStream("blah2".getBytes())); + delegator.setState(new ByteArrayInputStream("blah2".getBytes(UTF_8))); is = dirLink.getInputStream(); assertEquals(dirLinkTarget, delegator.lastPath()); assertSame(delegator.objectState(), is); -- cgit v1.2.3