From ae62818dd86e67e12eaf5d1d99e87c62e3676fcb Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Mon, 14 Jun 2021 16:52:28 -0700 Subject: [PATCH 1/3] Test include directory ordering Add a test to verify include directory ordering on the command line. Test: TestIncludeDirectryOrdering Change-Id: Iaf03c57201e9ec7a8daf417cb91470a5215e4053 --- cc/cc_test.go | 149 ++++++++++++++++++++++++++++++++++++++++++++++++++ cc/testing.go | 17 ++++-- 2 files changed, 162 insertions(+), 4 deletions(-) diff --git a/cc/cc_test.go b/cc/cc_test.go index e0fae5ad9..eaccfac88 100644 --- a/cc/cc_test.go +++ b/cc/cc_test.go @@ -4231,3 +4231,152 @@ func TestIncludeDirsExporting(t *testing.T) { ) }) } + +func TestIncludeDirectoryOrdering(t *testing.T) { + bp := ` + cc_library { + name: "libfoo", + srcs: ["foo.c"], + local_include_dirs: ["local_include_dirs"], + export_include_dirs: ["export_include_dirs"], + export_system_include_dirs: ["export_system_include_dirs"], + static_libs: ["libstatic1", "libstatic2"], + whole_static_libs: ["libwhole1", "libwhole2"], + shared_libs: ["libshared1", "libshared2"], + header_libs: ["libheader1", "libheader2"], + target: { + android: { + shared_libs: ["libandroid"], + local_include_dirs: ["android_local_include_dirs"], + export_include_dirs: ["android_export_include_dirs"], + }, + android_arm: { + shared_libs: ["libandroid_arm"], + local_include_dirs: ["android_arm_local_include_dirs"], + export_include_dirs: ["android_arm_export_include_dirs"], + }, + linux: { + shared_libs: ["liblinux"], + local_include_dirs: ["linux_local_include_dirs"], + export_include_dirs: ["linux_export_include_dirs"], + }, + }, + multilib: { + lib32: { + shared_libs: ["lib32"], + local_include_dirs: ["lib32_local_include_dirs"], + export_include_dirs: ["lib32_export_include_dirs"], + }, + }, + arch: { + arm: { + shared_libs: ["libarm"], + local_include_dirs: ["arm_local_include_dirs"], + export_include_dirs: ["arm_export_include_dirs"], + }, + }, + stl: "libc++", + sdk_version: "20", + } + + cc_library_headers { + name: "libheader1", + export_include_dirs: ["libheader1"], + sdk_version: "20", + stl: "none", + } + + cc_library_headers { + name: "libheader2", + export_include_dirs: ["libheader2"], + sdk_version: "20", + stl: "none", + } + ` + + libs := []string{ + "libstatic1", + "libstatic2", + "libwhole1", + "libwhole2", + "libshared1", + "libshared2", + "libandroid", + "libandroid_arm", + "liblinux", + "lib32", + "libarm", + } + + for _, lib := range libs { + bp += fmt.Sprintf(` + cc_library { + name: "%s", + export_include_dirs: ["%s"], + sdk_version: "20", + stl: "none", + } + `, lib, lib) + } + + ctx := PrepareForIntegrationTestWithCc.RunTestWithBp(t, bp) + // Use the arm variant instead of the arm64 variant so that it gets headers from + // ndk_libandroid_support to test LateStaticLibs. + cflags := ctx.ModuleForTests("libfoo", "android_arm_armv7-a-neon_sdk_static").Output("obj/foo.o").Args["cFlags"] + + var includes []string + flags := strings.Split(cflags, " ") + for i, flag := range flags { + if strings.Contains(flag, "Cflags") { + includes = append(includes, flag) + } else if strings.HasPrefix(flag, "-I") { + includes = append(includes, strings.TrimPrefix(flag, "-I")) + } else if flag == "-isystem" { + includes = append(includes, flags[i+1]) + } + } + + want := []string{ + "${config.ArmClangThumbCflags}", + "${config.ArmClangCflags}", + "${config.CommonClangGlobalCflags}", + "${config.DeviceClangGlobalCflags}", + "${config.ClangExternalCflags}", + "${config.ArmToolchainClangCflags}", + "${config.ArmClangArmv7ANeonCflags}", + "${config.ArmClangGenericCflags}", + "export_include_dirs", + "linux_export_include_dirs", + "android_export_include_dirs", + "arm_export_include_dirs", + "lib32_export_include_dirs", + "android_arm_export_include_dirs", + "android_arm_local_include_dirs", + "lib32_local_include_dirs", + "arm_local_include_dirs", + "android_local_include_dirs", + "linux_local_include_dirs", + "local_include_dirs", + ".", + "libheader1", + "libheader2", + "libwhole1", + "libwhole2", + "libstatic1", + "libstatic2", + "libshared1", + "libshared2", + "liblinux", + "libandroid", + "libarm", + "lib32", + "libandroid_arm", + "defaults/cc/common/ndk_libandroid_support", + "defaults/cc/common/ndk_libc++_shared", + "out/soong/ndk/sysroot/usr/include", + "out/soong/ndk/sysroot/usr/include/arm-linux-androideabi", + "${config.NoOverrideClangGlobalCflags}", + } + + android.AssertArrayString(t, "includes", want, includes) +} diff --git a/cc/testing.go b/cc/testing.go index f5c5ec5b1..fb9dc9c1b 100644 --- a/cc/testing.go +++ b/cc/testing.go @@ -34,6 +34,7 @@ func RegisterRequiredBuildComponentsForTest(ctx android.RegistrationContext) { ctx.RegisterModuleType("cc_object", ObjectFactory) ctx.RegisterModuleType("cc_genrule", genRuleFactory) ctx.RegisterModuleType("ndk_prebuilt_shared_stl", NdkPrebuiltSharedStlFactory) + ctx.RegisterModuleType("ndk_prebuilt_static_stl", NdkPrebuiltStaticStlFactory) ctx.RegisterModuleType("ndk_prebuilt_object", NdkPrebuiltObjectFactory) ctx.RegisterModuleType("ndk_library", NdkLibraryFactory) } @@ -403,7 +404,7 @@ func commonDefaultModules() string { cc_library { name: "ndk_libunwind", - sdk_version: "current", + sdk_version: "minimum", stl: "none", system_shared_libs: [], } @@ -428,6 +429,12 @@ func commonDefaultModules() string { ndk_prebuilt_shared_stl { name: "ndk_libc++_shared", + export_include_dirs: ["ndk_libc++_shared"], + } + + ndk_prebuilt_static_stl { + name: "ndk_libandroid_support", + export_include_dirs: ["ndk_libandroid_support"], } cc_library_static { @@ -578,9 +585,11 @@ var PrepareForTestWithCcDefaultModules = android.GroupFixturePreparers( // Additional files needed in tests that disallow non-existent source. android.MockFS{ - "defaults/cc/common/libc.map.txt": nil, - "defaults/cc/common/libdl.map.txt": nil, - "defaults/cc/common/libm.map.txt": nil, + "defaults/cc/common/libc.map.txt": nil, + "defaults/cc/common/libdl.map.txt": nil, + "defaults/cc/common/libm.map.txt": nil, + "defaults/cc/common/ndk_libandroid_support": nil, + "defaults/cc/common/ndk_libc++_shared": nil, }.AddToFixture(), // Place the default cc test modules that are common to all platforms in a location that will not From fe9acfecb307e69963cf5447bd50e203593cb219 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Mon, 14 Jun 2021 16:13:03 -0700 Subject: [PATCH 2/3] Move LateStaticLibs after SharedLibs in the dependency include order LateStaticLibs has always been after StaticLibs, move it after SharedLibs too so that exported headers from LateStaticLibs dependencies come after exported headers from SharedLibs dependencies. This will be used to ensure the ndk_libandroid_support headers come after the ndk_libc++_shared headers. Test: m checkbuild Change-Id: I5ef180c99cefcd823c6970c06d0e772a10fa0456 --- cc/cc.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cc/cc.go b/cc/cc.go index 555cb6ca1..818490aa5 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -2180,13 +2180,6 @@ func (c *Module) DepsMutator(actx android.BottomUpMutatorContext) { }, depTag, RewriteSnapshotLib(staticUnwinder(actx), GetSnapshot(c, &snapshotInfo, actx).StaticLibs)) } - for _, lib := range deps.LateStaticLibs { - depTag := libraryDependencyTag{Kind: staticLibraryDependency, Order: lateLibraryDependency} - actx.AddVariationDependencies([]blueprint.Variation{ - {Mutator: "link", Variation: "static"}, - }, depTag, RewriteSnapshotLib(lib, GetSnapshot(c, &snapshotInfo, actx).StaticLibs)) - } - // shared lib names without the #version suffix var sharedLibNames []string @@ -2212,6 +2205,13 @@ func (c *Module) DepsMutator(actx android.BottomUpMutatorContext) { AddSharedLibDependenciesWithVersions(ctx, c, variations, depTag, name, version, false) } + for _, lib := range deps.LateStaticLibs { + depTag := libraryDependencyTag{Kind: staticLibraryDependency, Order: lateLibraryDependency} + actx.AddVariationDependencies([]blueprint.Variation{ + {Mutator: "link", Variation: "static"}, + }, depTag, RewriteSnapshotLib(lib, GetSnapshot(c, &snapshotInfo, actx).StaticLibs)) + } + for _, lib := range deps.LateSharedLibs { if inList(lib, sharedLibNames) { // This is to handle the case that some of the late shared libs (libc, libdl, libm, ...) From c113e3cbe741bc6a70a6624dfb1597434e2f4642 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Mon, 14 Jun 2021 16:15:15 -0700 Subject: [PATCH 3/3] Use LateStaticLibs for ndk_libandroid_support This will obsolete the workaround in prebuilts/ndk/Android.bp to export the ndk_libandroid_support headers from ndk_libc++_shared, which would no longer have worked after the next patch. Test: m checkbuild Test: TestIncludeDirectoryOrdering Change-Id: I9b4e5799d939433da547661b862e9db5a4aacb09 --- cc/cc_test.go | 2 +- cc/stl.go | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/cc/cc_test.go b/cc/cc_test.go index eaccfac88..2d0d78b86 100644 --- a/cc/cc_test.go +++ b/cc/cc_test.go @@ -4371,8 +4371,8 @@ func TestIncludeDirectoryOrdering(t *testing.T) { "libarm", "lib32", "libandroid_arm", - "defaults/cc/common/ndk_libandroid_support", "defaults/cc/common/ndk_libc++_shared", + "defaults/cc/common/ndk_libandroid_support", "out/soong/ndk/sysroot/usr/include", "out/soong/ndk/sysroot/usr/include/arm-linux-androideabi", "${config.NoOverrideClangGlobalCflags}", diff --git a/cc/stl.go b/cc/stl.go index 75921c6ff..06dc840e8 100644 --- a/cc/stl.go +++ b/cc/stl.go @@ -199,7 +199,9 @@ func (stl *stl) deps(ctx BaseModuleContext, deps Deps) Deps { deps.StaticLibs = append(deps.StaticLibs, stl.Properties.SelectedStl, "ndk_libc++abi") } if needsLibAndroidSupport(ctx) { - deps.StaticLibs = append(deps.StaticLibs, "ndk_libandroid_support") + // Use LateStaticLibs for ndk_libandroid_support so that its include directories + // come after ndk_libc++_static or ndk_libc++_shared. + deps.LateStaticLibs = append(deps.LateStaticLibs, "ndk_libandroid_support") } deps.StaticLibs = append(deps.StaticLibs, "ndk_libunwind") default: