From e4f6ebaf6c37f8c7e3e3d671f87b7506f09681d4 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Tue, 22 Sep 2020 18:11:25 -0700 Subject: [PATCH 1/3] Simplify missing whole_static_libs checking Whole_static_libs required custom error checking when AllowMissingDependencies was set because it could end up depending on an empty list of objects, which would leave nothing in the dependency tree that had been replaced with an ErrorRule. Reuse the prebuilts case to depend on the .a file when there are no objects and remove the custom error handling. Test: TestEmptyWholeStaticLibsAllowMissingDependencies Change-Id: Ic3216235f7e5ae8b5b6ab31ef2ca35c3994d82aa --- cc/cc.go | 23 +++++++++++++---------- cc/cc_test.go | 43 +++++++++++++++++++++++++++++++++++++++++++ cc/library.go | 11 ----------- 3 files changed, 56 insertions(+), 21 deletions(-) diff --git a/cc/cc.go b/cc/cc.go index e71f85924..fb7c8cf10 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -2559,17 +2559,20 @@ func (c *Module) depsToPaths(ctx android.ModuleContext) PathDeps { // in the context of proper cc.Modules. if ccWholeStaticLib, ok := ccDep.(*Module); ok { staticLib := ccWholeStaticLib.linker.(libraryInterface) - if missingDeps := staticLib.getWholeStaticMissingDeps(); missingDeps != nil { - postfix := " (required by " + ctx.OtherModuleName(dep) + ")" - for i := range missingDeps { - missingDeps[i] += postfix - } - ctx.AddMissingDependencies(missingDeps) - } - if _, ok := ccWholeStaticLib.linker.(prebuiltLinkerInterface); ok { - depPaths.WholeStaticLibsFromPrebuilts = append(depPaths.WholeStaticLibsFromPrebuilts, linkFile.Path()) + if objs := staticLib.objs(); len(objs.objFiles) > 0 { + depPaths.WholeStaticLibObjs = depPaths.WholeStaticLibObjs.Append(objs) } else { - depPaths.WholeStaticLibObjs = depPaths.WholeStaticLibObjs.Append(staticLib.objs()) + // This case normally catches prebuilt static + // libraries, but it can also occur when + // AllowMissingDependencies is on and the + // dependencies has no sources of its own + // but has a whole_static_libs dependency + // on a missing library. We want to depend + // on the .a file so that there is something + // in the dependency tree that contains the + // error rule for the missing transitive + // dependency. + depPaths.WholeStaticLibsFromPrebuilts = append(depPaths.WholeStaticLibsFromPrebuilts, linkFile.Path()) } } else { ctx.ModuleErrorf( diff --git a/cc/cc_test.go b/cc/cc_test.go index 132d09136..e0d464093 100644 --- a/cc/cc_test.go +++ b/cc/cc_test.go @@ -3780,3 +3780,46 @@ func TestProductVariableDefaults(t *testing.T) { t.Errorf("expected -DBAR in cppflags, got %q", libfoo.flags.Local.CppFlags) } } + +func TestEmptyWholeStaticLibsAllowMissingDependencies(t *testing.T) { + t.Parallel() + bp := ` + cc_library_static { + name: "libfoo", + srcs: ["foo.c"], + whole_static_libs: ["libbar"], + } + + cc_library_static { + name: "libbar", + whole_static_libs: ["libmissing"], + } + ` + + config := TestConfig(buildDir, android.Android, nil, bp, nil) + config.TestProductVariables.Allow_missing_dependencies = BoolPtr(true) + + ctx := CreateTestContext() + ctx.SetAllowMissingDependencies(true) + ctx.Register(config) + + _, errs := ctx.ParseFileList(".", []string{"Android.bp"}) + android.FailIfErrored(t, errs) + _, errs = ctx.PrepareBuildActions(config) + android.FailIfErrored(t, errs) + + libbar := ctx.ModuleForTests("libbar", "android_arm64_armv8-a_static").Output("libbar.a") + if g, w := libbar.Rule, android.ErrorRule; g != w { + t.Fatalf("Expected libbar rule to be %q, got %q", w, g) + } + + if g, w := libbar.Args["error"], "missing dependencies: libmissing"; !strings.Contains(g, w) { + t.Errorf("Expected libbar error to contain %q, was %q", w, g) + } + + libfoo := ctx.ModuleForTests("libfoo", "android_arm64_armv8-a_static").Output("libfoo.a") + if g, w := libfoo.Inputs.Strings(), libbar.Output.String(); !android.InList(w, g) { + t.Errorf("Expected libfoo.a to depend on %q, got %q", w, g) + } + +} diff --git a/cc/library.go b/cc/library.go index 8048f0002..bf7868ff7 100644 --- a/cc/library.go +++ b/cc/library.go @@ -338,10 +338,6 @@ type libraryDecorator struct { flagExporter stripper Stripper - // If we're used as a whole_static_lib, our missing dependencies need - // to be given - wholeStaticMissingDeps []string - // For whole_static_libs objects Objects @@ -682,7 +678,6 @@ func (library *libraryDecorator) compile(ctx ModuleContext, flags Flags, deps Pa } type libraryInterface interface { - getWholeStaticMissingDeps() []string static() bool shared() bool objs() Objects @@ -889,8 +884,6 @@ func (library *libraryDecorator) linkStatic(ctx ModuleContext, library.coverageOutputFile = TransformCoverageFilesToZip(ctx, library.objects, ctx.ModuleName()) - library.wholeStaticMissingDeps = ctx.GetMissingDependencies() - ctx.CheckbuildFile(outputFile) return outputFile @@ -1182,10 +1175,6 @@ func (library *libraryDecorator) buildShared() bool { BoolDefault(library.SharedProperties.Shared.Enabled, true) } -func (library *libraryDecorator) getWholeStaticMissingDeps() []string { - return append([]string(nil), library.wholeStaticMissingDeps...) -} - func (library *libraryDecorator) objs() Objects { return library.objects } From d3e05caa477e03767de1baf1e30f9cabbf7e232b Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Wed, 23 Sep 2020 16:49:23 -0700 Subject: [PATCH 2/3] Don't export flags from SourceProvider variants There is no need to export the linkdirs from the SourceProvider variant. Remove them to reduce differences in build.ninja from a later patch that moves the flag exporting into compile(), which isn't called by the SourceProvider variant. Test: m checkbuild Change-Id: I9c4d3a336a07cb9074376303bfa277c05d893b98 --- rust/rust.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/rust/rust.go b/rust/rust.go index 22b81f135..1f8b904d3 100644 --- a/rust/rust.go +++ b/rust/rust.go @@ -695,6 +695,11 @@ func (mod *Module) GenerateAndroidBuildActions(actx android.ModuleContext) { if mod.compiler.(libraryInterface).source() { mod.sourceProvider.GenerateSource(ctx, deps) mod.sourceProvider.setSubName(ctx.ModuleSubDir()) + if lib, ok := mod.compiler.(*libraryDecorator); ok { + lib.flagExporter.linkDirs = nil + lib.flagExporter.linkObjects = nil + lib.flagExporter.depFlags = nil + } } else { sourceMod := actx.GetDirectDepWithTag(mod.Name(), sourceDepTag) sourceLib := sourceMod.(*Module).compiler.(*libraryDecorator) From 7812fd38149d7f128783662969fc24ad528e0686 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Fri, 25 Sep 2020 12:35:10 -0700 Subject: [PATCH 3/3] Fix ChooseSdkVersion after api levels I2954bb21c1cfdeb305f25cfb6c8711c930f6ed50 switched normalizeVersions to work on ApiLevels, which inadvertantly caused it to return "current" instead of "10000" for libraries that specify "current" in their stubs property. ChooseSdkVersion couldn't handle "current" because it was manually converting the version to an int. Switch ChooseSdkVersion to use ApiLevels instead so that it can handle "current". Test: m checkbuild Change-Id: Id412359e092483ba419118dd03bc206fae702a96 --- android/apex.go | 16 +++++--- apex/apex_test.go | 99 +++++++++++++++++++++++++++++++++++++++++++++++ cc/cc.go | 4 +- 3 files changed, 111 insertions(+), 8 deletions(-) diff --git a/android/apex.go b/android/apex.go index 3cc663bab..7ae46d4be 100644 --- a/android/apex.go +++ b/android/apex.go @@ -136,7 +136,7 @@ type ApexModule interface { // Returns the highest version which is <= maxSdkVersion. // For example, with maxSdkVersion is 10 and versionList is [9,11] // it returns 9 as string - ChooseSdkVersion(versionList []string, maxSdkVersion int) (string, error) + ChooseSdkVersion(ctx BaseModuleContext, versionList []string, maxSdkVersion ApiLevel) (string, error) // Tests if the module comes from an updatable APEX. Updatable() bool @@ -320,14 +320,18 @@ func (m *ApexModuleBase) DepIsInSameApex(ctx BaseModuleContext, dep Module) bool return true } -func (m *ApexModuleBase) ChooseSdkVersion(versionList []string, maxSdkVersion int) (string, error) { +func (m *ApexModuleBase) ChooseSdkVersion(ctx BaseModuleContext, versionList []string, maxSdkVersion ApiLevel) (string, error) { for i := range versionList { - ver, _ := strconv.Atoi(versionList[len(versionList)-i-1]) - if ver <= maxSdkVersion { - return versionList[len(versionList)-i-1], nil + version := versionList[len(versionList)-i-1] + ver, err := ApiLevelFromUser(ctx, version) + if err != nil { + return "", err + } + if ver.LessThanOrEqualTo(maxSdkVersion) { + return version, nil } } - return "", fmt.Errorf("not found a version(<=%d) in versionList: %v", maxSdkVersion, versionList) + return "", fmt.Errorf("not found a version(<=%s) in versionList: %v", maxSdkVersion, versionList) } func (m *ApexModuleBase) checkApexAvailableProperty(mctx BaseModuleContext) { diff --git a/apex/apex_test.go b/apex/apex_test.go index 02689a0b7..20991096c 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -836,6 +836,105 @@ func TestApexWithStubs(t *testing.T) { }) } +func TestApexWithStubsWithMinSdkVersion(t *testing.T) { + t.Parallel() + ctx, _ := testApex(t, ` + apex { + name: "myapex", + key: "myapex.key", + native_shared_libs: ["mylib", "mylib3"], + min_sdk_version: "29", + } + + apex_key { + name: "myapex.key", + public_key: "testkey.avbpubkey", + private_key: "testkey.pem", + } + + cc_library { + name: "mylib", + srcs: ["mylib.cpp"], + shared_libs: ["mylib2", "mylib3"], + system_shared_libs: [], + stl: "none", + apex_available: [ "myapex" ], + min_sdk_version: "28", + } + + cc_library { + name: "mylib2", + srcs: ["mylib.cpp"], + cflags: ["-include mylib.h"], + system_shared_libs: [], + stl: "none", + stubs: { + versions: ["28", "29", "30", "current"], + }, + min_sdk_version: "28", + } + + cc_library { + name: "mylib3", + srcs: ["mylib.cpp"], + shared_libs: ["mylib4"], + system_shared_libs: [], + stl: "none", + stubs: { + versions: ["28", "29", "30", "current"], + }, + apex_available: [ "myapex" ], + min_sdk_version: "28", + } + + cc_library { + name: "mylib4", + srcs: ["mylib.cpp"], + system_shared_libs: [], + stl: "none", + apex_available: [ "myapex" ], + min_sdk_version: "28", + } + `) + + apexRule := ctx.ModuleForTests("myapex", "android_common_myapex_image").Rule("apexRule") + copyCmds := apexRule.Args["copy_commands"] + + // Ensure that direct non-stubs dep is always included + ensureContains(t, copyCmds, "image.apex/lib64/mylib.so") + + // Ensure that indirect stubs dep is not included + ensureNotContains(t, copyCmds, "image.apex/lib64/mylib2.so") + + // Ensure that direct stubs dep is included + ensureContains(t, copyCmds, "image.apex/lib64/mylib3.so") + + mylibLdFlags := ctx.ModuleForTests("mylib", "android_arm64_armv8-a_shared_apex29").Rule("ld").Args["libFlags"] + + // Ensure that mylib is linking with the version 29 stubs for mylib2 + ensureContains(t, mylibLdFlags, "mylib2/android_arm64_armv8-a_shared_29/mylib2.so") + // ... and not linking to the non-stub (impl) variant of mylib2 + ensureNotContains(t, mylibLdFlags, "mylib2/android_arm64_armv8-a_shared/mylib2.so") + + // Ensure that mylib is linking with the non-stub (impl) of mylib3 (because mylib3 is in the same apex) + ensureContains(t, mylibLdFlags, "mylib3/android_arm64_armv8-a_shared_apex29/mylib3.so") + // .. and not linking to the stubs variant of mylib3 + ensureNotContains(t, mylibLdFlags, "mylib3/android_arm64_armv8-a_shared_29/mylib3.so") + + // Ensure that stubs libs are built without -include flags + mylib2Cflags := ctx.ModuleForTests("mylib2", "android_arm64_armv8-a_static").Rule("cc").Args["cFlags"] + ensureNotContains(t, mylib2Cflags, "-include ") + + // Ensure that genstub is invoked with --apex + ensureContains(t, "--apex", ctx.ModuleForTests("mylib2", "android_arm64_armv8-a_static_29").Rule("genStubSrc").Args["flags"]) + + ensureExactContents(t, ctx, "myapex", "android_common_myapex_image", []string{ + "lib64/mylib.so", + "lib64/mylib3.so", + "lib64/mylib4.so", + }) +} + func TestApexWithExplicitStubsDependency(t *testing.T) { ctx, _ := testApex(t, ` apex { diff --git a/cc/cc.go b/cc/cc.go index fb7c8cf10..3d4545dd3 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -2465,7 +2465,7 @@ func (c *Module) depsToPaths(ctx android.ModuleContext) PathDeps { // when to use (unspecified) stubs, check min_sdk_version and choose the right one if useThisDep && depIsStubs && !libDepTag.explicitlyVersioned { - versionToUse, err := c.ChooseSdkVersion(ccDep.StubsVersions(), c.apexSdkVersion.FinalOrFutureInt()) + versionToUse, err := c.ChooseSdkVersion(ctx, ccDep.StubsVersions(), c.apexSdkVersion) if err != nil { ctx.OtherModuleErrorf(dep, err.Error()) return @@ -2488,7 +2488,7 @@ func (c *Module) depsToPaths(ctx android.ModuleContext) PathDeps { // if this is for use_vendor apex && dep has stubsVersions // apply the same rule of apex sdk enforcement to choose right version var err error - versionToUse, err = c.ChooseSdkVersion(versions, c.apexSdkVersion.FinalOrFutureInt()) + versionToUse, err = c.ChooseSdkVersion(ctx, versions, c.apexSdkVersion) if err != nil { ctx.OtherModuleErrorf(dep, err.Error()) return