From 4762436a8dbb3d791d91c1b9e575c79e8df2180f Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Wed, 20 May 2020 12:19:10 +0100 Subject: [PATCH 1/5] Retry: "java_sdk_library: Extract common stubs redirect code" The java_sdk_library and java_sdk_library_import redirect a request for header and implementation jars to broadly the same place although the java_sdk_library does have some special code that is mixed in with the common code. This change separates the java_sdk_library special code from the common code and moves the common code into its own method for use by both module types. That makes the special behavior clearer and ensures the common behavior remains consistent in future. Test: m nothing Bug: 155164730 (cherry picked from commit b05d4295de0883cb1e31c04971a6c5dbea719ed1) Change-Id: Id21cc74d2d37c5a75983fdcd7393f6a37f8872c9 --- java/sdk_library.go | 89 ++++++++++++++++++++------------------------- 1 file changed, 39 insertions(+), 50 deletions(-) diff --git a/java/sdk_library.go b/java/sdk_library.go index e85b692a1..e46460e51 100644 --- a/java/sdk_library.go +++ b/java/sdk_library.go @@ -564,6 +564,31 @@ func (c *commonToSdkLibraryAndImport) getScopePaths(scope *apiScope) *scopePaths return paths } +func (c *commonToSdkLibraryAndImport) sdkJarsCommon(ctx android.BaseModuleContext, sdkVersion sdkSpec, headerJars bool) android.Paths { + + // If a specific numeric version has been requested then use prebuilt versions of the sdk. + if sdkVersion.version.isNumbered() { + return PrebuiltJars(ctx, c.moduleBase.BaseModuleName(), sdkVersion) + } + + var apiScope *apiScope + switch sdkVersion.kind { + case sdkSystem: + apiScope = apiScopeSystem + case sdkTest: + apiScope = apiScopeTest + default: + apiScope = apiScopePublic + } + + paths := c.getScopePaths(apiScope) + if headerJars { + return paths.stubsHeaderPath + } else { + return paths.stubsImplPath + } +} + type SdkLibrary struct { Library @@ -1015,41 +1040,20 @@ func PrebuiltJars(ctx android.BaseModuleContext, baseName string, s sdkSpec) and return android.Paths{jarPath.Path()} } -func (module *SdkLibrary) sdkJars( - ctx android.BaseModuleContext, - sdkVersion sdkSpec, - headerJars bool) android.Paths { +func (module *SdkLibrary) sdkJars(ctx android.BaseModuleContext, sdkVersion sdkSpec, headerJars bool) android.Paths { - // If a specific numeric version has been requested then use prebuilt versions of the sdk. - if sdkVersion.version.isNumbered() { - return PrebuiltJars(ctx, module.BaseModuleName(), sdkVersion) - } else { - if !sdkVersion.specified() { - if headerJars { - return module.HeaderJars() - } else { - return module.ImplementationJars() - } - } - var apiScope *apiScope - switch sdkVersion.kind { - case sdkSystem: - apiScope = apiScopeSystem - case sdkTest: - apiScope = apiScopeTest - case sdkPrivate: - return module.HeaderJars() - default: - apiScope = apiScopePublic - } - - paths := module.getScopePaths(apiScope) + // Check any special cases for java_sdk_library. + if !sdkVersion.specified() { if headerJars { - return paths.stubsHeaderPath + return module.HeaderJars() } else { - return paths.stubsImplPath + return module.ImplementationJars() } + } else if sdkVersion.kind == sdkPrivate { + return module.HeaderJars() } + + return module.sdkJarsCommon(ctx, sdkVersion, headerJars) } // to satisfy SdkLibraryDependency interface @@ -1463,27 +1467,12 @@ func (module *sdkLibraryImport) GenerateAndroidBuildActions(ctx android.ModuleCo }) } -func (module *sdkLibraryImport) sdkJars( - ctx android.BaseModuleContext, - sdkVersion sdkSpec) android.Paths { +func (module *sdkLibraryImport) sdkJars(ctx android.BaseModuleContext, sdkVersion sdkSpec) android.Paths { - // If a specific numeric version has been requested then use prebuilt versions of the sdk. - if sdkVersion.version.isNumbered() { - return PrebuiltJars(ctx, module.BaseModuleName(), sdkVersion) - } - - var apiScope *apiScope - switch sdkVersion.kind { - case sdkSystem: - apiScope = apiScopeSystem - case sdkTest: - apiScope = apiScopeTest - default: - apiScope = apiScopePublic - } - - paths := module.getScopePaths(apiScope) - return paths.stubsHeaderPath + // The java_sdk_library_import can only ever give back header jars as it does not + // have an implementation jar. + headerJars := true + return module.sdkJarsCommon(ctx, sdkVersion, headerJars) } // to satisfy SdkLibraryDependency interface From 5ae30792b192e256008f1f5bec5977c2d1d0a5c8 Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Wed, 20 May 2020 11:52:25 +0100 Subject: [PATCH 2/5] Retry: "java_sdk_library: Add redirection to module-lib stubs" Previously, when using sdk_version: "module_current" any direct reference to an sdk library would use the public not module-lib stubs. This change corrects that. Prior to the addition of the module-lib api scope almost all java_sdk_library instances supported all the scopes to which a request for jars could be redirected, i.e. public, system and test. The exceptions to that are a few special instances that were used with sdk_version: "none" and so were either caught by the java_sdk_library special cases or dropped through to public. The addition of module-lib, plus the flexible control over which scopes are generated means that is no longer true. It is possible for a java_sdk_library to be requested for a scope it does not have which would have resulted in an empty set of paths being returned leading to confusing compilation errors. To avoid that this change also adds support for using the inheritance hierarchy defined by the apiScope.extends field to fall back to the closest available surface. Test: m nothing Bug: 155164730 (cherry picked from commit 803a9565cd3d85ba22fb00f870b8fffabc3ab30d) Change-Id: I89a0abf4033209f9a104285c990096bc0b4a0255 --- java/java_test.go | 39 ++++++++++++++++++++++++++++++++++++ java/sdk_library.go | 49 ++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 83 insertions(+), 5 deletions(-) diff --git a/java/java_test.go b/java/java_test.go index 01ddccfc1..b8abacb59 100644 --- a/java/java_test.go +++ b/java/java_test.go @@ -1261,6 +1261,45 @@ func TestJavaSdkLibrary_SdkVersion_ForScope(t *testing.T) { `) } +func TestJavaSdkLibrary_MissingScope(t *testing.T) { + testJavaError(t, `requires api scope module-lib from foo but it only has \[\] available`, ` + java_sdk_library { + name: "foo", + srcs: ["a.java"], + public: { + enabled: false, + }, + } + + java_library { + name: "baz", + srcs: ["a.java"], + libs: ["foo"], + sdk_version: "module_current", + } + `) +} + +func TestJavaSdkLibrary_FallbackScope(t *testing.T) { + testJava(t, ` + java_sdk_library { + name: "foo", + srcs: ["a.java"], + system: { + enabled: true, + }, + } + + java_library { + name: "baz", + srcs: ["a.java"], + libs: ["foo"], + // foo does not have module-lib scope so it should fallback to system + sdk_version: "module_current", + } + `) +} + var compilerFlagsTestCases = []struct { in string out bool diff --git a/java/sdk_library.go b/java/sdk_library.go index e46460e51..1fff3ba34 100644 --- a/java/sdk_library.go +++ b/java/sdk_library.go @@ -551,7 +551,7 @@ func (c *commonToSdkLibraryAndImport) apiModuleName(apiScope *apiScope) string { return c.namingScheme.apiModuleName(apiScope, c.moduleBase.BaseModuleName()) } -func (c *commonToSdkLibraryAndImport) getScopePaths(scope *apiScope) *scopePaths { +func (c *commonToSdkLibraryAndImport) getScopePathsCreateIfNeeded(scope *apiScope) *scopePaths { if c.scopePaths == nil { c.scopePaths = make(map[*apiScope]*scopePaths) } @@ -564,6 +564,28 @@ func (c *commonToSdkLibraryAndImport) getScopePaths(scope *apiScope) *scopePaths return paths } +func (c *commonToSdkLibraryAndImport) findScopePaths(scope *apiScope) *scopePaths { + if c.scopePaths == nil { + return nil + } + + return c.scopePaths[scope] +} + +// If this does not support the requested api scope then find the closest available +// scope it does support. Returns nil if no such scope is available. +func (c *commonToSdkLibraryAndImport) findClosestScopePath(scope *apiScope) *scopePaths { + for s := scope; s != nil; s = s.extends { + if paths := c.findScopePaths(s); paths != nil { + return paths + } + } + + // This should never happen outside tests as public should be the base scope for every + // scope and is enabled by default. + return nil +} + func (c *commonToSdkLibraryAndImport) sdkJarsCommon(ctx android.BaseModuleContext, sdkVersion sdkSpec, headerJars bool) android.Paths { // If a specific numeric version has been requested then use prebuilt versions of the sdk. @@ -575,13 +597,26 @@ func (c *commonToSdkLibraryAndImport) sdkJarsCommon(ctx android.BaseModuleContex switch sdkVersion.kind { case sdkSystem: apiScope = apiScopeSystem + case sdkModule: + apiScope = apiScopeModuleLib case sdkTest: apiScope = apiScopeTest default: apiScope = apiScopePublic } - paths := c.getScopePaths(apiScope) + paths := c.findClosestScopePath(apiScope) + if paths == nil { + var scopes []string + for _, s := range allApiScopes { + if c.findScopePaths(s) != nil { + scopes = append(scopes, s.name) + } + } + ctx.ModuleErrorf("requires api scope %s from %s but it only has %q available", apiScope.name, c.moduleBase.BaseModuleName(), scopes) + return nil + } + if headerJars { return paths.stubsHeaderPath } else { @@ -704,7 +739,7 @@ func (module *SdkLibrary) GenerateAndroidBuildActions(ctx android.ModuleContext) // Extract information from any of the scope specific dependencies. if scopeTag, ok := tag.(scopeDependencyTag); ok { apiScope := scopeTag.apiScope - scopePaths := module.getScopePaths(apiScope) + scopePaths := module.getScopePathsCreateIfNeeded(apiScope) // Extract information from the dependency. The exact information extracted // is determined by the nature of the dependency which is determined by the tag. @@ -1460,7 +1495,7 @@ func (module *sdkLibraryImport) GenerateAndroidBuildActions(ctx android.ModuleCo if lib, ok := to.(Dependency); ok { if scopeTag, ok := tag.(scopeDependencyTag); ok { apiScope := scopeTag.apiScope - scopePaths := module.getScopePaths(apiScope) + scopePaths := module.getScopePathsCreateIfNeeded(apiScope) scopePaths.stubsHeaderPath = lib.HeaderJars() } } @@ -1645,7 +1680,11 @@ func (s *sdkLibrarySdkMemberProperties) PopulateFromVariant(ctx android.SdkMembe s.Scopes = make(map[*apiScope]scopeProperties) for _, apiScope := range allApiScopes { - paths := sdk.getScopePaths(apiScope) + paths := sdk.findScopePaths(apiScope) + if paths == nil { + continue + } + jars := paths.stubsImplPath if len(jars) > 0 { properties := scopeProperties{} From 533f9c78cd61c35859f8a9e194b799f46b1df90f Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Wed, 20 May 2020 16:18:00 +0100 Subject: [PATCH 3/5] Retry: "java_sdk_library: Improve consistency with ..._import" The scopePaths struct is used by both java_sdk_library and its prebuilt but was not populated in the same way. This change addresses those discrepancies in preparation for a follow up change which will allow access to some of those fields through OutputFileProvider. Changes: * Document the scopePaths field and struct. * Switch those fields that may not be fully populated from Paths to OptionalPath to make that 100% clear and protect against unchecked use. * Switch java_sdk_library_import to use the dependency extraction mechanism driven by the dependency tag. This should actually have been part of the change that added that mechanism. * Only create prebuilt_stubs_sources if sources have been provided. * Add dependencies from java_sdk_library_import on its stubs source child modules if sources have been provided. That will ensure the stubsSrcJar field is updated. * Updates current/removedApiFilePath if provided for the scope in java_sdk_library_import. * Extracts ApiStubsSrcProvider from ApiStubsProvider to allow it to be implemented by PrebuiltStubsSources so that it can provide access to the stubs src jar that it creates. Test: m nothing Bug: 148080325 Bug: 155164730 (cherry picked from commit 0f8faffdc0e039bb73305d2a53a2d17029cdb469) Change-Id: Ib56ca608d6fc1357d3d89e9c4cfed6ff8da11735 --- java/droiddoc.go | 11 +++++- java/java_test.go | 1 + java/sdk_library.go | 93 ++++++++++++++++++++++++++++++++++----------- 3 files changed, 81 insertions(+), 24 deletions(-) diff --git a/java/droiddoc.go b/java/droiddoc.go index 4b4a245e7..c1cecb64a 100644 --- a/java/droiddoc.go +++ b/java/droiddoc.go @@ -350,11 +350,16 @@ type ApiFilePath interface { ApiFilePath() android.Path } +type ApiStubsSrcProvider interface { + StubsSrcJar() android.Path +} + // Provider of information about API stubs, used by java_sdk_library. type ApiStubsProvider interface { ApiFilePath RemovedApiFilePath() android.Path - StubsSrcJar() android.Path + + ApiStubsSrcProvider } // @@ -1912,6 +1917,10 @@ func (p *PrebuiltStubsSources) OutputFiles(tag string) (android.Paths, error) { } } +func (d *PrebuiltStubsSources) StubsSrcJar() android.Path { + return d.stubsSrcJar +} + func (p *PrebuiltStubsSources) GenerateAndroidBuildActions(ctx android.ModuleContext) { p.stubsSrcJar = android.PathForModuleOut(ctx, ctx.ModuleName()+"-"+"stubs.srcjar") diff --git a/java/java_test.go b/java/java_test.go index b8abacb59..16200119e 100644 --- a/java/java_test.go +++ b/java/java_test.go @@ -575,6 +575,7 @@ func TestJavaSdkLibraryImport(t *testing.T) { }, test: { jars: ["c.jar"], + stub_srcs: ["c.java"], }, } `) diff --git a/java/sdk_library.go b/java/sdk_library.go index 1fff3ba34..c91091ffc 100644 --- a/java/sdk_library.go +++ b/java/sdk_library.go @@ -433,12 +433,30 @@ type sdkLibraryProperties struct { //Html_doc *bool } +// Paths to outputs from java_sdk_library and java_sdk_library_import. +// +// Fields that are android.Paths are always set (during GenerateAndroidBuildActions). +// OptionalPaths are always set by java_sdk_library but may not be set by +// java_sdk_library_import as not all instances provide that information. type scopePaths struct { - stubsHeaderPath android.Paths - stubsImplPath android.Paths - currentApiFilePath android.Path - removedApiFilePath android.Path - stubsSrcJar android.Path + // The path (represented as Paths for convenience when returning) to the stubs header jar. + // + // That is the jar that is created by turbine. + stubsHeaderPath android.Paths + + // The path (represented as Paths for convenience when returning) to the stubs implementation jar. + // + // This is not the implementation jar, it still only contains stubs. + stubsImplPath android.Paths + + // The API specification file, e.g. system_current.txt. + currentApiFilePath android.OptionalPath + + // The specification of API elements removed since the last release. + removedApiFilePath android.OptionalPath + + // The stubs source jar. + stubsSrcJar android.OptionalPath } func (paths *scopePaths) extractStubsLibraryInfoFromDependency(dep android.Module) error { @@ -460,9 +478,18 @@ func (paths *scopePaths) treatDepAsApiStubsProvider(dep android.Module, action f } } +func (paths *scopePaths) treatDepAsApiStubsSrcProvider(dep android.Module, action func(provider ApiStubsSrcProvider)) error { + if apiStubsProvider, ok := dep.(ApiStubsSrcProvider); ok { + action(apiStubsProvider) + return nil + } else { + return fmt.Errorf("expected module that implements ApiStubsSrcProvider, e.g. droidstubs") + } +} + func (paths *scopePaths) extractApiInfoFromApiStubsProvider(provider ApiStubsProvider) { - paths.currentApiFilePath = provider.ApiFilePath() - paths.removedApiFilePath = provider.RemovedApiFilePath() + paths.currentApiFilePath = android.OptionalPathForPath(provider.ApiFilePath()) + paths.removedApiFilePath = android.OptionalPathForPath(provider.RemovedApiFilePath()) } func (paths *scopePaths) extractApiInfoFromDep(dep android.Module) error { @@ -471,12 +498,12 @@ func (paths *scopePaths) extractApiInfoFromDep(dep android.Module) error { }) } -func (paths *scopePaths) extractStubsSourceInfoFromApiStubsProviders(provider ApiStubsProvider) { - paths.stubsSrcJar = provider.StubsSrcJar() +func (paths *scopePaths) extractStubsSourceInfoFromApiStubsProviders(provider ApiStubsSrcProvider) { + paths.stubsSrcJar = android.OptionalPathForPath(provider.StubsSrcJar()) } func (paths *scopePaths) extractStubsSourceInfoFromDep(dep android.Module) error { - return paths.treatDepAsApiStubsProvider(dep, func(provider ApiStubsProvider) { + return paths.treatDepAsApiStubsSrcProvider(dep, func(provider ApiStubsSrcProvider) { paths.extractStubsSourceInfoFromApiStubsProviders(provider) }) } @@ -1319,10 +1346,10 @@ type sdkLibraryScopeProperties struct { Stub_srcs []string `android:"path"` // The current.txt - Current_api string `android:"path"` + Current_api *string `android:"path"` // The removed.txt - Removed_api string `android:"path"` + Removed_api *string `android:"path"` } type sdkLibraryImportProperties struct { @@ -1432,7 +1459,9 @@ func (module *sdkLibraryImport) createInternalModules(mctx android.DefaultableHo module.createJavaImportForStubs(mctx, apiScope, scopeProperties) - module.createPrebuiltStubsSources(mctx, apiScope, scopeProperties) + if len(scopeProperties.Stub_srcs) > 0 { + module.createPrebuiltStubsSources(mctx, apiScope, scopeProperties) + } } javaSdkLibraries := javaSdkLibraries(mctx.Config()) @@ -1484,22 +1513,40 @@ func (module *sdkLibraryImport) DepsMutator(ctx android.BottomUpMutatorContext) // Add dependencies to the prebuilt stubs library ctx.AddVariationDependencies(nil, apiScope.stubsTag, module.stubsLibraryModuleName(apiScope)) + + if len(scopeProperties.Stub_srcs) > 0 { + // Add dependencies to the prebuilt stubs source library + ctx.AddVariationDependencies(nil, apiScope.stubsSourceTag, module.stubsSourceModuleName(apiScope)) + } } } func (module *sdkLibraryImport) GenerateAndroidBuildActions(ctx android.ModuleContext) { - // Record the paths to the prebuilt stubs library. + // Record the paths to the prebuilt stubs library and stubs source. ctx.VisitDirectDeps(func(to android.Module) { tag := ctx.OtherModuleDependencyTag(to) - if lib, ok := to.(Dependency); ok { - if scopeTag, ok := tag.(scopeDependencyTag); ok { - apiScope := scopeTag.apiScope - scopePaths := module.getScopePathsCreateIfNeeded(apiScope) - scopePaths.stubsHeaderPath = lib.HeaderJars() - } + // Extract information from any of the scope specific dependencies. + if scopeTag, ok := tag.(scopeDependencyTag); ok { + apiScope := scopeTag.apiScope + scopePaths := module.getScopePathsCreateIfNeeded(apiScope) + + // Extract information from the dependency. The exact information extracted + // is determined by the nature of the dependency which is determined by the tag. + scopeTag.extractDepInfo(ctx, to, scopePaths) } }) + + // Populate the scope paths with information from the properties. + for apiScope, scopeProperties := range module.scopeProperties { + if len(scopeProperties.Jars) == 0 { + continue + } + + paths := module.getScopePathsCreateIfNeeded(apiScope) + paths.currentApiFilePath = android.OptionalPathForModuleSrc(ctx, scopeProperties.Current_api) + paths.removedApiFilePath = android.OptionalPathForModuleSrc(ctx, scopeProperties.Removed_api) + } } func (module *sdkLibraryImport) sdkJars(ctx android.BaseModuleContext, sdkVersion sdkSpec) android.Paths { @@ -1690,9 +1737,9 @@ func (s *sdkLibrarySdkMemberProperties) PopulateFromVariant(ctx android.SdkMembe properties := scopeProperties{} properties.Jars = jars properties.SdkVersion = sdk.sdkVersionForStubsLibrary(ctx.SdkModuleContext(), apiScope) - properties.StubsSrcJar = paths.stubsSrcJar - properties.CurrentApiFile = paths.currentApiFilePath - properties.RemovedApiFile = paths.removedApiFilePath + properties.StubsSrcJar = paths.stubsSrcJar.Path() + properties.CurrentApiFile = paths.currentApiFilePath.Path() + properties.RemovedApiFile = paths.removedApiFilePath.Path() s.Scopes[apiScope] = properties } } From 46fdda872f19689be7c7356e94e0ff0db48638d8 Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Thu, 14 May 2020 15:39:10 +0100 Subject: [PATCH 4/5] Retry: "java_sdk_library: Access outputs using tags" Previously, in order to access say the public stubs source jar it was necessary to directly reference the module by name. This change adds support for accessing them indirectly via tags. Test: nothing Bug: 155164730 (cherry picked from commit 46dc45aba9c60f8b6f90005f41dcbd2f771756e1) Change-Id: Icc21caee5b4b1da6f958470bd7bae8a37933f24b --- java/java_test.go | 107 ++++++++++++++++++++++++++++++++++++++++++++ java/sdk_library.go | 94 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 201 insertions(+) diff --git a/java/java_test.go b/java/java_test.go index 16200119e..4f3a803ff 100644 --- a/java/java_test.go +++ b/java/java_test.go @@ -1230,6 +1230,113 @@ func TestJavaSdkLibrary(t *testing.T) { } } +func TestJavaSdkLibrary_UseSourcesFromAnotherSdkLibrary(t *testing.T) { + testJava(t, ` + java_sdk_library { + name: "foo", + srcs: ["a.java"], + api_packages: ["foo"], + public: { + enabled: true, + }, + } + + java_library { + name: "bar", + srcs: ["b.java", ":foo{.public.stubs.source}"], + } + `) +} + +func TestJavaSdkLibrary_AccessOutputFiles_MissingScope(t *testing.T) { + testJavaError(t, `"foo" does not provide api scope system`, ` + java_sdk_library { + name: "foo", + srcs: ["a.java"], + api_packages: ["foo"], + public: { + enabled: true, + }, + } + + java_library { + name: "bar", + srcs: ["b.java", ":foo{.system.stubs.source}"], + } + `) +} + +func TestJavaSdkLibraryImport_AccessOutputFiles(t *testing.T) { + testJava(t, ` + java_sdk_library_import { + name: "foo", + public: { + jars: ["a.jar"], + stub_srcs: ["a.java"], + current_api: "api/current.txt", + removed_api: "api/removed.txt", + }, + } + + java_library { + name: "bar", + srcs: [":foo{.public.stubs.source}"], + java_resources: [ + ":foo{.public.api.txt}", + ":foo{.public.removed-api.txt}", + ], + } + `) +} + +func TestJavaSdkLibraryImport_AccessOutputFiles_Invalid(t *testing.T) { + bp := ` + java_sdk_library_import { + name: "foo", + public: { + jars: ["a.jar"], + }, + } + ` + + t.Run("stubs.source", func(t *testing.T) { + testJavaError(t, `stubs.source not available for api scope public`, bp+` + java_library { + name: "bar", + srcs: [":foo{.public.stubs.source}"], + java_resources: [ + ":foo{.public.api.txt}", + ":foo{.public.removed-api.txt}", + ], + } + `) + }) + + t.Run("api.txt", func(t *testing.T) { + testJavaError(t, `api.txt not available for api scope public`, bp+` + java_library { + name: "bar", + srcs: ["a.java"], + java_resources: [ + ":foo{.public.api.txt}", + ], + } + `) + }) + + t.Run("removed-api.txt", func(t *testing.T) { + testJavaError(t, `removed-api.txt not available for api scope public`, bp+` + java_library { + name: "bar", + srcs: ["a.java"], + java_resources: [ + ":foo{.public.removed-api.txt}", + ], + } + `) + }) +} + func TestJavaSdkLibrary_InvalidScopes(t *testing.T) { testJavaError(t, `module "foo": enabled api scope "system" depends on disabled scope "public"`, ` java_sdk_library { diff --git a/java/sdk_library.go b/java/sdk_library.go index c91091ffc..a95da9f02 100644 --- a/java/sdk_library.go +++ b/java/sdk_library.go @@ -19,6 +19,7 @@ import ( "path" "path/filepath" "reflect" + "regexp" "sort" "strings" "sync" @@ -145,6 +146,8 @@ type apiScope struct { // Initialize a scope, creating and adding appropriate dependency tags func initApiScope(scope *apiScope) *apiScope { name := scope.name + scopeByName[name] = scope + allScopeNames = append(allScopeNames, name) scope.propertyName = strings.ReplaceAll(name, "-", "_") scope.fieldName = proptools.FieldNameForProperty(scope.propertyName) scope.stubsTag = scopeDependencyTag{ @@ -217,6 +220,8 @@ func (scopes apiScopes) Strings(accessor func(*apiScope) string) []string { } var ( + scopeByName = make(map[string]*apiScope) + allScopeNames []string apiScopePublic = initApiScope(&apiScope{ name: "public", @@ -578,6 +583,82 @@ func (c *commonToSdkLibraryAndImport) apiModuleName(apiScope *apiScope) string { return c.namingScheme.apiModuleName(apiScope, c.moduleBase.BaseModuleName()) } +// The component names for different outputs of the java_sdk_library. +// +// They are similar to the names used for the child modules it creates +const ( + stubsSourceComponentName = "stubs.source" + + apiTxtComponentName = "api.txt" + + removedApiTxtComponentName = "removed-api.txt" +) + +// A regular expression to match tags that reference a specific stubs component. +// +// It will only match if given a valid scope and a valid component. It is verfy strict +// to ensure it does not accidentally match a similar looking tag that should be processed +// by the embedded Library. +var tagSplitter = func() *regexp.Regexp { + // Given a list of literal string items returns a regular expression that will + // match any one of the items. + choice := func(items ...string) string { + return `\Q` + strings.Join(items, `\E|\Q`) + `\E` + } + + // Regular expression to match one of the scopes. + scopesRegexp := choice(allScopeNames...) + + // Regular expression to match one of the components. + componentsRegexp := choice(stubsSourceComponentName, apiTxtComponentName, removedApiTxtComponentName) + + // Regular expression to match any combination of one scope and one component. + return regexp.MustCompile(fmt.Sprintf(`^\.(%s)\.(%s)$`, scopesRegexp, componentsRegexp)) +}() + +// For OutputFileProducer interface +// +// ..stubs.source +// ..api.txt +// ..removed-api.txt +func (c *commonToSdkLibraryAndImport) commonOutputFiles(tag string) (android.Paths, error) { + if groups := tagSplitter.FindStringSubmatch(tag); groups != nil { + scopeName := groups[1] + component := groups[2] + + if scope, ok := scopeByName[scopeName]; ok { + paths := c.findScopePaths(scope) + if paths == nil { + return nil, fmt.Errorf("%q does not provide api scope %s", c.moduleBase.BaseModuleName(), scopeName) + } + + switch component { + case stubsSourceComponentName: + if paths.stubsSrcJar.Valid() { + return android.Paths{paths.stubsSrcJar.Path()}, nil + } + + case apiTxtComponentName: + if paths.currentApiFilePath.Valid() { + return android.Paths{paths.currentApiFilePath.Path()}, nil + } + + case removedApiTxtComponentName: + if paths.removedApiFilePath.Valid() { + return android.Paths{paths.removedApiFilePath.Path()}, nil + } + } + + return nil, fmt.Errorf("%s not available for api scope %s", component, scopeName) + } else { + return nil, fmt.Errorf("unknown scope %s in %s", scope, tag) + } + + } else { + return nil, nil + } +} + func (c *commonToSdkLibraryAndImport) getScopePathsCreateIfNeeded(scope *apiScope) *scopePaths { if c.scopePaths == nil { c.scopePaths = make(map[*apiScope]*scopePaths) @@ -751,6 +832,15 @@ func (module *SdkLibrary) DepsMutator(ctx android.BottomUpMutatorContext) { module.Library.deps(ctx) } +func (module *SdkLibrary) OutputFiles(tag string) (android.Paths, error) { + paths, err := module.commonOutputFiles(tag) + if paths == nil && err == nil { + return module.Library.OutputFiles(tag) + } else { + return paths, err + } +} + func (module *SdkLibrary) GenerateAndroidBuildActions(ctx android.ModuleContext) { // Don't build an implementation library if this is api only. if !proptools.Bool(module.sdkLibraryProperties.Api_only) { @@ -1521,6 +1611,10 @@ func (module *sdkLibraryImport) DepsMutator(ctx android.BottomUpMutatorContext) } } +func (module *sdkLibraryImport) OutputFiles(tag string) (android.Paths, error) { + return module.commonOutputFiles(tag) +} + func (module *sdkLibraryImport) GenerateAndroidBuildActions(ctx android.ModuleContext) { // Record the paths to the prebuilt stubs library and stubs source. ctx.VisitDirectDeps(func(to android.Module) { From a3fb67dd92d04ae8d95ea19d5563e2e277a3fa22 Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Wed, 20 May 2020 14:20:02 +0100 Subject: [PATCH 5/5] Retry: "java_sdk_library: Do not expose stubs implementation jar" The stubs header jar is optimized for use as a dependency for others to use. It only changes if there is a significant difference in the externals of the classes, i.e. anything that a library being compiled against depends upon. So changes to implementations of method or the addition/removal of private methods, fields will have no impact. As there is no benefit in returning the implementation of the stubs jar this change removes it. The implementation is still used when taking a snapshot as the header jar is an internal build artefact that is not suitable for long term snapshot. Bug: 155164730 Test: m droid (cherry picked from commit 23970f4285f2ba70b5a35a56a6dffa9afc1f37d9) Change-Id: I8277ec643837514d74cb57ad4f236ceb1f5c6c5b --- java/sdk_library.go | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/java/sdk_library.go b/java/sdk_library.go index a95da9f02..b215a76f7 100644 --- a/java/sdk_library.go +++ b/java/sdk_library.go @@ -694,7 +694,7 @@ func (c *commonToSdkLibraryAndImport) findClosestScopePath(scope *apiScope) *sco return nil } -func (c *commonToSdkLibraryAndImport) sdkJarsCommon(ctx android.BaseModuleContext, sdkVersion sdkSpec, headerJars bool) android.Paths { +func (c *commonToSdkLibraryAndImport) selectHeaderJarsForSdkVersion(ctx android.BaseModuleContext, sdkVersion sdkSpec) android.Paths { // If a specific numeric version has been requested then use prebuilt versions of the sdk. if sdkVersion.version.isNumbered() { @@ -725,11 +725,7 @@ func (c *commonToSdkLibraryAndImport) sdkJarsCommon(ctx android.BaseModuleContex return nil } - if headerJars { - return paths.stubsHeaderPath - } else { - return paths.stubsImplPath - } + return paths.stubsHeaderPath } type SdkLibrary struct { @@ -1205,7 +1201,7 @@ func (module *SdkLibrary) sdkJars(ctx android.BaseModuleContext, sdkVersion sdkS return module.HeaderJars() } - return module.sdkJarsCommon(ctx, sdkVersion, headerJars) + return module.selectHeaderJarsForSdkVersion(ctx, sdkVersion) } // to satisfy SdkLibraryDependency interface @@ -1644,11 +1640,7 @@ func (module *sdkLibraryImport) GenerateAndroidBuildActions(ctx android.ModuleCo } func (module *sdkLibraryImport) sdkJars(ctx android.BaseModuleContext, sdkVersion sdkSpec) android.Paths { - - // The java_sdk_library_import can only ever give back header jars as it does not - // have an implementation jar. - headerJars := true - return module.sdkJarsCommon(ctx, sdkVersion, headerJars) + return module.selectHeaderJarsForSdkVersion(ctx, sdkVersion) } // to satisfy SdkLibraryDependency interface