Skip to content

Commit

Permalink
Fix ZipDecompressor to take the output directory into account
Browse files Browse the repository at this point in the history
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
  • Loading branch information
irengrig authored and copybara-github committed Mar 12, 2019
1 parent b4f1467 commit f8023b0
Show file tree
Hide file tree
Showing 16 changed files with 393 additions and 240 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -77,6 +78,19 @@ private StripPrefixedPath(PathFragment pathFragment, boolean found, boolean skip
this.skip = skip;
}

public static PathFragment maybeDeprefixSymlink(
PathFragment linkPathFragment, Optional<String> 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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.:
*
* <pre>
* $ unzip -lf some-repo.zip
Expand All @@ -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<String> prefix = descriptor.prefix();
boolean foundPrefix = false;
// Store link, target info of symlinks, we create them after regular files are extracted.
Map<Path, PathFragment> symlinks = new HashMap<>();

try (ZipReader reader = new ZipReader(descriptor.archivePath().getPathFile())) {
Collection<ZipFileEntry> entries = reader.entries();
// Store link, target info of symlinks, we create them after regular files are extracted.
Map<Path, PathFragment> 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<Path, PathFragment> symlink : symlinks.entrySet()) {
symlink.getKey().createSymbolicLink(symlink.getValue());
extractZipEntry(
reader, entry, destinationDirectory, entryPath.getPathFragment(), prefix, symlinks);
}

if (prefix.isPresent() && !foundPrefix) {
Expand All @@ -107,6 +109,10 @@ public Path decompress(DecompressorDescriptor descriptor) throws IOException {
}
}

for (Map.Entry<Path, PathFragment> symlink : symlinks.entrySet()) {
FileSystemUtils.ensureSymbolicLink(symlink.getKey(), symlink.getValue());
}

return destinationDirectory;
}

Expand All @@ -115,6 +121,7 @@ private static void extractZipEntry(
ZipFileEntry entry,
Path destinationDirectory,
PathFragment strippedRelativePath,
Optional<String> prefix,
Map<Path, PathFragment> symlinks)
throws IOException {
if (strippedRelativePath.isAbsolute()) {
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
Expand All @@ -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",
Expand All @@ -55,6 +58,7 @@ java_test(
"//third_party:maven",
"//third_party:mockito",
"//third_party:truth",
"@bazel_tools//tools/java/runfiles",
],
)

Expand Down
Original file line number Diff line number Diff line change
@@ -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 <ARCHIVE_NAME> <files...>" */
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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,35 +12,33 @@
// 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;

@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);
Expand Down Expand Up @@ -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();
Expand All @@ -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\")");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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"));
}
}
Loading

0 comments on commit f8023b0

Please sign in to comment.