From fa3f0782f7b1f179435f19d37e7e348cff626695 Mon Sep 17 00:00:00 2001 From: Jihoon Kang Date: Wed, 21 Aug 2024 20:42:18 +0000 Subject: [PATCH] Remove suffix based stub matching logic This change prevents non-stub modules with stub suffix from being determined as the stub module, and instead makes the check more robust by determining the condition based on the user-hidden `Stub_contributing_api` property, which is only set for the stub submodules generated by `java_sdk_library`. Test: m nothing --no-skip-soong-tests Bug: 361179822 Change-Id: I28a599c5b4fe1e8460e60580c0535aaf19e39ba3 --- android/sdk_version.go | 9 ++++++++ java/base.go | 45 ++++++++++++++++++++++++---------------- java/sdk_library.go | 43 ++++++++++++++++---------------------- java/sdk_library_test.go | 36 ++++++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+), 43 deletions(-) diff --git a/android/sdk_version.go b/android/sdk_version.go index 01b55d0da..a9b88fbce 100644 --- a/android/sdk_version.go +++ b/android/sdk_version.go @@ -93,6 +93,15 @@ func (k SdkKind) String() string { } } +func ToSdkKind(s string) SdkKind { + for kind := SdkNone; kind <= SdkPrivate; kind++ { + if s == kind.String() { + return kind + } + } + return SdkInvalid +} + func (k SdkKind) DefaultJavaLibraryName() string { switch k { case SdkPublic: diff --git a/java/base.go b/java/base.go index 4ab82c58f..910145785 100644 --- a/java/base.go +++ b/java/base.go @@ -229,6 +229,10 @@ type CommonProperties struct { Ravenizer struct { Enabled *bool } + + // Contributing api surface of the stub module. Is not visible to bp modules, and should + // only be set for stub submodules generated by the java_sdk_library + Stub_contributing_api *string `blueprint:"mutated"` } // Properties that are specific to device modules. Host module factories should not add these when @@ -1541,7 +1545,7 @@ func (j *Module) compile(ctx android.ModuleContext, extraSrcJars, extraClasspath // outputFile should be agnostic to the build configuration, // thus "combine" the single static lib in order to prevent the static lib from being exposed // to the copy rules. - stub, _ := moduleStubLinkType(ctx.ModuleName()) + stub, _ := moduleStubLinkType(j) if stub { combinedJar := android.PathForModuleOut(ctx, "combined", jarName) @@ -2169,6 +2173,25 @@ type moduleWithSdkDep interface { getSdkLinkType(ctx android.BaseModuleContext, name string) (ret sdkLinkType, stubs bool) } +func sdkLinkTypeFromSdkKind(k android.SdkKind) sdkLinkType { + switch k { + case android.SdkCore: + return javaCore + case android.SdkSystem: + return javaSystem + case android.SdkPublic: + return javaSdk + case android.SdkModule: + return javaModule + case android.SdkSystemServer: + return javaSystemServer + case android.SdkPrivate, android.SdkNone, android.SdkCorePlatform, android.SdkTest: + return javaPlatform + default: + return javaSdk + } +} + func (m *Module) getSdkLinkType(ctx android.BaseModuleContext, name string) (ret sdkLinkType, stubs bool) { switch name { case android.SdkCore.DefaultJavaLibraryName(), @@ -2190,30 +2213,16 @@ func (m *Module) getSdkLinkType(ctx android.BaseModuleContext, name string) (ret return javaSystem, true } - if stub, linkType := moduleStubLinkType(name); stub { + if stub, linkType := moduleStubLinkType(m); stub { return linkType, true } ver := m.SdkVersion(ctx) - switch ver.Kind { - case android.SdkCore: - return javaCore, false - case android.SdkSystem: - return javaSystem, false - case android.SdkPublic: - return javaSdk, false - case android.SdkModule: - return javaModule, false - case android.SdkSystemServer: - return javaSystemServer, false - case android.SdkPrivate, android.SdkNone, android.SdkCorePlatform, android.SdkTest: - return javaPlatform, false - } - if !ver.Valid() { panic(fmt.Errorf("sdk_version is invalid. got %q", ver.Raw)) } - return javaSdk, false + + return sdkLinkTypeFromSdkKind(ver.Kind), false } // checkSdkLinkType make sures the given dependency doesn't have a lower SDK link type rank than diff --git a/java/sdk_library.go b/java/sdk_library.go index 4f95a997d..765e96391 100644 --- a/java/sdk_library.go +++ b/java/sdk_library.go @@ -1794,7 +1794,8 @@ type libraryProperties struct { Dir *string Tag *string } - Is_stubs_module *bool + Is_stubs_module *bool + Stub_contributing_api *string } func (module *SdkLibrary) stubsLibraryProps(mctx android.DefaultableHookContext, apiScope *apiScope) libraryProperties { @@ -1820,6 +1821,7 @@ func (module *SdkLibrary) stubsLibraryProps(mctx android.DefaultableHookContext, // interop with older developer tools that don't support 1.9. props.Java_version = proptools.StringPtr("1.8") props.Is_stubs_module = proptools.BoolPtr(true) + props.Stub_contributing_api = proptools.StringPtr(apiScope.kind.String()) return props } @@ -2087,6 +2089,8 @@ func (module *SdkLibrary) topLevelStubsLibraryProps(mctx android.DefaultableHook } props.Compile_dex = compileDex + props.Stub_contributing_api = proptools.StringPtr(apiScope.kind.String()) + if !Bool(module.sdkLibraryProperties.No_dist) && doDist { props.Dist.Targets = []string{"sdk", "win_sdk"} props.Dist.Dest = proptools.StringPtr(fmt.Sprintf("%v.jar", module.distStem())) @@ -2423,36 +2427,25 @@ func (s *defaultNamingScheme) exportableSourceStubsLibraryModuleName(scope *apiS var _ sdkLibraryComponentNamingScheme = (*defaultNamingScheme)(nil) -func hasStubsLibrarySuffix(name string, apiScope *apiScope) bool { - return strings.HasSuffix(name, apiScope.stubsLibraryModuleNameSuffix()) || - strings.HasSuffix(name, apiScope.exportableStubsLibraryModuleNameSuffix()) -} - -func moduleStubLinkType(name string) (stub bool, ret sdkLinkType) { - name = strings.TrimSuffix(name, ".from-source") - - // This suffix-based approach is fragile and could potentially mis-trigger. - // TODO(b/155164730): Clean this up when modules no longer reference sdk_lib stubs directly. - if hasStubsLibrarySuffix(name, apiScopePublic) { - if name == "hwbinder.stubs" || name == "libcore_private.stubs" { - // Due to a previous bug, these modules were not considered stubs, so we retain that. - return false, javaPlatform - } +func moduleStubLinkType(j *Module) (stub bool, ret sdkLinkType) { + kind := android.ToSdkKind(proptools.String(j.properties.Stub_contributing_api)) + switch kind { + case android.SdkPublic: return true, javaSdk - } - if hasStubsLibrarySuffix(name, apiScopeSystem) { + case android.SdkSystem: return true, javaSystem - } - if hasStubsLibrarySuffix(name, apiScopeModuleLib) { + case android.SdkModule: return true, javaModule - } - if hasStubsLibrarySuffix(name, apiScopeTest) { + case android.SdkTest: return true, javaSystem - } - if hasStubsLibrarySuffix(name, apiScopeSystemServer) { + case android.SdkSystemServer: return true, javaSystemServer + // Default value for all modules other than java_sdk_library-generated stub submodules + case android.SdkInvalid: + return false, javaPlatform + default: + panic(fmt.Sprintf("stub_contributing_api set as an unsupported sdk kind %s", kind.String())) } - return false, javaPlatform } // java_sdk_library is a special Java library that provides optional platform APIs to apps. diff --git a/java/sdk_library_test.go b/java/sdk_library_test.go index 52d4af0fd..485776b99 100644 --- a/java/sdk_library_test.go +++ b/java/sdk_library_test.go @@ -1858,3 +1858,39 @@ func TestMultipleSdkLibraryPrebuilts(t *testing.T) { android.AssertStringListContains(t, "Could not find the expected stub on classpath", inputs, tc.expectedStubPath) } } + +func TestStubLinkType(t *testing.T) { + android.GroupFixturePreparers( + prepareForJavaTest, + PrepareForTestWithJavaSdkLibraryFiles, + FixtureWithLastReleaseApis("foo"), + ).ExtendWithErrorHandler(android.FixtureExpectsOneErrorPattern( + `module "baz" variant "android_common": compiles against system API, but dependency `+ + `"bar.stubs.system" is compiling against module API. In order to fix this, `+ + `consider adjusting sdk_version: OR platform_apis: property of the source or `+ + `target module so that target module is built with the same or smaller API set `+ + `when compared to the source.`), + ).RunTestWithBp(t, ` + java_sdk_library { + name: "foo", + srcs: ["a.java"], + sdk_version: "current", + } + java_library { + name: "bar.stubs.system", + srcs: ["a.java"], + sdk_version: "module_current", + is_stubs_module: false, + } + + java_library { + name: "baz", + srcs: ["b.java"], + libs: [ + "foo.stubs.system", + "bar.stubs.system", + ], + sdk_version: "system_current", + } + `) +}