From 2486065c435bbdcd7cdc6052036616643a8049a2 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Sat, 14 Jul 2018 22:19:14 -0700 Subject: [PATCH 1/2] Add tests for merge_zips Test: merge_zip_test.go Change-Id: I8fc577946b40cad193864aa2ebda1bef3a0e050e --- cmd/merge_zips/Android.bp | 3 + cmd/merge_zips/merge_zips.go | 13 +- cmd/merge_zips/merge_zips_test.go | 269 ++++++++++++++++++++++++++++++ 3 files changed, 279 insertions(+), 6 deletions(-) create mode 100644 cmd/merge_zips/merge_zips_test.go diff --git a/cmd/merge_zips/Android.bp b/cmd/merge_zips/Android.bp index ace079dc1..03a5f3bea 100644 --- a/cmd/merge_zips/Android.bp +++ b/cmd/merge_zips/Android.bp @@ -21,5 +21,8 @@ blueprint_go_binary { srcs: [ "merge_zips.go", ], + testSrcs: [ + "merge_zips_test.go", + ], } diff --git a/cmd/merge_zips/merge_zips.go b/cmd/merge_zips/merge_zips.go index 105765521..84eace608 100644 --- a/cmd/merge_zips/merge_zips.go +++ b/cmd/merge_zips/merge_zips.go @@ -114,7 +114,7 @@ func main() { log.Fatal(err) } defer reader.Close() - namedReader := namedZipReader{path: input, reader: reader} + namedReader := namedZipReader{path: input, reader: &reader.Reader} readers = append(readers, namedReader) } @@ -132,7 +132,7 @@ func main() { // do merge err = mergeZips(readers, writer, *manifest, *entrypoint, *pyMain, *sortEntries, *emulateJar, *emulatePar, - *stripDirEntries, *ignoreDuplicates) + *stripDirEntries, *ignoreDuplicates, []string(stripFiles), []string(stripDirs), map[string]bool(zipsToNotStrip)) if err != nil { log.Fatal(err) } @@ -141,7 +141,7 @@ func main() { // a namedZipReader reads a .zip file and can say which file it's reading type namedZipReader struct { path string - reader *zip.ReadCloser + reader *zip.Reader } // a zipEntryPath refers to a file contained in a zip @@ -224,7 +224,8 @@ type fileMapping struct { } func mergeZips(readers []namedZipReader, writer *zip.Writer, manifest, entrypoint, pyMain string, - sortEntries, emulateJar, emulatePar, stripDirEntries, ignoreDuplicates bool) error { + sortEntries, emulateJar, emulatePar, stripDirEntries, ignoreDuplicates bool, + stripFiles, stripDirs []string, zipsToNotStrip map[string]bool) error { sourceByDest := make(map[string]zipSource, 0) orderedMappings := []fileMapping{} @@ -338,7 +339,7 @@ func mergeZips(readers []namedZipReader, writer *zip.Writer, manifest, entrypoin for _, namedReader := range readers { _, skipStripThisZip := zipsToNotStrip[namedReader.path] for _, file := range namedReader.reader.File { - if !skipStripThisZip && shouldStripFile(emulateJar, file.Name) { + if !skipStripThisZip && shouldStripFile(emulateJar, stripFiles, stripDirs, file.Name) { continue } @@ -419,7 +420,7 @@ func pathBeforeLastSlash(path string) string { return ret } -func shouldStripFile(emulateJar bool, name string) bool { +func shouldStripFile(emulateJar bool, stripFiles, stripDirs []string, name string) bool { for _, dir := range stripDirs { if strings.HasPrefix(name, dir+"/") { if emulateJar { diff --git a/cmd/merge_zips/merge_zips_test.go b/cmd/merge_zips/merge_zips_test.go new file mode 100644 index 000000000..21a7e9a03 --- /dev/null +++ b/cmd/merge_zips/merge_zips_test.go @@ -0,0 +1,269 @@ +// Copyright 2018 Google Inc. 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 main + +import ( + "bytes" + "fmt" + "os" + "strconv" + "strings" + "testing" + + "android/soong/jar" + "android/soong/third_party/zip" +) + +type testZipEntry struct { + name string + mode os.FileMode + data []byte +} + +var ( + A = testZipEntry{"A", 0755, []byte("foo")} + a = testZipEntry{"a", 0755, []byte("foo")} + a2 = testZipEntry{"a", 0755, []byte("FOO2")} + a3 = testZipEntry{"a", 0755, []byte("Foo3")} + bDir = testZipEntry{"b/", os.ModeDir | 0755, nil} + bbDir = testZipEntry{"b/b/", os.ModeDir | 0755, nil} + bbb = testZipEntry{"b/b/b", 0755, nil} + ba = testZipEntry{"b/a", 0755, []byte("foob")} + bc = testZipEntry{"b/c", 0755, []byte("bar")} + bd = testZipEntry{"b/d", 0700, []byte("baz")} + be = testZipEntry{"b/e", 0700, []byte("")} + + metainfDir = testZipEntry{jar.MetaDir, os.ModeDir | 0755, nil} + manifestFile = testZipEntry{jar.ManifestFile, 0755, []byte("manifest")} + manifestFile2 = testZipEntry{jar.ManifestFile, 0755, []byte("manifest2")} + moduleInfoFile = testZipEntry{jar.ModuleInfoClass, 0755, []byte("module-info")} +) + +func TestMergeZips(t *testing.T) { + testCases := []struct { + name string + in [][]testZipEntry + stripFiles []string + stripDirs []string + jar bool + sort bool + ignoreDuplicates bool + stripDirEntries bool + zipsToNotStrip map[string]bool + + out []testZipEntry + err string + }{ + { + name: "duplicates error", + in: [][]testZipEntry{ + {a}, + {a2}, + {a3}, + }, + out: []testZipEntry{a}, + err: "duplicate", + }, + { + name: "duplicates take first", + in: [][]testZipEntry{ + {a}, + {a2}, + {a3}, + }, + out: []testZipEntry{a}, + + ignoreDuplicates: true, + }, + { + name: "sort", + in: [][]testZipEntry{ + {be, bc, bDir, bbDir, bbb, A, metainfDir, manifestFile}, + }, + out: []testZipEntry{A, metainfDir, manifestFile, bDir, bbDir, bbb, bc, be}, + + sort: true, + }, + { + name: "jar sort", + in: [][]testZipEntry{ + {be, bc, bDir, A, metainfDir, manifestFile}, + }, + out: []testZipEntry{metainfDir, manifestFile, A, bDir, bc, be}, + + jar: true, + }, + { + name: "jar merge", + in: [][]testZipEntry{ + {metainfDir, manifestFile, bDir, be}, + {metainfDir, manifestFile2, bDir, bc}, + {metainfDir, manifestFile2, A}, + }, + out: []testZipEntry{metainfDir, manifestFile, A, bDir, bc, be}, + + jar: true, + }, + { + name: "merge", + in: [][]testZipEntry{ + {bDir, be}, + {bDir, bc}, + {A}, + }, + out: []testZipEntry{bDir, be, bc, A}, + }, + { + name: "strip dir entries", + in: [][]testZipEntry{ + {a, bDir, bbDir, bbb, bc, bd, be}, + }, + out: []testZipEntry{a, bbb, bc, bd, be}, + + stripDirEntries: true, + }, + { + name: "strip file name", + in: [][]testZipEntry{ + {a, bDir, ba}, + }, + out: []testZipEntry{bDir}, + + stripFiles: []string{"a"}, + }, + { + name: "strip dirs", + in: [][]testZipEntry{ + {a, bDir, bbDir, bbb, bc, bd, be}, + }, + out: []testZipEntry{a}, + + stripDirs: []string{"b"}, + }, + { + name: "zips to not strip", + in: [][]testZipEntry{ + {a, bDir, bc}, + {bDir, bd}, + {bDir, be}, + }, + out: []testZipEntry{a, bDir, bd}, + + stripDirs: []string{"b"}, + zipsToNotStrip: map[string]bool{ + "in1": true, + }, + }, + } + + for _, test := range testCases { + t.Run(test.name, func(t *testing.T) { + var readers []namedZipReader + for i, in := range test.in { + r := testZipEntriesToZipReader(in) + readers = append(readers, namedZipReader{ + path: "in" + strconv.Itoa(i), + reader: r, + }) + } + + want := testZipEntriesToBuf(test.out) + + out := &bytes.Buffer{} + writer := zip.NewWriter(out) + + err := mergeZips(readers, writer, "", "", "", + test.sort, test.jar, false, test.stripDirEntries, test.ignoreDuplicates, + test.stripFiles, test.stripDirs, test.zipsToNotStrip) + + closeErr := writer.Close() + if closeErr != nil { + t.Fatal(err) + } + + if test.err != "" { + if err == nil { + t.Fatal("missing err, expected: ", test.err) + } else if !strings.Contains(strings.ToLower(err.Error()), strings.ToLower(test.err)) { + t.Fatal("incorrect err, want:", test.err, "got:", err) + } + return + } + + if !bytes.Equal(want, out.Bytes()) { + t.Error("incorrect zip output") + t.Errorf("want:\n%s", dumpZip(want)) + t.Errorf("got:\n%s", dumpZip(out.Bytes())) + } + }) + } +} + +func testZipEntriesToBuf(entries []testZipEntry) []byte { + b := &bytes.Buffer{} + zw := zip.NewWriter(b) + + for _, e := range entries { + fh := zip.FileHeader{ + Name: e.name, + } + fh.SetMode(e.mode) + + w, err := zw.CreateHeader(&fh) + if err != nil { + panic(err) + } + + _, err = w.Write(e.data) + if err != nil { + panic(err) + } + } + + err := zw.Close() + if err != nil { + panic(err) + } + + return b.Bytes() +} + +func testZipEntriesToZipReader(entries []testZipEntry) *zip.Reader { + b := testZipEntriesToBuf(entries) + r := bytes.NewReader(b) + + zr, err := zip.NewReader(r, int64(len(b))) + if err != nil { + panic(err) + } + + return zr +} + +func dumpZip(buf []byte) string { + r := bytes.NewReader(buf) + zr, err := zip.NewReader(r, int64(len(buf))) + if err != nil { + panic(err) + } + + var ret string + + for _, f := range zr.File { + ret += fmt.Sprintf("%v: %v %v %08x\n", f.Name, f.Mode(), f.UncompressedSize64, f.CRC32) + } + + return ret +} From 4c03f6876357c01b2977fd61157aad9c7984b741 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Sun, 15 Jul 2018 08:16:31 -0700 Subject: [PATCH 2/2] Make merge_zips -stripFile use blueprint style globs merge_zips -stripFile was only considering the name of the file and ignoring the path. Make it more useful by supporting blueprint style globs. The previous behavior can be recreated by prefixing with **/. Bug: 111389216 Test: m checkbuild Change-Id: I25760fe3f1f77704dd9da9d107d9a38a415d681f --- cmd/merge_zips/Android.bp | 1 + cmd/merge_zips/merge_zips.go | 52 +++++++++++++++++++++---------- cmd/merge_zips/merge_zips_test.go | 31 +++++++++++++++++- java/dex.go | 4 +-- java/java.go | 4 ++- 5 files changed, 72 insertions(+), 20 deletions(-) diff --git a/cmd/merge_zips/Android.bp b/cmd/merge_zips/Android.bp index 03a5f3bea..ab658fd0d 100644 --- a/cmd/merge_zips/Android.bp +++ b/cmd/merge_zips/Android.bp @@ -16,6 +16,7 @@ blueprint_go_binary { name: "merge_zips", deps: [ "android-archive-zip", + "blueprint-pathtools", "soong-jar", ], srcs: [ diff --git a/cmd/merge_zips/merge_zips.go b/cmd/merge_zips/merge_zips.go index 84eace608..95ff70b06 100644 --- a/cmd/merge_zips/merge_zips.go +++ b/cmd/merge_zips/merge_zips.go @@ -24,7 +24,8 @@ import ( "os" "path/filepath" "sort" - "strings" + + "github.com/google/blueprint/pathtools" "android/soong/jar" "android/soong/third_party/zip" @@ -69,8 +70,8 @@ var ( ) func init() { - flag.Var(&stripDirs, "stripDir", "the prefix of file path to be excluded from the output zip") - flag.Var(&stripFiles, "stripFile", "filenames to be excluded from the output zip, accepts wildcards") + flag.Var(&stripDirs, "stripDir", "directories to be excluded from the output zip, accepts wildcards") + flag.Var(&stripFiles, "stripFile", "files to be excluded from the output zip, accepts wildcards") flag.Var(&zipsToNotStrip, "zipToNotStrip", "the input zip file which is not applicable for stripping") } @@ -339,8 +340,12 @@ func mergeZips(readers []namedZipReader, writer *zip.Writer, manifest, entrypoin for _, namedReader := range readers { _, skipStripThisZip := zipsToNotStrip[namedReader.path] for _, file := range namedReader.reader.File { - if !skipStripThisZip && shouldStripFile(emulateJar, stripFiles, stripDirs, file.Name) { - continue + if !skipStripThisZip { + if skip, err := shouldStripEntry(emulateJar, stripFiles, stripDirs, file.Name); err != nil { + return err + } else if skip { + continue + } } if stripDirEntries && file.FileInfo().IsDir() { @@ -420,26 +425,41 @@ func pathBeforeLastSlash(path string) string { return ret } -func shouldStripFile(emulateJar bool, stripFiles, stripDirs []string, name string) bool { +func shouldStripEntry(emulateJar bool, stripFiles, stripDirs []string, name string) (bool, error) { for _, dir := range stripDirs { - if strings.HasPrefix(name, dir+"/") { - if emulateJar { - if name != jar.MetaDir && name != jar.ManifestFile { - return true + dir = filepath.Clean(dir) + patterns := []string{ + dir + "/", // the directory itself + dir + "/**/*", // files recursively in the directory + dir + "/**/*/", // directories recursively in the directory + } + + for _, pattern := range patterns { + match, err := pathtools.Match(pattern, name) + if err != nil { + return false, fmt.Errorf("%s: %s", err.Error(), pattern) + } else if match { + if emulateJar { + // When merging jar files, don't strip META-INF/MANIFEST.MF even if stripping META-INF is + // requested. + // TODO(ccross): which files does this affect? + if name != jar.MetaDir && name != jar.ManifestFile { + return true, nil + } } - } else { - return true + return true, nil } } } + for _, pattern := range stripFiles { - if match, err := filepath.Match(pattern, filepath.Base(name)); err != nil { - panic(fmt.Errorf("%s: %s", err.Error(), pattern)) + if match, err := pathtools.Match(pattern, name); err != nil { + return false, fmt.Errorf("%s: %s", err.Error(), pattern) } else if match { - return true + return true, nil } } - return false + return false, nil } func jarSort(files []fileMapping) { diff --git a/cmd/merge_zips/merge_zips_test.go b/cmd/merge_zips/merge_zips_test.go index 21a7e9a03..f91111f14 100644 --- a/cmd/merge_zips/merge_zips_test.go +++ b/cmd/merge_zips/merge_zips_test.go @@ -135,13 +135,33 @@ func TestMergeZips(t *testing.T) { stripDirEntries: true, }, { + name: "strip files", + in: [][]testZipEntry{ + {a, bDir, bbDir, bbb, bc, bd, be}, + }, + out: []testZipEntry{a, bDir, bbDir, bbb, bc}, + + stripFiles: []string{"b/d", "b/e"}, + }, + { + // merge_zips used to treat -stripFile a as stripping any file named a, it now only strips a in the + // root of the zip. name: "strip file name", in: [][]testZipEntry{ {a, bDir, ba}, }, + out: []testZipEntry{bDir, ba}, + + stripFiles: []string{"a"}, + }, + { + name: "strip files glob", + in: [][]testZipEntry{ + {a, bDir, ba}, + }, out: []testZipEntry{bDir}, - stripFiles: []string{"a"}, + stripFiles: []string{"**/a"}, }, { name: "strip dirs", @@ -152,6 +172,15 @@ func TestMergeZips(t *testing.T) { stripDirs: []string{"b"}, }, + { + name: "strip dirs glob", + in: [][]testZipEntry{ + {a, bDir, bbDir, bbb, bc, bd, be}, + }, + out: []testZipEntry{a, bDir, bc, bd, be}, + + stripDirs: []string{"b/*"}, + }, { name: "zips to not strip", in: [][]testZipEntry{ diff --git a/java/dex.go b/java/dex.go index 06ee272de..db240fc53 100644 --- a/java/dex.go +++ b/java/dex.go @@ -27,7 +27,7 @@ var d8 = pctx.AndroidStaticRule("d8", Command: `rm -rf "$outDir" && mkdir -p "$outDir" && ` + `${config.D8Cmd} --output $outDir $dxFlags $in && ` + `${config.SoongZipCmd} -o $outDir/classes.dex.jar -C $outDir -D $outDir && ` + - `${config.MergeZipsCmd} -D -stripFile "*.class" $out $outDir/classes.dex.jar $in`, + `${config.MergeZipsCmd} -D -stripFile "**/*.class" $out $outDir/classes.dex.jar $in`, CommandDeps: []string{ "${config.D8Cmd}", "${config.SoongZipCmd}", @@ -44,7 +44,7 @@ var r8 = pctx.AndroidStaticRule("r8", `-printmapping $outDict ` + `$dxFlags $r8Flags && ` + `${config.SoongZipCmd} -o $outDir/classes.dex.jar -C $outDir -D $outDir && ` + - `${config.MergeZipsCmd} -D -stripFile "*.class" $out $outDir/classes.dex.jar $in`, + `${config.MergeZipsCmd} -D -stripFile "**/*.class" $out $outDir/classes.dex.jar $in`, CommandDeps: []string{ "${config.R8Cmd}", "${config.SoongZipCmd}", diff --git a/java/java.go b/java/java.go index 5cbd8bacc..89aa0e14f 100644 --- a/java/java.go +++ b/java/java.go @@ -1032,7 +1032,9 @@ func (j *Module) compile(ctx android.ModuleContext, extraSrcJars ...android.Path if Bool(j.properties.Renamed_kotlin_stdlib) { // Remove any kotlin-reflect related files // TODO(pszczepaniak): Support kotlin-reflect - stripFiles = append(stripFiles, "*.kotlin_module", "*.kotlin_builtin") + stripFiles = append(stripFiles, + "**/*.kotlin_module", + "**/*.kotlin_builtin") } else { // Only add kotlin-stdlib if not using (on-device) renamed stdlib // (it's expected to be on device bootclasspath)