Add imports property to py_library rules
This is to avoid having it hardcoded in a fork of the
py_library rule.
Most import attributes should just be set to ".", but
our previous solution always hardcoded it to ".." instead,
for ndkstubgen. ndkstubgen uses pkg_path: "ndkstubgen",
i.e., it set pkg_path to the name of the folder that
contained the Android.bp file. In this specific scenario,
imports = ".." works. Recreate that behavior here as well,
because we don't handle pkg_path properly yet.
Fixes: 233081071
Test: build/bazel/ci/bp2build.sh
Change-Id: Ib5e6a8edf428c74d4b5947f0ec53a2151001367a
diff --git a/bp2build/build_conversion_test.go b/bp2build/build_conversion_test.go
index 0f3ca79..0f6e139 100644
--- a/bp2build/build_conversion_test.go
+++ b/bp2build/build_conversion_test.go
@@ -1296,6 +1296,7 @@
"//conditions:default": [],
})`,
"srcs_version": `"PY3"`,
+ "imports": `["."]`,
}),
},
},
@@ -1321,6 +1322,7 @@
":reqd",
]`,
"srcs_version": `"PY3"`,
+ "imports": `["."]`,
}),
},
},
diff --git a/bp2build/python_library_conversion_test.go b/bp2build/python_library_conversion_test.go
index 356d52e..66c2290 100644
--- a/bp2build/python_library_conversion_test.go
+++ b/bp2build/python_library_conversion_test.go
@@ -2,6 +2,7 @@
import (
"fmt"
+ "strings"
"testing"
"android/soong/android"
@@ -16,6 +17,8 @@
filesystem map[string]string
blueprint string
expectedBazelTargets []testBazelTarget
+ dir string
+ expectedError error
}
func convertPythonLibTestCaseToBp2build_Host(tc pythonLibBp2BuildTestCase) bp2buildTestCase {
@@ -34,11 +37,19 @@
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 @@
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 @@
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 @@
"b/d.py",
]`,
"srcs_version": `"PY3"`,
+ "imports": `["."]`,
},
},
},
@@ -136,6 +158,7 @@
attrs: attrNameToString{
"srcs": `["a.py"]`,
"srcs_version": `"PY2"`,
+ "imports": `["."]`,
},
},
},
@@ -163,6 +186,7 @@
attrs: attrNameToString{
"srcs": `["a.py"]`,
"srcs_version": `"PY3"`,
+ "imports": `["."]`,
},
},
},
@@ -189,11 +213,54 @@
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 @@
"//conditions:default": [],
})`,
"srcs_version": `"PY3"`,
+ "imports": `["."]`,
},
},
},
diff --git a/bp2build/testing.go b/bp2build/testing.go
index 029ba49..9341495 100644
--- a/bp2build/testing.go
+++ b/bp2build/testing.go
@@ -119,8 +119,8 @@
return
}
- errs := append(parseErrs, resolveDepsErrs...)
- if tc.expectedErr != nil && checkError(t, errs, tc.expectedErr) {
+ parseAndResolveErrs := append(parseErrs, resolveDepsErrs...)
+ if tc.expectedErr != nil && checkError(t, parseAndResolveErrs, tc.expectedErr) {
return
}
@@ -135,7 +135,7 @@
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)
diff --git a/python/library.go b/python/library.go
index d026c13..3a27591 100644
--- a/python/library.go
+++ b/python/library.go
@@ -17,6 +17,9 @@
// This file contains the module types for building Python library.
import (
+ "path/filepath"
+ "strings"
+
"android/soong/android"
"android/soong/bazel"
@@ -43,6 +46,7 @@
type bazelPythonLibraryAttributes struct {
Srcs bazel.LabelListAttribute
Deps bazel.LabelListAttribute
+ Imports bazel.StringListAttribute
Srcs_version *string
}
@@ -64,16 +68,44 @@
// 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",
- Bzl_load_location: "//build/bazel/rules/python:library.bzl",
+ // Use the native py_library rule.
+ Rule_class: "py_library",
}
ctx.CreateBazelTargetModule(props, android.CommonAttributes{