From 35687bc77a6690e3ef27bb4d3fbf70bdc236cc1a Mon Sep 17 00:00:00 2001 From: Liz Kammer Date: Fri, 10 Sep 2021 10:07:07 -0400 Subject: [PATCH] Split local/absolute include into attributes Previously these were expanded into copts, requiring making all includes absolute and duplicating includes to account for potentially generated files. We now can handle both of these properly on the Bazel side, so let's clean up build files a bit. Test: bp2build.sh Change-Id: I6c6160738cd6c269408c6c7a37010654d84f3c9d --- bp2build/cc_library_conversion_test.go | 22 ++---- .../cc_library_headers_conversion_test.go | 4 -- bp2build/cc_library_static_conversion_test.go | 72 ++++++++----------- bp2build/cc_object_conversion_test.go | 11 ++- cc/bp2build.go | 63 ++++++---------- cc/library.go | 8 +++ cc/library_headers.go | 3 - cc/object.go | 26 ++++--- 8 files changed, 84 insertions(+), 125 deletions(-) diff --git a/bp2build/cc_library_conversion_test.go b/bp2build/cc_library_conversion_test.go index c2c35e7e0..371593b5e 100644 --- a/bp2build/cc_library_conversion_test.go +++ b/bp2build/cc_library_conversion_test.go @@ -255,13 +255,11 @@ cc_library { blueprint: soongCcLibraryPreamble, expectedBazelTargets: []string{`cc_library( name = "fake-libarm-optimized-routines-math", - copts = [ - "-Iexternal", - "-I$(BINDIR)/external", - ] + select({ + copts = select({ "//build/bazel/platforms/arch:arm64": ["-DHAVE_FAST_FMA=1"], "//conditions:default": [], }), + local_includes = ["."], srcs_c = ["math/cosf.c"], )`}, }) @@ -494,12 +492,9 @@ cc_library_static { name: "android_dep_for_shared" } blueprint: soongCcLibraryPreamble, expectedBazelTargets: []string{`cc_library( name = "a", - copts = [ - "bothflag", - "-Ifoo/bar", - "-I$(BINDIR)/foo/bar", - ], + copts = ["bothflag"], implementation_deps = [":static_dep_for_both"], + local_includes = ["."], shared = { "copts": ["sharedflag"] + select({ "//build/bazel/platforms/arch:arm": ["-DARM_SHARED"], @@ -635,14 +630,7 @@ filegroup { blueprint: soongCcLibraryPreamble, expectedBazelTargets: []string{`cc_library( name = "a", - asflags = [ - "-Ifoo/bar", - "-I$(BINDIR)/foo/bar", - ], - copts = [ - "-Ifoo/bar", - "-I$(BINDIR)/foo/bar", - ], + local_includes = ["."], shared = { "srcs": [ ":shared_filegroup_cpp_srcs", diff --git a/bp2build/cc_library_headers_conversion_test.go b/bp2build/cc_library_headers_conversion_test.go index 8b490f885..37d806cb1 100644 --- a/bp2build/cc_library_headers_conversion_test.go +++ b/bp2build/cc_library_headers_conversion_test.go @@ -130,10 +130,6 @@ cc_library_headers { }`, expectedBazelTargets: []string{`cc_library_headers( name = "foo_headers", - copts = [ - "-I.", - "-I$(BINDIR)/.", - ], export_includes = [ "dir-1", "dir-2", diff --git a/bp2build/cc_library_static_conversion_test.go b/bp2build/cc_library_static_conversion_test.go index 5c9afbf7f..72034faaa 100644 --- a/bp2build/cc_library_static_conversion_test.go +++ b/bp2build/cc_library_static_conversion_test.go @@ -184,19 +184,13 @@ cc_library_static { }`, expectedBazelTargets: []string{`cc_library_static( name = "foo_static", + absolute_includes = [ + "include_dir_1", + "include_dir_2", + ], copts = [ "-Dflag1", "-Dflag2", - "-Iinclude_dir_1", - "-I$(BINDIR)/include_dir_1", - "-Iinclude_dir_2", - "-I$(BINDIR)/include_dir_2", - "-Ilocal_include_dir_1", - "-I$(BINDIR)/local_include_dir_1", - "-Ilocal_include_dir_2", - "-I$(BINDIR)/local_include_dir_2", - "-I.", - "-I$(BINDIR)/.", ], export_includes = [ "export_include_dir_1", @@ -209,6 +203,11 @@ cc_library_static { ":static_lib_2", ], linkstatic = True, + local_includes = [ + "local_include_dir_1", + "local_include_dir_2", + ".", + ], srcs = [ "foo_static1.cc", "foo_static2.cc", @@ -244,21 +243,16 @@ func TestCcLibraryStaticSubpackage(t *testing.T) { blueprint: soongCcLibraryStaticPreamble + ` cc_library_static { name: "foo_static", - srcs: [ - ], + srcs: [], include_dirs: [ - "subpackage", + "subpackage", ], }`, expectedBazelTargets: []string{`cc_library_static( name = "foo_static", - copts = [ - "-Isubpackage", - "-I$(BINDIR)/subpackage", - "-I.", - "-I$(BINDIR)/.", - ], + absolute_includes = ["subpackage"], linkstatic = True, + local_includes = ["."], )`}, }) } @@ -347,20 +341,17 @@ cc_library_static { blueprint: soongCcLibraryStaticPreamble, expectedBazelTargets: []string{`cc_library_static( name = "foo_static", - copts = [ - "-Isubpackage/subsubpackage", - "-I$(BINDIR)/subpackage/subsubpackage", - "-Isubpackage2", - "-I$(BINDIR)/subpackage2", - "-Isubpackage3/subsubpackage", - "-I$(BINDIR)/subpackage3/subsubpackage", - "-Isubpackage/subsubpackage2", - "-I$(BINDIR)/subpackage/subsubpackage2", - "-Isubpackage", - "-I$(BINDIR)/subpackage", + absolute_includes = [ + "subpackage/subsubpackage", + "subpackage2", + "subpackage3/subsubpackage", ], export_includes = ["./exported_subsubpackage"], linkstatic = True, + local_includes = [ + "subsubpackage2", + ".", + ], )`}, }) } @@ -386,13 +377,9 @@ cc_library_static { }`, expectedBazelTargets: []string{`cc_library_static( name = "foo_static", - copts = [ - "-Isubpackage", - "-I$(BINDIR)/subpackage", - "-Isubpackage2", - "-I$(BINDIR)/subpackage2", - ], + absolute_includes = ["subpackage"], linkstatic = True, + local_includes = ["subpackage2"], )`}, }) } @@ -420,15 +407,12 @@ cc_library_static { }`, expectedBazelTargets: []string{`cc_library_static( name = "foo_static", - copts = [ - "-Isubpackage", - "-I$(BINDIR)/subpackage", - "-Isubpackage2", - "-I$(BINDIR)/subpackage2", - "-I.", - "-I$(BINDIR)/.", - ], + absolute_includes = ["subpackage"], linkstatic = True, + local_includes = [ + "subpackage2", + ".", + ], )`}, }) } diff --git a/bp2build/cc_object_conversion_test.go b/bp2build/cc_object_conversion_test.go index 4bda539fb..b0a88ae0d 100644 --- a/bp2build/cc_object_conversion_test.go +++ b/bp2build/cc_object_conversion_test.go @@ -65,10 +65,10 @@ func TestCcObjectSimple(t *testing.T) { "-Wno-gcc-compat", "-Wall", "-Werror", - "-Iinclude", - "-I$(BINDIR)/include", - "-I.", - "-I$(BINDIR)/.", + ], + local_includes = [ + "include", + ".", ], srcs = ["a/b/c.c"], )`, @@ -113,9 +113,8 @@ cc_defaults { "-Wall", "-Werror", "-fno-addrsig", - "-I.", - "-I$(BINDIR)/.", ], + local_includes = ["."], srcs = ["a/b/c.c"], )`, }}) diff --git a/cc/bp2build.go b/cc/bp2build.go index fffb0933a..7a98fd0e4 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -216,6 +216,9 @@ type compilerAttributes struct { srcs bazel.LabelListAttribute rtti bazel.BoolAttribute + + localIncludes bazel.StringListAttribute + absoluteIncludes bazel.StringListAttribute } // bp2BuildParseCompilerProps returns copts, srcs and hdrs and other attributes. @@ -226,28 +229,8 @@ func bp2BuildParseCompilerProps(ctx android.TopDownMutatorContext, module *Modul var conlyFlags bazel.StringListAttribute var cppFlags bazel.StringListAttribute var rtti bazel.BoolAttribute - - // Creates the -I flags for a directory, while making the directory relative - // to the exec root for Bazel to work. - includeFlags := func(dir string) []string { - // filepath.Join canonicalizes the path, i.e. it takes care of . or .. elements. - moduleDirRootedPath := filepath.Join(ctx.ModuleDir(), dir) - return []string{ - "-I" + moduleDirRootedPath, - // Include the bindir-rooted path (using make variable substitution). This most - // closely matches Bazel's native include path handling, which allows for dependency - // on generated headers in these directories. - // TODO(b/188084383): Handle local include directories in Bazel. - "-I$(BINDIR)/" + moduleDirRootedPath, - } - } - - // Parse the list of module-relative include directories (-I). - parseLocalIncludeDirs := func(baseCompilerProps *BaseCompilerProperties) []string { - // include_dirs are root-relative, not module-relative. - includeDirs := bp2BuildMakePathsRelativeToModule(ctx, baseCompilerProps.Include_dirs) - return append(includeDirs, baseCompilerProps.Local_include_dirs...) - } + var localIncludes bazel.StringListAttribute + var absoluteIncludes bazel.StringListAttribute parseCommandLineFlags := func(soongFlags []string) []string { var result []string @@ -285,18 +268,14 @@ func bp2BuildParseCompilerProps(ctx android.TopDownMutatorContext, module *Modul archVariantCopts := parseCommandLineFlags(baseCompilerProps.Cflags) archVariantAsflags := parseCommandLineFlags(baseCompilerProps.Asflags) - for _, dir := range parseLocalIncludeDirs(baseCompilerProps) { - archVariantCopts = append(archVariantCopts, includeFlags(dir)...) - archVariantAsflags = append(archVariantAsflags, includeFlags(dir)...) + + localIncludeDirs := baseCompilerProps.Local_include_dirs + if axis == bazel.NoConfigAxis && includeBuildDirectory(baseCompilerProps.Include_build_directory) { + localIncludeDirs = append(localIncludeDirs, ".") } - if axis == bazel.NoConfigAxis { - if includeBuildDirectory(baseCompilerProps.Include_build_directory) { - flags := includeFlags(".") - archVariantCopts = append(archVariantCopts, flags...) - archVariantAsflags = append(archVariantAsflags, flags...) - } - } + absoluteIncludes.SetSelectValue(axis, config, baseCompilerProps.Include_dirs) + localIncludes.SetSelectValue(axis, config, localIncludeDirs) copts.SetSelectValue(axis, config, archVariantCopts) asFlags.SetSelectValue(axis, config, archVariantAsflags) @@ -308,6 +287,8 @@ func bp2BuildParseCompilerProps(ctx android.TopDownMutatorContext, module *Modul } srcs.ResolveExcludes() + absoluteIncludes.DeduplicateAxesFromBase() + localIncludes.DeduplicateAxesFromBase() productVarPropNameToAttribute := map[string]*bazel.StringListAttribute{ "Cflags": &copts, @@ -331,14 +312,16 @@ func bp2BuildParseCompilerProps(ctx android.TopDownMutatorContext, module *Modul srcs, cSrcs, asSrcs := groupSrcsByExtension(ctx, srcs) return compilerAttributes{ - copts: copts, - srcs: srcs, - asFlags: asFlags, - asSrcs: asSrcs, - cSrcs: cSrcs, - conlyFlags: conlyFlags, - cppFlags: cppFlags, - rtti: rtti, + copts: copts, + srcs: srcs, + asFlags: asFlags, + asSrcs: asSrcs, + cSrcs: cSrcs, + conlyFlags: conlyFlags, + cppFlags: cppFlags, + rtti: rtti, + localIncludes: localIncludes, + absoluteIncludes: absoluteIncludes, } } diff --git a/cc/library.go b/cc/library.go index 28bd741c5..f56824706 100644 --- a/cc/library.go +++ b/cc/library.go @@ -236,6 +236,8 @@ type bazelCcLibraryAttributes struct { System_dynamic_deps bazel.LabelListAttribute Export_includes bazel.StringListAttribute Export_system_includes bazel.StringListAttribute + Local_includes bazel.StringListAttribute + Absolute_includes bazel.StringListAttribute Linkopts bazel.StringListAttribute Use_libcrt bazel.BoolAttribute Rtti bazel.BoolAttribute @@ -307,6 +309,8 @@ func CcLibraryBp2Build(ctx android.TopDownMutatorContext) { System_dynamic_deps: linkerAttrs.systemDynamicDeps, Export_includes: exportedIncludes.Includes, Export_system_includes: exportedIncludes.SystemIncludes, + Local_includes: compilerAttrs.localIncludes, + Absolute_includes: compilerAttrs.absoluteIncludes, Linkopts: linkerAttrs.linkopts, Use_libcrt: linkerAttrs.useLibcrt, Rtti: compilerAttrs.rtti, @@ -2333,6 +2337,8 @@ type bazelCcLibraryStaticAttributes struct { Rtti bazel.BoolAttribute Export_includes bazel.StringListAttribute Export_system_includes bazel.StringListAttribute + Local_includes bazel.StringListAttribute + Absolute_includes bazel.StringListAttribute Hdrs bazel.LabelListAttribute Cppflags bazel.StringListAttribute @@ -2384,6 +2390,8 @@ func ccLibraryStaticBp2BuildInternal(ctx android.TopDownMutatorContext, module * Rtti: compilerAttrs.rtti, Export_includes: exportedIncludes.Includes, Export_system_includes: exportedIncludes.SystemIncludes, + Local_includes: compilerAttrs.localIncludes, + Absolute_includes: compilerAttrs.absoluteIncludes, Cppflags: compilerAttrs.cppFlags, Srcs_c: compilerAttrs.cSrcs, diff --git a/cc/library_headers.go b/cc/library_headers.go index 30a81cc39..14b90c1e5 100644 --- a/cc/library_headers.go +++ b/cc/library_headers.go @@ -103,7 +103,6 @@ func prebuiltLibraryHeaderFactory() android.Module { } type bazelCcLibraryHeadersAttributes struct { - Copts bazel.StringListAttribute Hdrs bazel.LabelListAttribute Export_includes bazel.StringListAttribute Export_system_includes bazel.StringListAttribute @@ -128,11 +127,9 @@ func CcLibraryHeadersBp2Build(ctx android.TopDownMutatorContext) { } exportedIncludes := bp2BuildParseExportedIncludes(ctx, module) - compilerAttrs := bp2BuildParseCompilerProps(ctx, module) linkerAttrs := bp2BuildParseLinkerProps(ctx, module) attrs := &bazelCcLibraryHeadersAttributes{ - Copts: compilerAttrs.copts, Export_includes: exportedIncludes.Includes, Export_system_includes: exportedIncludes.SystemIncludes, Implementation_deps: linkerAttrs.deps, diff --git a/cc/object.go b/cc/object.go index 606e368b5..99b257aaa 100644 --- a/cc/object.go +++ b/cc/object.go @@ -122,12 +122,14 @@ func ObjectFactory() android.Module { // For bp2build conversion. type bazelObjectAttributes struct { - Srcs bazel.LabelListAttribute - Srcs_as bazel.LabelListAttribute - Hdrs bazel.LabelListAttribute - Deps bazel.LabelListAttribute - Copts bazel.StringListAttribute - Asflags bazel.StringListAttribute + Srcs bazel.LabelListAttribute + Srcs_as bazel.LabelListAttribute + Hdrs bazel.LabelListAttribute + Deps bazel.LabelListAttribute + Copts bazel.StringListAttribute + Asflags bazel.StringListAttribute + Local_includes bazel.StringListAttribute + Absolute_includes bazel.StringListAttribute } // ObjectBp2Build is the bp2build converter from cc_object modules to the @@ -170,11 +172,13 @@ func ObjectBp2Build(ctx android.TopDownMutatorContext) { } attrs := &bazelObjectAttributes{ - Srcs: srcs, - Srcs_as: compilerAttrs.asSrcs, - Deps: deps, - Copts: compilerAttrs.copts, - Asflags: asFlags, + Srcs: srcs, + Srcs_as: compilerAttrs.asSrcs, + Deps: deps, + Copts: compilerAttrs.copts, + Asflags: asFlags, + Local_includes: compilerAttrs.localIncludes, + Absolute_includes: compilerAttrs.absoluteIncludes, } props := bazel.BazelTargetModuleProperties{