tequilaOS/platform_build_soong
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

apex_available tracks static dependencies

Browse source 
This change fixes a bug that apex_available is not enforced for static
dependencies. For example, a module with 'apex_available:
["//apex_available:platform"]' was able to be statically linked to any
APEX. This was happening because the check was done on the modules that
are actually installed to an APEX. Static dependencies of the modules
were not counted as they are not installed to the APEX as files.

Fixing this bug by doing the check by traversing the tree in the method
checkApexAvailability.

This change includes a few number of related changes:

1) DepIsInSameApex implementation for cc.Module was changed as well.
Previuosly, it returned false only when the dependency is actually a
stub variant of a lib. Now, it returns false when the dependency has one
or more stub variants. To understand why, we need to recall that when
there is a dependency to a lib having stubs, we actually create two
dependencies: to the non-stub variant and to the stub variant during the
DepsMutator phase. And later in the build action generation phase, we
choose one of them depending on the context. Also recall that an APEX
variant is created only when DepIsInSameApex returns true. Given these,
with the previous implementatin of DepIsInSameApex, we did create apex
variants of the non-stub variant of the dependency, while not creating
the apex variant for the stub variant. This is not right; we needlessly
created the apex variant. The extra apex variant has caused no harm so
far, but since the apex_available check became more correct, it actually
breaks the build. To fix the issue, we stop creating the APEX variant
both for non-stub and stub variants.

2) platform variant is created regardless of the apex_available value.
This is required for the case when a library X that provides stub is in
an APEX A and is configured to be available only for A. In that case,
libs in other APEX can't use the stub library since the stub library is
mutated only for apex A. By creating the platform variant for the stub
library, it can be used from outside as the default dependency variation
is set to the platform variant when creating the APEX variations.

3) The ApexAvailableWhitelist is added with the dependencies that were
revealed with this change.

Exempt-From-Owner-Approval: cherry-pick from internal

Bug: 147671264
Test: m

Merged-In: Iaedc05494085ff4e8af227a6392bdd0c338b8e6e
(cherry picked from commit fa89944c79)
Change-Id: Iaedc05494085ff4e8af227a6392bdd0c338b8e6e
This commit is contained in:
Jiyong Park 2020-01-31 02:49:53 +09:00
parent 22b08b4af6
commit 0f80c1848a
7 changed files with 906 additions and 114 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
android/apex.go
View file

@ -180,20 +180,20 @@ func (m *ApexModuleBase) checkApexAvailableProperty(mctx BaseModuleContext) {
func (m *ApexModuleBase) CreateApexVariations(mctx BottomUpMutatorContext) []Module {
if len(m.apexVariations) > 0 {
m.checkApexAvailableProperty(mctx)
sort.Strings(m.apexVariations)
variations := []string{}
availableForPlatform := mctx.Module().(ApexModule).AvailableFor(AvailableToPlatform) || mctx.Host()
if availableForPlatform {
variations = append(variations, "") // Original variation for platform
}
variations = append(variations, "") // Original variation for platform
variations = append(variations, m.apexVariations...)
defaultVariation := ""
mctx.SetDefaultDependencyVariation(&defaultVariation)
modules := mctx.CreateVariations(variations...)
for i, m := range modules {
if availableForPlatform && i == 0 {
continue
platformVariation := i == 0
if platformVariation && !mctx.Host() && !m.(ApexModule).AvailableFor(AvailableToPlatform) {
m.SkipInstall()
}
m.(ApexModule).setApexName(variations[i])
}

927
apex/apex.go
View file

File diff suppressed because it is too large Load diff

38
apex/apex_test.go
View file

@ -469,6 +469,11 @@ func TestBasicApex(t *testing.T) {
sdk_version: "none",
system_modules: "none",
compile_dex: true,
// TODO: remove //apex_available:platform
apex_available: [
"//apex_available:platform",
"myapex",
],
}
java_library {
@ -760,7 +765,7 @@ func TestApexWithStubs(t *testing.T) {
ensureNotContains(t, mylibLdFlags, "mylib3/android_arm64_armv8-a_shared_12_myapex/mylib3.so")
// Ensure that stubs libs are built without -include flags
mylib2Cflags := ctx.ModuleForTests("mylib2", "android_arm64_armv8-a_static_myapex").Rule("cc").Args["cFlags"]
mylib2Cflags := ctx.ModuleForTests("mylib2", "android_arm64_armv8-a_static").Rule("cc").Args["cFlags"]
ensureNotContains(t, mylib2Cflags, "-include ")
// Ensure that genstub is invoked with --apex
@ -886,6 +891,7 @@ func TestApexWithRuntimeLibsDependency(t *testing.T) {
stubs: {
versions: ["10", "20", "30"],
},
apex_available: [ "myapex" ],
}
cc_library {
@ -1573,6 +1579,7 @@ func TestHeaderLibsDependency(t *testing.T) {
export_include_dirs: ["my_include"],
system_shared_libs: [],
stl: "none",
apex_available: [ "myapex" ],
}
cc_library {
@ -3026,6 +3033,7 @@ func TestApexWithApps(t *testing.T) {
srcs: ["mylib.cpp"],
stl: "none",
system_shared_libs: [],
apex_available: [ "myapex" ],
}
`)
@ -3281,10 +3289,15 @@ func TestApexAvailable(t *testing.T) {
}`)
// check that libfoo and libbar are created only for myapex, but not for the platform
ensureListContains(t, ctx.ModuleVariantsForTests("libfoo"), "android_arm64_armv8-a_shared_myapex")
ensureListNotContains(t, ctx.ModuleVariantsForTests("libfoo"), "android_arm64_armv8-a_shared")
ensureListContains(t, ctx.ModuleVariantsForTests("libbar"), "android_arm64_armv8-a_shared_myapex")
ensureListNotContains(t, ctx.ModuleVariantsForTests("libbar"), "android_arm64_armv8-a_shared")
// TODO(jiyong) the checks for the platform variant are removed because we now create
// the platform variant regardless of the apex_availability. Instead, we will make sure that
// the platform variants are not used from other platform modules. When that is done,
// these checks will be replaced by expecting a specific error message that will be
// emitted when the platform variant is used.
// ensureListContains(t, ctx.ModuleVariantsForTests("libfoo"), "android_arm64_armv8-a_shared_myapex")
// ensureListNotContains(t, ctx.ModuleVariantsForTests("libfoo"), "android_arm64_armv8-a_shared")
// ensureListContains(t, ctx.ModuleVariantsForTests("libbar"), "android_arm64_armv8-a_shared_myapex")
// ensureListNotContains(t, ctx.ModuleVariantsForTests("libbar"), "android_arm64_armv8-a_shared")
ctx, _ = testApex(t, `
apex {
@ -3333,11 +3346,16 @@ func TestApexAvailable(t *testing.T) {
}`)
// shared variant of libfoo is only available to myapex
ensureListContains(t, ctx.ModuleVariantsForTests("libfoo"), "android_arm64_armv8-a_shared_myapex")
ensureListNotContains(t, ctx.ModuleVariantsForTests("libfoo"), "android_arm64_armv8-a_shared")
// but the static variant is available to both myapex and the platform
ensureListContains(t, ctx.ModuleVariantsForTests("libfoo"), "android_arm64_armv8-a_static_myapex")
ensureListContains(t, ctx.ModuleVariantsForTests("libfoo"), "android_arm64_armv8-a_static")
// TODO(jiyong) the checks for the platform variant are removed because we now create
// the platform variant regardless of the apex_availability. Instead, we will make sure that
// the platform variants are not used from other platform modules. When that is done,
// these checks will be replaced by expecting a specific error message that will be
// emitted when the platform variant is used.
// ensureListContains(t, ctx.ModuleVariantsForTests("libfoo"), "android_arm64_armv8-a_shared_myapex")
// ensureListNotContains(t, ctx.ModuleVariantsForTests("libfoo"), "android_arm64_armv8-a_shared")
// // but the static variant is available to both myapex and the platform
// ensureListContains(t, ctx.ModuleVariantsForTests("libfoo"), "android_arm64_armv8-a_static_myapex")
// ensureListContains(t, ctx.ModuleVariantsForTests("libfoo"), "android_arm64_armv8-a_static")
}
func TestOverrideApex(t *testing.T) {

2
cc/cc.go
View file

@ -2547,7 +2547,7 @@ func (c *Module) AndroidMkWriteAdditionalDependenciesForSourceAbiDiff(w io.Write
func (c *Module) DepIsInSameApex(ctx android.BaseModuleContext, dep android.Module) bool {
if depTag, ok := ctx.OtherModuleDependencyTag(dep).(DependencyTag); ok {
if cc, ok := dep.(*Module); ok && cc.IsStubs() && depTag.Shared {
if cc, ok := dep.(*Module); ok && cc.HasStubsVariants() && depTag.Shared && depTag.Library {
// dynamic dep to a stubs lib crosses APEX boundary
return false
}

7
java/app.go
View file

@ -583,6 +583,13 @@ func (a *AndroidApp) getCertString(ctx android.BaseModuleContext) string {
return String(a.overridableAppProperties.Certificate)
}
func (a *AndroidApp) DepIsInSameApex(ctx android.BaseModuleContext, dep android.Module) bool {
if IsJniDepTag(ctx.OtherModuleDependencyTag(dep)) {
return true
}
return a.Library.DepIsInSameApex(ctx, dep)
}
// For OutputFileProducer interface
func (a *AndroidApp) OutputFiles(tag string) (android.Paths, error) {
switch tag {

14
java/java.go
View file

@ -1717,8 +1717,10 @@ 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
return depTag == staticLibTag
// Dependencies other than the static linkage are all considered crossing APEX boundary
// 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()
}
func (j *Module) Stem() string {
@ -2406,6 +2408,14 @@ func (j *Import) SrcJarArgs() ([]string, android.Paths) {
return nil, nil
}
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
// 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()
}
// Add compile time check for interface implementation
var _ android.IDEInfo = (*Import)(nil)
var _ android.IDECustomizedModuleName = (*Import)(nil)

20
sdk/java_sdk_test.go
View file

@ -34,7 +34,7 @@ func TestBasicSdkWithJavaLibrary(t *testing.T) {
result := testSdkWithJava(t, `
sdk {
name: "mysdk",
java_header_libs: ["myjavalib"],
java_header_libs: ["sdkmember"],
}
sdk_snapshot {
@ -47,22 +47,36 @@ func TestBasicSdkWithJavaLibrary(t *testing.T) {
java_header_libs: ["sdkmember_mysdk_2"],
}
java_import {
java_library {
name: "sdkmember",
prefer: false,
srcs: ["Test.java"],
system_modules: "none",
sdk_version: "none",
host_supported: true,
apex_available: [
"//apex_available:platform",
"//apex_available:anyapex",
],
}
java_import {
name: "sdkmember_mysdk_1",
sdk_member_name: "sdkmember",
host_supported: true,
apex_available: [
"//apex_available:platform",
"//apex_available:anyapex",
],
}
java_import {
name: "sdkmember_mysdk_2",
sdk_member_name: "sdkmember",
host_supported: true,
apex_available: [
"//apex_available:platform",
"//apex_available:anyapex",
],
}
java_library {