From ab29f57ce6ceb9d4b2fa0ebc144776887ea061e6 Mon Sep 17 00:00:00 2001
From: Spandan Das <spandandas@google.com>
Date: Fri, 1 Sep 2023 19:43:56 +0000
Subject: [PATCH] Fix target_compatible_with non-determinism for
 proto.include_dirs

A single proto providing directory can be used by multiple soong modules. Some of these
can be
1. Host specific
2. Device specific
3. Both host and device

Since the generated proto_library can have 1:many mapping, it should
have an empty target_compatible_with. Compatiblity will be enforced at
the top-level {cc|java|python}_proto_library.

(This is a followup to aosp/2727054 which did not handle this correctly)

Test: Added a unit test

Change-Id: I09b3def70e3d043fd8ba0d1eb4ffff1910f097d1
---
 android/module.go                      |  2 +-
 android/proto.go                       |  5 ++++
 bp2build/cc_library_conversion_test.go | 38 ++++++++++++++++++++++++++
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/android/module.go b/android/module.go
index 19502bae8..f9ea89778 100644
--- a/android/module.go
+++ b/android/module.go
@@ -1371,7 +1371,7 @@ func (attrs *CommonAttributes) fillCommonBp2BuildModuleAttrs(ctx *topDownMutator
 		for _, axis := range enabledPropertyOverrides.SortedConfigurationAxes() {
 			configToBools := enabledPropertyOverrides.ConfigurableValues[axis]
 			for cfg, val := range configToBools {
-				if axis != bazel.OsConfigurationAxis || osSupport[cfg] {
+				if axis != bazel.OsConfigurationAxis || osSupport[cfg] || val /*If enabled is explicitly requested via overrides */ {
 					enabledProperty.SetSelectValue(axis, cfg, &val)
 				}
 			}
diff --git a/android/proto.go b/android/proto.go
index 7c418c693..fc21d0177 100644
--- a/android/proto.go
+++ b/android/proto.go
@@ -408,6 +408,11 @@ func createProtoLibraryTargetsForIncludeDirs(ctx Bp2buildMutatorContext, include
 			// As a workarounds, delete `target_compatible_with`
 			alwaysEnabled := bazel.BoolAttribute{}
 			alwaysEnabled.Value = proptools.BoolPtr(true)
+			// Add android and linux explicitly so that fillcommonbp2buildmoduleattrs can override these configs
+			// When we extend b support for other os'es (darwin/windows), we should add those configs here as well
+			alwaysEnabled.SetSelectValue(bazel.OsConfigurationAxis, bazel.OsAndroid, proptools.BoolPtr(true))
+			alwaysEnabled.SetSelectValue(bazel.OsConfigurationAxis, bazel.OsLinux, proptools.BoolPtr(true))
+
 			ctx.CreateBazelTargetModuleWithRestrictions(
 				bazel.BazelTargetModuleProperties{Rule_class: "proto_library"},
 				CommonAttributes{
diff --git a/bp2build/cc_library_conversion_test.go b/bp2build/cc_library_conversion_test.go
index 02ed57a51..e50c710d2 100644
--- a/bp2build/cc_library_conversion_test.go
+++ b/bp2build/cc_library_conversion_test.go
@@ -5241,3 +5241,41 @@ func TestProtoLocalIncludeDirs(t *testing.T) {
 	}
 	runCcLibraryTestCase(t, tc)
 }
+
+// `foo_device` and `bar_host` can depend on .proto files of a specific dir,
+// the dynamically generated proto_library should not have any target_compatible_with
+func TestProtoLibraryForIncludeDirsIsOsAgnostic(t *testing.T) {
+	tc := Bp2buildTestCase{
+		Description:                "proto_library generated for proto.include_dirs is compatible for all axes",
+		ModuleTypeUnderTest:        "cc_library",
+		ModuleTypeUnderTestFactory: cc.LibraryFactory,
+		Blueprint: simpleModuleDoNotConvertBp2build("cc_library", "libprotobuf-cpp-lite") + `
+cc_library {
+	name: "foo_device",
+	device_supported: true, // this is the default behavior, but added explicitly here for illustration
+	host_supported: false,
+	proto: {include_dirs: ["dir"]},
+}
+cc_library {
+	name: "bar_host",
+	device_supported: false,
+	host_supported: true,
+	srcs: ["bar.proto"],
+	proto: {include_dirs: ["dir"]},
+}
+`,
+		Filesystem: map[string]string{
+			"dir/Android.bp": "",
+			"dir/dir.proto":  "",
+		},
+		Dir: "dir", // check for the generated proto_library
+		ExpectedBazelTargets: []string{
+			MakeBazelTargetNoRestrictions("proto_library", "dir.include_dir_bp2build_generated_proto", AttrNameToString{
+				"srcs":                `["dir.proto"]`,
+				"strip_import_prefix": `""`,
+				"tags":                `["manual"]`,
+			}),
+		},
+	}
+	runCcLibraryTestCase(t, tc)
+}