Merge "Add imports property to py_library rules" am: bd4c14a7f9

Original change: https://android-review.googlesource.com/c/platform/build/soong/+/2100587

Change-Id: I5f7694e23d843d36c655b6ac0c252e4ec2551e13
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

bp2build/build_conversion_test.go

@@ -1296,6 +1296,7 @@ python_library {
         "//conditions:default": [],
     })`,
 					"srcs_version": `"PY3"`,
+					"imports":      `["."]`,
 				}),
 			},
 		},
@@ -1321,6 +1322,7 @@ python_library {
         ":reqd",
     ]`,
 					"srcs_version": `"PY3"`,
+					"imports":      `["."]`,
 				}),
 			},
 		},

bp2build/python_library_conversion_test.go

@@ -2,6 +2,7 @@ package bp2build
 
 import (
 	"fmt"
+	"strings"
 	"testing"
 
 	"android/soong/android"
@@ -16,6 +17,8 @@ type pythonLibBp2BuildTestCase struct {
 	filesystem           map[string]string
 	blueprint            string
 	expectedBazelTargets []testBazelTarget
+	dir                  string
+	expectedError        error
 }
 
 func convertPythonLibTestCaseToBp2build_Host(tc pythonLibBp2BuildTestCase) bp2buildTestCase {
@@ -34,11 +37,19 @@ func convertPythonLibTestCaseToBp2build(tc pythonLibBp2BuildTestCase) bp2buildTe
 	for _, t := range tc.expectedBazelTargets {
 		bp2BuildTargets = append(bp2BuildTargets, makeBazelTarget(t.typ, t.name, t.attrs))
 	}
+	// Copy the filesystem so that we can change stuff in it later without it
+	// affecting the original pythonLibBp2BuildTestCase
+	filesystemCopy := make(map[string]string)
+	for k, v := range tc.filesystem {
+		filesystemCopy[k] = v
+	}
 	return bp2buildTestCase{
 		description:          tc.description,
-		filesystem:           tc.filesystem,
+		filesystem:           filesystemCopy,
 		blueprint:            tc.blueprint,
 		expectedBazelTargets: bp2BuildTargets,
+		dir:                  tc.dir,
+		expectedErr:          tc.expectedError,
 	}
 }
 
@@ -47,6 +58,11 @@ func runPythonLibraryTestCase(t *testing.T, tc pythonLibBp2BuildTestCase) {
 	testCase := convertPythonLibTestCaseToBp2build(tc)
 	testCase.description = fmt.Sprintf(testCase.description, "python_library")
 	testCase.blueprint = fmt.Sprintf(testCase.blueprint, "python_library")
+	for name, contents := range testCase.filesystem {
+		if strings.HasSuffix(name, "Android.bp") {
+			testCase.filesystem[name] = fmt.Sprintf(contents, "python_library")
+		}
+	}
 	testCase.moduleTypeUnderTest = "python_library"
 	testCase.moduleTypeUnderTestFactory = python.PythonLibraryFactory
 
@@ -58,6 +74,11 @@ func runPythonLibraryHostTestCase(t *testing.T, tc pythonLibBp2BuildTestCase) {
 	testCase := convertPythonLibTestCaseToBp2build_Host(tc)
 	testCase.description = fmt.Sprintf(testCase.description, "python_library_host")
 	testCase.blueprint = fmt.Sprintf(testCase.blueprint, "python_library_host")
+	for name, contents := range testCase.filesystem {
+		if strings.HasSuffix(name, "Android.bp") {
+			testCase.filesystem[name] = fmt.Sprintf(contents, "python_library_host")
+		}
+	}
 	testCase.moduleTypeUnderTest = "python_library_host"
 	testCase.moduleTypeUnderTestFactory = python.PythonLibraryHostFactory
 	runBp2BuildTestCase(t, func(ctx android.RegistrationContext) {
@@ -109,6 +130,7 @@ func TestSimplePythonLib(t *testing.T) {
         "b/d.py",
     ]`,
 						"srcs_version": `"PY3"`,
+						"imports":      `["."]`,
 					},
 				},
 			},
@@ -136,6 +158,7 @@ func TestSimplePythonLib(t *testing.T) {
 					attrs: attrNameToString{
 						"srcs":         `["a.py"]`,
 						"srcs_version": `"PY2"`,
+						"imports":      `["."]`,
 					},
 				},
 			},
@@ -163,6 +186,7 @@ func TestSimplePythonLib(t *testing.T) {
 					attrs: attrNameToString{
 						"srcs":         `["a.py"]`,
 						"srcs_version": `"PY3"`,
+						"imports":      `["."]`,
 					},
 				},
 			},
@@ -189,11 +213,54 @@ func TestSimplePythonLib(t *testing.T) {
 					typ:  "py_library",
 					name: "foo",
 					attrs: attrNameToString{
-						"srcs": `["a.py"]`,
+						"srcs":    `["a.py"]`,
+						"imports": `["."]`,
 					},
 				},
 			},
 		},
+		{
+			description: "%s: pkg_path in a subdirectory of the same name converts correctly",
+			dir:         "mylib/subpackage",
+			filesystem: map[string]string{
+				"mylib/subpackage/a.py": "",
+				"mylib/subpackage/Android.bp": `%s {
+				name: "foo",
+				srcs: ["a.py"],
+				pkg_path: "mylib/subpackage",
+
+				bazel_module: { bp2build_available: true },
+			}`,
+			},
+			blueprint: `%s {name: "bar"}`,
+			expectedBazelTargets: []testBazelTarget{
+				{
+					// srcs_version is PY2ANDPY3 by default.
+					typ:  "py_library",
+					name: "foo",
+					attrs: attrNameToString{
+						"srcs":         `["a.py"]`,
+						"imports":      `["../.."]`,
+						"srcs_version": `"PY3"`,
+					},
+				},
+			},
+		},
+		{
+			description: "%s: pkg_path in a subdirectory of a different name fails",
+			dir:         "mylib/subpackage",
+			filesystem: map[string]string{
+				"mylib/subpackage/a.py": "",
+				"mylib/subpackage/Android.bp": `%s {
+				name: "foo",
+				srcs: ["a.py"],
+				pkg_path: "mylib/subpackage2",
+				bazel_module: { bp2build_available: true },
+			}`,
+			},
+			blueprint:     `%s {name: "bar"}`,
+			expectedError: fmt.Errorf("Currently, bp2build only supports pkg_paths that are the same as the folders the Android.bp file is in."),
+		},
 	}
 
 	for _, tc := range testCases {
@@ -232,6 +299,7 @@ func TestPythonArchVariance(t *testing.T) {
         "//conditions:default": [],
     })`,
 					"srcs_version": `"PY3"`,
+					"imports":      `["."]`,
 				},
 			},
 		},

bp2build/testing.go

@@ -119,8 +119,8 @@ func runBp2BuildTestCase(t *testing.T, registerModuleTypes func(ctx android.Regi
 		return
 	}
 
-	errs := append(parseErrs, resolveDepsErrs...)
+	parseAndResolveErrs := append(parseErrs, resolveDepsErrs...)
-	if tc.expectedErr != nil && checkError(t, errs, tc.expectedErr) {
+	if tc.expectedErr != nil && checkError(t, parseAndResolveErrs, tc.expectedErr) {
 		return
 	}
 
@@ -135,7 +135,7 @@ func runBp2BuildTestCase(t *testing.T, registerModuleTypes func(ctx android.Regi
 		if checkError(t, errs, tc.expectedErr) {
 			return
 		} else {
-			t.Errorf("Expected error: %q, got: %q", tc.expectedErr, errs)
+			t.Errorf("Expected error: %q, got: %q and %q", tc.expectedErr, errs, parseAndResolveErrs)
 		}
 	} else {
 		android.FailIfErrored(t, errs)

python/library.go

@@ -17,6 +17,9 @@ package python
 // This file contains the module types for building Python library.
 
 import (
+	"path/filepath"
+	"strings"
+
 	"android/soong/android"
 	"android/soong/bazel"
 
@@ -43,6 +46,7 @@ func PythonLibraryHostFactory() android.Module {
 type bazelPythonLibraryAttributes struct {
 	Srcs         bazel.LabelListAttribute
 	Deps         bazel.LabelListAttribute
+	Imports      bazel.StringListAttribute
 	Srcs_version *string
 }
 
@@ -64,16 +68,44 @@ func pythonLibBp2Build(ctx android.TopDownMutatorContext, m *Module) {
 		// do nothing, since python_version defaults to PY2ANDPY3
 	}
 
+	// Bazel normally requires `import path.from.top.of.tree` statements in
+	// python code, but with soong you can directly import modules from libraries.
+	// Add "imports" attributes to the bazel library so it matches soong's behavior.
+	imports := "."
+	if m.properties.Pkg_path != nil {
+		// TODO(b/215119317) This is a hack to handle the fact that we don't convert
+		// pkg_path properly right now. If the folder structure that contains this
+		// Android.bp file matches pkg_path, we can set imports to an appropriate
+		// number of ../..s to emulate moving the files under a pkg_path folder.
+		pkg_path := filepath.Clean(*m.properties.Pkg_path)
+		if strings.HasPrefix(pkg_path, "/") {
+			ctx.ModuleErrorf("pkg_path cannot start with a /: %s", pkg_path)
+			return
+		}
+
+		if !strings.HasSuffix(ctx.ModuleDir(), "/"+pkg_path) && ctx.ModuleDir() != pkg_path {
+			ctx.ModuleErrorf("Currently, bp2build only supports pkg_paths that are the same as the folders the Android.bp file is in. pkg_path: %s, module directory: %s", pkg_path, ctx.ModuleDir())
+			return
+		}
+		numFolders := strings.Count(pkg_path, "/") + 1
+		dots := make([]string, numFolders)
+		for i := 0; i < numFolders; i++ {
+			dots[i] = ".."
+		}
+		imports = strings.Join(dots, "/")
+	}
+
 	baseAttrs := m.makeArchVariantBaseAttributes(ctx)
 	attrs := &bazelPythonLibraryAttributes{
 		Srcs:         baseAttrs.Srcs,
 		Deps:         baseAttrs.Deps,
 		Srcs_version: python_version,
+		Imports:      bazel.MakeStringListAttribute([]string{imports}),
 	}
 
 	props := bazel.BazelTargetModuleProperties{
-		Rule_class:        "py_library",
+		// Use the native py_library rule.
-		Bzl_load_location: "//build/bazel/rules/python:library.bzl",
+		Rule_class: "py_library",
 	}
 
 	ctx.CreateBazelTargetModule(props, android.CommonAttributes{