From 2856e8d157bacee20d1d68e6cb1133d6b04c9c92 Mon Sep 17 00:00:00 2001 From: John Millikin Date: Fri, 5 Aug 2022 20:50:01 +0900 Subject: [PATCH] repository_ctx: Allow renaming archive entries during extraction. --- .../lib/bazel/debug/WorkspaceRuleEvent.java | 22 +++++++++--- .../build/lib/bazel/debug/workspace_log.proto | 4 +++ .../lib/bazel/repository/ArFunction.java | 9 ++++- .../repository/CompressedTarFunction.java | 7 +++- .../repository/DecompressorDescriptor.java | 23 ++++++++++-- .../lib/bazel/repository/ZipDecompressor.java | 7 +++- .../starlark/StarlarkRepositoryContext.java | 36 ++++++++++++++++++- .../lib/bazel/repository/ArFunctionTest.java | 18 ++++++++++ .../repository/CompressedTarFunctionTest.java | 22 ++++++++++++ .../bazel/repository/ZipDecompressorTest.java | 22 ++++++++++++ src/test/shell/bazel/bazel_workspaces_test.sh | 32 +++++++++++++++++ 11 files changed, 190 insertions(+), 12 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/debug/WorkspaceRuleEvent.java b/src/main/java/com/google/devtools/build/lib/bazel/debug/WorkspaceRuleEvent.java index a0dce42bea131a..e44c1feb440e84 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/debug/WorkspaceRuleEvent.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/debug/WorkspaceRuleEvent.java @@ -110,17 +110,25 @@ public static WorkspaceRuleEvent newDownloadEvent( /** Creates a new WorkspaceRuleEvent for an extract event. */ public static WorkspaceRuleEvent newExtractEvent( - String archive, String output, String stripPrefix, String ruleLabel, Location location) { - ExtractEvent e = + String archive, + String output, + String stripPrefix, + Map renameFiles, + String ruleLabel, + Location location) { + + WorkspaceLogProtos.ExtractEvent.Builder e = WorkspaceLogProtos.ExtractEvent.newBuilder() .setArchive(archive) .setOutput(output) - .setStripPrefix(stripPrefix) - .build(); + .setStripPrefix(stripPrefix); + if (renameFiles != null) { + e = e.putAllRenameFiles(renameFiles); + } WorkspaceLogProtos.WorkspaceEvent.Builder result = WorkspaceLogProtos.WorkspaceEvent.newBuilder(); - result = result.setExtractEvent(e); + result = result.setExtractEvent(e.build()); if (location != null) { result = result.setLocation(location.toString()); } @@ -138,6 +146,7 @@ public static WorkspaceRuleEvent newDownloadAndExtractEvent( String integrity, String type, String stripPrefix, + Map renameFiles, String ruleLabel, Location location) { WorkspaceLogProtos.DownloadAndExtractEvent.Builder e = @@ -150,6 +159,9 @@ public static WorkspaceRuleEvent newDownloadAndExtractEvent( for (URL u : urls) { e.addUrl(u.toString()); } + if (renameFiles != null) { + e = e.putAllRenameFiles(renameFiles); + } WorkspaceLogProtos.WorkspaceEvent.Builder result = WorkspaceLogProtos.WorkspaceEvent.newBuilder(); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/debug/workspace_log.proto b/src/main/java/com/google/devtools/build/lib/bazel/debug/workspace_log.proto index d6ce5b8b080cf8..f975e8ef0fd003 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/debug/workspace_log.proto +++ b/src/main/java/com/google/devtools/build/lib/bazel/debug/workspace_log.proto @@ -59,6 +59,8 @@ message ExtractEvent { string output = 2; // A directory prefix to strip from extracted files. string strip_prefix = 3; + // Files to rename during extraction. + map rename_files = 4; } message DownloadAndExtractEvent { @@ -74,6 +76,8 @@ message DownloadAndExtractEvent { string strip_prefix = 5; // checksum in Subresource Integrity format, if specified string integrity = 6; + // Files to rename during extraction. + map rename_files = 7; } // Information on "file" event in repository_ctx. diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/ArFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/ArFunction.java index dc4ac44f96f8ea..66fa9dccb1abd2 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/ArFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/ArFunction.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.util.Map; import org.apache.commons.compress.archivers.ar.ArArchiveEntry; import org.apache.commons.compress.archivers.ar.ArArchiveInputStream; @@ -49,11 +50,17 @@ public Path decompress(DecompressorDescriptor descriptor) throw new InterruptedException(); } + Map renameFiles = descriptor.renameFiles(); + try (InputStream decompressorStream = getDecompressorStream(descriptor)) { ArArchiveInputStream arStream = new ArArchiveInputStream(decompressorStream); ArArchiveEntry entry; while ((entry = arStream.getNextArEntry()) != null) { - Path filePath = descriptor.repositoryPath().getRelative(entry.getName()); + String entryName = entry.getName(); + if (renameFiles != null) { + entryName = renameFiles.getOrDefault(entryName, entryName); + } + Path filePath = descriptor.repositoryPath().getRelative(entryName); filePath.getParentDirectory().createDirectoryAndParents(); if (entry.isDirectory()) { // ar archives don't contain any directory information, so this should never 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 deaaa838329222..9b5d62fe1ab8ea 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 @@ -47,6 +47,7 @@ public Path decompress(DecompressorDescriptor descriptor) throw new InterruptedException(); } Optional prefix = descriptor.prefix(); + Map renameFiles = descriptor.renameFiles(); boolean foundPrefix = false; Set availablePrefixes = new HashSet<>(); // Store link, target info of symlinks, we create them after regular files are extracted. @@ -56,7 +57,11 @@ public Path decompress(DecompressorDescriptor descriptor) TarArchiveInputStream tarStream = new TarArchiveInputStream(decompressorStream); TarArchiveEntry entry; while ((entry = tarStream.getNextTarEntry()) != null) { - StripPrefixedPath entryPath = StripPrefixedPath.maybeDeprefix(entry.getName(), prefix); + String entryName = entry.getName(); + if (renameFiles != null) { + entryName = renameFiles.getOrDefault(entryName, entryName); + } + StripPrefixedPath entryPath = StripPrefixedPath.maybeDeprefix(entryName, prefix); foundPrefix = foundPrefix || entryPath.foundPrefix(); if (prefix.isPresent() && !foundPrefix) { 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 485fb0b18803c3..e1f3b20208485c 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 @@ -19,6 +19,7 @@ import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException; import com.google.devtools.build.lib.vfs.Path; import com.google.errorprone.annotations.CanIgnoreReturnValue; +import java.util.Map; import java.util.Objects; import javax.annotation.Nullable; @@ -33,17 +34,21 @@ public class DecompressorDescriptor { private final Path repositoryPath; private final Optional prefix; private final boolean executable; + private final Map renameFiles; private final Decompressor decompressor; private DecompressorDescriptor( String targetKind, String targetName, Path archivePath, Path repositoryPath, - @Nullable String prefix, boolean executable, Decompressor decompressor) { + @Nullable String prefix, boolean executable, + @Nullable Map renameFiles, + Decompressor decompressor) { this.targetKind = targetKind; this.targetName = targetName; this.archivePath = archivePath; this.repositoryPath = repositoryPath; this.prefix = Optional.fromNullable(prefix); this.executable = executable; + this.renameFiles = renameFiles; this.decompressor = decompressor; } @@ -71,6 +76,10 @@ public boolean executable() { return executable; } + public Map renameFiles() { + return renameFiles; + } + public Decompressor getDecompressor() { return decompressor; } @@ -92,12 +101,13 @@ public boolean equals(Object other) { && Objects.equals(repositoryPath, descriptor.repositoryPath) && Objects.equals(prefix, descriptor.prefix) && Objects.equals(executable, descriptor.executable) + && Objects.equals(renameFiles, descriptor.renameFiles) && decompressor == descriptor.decompressor; } @Override public int hashCode() { - return Objects.hash(targetKind, targetName, archivePath, repositoryPath, prefix); + return Objects.hash(targetKind, targetName, archivePath, repositoryPath, prefix, renameFiles); } public static Builder builder() { @@ -115,6 +125,7 @@ public static class Builder { private Path repositoryPath; private String prefix; private boolean executable; + private Map renameFiles; private Decompressor decompressor; private Builder() { @@ -125,7 +136,7 @@ public DecompressorDescriptor build() throws RepositoryFunctionException { decompressor = DecompressorValue.getDecompressor(archivePath); } return new DecompressorDescriptor( - targetKind, targetName, archivePath, repositoryPath, prefix, executable, decompressor); + targetKind, targetName, archivePath, repositoryPath, prefix, executable, renameFiles, decompressor); } @CanIgnoreReturnValue @@ -164,6 +175,12 @@ public Builder setExecutable(boolean executable) { return this; } + @CanIgnoreReturnValue + public Builder setRenameFiles(Map renameFiles) { + this.renameFiles = renameFiles; + return this; + } + @CanIgnoreReturnValue public Builder setDecompressor(Decompressor decompressor) { this.decompressor = decompressor; 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 f2bf17cc235ddd..9dc7a478288715 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 @@ -79,6 +79,7 @@ public Path decompress(DecompressorDescriptor descriptor) throws IOException, InterruptedException { Path destinationDirectory = descriptor.repositoryPath(); Optional prefix = descriptor.prefix(); + Map renameFiles = descriptor.renameFiles(); boolean foundPrefix = false; // Store link, target info of symlinks, we create them after regular files are extracted. Map symlinks = new HashMap<>(); @@ -86,7 +87,11 @@ public Path decompress(DecompressorDescriptor descriptor) try (ZipReader reader = new ZipReader(descriptor.archivePath().getPathFile())) { Collection entries = reader.entries(); for (ZipFileEntry entry : entries) { - StripPrefixedPath entryPath = StripPrefixedPath.maybeDeprefix(entry.getName(), prefix); + String entryName = entry.getName(); + if (renameFiles != null) { + entryName = renameFiles.getOrDefault(entryName, entryName); + } + StripPrefixedPath entryPath = StripPrefixedPath.maybeDeprefix(entryName, prefix); foundPrefix = foundPrefix || entryPath.foundPrefix(); if (entryPath.skip()) { continue; diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java index 242225ed06a981..c2a77fed4c6071 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java @@ -749,8 +749,22 @@ public StructImpl download( + " archive. Instead of needing to specify this prefix over and over in the" + " build_file, this field can be used to strip it from extracted" + " files."), + @Param( + name = "renameFiles", + defaultValue = "{}", + named = true, + positional = false, + doc = + "An optional dict specifying files to rename during the extraction. Archive entries" + + " with names exactly matching a key will be renamed to the value, prior to" + + " any directory prefix adjustment."), }) - public void extract(Object archive, Object output, String stripPrefix, StarlarkThread thread) + public void extract( + Object archive, + Object output, + String stripPrefix, + Dict renameFiles, // expected + StarlarkThread thread) throws RepositoryFunctionException, InterruptedException, EvalException { StarlarkPath archivePath = getPath("extract()", archive); @@ -762,11 +776,15 @@ public void extract(Object archive, Object output, String stripPrefix, StarlarkT StarlarkPath outputPath = getPath("extract()", output); checkInOutputDirectory("write", outputPath); + Map renameFilesMap = + Dict.cast(renameFiles, String.class, String.class, "renameFiles"); + WorkspaceRuleEvent w = WorkspaceRuleEvent.newExtractEvent( archive.toString(), output.toString(), stripPrefix, + renameFilesMap, rule.getLabel().toString(), thread.getCallerLocation()); env.getListener().post(w); @@ -782,6 +800,7 @@ public void extract(Object archive, Object output, String stripPrefix, StarlarkT .setArchivePath(archivePath.getPath()) .setRepositoryPath(outputPath.getPath()) .setPrefix(stripPrefix) + .setRenameFiles(renameFilesMap) .build()); env.getListener().post(new ExtractProgress(outputPath.getPath().toString())); } @@ -881,6 +900,15 @@ public void extract(Object archive, Object output, String stripPrefix, StarlarkT + " risk to omit the checksum as remote files can change. At best omitting this" + " field will make your build non-hermetic. It is optional to make development" + " easier but should be set before shipping."), + @Param( + name = "renameFiles", + defaultValue = "{}", + named = true, + positional = false, + doc = + "An optional dict specifying files to rename during the extraction. Archive entries" + + " with names exactly matching a key will be renamed to the value, prior to" + + " any directory prefix adjustment."), }) public StructImpl downloadAndExtract( Object url, @@ -892,6 +920,7 @@ public StructImpl downloadAndExtract( String canonicalId, Dict auth, // expected String integrity, + Dict renameFiles, // expected StarlarkThread thread) throws RepositoryFunctionException, InterruptedException, EvalException { Map> authHeaders = getAuthHeaders(getAuthContents(auth, "auth")); @@ -911,6 +940,9 @@ public StructImpl downloadAndExtract( checksumValidation = e; } + Map renameFilesMap = + Dict.cast(renameFiles, String.class, String.class, "renameFiles"); + WorkspaceRuleEvent w = WorkspaceRuleEvent.newDownloadAndExtractEvent( urls, @@ -919,6 +951,7 @@ public StructImpl downloadAndExtract( integrity, type, stripPrefix, + renameFilesMap, rule.getLabel().toString(), thread.getCallerLocation()); @@ -977,6 +1010,7 @@ public StructImpl downloadAndExtract( .setArchivePath(downloadedPath) .setRepositoryPath(outputPath.getPath()) .setPrefix(stripPrefix) + .setRenameFiles(renameFilesMap) .build()); env.getListener().post(new ExtractProgress(outputPath.getPath().toString())); } diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/ArFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/ArFunctionTest.java index 96077f04a0be9b..4c481a5794b85a 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/ArFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/ArFunctionTest.java @@ -27,6 +27,7 @@ import com.google.devtools.build.runfiles.Runfiles; import java.io.File; import java.io.IOException; +import java.util.HashMap; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -62,6 +63,23 @@ public void testDecompress() throws Exception { assertThat(secondFile.isSymbolicLink()).isFalse(); } + /** + * Test decompressing an ar file, with some entries being renamed during the + * extraction process. + */ + @Test + public void testDecompressWithRenamedFiles() throws Exception { + HashMap renameFiles = new HashMap<>(); + renameFiles.put("archived_first.txt", "renamed_file.txt"); + DecompressorDescriptor.Builder descriptorBuilder = + createDescriptorBuilder().setRenameFiles(renameFiles); + Path outputDir = decompress(descriptorBuilder); + + assertThat(outputDir.exists()).isTrue(); + Path renamedFile = outputDir.getRelative("renamed_file.txt"); + assertThat(renamedFile.exists()).isTrue(); + } + private Path decompress(DecompressorDescriptor.Builder descriptorBuilder) throws Exception { descriptorBuilder.setDecompressor(ArFunction.INSTANCE); return new ArFunction().decompress(descriptorBuilder.build()); 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 index 28531368591ba9..2eba1421e6b6bc 100644 --- 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 @@ -14,6 +14,7 @@ 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; @@ -21,6 +22,7 @@ import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; +import java.util.HashMap; import java.util.zip.GZIPInputStream; import org.junit.Before; import org.junit.Test; @@ -64,6 +66,26 @@ public void testDecompressWithPrefix() throws Exception { archiveDescriptor.assertOutputFiles(outputDir, INNER_FOLDER_NAME); } + /** + * Test decompressing a tar.gz file, with some entries being renamed during the + * extraction process. + */ + @Test + public void testDecompressWithRenamedFiles() throws Exception { + String innerDirName = ROOT_FOLDER_NAME + "/" + INNER_FOLDER_NAME; + + HashMap renameFiles = new HashMap<>(); + renameFiles.put( + innerDirName + "/hardLinkFile", + innerDirName + "/renamedFile"); + DecompressorDescriptor.Builder descriptorBuilder = + archiveDescriptor.createDescriptorBuilder().setRenameFiles(renameFiles); + Path outputDir = decompress(descriptorBuilder); + + Path innerDir = outputDir.getRelative(ROOT_FOLDER_NAME).getRelative(INNER_FOLDER_NAME); + assertThat(innerDir.getRelative("renamedFile").exists()).isTrue(); + } + private Path decompress(DecompressorDescriptor.Builder descriptorBuilder) throws Exception { descriptorBuilder.setDecompressor(TarGzFunction.INSTANCE); return new CompressedTarFunction() { 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 88936a1218af5f..c318b1c4ad76ad 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 @@ -19,6 +19,7 @@ import static com.google.devtools.build.lib.bazel.repository.TestArchiveDescriptor.ROOT_FOLDER_NAME; import com.google.devtools.build.lib.vfs.Path; +import java.util.HashMap; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -68,6 +69,27 @@ public void testDecompressWithPrefix() throws Exception { archiveDescriptor.assertOutputFiles(outputDir, INNER_FOLDER_NAME); } + /** + * Test decompressing a zip file, with some entries being renamed during the + * extraction process. + */ + @Test + public void testDecompressWithRenamedFiles() throws Exception { + TestArchiveDescriptor archiveDescriptor = new TestArchiveDescriptor(ARCHIVE_NAME, "out", false); + String innerDirName = ROOT_FOLDER_NAME + "/" + INNER_FOLDER_NAME; + + HashMap renameFiles = new HashMap<>(); + renameFiles.put( + innerDirName + "/hardLinkFile", + innerDirName + "/renamedFile"); + DecompressorDescriptor.Builder descriptorBuilder = + archiveDescriptor.createDescriptorBuilder().setRenameFiles(renameFiles); + Path outputDir = decompress(descriptorBuilder); + + Path innerDir = outputDir.getRelative(ROOT_FOLDER_NAME).getRelative(INNER_FOLDER_NAME); + assertThat(innerDir.getRelative("renamedFile").exists()).isTrue(); + } + private Path decompress(DecompressorDescriptor.Builder descriptorBuilder) throws Exception { descriptorBuilder.setDecompressor(ZipDecompressor.INSTANCE); return ZipDecompressor.INSTANCE.decompress(descriptorBuilder.build()); diff --git a/src/test/shell/bazel/bazel_workspaces_test.sh b/src/test/shell/bazel/bazel_workspaces_test.sh index e739bcfc51d97b..cd1611d69560a1 100755 --- a/src/test/shell/bazel/bazel_workspaces_test.sh +++ b/src/test/shell/bazel/bazel_workspaces_test.sh @@ -417,6 +417,38 @@ function test_download_and_extract() { ensure_output_contains_exactly_once "external/repo/out_dir/download_and_extract.txt" "This is one file" } +function test_extract_rename_files() { + local archive_tar="${TEST_TMPDIR}/archive.tar" + + # Create a tar archive with two entries, which would have conflicting + # paths if extracted to a case-insensitive filesystem. + pushd "${TEST_TMPDIR}" + mkdir prefix + echo "First file: a" > prefix/a.txt + tar -cvf archive.tar prefix + rm prefix/a.txt + echo "Second file: A" > prefix/A.txt + tar -rvf archive.tar prefix + popd + + set_workspace_command " + repository_ctx.extract('${archive_tar}', 'out_dir', 'prefix/', renameFiles={ + 'prefix/A.txt': 'prefix/renamed-A.txt', + })" + + build_and_process_log --exclude_rule "//external:local_config_cc" + + ensure_contains_exactly 'location: .*repos.bzl:3:25' 1 + ensure_contains_atleast 'rule: "//external:repo"' 2 + ensure_contains_exactly 'extract_event' 1 + ensure_contains_exactly 'rename_files' 1 + ensure_contains_exactly 'key: "prefix/A.txt"' 1 + ensure_contains_exactly 'value: "prefix/renamed.A.txt"' 1 + + ensure_output_contains_exactly_once "external/repo/out_dir/a.txt" "First file: a" + ensure_output_contains_exactly_once "external/repo/out_dir/renamed-A.txt" "Second file: A" +} + function test_file() { set_workspace_command 'repository_ctx.file("filefile.sh", "echo filefile", True)'