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
diff --git a/androidmk/cmd/androidmk/android.go b/androidmk/cmd/androidmk/android.go
index 8cf93e9..37877c8 100644
--- a/androidmk/cmd/androidmk/android.go
+++ b/androidmk/cmd/androidmk/android.go
@@ -64,6 +64,9 @@
"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 @@
"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 @@
"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 @@
"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
diff --git a/androidmk/cmd/androidmk/androidmk.go b/androidmk/cmd/androidmk/androidmk.go
index 6e0b474..b6a973c 100644
--- a/androidmk/cmd/androidmk/androidmk.go
+++ b/androidmk/cmd/androidmk/androidmk.go
@@ -239,7 +239,8 @@
}
// 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}
}
diff --git a/androidmk/cmd/androidmk/androidmk_test.go b/androidmk/cmd/androidmk/androidmk_test.go
index 45df1a5..1840e02 100644
--- a/androidmk/cmd/androidmk/androidmk_test.go
+++ b/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 $(CLEAR_VARS)
LOCAL_SRC_FILES := test.jar
LOCAL_MODULE_CLASS := JAVA_LIBRARIES
+ LOCAL_STATIC_ANDROID_LIBRARIES :=
include $(BUILD_PREBUILT)
`,
expected: `
@@ -522,28 +521,68 @@
}
`,
},
-}
-func reformatBlueprint(input string) string {
- file, errs := bpparser.Parse("<testcase>", bytes.NewBufferString(input), bpparser.NewScope(nil))
- if len(errs) > 0 {
- for _, err := range errs {
- fmt.Fprintln(os.Stderr, err)
- }
- panic(fmt.Sprintf("%d parsing errors in testcase:\n%s", len(errs), input))
- }
+ {
+ desc: "aar",
+ in: `
+ include $(CLEAR_VARS)
+ LOCAL_SRC_FILES := test.java
+ LOCAL_RESOURCE_DIR := res
+ include $(BUILD_STATIC_JAVA_LIBRARY)
- res, err := bpparser.Print(file)
- if err != nil {
- panic(fmt.Sprintf("Error printing testcase: %q", err))
- }
+ include $(CLEAR_VARS)
+ LOCAL_SRC_FILES := test.java
+ 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 {
diff --git a/bpfix/bpfix/bpfix.go b/bpfix/bpfix/bpfix.go
index 2c3cc6c..e4d4e34 100644
--- a/bpfix/bpfix/bpfix.go
+++ b/bpfix/bpfix/bpfix.go
@@ -18,17 +18,36 @@
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
- rewriteIncorrectAndroidmkPrebuilts bool
+ simplifyKnownRedundantVariables bool
+ rewriteIncorrectAndroidmkPrebuilts bool
+ rewriteIncorrectAndroidmkAndroidLibraries bool
+ mergeMatchingModuleProperties bool
}
func NewFixRequest() FixRequest {
@@ -39,24 +58,39 @@
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 {
- prevIdentifier, err := fingerprint(tree)
+func (f *Fixer) Fix(config FixRequest) (*parser.File, error) {
+ prevIdentifier, err := f.fingerprint()
if err != nil {
- return err
+ return nil, err
}
maxNumIterations := 20
i := 0
for {
- err = fixTreeOnce(tree, config)
- newIdentifier, err := fingerprint(tree)
+ err = f.fixTreeOnce(config)
+ newIdentifier, err := f.fingerprint()
if err != nil {
- return err
+ return nil, err
}
if bytes.Equal(newIdentifier, prevIdentifier) {
break
@@ -67,31 +101,70 @@
// 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) {
- bytes, err := parser.Print(tree)
+func (f *Fixer) fingerprint() (fingerprint []byte, err error) {
+ 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 @@
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 {
- for _, def := range tree.Defs {
+func (f *Fixer) rewriteIncorrectAndroidmkPrebuilts() error {
+ for _, def := range f.tree.Defs {
mod, ok := def.(*parser.Module)
if !ok {
continue
@@ -127,6 +200,7 @@
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 @@
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 @@
}
// 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 {
- for _, def := range tree.Defs {
+func (f *Fixer) removeMatchingModuleListProperties(canonicalName string, legacyName string) error {
+ for _, def := range f.tree.Defs {
mod, ok := def.(*parser.Module)
if !ok {
continue
@@ -182,6 +396,11 @@
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 {
diff --git a/bpfix/bpfix/bpfix_test.go b/bpfix/bpfix/bpfix_test.go
index e17e815..51708eb 100644
--- a/bpfix/bpfix/bpfix_test.go
+++ b/bpfix/bpfix/bpfix_test.go
@@ -17,6 +17,7 @@
package bpfix
import (
+ "bytes"
"fmt"
"strings"
"testing"
@@ -62,14 +63,16 @@
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 @@
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)
+ }
+
+ })
+ }
+}
diff --git a/bpfix/cmd/bpfix.go b/bpfix/cmd/bpfix.go
index 461f41d..2fde383 100644
--- a/bpfix/cmd/bpfix.go
+++ b/bpfix/cmd/bpfix.go
@@ -75,7 +75,8 @@
}
// 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
}