Remove duplicate component from sdk snapshot

Previously, an sdk snapshot could contain the following:
* A java_sdk_library_import module, e.g. "foo" which creates component
  modules "foo.stubs", etc.
* A corresponding versioned module, e.g. "sdk_foo@current" which
  created component modules "sdk_foo@current.stubs", etc.
* An internal (to the sdk snapshot) java_import for one of "foo"'s
  components, e.g. "sdk_foo.stubs"
* A corresponding versioned module, e.g. "sdk_foo.stubs@current".

That causes a few problems:
1. The "foo.stubs" is duplicated.
2. The names of the components created by the versioned
   java_sdk_library_import are invalid, as they append the component's
   suffix to the version and not the name before the version.

The latter causes problems when building against prebuilts and fixing
that causes the generated snapshot to be invalid because it contains
duplicate definitions of the "sdk_foo.stubs@current" module. One
explicitly in the Android.bp file and one created by the
"sdk_foo@current" module.

Removing the duplicates from the snapshot causes errors as the name
generated by the snapshot for the component module, i.e.
"sdk_foo.stubs@current" does not match the name generated by the
"sdk_foo@current", i.e. "sdk_foo@current.stubs".

This change fixes them together.

Bug: 179354495
Test: m nothing
Change-Id: I515f235fe21755b5275af12366e96c24c94c0273

android/sdk.go

@@ -38,6 +38,36 @@ type RequiredSdks interface {
 type sdkAwareWithoutModule interface {
 	RequiredSdks
 
+	// SdkMemberComponentName will return the name to use for a component of this module based on the
+	// base name of this module.
+	//
+	// The baseName is the name returned by ModuleBase.BaseModuleName(), i.e. the name specified in
+	// the name property in the .bp file so will not include the prebuilt_ prefix.
+	//
+	// The componentNameCreator is a func for creating the name of a component from the base name of
+	// the module, e.g. it could just append ".component" to the name passed in.
+	//
+	// This is intended to be called by prebuilt modules that create component models. It is because
+	// prebuilt module base names come in a variety of different forms:
+	// * unversioned - this is the same as the source module.
+	// * internal to an sdk - this is the unversioned name prefixed by the base name of the sdk
+	//   module.
+	// * versioned - this is the same as the internal with the addition of an "@<version>" suffix.
+	//
+	// While this can be called from a source module in that case it will behave the same way as the
+	// unversioned name and return the result of calling the componentNameCreator func on the supplied
+	// base name.
+	//
+	// e.g. Assuming the componentNameCreator func simply appends ".component" to the name passed in
+	// then this will work as follows:
+	// * An unversioned name of "foo" will return "foo.component".
+	// * An internal to the sdk name of "sdk_foo" will return "sdk_foo.component".
+	// * A versioned name of "sdk_foo@current" will return "sdk_foo.component@current".
+	//
+	// Note that in the latter case the ".component" suffix is added before the version. Adding it
+	// after would change the version.
+	SdkMemberComponentName(baseName string, componentNameCreator func(string) string) string
+
 	sdkBase() *SdkBase
 	MakeMemberOf(sdk SdkRef)
 	IsInAnySdk() bool
@@ -135,6 +165,18 @@ func (s *SdkBase) sdkBase() *SdkBase {
 	return s
 }
 
+func (s *SdkBase) SdkMemberComponentName(baseName string, componentNameCreator func(string) string) string {
+	if s.MemberName() == "" {
+		return componentNameCreator(baseName)
+	} else {
+		index := strings.LastIndex(baseName, "@")
+		unversionedName := baseName[:index]
+		unversionedComponentName := componentNameCreator(unversionedName)
+		versionSuffix := baseName[index:]
+		return unversionedComponentName + versionSuffix
+	}
+}
+
 // MakeMemberOf sets this module to be a member of a specific SDK
 func (s *SdkBase) MakeMemberOf(sdk SdkRef) {
 	s.properties.ContainingSdk = &sdk
@@ -643,3 +685,30 @@ type SdkMemberContext interface {
 	// into which to copy the prebuilt files.
 	Name() string
 }
+
+// ExportedComponentsInfo contains information about the components that this module exports to an
+// sdk snapshot.
+//
+// A component of a module is a child module that the module creates and which forms an integral
+// part of the functionality that the creating module provides. A component module is essentially
+// owned by its creator and is tightly coupled to the creator and other components.
+//
+// e.g. the child modules created by prebuilt_apis are not components because they are not tightly
+// coupled to the prebuilt_apis module. Once they are created the prebuilt_apis ignores them. The
+// child impl and stub library created by java_sdk_library (and corresponding import) are components
+// because the creating module depends upon them in order to provide some of its own functionality.
+//
+// A component is exported if it is part of an sdk snapshot. e.g. The xml and impl child modules are
+// components but they are not exported as they are not part of an sdk snapshot.
+//
+// This information is used by the sdk snapshot generation code to ensure that it does not create
+// an sdk snapshot that contains a declaration of the component module and the module that creates
+// it as that would result in duplicate modules when attempting to use the snapshot. e.g. a snapshot
+// that included the java_sdk_library_import "foo" and also a java_import "foo.stubs" would fail
+// as there would be two modules called "foo.stubs".
+type ExportedComponentsInfo struct {
+	// The names of the exported components.
+	Components []string
+}
+
+var ExportedComponentsInfoProvider = blueprint.NewProvider(ExportedComponentsInfo{})

java/sdk_library.go

@@ -634,7 +634,7 @@ type commonToSdkLibraryAndImportProperties struct {
 // commonSdkLibraryAndImportModule defines the interface that must be provided by a module that
 // embeds the commonToSdkLibraryAndImport struct.
 type commonSdkLibraryAndImportModule interface {
-	android.Module
+	android.SdkAware
 
 	BaseModuleName() string
 }
@@ -700,13 +700,19 @@ func (c *commonToSdkLibraryAndImport) xmlPermissionsModuleName() string {
 
 // Name of the java_library module that compiles the stubs source.
 func (c *commonToSdkLibraryAndImport) stubsLibraryModuleName(apiScope *apiScope) string {
-	return c.namingScheme.stubsLibraryModuleName(apiScope, c.module.BaseModuleName())
+	baseName := c.module.BaseModuleName()
+	return c.module.SdkMemberComponentName(baseName, func(name string) string {
+		return c.namingScheme.stubsLibraryModuleName(apiScope, name)
+	})
 }
 
 // Name of the droidstubs module that generates the stubs source and may also
 // generate/check the API.
 func (c *commonToSdkLibraryAndImport) stubsSourceModuleName(apiScope *apiScope) string {
-	return c.namingScheme.stubsSourceModuleName(apiScope, c.module.BaseModuleName())
+	baseName := c.module.BaseModuleName()
+	return c.module.SdkMemberComponentName(baseName, func(name string) string {
+		return c.namingScheme.stubsSourceModuleName(apiScope, name)
+	})
 }
 
 // The component names for different outputs of the java_sdk_library.
@@ -1170,6 +1176,10 @@ func (module *SdkLibrary) GenerateAndroidBuildActions(ctx android.ModuleContext)
 		module.Library.GenerateAndroidBuildActions(ctx)
 	}
 
+	// Collate the components exported by this module. All scope specific modules are exported but
+	// the impl and xml component modules are not.
+	exportedComponents := map[string]struct{}{}
+
 	// Record the paths to the header jars of the library (stubs and impl).
 	// When this java_sdk_library is depended upon from others via "libs" property,
 	// the recorded paths will be returned depending on the link type of the caller.
@@ -1184,8 +1194,14 @@ func (module *SdkLibrary) GenerateAndroidBuildActions(ctx android.ModuleContext)
 			// 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)
+
+			exportedComponents[ctx.OtherModuleName(to)] = struct{}{}
 		}
 	})
+
+	// Make the set of components exported by this module available for use elsewhere.
+	exportedComponentInfo := android.ExportedComponentsInfo{Components: android.SortedStringKeys(exportedComponents)}
+	ctx.SetProvider(android.ExportedComponentsInfoProvider, exportedComponentInfo)
 }
 
 func (module *SdkLibrary) AndroidMkEntries() []android.AndroidMkEntries {

java/sdk_library_test.go

@@ -110,7 +110,7 @@ func TestJavaSdkLibrary(t *testing.T) {
 		`)
 
 	// check the existence of the internal modules
-	result.ModuleForTests("foo", "android_common")
+	foo := result.ModuleForTests("foo", "android_common")
 	result.ModuleForTests(apiScopePublic.stubsLibraryModuleName("foo"), "android_common")
 	result.ModuleForTests(apiScopeSystem.stubsLibraryModuleName("foo"), "android_common")
 	result.ModuleForTests(apiScopeTest.stubsLibraryModuleName("foo"), "android_common")
@@ -122,6 +122,17 @@ func TestJavaSdkLibrary(t *testing.T) {
 	result.ModuleForTests("foo.api.system.28", "")
 	result.ModuleForTests("foo.api.test.28", "")
 
+	exportedComponentsInfo := result.ModuleProvider(foo.Module(), ExportedComponentsInfoProvider).(ExportedComponentsInfo)
+	expectedFooExportedComponents := []string{
+		"foo.stubs",
+		"foo.stubs.source",
+		"foo.stubs.source.system",
+		"foo.stubs.source.test",
+		"foo.stubs.system",
+		"foo.stubs.test",
+	}
+	android.AssertArrayString(t, "foo exported components", expectedFooExportedComponents, exportedComponentsInfo.Components)
+
 	bazJavac := result.ModuleForTests("baz", "android_common").Rule("javac")
 	// tests if baz is actually linked to the stubs lib
 	android.AssertStringDoesContain(t, "baz javac classpath", bazJavac.Args["classpath"], "foo.stubs.system.jar")

sdk/java_sdk_test.go

@@ -722,14 +722,6 @@ java_import {
     jars: ["java/system-module.jar"],
 }
 
-java_import {
-    name: "mysdk_myjavalib.stubs",
-    prefer: false,
-    visibility: ["//visibility:private"],
-    apex_available: ["//apex_available:platform"],
-    jars: ["java/myjavalib.stubs.jar"],
-}
-
 java_sdk_library_import {
     name: "myjavalib",
     prefer: false,
@@ -752,7 +744,7 @@ java_system_modules_import {
     libs: [
         "mysdk_system-module",
         "exported-system-module",
-        "mysdk_myjavalib.stubs",
+        "myjavalib.stubs",
     ],
 }
 `),
@@ -775,14 +767,6 @@ java_import {
     jars: ["java/system-module.jar"],
 }
 
-java_import {
-    name: "mysdk_myjavalib.stubs@current",
-    sdk_member_name: "myjavalib.stubs",
-    visibility: ["//visibility:private"],
-    apex_available: ["//apex_available:platform"],
-    jars: ["java/myjavalib.stubs.jar"],
-}
-
 java_sdk_library_import {
     name: "mysdk_myjavalib@current",
     sdk_member_name: "myjavalib",
@@ -820,7 +804,6 @@ sdk_snapshot {
 		checkAllCopyRules(`
 .intermediates/exported-system-module/android_common/turbine-combined/exported-system-module.jar -> java/exported-system-module.jar
 .intermediates/system-module/android_common/turbine-combined/system-module.jar -> java/system-module.jar
-.intermediates/myjavalib.stubs/android_common/turbine-combined/myjavalib.stubs.jar -> java/myjavalib.stubs.jar
 .intermediates/myjavalib.stubs/android_common/javac/myjavalib.stubs.jar -> sdk_library/public/myjavalib-stubs.jar
 .intermediates/myjavalib.stubs.source/android_common/metalava/myjavalib.stubs.source_api.txt -> sdk_library/public/myjavalib.txt
 .intermediates/myjavalib.stubs.source/android_common/metalava/myjavalib.stubs.source_removed.txt -> sdk_library/public/myjavalib-removed.txt
@@ -1153,10 +1136,10 @@ sdk_snapshot {
 			".intermediates/mysdk/common_os/tmp/sdk_library/test/myjavalib_stub_sources.zip",
 		),
 		snapshotTestChecker(checkSnapshotWithoutSource, func(t *testing.T, result *android.TestResult) {
-			// Show that the existing behavior is incorrect as the suffix for the child modules is added
+			// Make sure that the name of the child modules created by a versioned java_sdk_library_import
-			// to the version not before it.
+			// module is correct, i.e. the suffix is added before the version and not after.
-			result.Module("mysdk_myjavalib@current.stubs", "android_common")
+			result.Module("mysdk_myjavalib.stubs@current", "android_common")
-			result.Module("mysdk_myjavalib@current.stubs.source", "android_common")
+			result.Module("mysdk_myjavalib.stubs.source@current", "android_common")
 		}),
 	)
 }

sdk/update.go

@@ -22,7 +22,6 @@ import (
 
 	"android/soong/apex"
 	"android/soong/cc"
-
 	"github.com/google/blueprint"
 	"github.com/google/blueprint/proptools"
 
@@ -140,8 +139,8 @@ func (gf *generatedFile) build(pctx android.PackageContext, ctx android.BuilderC
 
 // Collect all the members.
 //
-// Updates the sdk module with a list of sdkMemberVariantDeps and details as to which multilibs
+// Updates the sdk module with a list of sdkMemberVariantDep instances and details as to which
-// (32/64/both) are used by this sdk variant.
+// multilibs (32/64/both) are used by this sdk variant.
 func (s *sdk) collectMembers(ctx android.ModuleContext) {
 	s.multilibUsages = multilibNone
 	ctx.WalkDeps(func(child android.Module, parent android.Module) bool {
@@ -157,8 +156,15 @@ func (s *sdk) collectMembers(ctx android.ModuleContext) {
 			// Keep track of which multilib variants are used by the sdk.
 			s.multilibUsages = s.multilibUsages.addArchType(child.Target().Arch.ArchType)
 
+			var exportedComponentsInfo android.ExportedComponentsInfo
+			if ctx.OtherModuleHasProvider(child, android.ExportedComponentsInfoProvider) {
+				exportedComponentsInfo = ctx.OtherModuleProvider(child, android.ExportedComponentsInfoProvider).(android.ExportedComponentsInfo)
+			}
+
 			export := memberTag.ExportMember()
-			s.memberVariantDeps = append(s.memberVariantDeps, sdkMemberVariantDep{s, memberType, child.(android.SdkAware), export})
+			s.memberVariantDeps = append(s.memberVariantDeps, sdkMemberVariantDep{
+				s, memberType, child.(android.SdkAware), export, exportedComponentsInfo,
+			})
 
 			// Recurse down into the member's dependencies as it may have dependencies that need to be
 			// automatically added to the sdk.
@@ -245,26 +251,41 @@ func versionedSdkMemberName(ctx android.ModuleContext, memberName string, versio
 // the contents (header files, stub libraries, etc) into the zip file.
 func (s *sdk) buildSnapshot(ctx android.ModuleContext, sdkVariants []*sdk) android.OutputPath {
 
-	allMembersByName := make(map[string]struct{})
+	// Aggregate all the sdkMemberVariantDep instances from all the sdk variants.
-	exportedMembersByName := make(map[string]struct{})
 	hasLicenses := false
 	var memberVariantDeps []sdkMemberVariantDep
 	for _, sdkVariant := range sdkVariants {
 		memberVariantDeps = append(memberVariantDeps, sdkVariant.memberVariantDeps...)
+	}
 
-		// Record the names of all the members, both explicitly specified and implicitly
+	// Filter out any sdkMemberVariantDep that is a component of another.
-		// included.
+	memberVariantDeps = filterOutComponents(ctx, memberVariantDeps)
-		for _, memberVariantDep := range sdkVariant.memberVariantDeps {
-			name := memberVariantDep.variant.Name()
-			allMembersByName[name] = struct{}{}
 
-			if memberVariantDep.export {
+	// Record the names of all the members, both explicitly specified and implicitly
-				exportedMembersByName[name] = struct{}{}
+	// included.
-			}
+	allMembersByName := make(map[string]struct{})
+	exportedMembersByName := make(map[string]struct{})
 
-			if memberVariantDep.memberType == android.LicenseModuleSdkMemberType {
+	addMember := func(name string, export bool) {
-				hasLicenses = true
+		allMembersByName[name] = struct{}{}
-			}
+		if export {
+			exportedMembersByName[name] = struct{}{}
+		}
+	}
+
+	for _, memberVariantDep := range memberVariantDeps {
+		name := memberVariantDep.variant.Name()
+		export := memberVariantDep.export
+
+		addMember(name, export)
+
+		// Add any components provided by the module.
+		for _, component := range memberVariantDep.exportedComponentsInfo.Components {
+			addMember(component, export)
+		}
+
+		if memberVariantDep.memberType == android.LicenseModuleSdkMemberType {
+			hasLicenses = true
 		}
 	}
 
@@ -423,6 +444,47 @@ be unnecessary as every module in the sdk already has its own licenses property.
 	return outputZipFile
 }
 
+// filterOutComponents removes any item from the deps list that is a component of another item in
+// the deps list, e.g. if the deps list contains "foo" and "foo.stubs" which is component of "foo"
+// then it will remove "foo.stubs" from the deps.
+func filterOutComponents(ctx android.ModuleContext, deps []sdkMemberVariantDep) []sdkMemberVariantDep {
+	// Collate the set of components that all the modules added to the sdk provide.
+	components := map[string]*sdkMemberVariantDep{}
+	for i, _ := range deps {
+		dep := &deps[i]
+		for _, c := range dep.exportedComponentsInfo.Components {
+			components[c] = dep
+		}
+	}
+
+	// If no module provides components then return the input deps unfiltered.
+	if len(components) == 0 {
+		return deps
+	}
+
+	filtered := make([]sdkMemberVariantDep, 0, len(deps))
+	for _, dep := range deps {
+		name := android.RemoveOptionalPrebuiltPrefix(ctx.OtherModuleName(dep.variant))
+		if owner, ok := components[name]; ok {
+			// This is a component of another module that is a member of the sdk.
+
+			// If the component is exported but the owning module is not then the configuration is not
+			// supported.
+			if dep.export && !owner.export {
+				ctx.ModuleErrorf("Module %s is internal to the SDK but provides component %s which is used outside the SDK")
+				continue
+			}
+
+			// This module must not be added to the list of members of the sdk as that would result in a
+			// duplicate module in the sdk snapshot.
+			continue
+		}
+
+		filtered = append(filtered, dep)
+	}
+	return filtered
+}
+
 // addSnapshotModule adds the sdk_snapshot/module_exports_snapshot module to the builder.
 func (s *sdk) addSnapshotModule(ctx android.ModuleContext, builder *snapshotBuilder, sdkVariants []*sdk, memberVariantDeps []sdkMemberVariantDep) {
 	bpFile := builder.bpFile
@@ -1086,9 +1148,18 @@ func addSdkMemberPropertiesToSet(ctx *memberContext, memberProperties android.Sd
 type sdkMemberVariantDep struct {
 	// The sdk variant that depends (possibly indirectly) on the member variant.
 	sdkVariant *sdk
+
+	// The type of sdk member the variant is to be treated as.
 	memberType android.SdkMemberType
-	variant    android.SdkAware
+
-	export     bool
+	// The variant that is added to the sdk.
+	variant android.SdkAware
+
+	// True if the member should be exported, i.e. accessible, from outside the sdk.
+	export bool
+
+	// The names of additional component modules provided by the variant.
+	exportedComponentsInfo android.ExportedComponentsInfo
 }
 
 var _ android.SdkMember = (*sdkMember)(nil)