From 4562a3b218e1c25e1bf76285747f963d26b10917 Mon Sep 17 00:00:00 2001 From: Liz Kammer Date: Wed, 21 Apr 2021 18:15:34 -0400 Subject: [PATCH] Add bp2build arch-specific paths mutator Adds deps for properties tagged `android:"path"` within arch, multilib, and target properties. Test: build/bazel/ci/bp2build.sh Test: m nothing Bug: 185217298 Change-Id: I0230da399d2c4e984b837f69523fa09eadba3ff1 --- android/arch.go | 58 +++++++++++++++++- android/arch_test.go | 6 +- android/bazel.go | 2 +- android/mutator.go | 1 + android/path_properties.go | 3 + bp2build/build_conversion_test.go | 99 +++++++++++++++++++++++++++---- bp2build/bzl_conversion_test.go | 3 + bp2build/testing.go | 14 ++++- 8 files changed, 166 insertions(+), 20 deletions(-) diff --git a/android/arch.go b/android/arch.go index 9f937522f..a63d63070 100644 --- a/android/arch.go +++ b/android/arch.go @@ -412,6 +412,54 @@ func (target Target) Variations() []blueprint.Variation { } } +func registerBp2buildArchPathDepsMutator(ctx RegisterMutatorsContext) { + ctx.BottomUp("bp2build-arch-pathdeps", bp2buildArchPathDepsMutator).Parallel() +} + +// add dependencies for architecture specific properties tagged with `android:"path"` +func bp2buildArchPathDepsMutator(ctx BottomUpMutatorContext) { + var module Module + module = ctx.Module() + + m := module.base() + if !m.ArchSpecific() { + return + } + + // addPathDepsForProps does not descend into sub structs, so we need to descend into the + // arch-specific properties ourselves + properties := []interface{}{} + for _, archProperties := range m.archProperties { + for _, archProps := range archProperties { + archPropValues := reflect.ValueOf(archProps).Elem() + // there are three "arch" variations, descend into each + for _, variant := range []string{"Arch", "Multilib", "Target"} { + // The properties are an interface, get the value (a pointer) that it points to + archProps := archPropValues.FieldByName(variant).Elem() + if archProps.IsNil() { + continue + } + // And then a pointer to a struct + archProps = archProps.Elem() + for i := 0; i < archProps.NumField(); i += 1 { + f := archProps.Field(i) + // If the value of the field is a struct (as opposed to a pointer to a struct) then step + // into the BlueprintEmbed field. + if f.Kind() == reflect.Struct { + f = f.FieldByName("BlueprintEmbed") + } + if f.IsZero() { + continue + } + props := f.Interface().(interface{}) + properties = append(properties, props) + } + } + } + } + addPathDepsForProps(ctx, properties) +} + // osMutator splits an arch-specific module into a variant for each OS that is enabled for the // module. It uses the HostOrDevice value passed to InitAndroidArchModule and the // device_supported and host_supported properties to determine which OsTypes are enabled for this @@ -899,13 +947,17 @@ func filterArchStruct(field reflect.StructField, prefix string) (bool, reflect.S if string(field.Tag) != `android:"`+strings.Join(values, ",")+`"` { panic(fmt.Errorf("unexpected tag format %q", field.Tag)) } + // don't delete path tag as it is needed for bp2build // these tags don't need to be present in the runtime generated struct type. - values = RemoveListFromList(values, []string{"arch_variant", "variant_prepend", "path"}) - if len(values) > 0 { + values = RemoveListFromList(values, []string{"arch_variant", "variant_prepend"}) + if len(values) > 0 && values[0] != "path" { panic(fmt.Errorf("unknown tags %q in field %q", values, prefix+field.Name)) + } else if len(values) == 1 { + field.Tag = reflect.StructTag(`android:"` + strings.Join(values, ",") + `"`) + } else { + field.Tag = `` } - field.Tag = "" return true, field } return false, field diff --git a/android/arch_test.go b/android/arch_test.go index 633ddaa99..3aa4779fe 100644 --- a/android/arch_test.go +++ b/android/arch_test.go @@ -66,9 +66,9 @@ func TestFilterArchStruct(t *testing.T) { }{}, out: &struct { A *string - B *string - C *string - D *string + B *string `android:"path"` + C *string `android:"path"` + D *string `android:"path"` }{}, filtered: true, }, diff --git a/android/bazel.go b/android/bazel.go index 1f7f7e6b3..9806acca5 100644 --- a/android/bazel.go +++ b/android/bazel.go @@ -151,7 +151,7 @@ var ( "libc_bionic_ndk", // ruperts@, cc_library_static, depends on //bionic/libc/system_properties "libc_bionic_systrace", // ruperts@, cc_library_static, 'private/bionic_systrace.h' file not found "libc_pthread", // ruperts@, cc_library_static, 'private/bionic_defs.h' file not found - "libc_syscalls", // ruperts@, cc_library_static, mutator panic cannot get direct dep syscalls-arm64.S of libc_syscalls + "libc_syscalls", // eakammer@, cc_library_static, 'private/bionic_asm.h' file not found "libc_ndk", // ruperts@, cc_library_static, depends on //bionic/libm:libm "libc_nopthread", // ruperts@, cc_library_static, depends on //external/arm-optimized-routines "libc_common", // ruperts@, cc_library_static, depends on //bionic/libc:libc_nopthread diff --git a/android/mutator.go b/android/mutator.go index e25e2e8f1..365bf290b 100644 --- a/android/mutator.go +++ b/android/mutator.go @@ -55,6 +55,7 @@ func RegisterMutatorsForBazelConversion(ctx *Context, preArchMutators, depsMutat bp2buildDepsMutators = append([]RegisterMutatorFunc{ registerDepsMutatorBp2Build, registerPathDepsMutator, + registerBp2buildArchPathDepsMutator, }, depsMutators...) for _, f := range bp2buildDepsMutators { diff --git a/android/path_properties.go b/android/path_properties.go index 2c8d27c56..44467730d 100644 --- a/android/path_properties.go +++ b/android/path_properties.go @@ -34,7 +34,10 @@ func registerPathDepsMutator(ctx RegisterMutatorsContext) { // ":module" module reference syntax in a property that is tagged with `android:"path"`. func pathDepsMutator(ctx BottomUpMutatorContext) { props := ctx.Module().base().generalProperties + addPathDepsForProps(ctx, props) +} +func addPathDepsForProps(ctx BottomUpMutatorContext, props []interface{}) { // Iterate through each property struct of the module extracting the contents of all properties // tagged with `android:"path"`. var pathProperties []string diff --git a/bp2build/build_conversion_test.go b/bp2build/build_conversion_test.go index 1ede4428a..21d7062fa 100644 --- a/bp2build/build_conversion_test.go +++ b/bp2build/build_conversion_test.go @@ -204,8 +204,9 @@ func TestGenerateSoongModuleTargets(t *testing.T) { func TestGenerateBazelTargetModules(t *testing.T) { testCases := []struct { - bp string - expectedBazelTarget string + name string + bp string + expectedBazelTargets []string }{ { bp: `custom { @@ -214,7 +215,7 @@ func TestGenerateBazelTargetModules(t *testing.T) { string_prop: "a", bazel_module: { bp2build_available: true }, }`, - expectedBazelTarget: `custom( + expectedBazelTargets: []string{`custom( name = "foo", string_list_prop = [ "a", @@ -222,6 +223,7 @@ func TestGenerateBazelTargetModules(t *testing.T) { ], string_prop = "a", )`, + }, }, { bp: `custom { @@ -230,7 +232,7 @@ func TestGenerateBazelTargetModules(t *testing.T) { string_prop: "a\t\n\r", bazel_module: { bp2build_available: true }, }`, - expectedBazelTarget: `custom( + expectedBazelTargets: []string{`custom( name = "control_characters", string_list_prop = [ "\t", @@ -238,6 +240,77 @@ func TestGenerateBazelTargetModules(t *testing.T) { ], string_prop = "a\t\n\r", )`, + }, + }, + { + bp: `custom { + name: "has_dep", + arch_paths: [":dep"], + bazel_module: { bp2build_available: true }, +} + +custom { + name: "dep", + arch_paths: ["abc"], + bazel_module: { bp2build_available: true }, +}`, + expectedBazelTargets: []string{`custom( + name = "dep", + arch_paths = ["abc"], +)`, + `custom( + name = "has_dep", + arch_paths = [":dep"], +)`, + }, + }, + { + bp: `custom { + name: "arch_paths", + arch: { + x86: { + arch_paths: ["abc"], + }, + }, + bazel_module: { bp2build_available: true }, +}`, + expectedBazelTargets: []string{`custom( + name = "arch_paths", + arch_paths = select({ + "//build/bazel/platforms/arch:x86": ["abc"], + "//conditions:default": [], + }), +)`, + }, + }, + { + bp: `custom { + name: "has_dep", + arch: { + x86: { + arch_paths: [":dep"], + }, + }, + bazel_module: { bp2build_available: true }, +} + +custom { + name: "dep", + arch_paths: ["abc"], + bazel_module: { bp2build_available: true }, +}`, + expectedBazelTargets: []string{`custom( + name = "dep", + arch_paths = ["abc"], +)`, + `custom( + name = "has_dep", + arch_paths = select({ + "//build/bazel/platforms/arch:x86": [":dep"], + "//conditions:default": [], + }), +)`, + }, }, } @@ -262,16 +335,18 @@ func TestGenerateBazelTargetModules(t *testing.T) { codegenCtx := NewCodegenContext(config, *ctx.Context, Bp2Build) bazelTargets := generateBazelTargetsForDir(codegenCtx, dir) - if actualCount, expectedCount := len(bazelTargets), 1; actualCount != expectedCount { + if actualCount, expectedCount := len(bazelTargets), len(testCase.expectedBazelTargets); actualCount != expectedCount { t.Errorf("Expected %d bazel target, got %d", expectedCount, actualCount) } else { - actualBazelTarget := bazelTargets[0] - if actualBazelTarget.content != testCase.expectedBazelTarget { - t.Errorf( - "Expected generated Bazel target to be '%s', got '%s'", - testCase.expectedBazelTarget, - actualBazelTarget.content, - ) + for i, expectedBazelTarget := range testCase.expectedBazelTargets { + actualBazelTarget := bazelTargets[i] + if actualBazelTarget.content != expectedBazelTarget { + t.Errorf( + "Expected generated Bazel target to be '%s', got '%s'", + expectedBazelTarget, + actualBazelTarget.content, + ) + } } } } diff --git a/bp2build/bzl_conversion_test.go b/bp2build/bzl_conversion_test.go index 30c1a5b6a..32b12e42e 100644 --- a/bp2build/bzl_conversion_test.go +++ b/bp2build/bzl_conversion_test.go @@ -86,6 +86,7 @@ custom = rule( "soong_module_name": attr.string(mandatory = True), "soong_module_variant": attr.string(), "soong_module_deps": attr.label_list(providers = [SoongModuleInfo]), + "arch_paths": attr.string_list(), # bazel_module start # "label": attr.string(), # "bp2build_available": attr.bool(), @@ -114,6 +115,7 @@ custom_defaults = rule( "soong_module_name": attr.string(mandatory = True), "soong_module_variant": attr.string(), "soong_module_deps": attr.label_list(providers = [SoongModuleInfo]), + "arch_paths": attr.string_list(), "bool_prop": attr.bool(), "bool_ptr_prop": attr.bool(), "int64_ptr_prop": attr.int(), @@ -138,6 +140,7 @@ custom_test_ = rule( "soong_module_name": attr.string(mandatory = True), "soong_module_variant": attr.string(), "soong_module_deps": attr.label_list(providers = [SoongModuleInfo]), + "arch_paths": attr.string_list(), "bool_prop": attr.bool(), "bool_ptr_prop": attr.bool(), "int64_ptr_prop": attr.int(), diff --git a/bp2build/testing.go b/bp2build/testing.go index ef3a78fd3..d65aa0b8f 100644 --- a/bp2build/testing.go +++ b/bp2build/testing.go @@ -28,6 +28,8 @@ type customProps struct { Nested_props nestedProps Nested_props_ptr *nestedProps + + Arch_paths []string `android:"path,arch_variant"` } type customModule struct { @@ -56,7 +58,7 @@ func customModuleFactoryBase() android.Module { func customModuleFactory() android.Module { m := customModuleFactoryBase() - android.InitAndroidModule(m) + android.InitAndroidArchModule(m, android.HostAndDeviceSupported, android.MultilibBoth) return m } @@ -114,6 +116,7 @@ func customDefaultsModuleFactory() android.Module { type customBazelModuleAttributes struct { String_prop string String_list_prop []string + Arch_paths bazel.LabelListAttribute } type customBazelModule struct { @@ -137,9 +140,18 @@ func customBp2BuildMutator(ctx android.TopDownMutatorContext) { return } + paths := bazel.MakeLabelListAttribute(android.BazelLabelForModuleSrc(ctx, m.props.Arch_paths)) + + for arch, props := range m.GetArchProperties(&customProps{}) { + if archProps, ok := props.(*customProps); ok && archProps.Arch_paths != nil { + paths.SetValueForArch(arch.Name, android.BazelLabelForModuleSrc(ctx, archProps.Arch_paths)) + } + } + attrs := &customBazelModuleAttributes{ String_prop: m.props.String_prop, String_list_prop: m.props.String_list_prop, + Arch_paths: paths, } props := bazel.BazelTargetModuleProperties{