Bladeren bron

Add apexes property to sdk

Specifying an apex in the apexes propety will cause all the
*classpath_fragments that are contents of the APEX to be automatically
added as members of the sdk and appear in the snapshot.

The purpose of this change is to dedup the APEX and sdk definitions and
try and avoid some of the issues that we have been finding while
attempting to build against the prebuilts.

Two tests, one each for bootclasspath_fragment and
systemserverclasspath_fragment, have been refactored to compare the
output when adding the *fragment to the sdk directly of via the APEX.
That ensures switching to use the APEX will not change the sdk snapshot
unless it was previously missing a *fragment.

There was also a slight difference in where the hidden API flags were
copied from. That should have no impact on the output as the flags are
identical.

The sdk snapshot generation needed some tweaks to avoid generating a
prebuilt for the APEX.

Bug: 232401814
Test: m nothing
Change-Id: I7aaf16a3a0ab4bebf97765d1484215cc008dc4b8
Paul Duffin 2 jaren geleden
bovenliggende
commit
4e7d1c43e2

+ 19 - 1
android/sdk.go

@@ -677,6 +677,10 @@ type SdkMemberType interface {
 	// SupportedLinkages returns the names of the linkage variants supported by this module.
 	SupportedLinkages() []string
 
+	// ArePrebuiltsRequired returns true if prebuilts are required in the sdk snapshot, false
+	// otherwise.
+	ArePrebuiltsRequired() bool
+
 	// AddDependencies adds dependencies from the SDK module to all the module variants the member
 	// type contributes to the SDK. `names` is the list of module names given in the member type
 	// property (as returned by SdkPropertyName()) in the SDK module. The exact set of variants
@@ -782,7 +786,12 @@ type SdkMemberTypeBase struct {
 	// If not specified then it is assumed to be available on all targeted build releases.
 	SupportedBuildReleaseSpecification string
 
-	SupportsSdk     bool
+	// Set to true if this must be usable with the sdk/sdk_snapshot module types. Otherwise, it will
+	// only be usable with module_exports/module_exports_snapshots module types.
+	SupportsSdk bool
+
+	// Set to true if prebuilt host artifacts of this member may be specific to the host OS. Only
+	// applicable to modules where HostSupported() is true.
 	HostOsDependent bool
 
 	// When set to true UseSourceModuleTypeInSnapshot indicates that the member type creates a source
@@ -790,6 +799,11 @@ type SdkMemberTypeBase struct {
 	// code from automatically adding a prefer: true flag.
 	UseSourceModuleTypeInSnapshot bool
 
+	// Set to proptools.BoolPtr(false) if this member does not generate prebuilts but is only provided
+	// to allow the sdk to gather members from this member's dependencies. If not specified then
+	// defaults to true.
+	PrebuiltsRequired *bool
+
 	// The list of supported traits.
 	Traits []SdkMemberTrait
 }
@@ -814,6 +828,10 @@ func (b *SdkMemberTypeBase) IsHostOsDependent() bool {
 	return b.HostOsDependent
 }
 
+func (b *SdkMemberTypeBase) ArePrebuiltsRequired() bool {
+	return proptools.BoolDefault(b.PrebuiltsRequired, true)
+}
+
 func (b *SdkMemberTypeBase) UsesSourceModuleTypeInSnapshot() bool {
 	return b.UseSourceModuleTypeInSnapshot
 }

+ 1 - 0
apex/Android.bp

@@ -23,6 +23,7 @@ bootstrap_go_package {
     srcs: [
         "androidmk.go",
         "apex.go",
+        "apex_sdk_member.go",
         "apex_singleton.go",
         "builder.go",
         "constants.go",

+ 15 - 2
apex/apex.go

@@ -606,6 +606,18 @@ type dependencyTag struct {
 	// replacement. This is needed because some prebuilt modules do not provide all the information
 	// needed by the apex.
 	sourceOnly bool
+
+	// If not-nil and an APEX is a member of an SDK then dependencies of that APEX with this tag will
+	// also be added as exported members of that SDK.
+	memberType android.SdkMemberType
+}
+
+func (d *dependencyTag) SdkMemberType(_ android.Module) android.SdkMemberType {
+	return d.memberType
+}
+
+func (d *dependencyTag) ExportMember() bool {
+	return true
 }
 
 func (d *dependencyTag) String() string {
@@ -617,6 +629,7 @@ func (d *dependencyTag) ReplaceSourceWithPrebuilt() bool {
 }
 
 var _ android.ReplaceSourceWithPrebuilt = &dependencyTag{}
+var _ android.SdkMemberDependencyTag = &dependencyTag{}
 
 var (
 	androidAppTag   = &dependencyTag{name: "androidApp", payload: true}
@@ -624,8 +637,8 @@ var (
 	certificateTag  = &dependencyTag{name: "certificate"}
 	executableTag   = &dependencyTag{name: "executable", payload: true}
 	fsTag           = &dependencyTag{name: "filesystem", payload: true}
-	bcpfTag         = &dependencyTag{name: "bootclasspathFragment", payload: true, sourceOnly: true}
-	sscpfTag        = &dependencyTag{name: "systemserverclasspathFragment", payload: true, sourceOnly: true}
+	bcpfTag         = &dependencyTag{name: "bootclasspathFragment", payload: true, sourceOnly: true, memberType: java.BootclasspathFragmentSdkMemberType}
+	sscpfTag        = &dependencyTag{name: "systemserverclasspathFragment", payload: true, sourceOnly: true, memberType: java.SystemServerClasspathFragmentSdkMemberType}
 	compatConfigTag = &dependencyTag{name: "compatConfig", payload: true, sourceOnly: true}
 	javaLibTag      = &dependencyTag{name: "javaLib", payload: true}
 	jniLibTag       = &dependencyTag{name: "jniLib", payload: true}

+ 58 - 0
apex/apex_sdk_member.go

@@ -0,0 +1,58 @@
+// Copyright (C) 2022 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package apex
+
+import (
+	"android/soong/android"
+	"github.com/google/blueprint"
+	"github.com/google/blueprint/proptools"
+)
+
+// This file contains support for using apex modules within an sdk.
+
+func init() {
+	// Register sdk member types.
+	android.RegisterSdkMemberType(&apexSdkMemberType{
+		SdkMemberTypeBase: android.SdkMemberTypeBase{
+			PropertyName: "apexes",
+			SupportsSdk:  true,
+
+			// The apexes property does not need to be included in the snapshot as adding an apex to an
+			// sdk does not produce any prebuilts of the apex.
+			PrebuiltsRequired: proptools.BoolPtr(false),
+		},
+	})
+}
+
+type apexSdkMemberType struct {
+	android.SdkMemberTypeBase
+}
+
+func (mt *apexSdkMemberType) AddDependencies(ctx android.SdkDependencyContext, dependencyTag blueprint.DependencyTag, names []string) {
+	ctx.AddVariationDependencies(nil, dependencyTag, names...)
+}
+
+func (mt *apexSdkMemberType) IsInstance(module android.Module) bool {
+	_, ok := module.(*apexBundle)
+	return ok
+}
+
+func (mt *apexSdkMemberType) AddPrebuiltModule(ctx android.SdkMemberContext, member android.SdkMember) android.BpModule {
+	panic("Sdk does not create prebuilts of the apexes in its snapshot")
+}
+
+func (mt *apexSdkMemberType) CreateVariantPropertiesStruct() android.SdkMemberProperties {
+	panic("Sdk does not create prebuilts of the apexes in its snapshot")
+}

+ 10 - 6
java/bootclasspath_fragment.go

@@ -32,12 +32,7 @@ import (
 func init() {
 	registerBootclasspathFragmentBuildComponents(android.InitRegistrationContext)
 
-	android.RegisterSdkMemberType(&bootclasspathFragmentMemberType{
-		SdkMemberTypeBase: android.SdkMemberTypeBase{
-			PropertyName: "bootclasspath_fragments",
-			SupportsSdk:  true,
-		},
-	})
+	android.RegisterSdkMemberType(BootclasspathFragmentSdkMemberType)
 }
 
 func registerBootclasspathFragmentBuildComponents(ctx android.RegistrationContext) {
@@ -46,6 +41,15 @@ func registerBootclasspathFragmentBuildComponents(ctx android.RegistrationContex
 	ctx.RegisterModuleType("prebuilt_bootclasspath_fragment", prebuiltBootclasspathFragmentFactory)
 }
 
+// BootclasspathFragmentSdkMemberType is the member type used to add bootclasspath_fragments to
+// the SDK snapshot. It is exported for use by apex.
+var BootclasspathFragmentSdkMemberType = &bootclasspathFragmentMemberType{
+	SdkMemberTypeBase: android.SdkMemberTypeBase{
+		PropertyName: "bootclasspath_fragments",
+		SupportsSdk:  true,
+	},
+}
+
 type bootclasspathFragmentContentDependencyTag struct {
 	blueprint.BaseDependencyTag
 }

+ 12 - 9
java/systemserver_classpath_fragment.go

@@ -24,15 +24,7 @@ import (
 func init() {
 	registerSystemserverClasspathBuildComponents(android.InitRegistrationContext)
 
-	android.RegisterSdkMemberType(&systemServerClasspathFragmentMemberType{
-		SdkMemberTypeBase: android.SdkMemberTypeBase{
-			PropertyName: "systemserverclasspath_fragments",
-			SupportsSdk:  true,
-
-			// This was only added in Tiramisu.
-			SupportedBuildReleaseSpecification: "Tiramisu+",
-		},
-	})
+	android.RegisterSdkMemberType(SystemServerClasspathFragmentSdkMemberType)
 }
 
 func registerSystemserverClasspathBuildComponents(ctx android.RegistrationContext) {
@@ -41,6 +33,17 @@ func registerSystemserverClasspathBuildComponents(ctx android.RegistrationContex
 	ctx.RegisterModuleType("prebuilt_systemserverclasspath_fragment", prebuiltSystemServerClasspathModuleFactory)
 }
 
+var SystemServerClasspathFragmentSdkMemberType = &systemServerClasspathFragmentMemberType{
+	SdkMemberTypeBase: android.SdkMemberTypeBase{
+		PropertyName: "systemserverclasspath_fragments",
+		SupportsSdk:  true,
+
+		// Support for adding systemserverclasspath_fragments to the sdk snapshot was only added in
+		// Tiramisu.
+		SupportedBuildReleaseSpecification: "Tiramisu+",
+	},
+}
+
 type platformSystemServerClasspathModule struct {
 	android.ModuleBase
 

+ 86 - 32
sdk/bootclasspath_fragment_sdk_test.go

@@ -190,7 +190,7 @@ func checkBootJarsPackageCheckRule(t *testing.T, result *android.TestResult, exp
 	android.AssertStringDoesContain(t, "boot jars package check", command, expectedCommandArgs)
 }
 
-func TestSnapshotWithBootClasspathFragment_Contents(t *testing.T) {
+func testSnapshotWithBootClasspathFragment_Contents(t *testing.T, sdk string, copyRules string) {
 	result := android.GroupFixturePreparers(
 		prepareForSdkTestWithJava,
 		java.PrepareForTestWithJavaDefaultModules,
@@ -202,19 +202,7 @@ func TestSnapshotWithBootClasspathFragment_Contents(t *testing.T) {
 		// Add a platform_bootclasspath that depends on the fragment.
 		fixtureAddPlatformBootclasspathForBootclasspathFragment("myapex", "mybootclasspathfragment"),
 
-		android.FixtureWithRootAndroidBp(`
-			sdk {
-				name: "mysdk",
-				bootclasspath_fragments: ["mybootclasspathfragment"],
-				java_sdk_libs: [
-					// This is not strictly needed as it should be automatically added to the sdk_snapshot as
-					// a java_sdk_libs module because it is used in the mybootclasspathfragment's
-					// api.stub_libs property. However, it is specified here to ensure that duplicates are
-					// correctly deduped.
-					"mysdklibrary",
-				],
-			}
-
+		android.FixtureWithRootAndroidBp(sdk+`
 			apex {
 				name: "myapex",
 				key: "myapex.key",
@@ -373,24 +361,7 @@ java_sdk_library_import {
     },
 }
 		`),
-		checkAllCopyRules(`
-.intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/annotation-flags.csv -> hiddenapi/annotation-flags.csv
-.intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/metadata.csv -> hiddenapi/metadata.csv
-.intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/index.csv -> hiddenapi/index.csv
-.intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/signature-patterns.csv -> hiddenapi/signature-patterns.csv
-.intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/filtered-stub-flags.csv -> hiddenapi/filtered-stub-flags.csv
-.intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/filtered-flags.csv -> hiddenapi/filtered-flags.csv
-.intermediates/mysdk/common_os/empty -> java_boot_libs/snapshot/jars/are/invalid/mybootlib.jar
-.intermediates/myothersdklibrary.stubs/android_common/javac/myothersdklibrary.stubs.jar -> sdk_library/public/myothersdklibrary-stubs.jar
-.intermediates/myothersdklibrary.stubs.source/android_common/metalava/myothersdklibrary.stubs.source_api.txt -> sdk_library/public/myothersdklibrary.txt
-.intermediates/myothersdklibrary.stubs.source/android_common/metalava/myothersdklibrary.stubs.source_removed.txt -> sdk_library/public/myothersdklibrary-removed.txt
-.intermediates/mysdklibrary.stubs/android_common/javac/mysdklibrary.stubs.jar -> sdk_library/public/mysdklibrary-stubs.jar
-.intermediates/mysdklibrary.stubs.source/android_common/metalava/mysdklibrary.stubs.source_api.txt -> sdk_library/public/mysdklibrary.txt
-.intermediates/mysdklibrary.stubs.source/android_common/metalava/mysdklibrary.stubs.source_removed.txt -> sdk_library/public/mysdklibrary-removed.txt
-.intermediates/mycoreplatform.stubs/android_common/javac/mycoreplatform.stubs.jar -> sdk_library/public/mycoreplatform-stubs.jar
-.intermediates/mycoreplatform.stubs.source/android_common/metalava/mycoreplatform.stubs.source_api.txt -> sdk_library/public/mycoreplatform.txt
-.intermediates/mycoreplatform.stubs.source/android_common/metalava/mycoreplatform.stubs.source_removed.txt -> sdk_library/public/mycoreplatform-removed.txt
-`),
+		checkAllCopyRules(copyRules),
 		snapshotTestPreparer(checkSnapshotWithoutSource, preparerForSnapshot),
 		snapshotTestChecker(checkSnapshotWithoutSource, func(t *testing.T, result *android.TestResult) {
 			module := result.ModuleForTests("platform-bootclasspath", "android_common")
@@ -427,6 +398,89 @@ java_sdk_library_import {
 	)
 }
 
+func TestSnapshotWithBootClasspathFragment_Contents(t *testing.T) {
+	t.Run("added-directly", func(t *testing.T) {
+		testSnapshotWithBootClasspathFragment_Contents(t, `
+			sdk {
+				name: "mysdk",
+				bootclasspath_fragments: ["mybootclasspathfragment"],
+				java_sdk_libs: [
+					// This is not strictly needed as it should be automatically added to the sdk_snapshot as
+					// a java_sdk_libs module because it is used in the mybootclasspathfragment's
+					// api.stub_libs property. However, it is specified here to ensure that duplicates are
+					// correctly deduped.
+					"mysdklibrary",
+				],
+			}
+		`, `
+.intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/annotation-flags.csv -> hiddenapi/annotation-flags.csv
+.intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/metadata.csv -> hiddenapi/metadata.csv
+.intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/index.csv -> hiddenapi/index.csv
+.intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/signature-patterns.csv -> hiddenapi/signature-patterns.csv
+.intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/filtered-stub-flags.csv -> hiddenapi/filtered-stub-flags.csv
+.intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/filtered-flags.csv -> hiddenapi/filtered-flags.csv
+.intermediates/mysdk/common_os/empty -> java_boot_libs/snapshot/jars/are/invalid/mybootlib.jar
+.intermediates/myothersdklibrary.stubs/android_common/javac/myothersdklibrary.stubs.jar -> sdk_library/public/myothersdklibrary-stubs.jar
+.intermediates/myothersdklibrary.stubs.source/android_common/metalava/myothersdklibrary.stubs.source_api.txt -> sdk_library/public/myothersdklibrary.txt
+.intermediates/myothersdklibrary.stubs.source/android_common/metalava/myothersdklibrary.stubs.source_removed.txt -> sdk_library/public/myothersdklibrary-removed.txt
+.intermediates/mysdklibrary.stubs/android_common/javac/mysdklibrary.stubs.jar -> sdk_library/public/mysdklibrary-stubs.jar
+.intermediates/mysdklibrary.stubs.source/android_common/metalava/mysdklibrary.stubs.source_api.txt -> sdk_library/public/mysdklibrary.txt
+.intermediates/mysdklibrary.stubs.source/android_common/metalava/mysdklibrary.stubs.source_removed.txt -> sdk_library/public/mysdklibrary-removed.txt
+.intermediates/mycoreplatform.stubs/android_common/javac/mycoreplatform.stubs.jar -> sdk_library/public/mycoreplatform-stubs.jar
+.intermediates/mycoreplatform.stubs.source/android_common/metalava/mycoreplatform.stubs.source_api.txt -> sdk_library/public/mycoreplatform.txt
+.intermediates/mycoreplatform.stubs.source/android_common/metalava/mycoreplatform.stubs.source_removed.txt -> sdk_library/public/mycoreplatform-removed.txt
+`)
+	})
+
+	copyBootclasspathFragmentFromApexVariantRules := `
+.intermediates/mybootclasspathfragment/android_common_myapex/modular-hiddenapi/annotation-flags.csv -> hiddenapi/annotation-flags.csv
+.intermediates/mybootclasspathfragment/android_common_myapex/modular-hiddenapi/metadata.csv -> hiddenapi/metadata.csv
+.intermediates/mybootclasspathfragment/android_common_myapex/modular-hiddenapi/index.csv -> hiddenapi/index.csv
+.intermediates/mybootclasspathfragment/android_common_myapex/modular-hiddenapi/signature-patterns.csv -> hiddenapi/signature-patterns.csv
+.intermediates/mybootclasspathfragment/android_common_myapex/modular-hiddenapi/filtered-stub-flags.csv -> hiddenapi/filtered-stub-flags.csv
+.intermediates/mybootclasspathfragment/android_common_myapex/modular-hiddenapi/filtered-flags.csv -> hiddenapi/filtered-flags.csv
+.intermediates/mysdk/common_os/empty -> java_boot_libs/snapshot/jars/are/invalid/mybootlib.jar
+.intermediates/myothersdklibrary.stubs/android_common/javac/myothersdklibrary.stubs.jar -> sdk_library/public/myothersdklibrary-stubs.jar
+.intermediates/myothersdklibrary.stubs.source/android_common/metalava/myothersdklibrary.stubs.source_api.txt -> sdk_library/public/myothersdklibrary.txt
+.intermediates/myothersdklibrary.stubs.source/android_common/metalava/myothersdklibrary.stubs.source_removed.txt -> sdk_library/public/myothersdklibrary-removed.txt
+.intermediates/mysdklibrary.stubs/android_common/javac/mysdklibrary.stubs.jar -> sdk_library/public/mysdklibrary-stubs.jar
+.intermediates/mysdklibrary.stubs.source/android_common/metalava/mysdklibrary.stubs.source_api.txt -> sdk_library/public/mysdklibrary.txt
+.intermediates/mysdklibrary.stubs.source/android_common/metalava/mysdklibrary.stubs.source_removed.txt -> sdk_library/public/mysdklibrary-removed.txt
+.intermediates/mycoreplatform.stubs/android_common/javac/mycoreplatform.stubs.jar -> sdk_library/public/mycoreplatform-stubs.jar
+.intermediates/mycoreplatform.stubs.source/android_common/metalava/mycoreplatform.stubs.source_api.txt -> sdk_library/public/mycoreplatform.txt
+.intermediates/mycoreplatform.stubs.source/android_common/metalava/mycoreplatform.stubs.source_removed.txt -> sdk_library/public/mycoreplatform-removed.txt
+`
+	t.Run("added-via-apex", func(t *testing.T) {
+		testSnapshotWithBootClasspathFragment_Contents(t, `
+			sdk {
+				name: "mysdk",
+				apexes: ["myapex"],
+			}
+		`, copyBootclasspathFragmentFromApexVariantRules)
+	})
+
+	t.Run("added-directly-and-indirectly", func(t *testing.T) {
+		testSnapshotWithBootClasspathFragment_Contents(t, `
+			sdk {
+				name: "mysdk",
+				apexes: ["myapex"],
+				// This is not strictly needed as it should be automatically added to the sdk_snapshot as
+				// a bootclasspath_fragments module because it is used in the myapex's
+				// bootclasspath_fragments property. However, it is specified here to ensure that duplicates
+				// are correctly deduped.
+				bootclasspath_fragments: ["mybootclasspathfragment"],
+				java_sdk_libs: [
+					// This is not strictly needed as it should be automatically added to the sdk_snapshot as
+					// a java_sdk_libs module because it is used in the mybootclasspathfragment's
+					// api.stub_libs property. However, it is specified here to ensure that duplicates are
+					// correctly deduped.
+					"mysdklibrary",
+				],
+			}
+		`, copyBootclasspathFragmentFromApexVariantRules)
+	})
+}
+
 // TestSnapshotWithBootClasspathFragment_Fragments makes sure that the fragments property of a
 // bootclasspath_fragment is correctly output to the sdk snapshot.
 func TestSnapshotWithBootClasspathFragment_Fragments(t *testing.T) {

+ 74 - 17
sdk/systemserverclasspath_fragment_sdk_test.go

@@ -22,7 +22,7 @@ import (
 	"android/soong/java"
 )
 
-func testSnapshotWithSystemServerClasspathFragment(t *testing.T, targetBuildRelease string, expectedSdkSnapshot string) {
+func testSnapshotWithSystemServerClasspathFragment(t *testing.T, sdk string, targetBuildRelease string, expectedSdkSnapshot string) {
 	result := android.GroupFixturePreparers(
 		prepareForSdkTestWithJava,
 		java.PrepareForTestWithJavaDefaultModules,
@@ -30,23 +30,13 @@ func testSnapshotWithSystemServerClasspathFragment(t *testing.T, targetBuildRele
 		java.FixtureWithLastReleaseApis("mysdklibrary"),
 		dexpreopt.FixtureSetApexSystemServerJars("myapex:mylib", "myapex:mysdklibrary"),
 		android.FixtureModifyEnv(func(env map[string]string) {
-			env["SOONG_SDK_SNAPSHOT_TARGET_BUILD_RELEASE"] = targetBuildRelease
+			if targetBuildRelease != "latest" {
+				env["SOONG_SDK_SNAPSHOT_TARGET_BUILD_RELEASE"] = targetBuildRelease
+			}
 		}),
 		prepareForSdkTestWithApex,
 
-		android.FixtureWithRootAndroidBp(`
-			sdk {
-				name: "mysdk",
-				systemserverclasspath_fragments: ["mysystemserverclasspathfragment"],
-				java_sdk_libs: [
-					// This is not strictly needed as it should be automatically added to the sdk_snapshot as
-					// a java_sdk_libs module because it is used in the mysystemserverclasspathfragment's
-					// contents property. However, it is specified here to ensure that duplicates are
-					// correctly deduped.
-					"mysdklibrary",
-				],
-			}
-
+		android.FixtureWithRootAndroidBp(sdk+`
 			apex {
 				name: "myapex",
 				key: "myapex.key",
@@ -91,8 +81,62 @@ func testSnapshotWithSystemServerClasspathFragment(t *testing.T, targetBuildRele
 }
 
 func TestSnapshotWithSystemServerClasspathFragment(t *testing.T) {
+
+	commonSdk := `
+sdk {
+	name: "mysdk",
+	systemserverclasspath_fragments: ["mysystemserverclasspathfragment"],
+	java_sdk_libs: [
+		// This is not strictly needed as it should be automatically added to the sdk_snapshot as
+		// a java_sdk_libs module because it is used in the mysystemserverclasspathfragment's
+		// contents property. However, it is specified here to ensure that duplicates are
+		// correctly deduped.
+		"mysdklibrary",
+	],
+}
+	`
+
+	expectedLatestSnapshot := `
+// This is auto-generated. DO NOT EDIT.
+
+java_sdk_library_import {
+    name: "mysdklibrary",
+    prefer: false,
+    visibility: ["//visibility:public"],
+    apex_available: ["myapex"],
+    shared_library: false,
+    public: {
+        jars: ["sdk_library/public/mysdklibrary-stubs.jar"],
+        stub_srcs: ["sdk_library/public/mysdklibrary_stub_sources"],
+        current_api: "sdk_library/public/mysdklibrary.txt",
+        removed_api: "sdk_library/public/mysdklibrary-removed.txt",
+        sdk_version: "current",
+    },
+}
+
+java_import {
+    name: "mylib",
+    prefer: false,
+    visibility: ["//visibility:public"],
+    apex_available: ["myapex"],
+    jars: ["java_systemserver_libs/snapshot/jars/are/invalid/mylib.jar"],
+    permitted_packages: ["mylib"],
+}
+
+prebuilt_systemserverclasspath_fragment {
+    name: "mysystemserverclasspathfragment",
+    prefer: false,
+    visibility: ["//visibility:public"],
+    apex_available: ["myapex"],
+    contents: [
+        "mylib",
+        "mysdklibrary",
+    ],
+}
+`
+
 	t.Run("target-s", func(t *testing.T) {
-		testSnapshotWithSystemServerClasspathFragment(t, "S", `
+		testSnapshotWithSystemServerClasspathFragment(t, commonSdk, "S", `
 // This is auto-generated. DO NOT EDIT.
 
 java_sdk_library_import {
@@ -113,7 +157,7 @@ java_sdk_library_import {
 	})
 
 	t.Run("target-t", func(t *testing.T) {
-		testSnapshotWithSystemServerClasspathFragment(t, "Tiramisu", `
+		testSnapshotWithSystemServerClasspathFragment(t, commonSdk, "Tiramisu", `
 // This is auto-generated. DO NOT EDIT.
 
 java_sdk_library_import {
@@ -152,4 +196,17 @@ prebuilt_systemserverclasspath_fragment {
 }
 `)
 	})
+
+	t.Run("added-directly", func(t *testing.T) {
+		testSnapshotWithSystemServerClasspathFragment(t, commonSdk, `latest`, expectedLatestSnapshot)
+	})
+
+	t.Run("added-via-apex", func(t *testing.T) {
+		testSnapshotWithSystemServerClasspathFragment(t, `
+			sdk {
+				name: "mysdk",
+				apexes: ["myapex"],
+			}
+		`, `latest`, expectedLatestSnapshot)
+	})
 }

+ 129 - 4
sdk/update.go

@@ -433,6 +433,9 @@ be unnecessary as every module in the sdk already has its own licenses property.
 	traits := s.gatherTraits()
 	for _, member := range members {
 		memberType := member.memberType
+		if !memberType.ArePrebuiltsRequired() {
+			continue
+		}
 
 		name := member.name
 		requiredTraits := traits[name]
@@ -1319,6 +1322,119 @@ func (m multilibUsage) String() string {
 	}
 }
 
+// TODO(187910671): BEGIN - Remove once modules do not have an APEX and default variant.
+// variantCoordinate contains the coordinates used to identify a variant of an SDK member.
+type variantCoordinate struct {
+	// osType identifies the OS target of a variant.
+	osType android.OsType
+	// archId identifies the architecture and whether it is for the native bridge.
+	archId archId
+	// image is the image variant name.
+	image string
+	// linkType is the link type name.
+	linkType string
+}
+
+func getVariantCoordinate(ctx *memberContext, variant android.Module) variantCoordinate {
+	linkType := ""
+	if len(ctx.MemberType().SupportedLinkages()) > 0 {
+		linkType = getLinkType(variant)
+	}
+	return variantCoordinate{
+		osType:   variant.Target().Os,
+		archId:   archIdFromTarget(variant.Target()),
+		image:    variant.ImageVariation().Variation,
+		linkType: linkType,
+	}
+}
+
+// selectApexVariantsWhereAvailable filters the input list of variants by selecting the APEX
+// specific variant for a specific variantCoordinate when there is both an APEX and default variant.
+//
+// There is a long-standing issue where a module that is added to an APEX has both an APEX and
+// default/platform variant created even when the module does not require a platform variant. As a
+// result an indirect dependency onto a module via the APEX will use the APEX variant, whereas a
+// direct dependency onto the module will use the default/platform variant. That would result in a
+// failure while attempting to optimize the properties for a member as it would have two variants
+// when only one was expected.
+//
+// This function mitigates that problem by detecting when there are two variants that differ only
+// by apex variant, where one is the default/platform variant and one is the APEX variant. In that
+// case it picks the APEX variant. It picks the APEX variant because that is the behavior that would
+// be expected
+func selectApexVariantsWhereAvailable(ctx *memberContext, variants []android.SdkAware) []android.SdkAware {
+	moduleCtx := ctx.sdkMemberContext
+
+	// Group the variants by coordinates.
+	variantsByCoord := make(map[variantCoordinate][]android.SdkAware)
+	for _, variant := range variants {
+		coord := getVariantCoordinate(ctx, variant)
+		variantsByCoord[coord] = append(variantsByCoord[coord], variant)
+	}
+
+	toDiscard := make(map[android.SdkAware]struct{})
+	for coord, list := range variantsByCoord {
+		count := len(list)
+		if count == 1 {
+			continue
+		}
+
+		variantsByApex := make(map[string]android.SdkAware)
+		conflictDetected := false
+		for _, variant := range list {
+			apexInfo := moduleCtx.OtherModuleProvider(variant, android.ApexInfoProvider).(android.ApexInfo)
+			apexVariationName := apexInfo.ApexVariationName
+			// If there are two variants for a specific APEX variation then there is conflict.
+			if _, ok := variantsByApex[apexVariationName]; ok {
+				conflictDetected = true
+				break
+			}
+			variantsByApex[apexVariationName] = variant
+		}
+
+		// If there are more than 2 apex variations or one of the apex variations is not the
+		// default/platform variation then there is a conflict.
+		if len(variantsByApex) != 2 {
+			conflictDetected = true
+		} else if _, ok := variantsByApex[""]; !ok {
+			conflictDetected = true
+		}
+
+		// If there are no conflicts then add the default/platform variation to the list to remove.
+		if !conflictDetected {
+			toDiscard[variantsByApex[""]] = struct{}{}
+			continue
+		}
+
+		// There are duplicate variants at this coordinate and they are not the default and APEX variant
+		// so fail.
+		variantDescriptions := []string{}
+		for _, m := range list {
+			variantDescriptions = append(variantDescriptions, fmt.Sprintf("    %s", m.String()))
+		}
+
+		moduleCtx.ModuleErrorf("multiple conflicting variants detected for OsType{%s}, %s, Image{%s}, Link{%s}\n%s",
+			coord.osType, coord.archId.String(), coord.image, coord.linkType,
+			strings.Join(variantDescriptions, "\n"))
+	}
+
+	// If there are any variants to discard then remove them from the list of variants, while
+	// preserving the order.
+	if len(toDiscard) > 0 {
+		filtered := []android.SdkAware{}
+		for _, variant := range variants {
+			if _, ok := toDiscard[variant]; !ok {
+				filtered = append(filtered, variant)
+			}
+		}
+		variants = filtered
+	}
+
+	return variants
+}
+
+// TODO(187910671): END - Remove once modules do not have an APEX and default variant.
+
 type baseInfo struct {
 	Properties android.SdkMemberProperties
 }
@@ -1374,7 +1490,14 @@ func newOsTypeSpecificInfo(ctx android.SdkMemberContext, osType android.OsType,
 
 	if commonVariants, ok := variantsByArchId[commonArchId]; ok {
 		if len(osTypeVariants) != 1 {
-			panic(fmt.Errorf("Expected to only have 1 variant when arch type is common but found %d", len(osTypeVariants)))
+			variants := []string{}
+			for _, m := range osTypeVariants {
+				variants = append(variants, fmt.Sprintf("    %s", m.String()))
+			}
+			panic(fmt.Errorf("expected to only have 1 variant of %q when arch type is common but found %d\n%s",
+				ctx.Name(),
+				len(osTypeVariants),
+				strings.Join(variants, "\n")))
 		}
 
 		// A common arch type only has one variant and its properties should be treated
@@ -1854,7 +1977,8 @@ func (s *sdk) createMemberSnapshot(ctx *memberContext, member *sdkMember, bpModu
 	memberType := member.memberType
 
 	// Do not add the prefer property if the member snapshot module is a source module type.
-	config := ctx.sdkMemberContext.Config()
+	moduleCtx := ctx.sdkMemberContext
+	config := moduleCtx.Config()
 	if !memberType.UsesSourceModuleTypeInSnapshot() {
 		// Set the prefer based on the environment variable. This is a temporary work around to allow a
 		// snapshot to be created that sets prefer: true.
@@ -1879,9 +2003,10 @@ func (s *sdk) createMemberSnapshot(ctx *memberContext, member *sdkMember, bpModu
 		}
 	}
 
+	variants := selectApexVariantsWhereAvailable(ctx, member.variants)
+
 	// Group the variants by os type.
 	variantsByOsType := make(map[android.OsType][]android.Module)
-	variants := member.Variants()
 	for _, variant := range variants {
 		osType := variant.Target().Os
 		variantsByOsType[osType] = append(variantsByOsType[osType], variant)
@@ -1927,7 +2052,7 @@ func (s *sdk) createMemberSnapshot(ctx *memberContext, member *sdkMember, bpModu
 	}
 
 	// Extract properties which are common across all architectures and os types.
-	extractCommonProperties(ctx.sdkMemberContext, commonValueExtractor, commonProperties, osSpecificPropertiesContainers)
+	extractCommonProperties(moduleCtx, commonValueExtractor, commonProperties, osSpecificPropertiesContainers)
 
 	// Add the common properties to the module.
 	addSdkMemberPropertiesToSet(ctx, commonProperties, bpModule)