From f8023b00f3291d19c4d7f30df9b68326b9347985 Mon Sep 17 00:00:00 2001 From: Irina Date: Tue, 12 Mar 2019 04:37:42 -0700 Subject: [PATCH] Fix ZipDecompressor to take the output directory into account Also, add tests, similar to those for tar decompressor family, and extract common functionality for working with the test data into a helper class. Add assertions into the jar decompressor test. Have integration tests for extract and download_and_extract also check the actually extracted archive contents, have also a test for extract() from tar archive. Closes #7545. PiperOrigin-RevId: 237990861 --- .../repository/CompressedTarFunction.java | 12 +- .../repository/DecompressorDescriptor.java | 2 - .../bazel/repository/StripPrefixedPath.java | 14 ++ .../lib/bazel/repository/ZipDecompressor.java | 30 +-- .../skylark/SkylarkRepositoryContext.java | 10 + .../devtools/build/lib/bazel/repository/BUILD | 10 +- .../repository/CompressedTarFunctionTest.java | 79 ++++++++ .../repository/JarDecompressorTest.java | 17 +- .../repository/StripPrefixedPathTest.java | 29 +++ .../repository/TestArchiveDescriptor.java | 130 ++++++++++++ .../bazel/repository/ZipDecompressorTest.java | 40 ++++ .../repository/test_decompress_archive.tar.gz | Bin .../repository/test_decompress_archive.zip | Bin 0 -> 1318 bytes .../devtools/build/lib/rules/repository/BUILD | 14 +- .../repository/CompressedTarFunctionTest.java | 186 ------------------ src/test/shell/bazel/bazel_workspaces_test.sh | 60 +++++- 16 files changed, 393 insertions(+), 240 deletions(-) create mode 100644 src/test/java/com/google/devtools/build/lib/bazel/repository/CompressedTarFunctionTest.java rename src/test/java/com/google/devtools/build/lib/{rules => bazel}/repository/JarDecompressorTest.java (90%) create mode 100644 src/test/java/com/google/devtools/build/lib/bazel/repository/TestArchiveDescriptor.java rename src/test/java/com/google/devtools/build/lib/{rules => bazel}/repository/test_decompress_archive.tar.gz (100%) create mode 100644 src/test/java/com/google/devtools/build/lib/bazel/repository/test_decompress_archive.zip delete mode 100644 src/test/java/com/google/devtools/build/lib/rules/repository/CompressedTarFunctionTest.java diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/CompressedTarFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/CompressedTarFunction.java index 9a8bde0d5d3c3b..bd054e40267c19 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/CompressedTarFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/CompressedTarFunction.java @@ -14,6 +14,8 @@ package com.google.devtools.build.lib.bazel.repository; +import static com.google.devtools.build.lib.bazel.repository.StripPrefixedPath.maybeDeprefixSymlink; + import com.google.common.base.Optional; import com.google.devtools.build.lib.bazel.repository.DecompressorValue.Decompressor; import com.google.devtools.build.lib.vfs.FileSystemUtils; @@ -68,15 +70,7 @@ public Path decompress(DecompressorDescriptor descriptor) throws IOException { } else { if (entry.isSymbolicLink() || entry.isLink()) { PathFragment linkName = PathFragment.create(entry.getLinkName()); - boolean wasAbsolute = linkName.isAbsolute(); - // Strip the prefix from the link path if set. - linkName = - StripPrefixedPath.maybeDeprefix(linkName.getPathString(), prefix).getPathFragment(); - if (wasAbsolute) { - // Recover the path to an absolute path as maybeDeprefix() relativize the path - // even if the prefix is not set - linkName = descriptor.repositoryPath().getRelative(linkName).asFragment(); - } + linkName = maybeDeprefixSymlink(linkName, prefix, descriptor.repositoryPath()); if (filename.exists()) { filename.delete(); } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/DecompressorDescriptor.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/DecompressorDescriptor.java index 40c7b7c297c506..8f2b89f9a2f02c 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/DecompressorDescriptor.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/DecompressorDescriptor.java @@ -18,9 +18,7 @@ import com.google.devtools.build.lib.bazel.repository.DecompressorValue.Decompressor; import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException; import com.google.devtools.build.lib.vfs.Path; - import java.util.Objects; - import javax.annotation.Nullable; /** diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/StripPrefixedPath.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/StripPrefixedPath.java index ea3f887c21bf81..77c5815e79a50d 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/StripPrefixedPath.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/StripPrefixedPath.java @@ -17,6 +17,7 @@ import com.google.common.base.Optional; import com.google.common.base.Preconditions; import com.google.devtools.build.lib.concurrent.ThreadSafety; +import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; /** @@ -77,6 +78,19 @@ private StripPrefixedPath(PathFragment pathFragment, boolean found, boolean skip this.skip = skip; } + public static PathFragment maybeDeprefixSymlink( + PathFragment linkPathFragment, Optional prefix, Path root) { + boolean wasAbsolute = linkPathFragment.isAbsolute(); + // Strip the prefix from the link path if set. + linkPathFragment = maybeDeprefix(linkPathFragment.getPathString(), prefix).getPathFragment(); + if (wasAbsolute) { + // Recover the path to an absolute path as maybeDeprefix() relativize the path + // even if the prefix is not set + return root.getRelative(linkPathFragment).asFragment(); + } + return linkPathFragment; + } + public PathFragment getPathFragment() { return pathFragment; } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/ZipDecompressor.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/ZipDecompressor.java index c0239043739fa5..acb34fd38c4172 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/ZipDecompressor.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/ZipDecompressor.java @@ -14,6 +14,8 @@ package com.google.devtools.build.lib.bazel.repository; +import static com.google.devtools.build.lib.bazel.repository.StripPrefixedPath.maybeDeprefixSymlink; + import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Optional; import com.google.common.base.Preconditions; @@ -56,8 +58,9 @@ private ZipDecompressor() { static final int WINDOWS_FILE = 0x20; /** - * This unzips the zip file to a sibling directory of {@link DecompressorDescriptor#archivePath}. - * The zip file is expected to have the WORKSPACE file at the top level, e.g.: + * This unzips the zip file to directory {@link DecompressorDescriptor#repositoryPath()}, which by + * default is empty relative [to the calling external repository rule] path. The zip file is + * expected to have the WORKSPACE file at the top level, e.g.: * * 
    * $ unzip -lf some-repo.zip
@@ -73,23 +76,22 @@ private ZipDecompressor() {
   @Override
   @Nullable
   public Path decompress(DecompressorDescriptor descriptor) throws IOException {
-    Path destinationDirectory = descriptor.archivePath().getParentDirectory();
+    Path destinationDirectory = descriptor.repositoryPath();
     Optional prefix = descriptor.prefix();
     boolean foundPrefix = false;
+    // Store link, target info of symlinks, we create them after regular files are extracted.
+    Map symlinks = new HashMap<>();
+
     try (ZipReader reader = new ZipReader(descriptor.archivePath().getPathFile())) {
       Collection entries = reader.entries();
-      // Store link, target info of symlinks, we create them after regular files are extracted.
-      Map symlinks = new HashMap<>();
       for (ZipFileEntry entry : entries) {
         StripPrefixedPath entryPath = StripPrefixedPath.maybeDeprefix(entry.getName(), prefix);
         foundPrefix = foundPrefix || entryPath.foundPrefix();
         if (entryPath.skip()) {
           continue;
         }
-        extractZipEntry(reader, entry, destinationDirectory, entryPath.getPathFragment(), symlinks);
-      }
-      for (Map.Entry symlink : symlinks.entrySet()) {
-        symlink.getKey().createSymbolicLink(symlink.getValue());
+        extractZipEntry(
+            reader, entry, destinationDirectory, entryPath.getPathFragment(), prefix, symlinks);
       }
 
       if (prefix.isPresent() && !foundPrefix) {
@@ -107,6 +109,10 @@ public Path decompress(DecompressorDescriptor descriptor) throws IOException {
       }
     }
 
+    for (Map.Entry symlink : symlinks.entrySet()) {
+      FileSystemUtils.ensureSymbolicLink(symlink.getKey(), symlink.getValue());
+    }
+
     return destinationDirectory;
   }
 
@@ -115,6 +121,7 @@ private static void extractZipEntry(
       ZipFileEntry entry,
       Path destinationDirectory,
       PathFragment strippedRelativePath,
+      Optional prefix,
       Map symlinks)
       throws IOException {
     if (strippedRelativePath.isAbsolute()) {
@@ -144,10 +151,7 @@ private static void extractZipEntry(
               + target);
         }
       }
-      if (target.isAbsolute()) {
-        target = target.relativeTo("/");
-        target = destinationDirectory.getRelative(target).asFragment();
-      }
+      target = maybeDeprefixSymlink(target, prefix, destinationDirectory);
       symlinks.put(outputPath, target);
     } else {
       // TODO(kchodorow): should be able to be removed when issue #236 is resolved, but for now
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java
index 5ab3b9bd2bbd43..523e6415e14b1d 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java
@@ -437,7 +437,17 @@ public StructImpl download(
   public void extract(Object archive, Object output, String stripPrefix, Location location)
       throws RepositoryFunctionException, InterruptedException, EvalException {
     SkylarkPath archivePath = getPath("extract()", archive);
+
+    if (!archivePath.exists()) {
+      throw new RepositoryFunctionException(
+          new EvalException(
+              Location.fromFile(archivePath.getPath()),
+              String.format("Archive path '%s' does not exist.", archivePath.toString())),
+          Transience.TRANSIENT);
+    }
+
     SkylarkPath outputPath = getPath("extract()", output);
+    checkInOutputDirectory(outputPath);
 
     WorkspaceRuleEvent w =
         WorkspaceRuleEvent.newExtractEvent(
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/BUILD b/src/test/java/com/google/devtools/build/lib/bazel/repository/BUILD
index 7e191f5c1e2dd1..737356c35f4261 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/repository/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/BUILD
@@ -15,9 +15,11 @@ filegroup(
 
 java_test(
     name = "RepositoryTests",
-    srcs = glob([
-        "**/*.java",
-    ]),
+    srcs = glob(["*.java"]),
+    data = [
+        "test_decompress_archive.tar.gz",
+        "test_decompress_archive.zip",
+    ],
     tags = [
         "rules",
     ],
@@ -30,6 +32,7 @@ java_test(
         "//src/main/java/com/google/devtools/build/lib:os_util",
         "//src/main/java/com/google/devtools/build/lib:packages-internal",
         "//src/main/java/com/google/devtools/build/lib:syntax",
+        "//src/main/java/com/google/devtools/build/lib:unix",
         "//src/main/java/com/google/devtools/build/lib:util",
         "//src/main/java/com/google/devtools/build/lib/analysis/platform",
         "//src/main/java/com/google/devtools/build/lib/analysis/platform:utils",
@@ -55,6 +58,7 @@ java_test(
         "//third_party:maven",
         "//third_party:mockito",
         "//third_party:truth",
+        "@bazel_tools//tools/java/runfiles",
     ],
 )
 
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/CompressedTarFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/CompressedTarFunctionTest.java
new file mode 100644
index 00000000000000..debcb140d3382d
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/CompressedTarFunctionTest.java
@@ -0,0 +1,79 @@
+// 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.bazel.repository;
+
+import static com.google.devtools.build.lib.bazel.repository.TestArchiveDescriptor.INNER_FOLDER_NAME;
+import static com.google.devtools.build.lib.bazel.repository.TestArchiveDescriptor.ROOT_FOLDER_NAME;
+
+import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException;
+import com.google.devtools.build.lib.vfs.Path;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+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 {
+  /* Tarball, created by "tar -czf  " */
+  private static final String ARCHIVE_NAME = "test_decompress_archive.tar.gz";
+
+  private TestArchiveDescriptor archiveDescriptor;
+
+  @Before
+  public void setUpFs() throws Exception {
+    archiveDescriptor = new TestArchiveDescriptor(ARCHIVE_NAME, "out", true);
+  }
+
+  /**
+   * Test decompressing a tar.gz file with hard link file and symbolic link file inside without
+   * stripping a prefix
+   */
+  @Test
+  public void testDecompressWithoutPrefix() throws Exception {
+    Path outputDir = decompress(archiveDescriptor.createDescriptorBuilder());
+
+    archiveDescriptor.assertOutputFiles(outputDir, ROOT_FOLDER_NAME, INNER_FOLDER_NAME);
+  }
+
+  /**
+   * Test decompressing a tar.gz file with hard link file and symbolic link file inside and
+   * stripping a prefix
+   */
+  @Test
+  public void testDecompressWithPrefix() throws Exception {
+    DecompressorDescriptor.Builder descriptorBuilder =
+        archiveDescriptor.createDescriptorBuilder().setPrefix(ROOT_FOLDER_NAME);
+    Path outputDir = decompress(descriptorBuilder);
+
+    archiveDescriptor.assertOutputFiles(outputDir, INNER_FOLDER_NAME);
+  }
+
+  private Path decompress(DecompressorDescriptor.Builder descriptorBuilder)
+      throws IOException, RepositoryFunctionException {
+    descriptorBuilder.setDecompressor(TarGzFunction.INSTANCE);
+    return new CompressedTarFunction() {
+      @Override
+      protected InputStream getDecompressorStream(DecompressorDescriptor descriptor)
+          throws IOException {
+        return new GZIPInputStream(new FileInputStream(descriptor.archivePath().getPathFile()));
+      }
+    }.decompress(descriptorBuilder.build());
+  }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/repository/JarDecompressorTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/JarDecompressorTest.java
similarity index 90%
rename from src/test/java/com/google/devtools/build/lib/rules/repository/JarDecompressorTest.java
rename to src/test/java/com/google/devtools/build/lib/bazel/repository/JarDecompressorTest.java
index 9077297e76dd1c..0c94bee0c5bf73 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/repository/JarDecompressorTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/JarDecompressorTest.java
@@ -12,27 +12,25 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package com.google.devtools.build.lib.rules.repository;
+package com.google.devtools.build.lib.bazel.repository;
 
 import static com.google.common.truth.Truth.assertThat;
 
 import com.google.common.base.Optional;
-import com.google.devtools.build.lib.bazel.repository.DecompressorDescriptor;
-import com.google.devtools.build.lib.bazel.repository.JarDecompressor;
 import com.google.devtools.build.lib.bazel.rules.workspace.MavenJarRule;
 import com.google.devtools.build.lib.testutil.Scratch;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.PathFragment;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
-/**
- * Tests expanding external jars into external repositories.
- */
+/** Tests expanding external jars into external repositories. */
 @RunWith(JUnit4.class)
 public class JarDecompressorTest {
+  private static final String OUTPUT_DIRECTORY = "/whatever/external/tester";
   private DecompressorDescriptor.Builder jarDescriptorBuilder;
   private DecompressorDescriptor.Builder srcjarDescriptorBuilder;
   private JarDecompressor decompressor;
@@ -40,7 +38,7 @@ public class JarDecompressorTest {
   @Before
   public void setUpFs() throws Exception {
     Scratch fs = new Scratch();
-    Path dir = fs.dir("/whatever/external/tester");
+    Path dir = fs.dir(OUTPUT_DIRECTORY);
     Path jar = fs.file("/foo.jar", "I'm a jar");
     Path srcjar = fs.file("/foo-sources.jar", "I'm a source jar");
     FileSystemUtils.createDirectoryAndParents(dir);
@@ -96,6 +94,7 @@ public void testTargetIsSource() throws Exception {
         decompressor.decompressWithSrcjar(
             srcjarDescriptorBuilder.build(),
             Optional.fromNullable(srcjarDescriptorBuilder.build()));
+    assertThat(outputDir.asFragment().endsWith(PathFragment.create(OUTPUT_DIRECTORY))).isTrue();
     assertThat(outputDir.exists()).isTrue();
     assertThat(outputDir.getRelative("jar/foo.jar").exists()).isFalse();
     assertThat(outputDir.getRelative("jar/foo-sources.jar").exists()).isTrue();
@@ -114,8 +113,8 @@ public void testWorkspaceGen() throws Exception {
     Path outputDir =
         decompressor.decompressWithSrcjar(jarDescriptorBuilder.build(), Optional.absent());
     assertThat(outputDir.exists()).isTrue();
-    String workspaceContent = new String(
-        FileSystemUtils.readContentAsLatin1(outputDir.getRelative("WORKSPACE")));
+    String workspaceContent =
+        new String(FileSystemUtils.readContentAsLatin1(outputDir.getRelative("WORKSPACE")));
     assertThat(workspaceContent).contains("workspace(name = \"tester\")");
   }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/StripPrefixedPathTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/StripPrefixedPathTest.java
index 51cef7f03bbc4f..77ecd725be9505 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/repository/StripPrefixedPathTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/StripPrefixedPathTest.java
@@ -17,8 +17,10 @@
 import static com.google.common.truth.Truth.assertThat;
 
 import com.google.common.base.Optional;
+import com.google.devtools.build.lib.clock.BlazeClock;
 import com.google.devtools.build.lib.util.OS;
 import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -80,4 +82,31 @@ public void testNormalize() {
     result = StripPrefixedPath.maybeDeprefix("foo/../baz", Optional.of("foo"));
     assertThat(result.getPathFragment()).isEqualTo(PathFragment.create("baz"));
   }
+
+  @Test
+  public void testDeprefixSymlink() {
+    InMemoryFileSystem fileSystem = new InMemoryFileSystem(BlazeClock.instance());
+
+    PathFragment relativeNoPrefix =
+        StripPrefixedPath.maybeDeprefixSymlink(
+            PathFragment.create("a/b"), Optional.absent(), fileSystem.getPath("/usr"));
+    // there is no attempt to get absolute path for the relative symlinks target path
+    assertThat(relativeNoPrefix).isEqualTo(PathFragment.create("a/b"));
+
+    PathFragment absoluteNoPrefix =
+        StripPrefixedPath.maybeDeprefixSymlink(
+            PathFragment.create("/a/b"), Optional.absent(), fileSystem.getPath("/usr"));
+    assertThat(absoluteNoPrefix).isEqualTo(PathFragment.create("/usr/a/b"));
+
+    PathFragment absolutePrefix =
+        StripPrefixedPath.maybeDeprefixSymlink(
+            PathFragment.create("/root/a/b"), Optional.of("root"), fileSystem.getPath("/usr"));
+    assertThat(absolutePrefix).isEqualTo(PathFragment.create("/usr/a/b"));
+
+    PathFragment relativePrefix =
+        StripPrefixedPath.maybeDeprefixSymlink(
+            PathFragment.create("root/a/b"), Optional.of("root"), fileSystem.getPath("/usr"));
+    // there is no attempt to get absolute path for the relative symlinks target path
+    assertThat(relativePrefix).isEqualTo(PathFragment.create("a/b"));
+  }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/TestArchiveDescriptor.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/TestArchiveDescriptor.java
new file mode 100644
index 00000000000000..a2e45e49ce76e6
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/TestArchiveDescriptor.java
@@ -0,0 +1,130 @@
+// Copyright 2019 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.bazel.repository;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.devtools.build.lib.testutil.TestConstants;
+import com.google.devtools.build.lib.testutil.TestUtils;
+import com.google.devtools.build.lib.unix.UnixFileSystem;
+import com.google.devtools.build.lib.util.OS;
+import com.google.devtools.build.lib.vfs.DigestHashFunction;
+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.PathFragment;
+import com.google.devtools.build.runfiles.Runfiles;
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+
+/**
+ * Helper class for working with test archive file.
+ *
+ * 
The archive has the following structure
+ *
+ * 
root_folder/ another_folder/ regularFile hardLinkFile hardlink to
+ * root_folder/another_folder/regularFile relativeSymbolicLinkFile -> regularFile
+ * absoluteSymbolicLinkFile -> /root_folder/another_folder/regularFile
+ */
+public class TestArchiveDescriptor {
+  /* 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 RELATIVE_SYMBOLIC_LINK_FILE_NAME = "relativeSymbolicLinkFile";
+  private static final String ABSOLUTE_SYMBOLIC_LINK_FILE_NAME = "absoluteSymbolicLinkFile";
+  private static final String PATH_TO_TEST_ARCHIVE =
+      "/com/google/devtools/build/lib/bazel/repository/";
+
+  static final String ROOT_FOLDER_NAME = "root_folder";
+  static final String INNER_FOLDER_NAME = "another_folder";
+
+  private final String archiveName;
+  private final String outDirName;
+  private final boolean withHardLinks;
+
+  TestArchiveDescriptor(String archiveName, String outDirName, boolean withHardLinks) {
+    this.archiveName = archiveName;
+    this.outDirName = outDirName;
+    this.withHardLinks = withHardLinks;
+  }
+
+  DecompressorDescriptor.Builder createDescriptorBuilder() throws IOException {
+    FileSystem testFS =
+        OS.getCurrent() == OS.WINDOWS
+            ? new JavaIoFileSystem(DigestHashFunction.DEFAULT_HASH_FOR_TESTS)
+            : new UnixFileSystem(DigestHashFunction.DEFAULT_HASH_FOR_TESTS);
+
+    // do not rely on TestConstants.JAVATESTS_ROOT end with slash, but ensure separators
+    // are not duplicated
+    String path =
+        (TestConstants.JAVATESTS_ROOT + PATH_TO_TEST_ARCHIVE + archiveName).replace("//", "/");
+    Path tarballPath = testFS.getPath(Runfiles.create().rlocation(path));
+
+    Path workingDir = testFS.getPath(new File(TestUtils.tmpDir()).getCanonicalPath());
+    Path outDir = workingDir.getRelative(outDirName);
+
+    return DecompressorDescriptor.builder().setRepositoryPath(outDir).setArchivePath(tarballPath);
+  }
+
+  /** Validate the content of the output directory */
+  void assertOutputFiles(Path rootOutputDir, String... relativePath) throws Exception {
+    assertThat(rootOutputDir.asFragment().endsWith(PathFragment.create(outDirName))).isTrue();
+    Path outputDir = rootOutputDir;
+    for (String part : relativePath) {
+      outputDir = outputDir.getRelative(part);
+    }
+
+    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(RELATIVE_SYMBOLIC_LINK_FILE_NAME).exists()).isTrue();
+    assertThat(outputDir.getRelative(RELATIVE_SYMBOLIC_LINK_FILE_NAME).getFileSize())
+        .isNotEqualTo(0);
+    assertThat(outputDir.getRelative(RELATIVE_SYMBOLIC_LINK_FILE_NAME).isSymbolicLink()).isTrue();
+    assertThat(outputDir.getRelative(ABSOLUTE_SYMBOLIC_LINK_FILE_NAME).exists()).isTrue();
+    assertThat(outputDir.getRelative(ABSOLUTE_SYMBOLIC_LINK_FILE_NAME).getFileSize())
+        .isNotEqualTo(0);
+    assertThat(outputDir.getRelative(ABSOLUTE_SYMBOLIC_LINK_FILE_NAME).isSymbolicLink()).isTrue();
+
+    if (withHardLinks) {
+      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(
+              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(RELATIVE_SYMBOLIC_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(ABSOLUTE_SYMBOLIC_LINK_FILE_NAME).toString())))
+        .isTrue();
+  }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/ZipDecompressorTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/ZipDecompressorTest.java
index 8af38d38155c38..7a4c3f86bd39a1 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/repository/ZipDecompressorTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/ZipDecompressorTest.java
@@ -15,7 +15,12 @@
 package com.google.devtools.build.lib.bazel.repository;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.devtools.build.lib.bazel.repository.TestArchiveDescriptor.INNER_FOLDER_NAME;
+import static com.google.devtools.build.lib.bazel.repository.TestArchiveDescriptor.ROOT_FOLDER_NAME;
 
+import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException;
+import com.google.devtools.build.lib.vfs.Path;
+import java.io.IOException;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -36,6 +41,41 @@ public class ZipDecompressorTest {
   private static final int EXECUTABLE_ATTRIBUTE = EXECUTABLE << 16;
   private static final int DIRECTORY_ATTRIBUTE = DIRECTORY << 16;
 
+  private static final String ARCHIVE_NAME = "test_decompress_archive.zip";
+
+  /**
+   * Test decompressing a tar.gz file with hard link file and symbolic link file inside without
+   * stripping a prefix
+   */
+  @Test
+  public void testDecompressWithoutPrefix() throws Exception {
+    TestArchiveDescriptor archiveDescriptor =
+        new TestArchiveDescriptor(ARCHIVE_NAME, "out/inner", false);
+    Path outputDir = decompress(archiveDescriptor.createDescriptorBuilder());
+
+    archiveDescriptor.assertOutputFiles(outputDir, ROOT_FOLDER_NAME, INNER_FOLDER_NAME);
+  }
+
+  /**
+   * Test decompressing a tar.gz file with hard link file and symbolic link file inside and
+   * stripping a prefix
+   */
+  @Test
+  public void testDecompressWithPrefix() throws Exception {
+    TestArchiveDescriptor archiveDescriptor = new TestArchiveDescriptor(ARCHIVE_NAME, "out", false);
+    DecompressorDescriptor.Builder descriptorBuilder =
+        archiveDescriptor.createDescriptorBuilder().setPrefix(ROOT_FOLDER_NAME);
+    Path outputDir = decompress(descriptorBuilder);
+
+    archiveDescriptor.assertOutputFiles(outputDir, INNER_FOLDER_NAME);
+  }
+
+  private Path decompress(DecompressorDescriptor.Builder descriptorBuilder)
+      throws IOException, RepositoryFunctionException {
+    descriptorBuilder.setDecompressor(ZipDecompressor.INSTANCE);
+    return ZipDecompressor.INSTANCE.decompress(descriptorBuilder.build());
+  }
+
   @Test
   public void testGetPermissions() throws Exception {
     int permissions = ZipDecompressor.getPermissions(FILE_ATTRIBUTE, "foo/bar");
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/bazel/repository/test_decompress_archive.tar.gz
similarity index 100%
rename from src/test/java/com/google/devtools/build/lib/rules/repository/test_decompress_archive.tar.gz
rename to src/test/java/com/google/devtools/build/lib/bazel/repository/test_decompress_archive.tar.gz
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/test_decompress_archive.zip b/src/test/java/com/google/devtools/build/lib/bazel/repository/test_decompress_archive.zip
new file mode 100644
index 0000000000000000000000000000000000000000..f23dde48798ba61680eaedc46f661703b7b1aa5b
GIT binary patch
literal 1318
zcmWIWW@h1H0D;$Q%{;*jD8a)Z!%&o;UlN~|pOcbWq#qi>$-wMs`XroJuQ8^yf}4Sn
z<#Ztj150o`BSQe(6qVX^KV(y+(M(Cq%P+}DEkZH0S+_9;uc4oSPK(>od`lE)G6<{T
zH?$}YnU6w-jo^|%l|LAO*Lzoi+8
zMJYa+dD#@&3JSl5mmk!*f!+jRWBj%jrRF4-WR|4{SLP<==VT_MxBwKcAdD7SNWp{f
zEy&*Lm7>Sgfp&s00ecgZit}?yOGvU;A2~Ilr2`Z  */
-  private static final String HARD_LINK_FILE_NAME = "hardLinkFile";
-
-  /* Symbolic(Soft) link file, created by ln -s   */
-  private static final String RELATIVE_SYMBOLIC_LINK_FILE_NAME = "relativeSymbolicLinkFile";
-  private static final String ABSOLUTE_SYMBOLIC_LINK_FILE_NAME = "absoluteSymbolicLinkFile";
-
-  private static final String PATH_TO_TEST_ARCHIVE =
-      "/com/google/devtools/build/lib/rules/repository/";
-
-  private static final String ROOT_FOLDER_NAME = "root_folder";
-
-  private static final String INNER_FOLDER_NAME = "another_folder";
-
-  /* Tarball, created by
-   * tar -czf    
-   *
-   * The tarball has the following structure
-   *
-   * root_folder/
-   *    another_folder/
-   *        regularFile
-   *        hardLinkFile hardlink to root_folder/another_folder/regularFile
-   *        relativeSymbolicLinkFile -> regularFile
-   *        absoluteSymbolicLinkFile -> /root_folder/another_folder/regularFile
-   */
-  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(DigestHashFunction.DEFAULT_HASH_FOR_TESTS)
-            : new UnixFileSystem(DigestHashFunction.DEFAULT_HASH_FOR_TESTS);
-
-    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 without
-   * stripping a prefix
-   *
-   * @throws Exception
-   */
-  @Test
-  public void testDecompressWithoutPrefix() 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());
-
-    assertOutputFiles(outputDir.getRelative(ROOT_FOLDER_NAME).getRelative(INNER_FOLDER_NAME));
-  }
-
-  /**
-   * Test decompressing a tar.gz file with hard link file and symbolic link file inside and
-   * stripping a prefix
-   *
-   * @throws Exception
-   */
-  @Test
-  public void testDecompressWithPrefix() throws Exception {
-
-    descriptorBuilder.setPrefix(ROOT_FOLDER_NAME);
-
-    Path outputDir =
-        new CompressedTarFunction() {
-          @Override
-          protected InputStream getDecompressorStream(DecompressorDescriptor descriptor)
-              throws IOException {
-            return new GZIPInputStream(new FileInputStream(descriptor.archivePath().getPathFile()));
-          }
-        }.decompress(descriptorBuilder.build());
-
-    assertOutputFiles(outputDir.getRelative(INNER_FOLDER_NAME));
-  }
-
-  /** Validate the content of the output directory */
-  private void assertOutputFiles(Path outputDir) throws Exception {
-
-    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(RELATIVE_SYMBOLIC_LINK_FILE_NAME).exists()).isTrue();
-    assertThat(outputDir.getRelative(RELATIVE_SYMBOLIC_LINK_FILE_NAME).getFileSize())
-        .isNotEqualTo(0);
-    assertThat(outputDir.getRelative(RELATIVE_SYMBOLIC_LINK_FILE_NAME).isSymbolicLink()).isTrue();
-    assertThat(outputDir.getRelative(ABSOLUTE_SYMBOLIC_LINK_FILE_NAME).exists()).isTrue();
-    assertThat(outputDir.getRelative(ABSOLUTE_SYMBOLIC_LINK_FILE_NAME).getFileSize())
-        .isNotEqualTo(0);
-    assertThat(outputDir.getRelative(ABSOLUTE_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(RELATIVE_SYMBOLIC_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(ABSOLUTE_SYMBOLIC_LINK_FILE_NAME).toString())))
-        .isTrue();
-  }
-}
diff --git a/src/test/shell/bazel/bazel_workspaces_test.sh b/src/test/shell/bazel/bazel_workspaces_test.sh
index 8d21cfa74880a2..4e4718a56511d5 100755
--- a/src/test/shell/bazel/bazel_workspaces_test.sh
+++ b/src/test/shell/bazel/bazel_workspaces_test.sh
@@ -75,6 +75,15 @@ function ensure_contains_atleast() {
   fi
 }
 
+function ensure_output_contains_exactly_once() {
+  file_path=$(bazel info output_base)/$1
+  num=`grep "$2" $file_path | wc -l`
+  if [ "$num" -ne 1 ]
+  then
+    fail "Expected to read \"$2\" in $1, but got $num occurrences: " `cat $file_path`
+  fi
+}
+
 function test_execute() {
   set_workspace_command 'repository_ctx.execute(["echo", "testing!"])'
   build_and_process_log
@@ -221,9 +230,9 @@ function test_download_then_extract() {
   local file_prefix="${server_dir}/download_then_extract"
 
   pushd ${TEST_TMPDIR}
-  echo "This is one file" > server_dir/download_then_extract.txt
-  zip -r server_dir/download_then_extract.zip server_dir
-  file_sha256="$(sha256sum server_dir/download_then_extract.zip | head -c 64)"
+  echo "This is one file" > ${server_dir}/download_then_extract.txt
+  zip -r ${server_dir}/download_then_extract.zip server_dir
+  file_sha256="$(sha256sum $server_dir/download_then_extract.zip | head -c 64)"
   popd
 
   # Start HTTP server with Python
@@ -246,6 +255,45 @@ function test_download_then_extract() {
   ensure_contains_exactly 'archive: "downloaded_file.zip"' 1
   ensure_contains_exactly 'output: "out_dir"' 1
   ensure_contains_exactly 'strip_prefix: "server_dir/"' 1
+
+  ensure_output_contains_exactly_once "external/repo/out_dir/download_then_extract.txt" "This is one file"
+}
+
+function test_download_then_extract_tar() {
+  # Prepare HTTP server with Python
+  local server_dir="${TEST_TMPDIR}/server_dir"
+  local data_dir="${TEST_TMPDIR}/data_dir"
+  mkdir -p "${server_dir}"
+  mkdir -p "${data_dir}"
+
+  pushd ${TEST_TMPDIR}
+  echo "Experiment with tar" > ${data_dir}/download_then_extract_tar.txt
+  tar -zcvf ${server_dir}/download_then_extract.tar.gz data_dir
+  file_sha256="$(sha256sum $server_dir/download_then_extract.tar.gz | head -c 64)"
+  popd
+
+  # Start HTTP server with Python
+  startup_server "${server_dir}"
+
+  set_workspace_command "
+  repository_ctx.download(\"http://localhost:${fileserver_port}/download_then_extract.tar.gz\", \"downloaded_file.tar.gz\", \"${file_sha256}\")
+  repository_ctx.extract(\"downloaded_file.tar.gz\", \"out_dir\", \"data_dir/\")"
+
+  build_and_process_log --exclude_rule "//external:local_config_cc"
+
+  ensure_contains_exactly 'location: .*repos.bzl:3:3' 1
+  ensure_contains_exactly 'location: .*repos.bzl:4:3' 1
+  ensure_contains_atleast 'rule: "//external:repo"' 2
+  ensure_contains_exactly 'download_event' 1
+  ensure_contains_exactly "url: \"http://localhost:${fileserver_port}/download_then_extract.tar.gz\"" 1
+  ensure_contains_exactly 'output: "downloaded_file.tar.gz"' 1
+  ensure_contains_exactly "sha256: \"${file_sha256}\"" 1
+  ensure_contains_exactly 'extract_event' 1
+  ensure_contains_exactly 'archive: "downloaded_file.tar.gz"' 1
+  ensure_contains_exactly 'output: "out_dir"' 1
+  ensure_contains_exactly 'strip_prefix: "data_dir/"' 1
+
+  ensure_output_contains_exactly_once "external/repo/out_dir/download_then_extract_tar.txt" "Experiment with tar"
 }
 
 function test_download_and_extract() {
@@ -255,8 +303,8 @@ function test_download_and_extract() {
   local file_prefix="${server_dir}/download_and_extract"
 
   pushd ${TEST_TMPDIR}
-  echo "This is one file" > server_dir/download_and_extract.txt
-  zip -r server_dir/download_and_extract.zip server_dir
+  echo "This is one file" > ${server_dir}/download_and_extract.txt
+  zip -r ${server_dir}/download_and_extract.zip server_dir
   file_sha256="$(sha256sum server_dir/download_and_extract.zip | head -c 64)"
   popd
 
@@ -275,6 +323,8 @@ function test_download_and_extract() {
   ensure_contains_exactly "sha256: \"${file_sha256}\"" 1
   ensure_contains_exactly 'type: "zip"' 1
   ensure_contains_exactly 'strip_prefix: "server_dir/"' 1
+
+  ensure_output_contains_exactly_once "external/repo/out_dir/download_and_extract.txt" "This is one file"
 }
 
 function test_file() {