Express no{,_lib}crt via features vs attrs

Given that we can map them directly to disabling the
corresponding Bazel features: `-{link_,use_lib}crt`

Test: Existing (adapted) Unit Tests
Test: bp2build.sh
Change-Id: Ib502f6fb929ace8e86a1001e3cc21f399317500c
This commit is contained in:
Alex Márquez Pérez Muñíz Díaz Puras Thaureaux
2023-01-30 22:53:04 +00:00
parent 92ac65952d
commit 01ec55ec92
7 changed files with 70 additions and 106 deletions

View File

@@ -365,7 +365,7 @@ func TestCcBinaryNocrtTests(t *testing.T) {
{
description: "nocrt: true",
soongProperty: `nocrt: true,`,
bazelAttr: AttrNameToString{"link_crt": `False`},
bazelAttr: AttrNameToString{"features": `["-link_crt"]`},
},
{
description: "nocrt: false",
@@ -408,12 +408,12 @@ func TestCcBinaryNo_libcrtTests(t *testing.T) {
{
description: "no_libcrt: true",
soongProperty: `no_libcrt: true,`,
bazelAttr: AttrNameToString{"use_libcrt": `False`},
bazelAttr: AttrNameToString{"features": `["-use_libcrt"]`},
},
{
description: "no_libcrt: false",
soongProperty: `no_libcrt: false,`,
bazelAttr: AttrNameToString{"use_libcrt": `True`},
bazelAttr: AttrNameToString{},
},
{
description: "no_libcrt: not set",

View File

@@ -1308,7 +1308,7 @@ cc_library {
func TestCCLibraryNoCrtTrue(t *testing.T) {
runCcLibraryTestCase(t, Bp2buildTestCase{
Description: "cc_library - nocrt: true emits attribute",
Description: "cc_library - nocrt: true disables feature",
ModuleTypeUnderTest: "cc_library",
ModuleTypeUnderTestFactory: cc.LibraryFactory,
Filesystem: map[string]string{
@@ -1323,7 +1323,7 @@ cc_library {
}
`,
ExpectedBazelTargets: makeCcLibraryTargets("foo-lib", AttrNameToString{
"link_crt": `False`,
"features": `["-link_crt"]`,
"srcs": `["impl.cpp"]`,
}),
},
@@ -1375,7 +1375,13 @@ cc_library {
include_build_directory: false,
}
`,
ExpectedErr: fmt.Errorf("module \"foo-lib\": nocrt is not supported for arch variants"),
ExpectedBazelTargets: makeCcLibraryTargets("foo-lib", AttrNameToString{
"features": `select({
"//build/bazel/platforms/arch:arm": ["-link_crt"],
"//conditions:default": [],
})`,
"srcs": `["impl.cpp"]`,
}),
})
}
@@ -1395,8 +1401,8 @@ cc_library {
}
`,
ExpectedBazelTargets: makeCcLibraryTargets("foo-lib", AttrNameToString{
"features": `["-use_libcrt"]`,
"srcs": `["impl.cpp"]`,
"use_libcrt": `False`,
}),
})
}
@@ -1446,7 +1452,6 @@ cc_library {
`,
ExpectedBazelTargets: makeCcLibraryTargets("foo-lib", AttrNameToString{
"srcs": `["impl.cpp"]`,
"use_libcrt": `True`,
}),
})
}
@@ -1475,10 +1480,10 @@ cc_library {
`,
ExpectedBazelTargets: makeCcLibraryTargets("foo-lib", AttrNameToString{
"srcs": `["impl.cpp"]`,
"use_libcrt": `select({
"//build/bazel/platforms/arch:arm": False,
"//build/bazel/platforms/arch:x86": False,
"//conditions:default": None,
"features": `select({
"//build/bazel/platforms/arch:arm": ["-use_libcrt"],
"//build/bazel/platforms/arch:x86": ["-use_libcrt"],
"//conditions:default": [],
})`,
}),
})
@@ -1512,17 +1517,15 @@ cc_library {
}
`,
ExpectedBazelTargets: makeCcLibraryTargets("foo-lib", AttrNameToString{
"srcs": `["impl.cpp"]`,
"use_libcrt": `select({
"//build/bazel/platforms/os_arch:android_arm": False,
"//build/bazel/platforms/os_arch:android_x86": False,
"//build/bazel/platforms/os_arch:darwin_arm64": False,
"//build/bazel/platforms/os_arch:darwin_x86_64": False,
"//build/bazel/platforms/os_arch:linux_glibc_x86": False,
"//build/bazel/platforms/os_arch:linux_musl_x86": False,
"//build/bazel/platforms/os_arch:windows_x86": False,
"//conditions:default": None,
"features": `select({
"//build/bazel/platforms/arch:arm": ["-use_libcrt"],
"//build/bazel/platforms/arch:x86": ["-use_libcrt"],
"//conditions:default": [],
}) + select({
"//build/bazel/platforms/os:darwin": ["-use_libcrt"],
"//conditions:default": [],
})`,
"srcs": `["impl.cpp"]`,
}),
})
}
@@ -1557,16 +1560,10 @@ cc_library {
`,
ExpectedBazelTargets: makeCcLibraryTargets("foo-lib", AttrNameToString{
"srcs": `["impl.cpp"]`,
"use_libcrt": `select({
"//build/bazel/platforms/os_arch:android_arm": False,
"//build/bazel/platforms/os_arch:android_x86_64": False,
"//build/bazel/platforms/os_arch:darwin_arm64": True,
"//build/bazel/platforms/os_arch:darwin_x86_64": False,
"//build/bazel/platforms/os_arch:linux_bionic_x86_64": False,
"//build/bazel/platforms/os_arch:linux_glibc_x86_64": False,
"//build/bazel/platforms/os_arch:linux_musl_x86_64": False,
"//build/bazel/platforms/os_arch:windows_x86_64": False,
"//conditions:default": None,
"features": `select({
"//build/bazel/platforms/arch:arm": ["-use_libcrt"],
"//build/bazel/platforms/arch:x86_64": ["-use_libcrt"],
"//conditions:default": [],
})`,
}),
})

View File

@@ -15,7 +15,6 @@
package bp2build
import (
"fmt"
"testing"
"android/soong/android"
@@ -405,7 +404,7 @@ cc_library_shared {
func TestCcLibrarySharedNoCrtTrue(t *testing.T) {
runCcLibrarySharedTestCase(t, Bp2buildTestCase{
Description: "cc_library_shared - nocrt: true emits attribute",
Description: "cc_library_shared - nocrt: true disables feature",
Filesystem: map[string]string{
"impl.cpp": "",
},
@@ -419,7 +418,7 @@ cc_library_shared {
`,
ExpectedBazelTargets: []string{
MakeBazelTarget("cc_library_shared", "foo_shared", AttrNameToString{
"link_crt": `False`,
"features": `["-link_crt"]`,
"srcs": `["impl.cpp"]`,
}),
},
@@ -428,7 +427,7 @@ cc_library_shared {
func TestCcLibrarySharedNoCrtFalse(t *testing.T) {
runCcLibrarySharedTestCase(t, Bp2buildTestCase{
Description: "cc_library_shared - nocrt: false doesn't emit attribute",
Description: "cc_library_shared - nocrt: false doesn't disable feature",
Filesystem: map[string]string{
"impl.cpp": "",
},
@@ -469,7 +468,15 @@ cc_library_shared {
include_build_directory: false,
}
`,
ExpectedErr: fmt.Errorf("module \"foo_shared\": nocrt is not supported for arch variants"),
ExpectedBazelTargets: []string{
MakeBazelTarget("cc_library_shared", "foo_shared", AttrNameToString{
"features": `select({
"//build/bazel/platforms/arch:arm": ["-link_crt"],
"//conditions:default": [],
})`,
"srcs": `["impl.cpp"]`,
}),
},
})
}

View File

@@ -151,7 +151,7 @@ func (binary *binaryDecorator) getStem(ctx BaseModuleContext) string {
// modules common to most binaries, such as bionic libraries.
func (binary *binaryDecorator) linkerDeps(ctx DepsContext, deps Deps) Deps {
deps = binary.baseLinker.linkerDeps(ctx, deps)
if !Bool(binary.baseLinker.Properties.Nocrt) {
if binary.baseLinker.Properties.crt() {
if binary.static() {
deps.CrtBegin = ctx.toolchain().CrtBeginStaticBinary()
deps.CrtEnd = ctx.toolchain().CrtEndStaticBinary()
@@ -630,8 +630,6 @@ func binaryBp2buildAttrs(ctx android.TopDownMutatorContext, m *Module) binaryAtt
Local_includes: baseAttrs.localIncludes,
Absolute_includes: baseAttrs.absoluteIncludes,
Linkopts: baseAttrs.linkopts,
Link_crt: baseAttrs.linkCrt,
Use_libcrt: baseAttrs.useLibcrt,
Use_version_lib: baseAttrs.useVersionLib,
Rtti: baseAttrs.rtti,
Stl: baseAttrs.stl,
@@ -695,9 +693,6 @@ type binaryAttributes struct {
Linkopts bazel.StringListAttribute
Additional_linker_inputs bazel.LabelListAttribute
Link_crt bazel.BoolAttribute
Use_libcrt bazel.BoolAttribute
Use_version_lib bazel.BoolAttribute
Rtti bazel.BoolAttribute

View File

@@ -965,8 +965,6 @@ type linkerAttributes struct {
systemDynamicDeps bazel.LabelListAttribute
usedSystemDynamicDepAsDynamicDep map[string]bool
linkCrt bazel.BoolAttribute
useLibcrt bazel.BoolAttribute
useVersionLib bazel.BoolAttribute
linkopts bazel.StringListAttribute
additionalLinkerInputs bazel.LabelListAttribute
@@ -1138,6 +1136,13 @@ func (la *linkerAttributes) bp2buildForAxisAndConfig(ctx android.BazelConversion
}
}
if !props.libCrt() {
axisFeatures = append(axisFeatures, "-use_libcrt")
}
if !props.crt() {
axisFeatures = append(axisFeatures, "-link_crt")
}
// This must happen before the addition of flags for Version Script and
// Dynamic List, as these flags must be split on spaces and those must not
linkerFlags = parseCommandLineFlags(linkerFlags, filterOutClangUnknownCflags)
@@ -1157,16 +1162,6 @@ func (la *linkerAttributes) bp2buildForAxisAndConfig(ctx android.BazelConversion
la.additionalLinkerInputs.SetSelectValue(axis, config, additionalLinkerInputs)
la.linkopts.SetSelectValue(axis, config, linkerFlags)
la.useLibcrt.SetSelectValue(axis, config, props.libCrt())
// it's very unlikely for nocrt to be arch variant, so bp2build doesn't support it.
if props.crt() != nil {
if axis == bazel.NoConfigAxis {
la.linkCrt.SetSelectValue(axis, config, props.crt())
} else if axis == bazel.ArchConfigurationAxis {
ctx.ModuleErrorf("nocrt is not supported for arch variants")
}
}
if axisFeatures != nil {
la.features.SetSelectValue(axis, config, axisFeatures)

View File

@@ -243,7 +243,6 @@ type bazelCcLibraryAttributes struct {
Local_includes bazel.StringListAttribute
Absolute_includes bazel.StringListAttribute
Linkopts bazel.StringListAttribute
Use_libcrt bazel.BoolAttribute
Rtti bazel.BoolAttribute
Stl *string
@@ -251,7 +250,6 @@ type bazelCcLibraryAttributes struct {
C_std *string
// This is shared only.
Link_crt bazel.BoolAttribute
Additional_linker_inputs bazel.LabelListAttribute
// Common properties shared between both shared and static variants.
@@ -360,7 +358,6 @@ func libraryBp2Build(ctx android.TopDownMutatorContext, m *Module) {
Export_system_includes: exportedIncludes.SystemIncludes,
Local_includes: compilerAttrs.localIncludes,
Absolute_includes: compilerAttrs.absoluteIncludes,
Use_libcrt: linkerAttrs.useLibcrt,
Rtti: compilerAttrs.rtti,
Stl: compilerAttrs.stl,
Cpp_std: compilerAttrs.cppStd,
@@ -381,8 +378,6 @@ func libraryBp2Build(ctx android.TopDownMutatorContext, m *Module) {
Local_includes: compilerAttrs.localIncludes,
Absolute_includes: compilerAttrs.absoluteIncludes,
Linkopts: linkerAttrs.linkopts,
Link_crt: linkerAttrs.linkCrt,
Use_libcrt: linkerAttrs.useLibcrt,
Rtti: compilerAttrs.rtti,
Stl: compilerAttrs.stl,
Cpp_std: compilerAttrs.cppStd,
@@ -1500,7 +1495,7 @@ func (library *libraryDecorator) linkerDeps(ctx DepsContext, deps Deps) Deps {
deps.ReexportSharedLibHeaders = append(deps.ReexportSharedLibHeaders, library.StaticProperties.Static.Export_shared_lib_headers...)
deps.ReexportStaticLibHeaders = append(deps.ReexportStaticLibHeaders, library.StaticProperties.Static.Export_static_lib_headers...)
} else if library.shared() {
if !Bool(library.baseLinker.Properties.Nocrt) {
if library.baseLinker.Properties.crt() {
deps.CrtBegin = append(deps.CrtBegin, ctx.toolchain().CrtBeginSharedLibrary()...)
deps.CrtEnd = append(deps.CrtEnd, ctx.toolchain().CrtEndSharedLibrary()...)
}
@@ -2884,9 +2879,6 @@ func sharedOrStaticLibraryBp2Build(ctx android.TopDownMutatorContext, module *Mo
commonAttrs.Deps.Add(baseAttributes.protoDependency)
attrs = &bazelCcLibraryStaticAttributes{
staticOrSharedAttributes: commonAttrs,
Use_libcrt: linkerAttrs.useLibcrt,
Rtti: compilerAttrs.rtti,
Stl: compilerAttrs.stl,
Cpp_std: compilerAttrs.cppStd,
@@ -2915,8 +2907,6 @@ func sharedOrStaticLibraryBp2Build(ctx android.TopDownMutatorContext, module *Mo
Asflags: asFlags,
Linkopts: linkerAttrs.linkopts,
Link_crt: linkerAttrs.linkCrt,
Use_libcrt: linkerAttrs.useLibcrt,
Use_version_lib: linkerAttrs.useVersionLib,
Rtti: compilerAttrs.rtti,
@@ -2974,9 +2964,7 @@ func sharedOrStaticLibraryBp2Build(ctx android.TopDownMutatorContext, module *Mo
type bazelCcLibraryStaticAttributes struct {
staticOrSharedAttributes
Use_libcrt bazel.BoolAttribute
Use_version_lib bazel.BoolAttribute
Rtti bazel.BoolAttribute
Stl *string
Cpp_std *string
@@ -3001,9 +2989,6 @@ type bazelCcLibrarySharedAttributes struct {
staticOrSharedAttributes
Linkopts bazel.StringListAttribute
Link_crt bazel.BoolAttribute // Only for linking shared library (and cc_binary)
Use_libcrt bazel.BoolAttribute
Use_version_lib bazel.BoolAttribute
Rtti bazel.BoolAttribute

View File

@@ -237,29 +237,14 @@ type BaseLinkerProperties struct {
Exclude_shared_libs []string `android:"arch_variant"`
}
func invertBoolPtr(value *bool) *bool {
if value == nil {
return nil
}
ret := !(*value)
return &ret
}
func (blp *BaseLinkerProperties) crt() *bool {
val := invertBoolPtr(blp.Nocrt)
if val != nil && *val {
// == True
//
func (blp *BaseLinkerProperties) crt() bool {
// Since crt is enabled for almost every module compiling against the Bionic runtime,
// use `nil` when it's enabled, and rely on the Starlark macro to set it to True by default.
// This keeps the BUILD files clean.
return nil
}
return val // can be False or nil
// we interpret `nil` as enabled.
return blp.Nocrt == nil || !*blp.Nocrt
}
func (blp *BaseLinkerProperties) libCrt() *bool {
return invertBoolPtr(blp.No_libcrt)
func (blp *BaseLinkerProperties) libCrt() bool {
return blp.No_libcrt == nil || !*blp.No_libcrt
}
func NewBaseLinker(sanitize *sanitize) *baseLinker {
@@ -392,7 +377,7 @@ func (linker *baseLinker) linkerDeps(ctx DepsContext, deps Deps) Deps {
if ctx.toolchain().Bionic() {
// libclang_rt.builtins has to be last on the command line
if !Bool(linker.Properties.No_libcrt) && !ctx.header() {
if linker.Properties.libCrt() && !ctx.header() {
deps.UnexportedStaticLibs = append(deps.UnexportedStaticLibs, config.BuiltinsRuntimeLibrary(ctx.toolchain()))
}
@@ -415,7 +400,7 @@ func (linker *baseLinker) linkerDeps(ctx DepsContext, deps Deps) Deps {
ctx.PropertyErrorf("system_shared_libs", "libdl must be after libc")
}
} else if ctx.toolchain().Musl() {
if !Bool(linker.Properties.No_libcrt) && !ctx.header() {
if linker.Properties.libCrt() && !ctx.header() {
deps.UnexportedStaticLibs = append(deps.UnexportedStaticLibs, config.BuiltinsRuntimeLibrary(ctx.toolchain()))
}
}