diff --git a/androidmk/androidmk/androidmk_test.go b/androidmk/androidmk/androidmk_test.go index ca40aaa93..a2d6992e6 100644 --- a/androidmk/androidmk/androidmk_test.go +++ b/androidmk/androidmk/androidmk_test.go @@ -1537,9 +1537,11 @@ android_app { }, { 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. + // When "android_license_files" is valid, the test requires an Android.mk file + // outside the current (and an Android.bp file is required as well if the license + // files locates directory), thus a mock file system is needed. The integration + // test cases for these scenarios have been added in + // $(ANDROID_BUILD_TOP)/build/soong/tests/androidmk_test.sh. in: ` include $(CLEAR_VARS) LOCAL_MODULE := foo diff --git a/bpfix/Android.bp b/bpfix/Android.bp index 345dbd078..a72d9b4e1 100644 --- a/bpfix/Android.bp +++ b/bpfix/Android.bp @@ -52,5 +52,6 @@ bootstrap_go_package { ], deps: [ "blueprint-parser", + "blueprint-pathtools", ], } diff --git a/bpfix/bpfix/bpfix.go b/bpfix/bpfix/bpfix.go index e1140b85f..c0925fe3b 100644 --- a/bpfix/bpfix/bpfix.go +++ b/bpfix/bpfix/bpfix.go @@ -22,6 +22,7 @@ import ( "flag" "fmt" "io" + "io/ioutil" "os" "path/filepath" "reflect" @@ -29,6 +30,7 @@ import ( "strings" "github.com/google/blueprint/parser" + "github.com/google/blueprint/pathtools" ) // Reformat takes a blueprint file as a string and returns a formatted version @@ -166,7 +168,7 @@ var fixStepsOnce = []FixStep{ }, { Name: "rewriteLicenseProperties", - Fix: runPatchListMod(rewriteLicenseProperties), + Fix: runPatchListMod(rewriteLicenseProperty(nil, "")), }, } @@ -1452,9 +1454,16 @@ func formatFlagProperties(mod *parser.Module, buf []byte, patchlist *parser.Patc return nil } +func rewriteLicenseProperty(fs pathtools.FileSystem, relativePath string) patchListModFunction { + return func(mod *parser.Module, buf []byte, patchList *parser.PatchList) error { + return rewriteLicenseProperties(mod, patchList, fs, relativePath) + } +} + // 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 { +func rewriteLicenseProperties(mod *parser.Module, patchList *parser.PatchList, fs pathtools.FileSystem, + relativePath string) error { // if a package module has been added, no more action is needed. for _, patch := range *patchList { if strings.Contains(patch.Replacement, "package {") { @@ -1462,15 +1471,33 @@ func rewriteLicenseProperties(mod *parser.Module, buf []byte, patchList *parser. } } + // initial the fs + if fs == nil { + fs = pathtools.NewOsFs(os.Getenv("ANDROID_BUILD_TOP")) + } + + // initial the relativePath + if len(relativePath) == 0 { + relativePath = getModuleRelativePath() + } + // validate the relativePath + ok := hasFile(relativePath+"/Android.mk", fs) + // some modules in the existing test cases in the androidmk_test.go do not have a valid path + if !ok && len(relativePath) > 0 { + return fmt.Errorf("Cannot find an Android.mk file at path %s", relativePath) + } + licenseKindsPropertyName := "android_license_kinds" licenseFilesPropertyName := "android_license_files" - androidBpFileErr := "// Error: No Android.bp file is found at\n" + + androidBpFileErr := "// Error: No Android.bp file is found at path\n" + "// %s\n" + - "// Please add one there with the needed license module first.\n" + "// Please add one there with the needed license module first.\n" + + "// Then reset the default_applicable_licenses property below with the license module name.\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" + "// If no such license module exists, please add one there first.\n" + + "// Then reset the default_applicable_licenses property below with the license module name.\n" defaultApplicableLicense := "Android-Apache-2.0" var licenseModuleName, licensePatch string @@ -1482,15 +1509,16 @@ func rewriteLicenseProperties(mod *parser.Module, buf []byte, patchList *parser. // 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) + bpPath, ok := getPathFromProperty(mod, licenseFilesPropertyName, fs, relativePath) if !ok { - bpDir, err := getDirFromProperty(mod, licenseFilesPropertyName) + bpDir, err := getDirFromProperty(mod, licenseFilesPropertyName, fs, relativePath) if err != nil { return err } licensePatch += fmt.Sprintf(androidBpFileErr, bpDir) + defaultApplicableLicense = "" } else { - licenseModuleName, _ = getModuleName(bpPath, "license") + licenseModuleName, _ = getModuleName(bpPath, "license", fs) if len(licenseModuleName) == 0 { licensePatch += fmt.Sprintf(licenseModuleErr, bpPath) } @@ -1498,7 +1526,6 @@ func rewriteLicenseProperties(mod *parser.Module, buf []byte, patchList *parser. } } 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") } @@ -1617,17 +1644,14 @@ func getModuleAbsolutePath() string { 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 +// check whether a file exists in a filesystem +func hasFile(path string, fs pathtools.FileSystem) bool { + ok, _, _ := fs.Exists(path) + return ok } // 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) { +func getDirFromProperty(mod *parser.Module, property string, fs pathtools.FileSystem, relativePath string) (string, error) { listValue, ok := getLiteralListPropertyValue(mod, property) if !ok { // if do not find @@ -1637,7 +1661,11 @@ func getDirFromProperty(mod *parser.Module, property string) (string, error) { // if empty return "", fmt.Errorf("Cannot find the value of the %s.%s property", mod.Type, property) } - path := getModuleAbsolutePath() + _, isDir, _ := fs.Exists(relativePath) + if !isDir { + return "", fmt.Errorf("Cannot find the path %s", relativePath) + } + path := relativePath for { if !strings.HasPrefix(listValue[0], "../") { break @@ -1645,25 +1673,29 @@ func getDirFromProperty(mod *parser.Module, property string) (string, error) { path = filepath.Dir(path) listValue[0] = strings.TrimPrefix(listValue[0], "../") } + _, isDir, _ = fs.Exists(path) + if !isDir { + return "", fmt.Errorf("Cannot find the path %s", path) + } 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) +func getPathFromProperty(mod *parser.Module, property string, fs pathtools.FileSystem, relativePath string) (string, bool) { + dir, err := getDirFromProperty(mod, property, fs, relativePath) if err != nil { return "", false } - err = hasFile(dir, "/Android.bp") - if err != nil { + ok := hasFile(dir+"/Android.bp", fs) + if !ok { 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) +func getModuleName(path string, moduleType string, fs pathtools.FileSystem) (string, error) { + tree, err := parserPath(path, fs) if err != nil { return "", err } @@ -1685,8 +1717,13 @@ func getModuleName(path string, moduleType string) (string, error) { } // parse an Android.bp file with the specific path -func parserPath(path string) (tree *parser.File, err error) { - fileContent, _ := os.ReadFile(path) +func parserPath(path string, fs pathtools.FileSystem) (tree *parser.File, err error) { + f, err := fs.Open(path) + if err != nil { + return tree, err + } + defer f.Close() + fileContent, _ := ioutil.ReadAll(f) tree, err = parse(path, bytes.NewBufferString(string(fileContent))) if err != nil { return tree, err diff --git a/bpfix/bpfix/bpfix_test.go b/bpfix/bpfix/bpfix_test.go index e6b6af5f8..221df45ce 100644 --- a/bpfix/bpfix/bpfix_test.go +++ b/bpfix/bpfix/bpfix_test.go @@ -25,6 +25,7 @@ import ( "reflect" "github.com/google/blueprint/parser" + "github.com/google/blueprint/pathtools" ) // TODO(jeffrygaston) remove this when position is removed from ParseNode (in b/38325146) and we can directly do reflect.DeepEqual @@ -1678,10 +1679,20 @@ func TestFormatFlagProperty(t *testing.T) { } } -func TestRewriteLicenseProperties(t *testing.T) { +func TestRewriteLicenseProperty(t *testing.T) { + mockFs := pathtools.MockFs(map[string][]byte{ + "a/b/c/d/Android.mk": []byte("this is not important."), + "a/b/LicenseFile1": []byte("LicenseFile1"), + "a/b/LicenseFile2": []byte("LicenseFile2"), + "a/b/Android.bp": []byte("license {\n\tname: \"reuse_a_b_license\",\n}\n"), + }) + relativePath := "a/b/c/d" + relativePathErr := "a/b/c" tests := []struct { name string in string + fs pathtools.FileSystem + path string out string }{ { @@ -1744,13 +1755,194 @@ func TestRewriteLicenseProperties(t *testing.T) { } `, }, - // 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. + { + name: "license rewriting with license files in the current directory", + in: ` + android_test { + name: "foo", + android_license_kinds: ["license_kind"], + android_license_conditions: ["license_notice"], + android_license_files: ["LicenseFile1", "LicenseFile2",], + } + `, + fs: mockFs, + path: relativePath, + out: ` + package { + // See: http://go/android-license-faq + default_applicable_licenses: [ + "a_b_c_d_license", + ], + } + + license { + name: "a_b_c_d_license", + visibility: [":__subpackages__"], + license_kinds: [ + "license_kind", + ], + license_text: [ + "LicenseFile1", + "LicenseFile2", + ], + } + + android_test { + name: "foo", + android_license_kinds: ["license_kind"], + android_license_conditions: ["license_notice"], + android_license_files: [ + "LicenseFile1", + "LicenseFile2", + ], + } + `, + }, + { + name: "license rewriting with license files outside the current directory", + in: ` + android_test { + name: "foo", + android_license_kinds: ["license_kind"], + android_license_conditions: ["license_notice"], + android_license_files: ["../../LicenseFile1", "../../LicenseFile2",], + } + `, + fs: mockFs, + path: relativePath, + out: ` + package { + // See: http://go/android-license-faq + default_applicable_licenses: [ + "reuse_a_b_license", + ], + } + + android_test { + name: "foo", + android_license_kinds: ["license_kind"], + android_license_conditions: ["license_notice"], + android_license_files: [ + "../../LicenseFile1", + "../../LicenseFile2", + ], + } + `, + }, + { + name: "license rewriting with no Android.bp file in the expected location", + in: ` + android_test { + name: "foo", + android_license_kinds: ["license_kind"], + android_license_conditions: ["license_notice"], + android_license_files: ["../../LicenseFile1", "../../LicenseFile2",], + } + `, + fs: pathtools.MockFs(map[string][]byte{ + "a/b/c/d/Android.mk": []byte("this is not important."), + "a/b/LicenseFile1": []byte("LicenseFile1"), + "a/b/LicenseFile2": []byte("LicenseFile2"), + "a/Android.bp": []byte("license {\n\tname: \"reuse_a_b_license\",\n}\n"), + }), + path: relativePath, + out: ` + // Error: No Android.bp file is found at path + // a/b + // Please add one there with the needed license module first. + // Then reset the default_applicable_licenses property below with the license module name. + package { + // See: http://go/android-license-faq + default_applicable_licenses: [ + "", + ], + } + + android_test { + name: "foo", + android_license_kinds: ["license_kind"], + android_license_conditions: ["license_notice"], + android_license_files: [ + "../../LicenseFile1", + "../../LicenseFile2", + ], + } + `, + }, + { + name: "license rewriting with an Android.bp file without a license module", + in: ` + android_test { + name: "foo", + android_license_kinds: ["license_kind"], + android_license_conditions: ["license_notice"], + android_license_files: ["../../LicenseFile1", "../../LicenseFile2",], + } + `, + fs: pathtools.MockFs(map[string][]byte{ + "a/b/c/d/Android.mk": []byte("this is not important."), + "a/b/LicenseFile1": []byte("LicenseFile1"), + "a/b/LicenseFile2": []byte("LicenseFile2"), + "a/b/Android.bp": []byte("non_license {\n\tname: \"reuse_a_b_license\",\n}\n"), + }), + path: relativePath, + out: ` + // Error: Cannot get the name of the license module in the + // a/b/Android.bp file. + // If no such license module exists, please add one there first. + // Then reset the default_applicable_licenses property below with the license module name. + package { + // See: http://go/android-license-faq + default_applicable_licenses: [ + "", + ], + } + + android_test { + name: "foo", + android_license_kinds: ["license_kind"], + android_license_conditions: ["license_notice"], + android_license_files: [ + "../../LicenseFile1", + "../../LicenseFile2", + ], + } + `, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - runPassOnce(t, test.in, test.out, runPatchListMod(rewriteLicenseProperties)) + runPassOnce(t, test.in, test.out, runPatchListMod(rewriteLicenseProperty(test.fs, test.path))) + }) + } + + testErrs := []struct { + name string + in string + fs pathtools.FileSystem + path string + expectedErr string + }{ + { + name: "license rewriting with a wrong path", + in: ` + android_test { + name: "foo", + android_license_kinds: ["license_kind"], + android_license_conditions: ["license_notice"], + android_license_files: ["../../LicenseFile1", "../../LicenseFile2",], + } + `, + fs: mockFs, + path: relativePathErr, + expectedErr: ` + Cannot find an Android.mk file at path a/b/c + `, + }, + } + for _, test := range testErrs { + t.Run(test.name, func(t *testing.T) { + checkError(t, test.in, test.expectedErr, runPatchListMod(rewriteLicenseProperty(test.fs, test.path))) }) } } diff --git a/tests/androidmk_test.sh b/tests/androidmk_test.sh new file mode 100755 index 000000000..331dc7770 --- /dev/null +++ b/tests/androidmk_test.sh @@ -0,0 +1,135 @@ +#!/bin/bash -eu + +set -o pipefail + +# How to run: bash path-to-script/androidmk_test.sh +# Tests of converting license functionality of the androidmk tool +REAL_TOP="$(readlink -f "$(dirname "$0")"/../../..)" +$REAL_TOP/build/soong/soong_ui.bash --make-mode androidmk + +source "$(dirname "$0")/lib.sh" + +# Expect to create a new license module +function test_rewrite_license_property_inside_current_directory { + setup + + # Create an Android.mk file + mkdir -p a/b + cat > a/b/Android.mk <<'EOF' +include $(CLEAR_VARS) +LOCAL_MODULE := foo +LOCAL_LICENSE_KINDS := license_kind1 license_kind2 +LOCAL_LICENSE_CONDITIONS := license_condition +LOCAL_NOTICE_FILE := $(LOCAL_PATH)/license_notice1 $(LOCAL_PATH)/license_notice2 +include $(BUILD_PACKAGE) +EOF + + # Create an expected Android.bp file for the module "foo" + cat > a/b/Android.bp <<'EOF' +package { + // See: http://go/android-license-faq + default_applicable_licenses: [ + "a_b_license", + ], +} + +license { + name: "a_b_license", + visibility: [":__subpackages__"], + license_kinds: [ + "license_kind1", + "license_kind2", + ], + license_text: [ + "license_notice1", + "license_notice2", + ], +} + +android_app { + name: "foo", +} +EOF + + run_androidmk_test "a/b/Android.mk" "a/b/Android.bp" +} + +# Expect to reference to an existing license module +function test_rewrite_license_property_outside_current_directory { + setup + + # Create an Android.mk file + mkdir -p a/b/c/d + cat > a/b/c/d/Android.mk <<'EOF' +include $(CLEAR_VARS) +LOCAL_MODULE := foo +LOCAL_LICENSE_KINDS := license_kind1 license_kind2 +LOCAL_LICENSE_CONDITIONS := license_condition +LOCAL_NOTICE_FILE := $(LOCAL_PATH)/../../license_notice1 $(LOCAL_PATH)/../../license_notice2 +include $(BUILD_PACKAGE) +EOF + + # Create an expected (input) Android.bp file at a/b/ + cat > a/b/Android.bp <<'EOF' +package { + // See: http://go/android-license-faq + default_applicable_licenses: [ + "a_b_license", + ], +} + +license { + name: "a_b_license", + visibility: [":__subpackages__"], + license_kinds: [ + "license_kind1", + "license_kind2", + ], + license_text: [ + "license_notice1", + "license_notice2", + ], +} + +android_app { + name: "bar", +} +EOF + + # Create an expected (output) Android.bp file for the module "foo" + cat > a/b/c/d/Android.bp <<'EOF' +package { + // See: http://go/android-license-faq + default_applicable_licenses: [ + "a_b_license", + ], +} + +android_app { + name: "foo", +} +EOF + + run_androidmk_test "a/b/c/d/Android.mk" "a/b/c/d/Android.bp" +} + +run_androidmk_test () { + export ANDROID_BUILD_TOP="$MOCK_TOP" + + local out=$($REAL_TOP/*/host/*/bin/androidmk "$1") + local expected=$(<"$2") + + if [[ "$out" != "$expected" ]]; then + ANDROID_BUILD_TOP="$REAL_TOP" + cleanup_mock_top + fail "The output is not the same as the expected" + fi + + ANDROID_BUILD_TOP="$REAL_TOP" + cleanup_mock_top + echo "Succeeded" +} + +test_rewrite_license_property_inside_current_directory + +test_rewrite_license_property_outside_current_directory diff --git a/tests/run_integration_tests.sh b/tests/run_integration_tests.sh index b19949a67..76a918be4 100755 --- a/tests/run_integration_tests.sh +++ b/tests/run_integration_tests.sh @@ -3,6 +3,7 @@ set -o pipefail TOP="$(readlink -f "$(dirname "$0")"/../../..)" +"$TOP/build/soong/tests/androidmk_test.sh" "$TOP/build/soong/tests/bootstrap_test.sh" "$TOP/build/soong/tests/mixed_mode_test.sh" "$TOP/build/soong/tests/bp2build_bazel_test.sh"