From ab5ac8f1696a6272c67906968478880d955e7c2d Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Wed, 18 Nov 2020 16:37:35 +0000 Subject: [PATCH 1/2] Revert "Use glob for java_sdk_library_import stub_srcs" This reverts commit 7f97957ded358b555e09dfbd607c99c90f6c935b. Reason for revert: breaks sdk snapshots b/173508731 Bug: 173508731 Test: Ran prebuilts/runtime/update.py and then m nothing Before revert it failed After revert it worked Change-Id: I9c081681fac589e37788a0d592435e3224011c58 --- java/sdk_library.go | 5 ++-- sdk/java_sdk_test.go | 56 ++++++++++++++++++++++---------------------- 2 files changed, 30 insertions(+), 31 deletions(-) diff --git a/java/sdk_library.go b/java/sdk_library.go index 13cd5f976..4369ffcf7 100644 --- a/java/sdk_library.go +++ b/java/sdk_library.go @@ -2300,11 +2300,10 @@ func (s *sdkLibrarySdkMemberProperties) AddToPropertySet(ctx android.SdkMemberCo scopeSet.AddProperty("jars", jars) // Merge the stubs source jar into the snapshot zip so that when it is unpacked - // the source files are also unpacked. Use a glob so that if the directory is missing - // (because there are no stubs sources for this scope) it will not fail. + // the source files are also unpacked. snapshotRelativeDir := filepath.Join(scopeDir, ctx.Name()+"_stub_sources") ctx.SnapshotBuilder().UnzipToSnapshot(properties.StubsSrcJar, snapshotRelativeDir) - scopeSet.AddProperty("stub_srcs", []string{snapshotRelativeDir + "/**/*.java"}) + scopeSet.AddProperty("stub_srcs", []string{snapshotRelativeDir}) if properties.CurrentApiFile != nil { currentApiSnapshotPath := filepath.Join(scopeDir, ctx.Name()+".txt") diff --git a/sdk/java_sdk_test.go b/sdk/java_sdk_test.go index ec8ebb3b4..b44f66e85 100644 --- a/sdk/java_sdk_test.go +++ b/sdk/java_sdk_test.go @@ -976,21 +976,21 @@ java_sdk_library_import { shared_library: false, public: { jars: ["sdk_library/public/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/public/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/public/myjavalib_stub_sources"], current_api: "sdk_library/public/myjavalib.txt", removed_api: "sdk_library/public/myjavalib-removed.txt", sdk_version: "current", }, system: { jars: ["sdk_library/system/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/system/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/system/myjavalib_stub_sources"], current_api: "sdk_library/system/myjavalib.txt", removed_api: "sdk_library/system/myjavalib-removed.txt", sdk_version: "system_current", }, test: { jars: ["sdk_library/test/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/test/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/test/myjavalib_stub_sources"], current_api: "sdk_library/test/myjavalib.txt", removed_api: "sdk_library/test/myjavalib-removed.txt", sdk_version: "test_current", @@ -1005,21 +1005,21 @@ java_sdk_library_import { shared_library: false, public: { jars: ["sdk_library/public/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/public/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/public/myjavalib_stub_sources"], current_api: "sdk_library/public/myjavalib.txt", removed_api: "sdk_library/public/myjavalib-removed.txt", sdk_version: "current", }, system: { jars: ["sdk_library/system/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/system/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/system/myjavalib_stub_sources"], current_api: "sdk_library/system/myjavalib.txt", removed_api: "sdk_library/system/myjavalib-removed.txt", sdk_version: "system_current", }, test: { jars: ["sdk_library/test/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/test/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/test/myjavalib_stub_sources"], current_api: "sdk_library/test/myjavalib.txt", removed_api: "sdk_library/test/myjavalib-removed.txt", sdk_version: "test_current", @@ -1077,7 +1077,7 @@ java_sdk_library_import { shared_library: true, public: { jars: ["sdk_library/public/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/public/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/public/myjavalib_stub_sources"], current_api: "sdk_library/public/myjavalib.txt", removed_api: "sdk_library/public/myjavalib-removed.txt", sdk_version: "none", @@ -1092,7 +1092,7 @@ java_sdk_library_import { shared_library: true, public: { jars: ["sdk_library/public/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/public/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/public/myjavalib_stub_sources"], current_api: "sdk_library/public/myjavalib.txt", removed_api: "sdk_library/public/myjavalib-removed.txt", sdk_version: "none", @@ -1146,7 +1146,7 @@ java_sdk_library_import { shared_library: true, public: { jars: ["sdk_library/public/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/public/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/public/myjavalib_stub_sources"], current_api: "sdk_library/public/myjavalib.txt", removed_api: "sdk_library/public/myjavalib-removed.txt", sdk_version: "module_current", @@ -1161,7 +1161,7 @@ java_sdk_library_import { shared_library: true, public: { jars: ["sdk_library/public/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/public/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/public/myjavalib_stub_sources"], current_api: "sdk_library/public/myjavalib.txt", removed_api: "sdk_library/public/myjavalib-removed.txt", sdk_version: "module_current", @@ -1218,14 +1218,14 @@ java_sdk_library_import { shared_library: true, public: { jars: ["sdk_library/public/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/public/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/public/myjavalib_stub_sources"], current_api: "sdk_library/public/myjavalib.txt", removed_api: "sdk_library/public/myjavalib-removed.txt", sdk_version: "current", }, system: { jars: ["sdk_library/system/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/system/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/system/myjavalib_stub_sources"], current_api: "sdk_library/system/myjavalib.txt", removed_api: "sdk_library/system/myjavalib-removed.txt", sdk_version: "system_current", @@ -1240,14 +1240,14 @@ java_sdk_library_import { shared_library: true, public: { jars: ["sdk_library/public/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/public/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/public/myjavalib_stub_sources"], current_api: "sdk_library/public/myjavalib.txt", removed_api: "sdk_library/public/myjavalib-removed.txt", sdk_version: "current", }, system: { jars: ["sdk_library/system/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/system/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/system/myjavalib_stub_sources"], current_api: "sdk_library/system/myjavalib.txt", removed_api: "sdk_library/system/myjavalib-removed.txt", sdk_version: "system_current", @@ -1311,21 +1311,21 @@ java_sdk_library_import { shared_library: true, public: { jars: ["sdk_library/public/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/public/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/public/myjavalib_stub_sources"], current_api: "sdk_library/public/myjavalib.txt", removed_api: "sdk_library/public/myjavalib-removed.txt", sdk_version: "current", }, system: { jars: ["sdk_library/system/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/system/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/system/myjavalib_stub_sources"], current_api: "sdk_library/system/myjavalib.txt", removed_api: "sdk_library/system/myjavalib-removed.txt", sdk_version: "system_current", }, module_lib: { jars: ["sdk_library/module-lib/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/module-lib/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/module-lib/myjavalib_stub_sources"], current_api: "sdk_library/module-lib/myjavalib.txt", removed_api: "sdk_library/module-lib/myjavalib-removed.txt", sdk_version: "module_current", @@ -1340,21 +1340,21 @@ java_sdk_library_import { shared_library: true, public: { jars: ["sdk_library/public/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/public/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/public/myjavalib_stub_sources"], current_api: "sdk_library/public/myjavalib.txt", removed_api: "sdk_library/public/myjavalib-removed.txt", sdk_version: "current", }, system: { jars: ["sdk_library/system/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/system/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/system/myjavalib_stub_sources"], current_api: "sdk_library/system/myjavalib.txt", removed_api: "sdk_library/system/myjavalib-removed.txt", sdk_version: "system_current", }, module_lib: { jars: ["sdk_library/module-lib/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/module-lib/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/module-lib/myjavalib_stub_sources"], current_api: "sdk_library/module-lib/myjavalib.txt", removed_api: "sdk_library/module-lib/myjavalib-removed.txt", sdk_version: "module_current", @@ -1419,14 +1419,14 @@ java_sdk_library_import { shared_library: true, public: { jars: ["sdk_library/public/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/public/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/public/myjavalib_stub_sources"], current_api: "sdk_library/public/myjavalib.txt", removed_api: "sdk_library/public/myjavalib-removed.txt", sdk_version: "current", }, system_server: { jars: ["sdk_library/system-server/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/system-server/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/system-server/myjavalib_stub_sources"], current_api: "sdk_library/system-server/myjavalib.txt", removed_api: "sdk_library/system-server/myjavalib-removed.txt", sdk_version: "system_server_current", @@ -1441,14 +1441,14 @@ java_sdk_library_import { shared_library: true, public: { jars: ["sdk_library/public/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/public/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/public/myjavalib_stub_sources"], current_api: "sdk_library/public/myjavalib.txt", removed_api: "sdk_library/public/myjavalib-removed.txt", sdk_version: "current", }, system_server: { jars: ["sdk_library/system-server/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/system-server/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/system-server/myjavalib_stub_sources"], current_api: "sdk_library/system-server/myjavalib.txt", removed_api: "sdk_library/system-server/myjavalib-removed.txt", sdk_version: "system_server_current", @@ -1508,7 +1508,7 @@ java_sdk_library_import { shared_library: true, public: { jars: ["sdk_library/public/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/public/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/public/myjavalib_stub_sources"], current_api: "sdk_library/public/myjavalib.txt", removed_api: "sdk_library/public/myjavalib-removed.txt", sdk_version: "current", @@ -1524,7 +1524,7 @@ java_sdk_library_import { shared_library: true, public: { jars: ["sdk_library/public/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/public/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/public/myjavalib_stub_sources"], current_api: "sdk_library/public/myjavalib.txt", removed_api: "sdk_library/public/myjavalib-removed.txt", sdk_version: "current", @@ -1584,7 +1584,7 @@ java_sdk_library_import { doctag_files: ["doctags/docs/known_doctags"], public: { jars: ["sdk_library/public/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/public/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/public/myjavalib_stub_sources"], current_api: "sdk_library/public/myjavalib.txt", removed_api: "sdk_library/public/myjavalib-removed.txt", sdk_version: "current", @@ -1600,7 +1600,7 @@ java_sdk_library_import { doctag_files: ["doctags/docs/known_doctags"], public: { jars: ["sdk_library/public/myjavalib-stubs.jar"], - stub_srcs: ["sdk_library/public/myjavalib_stub_sources/**/*.java"], + stub_srcs: ["sdk_library/public/myjavalib_stub_sources"], current_api: "sdk_library/public/myjavalib.txt", removed_api: "sdk_library/public/myjavalib-removed.txt", sdk_version: "current", From 1a39332cf6149b097c4f41e38bed61ecf674eb50 Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Wed, 18 Nov 2020 16:36:47 +0000 Subject: [PATCH 2/2] Fix prebuilt_stubs_sources to work with no stubs sources The framework-sdkextension java_sdk_library module defines an API for public, system and module_lib API surfaces but the public API is empty. The empty public API results in an empty .srcjar being repackaged and merged into the sdkextension-sdk snapshot and results in no directory for the public API stubs sources being created. Unfortunately, the Android.bp file in the snapshot is created by Soong and it does not know that the public API will be empty and so it creates an Android.bp file that references the directory into which the stubs sources should be added but which ends up not existing in the snapshot. Referencing a non-existent directory causes a build failure. This change fixes that issue by using PathForModuleSrc with no path components to get the path to the module directory (which must exist) and then resolving the module relative local src directory against that. The local src directory is globbed to find all the files, which will return an empty set of paths if the directory does not exist. Finally, the file paths are passed as an rsp file to soong_zip to avoid exceeding any command line limits. Many other different approaches were considered: * Adding a property to the java_sdk_library to indicate that the public API was actually empty. That would require extra maintenance by developers and would require some extra checks to be performed after generating the stubs source to ensure that it was empty which would complicate the build process. * Creating a directory with some placeholder file (empty directories don't work well with git) that would force the creation of the directory. That file would most likely be created whether the API was empty or not, would need to be stored in git alongside the source and could be quite confusing to reviewers. Bug: 173508731 Test: m nothing - to run new tests Build sdkextension-sdk, unpack it and then build the .srcjar files for the public, system and module_lib API surfaces. Without this change the build failed, reporting that the stubs_sources directory for the public API did not exist. With this change the build succeeded. Checked the contents of the resulting .srcjar files and made sure that the public one was empty and the others contained the SdkExtensions.java class and a package-info.java file. Change-Id: Ia468a3f37349f2dbc21db67744bda6461498d515 --- java/droiddoc.go | 30 ++++++++++++++++++------------ java/java_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 12 deletions(-) diff --git a/java/droiddoc.go b/java/droiddoc.go index c7a27c2a4..cf7f4fb5d 100644 --- a/java/droiddoc.go +++ b/java/droiddoc.go @@ -1748,8 +1748,6 @@ type PrebuiltStubsSources struct { properties PrebuiltStubsSourcesProperties - // The source directories containing stubs source files. - srcDirs android.Paths stubsSrcJar android.ModuleOutPath } @@ -1769,21 +1767,29 @@ func (d *PrebuiltStubsSources) StubsSrcJar() android.Path { func (p *PrebuiltStubsSources) GenerateAndroidBuildActions(ctx android.ModuleContext) { p.stubsSrcJar = android.PathForModuleOut(ctx, ctx.ModuleName()+"-"+"stubs.srcjar") - p.srcDirs = android.PathsForModuleSrc(ctx, p.properties.Srcs) + if len(p.properties.Srcs) != 1 { + ctx.PropertyErrorf("srcs", "must only specify one directory path, contains %d paths", len(p.properties.Srcs)) + return + } + + localSrcDir := p.properties.Srcs[0] + // Although PathForModuleSrc can return nil if either the path doesn't exist or + // the path components are invalid it won't in this case because no components + // are specified and the module directory must exist in order to get this far. + srcDir := android.PathForModuleSrc(ctx).(android.SourcePath).Join(ctx, localSrcDir) + + // Glob the contents of the directory just in case the directory does not exist. + srcGlob := localSrcDir + "/**/*" + srcPaths := android.PathsForModuleSrc(ctx, []string{srcGlob}) rule := android.NewRuleBuilder() - command := rule.Command(). + rule.Command(). BuiltTool(ctx, "soong_zip"). Flag("-write_if_changed"). Flag("-jar"). - FlagWithOutput("-o ", p.stubsSrcJar) - - for _, d := range p.srcDirs { - dir := d.String() - command. - FlagWithArg("-C ", dir). - FlagWithInput("-D ", d) - } + FlagWithOutput("-o ", p.stubsSrcJar). + FlagWithArg("-C ", srcDir.String()). + FlagWithRspFileInputList("-r ", srcPaths) rule.Restat() diff --git a/java/java_test.go b/java/java_test.go index 845a03a4d..cf01f3368 100644 --- a/java/java_test.go +++ b/java/java_test.go @@ -15,6 +15,7 @@ package java import ( + "fmt" "io/ioutil" "os" "path/filepath" @@ -622,6 +623,35 @@ func assertDeepEquals(t *testing.T, message string, expected interface{}, actual } } +func TestPrebuiltStubsSources(t *testing.T) { + test := func(t *testing.T, sourcesPath string, expectedInputs []string) { + ctx, _ := testJavaWithFS(t, fmt.Sprintf(` +prebuilt_stubs_sources { + name: "stubs-source", + srcs: ["%s"], +}`, sourcesPath), map[string][]byte{ + "stubs/sources/pkg/A.java": nil, + "stubs/sources/pkg/B.java": nil, + }) + + zipSrc := ctx.ModuleForTests("stubs-source", "android_common").Rule("zip_src") + if expected, actual := expectedInputs, zipSrc.Inputs.Strings(); !reflect.DeepEqual(expected, actual) { + t.Errorf("mismatch of inputs to soong_zip: expected %q, actual %q", expected, actual) + } + } + + t.Run("empty/missing directory", func(t *testing.T) { + test(t, "empty-directory", []string{}) + }) + + t.Run("non-empty set of sources", func(t *testing.T) { + test(t, "stubs/sources", []string{ + "stubs/sources/pkg/A.java", + "stubs/sources/pkg/B.java", + }) + }) +} + func TestJavaSdkLibraryImport(t *testing.T) { ctx, _ := testJava(t, ` java_library {