From 75539d62ae709549aa24dcf93bd648388454815a Mon Sep 17 00:00:00 2001 From: Sam Delmerico Date: Mon, 31 Jan 2022 14:37:29 +0000 Subject: [PATCH] add target_compatible_with stanza for host targets Soong modules that are specific for the host platform (e.g. java_library_host, cc_binary_host, java_genrule_host, etc.) should not be built on the target platform (Android), so we add a target_compatible_with attribute to skip this type of module on an Android target build. Bug: 215229742 Test: go test ./bp2build Change-Id: Ifb76ef4e0dc4cb3adb6a64b5c375ce36f7973e48 --- android/module.go | 5 + bp2build/cc_binary_conversion_test.go | 8 +- bp2build/genrule_conversion_test.go | 152 +++++++++++++----- bp2build/java_binary_host_conversion_test.go | 4 + bp2build/java_library_host_conversion_test.go | 8 + bp2build/python_binary_conversion_test.go | 16 ++ bp2build/python_library_conversion_test.go | 120 ++++++++++---- cc/binary.go | 11 +- java/plugin.go | 7 +- 9 files changed, 236 insertions(+), 95 deletions(-) diff --git a/android/module.go b/android/module.go index 00aed952f..d0807c3f6 100644 --- a/android/module.go +++ b/android/module.go @@ -1165,6 +1165,11 @@ func (attrs *CommonAttributes) fillCommonBp2BuildModuleAttrs(ctx *topDownMutator productConfigEnabledLabels, nil, }) + moduleSupportsDevice := mod.commonProperties.HostOrDeviceSupported&deviceSupported == deviceSupported + if mod.commonProperties.HostOrDeviceSupported != NeitherHostNorDeviceSupported && !moduleSupportsDevice { + enabledProperty.SetSelectValue(bazel.OsConfigurationAxis, Android.Name, proptools.BoolPtr(false)) + } + platformEnabledAttribute, err := enabledProperty.ToLabelListAttribute( bazel.LabelList{[]bazel.Label{bazel.Label{Label: "@platforms//:incompatible"}}, nil}, bazel.LabelList{[]bazel.Label{}, nil}) diff --git a/bp2build/cc_binary_conversion_test.go b/bp2build/cc_binary_conversion_test.go index a156480d2..f9adc78f8 100644 --- a/bp2build/cc_binary_conversion_test.go +++ b/bp2build/cc_binary_conversion_test.go @@ -84,13 +84,13 @@ func runCcHostBinaryTestCase(t *testing.T, tc ccBinaryBp2buildTestCase) { t.Helper() testCase := tc for i, tar := range testCase.targets { - if tar.typ != "cc_binary" { - continue - } - tar.attrs["target_compatible_with"] = `select({ + switch tar.typ { + case "cc_binary", "proto_library", "cc_lite_proto_library": + tar.attrs["target_compatible_with"] = `select({ "//build/bazel/platforms/os:android": ["@platforms//:incompatible"], "//conditions:default": [], })` + } testCase.targets[i] = tar } moduleTypeUnderTest := "cc_binary_host" diff --git a/bp2build/genrule_conversion_test.go b/bp2build/genrule_conversion_test.go index 0666da778..9244b9922 100644 --- a/bp2build/genrule_conversion_test.go +++ b/bp2build/genrule_conversion_test.go @@ -97,13 +97,22 @@ func TestGenruleCliVariableReplacement(t *testing.T) { }` for _, tc := range testCases { + moduleAttrs := attrNameToString{ + "cmd": fmt.Sprintf(`"$(location :foo.tool) --genDir=%s arg $(SRCS) $(OUTS)"`, tc.genDir), + "outs": `["foo.out"]`, + "srcs": `["foo.in"]`, + "tools": `[":foo.tool"]`, + } + + if tc.moduleType == "java_genrule_host" { + moduleAttrs["target_compatible_with"] = `select({ + "//build/bazel/platforms/os:android": ["@platforms//:incompatible"], + "//conditions:default": [], + })` + } + expectedBazelTargets := []string{ - makeBazelTarget("genrule", "foo", attrNameToString{ - "cmd": fmt.Sprintf(`"$(location :foo.tool) --genDir=%s arg $(SRCS) $(OUTS)"`, tc.genDir), - "outs": `["foo.out"]`, - "srcs": `["foo.in"]`, - "tools": `[":foo.tool"]`, - }), + makeBazelTarget("genrule", "foo", moduleAttrs), } t.Run(tc.moduleType, func(t *testing.T) { @@ -158,25 +167,36 @@ func TestGenruleLocationsLabel(t *testing.T) { bazel_module: { bp2build_available: true }, }` - expectedBazelTargets := - []string{ - makeBazelTarget("genrule", "foo", attrNameToString{ - "cmd": `"$(locations :foo.tools) -s $(OUTS) $(SRCS)"`, - "outs": `["foo.out"]`, - "srcs": `["foo.in"]`, - "tools": `[":foo.tools"]`, - }), - makeBazelTarget("genrule", "foo.tools", attrNameToString{ - "cmd": `"cp $(SRCS) $(OUTS)"`, - "outs": `[ + for _, tc := range testCases { + fooAttrs := attrNameToString{ + "cmd": `"$(locations :foo.tools) -s $(OUTS) $(SRCS)"`, + "outs": `["foo.out"]`, + "srcs": `["foo.in"]`, + "tools": `[":foo.tools"]`, + } + fooToolsAttrs := attrNameToString{ + "cmd": `"cp $(SRCS) $(OUTS)"`, + "outs": `[ "foo_tool.out", "foo_tool2.out", ]`, - "srcs": `["foo_tool.in"]`, - }), + "srcs": `["foo_tool.in"]`, + } + + if tc.moduleType == "java_genrule_host" { + compatibilityAttrs := `select({ + "//build/bazel/platforms/os:android": ["@platforms//:incompatible"], + "//conditions:default": [], + })` + fooAttrs["target_compatible_with"] = compatibilityAttrs + fooToolsAttrs["target_compatible_with"] = compatibilityAttrs + } + + expectedBazelTargets := []string{ + makeBazelTarget("genrule", "foo", fooAttrs), + makeBazelTarget("genrule", "foo.tools", fooToolsAttrs), } - for _, tc := range testCases { t.Run(tc.moduleType, func(t *testing.T) { runBp2BuildTestCase(t, func(ctx android.RegistrationContext) {}, bp2buildTestCase{ @@ -221,16 +241,25 @@ func TestGenruleLocationsAbsoluteLabel(t *testing.T) { bazel_module: { bp2build_available: true }, }` - expectedBazelTargets := []string{ - makeBazelTarget("genrule", "foo", attrNameToString{ + for _, tc := range testCases { + moduleAttrs := attrNameToString{ "cmd": `"$(locations //other:foo.tool) -s $(OUTS) $(SRCS)"`, "outs": `["foo.out"]`, "srcs": `["foo.in"]`, "tools": `["//other:foo.tool"]`, - }), - } + } + + if tc.moduleType == "java_genrule_host" { + moduleAttrs["target_compatible_with"] = `select({ + "//build/bazel/platforms/os:android": ["@platforms//:incompatible"], + "//conditions:default": [], + })` + } + + expectedBazelTargets := []string{ + makeBazelTarget("genrule", "foo", moduleAttrs), + } - for _, tc := range testCases { t.Run(tc.moduleType, func(t *testing.T) { runBp2BuildTestCase(t, func(ctx android.RegistrationContext) {}, bp2buildTestCase{ @@ -276,16 +305,25 @@ func TestGenruleSrcsLocationsAbsoluteLabel(t *testing.T) { bazel_module: { bp2build_available: true }, }` - expectedBazelTargets := []string{ - makeBazelTarget("genrule", "foo", attrNameToString{ + for _, tc := range testCases { + moduleAttrs := attrNameToString{ "cmd": `"$(locations //other:foo.tool) -s $(OUTS) $(location //other:other.tool)"`, "outs": `["foo.out"]`, "srcs": `["//other:other.tool"]`, "tools": `["//other:foo.tool"]`, - }), - } + } + + if tc.moduleType == "java_genrule_host" { + moduleAttrs["target_compatible_with"] = `select({ + "//build/bazel/platforms/os:android": ["@platforms//:incompatible"], + "//conditions:default": [], + })` + } + + expectedBazelTargets := []string{ + makeBazelTarget("genrule", "foo", moduleAttrs), + } - for _, tc := range testCases { t.Run(tc.moduleType, func(t *testing.T) { runBp2BuildTestCase(t, func(ctx android.RegistrationContext) {}, bp2buildTestCase{ @@ -331,8 +369,8 @@ func TestGenruleLocationLabelShouldSubstituteFirstToolLabel(t *testing.T) { bazel_module: { bp2build_available: true }, }` - expectedBazelTargets := []string{ - makeBazelTarget("genrule", "foo", attrNameToString{ + for _, tc := range testCases { + moduleAttrs := attrNameToString{ "cmd": `"$(location //other:foo.tool) -s $(OUTS) $(SRCS)"`, "outs": `["foo.out"]`, "srcs": `["foo.in"]`, @@ -340,9 +378,19 @@ func TestGenruleLocationLabelShouldSubstituteFirstToolLabel(t *testing.T) { "//other:foo.tool", "//other:other.tool", ]`, - })} + } + + if tc.moduleType == "java_genrule_host" { + moduleAttrs["target_compatible_with"] = `select({ + "//build/bazel/platforms/os:android": ["@platforms//:incompatible"], + "//conditions:default": [], + })` + } + + expectedBazelTargets := []string{ + makeBazelTarget("genrule", "foo", moduleAttrs), + } - for _, tc := range testCases { t.Run(tc.moduleType, func(t *testing.T) { runBp2BuildTestCase(t, func(ctx android.RegistrationContext) {}, bp2buildTestCase{ @@ -388,8 +436,8 @@ func TestGenruleLocationsLabelShouldSubstituteFirstToolLabel(t *testing.T) { bazel_module: { bp2build_available: true }, }` - expectedBazelTargets := []string{ - makeBazelTarget("genrule", "foo", attrNameToString{ + for _, tc := range testCases { + moduleAttrs := attrNameToString{ "cmd": `"$(locations //other:foo.tool) -s $(OUTS) $(SRCS)"`, "outs": `["foo.out"]`, "srcs": `["foo.in"]`, @@ -397,9 +445,19 @@ func TestGenruleLocationsLabelShouldSubstituteFirstToolLabel(t *testing.T) { "//other:foo.tool", "//other:other.tool", ]`, - })} + } + + if tc.moduleType == "java_genrule_host" { + moduleAttrs["target_compatible_with"] = `select({ + "//build/bazel/platforms/os:android": ["@platforms//:incompatible"], + "//conditions:default": [], + })` + } + + expectedBazelTargets := []string{ + makeBazelTarget("genrule", "foo", moduleAttrs), + } - for _, tc := range testCases { t.Run(tc.moduleType, func(t *testing.T) { runBp2BuildTestCase(t, func(ctx android.RegistrationContext) {}, bp2buildTestCase{ @@ -444,14 +502,24 @@ func TestGenruleWithoutToolsOrToolFiles(t *testing.T) { bazel_module: { bp2build_available: true }, }` - expectedBazelTargets := []string{ - makeBazelTarget("genrule", "foo", attrNameToString{ + for _, tc := range testCases { + moduleAttrs := attrNameToString{ "cmd": `"cp $(SRCS) $(OUTS)"`, "outs": `["foo.out"]`, "srcs": `["foo.in"]`, - })} + } + + if tc.moduleType == "java_genrule_host" { + moduleAttrs["target_compatible_with"] = `select({ + "//build/bazel/platforms/os:android": ["@platforms//:incompatible"], + "//conditions:default": [], + })` + } + + expectedBazelTargets := []string{ + makeBazelTarget("genrule", "foo", moduleAttrs), + } - for _, tc := range testCases { t.Run(tc.moduleType, func(t *testing.T) { runBp2BuildTestCase(t, func(ctx android.RegistrationContext) {}, bp2buildTestCase{ diff --git a/bp2build/java_binary_host_conversion_test.go b/bp2build/java_binary_host_conversion_test.go index c683b25f5..65136d9f1 100644 --- a/bp2build/java_binary_host_conversion_test.go +++ b/bp2build/java_binary_host_conversion_test.go @@ -59,6 +59,10 @@ func TestJavaBinaryHost(t *testing.T) { "deps": `["//other:jni-lib-1"]`, "jvm_flags": `["-Djava.library.path=$${RUNPATH}other"]`, "javacopts": `["-Xdoclint:all/protected"]`, + "target_compatible_with": `select({ + "//build/bazel/platforms/os:android": ["@platforms//:incompatible"], + "//conditions:default": [], + })`, }), }, }) diff --git a/bp2build/java_library_host_conversion_test.go b/bp2build/java_library_host_conversion_test.go index 6ac82dbd2..73abdd2ea 100644 --- a/bp2build/java_library_host_conversion_test.go +++ b/bp2build/java_library_host_conversion_test.go @@ -48,9 +48,17 @@ java_library_host { makeBazelTarget("java_library", "java-lib-host-1", attrNameToString{ "srcs": `["a.java"]`, "deps": `[":java-lib-host-2"]`, + "target_compatible_with": `select({ + "//build/bazel/platforms/os:android": ["@platforms//:incompatible"], + "//conditions:default": [], + })`, }), makeBazelTarget("java_library", "java-lib-host-2", attrNameToString{ "srcs": `["c.java"]`, + "target_compatible_with": `select({ + "//build/bazel/platforms/os:android": ["@platforms//:incompatible"], + "//conditions:default": [], + })`, }), }, }) diff --git a/bp2build/python_binary_conversion_test.go b/bp2build/python_binary_conversion_test.go index 40c8ba1e9..dfa11d1be 100644 --- a/bp2build/python_binary_conversion_test.go +++ b/bp2build/python_binary_conversion_test.go @@ -51,6 +51,10 @@ func TestPythonBinaryHostSimple(t *testing.T) { "b/c.py", "b/d.py", ]`, + "target_compatible_with": `select({ + "//build/bazel/platforms/os:android": ["@platforms//:incompatible"], + "//conditions:default": [], + })`, }), }, }) @@ -80,6 +84,10 @@ func TestPythonBinaryHostPy2(t *testing.T) { makeBazelTarget("py_binary", "foo", attrNameToString{ "python_version": `"PY2"`, "srcs": `["a.py"]`, + "target_compatible_with": `select({ + "//build/bazel/platforms/os:android": ["@platforms//:incompatible"], + "//conditions:default": [], + })`, }), }, }) @@ -109,6 +117,10 @@ func TestPythonBinaryHostPy3(t *testing.T) { // python_version is PY3 by default. makeBazelTarget("py_binary", "foo", attrNameToString{ "srcs": `["a.py"]`, + "target_compatible_with": `select({ + "//build/bazel/platforms/os:android": ["@platforms//:incompatible"], + "//conditions:default": [], + })`, }), }, }) @@ -140,6 +152,10 @@ func TestPythonBinaryHostArchVariance(t *testing.T) { "//build/bazel/platforms/arch:arm": ["arm.py"], "//build/bazel/platforms/arch:x86": ["x86.py"], "//conditions:default": [], + })`, + "target_compatible_with": `select({ + "//build/bazel/platforms/os:android": ["@platforms//:incompatible"], + "//conditions:default": [], })`, }), }, diff --git a/bp2build/python_library_conversion_test.go b/bp2build/python_library_conversion_test.go index 6b261052c..356d52e8c 100644 --- a/bp2build/python_library_conversion_test.go +++ b/bp2build/python_library_conversion_test.go @@ -11,19 +11,51 @@ import ( // TODO(alexmarquez): Should be lifted into a generic Bp2Build file type PythonLibBp2Build func(ctx android.TopDownMutatorContext) -func runPythonLibraryTestCase(t *testing.T, tc bp2buildTestCase) { +type pythonLibBp2BuildTestCase struct { + description string + filesystem map[string]string + blueprint string + expectedBazelTargets []testBazelTarget +} + +func convertPythonLibTestCaseToBp2build_Host(tc pythonLibBp2BuildTestCase) bp2buildTestCase { + for i := range tc.expectedBazelTargets { + tc.expectedBazelTargets[i].attrs["target_compatible_with"] = `select({ + "//build/bazel/platforms/os:android": ["@platforms//:incompatible"], + "//conditions:default": [], + })` + } + + return convertPythonLibTestCaseToBp2build(tc) +} + +func convertPythonLibTestCaseToBp2build(tc pythonLibBp2BuildTestCase) bp2buildTestCase { + var bp2BuildTargets []string + for _, t := range tc.expectedBazelTargets { + bp2BuildTargets = append(bp2BuildTargets, makeBazelTarget(t.typ, t.name, t.attrs)) + } + return bp2buildTestCase{ + description: tc.description, + filesystem: tc.filesystem, + blueprint: tc.blueprint, + expectedBazelTargets: bp2BuildTargets, + } +} + +func runPythonLibraryTestCase(t *testing.T, tc pythonLibBp2BuildTestCase) { t.Helper() - testCase := tc + testCase := convertPythonLibTestCaseToBp2build(tc) testCase.description = fmt.Sprintf(testCase.description, "python_library") testCase.blueprint = fmt.Sprintf(testCase.blueprint, "python_library") testCase.moduleTypeUnderTest = "python_library" testCase.moduleTypeUnderTestFactory = python.PythonLibraryFactory + runBp2BuildTestCaseSimple(t, testCase) } -func runPythonLibraryHostTestCase(t *testing.T, tc bp2buildTestCase) { +func runPythonLibraryHostTestCase(t *testing.T, tc pythonLibBp2BuildTestCase) { t.Helper() - testCase := tc + testCase := convertPythonLibTestCaseToBp2build_Host(tc) testCase.description = fmt.Sprintf(testCase.description, "python_library_host") testCase.blueprint = fmt.Sprintf(testCase.blueprint, "python_library_host") testCase.moduleTypeUnderTest = "python_library_host" @@ -34,14 +66,14 @@ func runPythonLibraryHostTestCase(t *testing.T, tc bp2buildTestCase) { testCase) } -func runPythonLibraryTestCases(t *testing.T, tc bp2buildTestCase) { +func runPythonLibraryTestCases(t *testing.T, tc pythonLibBp2BuildTestCase) { t.Helper() runPythonLibraryTestCase(t, tc) runPythonLibraryHostTestCase(t, tc) } func TestSimplePythonLib(t *testing.T) { - testCases := []bp2buildTestCase{ + testCases := []pythonLibBp2BuildTestCase{ { description: "simple %s converts to a native py_library", filesystem: map[string]string{ @@ -64,17 +96,21 @@ func TestSimplePythonLib(t *testing.T) { srcs: ["b/e.py"], bazel_module: { bp2build_available: false }, }`, - expectedBazelTargets: []string{ - makeBazelTarget("py_library", "foo", attrNameToString{ - "data": `["files/data.txt"]`, - "deps": `[":bar"]`, - "srcs": `[ + expectedBazelTargets: []testBazelTarget{ + { + typ: "py_library", + name: "foo", + attrs: attrNameToString{ + "data": `["files/data.txt"]`, + "deps": `[":bar"]`, + "srcs": `[ "a.py", "b/c.py", "b/d.py", ]`, - "srcs_version": `"PY3"`, - }), + "srcs_version": `"PY3"`, + }, + }, }, }, { @@ -93,11 +129,15 @@ func TestSimplePythonLib(t *testing.T) { bazel_module: { bp2build_available: true }, }`, - expectedBazelTargets: []string{ - makeBazelTarget("py_library", "foo", attrNameToString{ - "srcs": `["a.py"]`, - "srcs_version": `"PY2"`, - }), + expectedBazelTargets: []testBazelTarget{ + { + typ: "py_library", + name: "foo", + attrs: attrNameToString{ + "srcs": `["a.py"]`, + "srcs_version": `"PY2"`, + }, + }, }, }, { @@ -116,11 +156,15 @@ func TestSimplePythonLib(t *testing.T) { bazel_module: { bp2build_available: true }, }`, - expectedBazelTargets: []string{ - makeBazelTarget("py_library", "foo", attrNameToString{ - "srcs": `["a.py"]`, - "srcs_version": `"PY3"`, - }), + expectedBazelTargets: []testBazelTarget{ + { + typ: "py_library", + name: "foo", + attrs: attrNameToString{ + "srcs": `["a.py"]`, + "srcs_version": `"PY3"`, + }, + }, }, }, { @@ -139,11 +183,15 @@ func TestSimplePythonLib(t *testing.T) { bazel_module: { bp2build_available: true }, }`, - expectedBazelTargets: []string{ - // srcs_version is PY2ANDPY3 by default. - makeBazelTarget("py_library", "foo", attrNameToString{ - "srcs": `["a.py"]`, - }), + expectedBazelTargets: []testBazelTarget{ + { + // srcs_version is PY2ANDPY3 by default. + typ: "py_library", + name: "foo", + attrs: attrNameToString{ + "srcs": `["a.py"]`, + }, + }, }, }, } @@ -156,7 +204,7 @@ func TestSimplePythonLib(t *testing.T) { } func TestPythonArchVariance(t *testing.T) { - runPythonLibraryTestCases(t, bp2buildTestCase{ + runPythonLibraryTestCases(t, pythonLibBp2BuildTestCase{ description: "test %s arch variants", filesystem: map[string]string{ "dir/arm.py": "", @@ -173,15 +221,19 @@ func TestPythonArchVariance(t *testing.T) { }, }, }`, - expectedBazelTargets: []string{ - makeBazelTarget("py_library", "foo", attrNameToString{ - "srcs": `select({ + expectedBazelTargets: []testBazelTarget{ + { + typ: "py_library", + name: "foo", + attrs: attrNameToString{ + "srcs": `select({ "//build/bazel/platforms/arch:arm": ["arm.py"], "//build/bazel/platforms/arch:x86": ["x86.py"], "//conditions:default": [], })`, - "srcs_version": `"PY3"`, - }), + "srcs_version": `"PY3"`, + }, + }, }, }) } diff --git a/cc/binary.go b/cc/binary.go index bf77d3deb..05923b1b8 100644 --- a/cc/binary.go +++ b/cc/binary.go @@ -605,19 +605,12 @@ func binaryBp2build(ctx android.TopDownMutatorContext, m *Module, typ string) { Features: baseAttrs.features, } - var enabledProperty bazel.BoolAttribute - if typ == "cc_binary_host" { - falseVal := false - enabledProperty.SetSelectValue(bazel.OsConfigurationAxis, android.Android.Name, &falseVal) - } - - ctx.CreateBazelTargetModuleWithRestrictions(bazel.BazelTargetModuleProperties{ + ctx.CreateBazelTargetModule(bazel.BazelTargetModuleProperties{ Rule_class: "cc_binary", Bzl_load_location: "//build/bazel/rules:cc_binary.bzl", }, android.CommonAttributes{Name: m.Name()}, - attrs, - enabledProperty) + attrs) } // binaryAttributes contains Bazel attributes corresponding to a cc binary diff --git a/java/plugin.go b/java/plugin.go index f1a5ec451..4b174b930 100644 --- a/java/plugin.go +++ b/java/plugin.go @@ -17,8 +17,6 @@ package java import ( "android/soong/android" "android/soong/bazel" - - "github.com/google/blueprint/proptools" ) func init() { @@ -78,12 +76,9 @@ func (p *Plugin) ConvertWithBp2build(ctx android.TopDownMutatorContext) { attrs.Processor_class = p.pluginProperties.Processor_class } - var enabledProperty bazel.BoolAttribute - enabledProperty.SetSelectValue(bazel.OsConfigurationAxis, android.Android.Name, proptools.BoolPtr(false)) - props := bazel.BazelTargetModuleProperties{ Rule_class: "java_plugin", } - ctx.CreateBazelTargetModuleWithRestrictions(props, android.CommonAttributes{Name: p.Name()}, attrs, enabledProperty) + ctx.CreateBazelTargetModule(props, android.CommonAttributes{Name: p.Name()}, attrs) }