Skip to content

Commit

Permalink
Some tweaks to the aconfig flag collection logic
Browse files Browse the repository at this point in the history 
1. Output the aconfig flags pb and storage files to /etc
2. Fix a bug where aconfig flags were not collected for java_sdk_library

Bug: None
Test: manual and unit tests.
Change-Id: I0896e91918c1b53c98ac9dc0f4a636f158200891
  • Loading branch information
@yliu98
yliu98 committed Feb 28, 2024
1 parent 88c21f5 commit ab31c82
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 16 deletions.
10 changes: 10 additions & 0 deletions apex/apex.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2075,8 +2075,10 @@ func (a *apexBundle) depVisitor(vctx *visitorContext, ctx android.ModuleContext,
return true // track transitive dependencies
case *java.AndroidAppImport:
vctx.filesInfo = append(vctx.filesInfo, apexFilesForAndroidApp(ctx, ap)...)
addAconfigFiles(vctx, ctx, child)
case *java.AndroidTestHelperApp:
vctx.filesInfo = append(vctx.filesInfo, apexFilesForAndroidApp(ctx, ap)...)
addAconfigFiles(vctx, ctx, child)
case *java.AndroidAppSet:
appDir := "app"
if ap.Privileged() {
Expand All @@ -2090,6 +2092,7 @@ func (a *apexBundle) depVisitor(vctx *visitorContext, ctx android.ModuleContext,
af := newApexFile(ctx, ap.OutputFile(), ap.BaseModuleName(), appDirName, appSet, ap)
af.certificate = java.PresignedCertificate
vctx.filesInfo = append(vctx.filesInfo, af)
addAconfigFiles(vctx, ctx, child)
default:
ctx.PropertyErrorf("apps", "%q is not an android_app module", depName)
}
Expand Down Expand Up @@ -2118,6 +2121,7 @@ func (a *apexBundle) depVisitor(vctx *visitorContext, ctx android.ModuleContext,
case prebuiltTag:
if prebuilt, ok := child.(prebuilt_etc.PrebuiltEtcModule); ok {
vctx.filesInfo = append(vctx.filesInfo, apexFileForPrebuiltEtc(ctx, prebuilt, depName))
addAconfigFiles(vctx, ctx, child)
} else {
ctx.PropertyErrorf("prebuilts", "%q is not a prebuilt_etc module", depName)
}
Expand All @@ -2141,6 +2145,7 @@ func (a *apexBundle) depVisitor(vctx *visitorContext, ctx android.ModuleContext,
af := apexFileForExecutable(ctx, ccTest)
af.class = nativeTest
vctx.filesInfo = append(vctx.filesInfo, af)
addAconfigFiles(vctx, ctx, child)
}
return true // track transitive dependencies
} else {
Expand Down Expand Up @@ -2230,11 +2235,13 @@ func (a *apexBundle) depVisitor(vctx *visitorContext, ctx android.ModuleContext,
}

vctx.filesInfo = append(vctx.filesInfo, af)
addAconfigFiles(vctx, ctx, child)
return true // track transitive dependencies
} else if rm, ok := child.(*rust.Module); ok {
af := apexFileForRustLibrary(ctx, rm)
af.transitiveDep = true
vctx.filesInfo = append(vctx.filesInfo, af)
addAconfigFiles(vctx, ctx, child)
return true // track transitive dependencies
}
} else if cc.IsTestPerSrcDepTag(depTag) {
Expand Down Expand Up @@ -2263,6 +2270,7 @@ func (a *apexBundle) depVisitor(vctx *visitorContext, ctx android.ModuleContext,
af := apexFileForRustLibrary(ctx, rustm)
af.transitiveDep = true
vctx.filesInfo = append(vctx.filesInfo, af)
addAconfigFiles(vctx, ctx, child)
return true // track transitive dependencies
}
} else if rust.IsRlibDepTag(depTag) {
Expand All @@ -2281,6 +2289,7 @@ func (a *apexBundle) depVisitor(vctx *visitorContext, ctx android.ModuleContext,
return false
}
vctx.filesInfo = append(vctx.filesInfo, af)
addAconfigFiles(vctx, ctx, child)
return true // track transitive dependencies
default:
ctx.PropertyErrorf("bootclasspath_fragments",
Expand All @@ -2295,6 +2304,7 @@ func (a *apexBundle) depVisitor(vctx *visitorContext, ctx android.ModuleContext,
if profileAf := apexFileForJavaModuleProfile(ctx, child.(javaModule)); profileAf != nil {
vctx.filesInfo = append(vctx.filesInfo, *profileAf)
}
addAconfigFiles(vctx, ctx, child)
return true // track transitive dependencies
default:
ctx.PropertyErrorf("systemserverclasspath_fragments",
Expand Down
26 changes: 14 additions & 12 deletions apex/apex_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11120,10 +11120,10 @@ func TestAconfigFilesJavaDeps(t *testing.T) {
t.Fatalf("Expected 5 commands, got %d in:\n%s", len(copyCmds), s)
}

ensureMatches(t, copyCmds[4], "^cp -f .*/aconfig_flags.pb .*/image.apex$")
ensureMatches(t, copyCmds[5], "^cp -f .*/package.map .*/image.apex$")
ensureMatches(t, copyCmds[6], "^cp -f .*/flag.map .*/image.apex$")
ensureMatches(t, copyCmds[7], "^cp -f .*/flag.val .*/image.apex$")
ensureMatches(t, copyCmds[4], "^cp -f .*/aconfig_flags.pb .*/image.apex/etc$")
ensureMatches(t, copyCmds[5], "^cp -f .*/package.map .*/image.apex/etc$")
ensureMatches(t, copyCmds[6], "^cp -f .*/flag.map .*/image.apex/etc$")
ensureMatches(t, copyCmds[7], "^cp -f .*/flag.val .*/image.apex/etc$")

inputs := []string{
"my_aconfig_declarations_foo/intermediate.pb",
Expand Down Expand Up @@ -11244,10 +11244,10 @@ func TestAconfigFilesJavaAndCcDeps(t *testing.T) {
t.Fatalf("Expected 12 commands, got %d in:\n%s", len(copyCmds), s)
}

ensureMatches(t, copyCmds[8], "^cp -f .*/aconfig_flags.pb .*/image.apex$")
ensureMatches(t, copyCmds[9], "^cp -f .*/package.map .*/image.apex$")
ensureMatches(t, copyCmds[10], "^cp -f .*/flag.map .*/image.apex$")
ensureMatches(t, copyCmds[11], "^cp -f .*/flag.val .*/image.apex$")
ensureMatches(t, copyCmds[8], "^cp -f .*/aconfig_flags.pb .*/image.apex/etc$")
ensureMatches(t, copyCmds[9], "^cp -f .*/package.map .*/image.apex/etc$")
ensureMatches(t, copyCmds[10], "^cp -f .*/flag.map .*/image.apex/etc$")
ensureMatches(t, copyCmds[11], "^cp -f .*/flag.val .*/image.apex/etc$")

inputs := []string{
"my_aconfig_declarations_foo/intermediate.pb",
Expand Down Expand Up @@ -11385,13 +11385,15 @@ func TestAconfigFilesRustDeps(t *testing.T) {
t.Fatalf("Expected 26 commands, got %d in:\n%s", len(copyCmds), s)
}

ensureMatches(t, copyCmds[22], "^cp -f .*/aconfig_flags.pb .*/image.apex$")
ensureMatches(t, copyCmds[23], "^cp -f .*/package.map .*/image.apex$")
ensureMatches(t, copyCmds[24], "^cp -f .*/flag.map .*/image.apex$")
ensureMatches(t, copyCmds[25], "^cp -f .*/flag.val .*/image.apex$")
ensureMatches(t, copyCmds[22], "^cp -f .*/aconfig_flags.pb .*/image.apex/etc$")
ensureMatches(t, copyCmds[23], "^cp -f .*/package.map .*/image.apex/etc$")
ensureMatches(t, copyCmds[24], "^cp -f .*/flag.map .*/image.apex/etc$")
ensureMatches(t, copyCmds[25], "^cp -f .*/flag.val .*/image.apex/etc$")

inputs := []string{
"my_aconfig_declarations_foo/intermediate.pb",
"my_aconfig_declarations_bar/intermediate.pb",
"my_aconfig_declarations_baz/intermediate.pb",
"my_rust_binary/android_arm64_armv8-a_apex10000/myapex/aconfig_merged.pb",
}
VerifyAconfigRule(t, &mod, "combine_aconfig_declarations", inputs, "android_common_myapex/aconfig_flags.pb", "", "")
Expand Down
9 changes: 5 additions & 4 deletions apex/builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,7 @@ func (a *apexBundle) buildApex(ctx android.ModuleContext) {
prebuiltSdkToolsBinDir := filepath.Join("prebuilts", "sdk", "tools", runtime.GOOS, "bin")

defaultReadOnlyFiles := []string{"apex_manifest.json", "apex_manifest.pb"}
aconfigDest := imageDir.Join(ctx, "etc").String()
if len(a.aconfigFiles) > 0 {
apexAconfigFile := android.PathForModuleOut(ctx, "aconfig_flags.pb")
ctx.Build(pctx, android.BuildParams{
Expand All @@ -657,9 +658,9 @@ func (a *apexBundle) buildApex(ctx android.ModuleContext) {
},
})

copyCommands = append(copyCommands, "cp -f "+apexAconfigFile.String()+" "+imageDir.String())
copyCommands = append(copyCommands, "cp -f "+apexAconfigFile.String()+" "+aconfigDest)
implicitInputs = append(implicitInputs, apexAconfigFile)
defaultReadOnlyFiles = append(defaultReadOnlyFiles, apexAconfigFile.Base())
defaultReadOnlyFiles = append(defaultReadOnlyFiles, "etc/"+apexAconfigFile.Base())

for _, info := range createStorageInfo {
outputFile := android.PathForModuleOut(ctx, info.Output_file)
Expand All @@ -675,9 +676,9 @@ func (a *apexBundle) buildApex(ctx android.ModuleContext) {
},
})

copyCommands = append(copyCommands, "cp -f "+outputFile.String()+" "+imageDir.String())
copyCommands = append(copyCommands, "cp -f "+outputFile.String()+" "+aconfigDest)
implicitInputs = append(implicitInputs, outputFile)
defaultReadOnlyFiles = append(defaultReadOnlyFiles, outputFile.Base())
defaultReadOnlyFiles = append(defaultReadOnlyFiles, "etc/"+outputFile.Base())
}
}

Expand Down

0 comments on commit ab31c82

Please sign in to comment.