Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5e9013be authored by Jooyung Han's avatar Jooyung Han
Browse files

Fix apex_available

Checking apex_available was missing some corner cases.
For example, the deps of share deps of cc_library modules are missed
while those from cc_library_shared are correctly tracked.

This was due to..

* calling DepIsInSameApex in WalkDeps: both work fine separately, but
when they are used together, it fails to work. It's due to how WalkDeps
works. (We might fix this bug too risky since it is used very widely)
* incorrect receiver for DepIsInSameApex in apex_deps mutator: receiver
is supposed to be parent, but child was used before. Interestingly lots
of deps are within the same group of module types(cc to cc, java to
java), it has worked. (note that receiver's DepIsInSameApex
implementation can be different).

This change fixes them by..

* walkPayloadDeps is now relying on ApexVariation, which is calculated
correctly by TopDown apex_deps mutator.
* use correct receiver for DepIsInSameApex in apex_deps mutator, which
requires for java.SdkLibrary to override the method and for
java.Library/Import to use passed dep instead of receiver to check its
membership of sdk.

Bug: 151071238
Test: build/boot
Change-Id: I0569ef4bb8e79635e4d97a89f421a8d8b7d26456
parent a8166862
Loading
Loading
Loading
Loading
+33 −14
Original line number Diff line number Diff line
@@ -221,8 +221,11 @@ func makeApexAvailableWhitelist() map[string][]string {
		"bluetooth-protos-lite",
		"bluetooth.mapsapi",
		"com.android.vcard",
		"dnsresolver_aidl_interface-V2-java",
		"fmtlib",
		"guava",
		"ipmemorystore-aidl-interfaces-V5-java",
		"ipmemorystore-aidl-interfaces-java",
		"internal_include_headers",
		"lib-bt-packets",
		"lib-bt-packets-avrcp",
@@ -288,6 +291,12 @@ func makeApexAvailableWhitelist() map[string][]string {
		"libutils_headers",
		"libz",
		"media_plugin_headers",
		"net-utils-services-common",
		"netd_aidl_interface-unstable-java",
		"netd_event_listener_interface-java",
		"netlink-client",
		"networkstack-aidl-interfaces-unstable-java",
		"networkstack-client",
		"sap-api-java-static",
		"services.net",
	}
@@ -305,6 +314,7 @@ func makeApexAvailableWhitelist() map[string][]string {
		"libcrypto",
		"libnativehelper_header_only",
		"libssl",
		"unsupportedappusage",
	}
	//
	// Module separator
@@ -328,6 +338,7 @@ func makeApexAvailableWhitelist() map[string][]string {
		"cronet_impl_platform_java",
		"libcronet.80.0.3986.0",
		"org.chromium.net.cronet",
		"org.chromium.net.cronet.xml",
		"prebuilt_libcronet.80.0.3986.0",
	}
	//
@@ -566,6 +577,7 @@ func makeApexAvailableWhitelist() map[string][]string {
		"libFLAC-config",
		"libFLAC-headers",
		"libFraunhoferAAC",
		"libLibGuiProperties",
		"libarect",
		"libasync_safe",
		"libaudio_system_headers",
@@ -581,6 +593,7 @@ func makeApexAvailableWhitelist() map[string][]string {
		"libbase",
		"libbase_headers",
		"libbinder_headers",
		"libbinderthreadstateutils",
		"libbluetooth-types-header",
		"libbufferhub_headers",
		"libc++",
@@ -783,6 +796,7 @@ func makeApexAvailableWhitelist() map[string][]string {
		"libdexfile_external_headers",
		"libdexfile_support",
		"libdexfile_support_static",
		"libdl_static",
		"libgtest_prod",
		"libjemalloc5",
		"liblinker_main",
@@ -874,6 +888,7 @@ func makeApexAvailableWhitelist() map[string][]string {
	m["com.android.wifi"] = []string{
		"PlatformProperties",
		"android.hardware.wifi-V1.0-java",
		"android.hardware.wifi-V1.0-java-constants",
		"android.hardware.wifi-V1.1-java",
		"android.hardware.wifi-V1.2-java",
		"android.hardware.wifi-V1.3-java",
@@ -894,6 +909,8 @@ func makeApexAvailableWhitelist() map[string][]string {
		"bouncycastle-unbundled",
		"dnsresolver_aidl_interface-V2-java",
		"error_prone_annotations",
		"framework-wifi-pre-jarjar",
		"framework-wifi-util-lib",
		"ipmemorystore-aidl-interfaces-V3-java",
		"ipmemorystore-aidl-interfaces-java",
		"ksoap2",
@@ -1030,14 +1047,11 @@ func apexDepsMutator(mctx android.TopDownMutatorContext) {
	var directDep bool
	if a, ok := mctx.Module().(*apexBundle); ok && !a.vndkApex {
		minSdkVersion := a.minSdkVersion(mctx)

		apexBundles = []android.ApexInfo{
			android.ApexInfo{
		apexBundles = []android.ApexInfo{android.ApexInfo{
			ApexName:               mctx.ModuleName(),
			LegacyAndroid10Support: proptools.Bool(a.properties.Legacy_android10_support),
			MinSdkVersion:          minSdkVersion,
			},
		}
		}}
		directDep = true
	} else if am, ok := mctx.Module().(android.ApexModule); ok {
		apexBundles = am.ApexVariations()
@@ -1048,10 +1062,14 @@ func apexDepsMutator(mctx android.TopDownMutatorContext) {
		return
	}

	cur := mctx.Module().(interface {
		DepIsInSameApex(android.BaseModuleContext, android.Module) bool
	})

	mctx.VisitDirectDeps(func(child android.Module) {
		depName := mctx.OtherModuleName(child)
		if am, ok := child.(android.ApexModule); ok && am.CanHaveApexVariants() &&
			(directDep || am.DepIsInSameApex(mctx, child)) {
			cur.DepIsInSameApex(mctx, child) {
			android.UpdateApexDependency(apexBundles, depName, directDep)
			am.BuildForApexes(apexBundles)
		}
@@ -1964,7 +1982,7 @@ func (a *apexBundle) walkPayloadDeps(ctx android.ModuleContext,
		}

		// Check for the indirect dependencies if it is considered as part of the APEX
		if am.DepIsInSameApex(ctx, am) {
		if am.ApexName() != "" {
			do(ctx, parent, am, false /* externalDep */)
			return true
		}
@@ -1997,10 +2015,12 @@ func (a *apexBundle) checkApexAvailability(ctx android.ModuleContext) {

	a.walkPayloadDeps(ctx, func(ctx android.ModuleContext, from blueprint.Module, to android.ApexModule, externalDep bool) {
		apexName := ctx.ModuleName()
		if externalDep || to.AvailableFor(apexName) || whitelistedApexAvailable(apexName, to) {
		fromName := ctx.OtherModuleName(from)
		toName := ctx.OtherModuleName(to)
		if externalDep || to.AvailableFor(apexName) || whitelistedApexAvailable(apexName, toName) {
			return
		}
		ctx.ModuleErrorf("%q requires %q that is not available for the APEX.", from.Name(), to.Name())
		ctx.ModuleErrorf("%q requires %q that is not available for the APEX.", fromName, toName)
	})
}

@@ -2377,13 +2397,12 @@ func (a *apexBundle) GenerateAndroidBuildActions(ctx android.ModuleContext) {
	a.buildApexDependencyInfo(ctx)
}

func whitelistedApexAvailable(apex string, module android.Module) bool {
func whitelistedApexAvailable(apex, moduleName string) bool {
	key := apex
	key = strings.Replace(key, "test_", "", 1)
	key = strings.Replace(key, "com.android.art.debug", "com.android.art", 1)
	key = strings.Replace(key, "com.android.art.release", "com.android.art", 1)

	moduleName := module.Name()
	// Prebuilt modules (e.g. java_import, etc.) have "prebuilt_" prefix added by the build
	// system. Trim the prefix for the check since they are confusing
	moduleName = strings.TrimPrefix(moduleName, "prebuilt_")
+15 −17
Original line number Diff line number Diff line
@@ -3164,6 +3164,7 @@ func TestErrorsIfDepsAreNotEnabled(t *testing.T) {
			stl: "none",
			system_shared_libs: [],
			enabled: false,
			apex_available: ["myapex"],
		}
	`)
	testApexError(t, `module "myapex" .* depends on disabled module "myjar"`, `
@@ -3185,6 +3186,7 @@ func TestErrorsIfDepsAreNotEnabled(t *testing.T) {
			sdk_version: "none",
			system_modules: "none",
			enabled: false,
			apex_available: ["myapex"],
		}
	`)
}
@@ -3345,7 +3347,7 @@ func TestApexWithTestHelperApp(t *testing.T) {

func TestApexPropertiesShouldBeDefaultable(t *testing.T) {
	// libfoo's apex_available comes from cc_defaults
	testApexError(t, `"myapex" .*: "myapex" requires "libfoo" that is not available for the APEX`, `
	testApexError(t, `requires "libfoo" that is not available for the APEX`, `
	apex {
		name: "myapex",
		key: "myapex.key",
@@ -3411,8 +3413,8 @@ func TestApexAvailable(t *testing.T) {
		apex_available: ["otherapex"],
	}`)

	// libbar is an indirect dep
	testApexError(t, "requires \"libbar\" that is not available for the APEX", `
	// libbbaz is an indirect dep
	testApexError(t, "requires \"libbaz\" that is not available for the APEX", `
	apex {
		name: "myapex",
		key: "myapex.key",
@@ -3425,31 +3427,26 @@ func TestApexAvailable(t *testing.T) {
		private_key: "testkey.pem",
	}

	apex {
		name: "otherapex",
		key: "otherapex.key",
		native_shared_libs: ["libfoo"],
	}

	apex_key {
		name: "otherapex.key",
		public_key: "testkey.avbpubkey",
		private_key: "testkey.pem",
	}

	cc_library {
		name: "libfoo",
		stl: "none",
		shared_libs: ["libbar"],
		system_shared_libs: [],
		apex_available: ["myapex", "otherapex"],
		apex_available: ["myapex"],
	}

	cc_library {
		name: "libbar",
		stl: "none",
		shared_libs: ["libbaz"],
		system_shared_libs: [],
		apex_available: ["myapex"],
	}

	cc_library {
		name: "libbaz",
		stl: "none",
		system_shared_libs: [],
		apex_available: ["otherapex"],
	}`)

	testApexError(t, "\"otherapex\" is not a valid module name", `
@@ -3796,6 +3793,7 @@ func TestRejectNonInstallableJavaLibrary(t *testing.T) {
			sdk_version: "none",
			system_modules: "none",
			compile_dex: false,
			apex_available: ["myapex"],
		}
	`)
}
+14 −4
Original line number Diff line number Diff line
@@ -1796,11 +1796,16 @@ func (j *Module) hasCode(ctx android.ModuleContext) bool {
}

func (j *Module) DepIsInSameApex(ctx android.BaseModuleContext, dep android.Module) bool {
	depTag := ctx.OtherModuleDependencyTag(dep)
	// Dependencies other than the static linkage are all considered crossing APEX boundary
	if staticLibTag == ctx.OtherModuleDependencyTag(dep) {
		return true
	}
	// Also, a dependency to an sdk member is also considered as such. This is required because
	// sdk members should be mutated into APEXes. Refer to sdk.sdkDepsReplaceMutator.
	return depTag == staticLibTag || j.IsInAnySdk()
	if sa, ok := dep.(android.SdkAware); ok && sa.IsInAnySdk() {
		return true
	}
	return false
}

func (j *Module) Stem() string {
@@ -2504,11 +2509,16 @@ func (j *Import) SrcJarArgs() ([]string, android.Paths) {
}

func (j *Import) DepIsInSameApex(ctx android.BaseModuleContext, dep android.Module) bool {
	depTag := ctx.OtherModuleDependencyTag(dep)
	// dependencies other than the static linkage are all considered crossing APEX boundary
	if staticLibTag == ctx.OtherModuleDependencyTag(dep) {
		return true
	}
	// Also, a dependency to an sdk member is also considered as such. This is required because
	// sdk members should be mutated into APEXes. Refer to sdk.sdkDepsReplaceMutator.
	return depTag == staticLibTag || j.IsInAnySdk()
	if sa, ok := dep.(android.SdkAware); ok && sa.IsInAnySdk() {
		return true
	}
	return false
}

// Add compile time check for interface implementation
+12 −2
Original line number Diff line number Diff line
@@ -581,6 +581,14 @@ func (module *SdkLibrary) createStubsSources(mctx android.LoadHookContext, apiSc
	mctx.CreateModule(DroidstubsFactory, &props)
}

func (module *SdkLibrary) DepIsInSameApex(mctx android.BaseModuleContext, dep android.Module) bool {
	depTag := mctx.OtherModuleDependencyTag(dep)
	if depTag == xmlPermissionsFileTag {
		return true
	}
	return module.Library.DepIsInSameApex(mctx, dep)
}

// Creates the xml file that publicizes the runtime library
func (module *SdkLibrary) createXmlFile(mctx android.LoadHookContext) {
	props := struct {
@@ -590,9 +598,11 @@ func (module *SdkLibrary) createXmlFile(mctx android.LoadHookContext) {
		Device_specific     *bool
		Product_specific    *bool
		System_ext_specific *bool
		Apex_available      []string
	}{
		Name:           proptools.StringPtr(module.xmlFileName()),
		Lib_name:       proptools.StringPtr(module.BaseModuleName()),
		Apex_available: module.ApexProperties.Apex_available,
	}

	if module.SocSpecific() {
+2 −1
Original line number Diff line number Diff line
@@ -58,6 +58,7 @@ func TestDepNotInRequiredSdks(t *testing.T) {
			sdk_version: "none",
			compile_dex: true,
			host_supported: true,
			apex_available: ["myapex"],
		}

		// this lib is no in mysdk