Rearrange manifest file handling in merge_zips and soong_zip

Jar always puts default MANIFEST.MF files in if none was specified.
Copying that behavior in soong_zip causes problems with merge_zips,
because it ends up taking the default manifest from the classes.jar
instead of the user's manifest from res.jar.  We don't want the
user's manifest in the classes.jar, otherwise a change to the
manifest will cause all the class files to rebuild.  Instead,
move the manifest insertion to the final merge_zips stage.

Test: m -j checkbuild
Change-Id: Id6376961dbaf743c2fb92843f9bdf2e44b963be0

cmd/merge_zips/merge_zips.go

@@ -15,8 +15,10 @@
 package main
 
 import (
+	"errors"
 	"flag"
 	"fmt"
+	"hash/crc32"
 	"log"
 	"os"
 	"path/filepath"
@@ -52,10 +54,12 @@ func (s *zipToNotStrip) Set(zip_path string) error {
 }
 
 var (
-	sortEntries    = flag.Bool("s", false, "sort entries (defaults to the order from the input zip files)")
+	sortEntries     = flag.Bool("s", false, "sort entries (defaults to the order from the input zip files)")
-	emulateJar     = flag.Bool("j", false, "sort zip entries using jar ordering (META-INF first)")
+	emulateJar      = flag.Bool("j", false, "sort zip entries using jar ordering (META-INF first)")
-	stripDirs      []string
+	stripDirs       []string
-	zipsToNotStrip = make(map[string]bool)
+	zipsToNotStrip  = make(map[string]bool)
+	stripDirEntries = flag.Bool("D", false, "strip directory entries from the output zip file")
+	manifest        = flag.String("m", "", "manifest file to insert in jar")
 )
 
 func init() {
@@ -65,7 +69,7 @@ func init() {
 
 func main() {
 	flag.Usage = func() {
-		fmt.Fprintln(os.Stderr, "usage: merge_zips [-j] output [inputs...]")
+		fmt.Fprintln(os.Stderr, "usage: merge_zips [-jsD] [-m manifest] output [inputs...]")
 		flag.PrintDefaults()
 	}
 
@@ -107,8 +111,13 @@ func main() {
 		readers = append(readers, namedReader)
 	}
 
+	if *manifest != "" && !*emulateJar {
+		log.Fatal(errors.New("must specify -j when specifying a manifest via -m"))
+	}
+
 	// do merge
-	if err := mergeZips(readers, writer, *sortEntries, *emulateJar); err != nil {
+	err = mergeZips(readers, writer, *manifest, *sortEntries, *emulateJar, *stripDirEntries)
+	if err != nil {
 		log.Fatal(err)
 	}
 }
@@ -129,23 +138,109 @@ func (p zipEntryPath) String() string {
 	return p.zipName + "/" + p.entryName
 }
 
-// a zipEntry knows the location and content of a file within a zip
+// a zipEntry is a zipSource that pulls its content from another zip
 type zipEntry struct {
 	path    zipEntryPath
 	content *zip.File
 }
 
-// a fileMapping specifies to copy a zip entry from one place to another
+func (ze zipEntry) String() string {
-type fileMapping struct {
+	return ze.path.String()
-	source zipEntry
-	dest   string
 }
 
-func mergeZips(readers []namedZipReader, writer *zip.Writer, sortEntries bool, emulateJar bool) error {
+func (ze zipEntry) IsDir() bool {
+	return ze.content.FileInfo().IsDir()
+}
 
-	mappingsByDest := make(map[string]fileMapping, 0)
+func (ze zipEntry) CRC32() uint32 {
+	return ze.content.FileHeader.CRC32
+}
+
+func (ze zipEntry) WriteToZip(dest string, zw *zip.Writer) error {
+	return zw.CopyFrom(ze.content, dest)
+}
+
+// a bufferEntry is a zipSource that pulls its content from a []byte
+type bufferEntry struct {
+	fh      *zip.FileHeader
+	content []byte
+}
+
+func (be bufferEntry) String() string {
+	return "internal buffer"
+}
+
+func (be bufferEntry) IsDir() bool {
+	return be.fh.FileInfo().IsDir()
+}
+
+func (be bufferEntry) CRC32() uint32 {
+	return crc32.ChecksumIEEE(be.content)
+}
+
+func (be bufferEntry) WriteToZip(dest string, zw *zip.Writer) error {
+	w, err := zw.CreateHeader(be.fh)
+	if err != nil {
+		return err
+	}
+
+	if !be.IsDir() {
+		_, err = w.Write(be.content)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+type zipSource interface {
+	String() string
+	IsDir() bool
+	CRC32() uint32
+	WriteToZip(dest string, zw *zip.Writer) error
+}
+
+// a fileMapping specifies to copy a zip entry from one place to another
+type fileMapping struct {
+	dest   string
+	source zipSource
+}
+
+func mergeZips(readers []namedZipReader, writer *zip.Writer, manifest string,
+	sortEntries, emulateJar, stripDirEntries bool) error {
+
+	sourceByDest := make(map[string]zipSource, 0)
 	orderedMappings := []fileMapping{}
 
+	// if dest already exists returns a non-null zipSource for the existing source
+	addMapping := func(dest string, source zipSource) zipSource {
+		mapKey := filepath.Clean(dest)
+		if existingSource, exists := sourceByDest[mapKey]; exists {
+			return existingSource
+		}
+
+		sourceByDest[mapKey] = source
+		orderedMappings = append(orderedMappings, fileMapping{source: source, dest: dest})
+		return nil
+	}
+
+	if manifest != "" {
+		if !stripDirEntries {
+			dirHeader := jar.MetaDirFileHeader()
+			dirSource := bufferEntry{dirHeader, nil}
+			addMapping(jar.MetaDir, dirSource)
+		}
+
+		fh, buf, err := jar.ManifestFileContents(manifest)
+		if err != nil {
+			return err
+		}
+
+		fileSource := bufferEntry{fh, buf}
+		addMapping(jar.ManifestFile, fileSource)
+	}
+
 	for _, namedReader := range readers {
 		_, skipStripThisZip := zipsToNotStrip[namedReader.path]
 	FileLoop:
@@ -163,46 +258,39 @@ func mergeZips(readers []namedZipReader, writer *zip.Writer, sortEntries bool, e
 					}
 				}
 			}
+
+			if stripDirEntries && file.FileInfo().IsDir() {
+				continue
+			}
+
 			// check for other files or directories destined for the same path
 			dest := file.Name
-			mapKey := dest
-			if strings.HasSuffix(mapKey, "/") {
-				mapKey = mapKey[:len(mapKey)-1]
-			}
-			existingMapping, exists := mappingsByDest[mapKey]
 
 			// make a new entry to add
 			source := zipEntry{path: zipEntryPath{zipName: namedReader.path, entryName: file.Name}, content: file}
-			newMapping := fileMapping{source: source, dest: dest}
 
-			if exists {
+			if existingSource := addMapping(dest, source); existingSource != nil {
 				// handle duplicates
-				wasDir := existingMapping.source.content.FileHeader.FileInfo().IsDir()
+				if existingSource.IsDir() != source.IsDir() {
-				isDir := newMapping.source.content.FileHeader.FileInfo().IsDir()
-				if wasDir != isDir {
 					return fmt.Errorf("Directory/file mismatch at %v from %v and %v\n",
-						dest, existingMapping.source.path, newMapping.source.path)
+						dest, existingSource, source)
 				}
 				if emulateJar &&
 					file.Name == jar.ManifestFile || file.Name == jar.ModuleInfoClass {
 					// Skip manifest and module info files that are not from the first input file
 					continue
 				}
-				if !isDir {
+				if !source.IsDir() {
 					if emulateJar {
-						if existingMapping.source.content.CRC32 != newMapping.source.content.CRC32 {
+						if existingSource.CRC32() != source.CRC32() {
 							fmt.Fprintf(os.Stdout, "WARNING: Duplicate path %v found in %v and %v\n",
-								dest, existingMapping.source.path, newMapping.source.path)
+								dest, existingSource, source)
 						}
 					} else {
 						return fmt.Errorf("Duplicate path %v found in %v and %v\n",
-							dest, existingMapping.source.path, newMapping.source.path)
+							dest, existingSource, source)
 					}
 				}
-			} else {
-				// save entry
-				mappingsByDest[mapKey] = newMapping
-				orderedMappings = append(orderedMappings, newMapping)
 			}
 		}
 	}
@@ -214,7 +302,7 @@ func mergeZips(readers []namedZipReader, writer *zip.Writer, sortEntries bool, e
 	}
 
 	for _, entry := range orderedMappings {
-		if err := writer.CopyFrom(entry.source.content, entry.dest); err != nil {
+		if err := entry.source.WriteToZip(entry.dest, writer); err != nil {
 			return err
 		}
 	}

cmd/soong_zip/soong_zip.go

@@ -58,7 +58,7 @@ func (nopCloser) Close() error {
 }
 
 type byteReaderCloser struct {
-	bytes.Reader
+	*bytes.Reader
 	io.Closer
 }
 
@@ -252,7 +252,7 @@ func main() {
 	}
 
 	w := &zipWriter{
-		time:         time.Date(2009, 1, 1, 0, 0, 0, 0, time.UTC),
+		time:         jar.DefaultTime,
 		createdDirs:  make(map[string]string),
 		createdFiles: make(map[string]string),
 		directories:  *directories,
@@ -353,14 +353,14 @@ func (z *zipWriter) write(out string, pathMappings []pathMapping, manifest strin
 		z.memoryRateLimiter.Stop()
 	}()
 
-	if manifest != "" {
+	if manifest != "" && !*emulateJar {
-		if !*emulateJar {
+		return errors.New("must specify --jar when specifying a manifest via -m")
-			return errors.New("must specify --jar when specifying a manifest via -m")
-		}
-		pathMappings = append(pathMappings, pathMapping{jar.ManifestFile, manifest, zip.Deflate})
 	}
 
 	if *emulateJar {
+		// manifest may be empty, in which case addManifest will fill in a default
+		pathMappings = append(pathMappings, pathMapping{jar.ManifestFile, manifest, zip.Deflate})
+
 		jarSort(pathMappings)
 	}
 
@@ -524,11 +524,6 @@ func (z *zipWriter) addFile(dest, src string, method uint16) error {
 }
 
 func (z *zipWriter) addManifest(dest string, src string, method uint16) error {
-	givenBytes, err := ioutil.ReadFile(src)
-	if err != nil {
-		return err
-	}
-
 	if prev, exists := z.createdDirs[dest]; exists {
 		return fmt.Errorf("destination %q is both a directory %q and a file %q", dest, prev, src)
 	}
@@ -536,27 +531,18 @@ func (z *zipWriter) addManifest(dest string, src string, method uint16) error {
 		return fmt.Errorf("destination %q has two files %q and %q", dest, prev, src)
 	}
 
-	manifestMarker := []byte("Manifest-Version:")
+	if err := z.writeDirectory(filepath.Dir(dest), src); err != nil {
-	header := append(manifestMarker, []byte(" 1.0\nCreated-By: soong_zip\n")...)
+		return err
-
-	var finalBytes []byte
-	if !bytes.Contains(givenBytes, manifestMarker) {
-		finalBytes = append(append(header, givenBytes...), byte('\n'))
-	} else {
-		finalBytes = givenBytes
 	}
 
-	byteReader := bytes.NewReader(finalBytes)
+	fh, buf, err := jar.ManifestFileContents(src)
-
+	if err != nil {
-	reader := &byteReaderCloser{*byteReader, ioutil.NopCloser(nil)}
+		return err
-
-	fileHeader := &zip.FileHeader{
-		Name:               dest,
-		Method:             zip.Store,
-		UncompressedSize64: uint64(byteReader.Len()),
 	}
 
-	return z.writeFileContents(fileHeader, reader)
+	reader := &byteReaderCloser{bytes.NewReader(buf), ioutil.NopCloser(nil)}
+
+	return z.writeFileContents(fh, reader)
 }
 
 func (z *zipWriter) writeFileContents(header *zip.FileHeader, r readerSeekerCloser) (err error) {
@@ -770,19 +756,6 @@ func (z *zipWriter) compressWholeFile(ze *zipEntry, r io.ReadSeeker, compressCha
 	close(compressChan)
 }
 
-func (z *zipWriter) addExtraField(zipHeader *zip.FileHeader, fieldHeader [2]byte, data []byte) {
-	// add the field header in little-endian order
-	zipHeader.Extra = append(zipHeader.Extra, fieldHeader[1], fieldHeader[0])
-
-	// specify the length of the data (in little-endian order)
-	dataLength := len(data)
-	lengthBytes := []byte{byte(dataLength % 256), byte(dataLength / 256)}
-	zipHeader.Extra = append(zipHeader.Extra, lengthBytes...)
-
-	// add the contents of the extra field
-	zipHeader.Extra = append(zipHeader.Extra, data...)
-}
-
 // writeDirectory annotates that dir is a directory created for the src file or directory, and adds
 // the directory entry to the zip file if directories are enabled.
 func (z *zipWriter) writeDirectory(dir, src string) error {
@@ -810,17 +783,19 @@ func (z *zipWriter) writeDirectory(dir, src string) error {
 	if z.directories {
 		// make a directory entry for each uncreated directory
 		for _, cleanDir := range zipDirs {
-			dirHeader := &zip.FileHeader{
+			var dirHeader *zip.FileHeader
-				Name: cleanDir + "/",
-			}
-			dirHeader.SetMode(0700 | os.ModeDir)
-			dirHeader.SetModTime(z.time)
 
-			if *emulateJar && dir == "META-INF/" {
+			if *emulateJar && cleanDir+"/" == jar.MetaDir {
-				// Jar files have a 0-length extra field with header "CAFE"
+				dirHeader = jar.MetaDirFileHeader()
-				z.addExtraField(dirHeader, [2]byte{0xca, 0xfe}, []byte{})
+			} else {
+				dirHeader = &zip.FileHeader{
+					Name: cleanDir + "/",
+				}
+				dirHeader.SetMode(0700 | os.ModeDir)
 			}
 
+			dirHeader.SetModTime(z.time)
+
 			ze := make(chan *zipEntry, 1)
 			ze <- &zipEntry{
 				fh: dirHeader,

jar/Android.bp

@@ -18,5 +18,8 @@ bootstrap_go_package {
     srcs: [
         "jar.go",
     ],
+    deps: [
+        "android-archive-zip",
+    ],
 }
 

jar/jar.go

@@ -15,8 +15,14 @@
 package jar
 
 import (
+	"bytes"
 	"fmt"
+	"io/ioutil"
+	"os"
 	"strings"
+	"time"
+
+	"android/soong/third_party/zip"
 )
 
 const (
@@ -25,6 +31,10 @@ const (
 	ModuleInfoClass = "module-info.class"
 )
 
+var DefaultTime = time.Date(2009, 1, 1, 0, 0, 0, 0, time.UTC)
+
+var MetaDirExtra = [2]byte{0xca, 0xfe}
+
 // EntryNamesLess tells whether <filepathA> should precede <filepathB> in
 // the order of files with a .jar
 func EntryNamesLess(filepathA string, filepathB string) (less bool) {
@@ -59,3 +69,56 @@ func index(name string) int {
 	}
 	panic(fmt.Errorf("file %q did not match any pattern", name))
 }
+
+func MetaDirFileHeader() *zip.FileHeader {
+	dirHeader := &zip.FileHeader{
+		Name:  MetaDir,
+		Extra: []byte{MetaDirExtra[1], MetaDirExtra[0], 0, 0},
+	}
+	dirHeader.SetMode(0700 | os.ModeDir)
+	dirHeader.SetModTime(DefaultTime)
+
+	return dirHeader
+}
+
+// Convert manifest source path to zip header and contents.  If path is empty uses a default
+// manifest.
+func ManifestFileContents(src string) (*zip.FileHeader, []byte, error) {
+	b, err := manifestContents(src)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	fh := &zip.FileHeader{
+		Name:               ManifestFile,
+		Method:             zip.Store,
+		UncompressedSize64: uint64(len(b)),
+	}
+
+	return fh, b, nil
+}
+
+// Convert manifest source path to contents.  If path is empty uses a default manifest.
+func manifestContents(src string) ([]byte, error) {
+	var givenBytes []byte
+	var err error
+
+	if src != "" {
+		givenBytes, err = ioutil.ReadFile(src)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	manifestMarker := []byte("Manifest-Version:")
+	header := append(manifestMarker, []byte(" 1.0\nCreated-By: soong_zip\n")...)
+
+	var finalBytes []byte
+	if !bytes.Contains(givenBytes, manifestMarker) {
+		finalBytes = append(append(header, givenBytes...), byte('\n'))
+	} else {
+		finalBytes = givenBytes
+	}
+
+	return finalBytes, nil
+}

java/builder.go

@@ -78,10 +78,10 @@ var (
 
 	combineJar = pctx.AndroidStaticRule("combineJar",
 		blueprint.RuleParams{
-			Command:     `${config.MergeZipsCmd} -j $out $in`,
+			Command:     `${config.MergeZipsCmd} -j $jarArgs $out $in`,
 			CommandDeps: []string{"${config.MergeZipsCmd}"},
 		},
-		"outDir")
+		"jarArgs")
 
 	dx = pctx.AndroidStaticRule("dx",
 		blueprint.RuleParams{
@@ -126,7 +126,7 @@ func TransformJavaToClasses(ctx android.ModuleContext, srcFiles, srcFileLists an
 
 	classDir := android.PathForModuleOut(ctx, "classes")
 	annoDir := android.PathForModuleOut(ctx, "anno")
-	classJar := android.PathForModuleOut(ctx, "classes.jar")
+	classJar := android.PathForModuleOut(ctx, "classes-compiled.jar")
 
 	javacFlags := flags.javacFlags + android.JoinWithPrefix(srcFileLists.Strings(), "@")
 
@@ -187,7 +187,7 @@ func RunErrorProne(ctx android.ModuleContext, srcFiles android.Paths, srcFileLis
 }
 
 func TransformResourcesToJar(ctx android.ModuleContext, resources []jarSpec,
-	manifest android.OptionalPath, deps android.Paths) android.Path {
+	deps android.Paths) android.Path {
 
 	outputFile := android.PathForModuleOut(ctx, "res.jar")
 
@@ -198,11 +198,6 @@ func TransformResourcesToJar(ctx android.ModuleContext, resources []jarSpec,
 		jarArgs = append(jarArgs, j.soongJarArgs())
 	}
 
-	if manifest.Valid() {
-		deps = append(deps, manifest.Path())
-		jarArgs = append(jarArgs, "-m "+manifest.String())
-	}
-
 	ctx.ModuleBuild(pctx, android.ModuleBuildParams{
 		Rule:        jar,
 		Description: "jar",
@@ -216,19 +211,36 @@ func TransformResourcesToJar(ctx android.ModuleContext, resources []jarSpec,
 	return outputFile
 }
 
-func TransformJarsToJar(ctx android.ModuleContext, stem string, jars android.Paths) android.Path {
+func TransformJarsToJar(ctx android.ModuleContext, stem string, jars android.Paths,
+	manifest android.OptionalPath, stripDirs bool) android.Path {
 
 	outputFile := android.PathForModuleOut(ctx, stem)
 
-	if len(jars) == 1 {
+	if len(jars) == 1 && !manifest.Valid() {
 		return jars[0]
 	}
 
+	var deps android.Paths
+
+	var jarArgs []string
+	if manifest.Valid() {
+		jarArgs = append(jarArgs, "-m "+manifest.String())
+		deps = append(deps, manifest.Path())
+	}
+
+	if stripDirs {
+		jarArgs = append(jarArgs, "-D")
+	}
+
 	ctx.ModuleBuild(pctx, android.ModuleBuildParams{
 		Rule:        combineJar,
 		Description: "combine jars",
 		Output:      outputFile,
 		Inputs:      jars,
+		Implicits:   deps,
+		Args: map[string]string{
+			"jarArgs": strings.Join(jarArgs, " "),
+		},
 	})
 
 	return outputFile

java/java.go

@@ -385,11 +385,9 @@ func (j *Module) compile(ctx android.ModuleContext) {
 	}
 
 	resourceJarSpecs := ResourceDirsToJarSpecs(ctx, j.properties.Resource_dirs, j.properties.Exclude_resource_dirs)
-	manifest := android.OptionalPathForModuleSrc(ctx, j.properties.Manifest)
+	if len(resourceJarSpecs) > 0 {
-
-	if len(resourceJarSpecs) > 0 || manifest.Valid() {
 		// Combine classes + resources into classes-full-debug.jar
-		resourceJar := TransformResourcesToJar(ctx, resourceJarSpecs, manifest, extraJarDeps)
+		resourceJar := TransformResourcesToJar(ctx, resourceJarSpecs, extraJarDeps)
 		if ctx.Failed() {
 			return
 		}
@@ -399,9 +397,11 @@ func (j *Module) compile(ctx android.ModuleContext) {
 
 	jars = append(jars, deps.staticJars...)
 
+	manifest := android.OptionalPathForModuleSrc(ctx, j.properties.Manifest)
+
 	// Combine the classes built from sources, any manifests, and any static libraries into
 	// classes-combined.jar.  If there is only one input jar this step will be skipped.
-	outputFile := TransformJarsToJar(ctx, "classes-combined.jar", jars)
+	outputFile := TransformJarsToJar(ctx, "classes.jar", jars, manifest, false)
 
 	if j.properties.Jarjar_rules != nil {
 		jarjar_rules := android.PathForModuleSrc(ctx, *j.properties.Jarjar_rules)
@@ -616,7 +616,7 @@ func (j *Import) DepsMutator(ctx android.BottomUpMutatorContext) {
 func (j *Import) GenerateAndroidBuildActions(ctx android.ModuleContext) {
 	j.classpathFiles = android.PathsForModuleSrc(ctx, j.properties.Jars)
 
-	j.combinedClasspathFile = TransformJarsToJar(ctx, "classes.jar", j.classpathFiles)
+	j.combinedClasspathFile = TransformJarsToJar(ctx, "classes.jar", j.classpathFiles, android.OptionalPath{}, false)
 }
 
 var _ Dependency = (*Import)(nil)

java/java_test.go

@@ -119,8 +119,8 @@ func TestSimple(t *testing.T) {
 		t.Errorf(`foo inputs %v != ["a.java"]`, javac.Inputs)
 	}
 
-	bar := filepath.Join(buildDir, ".intermediates", "bar", "classes.jar")
+	bar := filepath.Join(buildDir, ".intermediates", "bar", "classes-compiled.jar")
-	baz := filepath.Join(buildDir, ".intermediates", "baz", "classes.jar")
+	baz := filepath.Join(buildDir, ".intermediates", "baz", "classes-compiled.jar")
 
 	if !strings.Contains(javac.Args["classpath"], bar) {
 		t.Errorf("foo classpath %v does not contain %q", javac.Args["classpath"], bar)
@@ -182,7 +182,7 @@ func TestSdk(t *testing.T) {
 
 	check := func(module string, depType depType, deps ...string) {
 		for i := range deps {
-			deps[i] = filepath.Join(buildDir, ".intermediates", deps[i], "classes.jar")
+			deps[i] = filepath.Join(buildDir, ".intermediates", deps[i], "classes-compiled.jar")
 		}
 		dep := strings.Join(deps, ":")
 
@@ -279,12 +279,12 @@ func TestDefaults(t *testing.T) {
 		t.Errorf(`foo inputs %v != ["a.java"]`, javac.Inputs)
 	}
 
-	bar := filepath.Join(buildDir, ".intermediates", "bar", "classes.jar")
+	bar := filepath.Join(buildDir, ".intermediates", "bar", "classes-compiled.jar")
 	if !strings.Contains(javac.Args["classpath"], bar) {
 		t.Errorf("foo classpath %v does not contain %q", javac.Args["classpath"], bar)
 	}
 
-	baz := filepath.Join(buildDir, ".intermediates", "baz", "classes.jar")
+	baz := filepath.Join(buildDir, ".intermediates", "baz", "classes-compiled.jar")
 	if len(combineJar.Inputs) != 2 || combineJar.Inputs[1].String() != baz {
 		t.Errorf("foo combineJar inputs %v does not contain %q", combineJar.Inputs, baz)
 	}