From 3dd662509d02914a09f9954b37eaa7dd164a3523 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Tue, 1 Jun 2021 14:05:09 -0700 Subject: [PATCH 1/4] Make the default java_sdk_library dist_group "unknown" Change the default dist_group from "android" to "unknown" to prevent accidentally including java_sdk_library stubs that do not set dist_group or owner in the public SDK. Bug: 186723288 Test: TestJavaSdkLibraryDist Change-Id: I9aae2a16254ac1a8d444acfa63bc571d1ef4b045 --- java/sdk_library.go | 5 ++--- java/sdk_library_test.go | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/java/sdk_library.go b/java/sdk_library.go index ecf2b1a03..1936bf5fd 100644 --- a/java/sdk_library.go +++ b/java/sdk_library.go @@ -458,7 +458,7 @@ type sdkLibraryProperties struct { Dist_stem *string // The subdirectory for the artifacts that are copied to the dist directory. If not specified - // then defaults to "android". Should be set to "android" for anything that should be published + // then defaults to "unknown". Should be set to "android" for anything that should be published // in the public Android SDK. Dist_group *string @@ -1240,8 +1240,7 @@ func (module *SdkLibrary) distGroup() string { if owner := module.ModuleBase.Owner(); owner != "" { return owner } - // TODO(b/186723288): Make this "unknown". - return "android" + return "unknown" } func (module *SdkLibrary) latestApiFilegroupName(apiScope *apiScope) string { diff --git a/java/sdk_library_test.go b/java/sdk_library_test.go index 0fe6e72ef..f0f3a197f 100644 --- a/java/sdk_library_test.go +++ b/java/sdk_library_test.go @@ -888,7 +888,7 @@ func TestJavaSdkLibraryDist(t *testing.T) { testCases := []testCase{ { module: "sdklib_no_owner", - distDir: "apistubs/android/public", + distDir: "apistubs/unknown/public", distStem: "sdklib_no_owner.jar", }, { @@ -903,7 +903,7 @@ func TestJavaSdkLibraryDist(t *testing.T) { }, { module: "sdklib_stem_foo", - distDir: "apistubs/android/public", + distDir: "apistubs/unknown/public", distStem: "foo.jar", }, { From 59b92bfdb300d93558d685e9ea9f4cb5c7378a3b Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Tue, 1 Jun 2021 14:07:56 -0700 Subject: [PATCH 2/4] Ignore owner property when computing java_sdk_library dist subdirectory The owner property is no longer used, the dist subdirectory is determined by the dist_group property. Bug: 186723288 Test: TestJavaSdkLibraryDist Change-Id: Id6d997eef05f6511070677974219674f248cb754 --- java/sdk_library.go | 9 +-------- java/sdk_library_test.go | 9 +++++---- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/java/sdk_library.go b/java/sdk_library.go index 1936bf5fd..7fec13871 100644 --- a/java/sdk_library.go +++ b/java/sdk_library.go @@ -1233,14 +1233,7 @@ func (module *SdkLibrary) distStem() string { // distGroup returns the subdirectory of the dist path of the stub artifacts. func (module *SdkLibrary) distGroup() string { - if group := proptools.String(module.sdkLibraryProperties.Dist_group); group != "" { - return group - } - // TODO(b/186723288): Remove this once everything uses dist_group. - if owner := module.ModuleBase.Owner(); owner != "" { - return owner - } - return "unknown" + return proptools.StringDefault(module.sdkLibraryProperties.Dist_group, "unknown") } func (module *SdkLibrary) latestApiFilegroupName(apiScope *apiScope) string { diff --git a/java/sdk_library_test.go b/java/sdk_library_test.go index f0f3a197f..fe21a20d0 100644 --- a/java/sdk_library_test.go +++ b/java/sdk_library_test.go @@ -846,7 +846,7 @@ func TestJavaSdkLibraryDist(t *testing.T) { PrepareForTestWithJavaSdkLibraryFiles, ).RunTestWithBp(t, ` java_sdk_library { - name: "sdklib_no_owner", + name: "sdklib_no_group", unsafe_ignore_missing_latest_api: true, srcs: ["foo.java"], } @@ -887,9 +887,9 @@ func TestJavaSdkLibraryDist(t *testing.T) { } testCases := []testCase{ { - module: "sdklib_no_owner", + module: "sdklib_no_group", distDir: "apistubs/unknown/public", - distStem: "sdklib_no_owner.jar", + distStem: "sdklib_no_group.jar", }, { module: "sdklib_group_foo", @@ -897,8 +897,9 @@ func TestJavaSdkLibraryDist(t *testing.T) { distStem: "sdklib_group_foo.jar", }, { + // Owner doesn't affect distDir after b/186723288. module: "sdklib_owner_foo", - distDir: "apistubs/foo/public", + distDir: "apistubs/unknown/public", distStem: "sdklib_owner_foo.jar", }, { From f0eace9eed83c0d7cd06df29379ed7e090bd2c27 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Wed, 2 Jun 2021 13:02:23 -0700 Subject: [PATCH 3/4] Remove core_lib property from java_sdk_library Its not used, remove it. Bug: 186723288 Test: TestJavaSdkLibraryDist Change-Id: I1689b670a8ae6a614e5e4ec5e79cb5e283b2e277 --- java/sdk_library.go | 9 +-------- java/sdk_library_test.go | 12 ------------ 2 files changed, 1 insertion(+), 20 deletions(-) diff --git a/java/sdk_library.go b/java/sdk_library.go index 7fec13871..94927291c 100644 --- a/java/sdk_library.go +++ b/java/sdk_library.go @@ -435,9 +435,6 @@ type sdkLibraryProperties struct { // a list of top-level directories containing Java stub files to merge show/hide annotations from. Merge_inclusion_annotations_dirs []string - // If set to true, the path of dist files is apistubs/core. Defaults to false. - Core_lib *bool - // If set to true then don't create dist rules. No_dist *bool @@ -1203,11 +1200,7 @@ func (module *SdkLibrary) AndroidMkEntries() []android.AndroidMkEntries { // The dist path of the stub artifacts func (module *SdkLibrary) apiDistPath(apiScope *apiScope) string { - if Bool(module.sdkLibraryProperties.Core_lib) { - return path.Join("apistubs", "core", apiScope.name) - } else { - return path.Join("apistubs", module.distGroup(), apiScope.name) - } + return path.Join("apistubs", module.distGroup(), apiScope.name) } // Get the sdk version for use when compiling the stubs library. diff --git a/java/sdk_library_test.go b/java/sdk_library_test.go index fe21a20d0..e051bbe2c 100644 --- a/java/sdk_library_test.go +++ b/java/sdk_library_test.go @@ -871,13 +871,6 @@ func TestJavaSdkLibraryDist(t *testing.T) { srcs: ["foo.java"], dist_stem: "foo", } - - java_sdk_library { - name: "sdklib_core_lib", - unsafe_ignore_missing_latest_api: true, - srcs: ["foo.java"], - core_lib: true, - } `) type testCase struct { @@ -907,11 +900,6 @@ func TestJavaSdkLibraryDist(t *testing.T) { distDir: "apistubs/unknown/public", distStem: "foo.jar", }, - { - module: "sdklib_core_lib", - distDir: "apistubs/core/public", - distStem: "sdklib_core_lib.jar", - }, } for _, tt := range testCases { From 67c17aede87e3e6d9e1abefc6ad86e987fc9bee9 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Wed, 2 Jun 2021 13:02:51 -0700 Subject: [PATCH 4/4] Don't use unsafe_ignore_missing_latest_api in TestJavaSdkLibraryDist Use FixtureWithLastReleaseApis to create the necessary files instead of disabling the check with unsafe_ignore_missing_latest_api. Bug: 186723288 Test: TestJavaSdkLibraryDist Change-Id: I162b0f68eec596274f8d98dca9d3f0500ab13f5d --- java/sdk_library_test.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/java/sdk_library_test.go b/java/sdk_library_test.go index e051bbe2c..2520dde6e 100644 --- a/java/sdk_library_test.go +++ b/java/sdk_library_test.go @@ -844,30 +844,31 @@ func TestJavaSdkLibraryDist(t *testing.T) { PrepareForTestWithJavaBuildComponents, PrepareForTestWithJavaDefaultModules, PrepareForTestWithJavaSdkLibraryFiles, + FixtureWithLastReleaseApis( + "sdklib_no_group", + "sdklib_group_foo", + "sdklib_owner_foo", + "foo"), ).RunTestWithBp(t, ` java_sdk_library { name: "sdklib_no_group", - unsafe_ignore_missing_latest_api: true, srcs: ["foo.java"], } java_sdk_library { name: "sdklib_group_foo", - unsafe_ignore_missing_latest_api: true, srcs: ["foo.java"], dist_group: "foo", } java_sdk_library { name: "sdklib_owner_foo", - unsafe_ignore_missing_latest_api: true, srcs: ["foo.java"], owner: "foo", } java_sdk_library { name: "sdklib_stem_foo", - unsafe_ignore_missing_latest_api: true, srcs: ["foo.java"], dist_stem: "foo", }