Fix some issues in bp2build converter for python_binary_host.

1) Bp2build convert python_binary_host main attribute as LabelAttribute. Currently "main" attribute in python_binary_host is handled as string but for some modules (e.g certify_bootimg) the "main" attribute points to a file in its subpackage like "subpackage/file.py" and should be converted to "//.../subpackage:file.py".

2) Filter out duplicated labels in the merged list of "required" attributes of python_binary_host and its defaults.

Test: b build //system/tools/mkbootimg:certify_bootimg
Test: b build //build/make/tools/releasetools:check_target_files_signatures
Bug: 253081249
Bug: 253101186

Change-Id: Ic2cb4cadec2c1348da70af9f0730da9914d3a8ca
This commit is contained in:
Wei Li 2022-10-11 14:38:16 -07:00
parent 922875d187
commit 7d8f6182f9
5 changed files with 201 additions and 16 deletions

View file

@ -47,11 +47,13 @@ var (
"bionic": Bp2BuildDefaultTrueRecursively, "bionic": Bp2BuildDefaultTrueRecursively,
"bootable/recovery/minadbd": Bp2BuildDefaultTrue, "bootable/recovery/minadbd": Bp2BuildDefaultTrue,
"bootable/recovery/minui": Bp2BuildDefaultTrue, "bootable/recovery/minui": Bp2BuildDefaultTrue,
"bootable/recovery/applypatch": Bp2BuildDefaultTrue,
"bootable/recovery/recovery_utils": Bp2BuildDefaultTrue, "bootable/recovery/recovery_utils": Bp2BuildDefaultTrue,
"bootable/recovery/tools/recovery_l10n": Bp2BuildDefaultTrue, "bootable/recovery/tools/recovery_l10n": Bp2BuildDefaultTrue,
"build/bazel": Bp2BuildDefaultTrueRecursively, "build/bazel": Bp2BuildDefaultTrueRecursively,
"build/make/target/product/security": Bp2BuildDefaultTrue, "build/make/target/product/security": Bp2BuildDefaultTrue,
"build/make/tools/releasetools": Bp2BuildDefaultTrue,
"build/make/tools/signapk": Bp2BuildDefaultTrue, "build/make/tools/signapk": Bp2BuildDefaultTrue,
"build/make/tools/zipalign": Bp2BuildDefaultTrueRecursively, "build/make/tools/zipalign": Bp2BuildDefaultTrueRecursively,
"build/soong": Bp2BuildDefaultTrue, "build/soong": Bp2BuildDefaultTrue,
@ -109,6 +111,8 @@ var (
"external/boringssl": Bp2BuildDefaultTrueRecursively, "external/boringssl": Bp2BuildDefaultTrueRecursively,
"external/bouncycastle": Bp2BuildDefaultTrue, "external/bouncycastle": Bp2BuildDefaultTrue,
"external/brotli": Bp2BuildDefaultTrue, "external/brotli": Bp2BuildDefaultTrue,
"external/bsdiff": Bp2BuildDefaultTrueRecursively,
"external/bzip2": Bp2BuildDefaultTrueRecursively,
"external/conscrypt": Bp2BuildDefaultTrue, "external/conscrypt": Bp2BuildDefaultTrue,
"external/e2fsprogs": Bp2BuildDefaultTrueRecursively, "external/e2fsprogs": Bp2BuildDefaultTrueRecursively,
"external/eigen": Bp2BuildDefaultTrueRecursively, "external/eigen": Bp2BuildDefaultTrueRecursively,
@ -138,6 +142,7 @@ var (
"external/libcap": Bp2BuildDefaultTrueRecursively, "external/libcap": Bp2BuildDefaultTrueRecursively,
"external/libcxx": Bp2BuildDefaultTrueRecursively, "external/libcxx": Bp2BuildDefaultTrueRecursively,
"external/libcxxabi": Bp2BuildDefaultTrueRecursively, "external/libcxxabi": Bp2BuildDefaultTrueRecursively,
"external/libdivsufsort": Bp2BuildDefaultTrueRecursively,
"external/libdrm": Bp2BuildDefaultTrue, "external/libdrm": Bp2BuildDefaultTrue,
"external/libevent": Bp2BuildDefaultTrueRecursively, "external/libevent": Bp2BuildDefaultTrueRecursively,
"external/libgav1": Bp2BuildDefaultTrueRecursively, "external/libgav1": Bp2BuildDefaultTrueRecursively,
@ -148,6 +153,7 @@ var (
"external/libvpx": Bp2BuildDefaultTrueRecursively, "external/libvpx": Bp2BuildDefaultTrueRecursively,
"external/libyuv": Bp2BuildDefaultTrueRecursively, "external/libyuv": Bp2BuildDefaultTrueRecursively,
"external/lz4/lib": Bp2BuildDefaultTrue, "external/lz4/lib": Bp2BuildDefaultTrue,
"external/lz4/programs": Bp2BuildDefaultTrue,
"external/lzma/C": Bp2BuildDefaultTrueRecursively, "external/lzma/C": Bp2BuildDefaultTrueRecursively,
"external/mdnsresponder": Bp2BuildDefaultTrueRecursively, "external/mdnsresponder": Bp2BuildDefaultTrueRecursively,
"external/minijail": Bp2BuildDefaultTrueRecursively, "external/minijail": Bp2BuildDefaultTrueRecursively,
@ -282,6 +288,7 @@ var (
"system/core/libsysutils": Bp2BuildDefaultTrueRecursively, "system/core/libsysutils": Bp2BuildDefaultTrueRecursively,
"system/core/libutils": Bp2BuildDefaultTrueRecursively, "system/core/libutils": Bp2BuildDefaultTrueRecursively,
"system/core/libvndksupport": Bp2BuildDefaultTrueRecursively, "system/core/libvndksupport": Bp2BuildDefaultTrueRecursively,
"system/core/mkbootfs": Bp2BuildDefaultTrueRecursively,
"system/core/property_service/libpropertyinfoparser": Bp2BuildDefaultTrueRecursively, "system/core/property_service/libpropertyinfoparser": Bp2BuildDefaultTrueRecursively,
"system/core/property_service/libpropertyinfoserializer": Bp2BuildDefaultTrueRecursively, "system/core/property_service/libpropertyinfoserializer": Bp2BuildDefaultTrueRecursively,
"system/extras/toolchain-extras": Bp2BuildDefaultTrue, "system/extras/toolchain-extras": Bp2BuildDefaultTrue,
@ -315,6 +322,7 @@ var (
"system/timezone/apex": Bp2BuildDefaultTrueRecursively, "system/timezone/apex": Bp2BuildDefaultTrueRecursively,
"system/timezone/output_data": Bp2BuildDefaultTrueRecursively, "system/timezone/output_data": Bp2BuildDefaultTrueRecursively,
"system/tools/aidl/build/tests_bp2build": Bp2BuildDefaultTrue, "system/tools/aidl/build/tests_bp2build": Bp2BuildDefaultTrue,
"system/tools/mkbootimg": Bp2BuildDefaultTrueRecursively,
"system/tools/sysprop": Bp2BuildDefaultTrue, "system/tools/sysprop": Bp2BuildDefaultTrue,
"system/unwinding/libunwindstack": Bp2BuildDefaultTrueRecursively, "system/unwinding/libunwindstack": Bp2BuildDefaultTrueRecursively,
@ -474,7 +482,6 @@ var (
"prebuilt_stats-log-api-gen", "prebuilt_stats-log-api-gen",
// fastboot // fastboot
"bootimg_headers",
"fastboot", "fastboot",
"libfastboot", "libfastboot",
"liblp", "liblp",
@ -507,6 +514,7 @@ var (
//system/extras/verity/fec //system/extras/verity/fec
"fec", "fec",
"boot_signer",
//packages/apps/Car/libs/car-ui-lib/car-ui-androidx //packages/apps/Car/libs/car-ui-lib/car-ui-androidx
// genrule dependencies for java_imports // genrule dependencies for java_imports
@ -1245,6 +1253,29 @@ var (
"launcherprotosnano", "launcherprotosnano",
"datastallprotosnano", "datastallprotosnano",
"devicepolicyprotosnano", "devicepolicyprotosnano",
"ota_metadata_proto_java",
"merge_ota",
// releasetools
"releasetools_fsverity_metadata_generator",
"verity_utils",
"check_ota_package_signature",
"check_target_files_vintf",
"releasetools_check_target_files_vintf",
"releasetools_verity_utils",
"build_image",
"ota_from_target_files",
"releasetools_ota_from_target_files",
"releasetools_build_image",
"add_img_to_target_files",
"releasetools_add_img_to_target_files",
"fsverity_metadata_generator",
"sign_target_files_apks",
// depends on the support of yacc file
"libapplypatch",
"libapplypatch_modes",
"applypatch",
} }
Bp2buildCcLibraryStaticOnlyList = []string{} Bp2buildCcLibraryStaticOnlyList = []string{}

View file

@ -1246,12 +1246,14 @@ func (attrs *CommonAttributes) fillCommonBp2BuildModuleAttrs(ctx *topDownMutator
// when generated as the 'data' label list attribute in Bazel. Remove it if // when generated as the 'data' label list attribute in Bazel. Remove it if
// it exists. See b/247985196. // it exists. See b/247985196.
_, requiredWithoutCycles := RemoveFromList(ctx.ModuleName(), mod.commonProperties.Required) _, requiredWithoutCycles := RemoveFromList(ctx.ModuleName(), mod.commonProperties.Required)
requiredWithoutCycles = FirstUniqueStrings(requiredWithoutCycles)
required := depsToLabelList(requiredWithoutCycles) required := depsToLabelList(requiredWithoutCycles)
archVariantProps := mod.GetArchVariantProperties(ctx, &commonProperties{}) archVariantProps := mod.GetArchVariantProperties(ctx, &commonProperties{})
for axis, configToProps := range archVariantProps { for axis, configToProps := range archVariantProps {
for config, _props := range configToProps { for config, _props := range configToProps {
if archProps, ok := _props.(*commonProperties); ok { if archProps, ok := _props.(*commonProperties); ok {
_, requiredWithoutCycles := RemoveFromList(ctx.ModuleName(), archProps.Required) _, requiredWithoutCycles := RemoveFromList(ctx.ModuleName(), archProps.Required)
requiredWithoutCycles = FirstUniqueStrings(requiredWithoutCycles)
required.SetSelectValue(axis, config, depsToLabelList(requiredWithoutCycles).Value) required.SetSelectValue(axis, config, depsToLabelList(requiredWithoutCycles).Value)
if !neitherHostNorDevice { if !neitherHostNorDevice {
if archProps.Enabled != nil { if archProps.Enabled != nil {

View file

@ -4,6 +4,7 @@ import (
"testing" "testing"
"android/soong/android" "android/soong/android"
"android/soong/genrule"
"android/soong/python" "android/soong/python"
) )
@ -12,6 +13,8 @@ func runBp2BuildTestCaseWithPythonLibraries(t *testing.T, tc Bp2buildTestCase) {
RunBp2BuildTestCase(t, func(ctx android.RegistrationContext) { RunBp2BuildTestCase(t, func(ctx android.RegistrationContext) {
ctx.RegisterModuleType("python_library", python.PythonLibraryFactory) ctx.RegisterModuleType("python_library", python.PythonLibraryFactory)
ctx.RegisterModuleType("python_library_host", python.PythonLibraryHostFactory) ctx.RegisterModuleType("python_library_host", python.PythonLibraryHostFactory)
ctx.RegisterModuleType("genrule", genrule.GenRuleFactory)
ctx.RegisterModuleType("python_defaults", python.DefaultsFactory)
}, tc) }, tc)
} }
@ -165,3 +168,152 @@ func TestPythonBinaryHostArchVariance(t *testing.T) {
}, },
}) })
} }
func TestPythonBinaryMainIsNotSpecified(t *testing.T) {
runBp2BuildTestCaseWithPythonLibraries(t, Bp2buildTestCase{
Description: "python_binary_host main label in same package",
ModuleTypeUnderTest: "python_binary_host",
ModuleTypeUnderTestFactory: python.PythonBinaryHostFactory,
Blueprint: `python_binary_host {
name: "foo",
bazel_module: { bp2build_available: true },
}
`,
ExpectedBazelTargets: []string{
MakeBazelTarget("py_binary", "foo", AttrNameToString{
"imports": `["."]`,
"target_compatible_with": `select({
"//build/bazel/platforms/os:android": ["@platforms//:incompatible"],
"//conditions:default": [],
})`,
}),
},
})
}
func TestPythonBinaryMainIsLabel(t *testing.T) {
runBp2BuildTestCaseWithPythonLibraries(t, Bp2buildTestCase{
Description: "python_binary_host main label in same package",
ModuleTypeUnderTest: "python_binary_host",
ModuleTypeUnderTestFactory: python.PythonBinaryHostFactory,
Blueprint: `python_binary_host {
name: "foo",
main: ":a",
bazel_module: { bp2build_available: true },
}
genrule {
name: "a",
bazel_module: { bp2build_available: false },
}
`,
ExpectedBazelTargets: []string{
MakeBazelTarget("py_binary", "foo", AttrNameToString{
"main": `":a"`,
"imports": `["."]`,
"target_compatible_with": `select({
"//build/bazel/platforms/os:android": ["@platforms//:incompatible"],
"//conditions:default": [],
})`,
}),
},
})
}
func TestPythonBinaryMainIsSubpackageFile(t *testing.T) {
runBp2BuildTestCaseWithPythonLibraries(t, Bp2buildTestCase{
Description: "python_binary_host main is subpackage file",
ModuleTypeUnderTest: "python_binary_host",
ModuleTypeUnderTestFactory: python.PythonBinaryHostFactory,
Filesystem: map[string]string{
"a/Android.bp": "",
"a/b.py": "",
},
Blueprint: `python_binary_host {
name: "foo",
main: "a/b.py",
bazel_module: { bp2build_available: true },
}
`,
ExpectedBazelTargets: []string{
MakeBazelTarget("py_binary", "foo", AttrNameToString{
"main": `"//a:b.py"`,
"imports": `["."]`,
"target_compatible_with": `select({
"//build/bazel/platforms/os:android": ["@platforms//:incompatible"],
"//conditions:default": [],
})`,
}),
},
})
}
func TestPythonBinaryMainIsSubDirFile(t *testing.T) {
runBp2BuildTestCaseWithPythonLibraries(t, Bp2buildTestCase{
Description: "python_binary_host main is file in sub directory that is not Bazel package",
ModuleTypeUnderTest: "python_binary_host",
ModuleTypeUnderTestFactory: python.PythonBinaryHostFactory,
Filesystem: map[string]string{
"a/b.py": "",
},
Blueprint: `python_binary_host {
name: "foo",
main: "a/b.py",
bazel_module: { bp2build_available: true },
}
`,
ExpectedBazelTargets: []string{
MakeBazelTarget("py_binary", "foo", AttrNameToString{
"main": `"a/b.py"`,
"imports": `["."]`,
"target_compatible_with": `select({
"//build/bazel/platforms/os:android": ["@platforms//:incompatible"],
"//conditions:default": [],
})`,
}),
},
})
}
func TestPythonBinaryDuplicatesInRequired(t *testing.T) {
runBp2BuildTestCaseWithPythonLibraries(t, Bp2buildTestCase{
Description: "python_binary_host duplicates in required attribute of the module and its defaults",
ModuleTypeUnderTest: "python_binary_host",
ModuleTypeUnderTestFactory: python.PythonBinaryHostFactory,
Blueprint: `python_binary_host {
name: "foo",
main: "a.py",
defaults: ["d"],
required: [
"r1",
],
bazel_module: { bp2build_available: true },
}
python_defaults {
name: "d",
required: [
"r1",
"r2",
],
}` + simpleModuleDoNotConvertBp2build("genrule", "r1") +
simpleModuleDoNotConvertBp2build("genrule", "r2"),
ExpectedBazelTargets: []string{
MakeBazelTarget("py_binary", "foo", AttrNameToString{
"main": `"a.py"`,
"imports": `["."]`,
"data": `[
":r1",
":r2",
]`,
"target_compatible_with": `select({
"//build/bazel/platforms/os:android": ["@platforms//:incompatible"],
"//conditions:default": [],
})`,
}),
},
})
}

View file

@ -34,7 +34,7 @@ func registerPythonBinaryComponents(ctx android.RegistrationContext) {
} }
type bazelPythonBinaryAttributes struct { type bazelPythonBinaryAttributes struct {
Main *string Main *bazel.Label
Srcs bazel.LabelListAttribute Srcs bazel.LabelListAttribute
Deps bazel.LabelListAttribute Deps bazel.LabelListAttribute
Python_version *string Python_version *string
@ -42,17 +42,6 @@ type bazelPythonBinaryAttributes struct {
} }
func pythonBinaryBp2Build(ctx android.TopDownMutatorContext, m *Module) { func pythonBinaryBp2Build(ctx android.TopDownMutatorContext, m *Module) {
var main *string
for _, propIntf := range m.GetProperties() {
if props, ok := propIntf.(*BinaryProperties); ok {
// main is optional.
if props.Main != nil {
main = props.Main
break
}
}
}
// TODO(b/182306917): this doesn't fully handle all nested props versioned // TODO(b/182306917): this doesn't fully handle all nested props versioned
// by the python version, which would have been handled by the version split // by the python version, which would have been handled by the version split
// mutator. This is sufficient for very simple python_binary_host modules // mutator. This is sufficient for very simple python_binary_host modules
@ -72,13 +61,24 @@ func pythonBinaryBp2Build(ctx android.TopDownMutatorContext, m *Module) {
baseAttrs := m.makeArchVariantBaseAttributes(ctx) baseAttrs := m.makeArchVariantBaseAttributes(ctx)
attrs := &bazelPythonBinaryAttributes{ attrs := &bazelPythonBinaryAttributes{
Main: main, Main: nil,
Srcs: baseAttrs.Srcs, Srcs: baseAttrs.Srcs,
Deps: baseAttrs.Deps, Deps: baseAttrs.Deps,
Python_version: python_version, Python_version: python_version,
Imports: baseAttrs.Imports, Imports: baseAttrs.Imports,
} }
for _, propIntf := range m.GetProperties() {
if props, ok := propIntf.(*BinaryProperties); ok {
// main is optional.
if props.Main != nil {
main := android.BazelLabelForModuleSrcSingle(ctx, *props.Main)
attrs.Main = &main
break
}
}
}
props := bazel.BazelTargetModuleProperties{ props := bazel.BazelTargetModuleProperties{
// Use the native py_binary rule. // Use the native py_binary rule.
Rule_class: "py_binary", Rule_class: "py_binary",

View file

@ -19,7 +19,7 @@ import (
) )
func init() { func init() {
android.RegisterModuleType("python_defaults", defaultsFactory) android.RegisterModuleType("python_defaults", DefaultsFactory)
} }
type Defaults struct { type Defaults struct {
@ -30,7 +30,7 @@ type Defaults struct {
func (d *Defaults) GenerateAndroidBuildActions(ctx android.ModuleContext) { func (d *Defaults) GenerateAndroidBuildActions(ctx android.ModuleContext) {
} }
func defaultsFactory() android.Module { func DefaultsFactory() android.Module {
module := &Defaults{} module := &Defaults{}
module.AddProperties( module.AddProperties(