From 01243368d7a5ef6100582c2efade729888d646f8 Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Thu, 2 Jun 2022 12:11:12 -0700 Subject: [PATCH] Allowlist apexer for bp2build This also introduces a workaround for the fact that apexer depends on aapt2, but aapt2 doesn't build with bp2build yet. Aapt2 is removed from apexer's requirements during bp2build. Bug: 204244290 Test: ./build/bazel/ci/bp2build.sh Change-Id: I837597ce035c7d5c06e1a3957166583a7a94b5c7 --- android/allowlists/allowlists.go | 9 ++++--- android/module.go | 14 ++++++++-- bp2build/python_binary_conversion_test.go | 12 ++++++--- genrule/genrule.go | 2 +- python/binary.go | 2 ++ python/library.go | 32 +--------------------- python/python.go | 33 +++++++++++++++++++++-- 7 files changed, 60 insertions(+), 44 deletions(-) diff --git a/android/allowlists/allowlists.go b/android/allowlists/allowlists.go index 672d7f658..a407b5e13 100644 --- a/android/allowlists/allowlists.go +++ b/android/allowlists/allowlists.go @@ -329,6 +329,7 @@ var ( //system/extras/ext4_utils "libext4_utils", + "mke2fs_conf", //system/extras/libfec "libfec", @@ -359,10 +360,10 @@ var ( "gen-kotlin-build-file.py", // TODO(b/198619163) module has same name as source "libgtest_ndk_c++", "libgtest_main_ndk_c++", // TODO(b/201816222): Requires sdk_version support. "linkerconfig", "mdnsd", // TODO(b/202876379): has arch-variant static_executable - "linker", // TODO(b/228316882): cc_binary uses link_crt - "libdebuggerd", // TODO(b/228314770): support product variable-specific header_libs - "versioner", // TODO(b/228313961): depends on prebuilt shared library libclang-cpp_host as a shared library, which does not supply expected providers for a shared library - "apexer", "apexer_test", // Requires aapt2 + "linker", // TODO(b/228316882): cc_binary uses link_crt + "libdebuggerd", // TODO(b/228314770): support product variable-specific header_libs + "versioner", // TODO(b/228313961): depends on prebuilt shared library libclang-cpp_host as a shared library, which does not supply expected providers for a shared library + "apexer_test", // Requires aapt2 "apexer_test_host_tools", "host_apex_verifier", diff --git a/android/module.go b/android/module.go index ad01e9ef7..292508199 100644 --- a/android/module.go +++ b/android/module.go @@ -1177,7 +1177,6 @@ func (attrs *CommonAttributes) fillCommonBp2BuildModuleAttrs(ctx *topDownMutator data := &attrs.Data - required := depsToLabelList(props.Required) archVariantProps := mod.GetArchVariantProperties(ctx, &commonProperties{}) var enabledProperty bazel.BoolAttribute @@ -1231,10 +1230,21 @@ func (attrs *CommonAttributes) fillCommonBp2BuildModuleAttrs(ctx *topDownMutator } } + required := depsToLabelList(props.Required) for axis, configToProps := range archVariantProps { for config, _props := range configToProps { if archProps, ok := _props.(*commonProperties); ok { - required.SetSelectValue(axis, config, depsToLabelList(archProps.Required).Value) + // TODO(b/234748998) Remove this requiredFiltered workaround when aapt2 converts successfully + requiredFiltered := archProps.Required + if name == "apexer" { + requiredFiltered = make([]string, 0, len(archProps.Required)) + for _, req := range archProps.Required { + if req != "aapt2" && req != "apexer" { + requiredFiltered = append(requiredFiltered, req) + } + } + } + required.SetSelectValue(axis, config, depsToLabelList(requiredFiltered).Value) if !neitherHostNorDevice { if archProps.Enabled != nil { if axis != bazel.OsConfigurationAxis || osSupport[config] { diff --git a/bp2build/python_binary_conversion_test.go b/bp2build/python_binary_conversion_test.go index dfa11d1be..22bd028e7 100644 --- a/bp2build/python_binary_conversion_test.go +++ b/bp2build/python_binary_conversion_test.go @@ -43,9 +43,10 @@ func TestPythonBinaryHostSimple(t *testing.T) { }`, expectedBazelTargets: []string{ makeBazelTarget("py_binary", "foo", attrNameToString{ - "data": `["files/data.txt"]`, - "deps": `[":bar"]`, - "main": `"a.py"`, + "data": `["files/data.txt"]`, + "deps": `[":bar"]`, + "main": `"a.py"`, + "imports": `["."]`, "srcs": `[ "a.py", "b/c.py", @@ -83,6 +84,7 @@ func TestPythonBinaryHostPy2(t *testing.T) { expectedBazelTargets: []string{ makeBazelTarget("py_binary", "foo", attrNameToString{ "python_version": `"PY2"`, + "imports": `["."]`, "srcs": `["a.py"]`, "target_compatible_with": `select({ "//build/bazel/platforms/os:android": ["@platforms//:incompatible"], @@ -116,7 +118,8 @@ func TestPythonBinaryHostPy3(t *testing.T) { expectedBazelTargets: []string{ // python_version is PY3 by default. makeBazelTarget("py_binary", "foo", attrNameToString{ - "srcs": `["a.py"]`, + "imports": `["."]`, + "srcs": `["a.py"]`, "target_compatible_with": `select({ "//build/bazel/platforms/os:android": ["@platforms//:incompatible"], "//conditions:default": [], @@ -148,6 +151,7 @@ func TestPythonBinaryHostArchVariance(t *testing.T) { }`, expectedBazelTargets: []string{ makeBazelTarget("py_binary", "foo-arm", attrNameToString{ + "imports": `["."]`, "srcs": `select({ "//build/bazel/platforms/arch:arm": ["arm.py"], "//build/bazel/platforms/arch:x86": ["x86.py"], diff --git a/genrule/genrule.go b/genrule/genrule.go index 818e1bcf1..2a8056344 100644 --- a/genrule/genrule.go +++ b/genrule/genrule.go @@ -266,7 +266,7 @@ func (g *Module) ProcessBazelQueryResponse(ctx android.ModuleContext) { var bazelOutputFiles android.Paths exportIncludeDirs := map[string]bool{} for _, bazelOutputFile := range filePaths { - bazelOutputFiles = append(bazelOutputFiles, android.PathForBazelOut(ctx, bazelOutputFile)) + bazelOutputFiles = append(bazelOutputFiles, android.PathForBazelOutRelative(ctx, ctx.ModuleDir(), bazelOutputFile)) exportIncludeDirs[filepath.Dir(bazelOutputFile)] = true } g.outputFiles = bazelOutputFiles diff --git a/python/binary.go b/python/binary.go index 99c625916..af29bb6b8 100644 --- a/python/binary.go +++ b/python/binary.go @@ -38,6 +38,7 @@ type bazelPythonBinaryAttributes struct { Srcs bazel.LabelListAttribute Deps bazel.LabelListAttribute Python_version *string + Imports bazel.StringListAttribute } func pythonBinaryBp2Build(ctx android.TopDownMutatorContext, m *Module) { @@ -75,6 +76,7 @@ func pythonBinaryBp2Build(ctx android.TopDownMutatorContext, m *Module) { Srcs: baseAttrs.Srcs, Deps: baseAttrs.Deps, Python_version: python_version, + Imports: baseAttrs.Imports, } props := bazel.BazelTargetModuleProperties{ diff --git a/python/library.go b/python/library.go index 5071b74cf..df92df42b 100644 --- a/python/library.go +++ b/python/library.go @@ -17,9 +17,6 @@ package python // This file contains the module types for building Python library. import ( - "path/filepath" - "strings" - "android/soong/android" "android/soong/bazel" @@ -72,40 +69,13 @@ 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}), + Imports: baseAttrs.Imports, } props := bazel.BazelTargetModuleProperties{ diff --git a/python/python.go b/python/python.go index 7e4cb837c..eb0d3cad0 100644 --- a/python/python.go +++ b/python/python.go @@ -131,7 +131,8 @@ type baseAttributes struct { Srcs bazel.LabelListAttribute Deps bazel.LabelListAttribute // Combines Data and Java_data (invariant) - Data bazel.LabelListAttribute + Data bazel.LabelListAttribute + Imports bazel.StringListAttribute } // Used to store files of current module after expanding dependencies @@ -230,6 +231,33 @@ func (m *Module) makeArchVariantBaseAttributes(ctx android.TopDownMutatorContext attrs.Deps.Add(bazel.MakeLabelAttribute(":" + pyProtoLibraryName)) } + + // 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) + } + + 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()) + } + numFolders := strings.Count(pkg_path, "/") + 1 + dots := make([]string, numFolders) + for i := 0; i < numFolders; i++ { + dots[i] = ".." + } + imports = strings.Join(dots, "/") + } + attrs.Imports = bazel.MakeStringListAttribute([]string{imports}) + return attrs } @@ -654,7 +682,8 @@ func (p *Module) createSrcsZip(ctx android.ModuleContext, pkgPath string) androi // in order to keep stable order of soong_zip params, we sort the keys here. roots := android.SortedStringKeys(relativeRootMap) - parArgs := []string{} + // Use -symlinks=false so that the symlinks in the bazel output directory are followed + parArgs := []string{"-symlinks=false"} if pkgPath != "" { // use package path as path prefix parArgs = append(parArgs, `-P `+pkgPath)