Merge changes from topic "merge_zips_strip"

* changes:
  Make merge_zips -stripFile use blueprint style globs
  Add tests for merge_zips

cmd/merge_zips/Android.bp

@@ -16,10 +16,14 @@ blueprint_go_binary {
     name: "merge_zips",
     deps: [
       "android-archive-zip",
+      "blueprint-pathtools",
       "soong-jar",
     ],
     srcs: [
         "merge_zips.go",
     ],
+    testSrcs: [
+        "merge_zips_test.go",
+    ],
 }
 

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")
 }
 
@@ -114,7 +115,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 +133,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 +142,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 +225,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,9 +340,13 @@ 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 {
+				if skip, err := shouldStripEntry(emulateJar, stripFiles, stripDirs, file.Name); err != nil {
+					return err
+				} else if skip {
 					continue
 				}
+			}
 
 			if stripDirEntries && file.FileInfo().IsDir() {
 				continue
@@ -419,26 +425,41 @@ func pathBeforeLastSlash(path string) string {
 	return ret
 }
 
-func shouldStripFile(emulateJar bool, 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
 		}
-			} else {
-				return true
-			}
-		}
-	}
-	for _, pattern := range stripFiles {
-		if match, err := filepath.Match(pattern, filepath.Base(name)); err != nil {
-			panic(fmt.Errorf("%s: %s", err.Error(), pattern))
+
+		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 {
-			return true
+				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
 					}
 				}
-	return false
+				return true, nil
+			}
+		}
+	}
+
+	for _, pattern := range stripFiles {
+		if match, err := pathtools.Match(pattern, name); err != nil {
+			return false, fmt.Errorf("%s: %s", err.Error(), pattern)
+		} else if match {
+			return true, nil
+		}
+	}
+	return false, nil
 }
 
 func jarSort(files []fileMapping) {

cmd/merge_zips/merge_zips_test.go

@@ -0,0 +1,298 @@
+// 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 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"},
+		},
+		{
+			name: "strip dirs",
+			in: [][]testZipEntry{
+				{a, bDir, bbDir, bbb, bc, bd, be},
+			},
+			out: []testZipEntry{a},
+
+			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{
+				{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
+}

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}",

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)