From b815168474b9a906997690b37ab9b3c203613155 Mon Sep 17 00:00:00 2001 From: Rupert Shuttleworth Date: Tue, 6 Apr 2021 20:06:21 +0000 Subject: [PATCH] Support arch variations for export_system_include_dirs in cc_library_headers bp2build converter. Test: Added unit test Test: bp2build-sync.py write; bazel build //bionic/... works for more cc_library_static targets (in a parent CL) Change-Id: Ib487216a4bcbc52958ff948722dae347b0d8b606 --- bazel/properties.go | 63 +++++++++++++++ bazel/properties_test.go | 76 +++++++++++++++++++ .../cc_library_headers_conversion_test.go | 58 +++++++++++--- cc/bp2build.go | 74 +++++++++++++++--- cc/library.go | 21 +++-- cc/library_headers.go | 12 +-- 6 files changed, 268 insertions(+), 36 deletions(-) diff --git a/bazel/properties.go b/bazel/properties.go index 2440ca13f..148386f6a 100644 --- a/bazel/properties.go +++ b/bazel/properties.go @@ -79,6 +79,63 @@ func UniqueBazelLabelList(originalLabelList LabelList) LabelList { return uniqueLabelList } +// Subtract needle from haystack +func SubtractStrings(haystack []string, needle []string) []string { + // This is really a set + remainder := make(map[string]bool) + + for _, s := range haystack { + remainder[s] = true + } + for _, s := range needle { + delete(remainder, s) + } + + var strings []string + for s, _ := range remainder { + strings = append(strings, s) + } + + sort.SliceStable(strings, func(i, j int) bool { + return strings[i] < strings[j] + }) + + return strings +} + +// Subtract needle from haystack +func SubtractBazelLabels(haystack []Label, needle []Label) []Label { + // This is really a set + remainder := make(map[Label]bool) + + for _, label := range haystack { + remainder[label] = true + } + for _, label := range needle { + delete(remainder, label) + } + + var labels []Label + for label, _ := range remainder { + labels = append(labels, label) + } + + sort.SliceStable(labels, func(i, j int) bool { + return labels[i].Label < labels[j].Label + }) + + return labels +} + +// Subtract needle from haystack +func SubtractBazelLabelList(haystack LabelList, needle LabelList) LabelList { + var result LabelList + result.Includes = SubtractBazelLabels(haystack.Includes, needle.Includes) + // NOTE: Excludes are intentionally not subtracted + result.Excludes = haystack.Excludes + return result +} + const ( // ArchType names in arch.go ARCH_ARM = "arm" @@ -257,6 +314,12 @@ type StringListAttribute struct { OsValues stringListOsValues } +// MakeStringListAttribute initializes a StringListAttribute with the non-arch specific value. +func MakeStringListAttribute(value []string) StringListAttribute { + // NOTE: These strings are not necessarily unique or sorted. + return StringListAttribute{Value: value} +} + // Arch-specific string_list typed Bazel attribute values. This should correspond // to the types of architectures supported for compilation in arch.go. type stringListArchValues struct { diff --git a/bazel/properties_test.go b/bazel/properties_test.go index 0fcb90427..56840efbe 100644 --- a/bazel/properties_test.go +++ b/bazel/properties_test.go @@ -46,6 +46,82 @@ func TestUniqueBazelLabels(t *testing.T) { } } +func TestSubtractStrings(t *testing.T) { + testCases := []struct { + haystack []string + needle []string + expectedResult []string + }{ + { + haystack: []string{ + "a", + "b", + "c", + }, + needle: []string{ + "a", + }, + expectedResult: []string{ + "b", "c", + }, + }, + } + for _, tc := range testCases { + actualResult := SubtractStrings(tc.haystack, tc.needle) + if !reflect.DeepEqual(tc.expectedResult, actualResult) { + t.Fatalf("Expected %v, got %v", tc.expectedResult, actualResult) + } + } +} + +func TestSubtractBazelLabelList(t *testing.T) { + testCases := []struct { + haystack LabelList + needle LabelList + expectedResult LabelList + }{ + { + haystack: LabelList{ + Includes: []Label{ + {Label: "a"}, + {Label: "b"}, + {Label: "c"}, + }, + Excludes: []Label{ + {Label: "x"}, + {Label: "y"}, + {Label: "z"}, + }, + }, + needle: LabelList{ + Includes: []Label{ + {Label: "a"}, + }, + Excludes: []Label{ + {Label: "z"}, + }, + }, + // NOTE: Excludes are intentionally not subtracted + expectedResult: LabelList{ + Includes: []Label{ + {Label: "b"}, + {Label: "c"}, + }, + Excludes: []Label{ + {Label: "x"}, + {Label: "y"}, + {Label: "z"}, + }, + }, + }, + } + for _, tc := range testCases { + actualResult := SubtractBazelLabelList(tc.haystack, tc.needle) + if !reflect.DeepEqual(tc.expectedResult, actualResult) { + t.Fatalf("Expected %v, got %v", tc.expectedResult, actualResult) + } + } +} func TestUniqueBazelLabelList(t *testing.T) { testCases := []struct { originalLabelList LabelList diff --git a/bp2build/cc_library_headers_conversion_test.go b/bp2build/cc_library_headers_conversion_test.go index d828168ac..9d91ffa1c 100644 --- a/bp2build/cc_library_headers_conversion_test.go +++ b/bp2build/cc_library_headers_conversion_test.go @@ -97,14 +97,17 @@ func TestCcLibraryHeadersBp2Build(t *testing.T) { moduleTypeUnderTestFactory: cc.LibraryHeaderFactory, moduleTypeUnderTestBp2BuildMutator: cc.CcLibraryHeadersBp2Build, filesystem: map[string]string{ - "lib-1/lib1a.h": "", - "lib-1/lib1b.h": "", - "lib-2/lib2a.h": "", - "lib-2/lib2b.h": "", - "dir-1/dir1a.h": "", - "dir-1/dir1b.h": "", - "dir-2/dir2a.h": "", - "dir-2/dir2b.h": "", + "lib-1/lib1a.h": "", + "lib-1/lib1b.h": "", + "lib-2/lib2a.h": "", + "lib-2/lib2b.h": "", + "dir-1/dir1a.h": "", + "dir-1/dir1b.h": "", + "dir-2/dir2a.h": "", + "dir-2/dir2b.h": "", + "arch_arm64_exported_include_dir/a.h": "", + "arch_x86_exported_include_dir/b.h": "", + "arch_x86_64_exported_include_dir/c.h": "", }, bp: soongCcLibraryPreamble + ` cc_library_headers { @@ -122,6 +125,19 @@ cc_library_headers { export_include_dirs: ["dir-1", "dir-2"], header_libs: ["lib-1", "lib-2"], + arch: { + arm64: { + // We expect dir-1 headers to be dropped, because dir-1 is already in export_include_dirs + export_include_dirs: ["arch_arm64_exported_include_dir", "dir-1"], + }, + x86: { + export_include_dirs: ["arch_x86_exported_include_dir"], + }, + x86_64: { + export_include_dirs: ["arch_x86_64_exported_include_dir"], + }, + }, + // TODO: Also support export_header_lib_headers }`, expectedBazelTargets: []string{`cc_library_headers( @@ -135,11 +151,33 @@ cc_library_headers { "dir-1/dir1b.h", "dir-2/dir2a.h", "dir-2/dir2b.h", - ], + ] + select({ + "//build/bazel/platforms/arch:arm64": [ + "arch_arm64_exported_include_dir/a.h", + ], + "//build/bazel/platforms/arch:x86": [ + "arch_x86_exported_include_dir/b.h", + ], + "//build/bazel/platforms/arch:x86_64": [ + "arch_x86_64_exported_include_dir/c.h", + ], + "//conditions:default": [], + }), includes = [ "dir-1", "dir-2", - ], + ] + select({ + "//build/bazel/platforms/arch:arm64": [ + "arch_arm64_exported_include_dir", + ], + "//build/bazel/platforms/arch:x86": [ + "arch_x86_exported_include_dir", + ], + "//build/bazel/platforms/arch:x86_64": [ + "arch_x86_64_exported_include_dir", + ], + "//conditions:default": [], + }), )`, `cc_library_headers( name = "lib-1", hdrs = [ diff --git a/cc/bp2build.go b/cc/bp2build.go index 497d227d3..cffeb2474 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -16,6 +16,7 @@ package cc import ( "android/soong/android" "android/soong/bazel" + "strings" ) // bp2build functions and helpers for converting cc_* modules to Bazel. @@ -109,23 +110,78 @@ func bp2BuildParseHeaderLibs(ctx android.TopDownMutatorContext, module *Module) return ret } +func bp2BuildListHeadersInDir(ctx android.TopDownMutatorContext, includeDir string) bazel.LabelList { + var globInfix string + + if includeDir == "." { + globInfix = "" + } else { + globInfix = "/**" + } + + var includeDirGlobs []string + includeDirGlobs = append(includeDirGlobs, includeDir+globInfix+"/*.h") + includeDirGlobs = append(includeDirGlobs, includeDir+globInfix+"/*.inc") + includeDirGlobs = append(includeDirGlobs, includeDir+globInfix+"/*.hpp") + + return android.BazelLabelForModuleSrc(ctx, includeDirGlobs) +} + +// Bazel wants include paths to be relative to the module +func bp2BuildMakePathsRelativeToModule(ctx android.TopDownMutatorContext, paths []string) []string { + var relativePaths []string + for _, path := range paths { + relativePath := strings.TrimPrefix(path, ctx.ModuleDir()+"/") + relativePaths = append(relativePaths, relativePath) + } + return relativePaths +} + // bp2BuildParseExportedIncludes creates a label list attribute contains the // exported included directories of a module. -func bp2BuildParseExportedIncludes(ctx android.TopDownMutatorContext, module *Module) (bazel.LabelListAttribute, bazel.LabelListAttribute) { +func bp2BuildParseExportedIncludes(ctx android.TopDownMutatorContext, module *Module) (bazel.StringListAttribute, bazel.LabelListAttribute) { libraryDecorator := module.linker.(*libraryDecorator) includeDirs := libraryDecorator.flagExporter.Properties.Export_system_include_dirs includeDirs = append(includeDirs, libraryDecorator.flagExporter.Properties.Export_include_dirs...) + includeDirs = bp2BuildMakePathsRelativeToModule(ctx, includeDirs) + includeDirsAttribute := bazel.MakeStringListAttribute(includeDirs) - includeDirsLabels := android.BazelLabelForModuleSrc(ctx, includeDirs) - - var includeDirGlobs []string + var headersAttribute bazel.LabelListAttribute + var headers bazel.LabelList for _, includeDir := range includeDirs { - includeDirGlobs = append(includeDirGlobs, includeDir+"/**/*.h") - includeDirGlobs = append(includeDirGlobs, includeDir+"/**/*.inc") - includeDirGlobs = append(includeDirGlobs, includeDir+"/**/*.hpp") + headers.Append(bp2BuildListHeadersInDir(ctx, includeDir)) + } + headers = bazel.UniqueBazelLabelList(headers) + headersAttribute.Value = headers + + for arch, props := range module.GetArchProperties(&FlagExporterProperties{}) { + if flagExporterProperties, ok := props.(*FlagExporterProperties); ok { + archIncludeDirs := flagExporterProperties.Export_system_include_dirs + archIncludeDirs = append(archIncludeDirs, flagExporterProperties.Export_include_dirs...) + archIncludeDirs = bp2BuildMakePathsRelativeToModule(ctx, archIncludeDirs) + + // To avoid duplicate includes when base includes + arch includes are combined + archIncludeDirs = bazel.SubtractStrings(archIncludeDirs, includeDirs) + + if len(archIncludeDirs) > 0 { + includeDirsAttribute.SetValueForArch(arch.Name, archIncludeDirs) + } + + var archHeaders bazel.LabelList + for _, archIncludeDir := range archIncludeDirs { + archHeaders.Append(bp2BuildListHeadersInDir(ctx, archIncludeDir)) + } + archHeaders = bazel.UniqueBazelLabelList(archHeaders) + + // To avoid duplicate headers when base headers + arch headers are combined + archHeaders = bazel.SubtractBazelLabelList(archHeaders, headers) + + if len(archHeaders.Includes) > 0 || len(archHeaders.Excludes) > 0 { + headersAttribute.SetValueForArch(arch.Name, archHeaders) + } + } } - headersLabels := android.BazelLabelForModuleSrc(ctx, includeDirGlobs) - return bazel.MakeLabelListAttribute(includeDirsLabels), bazel.MakeLabelListAttribute(headersLabels) + return includeDirsAttribute, headersAttribute } diff --git a/cc/library.go b/cc/library.go index 50fff7f9d..97c7ba1b7 100644 --- a/cc/library.go +++ b/cc/library.go @@ -2051,7 +2051,7 @@ type bazelCcLibraryStaticAttributes struct { Srcs bazel.LabelListAttribute Deps bazel.LabelListAttribute Linkstatic bool - Includes bazel.LabelListAttribute + Includes bazel.StringListAttribute Hdrs bazel.LabelListAttribute } @@ -2088,8 +2088,8 @@ func CcLibraryStaticBp2Build(ctx android.TopDownMutatorContext) { if baseCompilerProps, ok := props.(*BaseCompilerProperties); ok { copts = baseCompilerProps.Cflags srcs = baseCompilerProps.Srcs - includeDirs = baseCompilerProps.Include_dirs - localIncludeDirs = baseCompilerProps.Local_include_dirs + includeDirs = bp2BuildMakePathsRelativeToModule(ctx, baseCompilerProps.Include_dirs) + localIncludeDirs = bp2BuildMakePathsRelativeToModule(ctx, baseCompilerProps.Local_include_dirs) break } } @@ -2111,14 +2111,13 @@ func CcLibraryStaticBp2Build(ctx android.TopDownMutatorContext) { depsLabels := android.BazelLabelForModuleDeps(ctx, allDeps) + exportedIncludes, exportedIncludesHeaders := bp2BuildParseExportedIncludes(ctx, module) + // FIXME: Unify absolute vs relative paths // FIXME: Use -I copts instead of setting includes= ? - allIncludes := includeDirs - allIncludes = append(allIncludes, localIncludeDirs...) - includesLabels := android.BazelLabelForModuleSrc(ctx, allIncludes) - - exportedIncludesLabels, exportedIncludesHeadersLabels := bp2BuildParseExportedIncludes(ctx, module) - includesLabels.Append(exportedIncludesLabels.Value) + allIncludes := exportedIncludes + allIncludes.Value = append(allIncludes.Value, includeDirs...) + allIncludes.Value = append(allIncludes.Value, localIncludeDirs...) headerLibsLabels := bp2BuildParseHeaderLibs(ctx, module) depsLabels.Append(headerLibsLabels.Value) @@ -2128,8 +2127,8 @@ func CcLibraryStaticBp2Build(ctx android.TopDownMutatorContext) { Srcs: srcsLabels, Deps: bazel.MakeLabelListAttribute(depsLabels), Linkstatic: true, - Includes: bazel.MakeLabelListAttribute(includesLabels), - Hdrs: exportedIncludesHeadersLabels, + Includes: allIncludes, + Hdrs: exportedIncludesHeaders, } props := bazel.BazelTargetModuleProperties{ diff --git a/cc/library_headers.go b/cc/library_headers.go index 82af16a69..d35748bcb 100644 --- a/cc/library_headers.go +++ b/cc/library_headers.go @@ -64,7 +64,7 @@ func prebuiltLibraryHeaderFactory() android.Module { type bazelCcLibraryHeadersAttributes struct { Copts bazel.StringListAttribute Hdrs bazel.LabelListAttribute - Includes bazel.LabelListAttribute + Includes bazel.StringListAttribute Deps bazel.LabelListAttribute } @@ -95,15 +95,15 @@ func CcLibraryHeadersBp2Build(ctx android.TopDownMutatorContext) { return } - exportedIncludesLabels, exportedIncludesHeadersLabels := bp2BuildParseExportedIncludes(ctx, module) + exportedIncludes, exportedIncludesHeaders := bp2BuildParseExportedIncludes(ctx, module) - headerLibsLabels := bp2BuildParseHeaderLibs(ctx, module) + headerLibs := bp2BuildParseHeaderLibs(ctx, module) attrs := &bazelCcLibraryHeadersAttributes{ Copts: bp2BuildParseCflags(ctx, module), - Includes: exportedIncludesLabels, - Hdrs: exportedIncludesHeadersLabels, - Deps: headerLibsLabels, + Includes: exportedIncludes, + Hdrs: exportedIncludesHeaders, + Deps: headerLibs, } props := bazel.BazelTargetModuleProperties{