Merge "Ensure that DepIsInSameApex is not called for ExcludeFromApexContentsTag"

android/apex.go

@@ -140,9 +140,24 @@ type DepIsInSameApex interface {
 	// DepIsInSameApex tests if the other module 'dep' is considered as part of the same APEX as
 	// this module. For example, a static lib dependency usually returns true here, while a
 	// shared lib dependency to a stub library returns false.
+	//
+	// This method must not be called directly without first ignoring dependencies whose tags
+	// implement ExcludeFromApexContentsTag. Calls from within the func passed to WalkPayloadDeps()
+	// are fine as WalkPayloadDeps() will ignore those dependencies automatically. Otherwise, use
+	// IsDepInSameApex instead.
 	DepIsInSameApex(ctx BaseModuleContext, dep Module) bool
 }
 
+func IsDepInSameApex(ctx BaseModuleContext, module, dep Module) bool {
+	depTag := ctx.OtherModuleDependencyTag(dep)
+	if _, ok := depTag.(ExcludeFromApexContentsTag); ok {
+		// The tag defines a dependency that never requires the child module to be part of the same
+		// apex as the parent.
+		return false
+	}
+	return module.(DepIsInSameApex).DepIsInSameApex(ctx, dep)
+}
+
 // ApexModule is the interface that a module type is expected to implement if the module has to be
 // built differently depending on whether the module is destined for an APEX or not (i.e., installed
 // to one of the regular partitions).
@@ -260,6 +275,10 @@ type ApexProperties struct {
 //
 // Unless the tag also implements the AlwaysRequireApexVariantTag this will prevent an apex variant
 // from being created for the module.
+//
+// At the moment the sdk.sdkRequirementsMutator relies on the fact that the existing tags which
+// implement this interface do not define dependencies onto members of an sdk_snapshot. If that
+// changes then sdk.sdkRequirementsMutator will need fixing.
 type ExcludeFromApexContentsTag interface {
 	blueprint.DependencyTag
 

apex/apex.go

@@ -854,12 +854,7 @@ func (a *apexBundle) ApexInfoMutator(mctx android.TopDownMutatorContext) {
 		if required, ok := depTag.(android.AlwaysRequireApexVariantTag); ok && required.AlwaysRequireApexVariant() {
 			return true
 		}
-		if _, ok := depTag.(android.ExcludeFromApexContentsTag); ok {
+		if !android.IsDepInSameApex(mctx, parent, child) {
-			// The tag defines a dependency that never requires the child module to be part of the same
-			// apex as the parent so it does not need an apex variant created.
-			return false
-		}
-		if !parent.(android.DepIsInSameApex).DepIsInSameApex(mctx, child) {
 			return false
 		}
 		if excludeVndkLibs {
@@ -1003,11 +998,7 @@ func markPlatformAvailability(mctx android.BottomUpMutatorContext) {
 	// If any of the dep is not available to platform, this module is also considered as being
 	// not available to platform even if it has "//apex_available:platform"
 	mctx.VisitDirectDeps(func(child android.Module) {
-		depTag := mctx.OtherModuleDependencyTag(child)
+		if !android.IsDepInSameApex(mctx, am, child) {
-		if _, ok := depTag.(android.ExcludeFromApexContentsTag); ok {
-			return
-		}
-		if !am.DepIsInSameApex(mctx, child) {
 			// if the dependency crosses apex boundary, don't consider it
 			return
 		}
@@ -1872,7 +1863,10 @@ func (a *apexBundle) GenerateAndroidBuildActions(ctx android.ModuleContext) {
 						// like to record requiredNativeLibs even when
 						// DepIsInSameAPex is false. We also shouldn't do
 						// this for host.
-						if !am.DepIsInSameApex(ctx, am) {
+						//
+						// TODO(jiyong): explain why the same module is passed in twice.
+						// Switching the first am to parent breaks lots of tests.
+						if !android.IsDepInSameApex(ctx, am, am) {
 							return false
 						}
 
@@ -2195,6 +2189,8 @@ func (a *apexBundle) checkStaticLinkingToStubLibraries(ctx android.ModuleContext
 
 			// If `to` is not actually in the same APEX as `from` then it does not need
 			// apex_available and neither do any of its dependencies.
+			//
+			// It is ok to call DepIsInSameApex() directly from within WalkPayloadDeps().
 			if am, ok := from.(android.DepIsInSameApex); ok && !am.DepIsInSameApex(ctx, to) {
 				// As soon as the dependency graph crosses the APEX boundary, don't go further.
 				return false
@@ -2278,6 +2274,8 @@ func (a *apexBundle) checkApexAvailability(ctx android.ModuleContext) {
 
 		// If `to` is not actually in the same APEX as `from` then it does not need
 		// apex_available and neither do any of its dependencies.
+		//
+		// It is ok to call DepIsInSameApex() directly from within WalkPayloadDeps().
 		if am, ok := from.(android.DepIsInSameApex); ok && !am.DepIsInSameApex(ctx, to) {
 			// As soon as the dependency graph crosses the APEX boundary, don't go
 			// further.

sdk/sdk.go

@@ -445,20 +445,26 @@ func memberInterVersionMutator(mctx android.BottomUpMutatorContext) {
 	}
 }
 
+// An interface that encapsulates all the functionality needed to manage the sdk dependencies.
+//
+// It is a mixture of apex and sdk module functionality.
+type sdkAndApexModule interface {
+	android.Module
+	android.DepIsInSameApex
+	android.RequiredSdks
+}
+
 // Step 4: transitively ripple down the SDK requirements from the root modules like APEX to its
 // descendants
 func sdkDepsMutator(mctx android.TopDownMutatorContext) {
-	if parent, ok := mctx.Module().(interface {
+	if parent, ok := mctx.Module().(sdkAndApexModule); ok {
-		android.DepIsInSameApex
-		android.RequiredSdks
-	}); ok {
 		// Module types for Mainline modules (e.g. APEX) are expected to implement RequiredSdks()
 		// by reading its own properties like `uses_sdks`.
 		requiredSdks := parent.RequiredSdks()
 		if len(requiredSdks) > 0 {
 			mctx.VisitDirectDeps(func(m android.Module) {
 				// Only propagate required sdks from the apex onto its contents.
-				if dep, ok := m.(android.SdkAware); ok && parent.DepIsInSameApex(mctx, dep) {
+				if dep, ok := m.(android.SdkAware); ok && android.IsDepInSameApex(mctx, parent, dep) {
 					dep.BuildWithSdks(requiredSdks)
 				}
 			})
@@ -497,10 +503,7 @@ func sdkDepsReplaceMutator(mctx android.BottomUpMutatorContext) {
 
 // Step 6: ensure that the dependencies outside of the APEX are all from the required SDKs
 func sdkRequirementsMutator(mctx android.TopDownMutatorContext) {
-	if m, ok := mctx.Module().(interface {
+	if m, ok := mctx.Module().(sdkAndApexModule); ok {
-		android.DepIsInSameApex
-		android.RequiredSdks
-	}); ok {
 		requiredSdks := m.RequiredSdks()
 		if len(requiredSdks) == 0 {
 			return
@@ -519,9 +522,18 @@ func sdkRequirementsMutator(mctx android.TopDownMutatorContext) {
 				return
 			}
 
-			// If the dep is outside of the APEX, but is not in any of the
+			// If the dep is outside of the APEX, but is not in any of the required SDKs, we know that the
-			// required SDKs, we know that the dep is a violation.
+			// dep is a violation.
 			if sa, ok := dep.(android.SdkAware); ok {
+				// It is not an error if a dependency that is excluded from the apex due to the tag is not
+				// in one of the required SDKs. That is because all of the existing tags that implement it
+				// do not depend on modules which can or should belong to an sdk_snapshot.
+				if _, ok := tag.(android.ExcludeFromApexContentsTag); ok {
+					// The tag defines a dependency that never requires the child module to be part of the
+					// same apex.
+					return
+				}
+
 				if !m.DepIsInSameApex(mctx, dep) && !requiredSdks.Contains(sa.ContainingSdk()) {
 					mctx.ModuleErrorf("depends on %q (in SDK %q) that isn't part of the required SDKs: %v",
 						sa.Name(), sa.ContainingSdk(), requiredSdks)