diff --git a/bazel/properties.go b/bazel/properties.go index 1a846ba6a..ed4e9fc9a 100644 --- a/bazel/properties.go +++ b/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) { diff --git a/bazel/properties_test.go b/bazel/properties_test.go index 9464245c0..85596e291 100644 --- a/bazel/properties_test.go +++ b/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) + } + } + } +} diff --git a/bp2build/cc_library_conversion_test.go b/bp2build/cc_library_conversion_test.go index c8400168c..086bafe9f 100644 --- a/bp2build/cc_library_conversion_test.go +++ b/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"], diff --git a/bp2build/cc_library_headers_conversion_test.go b/bp2build/cc_library_headers_conversion_test.go index ea2c10a97..3d85bfecd 100644 --- a/bp2build/cc_library_headers_conversion_test.go +++ b/bp2build/cc_library_headers_conversion_test.go @@ -132,11 +132,7 @@ cc_library_headers { "-I.", "-I$(BINDIR)/.", ], - implementation_deps = [ - ":lib-1", - ":lib-2", - ], - includes = [ + export_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"], )`}, }) } diff --git a/bp2build/cc_library_static_conversion_test.go b/bp2build/cc_library_static_conversion_test.go index d9145f668..f0225b168 100644 --- a/bp2build/cc_library_static_conversion_test.go +++ b/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, )`}, }) diff --git a/cc/bp2build.go b/cc/bp2build.go index 537f01c87..fffb0933a 100644 --- a/cc/bp2build.go +++ b/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 { - // 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 92d9771dd..28bd741c5 100644 --- a/cc/library.go +++ b/cc/library.go @@ -228,16 +228,17 @@ type bazelCcLibraryAttributes struct { 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 @@ func CcLibraryBp2Build(ctx android.TopDownMutatorContext) { 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 @@ func maybeInjectBoringSSLHash(ctx android.ModuleContext, outputFile android.Modu } 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 @@ func ccLibraryStaticBp2BuildInternal(ctx android.TopDownMutatorContext, module * 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 44a7a714f..30a81cc39 100644 --- a/cc/library_headers.go +++ b/cc/library_headers.go @@ -103,12 +103,13 @@ func prebuiltLibraryHeaderFactory() android.Module { } 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 @@ func CcLibraryHeadersBp2Build(ctx android.TopDownMutatorContext) { 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{