Merge "java_sdk_library does the apicheck by default"
diff --git a/bpfix/bpfix/bpfix.go b/bpfix/bpfix/bpfix.go
index a610a17..55de993 100644
--- a/bpfix/bpfix/bpfix.go
+++ b/bpfix/bpfix/bpfix.go
@@ -49,6 +49,7 @@
 	rewriteIncorrectAndroidmkPrebuilts        bool
 	rewriteIncorrectAndroidmkAndroidLibraries bool
 	mergeMatchingModuleProperties             bool
+	reorderCommonProperties                   bool
 }
 
 func NewFixRequest() FixRequest {
@@ -61,6 +62,7 @@
 	result.rewriteIncorrectAndroidmkPrebuilts = true
 	result.rewriteIncorrectAndroidmkAndroidLibraries = true
 	result.mergeMatchingModuleProperties = true
+	result.reorderCommonProperties = true
 	return result
 }
 
@@ -145,11 +147,12 @@
 
 func (f *Fixer) fixTreeOnce(config FixRequest) error {
 	if config.simplifyKnownRedundantVariables {
-		err := f.simplifyKnownPropertiesDuplicatingEachOther()
+		err := f.runPatchListMod(simplifyKnownPropertiesDuplicatingEachOther)
 		if err != nil {
 			return err
 		}
 	}
+
 	if config.rewriteIncorrectAndroidmkPrebuilts {
 		err := f.rewriteIncorrectAndroidmkPrebuilts()
 		if err != nil {
@@ -165,7 +168,14 @@
 	}
 
 	if config.mergeMatchingModuleProperties {
-		err := f.mergeMatchingModuleProperties()
+		err := f.runPatchListMod(mergeMatchingModuleProperties)
+		if err != nil {
+			return err
+		}
+	}
+
+	if config.reorderCommonProperties {
+		err := f.runPatchListMod(reorderCommonProperties)
 		if err != nil {
 			return err
 		}
@@ -173,9 +183,10 @@
 	return nil
 }
 
-func (f *Fixer) simplifyKnownPropertiesDuplicatingEachOther() error {
+func simplifyKnownPropertiesDuplicatingEachOther(mod *parser.Module, buf []byte, patchList *parser.PatchList) error {
 	// remove from local_include_dirs anything in export_include_dirs
-	return f.removeMatchingModuleListProperties("export_include_dirs", "local_include_dirs")
+	return removeMatchingModuleListProperties(mod, patchList,
+		"export_include_dirs", "local_include_dirs")
 }
 
 func (f *Fixer) rewriteIncorrectAndroidmkPrebuilts() error {
@@ -249,7 +260,7 @@
 	return nil
 }
 
-func (f *Fixer) mergeMatchingModuleProperties() error {
+func (f *Fixer) runPatchListMod(modFunc func(mod *parser.Module, buf []byte, patchlist *parser.PatchList) error) error {
 	// Make sure all the offsets are accurate
 	buf, err := f.reparse()
 	if err != nil {
@@ -263,7 +274,7 @@
 			continue
 		}
 
-		err := mergeMatchingProperties(&mod.Properties, buf, &patchlist)
+		err := modFunc(mod, buf, &patchlist)
 		if err != nil {
 			return err
 		}
@@ -275,9 +286,12 @@
 		return err
 	}
 
+	// Save a copy of the buffer to print for errors below
+	bufCopy := append([]byte(nil), newBuf.Bytes()...)
+
 	newTree, err := parse(f.tree.Name, newBuf)
 	if err != nil {
-		return err
+		return fmt.Errorf("Failed to parse: %v\nBuffer:\n%s", err, string(bufCopy))
 	}
 
 	f.tree = newTree
@@ -285,6 +299,63 @@
 	return nil
 }
 
+var commonPropertyPriorities = []string{
+	"name",
+	"defaults",
+	"device_supported",
+	"host_supported",
+}
+
+func reorderCommonProperties(mod *parser.Module, buf []byte, patchlist *parser.PatchList) error {
+	if len(mod.Properties) == 0 {
+		return nil
+	}
+
+	pos := mod.LBracePos.Offset + 1
+	stage := ""
+
+	for _, name := range commonPropertyPriorities {
+		idx := propertyIndex(mod.Properties, name)
+		if idx == -1 {
+			continue
+		}
+		if idx == 0 {
+			err := patchlist.Add(pos, pos, stage)
+			if err != nil {
+				return err
+			}
+			stage = ""
+
+			pos = mod.Properties[0].End().Offset + 1
+			mod.Properties = mod.Properties[1:]
+			continue
+		}
+
+		prop := mod.Properties[idx]
+		mod.Properties = append(mod.Properties[:idx], mod.Properties[idx+1:]...)
+
+		stage += string(buf[prop.Pos().Offset : prop.End().Offset+1])
+
+		err := patchlist.Add(prop.Pos().Offset, prop.End().Offset+2, "")
+		if err != nil {
+			return err
+		}
+	}
+
+	if stage != "" {
+		err := patchlist.Add(pos, pos, stage)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func mergeMatchingModuleProperties(mod *parser.Module, buf []byte, patchlist *parser.PatchList) error {
+	return mergeMatchingProperties(&mod.Properties, buf, patchlist)
+}
+
 func mergeMatchingProperties(properties *[]*parser.Property, buf []byte, patchlist *parser.PatchList) error {
 	seen := make(map[string]*parser.Property)
 	for i := 0; i < len(*properties); i++ {
@@ -359,7 +430,7 @@
 }
 
 // removes from <items> every item present in <removals>
-func filterExpressionList(items *parser.List, removals *parser.List) {
+func filterExpressionList(patchList *parser.PatchList, items *parser.List, removals *parser.List) {
 	writeIndex := 0
 	for _, item := range items.Values {
 		included := true
@@ -376,32 +447,39 @@
 		if included {
 			items.Values[writeIndex] = item
 			writeIndex++
+		} else {
+			patchList.Add(item.Pos().Offset, item.End().Offset+2, "")
 		}
 	}
 	items.Values = items.Values[:writeIndex]
 }
 
 // Remove each modules[i].Properties[<legacyName>][j] that matches a modules[i].Properties[<canonicalName>][k]
-func (f *Fixer) removeMatchingModuleListProperties(canonicalName string, legacyName string) error {
-	for _, def := range f.tree.Defs {
-		mod, ok := def.(*parser.Module)
-		if !ok {
-			continue
-		}
-		legacyList, ok := getLiteralListProperty(mod, legacyName)
-		if !ok || len(legacyList.Values) == 0 {
-			continue
-		}
-		canonicalList, ok := getLiteralListProperty(mod, canonicalName)
-		if !ok {
-			continue
-		}
-		filterExpressionList(legacyList, canonicalList)
+func removeMatchingModuleListProperties(mod *parser.Module, patchList *parser.PatchList, canonicalName string, legacyName string) error {
+	legacyProp, ok := mod.GetProperty(legacyName)
+	if !ok {
+		return nil
+	}
+	legacyList, ok := legacyProp.Value.(*parser.List)
+	if !ok || len(legacyList.Values) == 0 {
+		return nil
+	}
+	canonicalList, ok := getLiteralListProperty(mod, canonicalName)
+	if !ok {
+		return nil
+	}
 
-		if len(legacyList.Values) == 0 {
-			removeProperty(mod, legacyName)
+	localPatches := parser.PatchList{}
+	filterExpressionList(&localPatches, legacyList, canonicalList)
+
+	if len(legacyList.Values) == 0 {
+		patchList.Add(legacyProp.Pos().Offset, legacyProp.End().Offset+2, "")
+	} else {
+		for _, p := range localPatches {
+			patchList.Add(p.Start, p.End, p.Replacement)
 		}
 	}
+
 	return nil
 }
 
@@ -419,6 +497,15 @@
 	return list, ok
 }
 
+func propertyIndex(props []*parser.Property, propertyName string) int {
+	for i, prop := range props {
+		if prop.Name == propertyName {
+			return i
+		}
+	}
+	return -1
+}
+
 func renameProperty(mod *parser.Module, from, to string) {
 	for _, prop := range mod.Properties {
 		if prop.Name == from {
diff --git a/bpfix/bpfix/bpfix_test.go b/bpfix/bpfix/bpfix_test.go
index 8fed0a2..654ccf9 100644
--- a/bpfix/bpfix/bpfix_test.go
+++ b/bpfix/bpfix/bpfix_test.go
@@ -66,7 +66,7 @@
 	fixer := NewFixer(tree)
 
 	// apply simplifications
-	err := fixer.simplifyKnownPropertiesDuplicatingEachOther()
+	err := fixer.runPatchListMod(simplifyKnownPropertiesDuplicatingEachOther)
 	if len(errs) > 0 {
 		t.Fatal(err)
 	}
@@ -125,6 +125,49 @@
 	implFilterListTest(t, []string{}, []string{}, []string{})
 }
 
+func runPass(t *testing.T, in, out string, innerTest func(*Fixer) error) {
+	expected, err := Reformat(out)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	in, err = Reformat(in)
+	if err != nil {
+		t.Fatal(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 := innerTest(fixer)
+		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("output didn't match:\ninput:\n%s\n\nexpected:\n%s\ngot:\n%s\n",
+			in, expected, got)
+	}
+}
+
 func TestMergeMatchingProperties(t *testing.T) {
 	tests := []struct {
 		name string
@@ -207,47 +250,250 @@
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			expected, err := Reformat(test.out)
-			if err != nil {
-				t.Error(err)
-			}
+			runPass(t, test.in, test.out, func(fixer *Fixer) error {
+				return fixer.runPatchListMod(mergeMatchingModuleProperties)
+			})
+		})
+	}
+}
 
-			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)
+func TestReorderCommonProperties(t *testing.T) {
+	var tests = []struct {
+		name string
+		in   string
+		out  string
+	}{
+		{
+			name: "empty",
+			in:   `cc_library {}`,
+			out:  `cc_library {}`,
+		},
+		{
+			name: "only priority",
+			in: `
+				cc_library {
+					name: "foo",
 				}
-
-				out, err := parser.Print(fixer.tree)
-				if err != nil {
-					t.Fatal(err)
+			`,
+			out: `
+				cc_library {
+					name: "foo",
 				}
+			`,
+		},
+		{
+			name: "already in order",
+			in: `
+				cc_library {
+					name: "foo",
+					defaults: ["bar"],
+				}
+			`,
+			out: `
+				cc_library {
+					name: "foo",
+					defaults: ["bar"],
+				}
+			`,
+		},
+		{
+			name: "reorder only priority",
+			in: `
+				cc_library {
+					defaults: ["bar"],
+					name: "foo",
+				}
+			`,
+			out: `
+				cc_library {
+					name: "foo",
+					defaults: ["bar"],
+				}
+			`,
+		},
+		{
+			name: "reorder",
+			in: `
+				cc_library {
+					name: "foo",
+					srcs: ["a.c"],
+					host_supported: true,
+					defaults: ["bar"],
+					shared_libs: ["baz"],
+				}
+			`,
+			out: `
+				cc_library {
+					name: "foo",
+					defaults: ["bar"],
+					host_supported: true,
+					srcs: ["a.c"],
+					shared_libs: ["baz"],
+				}
+			`,
+		},
+	}
 
-				prev = got
-				got = string(out)
-				passes++
-			}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			runPass(t, test.in, test.out, func(fixer *Fixer) error {
+				return fixer.runPatchListMod(reorderCommonProperties)
+			})
+		})
+	}
+}
 
-			if got != expected {
-				t.Errorf("failed testcase '%s'\ninput:\n%s\n\nexpected:\n%s\ngot:\n%s\n",
-					test.name, in, expected, got)
-			}
+func TestRemoveMatchingModuleListProperties(t *testing.T) {
+	var tests = []struct {
+		name string
+		in   string
+		out  string
+	}{
+		{
+			name: "simple",
+			in: `
+				cc_library {
+					name: "foo",
+					foo: ["a"],
+					bar: ["a"],
+				}
+			`,
+			out: `
+				cc_library {
+					name: "foo",
+					bar: ["a"],
+				}
+			`,
+		},
+		{
+			name: "long",
+			in: `
+				cc_library {
+					name: "foo",
+					foo: [
+						"a",
+						"b",
+					],
+					bar: ["a"],
+				}
+			`,
+			out: `
+				cc_library {
+					name: "foo",
+					foo: [
+						"b",
+					],
+					bar: ["a"],
+				}
+			`,
+		},
+		{
+			name: "long fully removed",
+			in: `
+				cc_library {
+					name: "foo",
+					foo: [
+						"a",
+					],
+					bar: ["a"],
+				}
+			`,
+			out: `
+				cc_library {
+					name: "foo",
+					bar: ["a"],
+				}
+			`,
+		},
+		{
+			name: "comment",
+			in: `
+				cc_library {
+					name: "foo",
 
+					// comment
+					foo: ["a"],
+
+					bar: ["a"],
+				}
+			`,
+			out: `
+				cc_library {
+					name: "foo",
+
+					// comment
+
+					bar: ["a"],
+				}
+			`,
+		},
+		{
+			name: "inner comment",
+			in: `
+				cc_library {
+					name: "foo",
+					foo: [
+						// comment
+						"a",
+					],
+					bar: ["a"],
+				}
+			`,
+			out: `
+				cc_library {
+					name: "foo",
+					bar: ["a"],
+				}
+			`,
+		},
+		{
+			name: "eol comment",
+			in: `
+				cc_library {
+					name: "foo",
+					foo: ["a"], // comment
+					bar: ["a"],
+				}
+			`,
+			out: `
+				cc_library {
+					name: "foo",
+					// comment
+					bar: ["a"],
+				}
+			`,
+		},
+		{
+			name: "eol comment with blank lines",
+			in: `
+				cc_library {
+					name: "foo",
+
+					foo: ["a"], // comment
+
+					// bar
+					bar: ["a"],
+				}
+			`,
+			out: `
+				cc_library {
+					name: "foo",
+
+					// comment
+
+					// bar
+					bar: ["a"],
+				}
+			`,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			runPass(t, test.in, test.out, func(fixer *Fixer) error {
+				return fixer.runPatchListMod(func(mod *parser.Module, buf []byte, patchList *parser.PatchList) error {
+					return removeMatchingModuleListProperties(mod, patchList, "bar", "foo")
+				})
+			})
 		})
 	}
 }
diff --git a/cc/cc.go b/cc/cc.go
index 76e6645..fe337f5 100644
--- a/cc/cc.go
+++ b/cc/cc.go
@@ -552,13 +552,13 @@
 // Create source abi dumps if the module belongs to the list of VndkLibraries.
 func (ctx *moduleContextImpl) createVndkSourceAbiDump() bool {
 	skipAbiChecks := ctx.ctx.Config().IsEnvTrue("SKIP_ABI_CHECKS")
-	isUnsanitizedVariant := true
+	isVariantOnProductionDevice := true
 	sanitize := ctx.mod.sanitize
 	if sanitize != nil {
-		isUnsanitizedVariant = sanitize.isUnsanitizedVariant()
+		isVariantOnProductionDevice = sanitize.isVariantOnProductionDevice()
 	}
 	vendorAvailable := Bool(ctx.mod.VendorProperties.Vendor_available)
-	return !skipAbiChecks && isUnsanitizedVariant && ctx.ctx.Device() && ((ctx.useVndk() && ctx.isVndk() && vendorAvailable) || inList(ctx.baseModuleName(), llndkLibraries))
+	return !skipAbiChecks && isVariantOnProductionDevice && ctx.ctx.Device() && ((ctx.useVndk() && ctx.isVndk() && vendorAvailable) || inList(ctx.baseModuleName(), llndkLibraries))
 }
 
 func (ctx *moduleContextImpl) selectedStl() string {
diff --git a/cc/sanitize.go b/cc/sanitize.go
index 859d876..a6898a2 100644
--- a/cc/sanitize.go
+++ b/cc/sanitize.go
@@ -539,6 +539,11 @@
 		!sanitize.isSanitizerEnabled(cfi)
 }
 
+func (sanitize *sanitize) isVariantOnProductionDevice() bool {
+	return !sanitize.isSanitizerEnabled(asan) &&
+		!sanitize.isSanitizerEnabled(tsan)
+}
+
 func (sanitize *sanitize) SetSanitizer(t sanitizerType, b bool) {
 	switch t {
 	case asan:
diff --git a/ui/build/util.go b/ui/build/util.go
index f698ccd..96088fe 100644
--- a/ui/build/util.go
+++ b/ui/build/util.go
@@ -62,9 +62,24 @@
 func ensureEmptyDirectoriesExist(ctx Context, dirs ...string) {
 	// remove all the directories
 	for _, dir := range dirs {
-		err := os.RemoveAll(dir)
-		if err != nil {
-			ctx.Fatalf("Error removing %s: %q\n", dir, err)
+		seenErr := map[string]bool{}
+		for {
+			err := os.RemoveAll(dir)
+			if err == nil {
+				break
+			}
+
+			if pathErr, ok := err.(*os.PathError); !ok ||
+				dir == pathErr.Path || seenErr[pathErr.Path] {
+
+				ctx.Fatalf("Error removing %s: %q\n", dir, err)
+			} else {
+				seenErr[pathErr.Path] = true
+				err = os.Chmod(filepath.Dir(pathErr.Path), 0700)
+				if err != nil {
+					ctx.Fatal(err)
+				}
+			}
 		}
 	}
 	// recreate all the directories
diff --git a/ui/build/util_test.go b/ui/build/util_test.go
index e85eada..0e0dbdf 100644
--- a/ui/build/util_test.go
+++ b/ui/build/util_test.go
@@ -14,7 +14,41 @@
 
 package build
 
-import "testing"
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"android/soong/ui/logger"
+)
+
+func TestEnsureEmptyDirs(t *testing.T) {
+	ctx := testContext()
+	defer logger.Recover(func(err error) {
+		t.Error(err)
+	})
+
+	tmpDir, err := ioutil.TempDir("", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() {
+		err := os.RemoveAll(tmpDir)
+		if err != nil {
+			t.Errorf("Error removing tmpDir: %v", err)
+		}
+	}()
+
+	ensureEmptyDirectoriesExist(ctx, filepath.Join(tmpDir, "a/b"))
+
+	err = os.Chmod(filepath.Join(tmpDir, "a"), 0555)
+	if err != nil {
+		t.Fatalf("Failed to chown: %v", err)
+	}
+
+	ensureEmptyDirectoriesExist(ctx, filepath.Join(tmpDir, "a"))
+}
 
 func TestStripAnsiEscapes(t *testing.T) {
 	testcases := []struct {