From 214cdaf1d49ebaa1c3a77da8724072dd41606eba Mon Sep 17 00:00:00 2001 From: Yuntao Xu Date: Wed, 13 Oct 2021 10:47:54 -0700 Subject: [PATCH] Handle license conversions in androidmk 1. Support license conversions in the androidmk tool. More details can be found at go/license-androidmk; 2. As usage examples, locally this functionality has been applied to aosp/1881088 and aosp/1869664; Bug: 202964622 Test: m androidmk Test: manually ran androidmk Test: TreeHugger Change-Id: I7d5013d25ad8961e997109d0378f20f5085e0ecf --- androidmk/androidmk/android.go | 8 +- androidmk/androidmk/androidmk_test.go | 35 ++- bpfix/bpfix/bpfix.go | 339 ++++++++++++++++++++++++++ bpfix/bpfix/bpfix_test.go | 251 +++++++++++++++++-- 4 files changed, 616 insertions(+), 17 deletions(-) diff --git a/androidmk/androidmk/android.go b/androidmk/androidmk/android.go index f3ad1523b..1045ca6e4 100644 --- a/androidmk/androidmk/android.go +++ b/androidmk/androidmk/android.go @@ -58,6 +58,7 @@ var rewriteProperties = map[string](func(variableAssignmentContext) error){ "LOCAL_MODULE_STEM": stem, "LOCAL_MODULE_HOST_OS": hostOs, "LOCAL_RESOURCE_DIR": localizePathList("resource_dirs"), + "LOCAL_NOTICE_FILE": localizePathList("android_license_files"), "LOCAL_SANITIZE": sanitize(""), "LOCAL_SANITIZE_DIAG": sanitize("diag."), "LOCAL_STRIP_MODULE": strip(), @@ -111,7 +112,6 @@ func init() { "LOCAL_PROTOC_OPTIMIZE_TYPE": "proto.type", "LOCAL_MODULE_OWNER": "owner", "LOCAL_RENDERSCRIPT_TARGET_API": "renderscript.target_api", - "LOCAL_NOTICE_FILE": "notice", "LOCAL_JAVA_LANGUAGE_VERSION": "java_version", "LOCAL_INSTRUMENTATION_FOR": "instrumentation_for", "LOCAL_MANIFEST_FILE": "manifest", @@ -185,6 +185,12 @@ func init() { "LOCAL_JACK_COVERAGE_EXCLUDE_FILTER": "jacoco.exclude_filter", "LOCAL_FULL_LIBS_MANIFEST_FILES": "additional_manifests", + + // will be rewrite later to "license_kinds:" by byfix + "LOCAL_LICENSE_KINDS": "android_license_kinds", + // will be removed later by byfix + // TODO: does this property matter in the license module? + "LOCAL_LICENSE_CONDITIONS": "android_license_conditions", }) addStandardProperties(bpparser.BoolType, diff --git a/androidmk/androidmk/androidmk_test.go b/androidmk/androidmk/androidmk_test.go index 775a9a88e..ca40aaa93 100644 --- a/androidmk/androidmk/androidmk_test.go +++ b/androidmk/androidmk/androidmk_test.go @@ -1516,7 +1516,8 @@ android_app { ], } `, - }, { + }, + { desc: "Obsolete LOCAL_MODULE_PATH", in: ` include $(CLEAR_VARS) @@ -1532,7 +1533,37 @@ android_app { name: "foo", } -`}, +`, + }, + { + desc: "LOCAL_LICENSE_KINDS, LOCAL_LICENSE_CONDITIONS, LOCAL_NOTICE_FILE", + // TODO(b/205615944): When valid "android_license_files" exists, the test requires an Android.mk + // file (and an Android.bp file is required as well if the license files locates outside the current + // directory). So plan to use a mock file system to mock the Android.mk and Android.bp files. + in: ` +include $(CLEAR_VARS) +LOCAL_MODULE := foo +LOCAL_LICENSE_KINDS := license_kind +LOCAL_LICENSE_CONDITIONS := license_condition +LOCAL_NOTICE_FILE := license_notice +include $(BUILD_PACKAGE) +`, + expected: ` +package { + // See: http://go/android-license-faq + default_applicable_licenses: [ + "Android-Apache-2.0", + ], +} + +android_app { + name: "foo", + // ANDROIDMK TRANSLATION ERROR: Only $(LOCAL_PATH)/.. values are allowed + // LOCAL_NOTICE_FILE := license_notice + +} +`, + }, } func TestEndToEnd(t *testing.T) { diff --git a/bpfix/bpfix/bpfix.go b/bpfix/bpfix/bpfix.go index a60863036..e1140b85f 100644 --- a/bpfix/bpfix/bpfix.go +++ b/bpfix/bpfix/bpfix.go @@ -19,9 +19,13 @@ package bpfix import ( "bytes" "errors" + "flag" "fmt" "io" + "os" "path/filepath" + "reflect" + "sort" "strings" "github.com/google/blueprint/parser" @@ -136,12 +140,36 @@ var fixSteps = []FixStep{ Name: "removeScudoProperty", Fix: runPatchListMod(removeObsoleteProperty("sanitize.scudo")), }, + { + Name: "removeAndroidLicenseKinds", + Fix: runPatchListMod(removeIncorrectProperties("android_license_kinds")), + }, + { + Name: "removeAndroidLicenseConditions", + Fix: runPatchListMod(removeIncorrectProperties("android_license_conditions")), + }, + { + Name: "removeAndroidLicenseFiles", + Fix: runPatchListMod(removeIncorrectProperties("android_license_files")), + }, { Name: "formatFlagProperties", Fix: runPatchListMod(formatFlagProperties), }, } +// for fix that only need to run once +var fixStepsOnce = []FixStep{ + { + Name: "haveSameLicense", + Fix: haveSameLicense, + }, + { + Name: "rewriteLicenseProperties", + Fix: runPatchListMod(rewriteLicenseProperties), + }, +} + func NewFixRequest() FixRequest { return FixRequest{} } @@ -196,6 +224,16 @@ func (f *Fixer) Fix(config FixRequest) (*parser.File, error) { return nil, err } + // run fix that is expected to run once first + configOnce := NewFixRequest() + configOnce.steps = append(configOnce.steps, fixStepsOnce...) + if len(configOnce.steps) > 0 { + err = f.fixTreeOnce(configOnce) + if err != nil { + return nil, err + } + } + maxNumIterations := 20 i := 0 for { @@ -1413,3 +1451,304 @@ func formatFlagProperties(mod *parser.Module, buf []byte, patchlist *parser.Patc } return nil } + +// rewrite the "android_license_kinds" and "android_license_files" properties to a package module +// (and a license module when needed). +func rewriteLicenseProperties(mod *parser.Module, buf []byte, patchList *parser.PatchList) error { + // if a package module has been added, no more action is needed. + for _, patch := range *patchList { + if strings.Contains(patch.Replacement, "package {") { + return nil + } + } + + licenseKindsPropertyName := "android_license_kinds" + licenseFilesPropertyName := "android_license_files" + + androidBpFileErr := "// Error: No Android.bp file is found at\n" + + "// %s\n" + + "// Please add one there with the needed license module first.\n" + licenseModuleErr := "// Error: Cannot get the name of the license module in the\n" + + "// %s file.\n" + + "// If no such license module exists, please add one there first.\n" + + defaultApplicableLicense := "Android-Apache-2.0" + var licenseModuleName, licensePatch string + var hasFileInParentDir bool + + // when LOCAL_NOTICE_FILE is not empty + if hasNonEmptyLiteralListProperty(mod, licenseFilesPropertyName) { + hasFileInParentDir = hasValueStartWithTwoDotsLiteralList(mod, licenseFilesPropertyName) + // if have LOCAL_NOTICE_FILE outside the current directory, need to find and refer to the license + // module in the LOCAL_NOTICE_FILE location directly and no new license module needs to be created + if hasFileInParentDir { + bpPath, ok := getPathFromProperty(mod, licenseFilesPropertyName) + if !ok { + bpDir, err := getDirFromProperty(mod, licenseFilesPropertyName) + if err != nil { + return err + } + licensePatch += fmt.Sprintf(androidBpFileErr, bpDir) + } else { + licenseModuleName, _ = getModuleName(bpPath, "license") + if len(licenseModuleName) == 0 { + licensePatch += fmt.Sprintf(licenseModuleErr, bpPath) + } + defaultApplicableLicense = licenseModuleName + } + } else { + // if have LOCAL_NOTICE_FILE in the current directory, need to create a new license module + relativePath := getModuleRelativePath() + if len(relativePath) == 0 { + return fmt.Errorf("Cannot obtain the relative path of the Android.mk file") + } + licenseModuleName = strings.Replace(relativePath, "/", "_", -1) + "_license" + defaultApplicableLicense = licenseModuleName + } + } + + //add the package module + if hasNonEmptyLiteralListProperty(mod, licenseKindsPropertyName) { + licensePatch += "package {\n" + + " // See: http://go/android-license-faq\n" + + " default_applicable_licenses: [\n" + + " \"" + defaultApplicableLicense + "\",\n" + + " ],\n" + + "}\n" + + "\n" + } + + // append the license module when necessary + // when LOCAL_NOTICE_FILE is not empty and in the current directory, create a new license module + // otherwise, use the above default license directly + if hasNonEmptyLiteralListProperty(mod, licenseFilesPropertyName) && !hasFileInParentDir { + licenseKinds, err := mergeLiteralListPropertyValue(mod, licenseKindsPropertyName) + if err != nil { + return err + } + licenseFiles, err := mergeLiteralListPropertyValue(mod, licenseFilesPropertyName) + if err != nil { + return err + } + licensePatch += "license {\n" + + " name: \"" + licenseModuleName + "\",\n" + + " visibility: [\":__subpackages__\"],\n" + + " license_kinds: [\n" + + licenseKinds + + " ],\n" + + " license_text: [\n" + + licenseFiles + + " ],\n" + + "}\n" + + "\n" + } + + // add to the patchList + pos := mod.Pos().Offset + err := patchList.Add(pos, pos, licensePatch) + if err != nil { + return err + } + return nil +} + +// merge the string vaules in a list property of a module into one string with expected format +func mergeLiteralListPropertyValue(mod *parser.Module, property string) (s string, err error) { + listValue, ok := getLiteralListPropertyValue(mod, property) + if !ok { + // if do not find + return "", fmt.Errorf("Cannot retrieve the %s.%s field", mod.Type, property) + } + for i := 0; i < len(listValue); i++ { + s += " \"" + listValue[i] + "\",\n" + } + return s, nil +} + +// check whether a string list property has any value starting with `../` +func hasValueStartWithTwoDotsLiteralList(mod *parser.Module, property string) bool { + listValue, ok := getLiteralListPropertyValue(mod, property) + if ok { + for i := 0; i < len(listValue); i++ { + if strings.HasPrefix(listValue[i], "../") { + return true + } + } + } + return false +} + +// get the relative path from ANDROID_BUILD_TOP to the Android.mk file to be converted +func getModuleRelativePath() string { + // get the absolute path of the top of the tree + rootPath := os.Getenv("ANDROID_BUILD_TOP") + // get the absolute path of the `Android.mk` file to be converted + absPath := getModuleAbsolutePath() + // get the relative path of the `Android.mk` file to top of the tree + relModulePath, err := filepath.Rel(rootPath, absPath) + if err != nil { + return "" + } + return relModulePath +} + +// get the absolute path of the Android.mk file to be converted +func getModuleAbsolutePath() string { + // get the absolute path at where the `androidmk` commend is executed + curAbsPath, err := filepath.Abs(".") + if err != nil { + return "" + } + // the argument for the `androidmk` command could be + // 1. "./a/b/c/Android.mk"; 2. "a/b/c/Android.mk"; 3. "Android.mk" + argPath := flag.Arg(0) + if strings.HasPrefix(argPath, "./") { + argPath = strings.TrimPrefix(argPath, ".") + } + argPath = strings.TrimSuffix(argPath, "Android.mk") + if strings.HasSuffix(argPath, "/") { + argPath = strings.TrimSuffix(argPath, "/") + } + if len(argPath) > 0 && !strings.HasPrefix(argPath, "/") { + argPath = "/" + argPath + } + // get the absolute path of the `Android.mk` file to be converted + absPath := curAbsPath + argPath + return absPath +} + +// check whether a file exists in a directory +func hasFile(dir string, fileName string) error { + _, err := os.Stat(dir + fileName) + if err != nil { + return err + } + return nil +} + +// get the directory where an `Android.bp` file and the property files are expected to locate +func getDirFromProperty(mod *parser.Module, property string) (string, error) { + listValue, ok := getLiteralListPropertyValue(mod, property) + if !ok { + // if do not find + return "", fmt.Errorf("Cannot retrieve the %s.%s property", mod.Type, property) + } + if len(listValue) == 0 { + // if empty + return "", fmt.Errorf("Cannot find the value of the %s.%s property", mod.Type, property) + } + path := getModuleAbsolutePath() + for { + if !strings.HasPrefix(listValue[0], "../") { + break + } + path = filepath.Dir(path) + listValue[0] = strings.TrimPrefix(listValue[0], "../") + } + return path, nil +} + +// get the path of the `Android.bp` file at the expected location where the property files locate +func getPathFromProperty(mod *parser.Module, property string) (string, bool) { + dir, err := getDirFromProperty(mod, property) + if err != nil { + return "", false + } + err = hasFile(dir, "/Android.bp") + if err != nil { + return "", false + } + return dir + "/Android.bp", true +} + +// parse an Android.bp file to get the name of the first module with type of moduleType +func getModuleName(path string, moduleType string) (string, error) { + tree, err := parserPath(path) + if err != nil { + return "", err + } + for _, def := range tree.Defs { + mod, ok := def.(*parser.Module) + if !ok || mod.Type != moduleType { + continue + } + prop, ok := mod.GetProperty("name") + if !ok { + return "", fmt.Errorf("Cannot get the %s."+"name property", mod.Type) + } + propVal, ok := prop.Value.(*parser.String) + if ok { + return propVal.Value, nil + } + } + return "", fmt.Errorf("Cannot find the value of the %s."+"name property", moduleType) +} + +// parse an Android.bp file with the specific path +func parserPath(path string) (tree *parser.File, err error) { + fileContent, _ := os.ReadFile(path) + tree, err = parse(path, bytes.NewBufferString(string(fileContent))) + if err != nil { + return tree, err + } + return tree, nil +} + +// remove the incorrect property that Soong does not support +func removeIncorrectProperties(propName string) patchListModFunction { + return removeObsoleteProperty(propName) +} + +// the modules on the same Android.mk file are expected to have the same license +func haveSameLicense(f *Fixer) error { + androidLicenseProperties := []string{ + "android_license_kinds", + "android_license_conditions", + "android_license_files", + } + + var prevModuleName string + var prevLicenseKindsVals, prevLicenseConditionsVals, prevLicenseFilesVals []string + prevLicenseVals := [][]string{ + prevLicenseKindsVals, + prevLicenseConditionsVals, + prevLicenseFilesVals, + } + + for _, def := range f.tree.Defs { + mod, ok := def.(*parser.Module) + if !ok { + continue + } + for idx, property := range androidLicenseProperties { + curModuleName, ok := getLiteralStringPropertyValue(mod, "name") + // some modules in the existing test cases in the androidmk_test.go do not have name property + hasNameProperty := hasProperty(mod, "name") + if hasNameProperty && (!ok || len(curModuleName) == 0) { + return fmt.Errorf("Cannot retrieve the name property of a module of %s type.", mod.Type) + } + curVals, ok := getLiteralListPropertyValue(mod, property) + // some modules in the existing test cases in the androidmk_test.go do not have license-related property + hasLicenseProperty := hasProperty(mod, property) + if hasLicenseProperty && (!ok || len(curVals) == 0) { + // if do not find the property, or no value is found for the property + return fmt.Errorf("Cannot retrieve the %s.%s property", mod.Type, property) + } + if len(prevLicenseVals[idx]) > 0 { + if !reflect.DeepEqual(prevLicenseVals[idx], curVals) { + return fmt.Errorf("Modules %s and %s are expected to have the same %s property.", + prevModuleName, curModuleName, property) + } + } + sort.Strings(curVals) + prevLicenseVals[idx] = curVals + prevModuleName = curModuleName + } + } + return nil +} + +func hasProperty(mod *parser.Module, propName string) bool { + _, ok := mod.GetProperty(propName) + return ok +} diff --git a/bpfix/bpfix/bpfix_test.go b/bpfix/bpfix/bpfix_test.go index d8772c10d..e6b6af5f8 100644 --- a/bpfix/bpfix/bpfix_test.go +++ b/bpfix/bpfix/bpfix_test.go @@ -125,34 +125,103 @@ func TestSimplifyKnownVariablesDuplicatingEachOther(t *testing.T) { implFilterListTest(t, []string{}, []string{}, []string{}) } -func runPass(t *testing.T, in, out string, innerTest func(*Fixer) error) { - expected, err := Reformat(out) +func checkError(t *testing.T, in, expectedErr string, innerTest func(*Fixer) error) { + expected := preProcessOutErr(expectedErr) + runTestOnce(t, in, expected, innerTest) +} + +func runTestOnce(t *testing.T, in, expected string, innerTest func(*Fixer) error) { + fixer, err := preProcessIn(in) if err != nil { t.Fatal(err) } + out, err := runFixerOnce(fixer, innerTest) + if err != nil { + out = err.Error() + } + + compareResult := compareOutExpected(in, out, expected) + if len(compareResult) > 0 { + t.Errorf(compareResult) + } +} + +func preProcessOutErr(expectedErr string) string { + expected := strings.TrimSpace(expectedErr) + return expected +} + +func preProcessOut(out string) (expected string, err error) { + expected, err = Reformat(out) + if err != nil { + return expected, err + } + return expected, nil +} + +func preProcessIn(in string) (fixer *Fixer, err error) { in, err = Reformat(in) if err != nil { - t.Fatal(err) + return fixer, err } tree, errs := parser.Parse("", bytes.NewBufferString(in), parser.NewScope(nil)) if errs != nil { - t.Fatal(errs) + return fixer, err } - fixer := NewFixer(tree) + fixer = NewFixer(tree) + + return fixer, nil +} + +func runFixerOnce(fixer *Fixer, innerTest func(*Fixer) error) (string, error) { + err := innerTest(fixer) + if err != nil { + return "", err + } + + out, err := parser.Print(fixer.tree) + if err != nil { + return "", err + } + return string(out), nil +} + +func compareOutExpected(in, out, expected string) string { + if out != expected { + return fmt.Sprintf("output didn't match:\ninput:\n%s\n\nexpected:\n%s\ngot:\n%s\n", + in, expected, out) + } + return "" +} + +func runPassOnce(t *testing.T, in, out string, innerTest func(*Fixer) error) { + expected, err := preProcessOut(out) + if err != nil { + t.Fatal(err) + } + + runTestOnce(t, in, expected, innerTest) +} + +func runPass(t *testing.T, in, out string, innerTest func(*Fixer) error) { + expected, err := preProcessOut(out) + if err != nil { + t.Fatal(err) + } + + fixer, err := preProcessIn(in) + if err != nil { + t.Fatal(err) + } 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) + out, err = runFixerOnce(fixer, innerTest) if err != nil { t.Fatal(err) } @@ -162,9 +231,9 @@ func runPass(t *testing.T, in, out string, innerTest func(*Fixer) error) { passes++ } - if got != expected { - t.Errorf("output didn't match:\ninput:\n%s\n\nexpected:\n%s\ngot:\n%s\n", - in, expected, got) + compareResult := compareOutExpected(in, out, expected) + if len(compareResult) > 0 { + t.Errorf(compareResult) } } @@ -1608,3 +1677,157 @@ func TestFormatFlagProperty(t *testing.T) { }) } } + +func TestRewriteLicenseProperties(t *testing.T) { + tests := []struct { + name string + in string + out string + }{ + { + name: "license rewriting with one module", + in: ` + android_test { + name: "foo", + android_license_kinds: ["license_kind"], + android_license_conditions: ["license_notice"], + } + `, + out: ` + package { + // See: http://go/android-license-faq + default_applicable_licenses: [ + "Android-Apache-2.0", + ], + } + + android_test { + name: "foo", + android_license_kinds: ["license_kind"], + android_license_conditions: ["license_notice"], + } + `, + }, + { + name: "license rewriting with two modules", + in: ` + android_test { + name: "foo1", + android_license_kinds: ["license_kind1"], + android_license_conditions: ["license_notice1"], + } + + android_test { + name: "foo2", + android_license_kinds: ["license_kind2"], + android_license_conditions: ["license_notice2"], + } + `, + out: ` + package { + // See: http://go/android-license-faq + default_applicable_licenses: [ + "Android-Apache-2.0", + ], + } + + android_test { + name: "foo1", + android_license_kinds: ["license_kind1"], + android_license_conditions: ["license_notice1"], + } + + android_test { + name: "foo2", + android_license_kinds: ["license_kind2"], + android_license_conditions: ["license_notice2"], + } + `, + }, + // TODO(b/205615944): When valid "android_license_files" exists, the test requires an Android.mk + // file (and an Android.bp file is required as well if the license files locates outside the current + // directory). So plan to use a mock file system to mock the Android.mk and Android.bp files. + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + runPassOnce(t, test.in, test.out, runPatchListMod(rewriteLicenseProperties)) + }) + } +} + +func TestHaveSameLicense(t *testing.T) { + tests := []struct { + name string + in string + out string + }{ + { + name: "two modules with the same license", + in: ` + android_test { + name: "foo1", + android_license_kinds: ["license_kind"], + android_license_conditions: ["license_notice"], + } + + android_test { + name: "foo2", + android_license_kinds: ["license_kind"], + android_license_conditions: ["license_notice"], + } + `, + out: ` + android_test { + name: "foo1", + android_license_kinds: ["license_kind"], + android_license_conditions: ["license_notice"], + } + + android_test { + name: "foo2", + android_license_kinds: ["license_kind"], + android_license_conditions: ["license_notice"], + } + `, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + runPassOnce(t, test.in, test.out, func(fixer *Fixer) error { + return haveSameLicense(fixer) + }) + }) + } + testErrs := []struct { + name string + in string + expectedErr string + }{ + { + name: "two modules will different licenses", + in: ` + android_test { + name: "foo1", + android_license_kinds: ["license_kind1"], + android_license_conditions: ["license_notice1"], + } + + android_test { + name: "foo2", + android_license_kinds: ["license_kind2"], + android_license_conditions: ["license_notice2"], + } + `, + expectedErr: ` + Modules foo1 and foo2 are expected to have the same android_license_kinds property. + `, + }, + } + for _, test := range testErrs { + t.Run(test.name, func(t *testing.T) { + checkError(t, test.in, test.expectedErr, func(fixer *Fixer) error { + return haveSameLicense(fixer) + }) + }) + } +}