Merge changes I47b9def5,I16661701,I194aeeb8

* changes:
  Export manifest to Make
  Add support for android_test modules
  Reduce boilerplate around bpfix passes
This commit is contained in:
Colin Cross
2018-05-25 01:34:11 +00:00
committed by Gerrit Code Review
7 changed files with 267 additions and 105 deletions

View File

@@ -376,6 +376,33 @@ include $(BUILD_NATIVE_TEST)
expected: `
cc_test {
}
`,
},
{
desc: "Convert LOCAL_MODULE_TAGS tests to java_test",
in: `
include $(CLEAR_VARS)
LOCAL_MODULE_TAGS := tests
include $(BUILD_JAVA_LIBRARY)
include $(CLEAR_VARS)
LOCAL_MODULE_TAGS := tests
include $(BUILD_PACKAGE)
include $(CLEAR_VARS)
LOCAL_MODULE_TAGS := tests
include $(BUILD_HOST_JAVA_LIBRARY)
`,
expected: `
java_test {
}
android_test {
}
java_test_host {
}
`,
},

View File

@@ -45,12 +45,43 @@ func Reformat(input string) (string, error) {
// 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
rewriteIncorrectAndroidmkAndroidLibraries bool
mergeMatchingModuleProperties bool
reorderCommonProperties bool
removeTags bool
steps []fixStep
}
type fixStep struct {
name string
fix func(f *Fixer) error
}
var fixSteps = []fixStep{
{
name: "simplifyKnownRedundantVariables",
fix: runPatchListMod(simplifyKnownPropertiesDuplicatingEachOther),
},
{
name: "rewriteIncorrectAndroidmkPrebuilts",
fix: rewriteIncorrectAndroidmkPrebuilts,
},
{
name: "rewriteIncorrectAndroidmkAndroidLibraries",
fix: rewriteIncorrectAndroidmkAndroidLibraries,
},
{
name: "rewriteTestModuleTypes",
fix: rewriteTestModuleTypes,
},
{
name: "mergeMatchingModuleProperties",
fix: runPatchListMod(mergeMatchingModuleProperties),
},
{
name: "reorderCommonProperties",
fix: runPatchListMod(reorderCommonProperties),
},
{
name: "removeTags",
fix: runPatchListMod(removeTags),
},
}
func NewFixRequest() FixRequest {
@@ -58,13 +89,8 @@ func NewFixRequest() FixRequest {
}
func (r FixRequest) AddAll() (result FixRequest) {
result = r
result.simplifyKnownRedundantVariables = true
result.rewriteIncorrectAndroidmkPrebuilts = true
result.rewriteIncorrectAndroidmkAndroidLibraries = true
result.mergeMatchingModuleProperties = true
result.reorderCommonProperties = true
result.removeTags = true
result.steps = append([]fixStep(nil), r.steps...)
result.steps = append(result.steps, fixSteps...)
return result
}
@@ -148,43 +174,8 @@ func parse(name string, r io.Reader) (*parser.File, error) {
}
func (f *Fixer) fixTreeOnce(config FixRequest) error {
if config.simplifyKnownRedundantVariables {
err := f.runPatchListMod(simplifyKnownPropertiesDuplicatingEachOther)
if err != nil {
return err
}
}
if config.rewriteIncorrectAndroidmkPrebuilts {
err := f.rewriteIncorrectAndroidmkPrebuilts()
if err != nil {
return err
}
}
if config.rewriteIncorrectAndroidmkAndroidLibraries {
err := f.rewriteIncorrectAndroidmkAndroidLibraries()
if err != nil {
return err
}
}
if config.mergeMatchingModuleProperties {
err := f.runPatchListMod(mergeMatchingModuleProperties)
if err != nil {
return err
}
}
if config.reorderCommonProperties {
err := f.runPatchListMod(reorderCommonProperties)
if err != nil {
return err
}
}
if config.removeTags {
err := f.runPatchListMod(removeTags)
for _, fix := range config.steps {
err := fix.fix(f)
if err != nil {
return err
}
@@ -198,7 +189,7 @@ func simplifyKnownPropertiesDuplicatingEachOther(mod *parser.Module, buf []byte,
"export_include_dirs", "local_include_dirs")
}
func (f *Fixer) rewriteIncorrectAndroidmkPrebuilts() error {
func rewriteIncorrectAndroidmkPrebuilts(f *Fixer) error {
for _, def := range f.tree.Defs {
mod, ok := def.(*parser.Module)
if !ok {
@@ -234,7 +225,7 @@ func (f *Fixer) rewriteIncorrectAndroidmkPrebuilts() error {
return nil
}
func (f *Fixer) rewriteIncorrectAndroidmkAndroidLibraries() error {
func rewriteIncorrectAndroidmkAndroidLibraries(f *Fixer) error {
for _, def := range f.tree.Defs {
mod, ok := def.(*parser.Module)
if !ok {
@@ -269,7 +260,48 @@ func (f *Fixer) rewriteIncorrectAndroidmkAndroidLibraries() error {
return nil
}
func (f *Fixer) runPatchListMod(modFunc func(mod *parser.Module, buf []byte, patchlist *parser.PatchList) error) error {
// rewriteTestModuleTypes looks for modules that are identifiable as tests but for which Make doesn't have a separate
// module class, and moves them to the appropriate Soong module type.
func rewriteTestModuleTypes(f *Fixer) error {
for _, def := range f.tree.Defs {
mod, ok := def.(*parser.Module)
if !ok {
continue
}
if !strings.HasPrefix(mod.Type, "java_") && !strings.HasPrefix(mod.Type, "android_") {
continue
}
hasInstrumentationFor := hasNonEmptyLiteralStringProperty(mod, "instrumentation_for")
tags, _ := getLiteralListPropertyValue(mod, "tags")
var hasTestsTag bool
for _, tag := range tags {
if tag == "tests" {
hasTestsTag = true
}
}
isTest := hasInstrumentationFor || hasTestsTag
if isTest {
switch mod.Type {
case "android_app":
mod.Type = "android_test"
case "java_library":
mod.Type = "java_test"
case "java_library_host":
mod.Type = "java_test_host"
}
}
}
return nil
}
func runPatchListMod(modFunc func(mod *parser.Module, buf []byte, patchlist *parser.PatchList) error) func(*Fixer) error {
return func(f *Fixer) error {
// Make sure all the offsets are accurate
buf, err := f.reparse()
if err != nil {
@@ -307,6 +339,7 @@ func (f *Fixer) runPatchListMod(modFunc func(mod *parser.Module, buf []byte, pat
return nil
}
}
var commonPropertyPriorities = []string{
"name",
@@ -394,26 +427,34 @@ func removeTags(mod *parser.Module, buf []byte, patchlist *parser.PatchList) err
// force installation for -eng builds.
`
case "tests":
if strings.Contains(mod.Type, "cc_test") || strings.Contains(mod.Type, "cc_library_static") {
switch {
case strings.Contains(mod.Type, "cc_test"),
strings.Contains(mod.Type, "cc_library_static"),
strings.Contains(mod.Type, "java_test"),
mod.Type == "android_test":
continue
} else if strings.Contains(mod.Type, "cc_lib") {
case strings.Contains(mod.Type, "cc_lib"):
replaceStr += `// WARNING: Module tags are not supported in Soong.
// To make a shared library only for tests, use the "cc_test_library" module
// type. If you don't use gtest, set "gtest: false".
`
} else if strings.Contains(mod.Type, "cc_bin") {
case strings.Contains(mod.Type, "cc_bin"):
replaceStr += `// WARNING: Module tags are not supported in Soong.
// For native test binaries, use the "cc_test" module type. Some differences:
// - If you don't use gtest, set "gtest: false"
// - Binaries will be installed into /data/nativetest[64]/<name>/<name>
// - Both 32 & 64 bit versions will be built (as appropriate)
`
} else if strings.Contains(mod.Type, "java_lib") {
case strings.Contains(mod.Type, "java_lib"):
replaceStr += `// WARNING: Module tags are not supported in Soong.
// For JUnit or similar tests, use the "java_test" module type. A dependency on
// Junit will be added by default, if it is using some other runner, set "junit: false".
`
} else {
case mod.Type == "android_app":
replaceStr += `// WARNING: Module tags are not supported in Soong.
// For JUnit or instrumentataion app tests, use the "android_test" module type.
`
default:
replaceStr += `// WARNING: Module tags are not supported in Soong.
// In most cases, tests are now identified by their module type:
// cc_test, java_test, python_test
@@ -563,6 +604,11 @@ func hasNonEmptyLiteralListProperty(mod *parser.Module, name string) bool {
return found && len(list.Values) > 0
}
func hasNonEmptyLiteralStringProperty(mod *parser.Module, name string) bool {
s, found := getLiteralStringPropertyValue(mod, name)
return found && len(s) > 0
}
func getLiteralListProperty(mod *parser.Module, name string) (list *parser.List, found bool) {
prop, ok := mod.GetProperty(name)
if !ok {
@@ -572,6 +618,40 @@ func getLiteralListProperty(mod *parser.Module, name string) (list *parser.List,
return list, ok
}
func getLiteralListPropertyValue(mod *parser.Module, name string) (list []string, found bool) {
listValue, ok := getLiteralListProperty(mod, name)
if !ok {
return nil, false
}
for _, v := range listValue.Values {
stringValue, ok := v.(*parser.String)
if !ok {
return nil, false
}
list = append(list, stringValue.Value)
}
return list, true
}
func getLiteralStringProperty(mod *parser.Module, name string) (s *parser.String, found bool) {
prop, ok := mod.GetProperty(name)
if !ok {
return nil, false
}
s, ok = prop.Value.(*parser.String)
return s, ok
}
func getLiteralStringPropertyValue(mod *parser.Module, name string) (s string, found bool) {
stringValue, ok := getLiteralStringProperty(mod, name)
if !ok {
return "", false
}
return stringValue.Value, true
}
func propertyIndex(props []*parser.Property, propertyName string) int {
for i, prop := range props {
if prop.Name == propertyName {

View File

@@ -66,7 +66,7 @@ func implFilterListTest(t *testing.T, local_include_dirs []string, export_includ
fixer := NewFixer(tree)
// apply simplifications
err := fixer.runPatchListMod(simplifyKnownPropertiesDuplicatingEachOther)
err := runPatchListMod(simplifyKnownPropertiesDuplicatingEachOther)(fixer)
if len(errs) > 0 {
t.Fatal(err)
}
@@ -251,7 +251,7 @@ func TestMergeMatchingProperties(t *testing.T) {
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(mergeMatchingModuleProperties)
return runPatchListMod(mergeMatchingModuleProperties)(fixer)
})
})
}
@@ -337,7 +337,7 @@ func TestReorderCommonProperties(t *testing.T) {
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)
return runPatchListMod(reorderCommonProperties)(fixer)
})
})
}
@@ -490,9 +490,9 @@ func TestRemoveMatchingModuleListProperties(t *testing.T) {
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 runPatchListMod(func(mod *parser.Module, buf []byte, patchList *parser.PatchList) error {
return removeMatchingModuleListProperties(mod, patchList, "bar", "foo")
})
})(fixer)
})
})
}

View File

@@ -354,6 +354,7 @@ type AARImport struct {
proguardFlags android.WritablePath
exportPackage android.WritablePath
extraAaptPackagesFile android.WritablePath
manifest android.WritablePath
exportedStaticPackages android.Paths
}
@@ -413,12 +414,12 @@ func (a *AARImport) GenerateAndroidBuildActions(ctx android.ModuleContext) {
extractedResDir := extractedAARDir.Join(ctx, "res")
a.classpathFile = extractedAARDir.Join(ctx, "classes.jar")
a.proguardFlags = extractedAARDir.Join(ctx, "proguard.txt")
manifest := extractedAARDir.Join(ctx, "AndroidManifest.xml")
a.manifest = extractedAARDir.Join(ctx, "AndroidManifest.xml")
ctx.Build(pctx, android.BuildParams{
Rule: unzipAAR,
Input: aar,
Outputs: android.WritablePaths{a.classpathFile, a.proguardFlags, manifest},
Outputs: android.WritablePaths{a.classpathFile, a.proguardFlags, a.manifest},
Description: "unzip AAR",
Args: map[string]string{
"expectedDirs": extractedResDir.String(),
@@ -446,8 +447,8 @@ func (a *AARImport) GenerateAndroidBuildActions(ctx android.ModuleContext) {
"--auto-add-overlay",
}
linkFlags = append(linkFlags, "--manifest "+manifest.String())
linkDeps = append(linkDeps, manifest)
linkFlags = append(linkFlags, "--manifest "+a.manifest.String())
linkDeps = append(linkDeps, a.manifest)
transitiveStaticLibs, libDeps, libFlags := aaptLibs(ctx, String(a.properties.Sdk_version))

View File

@@ -136,6 +136,7 @@ func (prebuilt *AARImport) AndroidMk() android.AndroidMkData {
fmt.Fprintln(w, "LOCAL_SOONG_RESOURCE_EXPORT_PACKAGE :=", prebuilt.exportPackage.String())
fmt.Fprintln(w, "LOCAL_SOONG_EXPORT_PROGUARD_FLAGS :=", prebuilt.proguardFlags.String())
fmt.Fprintln(w, "LOCAL_SOONG_STATIC_LIBRARY_EXTRA_PACKAGES :=", prebuilt.extraAaptPackagesFile.String())
fmt.Fprintln(w, "LOCAL_FULL_MANIFEST_FILE :=", prebuilt.manifest.String())
fmt.Fprintln(w, "LOCAL_SDK_VERSION :=", String(prebuilt.properties.Sdk_version))
},
},
@@ -228,6 +229,19 @@ func (app *AndroidApp) AndroidMk() android.AndroidMkData {
}
}
func (a *AndroidTest) AndroidMk() android.AndroidMkData {
data := a.AndroidApp.AndroidMk()
data.Extra = append(data.Extra, func(w io.Writer, outputFile android.Path) {
fmt.Fprintln(w, "LOCAL_MODULE_TAGS := tests")
if len(a.testProperties.Test_suites) > 0 {
fmt.Fprintln(w, "LOCAL_COMPATIBILITY_SUITE :=",
strings.Join(a.testProperties.Test_suites, " "))
}
})
return data
}
func (a *AndroidLibrary) AndroidMk() android.AndroidMkData {
data := a.Library.AndroidMk()

View File

@@ -26,6 +26,7 @@ import (
func init() {
android.RegisterModuleType("android_app", AndroidAppFactory)
android.RegisterModuleType("android_test", AndroidTestFactory)
}
// AndroidManifest.xml merging
@@ -50,8 +51,6 @@ type appProperties struct {
// list of resource labels to generate individual resource packages
Package_splits []string
Instrumentation_for *string
}
type AndroidApp struct {
@@ -61,6 +60,8 @@ type AndroidApp struct {
certificate certificate
appProperties appProperties
extraLinkFlags []string
}
func (a *AndroidApp) ExportedProguardFlagFiles() android.Paths {
@@ -85,15 +86,12 @@ func (a *AndroidApp) DepsMutator(ctx android.BottomUpMutatorContext) {
}
func (a *AndroidApp) GenerateAndroidBuildActions(ctx android.ModuleContext) {
var linkFlags []string
if String(a.appProperties.Instrumentation_for) != "" {
linkFlags = append(linkFlags,
"--rename-instrumentation-target-package",
String(a.appProperties.Instrumentation_for))
} else {
a.properties.Instrument = true
a.generateAndroidBuildActions(ctx)
}
func (a *AndroidApp) generateAndroidBuildActions(ctx android.ModuleContext) {
linkFlags := append([]string(nil), a.extraLinkFlags...)
hasProduct := false
for _, f := range a.aaptProperties.Aaptflags {
if strings.HasPrefix(f, "--product") {
@@ -188,6 +186,8 @@ func AndroidAppFactory() android.Module {
module.Module.deviceProperties.Optimize.Enabled = proptools.BoolPtr(true)
module.Module.deviceProperties.Optimize.Shrink = proptools.BoolPtr(true)
module.Module.properties.Instrument = true
module.AddProperties(
&module.Module.properties,
&module.Module.deviceProperties,
@@ -198,3 +198,43 @@ func AndroidAppFactory() android.Module {
android.InitAndroidArchModule(module, android.DeviceSupported, android.MultilibCommon)
return module
}
type appTestProperties struct {
Instrumentation_for *string
}
type AndroidTest struct {
AndroidApp
appTestProperties appTestProperties
testProperties testProperties
}
func (a *AndroidTest) GenerateAndroidBuildActions(ctx android.ModuleContext) {
if String(a.appTestProperties.Instrumentation_for) != "" {
a.AndroidApp.extraLinkFlags = append(a.AndroidApp.extraLinkFlags,
"--rename-instrumentation-target-package",
String(a.appTestProperties.Instrumentation_for))
}
a.generateAndroidBuildActions(ctx)
}
func AndroidTestFactory() android.Module {
module := &AndroidTest{}
module.Module.deviceProperties.Optimize.Enabled = proptools.BoolPtr(true)
module.AddProperties(
&module.Module.properties,
&module.Module.deviceProperties,
&module.Module.protoProperties,
&module.aaptProperties,
&module.appProperties,
&module.appTestProperties)
android.InitAndroidArchModule(module, android.DeviceSupported, android.MultilibCommon)
return module
}

View File

@@ -212,8 +212,8 @@ type CompilerDeviceProperties struct {
}
Optimize struct {
// If false, disable all optimization. Defaults to true for apps, false for
// libraries and tests.
// If false, disable all optimization. Defaults to true for android_app and android_test
// modules, false for java_library and java_test modules.
Enabled *bool
// If true, optimize for size by removing unused code. Defaults to true for apps,