From 882bcc1c1cf4af1c3712c7d9c4fea315a8a8d08d Mon Sep 17 00:00:00 2001 From: Jingwen Chen Date: Tue, 27 Apr 2021 05:54:20 +0000 Subject: [PATCH] bp2build: remove header globs in generated srcs. Not needed anymore for bp2build-incremental since https://android-review.googlesource.com/q/topic:no-include-check. Not needed for mixed builds either, since cc compile actions aren't sandboxed. Fixes: 186488830 Test: treehugger and go tests Change-Id: Ib5d4908dcce6bf910a653c457bb251d726e717d4 --- bazel/properties.go | 37 ----- bp2build/cc_library_conversion_test.go | 23 +--- .../cc_library_headers_conversion_test.go | 19 --- bp2build/cc_library_static_conversion_test.go | 127 +----------------- bp2build/cc_object_conversion_test.go | 6 +- cc/bp2build.go | 55 +------- cc/library.go | 6 +- cc/library_headers.go | 3 +- 8 files changed, 14 insertions(+), 262 deletions(-) diff --git a/bazel/properties.go b/bazel/properties.go index 48d958972..a03b0270f 100644 --- a/bazel/properties.go +++ b/bazel/properties.go @@ -64,43 +64,6 @@ type LabelList struct { Excludes []Label } -// GlobsInDir returns a list of glob expressions for a list of extensions -// (optionally recursive) within a directory. -func GlobsInDir(dir string, recursive bool, extensions []string) []string { - globs := []string{} - - globInfix := "" - if dir == "." { - if recursive { - // e.g "**/*.h" - globInfix = "**/" - } // else e.g. "*.h" - for _, ext := range extensions { - globs = append(globs, globInfix+"*"+ext) - } - } else { - if recursive { - // e.g. "foo/bar/**/*.h" - dir += "/**" - } // else e.g. "foo/bar/*.h" - for _, ext := range extensions { - globs = append(globs, dir+"/*"+ext) - } - } - return globs -} - -// LooseHdrsGlobs returns the list of non-recursive header globs for each parent directory of -// each source file in this LabelList's Includes. -func (ll *LabelList) LooseHdrsGlobs(exts []string) []string { - var globs []string - for _, parentDir := range ll.uniqueParentDirectories() { - globs = append(globs, - GlobsInDir(parentDir, false, exts)...) - } - return globs -} - // uniqueParentDirectories returns a list of the unique parent directories for // all files in ll.Includes. func (ll *LabelList) uniqueParentDirectories() []string { diff --git a/bp2build/cc_library_conversion_test.go b/bp2build/cc_library_conversion_test.go index 4b6e888de..6762416a1 100644 --- a/bp2build/cc_library_conversion_test.go +++ b/bp2build/cc_library_conversion_test.go @@ -114,26 +114,13 @@ cc_library { "-I.", ], deps = [":some-headers"], - hdrs = ["foo-dir/a.h"], 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"], "//conditions:default": [], }), - srcs = [ - "impl.cpp", - "header.h", - "foo-dir/a.h", - "header.hh", - "header.hpp", - "header.hxx", - "header.h++", - "header.inl", - "header.inc", - "header.ipp", - "header.h.generic", - ] + select({ + srcs = ["impl.cpp"] + select({ "//build/bazel/platforms/arch:x86": ["x86.cpp"], "//build/bazel/platforms/arch:x86_64": ["x86_64.cpp"], "//conditions:default": [], @@ -209,13 +196,7 @@ cc_library { "//build/bazel/platforms/arch:x86_64": ["-Wl,--exclude-libs=libgcc_eh.a"], "//conditions:default": [], }), - srcs = [ - "ld_android.cpp", - "linked_list.h", - "linker.h", - "linker_block_allocator.h", - "linker_cfi.h", - ], + srcs = ["ld_android.cpp"], )`}, }, } diff --git a/bp2build/cc_library_headers_conversion_test.go b/bp2build/cc_library_headers_conversion_test.go index 00042abcb..0905aba1f 100644 --- a/bp2build/cc_library_headers_conversion_test.go +++ b/bp2build/cc_library_headers_conversion_test.go @@ -136,17 +136,6 @@ cc_library_headers { ":lib-1", ":lib-2", ], - hdrs = [ - "dir-1/dir1a.h", - "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", @@ -159,18 +148,10 @@ cc_library_headers { )`, `cc_library_headers( name = "lib-1", copts = ["-I."], - hdrs = [ - "lib-1/lib1a.h", - "lib-1/lib1b.h", - ], includes = ["lib-1"], )`, `cc_library_headers( name = "lib-2", copts = ["-I."], - hdrs = [ - "lib-2/lib2a.h", - "lib-2/lib2b.h", - ], includes = ["lib-2"], )`}, }, diff --git a/bp2build/cc_library_static_conversion_test.go b/bp2build/cc_library_static_conversion_test.go index 00325fb61..207a080e8 100644 --- a/bp2build/cc_library_static_conversion_test.go +++ b/bp2build/cc_library_static_conversion_test.go @@ -192,12 +192,6 @@ cc_library_static { ":whole_static_lib_1", ":whole_static_lib_2", ], - hdrs = [ - "export_include_dir_1/export_include_dir_1_a.h", - "export_include_dir_1/export_include_dir_1_b.h", - "export_include_dir_2/export_include_dir_2_a.h", - "export_include_dir_2/export_include_dir_2_b.h", - ], includes = [ "export_include_dir_1", "export_include_dir_2", @@ -206,105 +200,27 @@ cc_library_static { srcs = [ "foo_static1.cc", "foo_static2.cc", - "implicit_include_1.h", - "implicit_include_2.h", - "export_include_dir_1/export_include_dir_1_a.h", - "export_include_dir_1/export_include_dir_1_b.h", - "export_include_dir_2/export_include_dir_2_a.h", - "export_include_dir_2/export_include_dir_2_b.h", - "include_dir_1/include_dir_1_a.h", - "include_dir_1/include_dir_1_b.h", - "include_dir_2/include_dir_2_a.h", - "include_dir_2/include_dir_2_b.h", - "local_include_dir_1/local_include_dir_1_a.h", - "local_include_dir_1/local_include_dir_1_b.h", - "local_include_dir_2/local_include_dir_2_a.h", - "local_include_dir_2/local_include_dir_2_b.h", ], )`, `cc_library_static( name = "static_lib_1", copts = ["-I."], linkstatic = True, - srcs = [ - "static_lib_1.cc", - "implicit_include_1.h", - "implicit_include_2.h", - "export_include_dir_1/export_include_dir_1_a.h", - "export_include_dir_1/export_include_dir_1_b.h", - "export_include_dir_2/export_include_dir_2_a.h", - "export_include_dir_2/export_include_dir_2_b.h", - "include_dir_1/include_dir_1_a.h", - "include_dir_1/include_dir_1_b.h", - "include_dir_2/include_dir_2_a.h", - "include_dir_2/include_dir_2_b.h", - "local_include_dir_1/local_include_dir_1_a.h", - "local_include_dir_1/local_include_dir_1_b.h", - "local_include_dir_2/local_include_dir_2_a.h", - "local_include_dir_2/local_include_dir_2_b.h", - ], + srcs = ["static_lib_1.cc"], )`, `cc_library_static( name = "static_lib_2", copts = ["-I."], linkstatic = True, - srcs = [ - "static_lib_2.cc", - "implicit_include_1.h", - "implicit_include_2.h", - "export_include_dir_1/export_include_dir_1_a.h", - "export_include_dir_1/export_include_dir_1_b.h", - "export_include_dir_2/export_include_dir_2_a.h", - "export_include_dir_2/export_include_dir_2_b.h", - "include_dir_1/include_dir_1_a.h", - "include_dir_1/include_dir_1_b.h", - "include_dir_2/include_dir_2_a.h", - "include_dir_2/include_dir_2_b.h", - "local_include_dir_1/local_include_dir_1_a.h", - "local_include_dir_1/local_include_dir_1_b.h", - "local_include_dir_2/local_include_dir_2_a.h", - "local_include_dir_2/local_include_dir_2_b.h", - ], + srcs = ["static_lib_2.cc"], )`, `cc_library_static( name = "whole_static_lib_1", copts = ["-I."], linkstatic = True, - srcs = [ - "whole_static_lib_1.cc", - "implicit_include_1.h", - "implicit_include_2.h", - "export_include_dir_1/export_include_dir_1_a.h", - "export_include_dir_1/export_include_dir_1_b.h", - "export_include_dir_2/export_include_dir_2_a.h", - "export_include_dir_2/export_include_dir_2_b.h", - "include_dir_1/include_dir_1_a.h", - "include_dir_1/include_dir_1_b.h", - "include_dir_2/include_dir_2_a.h", - "include_dir_2/include_dir_2_b.h", - "local_include_dir_1/local_include_dir_1_a.h", - "local_include_dir_1/local_include_dir_1_b.h", - "local_include_dir_2/local_include_dir_2_a.h", - "local_include_dir_2/local_include_dir_2_b.h", - ], + srcs = ["whole_static_lib_1.cc"], )`, `cc_library_static( name = "whole_static_lib_2", copts = ["-I."], linkstatic = True, - srcs = [ - "whole_static_lib_2.cc", - "implicit_include_1.h", - "implicit_include_2.h", - "export_include_dir_1/export_include_dir_1_a.h", - "export_include_dir_1/export_include_dir_1_b.h", - "export_include_dir_2/export_include_dir_2_a.h", - "export_include_dir_2/export_include_dir_2_b.h", - "include_dir_1/include_dir_1_a.h", - "include_dir_1/include_dir_1_b.h", - "include_dir_2/include_dir_2_a.h", - "include_dir_2/include_dir_2_b.h", - "local_include_dir_1/local_include_dir_1_a.h", - "local_include_dir_1/local_include_dir_1_b.h", - "local_include_dir_2/local_include_dir_2_a.h", - "local_include_dir_2/local_include_dir_2_b.h", - ], + srcs = ["whole_static_lib_2.cc"], )`}, }, { @@ -342,14 +258,6 @@ cc_library_static { "-I.", ], linkstatic = True, - srcs = [ - "//subpackage:subpackage_header.h", - "//subpackage:subdirectory/subdirectory_header.h", - "//subpackage/subsubpackage:subsubpackage_header.h", - "//subpackage/subsubpackage:subdirectory/subdirectory_header.h", - "//subpackage/subsubpackage/subsubsubpackage:subsubsubpackage_header.h", - "//subpackage/subsubpackage/subsubsubpackage:subdirectory/subdirectory_header.h", - ], )`}, }, { @@ -371,16 +279,8 @@ cc_library_static { expectedBazelTargets: []string{`cc_library_static( name = "foo_static", copts = ["-I."], - hdrs = [ - "//subpackage:subdirectory/subdirectory_header.h", - "//subpackage:subpackage_header.h", - ], includes = ["subpackage"], linkstatic = True, - srcs = [ - "//subpackage:subpackage_header.h", - "//subpackage:subdirectory/subdirectory_header.h", - ], )`}, }, { @@ -402,16 +302,8 @@ cc_library_static { expectedBazelTargets: []string{`cc_library_static( name = "foo_static", copts = ["-I."], - hdrs = [ - "//subpackage:subdirectory/subdirectory_header.h", - "//subpackage:subpackage_header.h", - ], includes = ["subpackage"], linkstatic = True, - srcs = [ - "//subpackage:subpackage_header.h", - "//subpackage:subdirectory/subdirectory_header.h", - ], )`}, }, { @@ -452,14 +344,8 @@ cc_library_static { "-Isubpackage/subsubpackage2", "-Isubpackage", ], - hdrs = ["exported_subsubpackage/header.h"], includes = ["./exported_subsubpackage"], linkstatic = True, - srcs = [ - "exported_subsubpackage/header.h", - "subsubpackage/header.h", - "subsubpackage2/header.h", - ], )`}, }, { @@ -517,11 +403,6 @@ cc_library_static { "-I.", ], linkstatic = True, - srcs = [ - "//subpackage:subpackage_header.h", - "//subpackage:subdirectory/subdirectory_header.h", - "//subpackage2:subpackage2_header.h", - ], )`}, }, { diff --git a/bp2build/cc_object_conversion_test.go b/bp2build/cc_object_conversion_test.go index d00a1cb9c..9efdb53de 100644 --- a/bp2build/cc_object_conversion_test.go +++ b/bp2build/cc_object_conversion_test.go @@ -67,11 +67,7 @@ func TestCcObjectBp2Build(t *testing.T) { "-Iinclude", "-I.", ], - srcs = [ - "a/b/c.c", - "a/b/bar.h", - "a/b/foo.h", - ], + srcs = ["a/b/c.c"], )`, }, }, diff --git a/cc/bp2build.go b/cc/bp2build.go index efa275222..b11602dd4 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -80,7 +80,7 @@ type compilerAttributes struct { // bp2BuildParseCompilerProps returns copts, srcs and hdrs and other attributes. func bp2BuildParseCompilerProps(ctx android.TopDownMutatorContext, module *Module) compilerAttributes { - var localHdrs, srcs bazel.LabelListAttribute + var srcs bazel.LabelListAttribute var copts bazel.StringListAttribute // Creates the -I flag for a directory, while making the directory relative @@ -121,10 +121,8 @@ func bp2BuildParseCompilerProps(ctx android.TopDownMutatorContext, module *Modul if c, ok := module.compiler.(*baseCompiler); ok && c.includeBuildDirectory() { copts.Value = append(copts.Value, includeFlag(".")) - localHdrs.Value = bp2BuildListHeadersInDir(ctx, ".") } else if c, ok := module.compiler.(*libraryDecorator); ok && c.includeBuildDirectory() { copts.Value = append(copts.Value, includeFlag(".")) - localHdrs.Value = bp2BuildListHeadersInDir(ctx, ".") } for arch, props := range module.GetArchProperties(&BaseCompilerProperties{}) { @@ -143,9 +141,6 @@ func bp2BuildParseCompilerProps(ctx android.TopDownMutatorContext, module *Modul } } - // Combine local, non-exported hdrs into srcs - srcs.Append(localHdrs) - return compilerAttributes{ srcs: srcs, copts: copts, @@ -207,11 +202,6 @@ func bp2BuildParseLinkerProps(ctx android.TopDownMutatorContext, module *Module) } } -func bp2BuildListHeadersInDir(ctx android.TopDownMutatorContext, includeDir string) bazel.LabelList { - globs := bazel.GlobsInDir(includeDir, true, headerExts) - return android.BazelLabelForModuleSrc(ctx, globs) -} - // Relativize a list of root-relative paths with respect to the module's // directory. // @@ -236,9 +226,8 @@ func bp2BuildMakePathsRelativeToModule(ctx android.BazelConversionPathContext, p } // bp2BuildParseExportedIncludes creates a string list attribute contains the -// exported included directories of a module, and a label list attribute -// containing the exported headers of a module. -func bp2BuildParseExportedIncludes(ctx android.TopDownMutatorContext, module *Module) (bazel.StringListAttribute, bazel.LabelListAttribute) { +// exported included directories of a module. +func bp2BuildParseExportedIncludes(ctx android.TopDownMutatorContext, module *Module) bazel.StringListAttribute { libraryDecorator := module.linker.(*libraryDecorator) // Export_system_include_dirs and export_include_dirs are already module dir @@ -248,14 +237,6 @@ func bp2BuildParseExportedIncludes(ctx android.TopDownMutatorContext, module *Mo includeDirs = append(includeDirs, libraryDecorator.flagExporter.Properties.Export_include_dirs...) includeDirsAttribute := bazel.MakeStringListAttribute(includeDirs) - var headersAttribute bazel.LabelListAttribute - var headers bazel.LabelList - for _, includeDir := range includeDirs { - 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 @@ -268,20 +249,6 @@ func bp2BuildParseExportedIncludes(ctx android.TopDownMutatorContext, module *Mo 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 - // FIXME: This doesn't take conflicts between arch and os includes into account - archHeaders = bazel.SubtractBazelLabelList(archHeaders, headers) - - if len(archHeaders.Includes) > 0 || len(archHeaders.Excludes) > 0 { - headersAttribute.SetValueForArch(arch.Name, archHeaders) - } } } @@ -297,22 +264,8 @@ func bp2BuildParseExportedIncludes(ctx android.TopDownMutatorContext, module *Mo if len(osIncludeDirs) > 0 { includeDirsAttribute.SetValueForOS(os.Name, osIncludeDirs) } - - var osHeaders bazel.LabelList - for _, osIncludeDir := range osIncludeDirs { - osHeaders.Append(bp2BuildListHeadersInDir(ctx, osIncludeDir)) - } - osHeaders = bazel.UniqueBazelLabelList(osHeaders) - - // To avoid duplicate headers when base headers + os headers are combined - // FIXME: This doesn't take conflicts between arch and os includes into account - osHeaders = bazel.SubtractBazelLabelList(osHeaders, headers) - - if len(osHeaders.Includes) > 0 || len(osHeaders.Excludes) > 0 { - headersAttribute.SetValueForOS(os.Name, osHeaders) - } } } - return includeDirsAttribute, headersAttribute + return includeDirsAttribute } diff --git a/cc/library.go b/cc/library.go index af92b24b7..f49698e7a 100644 --- a/cc/library.go +++ b/cc/library.go @@ -259,11 +259,10 @@ func CcLibraryBp2Build(ctx android.TopDownMutatorContext) { compilerAttrs := bp2BuildParseCompilerProps(ctx, m) linkerAttrs := bp2BuildParseLinkerProps(ctx, m) - exportedIncludes, exportedIncludesHeaders := bp2BuildParseExportedIncludes(ctx, m) + exportedIncludes := bp2BuildParseExportedIncludes(ctx, m) attrs := &bazelCcLibraryAttributes{ Srcs: compilerAttrs.srcs, - Hdrs: exportedIncludesHeaders, Copts: compilerAttrs.copts, Linkopts: linkerAttrs.linkopts, Deps: linkerAttrs.deps, @@ -2176,7 +2175,7 @@ func CcLibraryStaticBp2Build(ctx android.TopDownMutatorContext) { compilerAttrs := bp2BuildParseCompilerProps(ctx, module) linkerAttrs := bp2BuildParseLinkerProps(ctx, module) - exportedIncludes, exportedIncludesHeaders := bp2BuildParseExportedIncludes(ctx, module) + exportedIncludes := bp2BuildParseExportedIncludes(ctx, module) attrs := &bazelCcLibraryStaticAttributes{ Copts: compilerAttrs.copts, @@ -2184,7 +2183,6 @@ func CcLibraryStaticBp2Build(ctx android.TopDownMutatorContext) { Deps: linkerAttrs.deps, Linkstatic: true, Includes: exportedIncludes, - Hdrs: exportedIncludesHeaders, } props := bazel.BazelTargetModuleProperties{ diff --git a/cc/library_headers.go b/cc/library_headers.go index f2cb52baf..0aba8dea8 100644 --- a/cc/library_headers.go +++ b/cc/library_headers.go @@ -142,14 +142,13 @@ func CcLibraryHeadersBp2Build(ctx android.TopDownMutatorContext) { return } - exportedIncludes, exportedHdrs := bp2BuildParseExportedIncludes(ctx, module) + exportedIncludes := bp2BuildParseExportedIncludes(ctx, module) compilerAttrs := bp2BuildParseCompilerProps(ctx, module) linkerAttrs := bp2BuildParseLinkerProps(ctx, module) attrs := &bazelCcLibraryHeadersAttributes{ Copts: compilerAttrs.copts, Includes: exportedIncludes, - Hdrs: exportedHdrs, Deps: linkerAttrs.deps, }