Merge changes I3ad2a356,I30955b6e,I1dc6fd99

* changes:
  Add more androidmk translations for android libraries
  Merge matching properties in bpfix
  Make bpfix not modify the input tree

androidmk/cmd/androidmk/android.go

@@ -64,6 +64,9 @@ var rewriteProperties = map[string](func(variableAssignmentContext) error){
 	"LOCAL_MODULE_SUFFIX":           skip, // TODO
 	"LOCAL_PATH":                    skip, // Nothing to do, except maybe avoid the "./" in paths?
 	"LOCAL_PRELINK_MODULE":          skip, // Already phased out
+	"LOCAL_BUILT_MODULE_STEM":       skip,
+	"LOCAL_USE_AAPT2":               skip, // Always enabled in Soong
+	"LOCAL_JAR_EXCLUDE_FILES":       skip, // Soong never excludes files from jars
 }
 
 // adds a group of properties all having the same type
@@ -94,6 +97,7 @@ func init() {
 			"LOCAL_NOTICE_FILE":             "notice",
 			"LOCAL_JAVA_LANGUAGE_VERSION":   "java_version",
 			"LOCAL_INSTRUMENTATION_FOR":     "instrumentation_for",
+			"LOCAL_MANIFEST_FILE":           "manifest",
 
 			"LOCAL_DEX_PREOPT_PROFILE_CLASS_LISTING": "dex_preopt.profile",
 		})
@@ -129,6 +133,7 @@ func init() {
 			"LOCAL_RENDERSCRIPT_FLAGS":    "renderscript.flags",
 
 			"LOCAL_JAVA_RESOURCE_DIRS":    "java_resource_dirs",
+			"LOCAL_RESOURCE_DIR":          "resource_dirs",
 			"LOCAL_JAVACFLAGS":            "javacflags",
 			"LOCAL_ERROR_PRONE_FLAGS":     "errorprone.javacflags",
 			"LOCAL_DX_FLAGS":              "dxflags",
@@ -143,7 +148,13 @@ func init() {
 
 			"LOCAL_PROGUARD_FLAGS":      "optimize.proguard_flags",
 			"LOCAL_PROGUARD_FLAG_FILES": "optimize.proguard_flag_files",
+
+			// These will be rewritten to libs/static_libs by bpfix, after their presence is used to convert
+			// java_library_static to android_library.
+			"LOCAL_SHARED_ANDROID_LIBRARIES": "android_libs",
+			"LOCAL_STATIC_ANDROID_LIBRARIES": "android_static_libs",
 		})
+
 	addStandardProperties(bpparser.BoolType,
 		map[string]string{
 			// Bool properties

androidmk/cmd/androidmk/androidmk.go

@@ -239,7 +239,8 @@ func convertFile(filename string, buffer *bytes.Buffer) (string, []error) {
 	}
 
 	// check for common supported but undesirable structures and clean them up
-	err := bpfix.FixTree(tree, bpfix.NewFixRequest().AddAll())
+	fixer := bpfix.NewFixer(tree)
+	tree, err := fixer.Fix(bpfix.NewFixRequest().AddAll())
 	if err != nil {
 		return "", []error{err}
 	}

androidmk/cmd/androidmk/androidmk_test.go

@@ -15,13 +15,11 @@
 package main
 
 import (
+	"android/soong/bpfix/bpfix"
 	"bytes"
 	"fmt"
-	"os"
 	"strings"
 	"testing"
-
-	bpparser "github.com/google/blueprint/parser"
 )
 
 var testCases = []struct {
@@ -498,6 +496,7 @@ include $(call all-makefiles-under,$(LOCAL_PATH))
 			include $(CLEAR_VARS)
 			LOCAL_SRC_FILES := test.jar
 			LOCAL_MODULE_CLASS := JAVA_LIBRARIES
+			LOCAL_STATIC_ANDROID_LIBRARIES :=
 			include $(BUILD_PREBUILT)
 		`,
 		expected: `
@@ -522,28 +521,68 @@ include $(call all-makefiles-under,$(LOCAL_PATH))
 			}
 		`,
 	},
-}
 
-func reformatBlueprint(input string) string {
+	{
-	file, errs := bpparser.Parse("<testcase>", bytes.NewBufferString(input), bpparser.NewScope(nil))
+		desc: "aar",
-	if len(errs) > 0 {
+		in: `
-		for _, err := range errs {
+			include $(CLEAR_VARS)
-			fmt.Fprintln(os.Stderr, err)
+			LOCAL_SRC_FILES := test.java
-		}
+			LOCAL_RESOURCE_DIR := res
-		panic(fmt.Sprintf("%d parsing errors in testcase:\n%s", len(errs), input))
+			include $(BUILD_STATIC_JAVA_LIBRARY)
-	}
 
-	res, err := bpparser.Print(file)
+			include $(CLEAR_VARS)
-	if err != nil {
+			LOCAL_SRC_FILES := test.java
-		panic(fmt.Sprintf("Error printing testcase: %q", err))
+			LOCAL_STATIC_LIBRARIES := foo
-	}
+			LOCAL_STATIC_ANDROID_LIBRARIES := bar
+			include $(BUILD_STATIC_JAVA_LIBRARY)
 
-	return string(res)
+			include $(CLEAR_VARS)
+			LOCAL_SRC_FILES := test.java
+			LOCAL_SHARED_LIBRARIES := foo
+			LOCAL_SHARED_ANDROID_LIBRARIES := bar
+			include $(BUILD_STATIC_JAVA_LIBRARY)
+
+			include $(CLEAR_VARS)
+			LOCAL_SRC_FILES := test.java
+			LOCAL_STATIC_ANDROID_LIBRARIES :=
+			include $(BUILD_STATIC_JAVA_LIBRARY)
+		`,
+		expected: `
+			android_library {
+				srcs: ["test.java"],
+				resource_dirs: ["res"],
+			}
+
+			android_library {
+				srcs: ["test.java"],
+				static_libs: [
+					"foo",
+					"bar",
+				],
+			}
+
+			android_library {
+				srcs: ["test.java"],
+				libs: [
+					"foo",
+					"bar",
+				],
+			}
+
+			java_library_static {
+				srcs: ["test.java"],
+				static_libs: [],
+			}
+		`,
+	},
 }
 
 func TestEndToEnd(t *testing.T) {
 	for i, test := range testCases {
-		expected := reformatBlueprint(test.expected)
+		expected, err := bpfix.Reformat(test.expected)
+		if err != nil {
+			t.Error(err)
+		}
 
 		got, errs := convertFile(fmt.Sprintf("<testcase %d>", i), bytes.NewBufferString(test.in))
 		if len(errs) > 0 {

bpfix/bpfix/bpfix.go

@@ -18,17 +18,36 @@ package bpfix
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
+	"io"
 	"path/filepath"
 
 	"github.com/google/blueprint/parser"
 )
 
+// Reformat takes a blueprint file as a string and returns a formatted version
+func Reformat(input string) (string, error) {
+	tree, err := parse("<string>", bytes.NewBufferString(input))
+	if err != nil {
+		return "", err
+	}
+
+	res, err := parser.Print(tree)
+	if err != nil {
+		return "", err
+	}
+
+	return string(res), nil
+}
+
 // A FixRequest specifies the details of which fixes to apply to an individual file
 // A FixRequest doesn't specify whether to do a dry run or where to write the results; that's in cmd/bpfix.go
 type FixRequest struct {
-	simplifyKnownRedundantVariables    bool
+	simplifyKnownRedundantVariables           bool
-	rewriteIncorrectAndroidmkPrebuilts bool
+	rewriteIncorrectAndroidmkPrebuilts        bool
+	rewriteIncorrectAndroidmkAndroidLibraries bool
+	mergeMatchingModuleProperties             bool
 }
 
 func NewFixRequest() FixRequest {
@@ -39,24 +58,39 @@ func (r FixRequest) AddAll() (result FixRequest) {
 	result = r
 	result.simplifyKnownRedundantVariables = true
 	result.rewriteIncorrectAndroidmkPrebuilts = true
+	result.rewriteIncorrectAndroidmkAndroidLibraries = true
+	result.mergeMatchingModuleProperties = true
 	return result
 }
 
-// FixTree repeatedly applies the fixes listed in the given FixRequest to the given File
+type Fixer struct {
+	tree *parser.File
+}
+
+func NewFixer(tree *parser.File) *Fixer {
+	fixer := &Fixer{tree}
+
+	// make a copy of the tree
+	fixer.reparse()
+
+	return fixer
+}
+
+// Fix repeatedly applies the fixes listed in the given FixRequest to the given File
 // until there is no fix that affects the tree
-func FixTree(tree *parser.File, config FixRequest) error {
+func (f *Fixer) Fix(config FixRequest) (*parser.File, error) {
-	prevIdentifier, err := fingerprint(tree)
+	prevIdentifier, err := f.fingerprint()
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	maxNumIterations := 20
 	i := 0
 	for {
-		err = fixTreeOnce(tree, config)
+		err = f.fixTreeOnce(config)
-		newIdentifier, err := fingerprint(tree)
+		newIdentifier, err := f.fingerprint()
 		if err != nil {
-			return err
+			return nil, err
 		}
 		if bytes.Equal(newIdentifier, prevIdentifier) {
 			break
@@ -67,31 +101,70 @@ func FixTree(tree *parser.File, config FixRequest) error {
 		// detect infinite loop
 		i++
 		if i >= maxNumIterations {
-			return fmt.Errorf("Applied fixes %d times and yet the tree continued to change. Is there an infinite loop?", i)
+			return nil, fmt.Errorf("Applied fixes %d times and yet the tree continued to change. Is there an infinite loop?", i)
 			break
 		}
 	}
-	return err
+	return f.tree, err
 }
 
 // returns a unique identifier for the given tree that can be used to determine whether the tree changed
-func fingerprint(tree *parser.File) (fingerprint []byte, err error) {
+func (f *Fixer) fingerprint() (fingerprint []byte, err error) {
-	bytes, err := parser.Print(tree)
+	bytes, err := parser.Print(f.tree)
 	if err != nil {
 		return nil, err
 	}
 	return bytes, nil
 }
 
-func fixTreeOnce(tree *parser.File, config FixRequest) error {
+func (f *Fixer) reparse() ([]byte, error) {
+	buf, err := parser.Print(f.tree)
+	if err != nil {
+		return nil, err
+	}
+	newTree, err := parse(f.tree.Name, bytes.NewReader(buf))
+	if err != nil {
+		return nil, err
+	}
+	f.tree = newTree
+	return buf, nil
+}
+
+func parse(name string, r io.Reader) (*parser.File, error) {
+	tree, errs := parser.Parse(name, r, parser.NewScope(nil))
+	if errs != nil {
+		s := "parse error: "
+		for _, err := range errs {
+			s += "\n" + err.Error()
+		}
+		return nil, errors.New(s)
+	}
+	return tree, nil
+}
+
+func (f *Fixer) fixTreeOnce(config FixRequest) error {
 	if config.simplifyKnownRedundantVariables {
-		err := simplifyKnownPropertiesDuplicatingEachOther(tree)
+		err := f.simplifyKnownPropertiesDuplicatingEachOther()
 		if err != nil {
 			return err
 		}
 	}
 	if config.rewriteIncorrectAndroidmkPrebuilts {
-		err := rewriteIncorrectAndroidmkPrebuilts(tree)
+		err := f.rewriteIncorrectAndroidmkPrebuilts()
+		if err != nil {
+			return err
+		}
+	}
+
+	if config.rewriteIncorrectAndroidmkAndroidLibraries {
+		err := f.rewriteIncorrectAndroidmkAndroidLibraries()
+		if err != nil {
+			return err
+		}
+	}
+
+	if config.mergeMatchingModuleProperties {
+		err := f.mergeMatchingModuleProperties()
 		if err != nil {
 			return err
 		}
@@ -99,13 +172,13 @@ func fixTreeOnce(tree *parser.File, config FixRequest) error {
 	return nil
 }
 
-func simplifyKnownPropertiesDuplicatingEachOther(tree *parser.File) error {
+func (f *Fixer) simplifyKnownPropertiesDuplicatingEachOther() error {
 	// remove from local_include_dirs anything in export_include_dirs
-	return removeMatchingModuleListProperties(tree, "export_include_dirs", "local_include_dirs")
+	return f.removeMatchingModuleListProperties("export_include_dirs", "local_include_dirs")
 }
 
-func rewriteIncorrectAndroidmkPrebuilts(tree *parser.File) error {
+func (f *Fixer) rewriteIncorrectAndroidmkPrebuilts() error {
-	for _, def := range tree.Defs {
+	for _, def := range f.tree.Defs {
 		mod, ok := def.(*parser.Module)
 		if !ok {
 			continue
@@ -127,6 +200,7 @@ func rewriteIncorrectAndroidmkPrebuilts(tree *parser.File) error {
 		switch filepath.Ext(src.Value) {
 		case ".jar":
 			renameProperty(mod, "srcs", "jars")
+
 		case ".aar":
 			renameProperty(mod, "srcs", "aars")
 			mod.Type = "android_library_import"
@@ -139,6 +213,146 @@ func rewriteIncorrectAndroidmkPrebuilts(tree *parser.File) error {
 	return nil
 }
 
+func (f *Fixer) rewriteIncorrectAndroidmkAndroidLibraries() error {
+	for _, def := range f.tree.Defs {
+		mod, ok := def.(*parser.Module)
+		if !ok {
+			continue
+		}
+
+		hasAndroidLibraries := hasNonEmptyLiteralListProperty(mod, "android_libs")
+		hasStaticAndroidLibraries := hasNonEmptyLiteralListProperty(mod, "android_static_libs")
+		hasResourceDirs := hasNonEmptyLiteralListProperty(mod, "resource_dirs")
+
+		if hasAndroidLibraries || hasStaticAndroidLibraries || hasResourceDirs {
+			if mod.Type == "java_library_static" {
+				mod.Type = "android_library"
+			}
+		}
+
+		if mod.Type == "java_import" && !hasStaticAndroidLibraries {
+			removeProperty(mod, "android_static_libs")
+		}
+
+		// These may conflict with existing libs and static_libs properties, but the
+		// mergeMatchingModuleProperties pass will fix it.
+		renameProperty(mod, "shared_libs", "libs")
+		renameProperty(mod, "android_libs", "libs")
+		renameProperty(mod, "android_static_libs", "static_libs")
+	}
+
+	return nil
+}
+
+func (f *Fixer) mergeMatchingModuleProperties() error {
+	// Make sure all the offsets are accurate
+	buf, err := f.reparse()
+	if err != nil {
+		return err
+	}
+
+	var patchlist parser.PatchList
+	for _, def := range f.tree.Defs {
+		mod, ok := def.(*parser.Module)
+		if !ok {
+			continue
+		}
+
+		err := mergeMatchingProperties(&mod.Properties, buf, &patchlist)
+		if err != nil {
+			return err
+		}
+	}
+
+	newBuf := new(bytes.Buffer)
+	err = patchlist.Apply(bytes.NewReader(buf), newBuf)
+	if err != nil {
+		return err
+	}
+
+	newTree, err := parse(f.tree.Name, newBuf)
+	if err != nil {
+		return err
+	}
+
+	f.tree = newTree
+
+	return nil
+}
+
+func mergeMatchingProperties(properties *[]*parser.Property, buf []byte, patchlist *parser.PatchList) error {
+	seen := make(map[string]*parser.Property)
+	for i := 0; i < len(*properties); i++ {
+		property := (*properties)[i]
+		if prev, exists := seen[property.Name]; exists {
+			err := mergeProperties(prev, property, buf, patchlist)
+			if err != nil {
+				return err
+			}
+			*properties = append((*properties)[:i], (*properties)[i+1:]...)
+		} else {
+			seen[property.Name] = property
+			if mapProperty, ok := property.Value.(*parser.Map); ok {
+				err := mergeMatchingProperties(&mapProperty.Properties, buf, patchlist)
+				if err != nil {
+					return err
+				}
+			}
+		}
+	}
+	return nil
+}
+
+func mergeProperties(a, b *parser.Property, buf []byte, patchlist *parser.PatchList) error {
+	if a.Value.Type() != b.Value.Type() {
+		return fmt.Errorf("type mismatch when merging properties %q: %s and %s", a.Name, a.Value.Type(), b.Value.Type())
+	}
+
+	switch a.Value.Type() {
+	case parser.StringType:
+		return fmt.Errorf("conflicting definitions of string property %q", a.Name)
+	case parser.ListType:
+		return mergeListProperties(a, b, buf, patchlist)
+	}
+
+	return nil
+}
+
+func mergeListProperties(a, b *parser.Property, buf []byte, patchlist *parser.PatchList) error {
+	aval, oka := a.Value.(*parser.List)
+	bval, okb := b.Value.(*parser.List)
+	if !oka || !okb {
+		// Merging expressions not supported yet
+		return nil
+	}
+
+	s := string(buf[bval.LBracePos.Offset+1 : bval.RBracePos.Offset])
+	if bval.LBracePos.Line != bval.RBracePos.Line {
+		if s[0] != '\n' {
+			panic("expected \n")
+		}
+		// If B is a multi line list, skip the first "\n" in case A already has a trailing "\n"
+		s = s[1:]
+	}
+	if aval.LBracePos.Line == aval.RBracePos.Line {
+		// A is a single line list with no trailing comma
+		if len(aval.Values) > 0 {
+			s = "," + s
+		}
+	}
+
+	err := patchlist.Add(aval.RBracePos.Offset, aval.RBracePos.Offset, s)
+	if err != nil {
+		return err
+	}
+	err = patchlist.Add(b.NamePos.Offset, b.End().Offset+2, "")
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
 // removes from <items> every item present in <removals>
 func filterExpressionList(items *parser.List, removals *parser.List) {
 	writeIndex := 0
@@ -163,8 +377,8 @@ func filterExpressionList(items *parser.List, removals *parser.List) {
 }
 
 // Remove each modules[i].Properties[<legacyName>][j] that matches a modules[i].Properties[<canonicalName>][k]
-func removeMatchingModuleListProperties(tree *parser.File, canonicalName string, legacyName string) error {
+func (f *Fixer) removeMatchingModuleListProperties(canonicalName string, legacyName string) error {
-	for _, def := range tree.Defs {
+	for _, def := range f.tree.Defs {
 		mod, ok := def.(*parser.Module)
 		if !ok {
 			continue
@@ -182,6 +396,11 @@ func removeMatchingModuleListProperties(tree *parser.File, canonicalName string,
 	return nil
 }
 
+func hasNonEmptyLiteralListProperty(mod *parser.Module, name string) bool {
+	list, found := getLiteralListProperty(mod, name)
+	return found && len(list.Values) > 0
+}
+
 func getLiteralListProperty(mod *parser.Module, name string) (list *parser.List, found bool) {
 	prop, ok := mod.GetProperty(name)
 	if !ok {

bpfix/bpfix/bpfix_test.go

@@ -17,6 +17,7 @@
 package bpfix
 
 import (
+	"bytes"
 	"fmt"
 	"strings"
 	"testing"
@@ -62,14 +63,16 @@ func implFilterListTest(t *testing.T, local_include_dirs []string, export_includ
 		t.Fatalf("%d parse errors", len(errs))
 	}
 
+	fixer := NewFixer(tree)
+
 	// apply simplifications
-	err := simplifyKnownPropertiesDuplicatingEachOther(tree)
+	err := fixer.simplifyKnownPropertiesDuplicatingEachOther()
 	if len(errs) > 0 {
 		t.Fatal(err)
 	}
 
 	// lookup legacy property
-	mod := tree.Defs[0].(*parser.Module)
+	mod := fixer.tree.Defs[0].(*parser.Module)
 	_, found := mod.GetProperty("local_include_dirs")
 	if !found {
 		t.Fatalf("failed to include key local_include_dirs in parse tree")
@@ -113,3 +116,130 @@ func TestSimplifyKnownVariablesDuplicatingEachOther(t *testing.T) {
 	implFilterListTest(t, []string{}, []string{"include"}, []string{})
 	implFilterListTest(t, []string{}, []string{}, []string{})
 }
+
+func TestMergeMatchingProperties(t *testing.T) {
+	tests := []struct {
+		name string
+		in   string
+		out  string
+	}{
+		{
+			name: "empty",
+			in: `
+				java_library {
+					name: "foo",
+					static_libs: [],
+					static_libs: [],
+				}
+			`,
+			out: `
+				java_library {
+					name: "foo",
+					static_libs: [],
+				}
+			`,
+		},
+		{
+			name: "single line into multiline",
+			in: `
+				java_library {
+					name: "foo",
+					static_libs: [
+						"a",
+						"b",
+					],
+					//c1
+					static_libs: ["c" /*c2*/],
+				}
+			`,
+			out: `
+				java_library {
+					name: "foo",
+					static_libs: [
+						"a",
+						"b",
+						"c", /*c2*/
+					],
+					//c1
+				}
+			`,
+		},
+		{
+			name: "multiline into multiline",
+			in: `
+				java_library {
+					name: "foo",
+					static_libs: [
+						"a",
+						"b",
+					],
+					//c1
+					static_libs: [
+						//c2
+						"c", //c3
+						"d",
+					],
+				}
+			`,
+			out: `
+				java_library {
+					name: "foo",
+					static_libs: [
+						"a",
+						"b",
+						//c2
+						"c", //c3
+						"d",
+					],
+					//c1
+				}
+			`,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			expected, err := Reformat(test.out)
+			if err != nil {
+				t.Error(err)
+			}
+
+			in, err := Reformat(test.in)
+			if err != nil {
+				t.Error(err)
+			}
+
+			tree, errs := parser.Parse("<testcase>", bytes.NewBufferString(in), parser.NewScope(nil))
+			if errs != nil {
+				t.Fatal(errs)
+			}
+
+			fixer := NewFixer(tree)
+
+			got := ""
+			prev := "foo"
+			passes := 0
+			for got != prev && passes < 10 {
+				err := fixer.mergeMatchingModuleProperties()
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				out, err := parser.Print(fixer.tree)
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				prev = got
+				got = string(out)
+				passes++
+			}
+
+			if got != expected {
+				t.Errorf("failed testcase '%s'\ninput:\n%s\n\nexpected:\n%s\ngot:\n%s\n",
+					test.name, in, expected, got)
+			}
+
+		})
+	}
+}

bpfix/cmd/bpfix.go

@@ -75,7 +75,8 @@ func processFile(filename string, in io.Reader, out io.Writer, fixRequest bpfix.
 	}
 
 	// compute and apply any requested fixes
-	err = bpfix.FixTree(file, fixRequest)
+	fixer := bpfix.NewFixer(file)
+	file, err = fixer.Fix(fixRequest)
 	if err != nil {
 		return err
 	}