bp2build: Split export_{includes,system_includes}
The specification of exporting includes vs system includes has an impact
on inclusion sort order. Conflating the two caused some symbols to not
be resolved correctly.
Bug: 198403271
Test: build/bazel/ci/bp2build.sh
Test: USE_BAZEL_ANALYSIS=1 m libbacktrace_no_dex succeeds with libc++_*
modules removed from mixed build denylist (would fail otherwise)
Change-Id: I08aff253d8962dc678ed10214b1c171330e0fe19
diff --git a/bazel/properties.go b/bazel/properties.go
index 1a846ba..ed4e9fc 100644
--- a/bazel/properties.go
+++ b/bazel/properties.go
@@ -743,6 +743,31 @@
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) {
diff --git a/bazel/properties_test.go b/bazel/properties_test.go
index 9464245..85596e2 100644
--- a/bazel/properties_test.go
+++ b/bazel/properties_test.go
@@ -293,3 +293,74 @@
}
}
}
+
+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)
+ }
+ }
+ }
+}
diff --git a/bp2build/cc_library_conversion_test.go b/bp2build/cc_library_conversion_test.go
index c840016..086bafe 100644
--- a/bp2build/cc_library_conversion_test.go
+++ b/bp2build/cc_library_conversion_test.go
@@ -122,8 +122,8 @@
"-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"],
diff --git a/bp2build/cc_library_headers_conversion_test.go b/bp2build/cc_library_headers_conversion_test.go
index ea2c10a..3d85bfe 100644
--- a/bp2build/cc_library_headers_conversion_test.go
+++ b/bp2build/cc_library_headers_conversion_test.go
@@ -132,11 +132,7 @@
"-I.",
"-I$(BINDIR)/.",
],
- implementation_deps = [
- ":lib-1",
- ":lib-2",
- ],
- includes = [
+ export_includes = [
"dir-1",
"dir-2",
] + select({
@@ -145,20 +141,24 @@
"//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 @@
"-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 @@
"-I.",
"-I$(BINDIR)/.",
],
- includes = ["lib-1"],
+ export_includes = ["lib-1"],
)`},
})
}
diff --git a/bp2build/cc_library_static_conversion_test.go b/bp2build/cc_library_static_conversion_test.go
index d9145f6..f0225b1 100644
--- a/bp2build/cc_library_static_conversion_test.go
+++ b/bp2build/cc_library_static_conversion_test.go
@@ -192,16 +192,16 @@
"-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 @@
"-I.",
"-I$(BINDIR)/.",
],
- includes = ["subpackage"],
+ export_includes = ["subpackage"],
linkstatic = True,
)`},
})
@@ -341,7 +341,7 @@
"-I.",
"-I$(BINDIR)/.",
],
- includes = ["subpackage"],
+ export_system_includes = ["subpackage"],
linkstatic = True,
)`},
})
@@ -391,7 +391,7 @@
"-Isubpackage",
"-I$(BINDIR)/subpackage",
],
- includes = ["./exported_subsubpackage"],
+ export_includes = ["./exported_subsubpackage"],
linkstatic = True,
)`},
})
diff --git a/cc/bp2build.go b/cc/bp2build.go
index 537f01c..fffb093 100644
--- a/cc/bp2build.go
+++ b/cc/bp2build.go
@@ -535,12 +535,21 @@
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 @@
// 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 {
- // Export_system_include_dirs and export_include_dirs are already module dir
- // 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
- }
-
+func bp2BuildParseExportedIncludesHelper(ctx android.TopDownMutatorContext, module *Module, libraryDecorator *libraryDecorator) BazelIncludes {
+ exported := BazelIncludes{}
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(archVariantIncludeDirs) > 0 {
- includeDirsAttribute.SetSelectValue(axis, config, archVariantIncludeDirs)
+ if len(flagExporterProperties.Export_include_dirs) > 0 {
+ exported.Includes.SetSelectValue(axis, config, flagExporterProperties.Export_include_dirs)
+ }
+ 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
}
diff --git a/cc/library.go b/cc/library.go
index 92d9771..28bd741 100644
--- a/cc/library.go
+++ b/cc/library.go
@@ -228,16 +228,17 @@
Conlyflags bazel.StringListAttribute
Asflags bazel.StringListAttribute
- Hdrs bazel.LabelListAttribute
- Deps bazel.LabelListAttribute
- Implementation_deps bazel.LabelListAttribute
- Dynamic_deps bazel.LabelListAttribute
- Whole_archive_deps bazel.LabelListAttribute
- System_dynamic_deps bazel.LabelListAttribute
- Includes bazel.StringListAttribute
- Linkopts bazel.StringListAttribute
- Use_libcrt bazel.BoolAttribute
- Rtti bazel.BoolAttribute
+ Hdrs bazel.LabelListAttribute
+ Deps bazel.LabelListAttribute
+ Implementation_deps bazel.LabelListAttribute
+ Dynamic_deps bazel.LabelListAttribute
+ Whole_archive_deps bazel.LabelListAttribute
+ System_dynamic_deps bazel.LabelListAttribute
+ Export_includes bazel.StringListAttribute
+ Export_system_includes bazel.StringListAttribute
+ Linkopts bazel.StringListAttribute
+ Use_libcrt bazel.BoolAttribute
+ Rtti bazel.BoolAttribute
// This is shared only.
Version_script bazel.LabelAttribute
@@ -299,15 +300,16 @@
Conlyflags: compilerAttrs.conlyFlags,
Asflags: asFlags,
- Implementation_deps: linkerAttrs.deps,
- Deps: linkerAttrs.exportedDeps,
- Dynamic_deps: linkerAttrs.dynamicDeps,
- Whole_archive_deps: linkerAttrs.wholeArchiveDeps,
- System_dynamic_deps: linkerAttrs.systemDynamicDeps,
- Includes: exportedIncludes,
- Linkopts: linkerAttrs.linkopts,
- Use_libcrt: linkerAttrs.useLibcrt,
- Rtti: compilerAttrs.rtti,
+ Implementation_deps: linkerAttrs.deps,
+ Deps: linkerAttrs.exportedDeps,
+ Dynamic_deps: linkerAttrs.dynamicDeps,
+ Whole_archive_deps: linkerAttrs.wholeArchiveDeps,
+ System_dynamic_deps: linkerAttrs.systemDynamicDeps,
+ Export_includes: exportedIncludes.Includes,
+ Export_system_includes: exportedIncludes.SystemIncludes,
+ Linkopts: linkerAttrs.linkopts,
+ Use_libcrt: linkerAttrs.useLibcrt,
+ Rtti: compilerAttrs.rtti,
Version_script: linkerAttrs.versionScript,
@@ -2318,19 +2320,20 @@
}
type bazelCcLibraryStaticAttributes struct {
- Copts bazel.StringListAttribute
- Srcs bazel.LabelListAttribute
- Implementation_deps bazel.LabelListAttribute
- Deps bazel.LabelListAttribute
- Whole_archive_deps bazel.LabelListAttribute
- Dynamic_deps bazel.LabelListAttribute
- System_dynamic_deps bazel.LabelListAttribute
- Linkopts bazel.StringListAttribute
- Linkstatic bool
- Use_libcrt bazel.BoolAttribute
- Rtti bazel.BoolAttribute
- Includes bazel.StringListAttribute
- Hdrs bazel.LabelListAttribute
+ Copts bazel.StringListAttribute
+ Srcs bazel.LabelListAttribute
+ Implementation_deps bazel.LabelListAttribute
+ Deps bazel.LabelListAttribute
+ Whole_archive_deps bazel.LabelListAttribute
+ Dynamic_deps bazel.LabelListAttribute
+ System_dynamic_deps bazel.LabelListAttribute
+ Linkopts bazel.StringListAttribute
+ Linkstatic bool
+ Use_libcrt bazel.BoolAttribute
+ Rtti bazel.BoolAttribute
+ Export_includes bazel.StringListAttribute
+ Export_system_includes bazel.StringListAttribute
+ Hdrs bazel.LabelListAttribute
Cppflags bazel.StringListAttribute
Srcs_c bazel.LabelListAttribute
@@ -2375,11 +2378,12 @@
Dynamic_deps: linkerAttrs.dynamicDeps,
System_dynamic_deps: linkerAttrs.systemDynamicDeps,
- Linkopts: linkerAttrs.linkopts,
- Linkstatic: true,
- Use_libcrt: linkerAttrs.useLibcrt,
- Rtti: compilerAttrs.rtti,
- Includes: exportedIncludes,
+ Linkopts: linkerAttrs.linkopts,
+ Linkstatic: true,
+ Use_libcrt: linkerAttrs.useLibcrt,
+ Rtti: compilerAttrs.rtti,
+ Export_includes: exportedIncludes.Includes,
+ Export_system_includes: exportedIncludes.SystemIncludes,
Cppflags: compilerAttrs.cppFlags,
Srcs_c: compilerAttrs.cSrcs,
diff --git a/cc/library_headers.go b/cc/library_headers.go
index 44a7a71..30a81cc 100644
--- a/cc/library_headers.go
+++ b/cc/library_headers.go
@@ -103,12 +103,13 @@
}
type bazelCcLibraryHeadersAttributes struct {
- Copts bazel.StringListAttribute
- Hdrs bazel.LabelListAttribute
- Includes bazel.StringListAttribute
- Deps bazel.LabelListAttribute
- Implementation_deps bazel.LabelListAttribute
- System_dynamic_deps bazel.LabelListAttribute
+ Copts bazel.StringListAttribute
+ Hdrs bazel.LabelListAttribute
+ Export_includes bazel.StringListAttribute
+ Export_system_includes bazel.StringListAttribute
+ Deps bazel.LabelListAttribute
+ Implementation_deps bazel.LabelListAttribute
+ System_dynamic_deps bazel.LabelListAttribute
}
func CcLibraryHeadersBp2Build(ctx android.TopDownMutatorContext) {
@@ -131,11 +132,12 @@
linkerAttrs := bp2BuildParseLinkerProps(ctx, module)
attrs := &bazelCcLibraryHeadersAttributes{
- Copts: compilerAttrs.copts,
- Includes: exportedIncludes,
- Implementation_deps: linkerAttrs.deps,
- Deps: linkerAttrs.exportedDeps,
- System_dynamic_deps: linkerAttrs.systemDynamicDeps,
+ Copts: compilerAttrs.copts,
+ Export_includes: exportedIncludes.Includes,
+ Export_system_includes: exportedIncludes.SystemIncludes,
+ Implementation_deps: linkerAttrs.deps,
+ Deps: linkerAttrs.exportedDeps,
+ System_dynamic_deps: linkerAttrs.systemDynamicDeps,
}
props := bazel.BazelTargetModuleProperties{