Merge "bp2build: Split export_{includes,system_includes}"

bazel/properties.go

@@ -743,6 +743,31 @@ func (sla *StringListAttribute) SortedConfigurationAxes() []ConfigurationAxis {
 	return keys
 }
 
+// DeduplicateAxesFromBase ensures no duplication of items between the no-configuration value and
+// configuration-specific values. For example, if we would convert this StringListAttribute as:
+// ["a", "b", "c"] + select({
+//    "//condition:one": ["a", "d"],
+//    "//conditions:default": [],
+// })
+// after this function, we would convert this StringListAttribute as:
+// ["a", "b", "c"] + select({
+//    "//condition:one": ["d"],
+//    "//conditions:default": [],
+// })
+func (sla *StringListAttribute) DeduplicateAxesFromBase() {
+	base := sla.Value
+	for axis, configToList := range sla.ConfigurableValues {
+		for config, list := range configToList {
+			remaining := SubtractStrings(list, base)
+			if len(remaining) == 0 {
+				delete(sla.ConfigurableValues[axis], config)
+			} else {
+				sla.ConfigurableValues[axis][config] = remaining
+			}
+		}
+	}
+}
+
 // TryVariableSubstitution, replace string substitution formatting within each string in slice with
 // Starlark string.format compatible tag for productVariable.
 func TryVariableSubstitutions(slice []string, productVariable string) ([]string, bool) {

bazel/properties_test.go

@@ -293,3 +293,74 @@ func TestResolveExcludes(t *testing.T) {
 		}
 	}
 }
+
+func TestDeduplicateAxesFromBase(t *testing.T) {
+	attr := StringListAttribute{
+		Value: []string{
+			"all_include",
+			"arm_include",
+			"android_include",
+			"linux_x86_include",
+		},
+		ConfigurableValues: configurableStringLists{
+			ArchConfigurationAxis: stringListSelectValues{
+				"arm": []string{"arm_include"},
+				"x86": []string{"x86_include"},
+			},
+			OsConfigurationAxis: stringListSelectValues{
+				"android": []string{"android_include"},
+				"linux":   []string{"linux_include"},
+			},
+			OsArchConfigurationAxis: stringListSelectValues{
+				"linux_x86": {"linux_x86_include"},
+			},
+			ProductVariableConfigurationAxis("a"): stringListSelectValues{
+				"a": []string{"not_in_value"},
+			},
+		},
+	}
+
+	attr.DeduplicateAxesFromBase()
+
+	expectedBaseIncludes := []string{
+		"all_include",
+		"arm_include",
+		"android_include",
+		"linux_x86_include",
+	}
+	if !reflect.DeepEqual(expectedBaseIncludes, attr.Value) {
+		t.Errorf("Expected Value includes %q, got %q", attr.Value, expectedBaseIncludes)
+	}
+	expectedConfiguredIncludes := configurableStringLists{
+		ArchConfigurationAxis: stringListSelectValues{
+			"x86": []string{"x86_include"},
+		},
+		OsConfigurationAxis: stringListSelectValues{
+			"linux": []string{"linux_include"},
+		},
+		OsArchConfigurationAxis: stringListSelectValues{},
+		ProductVariableConfigurationAxis("a"): stringListSelectValues{
+			"a": []string{"not_in_value"},
+		},
+	}
+	for _, axis := range attr.SortedConfigurationAxes() {
+		if _, ok := expectedConfiguredIncludes[axis]; !ok {
+			t.Errorf("Found unexpected axis %s", axis)
+			continue
+		}
+		expectedForAxis := expectedConfiguredIncludes[axis]
+		gotForAxis := attr.ConfigurableValues[axis]
+		if len(expectedForAxis) != len(gotForAxis) {
+			t.Errorf("Expected %d configs for %s, got %d: %s", len(expectedForAxis), axis, len(gotForAxis), gotForAxis)
+		}
+		for config, value := range gotForAxis {
+			if expected, ok := expectedForAxis[config]; ok {
+				if !reflect.DeepEqual(expected, value) {
+					t.Errorf("For %s, expected: %#v, got %#v", axis, expected, value)
+				}
+			} else {
+				t.Errorf("Got unexpected config %q for %s", config, axis)
+			}
+		}
+	}
+}

bp2build/cc_library_conversion_test.go

@@ -122,8 +122,8 @@ cc_library {
         "-I.",
         "-I$(BINDIR)/.",
     ],
+    export_includes = ["foo-dir"],
     implementation_deps = [":some-headers"],
-    includes = ["foo-dir"],
     linkopts = ["-Wl,--exclude-libs=bar.a"] + select({
         "//build/bazel/platforms/arch:x86": ["-Wl,--exclude-libs=baz.a"],
         "//build/bazel/platforms/arch:x86_64": ["-Wl,--exclude-libs=qux.a"],

bp2build/cc_library_headers_conversion_test.go

@@ -132,11 +132,7 @@ cc_library_headers {
         "-I.",
         "-I$(BINDIR)/.",
     ],
-    implementation_deps = [
+    export_includes = [
-        ":lib-1",
-        ":lib-2",
-    ],
-    includes = [
         "dir-1",
         "dir-2",
     ] + select({
@@ -145,20 +141,24 @@ cc_library_headers {
         "//build/bazel/platforms/arch:x86_64": ["arch_x86_64_exported_include_dir"],
         "//conditions:default": [],
     }),
+    implementation_deps = [
+        ":lib-1",
+        ":lib-2",
+    ],
 )`, `cc_library_headers(
     name = "lib-1",
     copts = [
         "-I.",
         "-I$(BINDIR)/.",
     ],
-    includes = ["lib-1"],
+    export_includes = ["lib-1"],
 )`, `cc_library_headers(
     name = "lib-2",
     copts = [
         "-I.",
         "-I$(BINDIR)/.",
     ],
-    includes = ["lib-2"],
+    export_includes = ["lib-2"],
 )`},
 	})
 }
@@ -337,7 +337,7 @@ func TestCcLibraryHeadersArchAndTargetExportSystemIncludes(t *testing.T) {
         "-I.",
         "-I$(BINDIR)/.",
     ],
-    includes = ["shared_include_dir"] + select({
+    export_system_includes = ["shared_include_dir"] + select({
         "//build/bazel/platforms/arch:arm": ["arm_include_dir"],
         "//build/bazel/platforms/arch:x86_64": ["x86_64_include_dir"],
         "//conditions:default": [],
@@ -382,7 +382,7 @@ cc_library_headers {
         "-I.",
         "-I$(BINDIR)/.",
     ],
-    includes = ["lib-1"],
+    export_includes = ["lib-1"],
 )`},
 	})
 }

bp2build/cc_library_static_conversion_test.go

@@ -192,16 +192,16 @@ cc_library_static {
         "-I.",
         "-I$(BINDIR)/.",
     ],
+    export_includes = [
+        "export_include_dir_1",
+        "export_include_dir_2",
+    ],
     implementation_deps = [
         ":header_lib_1",
         ":header_lib_2",
         ":static_lib_1",
         ":static_lib_2",
     ],
-    includes = [
-        "export_include_dir_1",
-        "export_include_dir_2",
-    ],
     linkstatic = True,
     srcs = [
         "foo_static1.cc",
@@ -312,7 +312,7 @@ cc_library_static {
         "-I.",
         "-I$(BINDIR)/.",
     ],
-    includes = ["subpackage"],
+    export_includes = ["subpackage"],
     linkstatic = True,
 )`},
 	})
@@ -341,7 +341,7 @@ cc_library_static {
         "-I.",
         "-I$(BINDIR)/.",
     ],
-    includes = ["subpackage"],
+    export_system_includes = ["subpackage"],
     linkstatic = True,
 )`},
 	})
@@ -391,7 +391,7 @@ cc_library_static {
         "-Isubpackage",
         "-I$(BINDIR)/subpackage",
     ],
-    includes = ["./exported_subsubpackage"],
+    export_includes = ["./exported_subsubpackage"],
     linkstatic = True,
 )`},
 	})

cc/bp2build.go

@@ -535,12 +535,21 @@ func bp2BuildMakePathsRelativeToModule(ctx android.BazelConversionPathContext, p
 	return relativePaths
 }
 
-func bp2BuildParseExportedIncludes(ctx android.TopDownMutatorContext, module *Module) bazel.StringListAttribute {
+// BazelIncludes contains information about -I and -isystem paths from a module converted to Bazel
+// attributes.
+type BazelIncludes struct {
+	Includes       bazel.StringListAttribute
+	SystemIncludes bazel.StringListAttribute
+}
+
+func bp2BuildParseExportedIncludes(ctx android.TopDownMutatorContext, module *Module) BazelIncludes {
 	libraryDecorator := module.linker.(*libraryDecorator)
 	return bp2BuildParseExportedIncludesHelper(ctx, module, libraryDecorator)
 }
 
-func Bp2BuildParseExportedIncludesForPrebuiltLibrary(ctx android.TopDownMutatorContext, module *Module) bazel.StringListAttribute {
+// Bp2buildParseExportedIncludesForPrebuiltLibrary returns a BazelIncludes with Bazel-ified values
+// to export includes from the underlying module's properties.
+func Bp2BuildParseExportedIncludesForPrebuiltLibrary(ctx android.TopDownMutatorContext, module *Module) BazelIncludes {
 	prebuiltLibraryLinker := module.linker.(*prebuiltLibraryLinker)
 	libraryDecorator := prebuiltLibraryLinker.libraryDecorator
 	return bp2BuildParseExportedIncludesHelper(ctx, module, libraryDecorator)
@@ -548,36 +557,22 @@ func Bp2BuildParseExportedIncludesForPrebuiltLibrary(ctx android.TopDownMutatorC
 
 // bp2BuildParseExportedIncludes creates a string list attribute contains the
 // exported included directories of a module.
-func bp2BuildParseExportedIncludesHelper(ctx android.TopDownMutatorContext, module *Module, libraryDecorator *libraryDecorator) bazel.StringListAttribute {
+func bp2BuildParseExportedIncludesHelper(ctx android.TopDownMutatorContext, module *Module, libraryDecorator *libraryDecorator) BazelIncludes {
-	// Export_system_include_dirs and export_include_dirs are already module dir
+	exported := BazelIncludes{}
-	// relative, so they don't need to be relativized like include_dirs, which
-	// are root-relative.
-	includeDirs := libraryDecorator.flagExporter.Properties.Export_system_include_dirs
-	includeDirs = append(includeDirs, libraryDecorator.flagExporter.Properties.Export_include_dirs...)
-	var includeDirsAttribute bazel.StringListAttribute
-
-	getVariantIncludeDirs := func(includeDirs []string, flagExporterProperties *FlagExporterProperties, subtract bool) []string {
-		variantIncludeDirs := flagExporterProperties.Export_system_include_dirs
-		variantIncludeDirs = append(variantIncludeDirs, flagExporterProperties.Export_include_dirs...)
-
-		if subtract {
-			// To avoid duplicate includes when base includes + arch includes are combined
-			// TODO: Add something similar to ResolveExcludes() in bazel/properties.go
-			variantIncludeDirs = bazel.SubtractStrings(variantIncludeDirs, includeDirs)
-		}
-		return variantIncludeDirs
-	}
-
 	for axis, configToProps := range module.GetArchVariantProperties(ctx, &FlagExporterProperties{}) {
 		for config, props := range configToProps {
 			if flagExporterProperties, ok := props.(*FlagExporterProperties); ok {
-				archVariantIncludeDirs := getVariantIncludeDirs(includeDirs, flagExporterProperties, axis != bazel.NoConfigAxis)
+				if len(flagExporterProperties.Export_include_dirs) > 0 {
-				if len(archVariantIncludeDirs) > 0 {
+					exported.Includes.SetSelectValue(axis, config, flagExporterProperties.Export_include_dirs)
-					includeDirsAttribute.SetSelectValue(axis, config, archVariantIncludeDirs)
+				}
+				if len(flagExporterProperties.Export_system_include_dirs) > 0 {
+					exported.SystemIncludes.SetSelectValue(axis, config, flagExporterProperties.Export_system_include_dirs)
 				}
 			}
 		}
 	}
+	exported.Includes.DeduplicateAxesFromBase()
+	exported.SystemIncludes.DeduplicateAxesFromBase()
 
-	return includeDirsAttribute
+	return exported
 }

cc/library.go

@@ -234,7 +234,8 @@ type bazelCcLibraryAttributes struct {
 	Dynamic_deps           bazel.LabelListAttribute
 	Whole_archive_deps     bazel.LabelListAttribute
 	System_dynamic_deps    bazel.LabelListAttribute
-	Includes            bazel.StringListAttribute
+	Export_includes        bazel.StringListAttribute
+	Export_system_includes bazel.StringListAttribute
 	Linkopts               bazel.StringListAttribute
 	Use_libcrt             bazel.BoolAttribute
 	Rtti                   bazel.BoolAttribute
@@ -304,7 +305,8 @@ func CcLibraryBp2Build(ctx android.TopDownMutatorContext) {
 		Dynamic_deps:           linkerAttrs.dynamicDeps,
 		Whole_archive_deps:     linkerAttrs.wholeArchiveDeps,
 		System_dynamic_deps:    linkerAttrs.systemDynamicDeps,
-		Includes:            exportedIncludes,
+		Export_includes:        exportedIncludes.Includes,
+		Export_system_includes: exportedIncludes.SystemIncludes,
 		Linkopts:               linkerAttrs.linkopts,
 		Use_libcrt:             linkerAttrs.useLibcrt,
 		Rtti:                   compilerAttrs.rtti,
@@ -2329,7 +2331,8 @@ type bazelCcLibraryStaticAttributes struct {
 	Linkstatic             bool
 	Use_libcrt             bazel.BoolAttribute
 	Rtti                   bazel.BoolAttribute
-	Includes            bazel.StringListAttribute
+	Export_includes        bazel.StringListAttribute
+	Export_system_includes bazel.StringListAttribute
 	Hdrs                   bazel.LabelListAttribute
 
 	Cppflags   bazel.StringListAttribute
@@ -2379,7 +2382,8 @@ func ccLibraryStaticBp2BuildInternal(ctx android.TopDownMutatorContext, module *
 		Linkstatic:             true,
 		Use_libcrt:             linkerAttrs.useLibcrt,
 		Rtti:                   compilerAttrs.rtti,
-		Includes:   exportedIncludes,
+		Export_includes:        exportedIncludes.Includes,
+		Export_system_includes: exportedIncludes.SystemIncludes,
 
 		Cppflags:   compilerAttrs.cppFlags,
 		Srcs_c:     compilerAttrs.cSrcs,

cc/library_headers.go

@@ -105,7 +105,8 @@ func prebuiltLibraryHeaderFactory() android.Module {
 type bazelCcLibraryHeadersAttributes struct {
 	Copts                  bazel.StringListAttribute
 	Hdrs                   bazel.LabelListAttribute
-	Includes            bazel.StringListAttribute
+	Export_includes        bazel.StringListAttribute
+	Export_system_includes bazel.StringListAttribute
 	Deps                   bazel.LabelListAttribute
 	Implementation_deps    bazel.LabelListAttribute
 	System_dynamic_deps    bazel.LabelListAttribute
@@ -132,7 +133,8 @@ func CcLibraryHeadersBp2Build(ctx android.TopDownMutatorContext) {
 
 	attrs := &bazelCcLibraryHeadersAttributes{
 		Copts:                  compilerAttrs.copts,
-		Includes:            exportedIncludes,
+		Export_includes:        exportedIncludes.Includes,
+		Export_system_includes: exportedIncludes.SystemIncludes,
 		Implementation_deps:    linkerAttrs.deps,
 		Deps:                   linkerAttrs.exportedDeps,
 		System_dynamic_deps:    linkerAttrs.systemDynamicDeps,