Remove --sort_entries from soong_zip

We can just always sort the entries and not worry about passing the
flag or not.

Bug: 395160816
Test: Presubmits
Change-Id: Ic1a8b8ef9c74368d1aed77a228c52ccc08b3375f

3 files changed, 76 insertions, 36 deletions

diff --git a/zip/cmd/main.go b/zip/cmd/main.go
index 831f6d421..37537ab8b 100644
--- a/zip/cmd/main.go
+++ b/zip/cmd/main.go
@@ -164,7 +164,6 @@ func main() {
 	directories := flags.Bool("d", false, "include directories in zip")
 	compLevel := flags.Int("L", 5, "deflate compression level (0-9)")
 	emulateJar := flags.Bool("jar", false, "modify the resultant .zip to emulate the output of 'jar'")
-	sortEntries := flags.Bool("sort_entries", false, "sort the zip entries")
 	writeIfChanged := flags.Bool("write_if_changed", false, "only update resultant .zip if it has changed")
 	ignoreMissingFiles := flags.Bool("ignore_missing_files", false, "continue if a requested file does not exist")
 	symlinks := flags.Bool("symlinks", true, "store symbolic links in zip instead of following them")
@@ -229,7 +228,6 @@ func main() {
 		FileArgs:                 fileArgsBuilder.FileArgs(),
 		OutputFilePath:           *out,
 		EmulateJar:               *emulateJar,
-		SortEntries:              *sortEntries,
 		SrcJar:                   *srcJar,
 		AddDirectoryEntriesToZip: *directories,
 		CompressionLevel:         *compLevel,
diff --git a/zip/zip.go b/zip/zip.go
index e4e9585d5..22b770474 100644
--- a/zip/zip.go
+++ b/zip/zip.go
@@ -272,7 +272,6 @@ type ZipArgs struct {
 	FileArgs                 []FileArg
 	OutputFilePath           string
 	EmulateJar               bool
-	SortEntries              bool
 	SrcJar                   bool
 	AddDirectoryEntriesToZip bool
 	CompressionLevel         int
@@ -395,7 +394,7 @@ func zipTo(args ZipArgs, w io.Writer) error {
 		}
 	}
 
-	return z.write(w, pathMappings, args.ManifestSourcePath, args.EmulateJar, args.SortEntries, args.SrcJar, args.NumParallelJobs)
+	return z.write(w, pathMappings, args.ManifestSourcePath, args.EmulateJar, args.SrcJar, args.NumParallelJobs)
 }
 
 // Zip creates an output zip archive from given sources.
@@ -476,6 +475,42 @@ func fillPathPairs(fa FileArg, src string, pathMappings *[]pathMapping,
 	return nil
 }
 
+func (z *ZipWriter) moveJavaFileBasedOnPackage(mapping *pathMapping) error {
+	src := mapping.src
+	var s os.FileInfo
+	var err error
+	if z.followSymlinks {
+		s, err = z.fs.Stat(src)
+	} else {
+		s, err = z.fs.Lstat(src)
+	}
+	if err != nil {
+		if os.IsNotExist(err) && z.ignoreMissingFiles {
+			return nil
+		}
+		return err
+	}
+	if !s.Mode().IsRegular() {
+		return nil
+	}
+	r, err := z.fs.Open(src)
+	if err != nil {
+		return err
+	}
+	// rewrite the destination using the package path if it can be determined
+	pkg, err := jar.JavaPackage(r, src)
+	err2 := r.Close()
+	if err2 != nil {
+		return err2
+	}
+	if err != nil {
+		// ignore errors for now, leaving the file at in its original location in the zip
+	} else {
+		mapping.dest = filepath.Join(filepath.Join(strings.Split(pkg, ".")...), filepath.Base(src))
+	}
+	return nil
+}
+
 func jarSort(mappings []pathMapping) {
 	sort.SliceStable(mappings, func(i int, j int) bool {
 		return jar.EntryNamesLess(mappings[i].dest, mappings[j].dest)
@@ -483,7 +518,7 @@ func jarSort(mappings []pathMapping) {
 }
 
 func (z *ZipWriter) write(f io.Writer, pathMappings []pathMapping, manifest string,
-	emulateJar, sortEntries, srcJar bool,
+	emulateJar, srcJar bool,
 	parallelJobs int) error {
 
 	z.errors = make(chan error)
@@ -513,16 +548,38 @@ func (z *ZipWriter) write(f io.Writer, pathMappings []pathMapping, manifest stri
 		return errors.New("must specify --jar when specifying a manifest via -m")
 	}
 
-	if emulateJar && sortEntries {
-		return errors.New("Cannot specify both --jar and --sort_entries (--jar implies sorting with a different algorithm)")
+	// move java source files to the correct folder based on the package statement inside of them.
+	// This is done before the entry sorting so that they're still in the right order.
+	if srcJar {
+		var javaMoveErrors []error
+		var javaMoveErrorsLock sync.Mutex
+		var wg sync.WaitGroup
+		for i := range pathMappings {
+			if filepath.Ext(pathMappings[i].src) == ".java" {
+				wg.Add(1)
+				go func() {
+					err := z.moveJavaFileBasedOnPackage(&pathMappings[i])
+					if err != nil {
+						javaMoveErrorsLock.Lock()
+						javaMoveErrors = append(javaMoveErrors, err)
+						javaMoveErrorsLock.Unlock()
+					}
+					wg.Done()
+				}()
+			}
+		}
+		wg.Wait()
+		if len(javaMoveErrors) > 0 {
+			return errors.Join(javaMoveErrors...)
+		}
 	}
+
 	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)
-	}
-	if sortEntries {
+	} else {
 		sort.SliceStable(pathMappings, func(i int, j int) bool {
 			return pathMappings[i].dest < pathMappings[j].dest
 		})
@@ -536,7 +593,7 @@ func (z *ZipWriter) write(f io.Writer, pathMappings []pathMapping, manifest stri
 			if emulateJar && ele.dest == jar.ManifestFile {
 				err = z.addManifest(ele.dest, ele.src, ele.zipMethod)
 			} else {
-				err = z.addFile(ele.dest, ele.src, ele.zipMethod, emulateJar, srcJar)
+				err = z.addFile(ele.dest, ele.src, ele.zipMethod, emulateJar)
 			}
 			if err != nil {
 				z.errors <- err
@@ -635,7 +692,7 @@ func (z *ZipWriter) write(f io.Writer, pathMappings []pathMapping, manifest stri
 }
 
 // imports (possibly with compression) <src> into the zip at sub-path <dest>
-func (z *ZipWriter) addFile(dest, src string, method uint16, emulateJar, srcJar bool) error {
+func (z *ZipWriter) addFile(dest, src string, method uint16, emulateJar bool) error {
 	var fileSize int64
 	var executable bool
 
@@ -709,21 +766,6 @@ func (z *ZipWriter) addFile(dest, src string, method uint16, emulateJar, srcJar
 			return err
 		}
 
-		if srcJar && filepath.Ext(src) == ".java" {
-			// rewrite the destination using the package path if it can be determined
-			pkg, err := jar.JavaPackage(r, src)
-			if err != nil {
-				// ignore errors for now, leaving the file at in its original location in the zip
-			} else {
-				dest = filepath.Join(filepath.Join(strings.Split(pkg, ".")...), filepath.Base(src))
-			}
-
-			_, err = r.Seek(0, io.SeekStart)
-			if err != nil {
-				return err
-			}
-		}
-
 		fileSize = s.Size()
 		executable = s.Mode()&0100 != 0
 
diff --git a/zip/zip_test.go b/zip/zip_test.go
index c64c3f499..8f100d8dc 100644
--- a/zip/zip_test.go
+++ b/zip/zip_test.go
@@ -159,10 +159,10 @@ func TestZip(t *testing.T) {
 			compressionLevel: 9,
 
 			files: []zip.FileHeader{
+				fh("[", fileEmpty, zip.Store),
 				fh("a/a/a", fileA, zip.Deflate),
 				fh("a/a/b", fileB, zip.Deflate),
 				fh("c", fileC, zip.Deflate),
-				fh("[", fileEmpty, zip.Store),
 			},
 		},
 		{
@@ -261,10 +261,10 @@ func TestZip(t *testing.T) {
 			compressionLevel: 9,
 
 			files: []zip.FileHeader{
+				fh("[", fileEmpty, zip.Store),
 				fh("a/a/a", fileA, zip.Deflate),
 				fh("a/a/b", fileB, zip.Deflate),
 				fh("c", fileC, zip.Deflate),
-				fh("[", fileEmpty, zip.Store),
 			},
 		},
 		{
@@ -274,10 +274,10 @@ func TestZip(t *testing.T) {
 			compressionLevel: 9,
 
 			files: []zip.FileHeader{
+				fh("[", fileEmpty, zip.Store),
 				fh("a/a/a", fileA, zip.Deflate),
 				fh("a/a/b", fileB, zip.Deflate),
 				fh("c", fileC, zip.Deflate),
-				fh("[", fileEmpty, zip.Store),
 			},
 		},
 		{
@@ -287,11 +287,11 @@ func TestZip(t *testing.T) {
 			compressionLevel: 9,
 
 			files: []zip.FileHeader{
+				fh("@", fileC, zip.Deflate),
+				fh("[", fileEmpty, zip.Store),
 				fh("a/a/a", fileA, zip.Deflate),
 				fh("a/a/b", fileB, zip.Deflate),
-				fh("@", fileC, zip.Deflate),
 				fh("foo'bar", fileC, zip.Deflate),
-				fh("[", fileEmpty, zip.Store),
 			},
 		},
 		{
@@ -463,8 +463,8 @@ func TestZip(t *testing.T) {
 			compressionLevel: 9,
 
 			files: []zip.FileHeader{
-				fh("foo", fileA, zip.Deflate),
 				fh("a/a/b", fileB, zip.Deflate),
+				fh("foo", fileA, zip.Deflate),
 			},
 		},
 		{
@@ -477,8 +477,8 @@ func TestZip(t *testing.T) {
 			compressionLevel: 9,
 
 			files: []zip.FileHeader{
-				fh("prefix/foo", fileA, zip.Deflate),
 				fh("prefix/a/a/b", fileB, zip.Deflate),
+				fh("prefix/foo", fileA, zip.Deflate),
 			},
 		},
 		{
@@ -490,8 +490,8 @@ func TestZip(t *testing.T) {
 			compressionLevel: 9,
 
 			files: []zip.FileHeader{
-				fh("foo", fileA, zip.Deflate),
 				fh("a/a/b", fileB, zip.Deflate),
+				fh("foo", fileA, zip.Deflate),
 			},
 		},
 		{
@@ -504,8 +504,8 @@ func TestZip(t *testing.T) {
 			compressionLevel: 9,
 
 			files: []zip.FileHeader{
-				fh("foo/bar", fileA, zip.Deflate),
 				fh("b", fileB, zip.Deflate),
+				fh("foo/bar", fileA, zip.Deflate),
 			},
 		},
 
@@ -688,8 +688,8 @@ func TestSrcJar(t *testing.T) {
 
 	want := []string{
 		"foo/",
-		"foo/wrong_package.java",
 		"foo/correct_package.java",
+		"foo/wrong_package.java",
 		"no_package.java",
 		"src2/",
 		"src2/parse_error.java",