From 1364891fad13901c3b47d31e9d9dfaaf04ecaa79 Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Fri, 15 Jul 2022 13:12:35 +0000 Subject: [PATCH] Use implementation jar for updatable-media in snapshot for S While enabling prebuilts in T we hit b/229932396 which was caused by some parts of the build depending on the prebuilt updatable-media jar which used to be a full implementation jar but which is now an invalid jar as the snapshot must not be including implementation details. We fixed the issue in T but we are hitting the same problem in S with the M-2022-07. That is the first train in which the prebuilt updatable-media module provides an invalid jar, prior to that it was always providing an implementation jar. This change tweaks the sdk snapshot generation code to use an implementation jar for updatable-media in the S snapshot to avoid partners having to cherry pick changes similar to those needed to fix b/229932396 in T. Bug: 239121291 Test: packages/modules/common/build/mainline_modules_sdks.sh # Check that S media snapshot includes implementation jar. # Check that S art snapshot includes invalid jar. # Check that T media snapshot includes invalid jar. Change-Id: Ib49484d00a60b4ed7f8268e04f9c10a3498edb56 --- android/sdk.go | 4 +++ java/java.go | 30 ++++++++++++++++----- sdk/build_release.go | 12 ++++++--- sdk/java_sdk_test.go | 63 +++++++++++++++++++++++++++++++++++++++++--- sdk/update.go | 6 +++++ 5 files changed, 101 insertions(+), 14 deletions(-) diff --git a/android/sdk.go b/android/sdk.go index 07b94b246..a71f7f211 100644 --- a/android/sdk.go +++ b/android/sdk.go @@ -985,6 +985,10 @@ type SdkMemberContext interface { // RequiresTrait returns true if this member is expected to provide the specified trait. RequiresTrait(trait SdkMemberTrait) bool + + // IsTargetBuildBeforeTiramisu return true if the target build release for which this snapshot is + // being generated is before Tiramisu, i.e. S. + IsTargetBuildBeforeTiramisu() bool } // ExportedComponentsInfo contains information about the components that this module exports to an diff --git a/java/java.go b/java/java.go index 6e0515964..5a08f6d6f 100644 --- a/java/java.go +++ b/java/java.go @@ -118,6 +118,16 @@ var ( copyEverythingToSnapshot, } + snapshotRequiresImplementationJar = func(ctx android.SdkMemberContext) bool { + // In the S build the build will break if updatable-media does not provide a full implementation + // jar. That issue was fixed in Tiramisu by b/229932396. + if ctx.IsTargetBuildBeforeTiramisu() && ctx.Name() == "updatable-media" { + return true + } + + return false + } + // Supports adding java boot libraries to module_exports and sdk. // // The build has some implicit dependencies (via the boot jars configuration) on a number of @@ -135,13 +145,21 @@ var ( SupportsSdk: true, }, func(ctx android.SdkMemberContext, j *Library) android.Path { + if snapshotRequiresImplementationJar(ctx) { + return exportImplementationClassesJar(ctx, j) + } + // Java boot libs are only provided in the SDK to provide access to their dex implementation // jar for use by dexpreopting and boot jars package check. They do not need to provide an // actual implementation jar but the java_import will need a file that exists so just copy an // empty file. Any attempt to use that file as a jar will cause a build error. return ctx.SnapshotBuilder().EmptyFile() }, - func(osPrefix, name string) string { + func(ctx android.SdkMemberContext, osPrefix, name string) string { + if snapshotRequiresImplementationJar(ctx) { + return sdkSnapshotFilePathForJar(ctx, osPrefix, name) + } + // Create a special name for the implementation jar to try and provide some useful information // to a developer that attempts to compile against this. // TODO(b/175714559): Provide a proper error message in Soong not ninja. @@ -175,7 +193,7 @@ var ( // file. Any attempt to use that file as a jar will cause a build error. return ctx.SnapshotBuilder().EmptyFile() }, - func(osPrefix, name string) string { + func(_ android.SdkMemberContext, osPrefix, name string) string { // Create a special name for the implementation jar to try and provide some useful information // to a developer that attempts to compile against this. // TODO(b/175714559): Provide a proper error message in Soong not ninja. @@ -672,7 +690,7 @@ const ( ) // path to the jar file of a java library. Relative to / -func sdkSnapshotFilePathForJar(osPrefix, name string) string { +func sdkSnapshotFilePathForJar(_ android.SdkMemberContext, osPrefix, name string) string { return sdkSnapshotFilePathForMember(osPrefix, name, jarFileSuffix) } @@ -689,7 +707,7 @@ type librarySdkMemberType struct { // Function to compute the snapshot relative path to which the named library's // jar should be copied. - snapshotPathGetter func(osPrefix, name string) string + snapshotPathGetter func(ctx android.SdkMemberContext, osPrefix, name string) string // True if only the jar should be copied to the snapshot, false if the jar plus any additional // files like aidl files should also be copied. @@ -747,7 +765,7 @@ func (p *librarySdkMemberProperties) AddToPropertySet(ctx android.SdkMemberConte exportedJar := p.JarToExport if exportedJar != nil { // Delegate the creation of the snapshot relative path to the member type. - snapshotRelativeJavaLibPath := memberType.snapshotPathGetter(p.OsPrefix(), ctx.Name()) + snapshotRelativeJavaLibPath := memberType.snapshotPathGetter(ctx, p.OsPrefix(), ctx.Name()) // Copy the exported jar to the snapshot. builder.CopyToSnapshot(exportedJar, snapshotRelativeJavaLibPath) @@ -1213,7 +1231,7 @@ func (p *testSdkMemberProperties) AddToPropertySet(ctx android.SdkMemberContext, exportedJar := p.JarToExport if exportedJar != nil { - snapshotRelativeJavaLibPath := sdkSnapshotFilePathForJar(p.OsPrefix(), ctx.Name()) + snapshotRelativeJavaLibPath := sdkSnapshotFilePathForJar(ctx, p.OsPrefix(), ctx.Name()) builder.CopyToSnapshot(exportedJar, snapshotRelativeJavaLibPath) propertySet.AddProperty("jars", []string{snapshotRelativeJavaLibPath}) diff --git a/sdk/build_release.go b/sdk/build_release.go index 4c2277e85..0494a28ee 100644 --- a/sdk/build_release.go +++ b/sdk/build_release.go @@ -24,18 +24,22 @@ import ( // buildRelease represents the version of a build system used to create a specific release. // -// The name of the release, is the same as the code for the dessert release, e.g. S, T, etc. +// The name of the release, is the same as the code for the dessert release, e.g. S, Tiramisu, etc. type buildRelease struct { - // The name of the release, e.g. S, T, etc. + // The name of the release, e.g. S, Tiramisu, etc. name string // The index of this structure within the buildReleases list. ordinal int } +func (br *buildRelease) EarlierThan(other *buildRelease) bool { + return br.ordinal < other.ordinal +} + // String returns the name of the build release. -func (s *buildRelease) String() string { - return s.name +func (br *buildRelease) String() string { + return br.name } // buildReleaseSet represents a set of buildRelease objects. diff --git a/sdk/java_sdk_test.go b/sdk/java_sdk_test.go index d25138f5b..57c171fed 100644 --- a/sdk/java_sdk_test.go +++ b/sdk/java_sdk_test.go @@ -15,6 +15,7 @@ package sdk import ( + "fmt" "testing" "android/soong/android" @@ -257,8 +258,8 @@ func TestSnapshotWithJavaBootLibrary(t *testing.T) { android.FixtureAddFile("aidl", nil), android.FixtureAddFile("resource.txt", nil), ).RunTestWithBp(t, ` - module_exports { - name: "myexports", + sdk { + name: "mysdk", java_boot_libs: ["myjavalib"], } @@ -278,7 +279,7 @@ func TestSnapshotWithJavaBootLibrary(t *testing.T) { } `) - CheckSnapshot(t, result, "myexports", "", + CheckSnapshot(t, result, "mysdk", "", checkAndroidBpContents(` // This is auto-generated. DO NOT EDIT. @@ -292,11 +293,65 @@ java_import { } `), checkAllCopyRules(` -.intermediates/myexports/common_os/empty -> java_boot_libs/snapshot/jars/are/invalid/myjavalib.jar +.intermediates/mysdk/common_os/empty -> java_boot_libs/snapshot/jars/are/invalid/myjavalib.jar `), ) } +func TestSnapshotWithJavaBootLibrary_UpdatableMedia(t *testing.T) { + runTest := func(t *testing.T, targetBuildRelease, expectedJarPath, expectedCopyRule string) { + result := android.GroupFixturePreparers( + prepareForSdkTestWithJava, + android.FixtureMergeEnv(map[string]string{ + "SOONG_SDK_SNAPSHOT_TARGET_BUILD_RELEASE": targetBuildRelease, + }), + ).RunTestWithBp(t, ` + sdk { + name: "mysdk", + java_boot_libs: ["updatable-media"], + } + + java_library { + name: "updatable-media", + srcs: ["Test.java"], + system_modules: "none", + sdk_version: "none", + compile_dex: true, + permitted_packages: ["pkg.media"], + apex_available: ["com.android.media"], + } + `) + + CheckSnapshot(t, result, "mysdk", "", + checkAndroidBpContents(fmt.Sprintf(` +// This is auto-generated. DO NOT EDIT. + +java_import { + name: "updatable-media", + prefer: false, + visibility: ["//visibility:public"], + apex_available: ["com.android.media"], + jars: ["%s"], + permitted_packages: ["pkg.media"], +} +`, expectedJarPath)), + checkAllCopyRules(expectedCopyRule), + ) + } + + t.Run("updatable-media in S", func(t *testing.T) { + runTest(t, "S", "java/updatable-media.jar", ` +.intermediates/updatable-media/android_common/package-check/updatable-media.jar -> java/updatable-media.jar +`) + }) + + t.Run("updatable-media in T", func(t *testing.T) { + runTest(t, "Tiramisu", "java_boot_libs/snapshot/jars/are/invalid/updatable-media.jar", ` +.intermediates/mysdk/common_os/empty -> java_boot_libs/snapshot/jars/are/invalid/updatable-media.jar +`) + }) +} + func TestSnapshotWithJavaSystemserverLibrary(t *testing.T) { result := android.GroupFixturePreparers( prepareForSdkTestWithJava, diff --git a/sdk/update.go b/sdk/update.go index b9ef3d0f4..995da741c 100644 --- a/sdk/update.go +++ b/sdk/update.go @@ -1972,6 +1972,12 @@ func (m *memberContext) RequiresTrait(trait android.SdkMemberTrait) bool { return m.requiredTraits.Contains(trait) } +func (m *memberContext) IsTargetBuildBeforeTiramisu() bool { + return m.builder.targetBuildRelease.EarlierThan(buildReleaseT) +} + +var _ android.SdkMemberContext = (*memberContext)(nil) + func (s *sdk) createMemberSnapshot(ctx *memberContext, member *sdkMember, bpModule *bpModule) { memberType := member.memberType