From ff60a73d899d2d0d1c23eca503a2aeb3fdbd1feb Mon Sep 17 00:00:00 2001 From: Dan Willemsen Date: Thu, 13 Jun 2019 16:52:01 +0000 Subject: [PATCH 1/2] Depend on all the files from system modules Instead of just one of the files that we pass into javac. Test: treehugger Change-Id: I8478e88656487c9f667893d7c17839f0ea63c78f --- java/builder.go | 23 ++++++++++++----------- java/droiddoc.go | 6 ++++-- java/java.go | 7 +++++-- java/sdk_test.go | 2 +- java/system_modules.go | 29 ++++++++++++++++++----------- 5 files changed, 40 insertions(+), 27 deletions(-) diff --git a/java/builder.go b/java/builder.go index e1a912b2f..a48e8b1a2 100644 --- a/java/builder.go +++ b/java/builder.go @@ -148,15 +148,16 @@ func init() { } type javaBuilderFlags struct { - javacFlags string - bootClasspath classpath - classpath classpath - processorPath classpath - processor string - systemModules classpath - aidlFlags string - aidlDeps android.Paths - javaVersion string + javacFlags string + bootClasspath classpath + classpath classpath + processorPath classpath + processor string + systemModules classpath + systemModulesDeps android.Paths + aidlFlags string + aidlDeps android.Paths + javaVersion string errorProneExtraJavacFlags string errorProneProcessorPath classpath @@ -248,7 +249,7 @@ func transformJavaToClasses(ctx android.ModuleContext, outputFile android.Writab var bootClasspath string if flags.javaVersion == "1.9" { - deps = append(deps, flags.systemModules...) + deps = append(deps, flags.systemModulesDeps...) bootClasspath = flags.systemModules.FormJavaSystemModulesPath("--system=", ctx.Device()) } else { deps = append(deps, flags.bootClasspath...) @@ -430,7 +431,7 @@ func (x *classpath) FormJavaSystemModulesPath(optName string, forceEmpty bool) s if len(*x) > 1 { panic("more than one system module") } else if len(*x) == 1 { - return optName + strings.TrimSuffix((*x)[0].String(), "lib/modules") + return optName + (*x)[0].String() } else if forceEmpty { return optName + "none" } else { diff --git a/java/droiddoc.go b/java/droiddoc.go index a8cf1c034..be1b28153 100644 --- a/java/droiddoc.go +++ b/java/droiddoc.go @@ -692,10 +692,11 @@ func (j *Javadoc) collectDeps(ctx android.ModuleContext) deps { panic("Found two system module dependencies") } sm := module.(*SystemModules) - if sm.outputFile == nil { + if sm.outputDir == nil && len(sm.outputDeps) == 0 { panic("Missing directory for system module dependency") } - deps.systemModules = sm.outputFile + deps.systemModules = sm.outputDir + deps.systemModulesDeps = sm.outputDeps } }) // do not pass exclude_srcs directly when expanding srcFiles since exclude_srcs @@ -776,6 +777,7 @@ func (j *Javadoc) GenerateAndroidBuildActions(ctx android.ModuleContext) { if deps.systemModules != nil { systemModules = append(systemModules, deps.systemModules) } + implicits = append(implicits, deps.systemModulesDeps...) bootClasspathArgs = systemModules.FormJavaSystemModulesPath("--system ", ctx.Device()) bootClasspathArgs = bootClasspathArgs + " --patch-module java.base=." } diff --git a/java/java.go b/java/java.go index 4b3845161..a2e9ab023 100644 --- a/java/java.go +++ b/java/java.go @@ -628,6 +628,7 @@ type deps struct { srcs android.Paths srcJars android.Paths systemModules android.Path + systemModulesDeps android.Paths aidlPreprocess android.OptionalPath kotlinStdlib android.Paths kotlinAnnotations android.Paths @@ -835,10 +836,11 @@ func (j *Module) collectDeps(ctx android.ModuleContext) deps { panic("Found two system module dependencies") } sm := module.(*SystemModules) - if sm.outputFile == nil { + if sm.outputDir == nil || len(sm.outputDeps) == 0 { panic("Missing directory for system module dependency") } - deps.systemModules = sm.outputFile + deps.systemModules = sm.outputDir + deps.systemModulesDeps = sm.outputDeps } } }) @@ -968,6 +970,7 @@ func (j *Module) collectBuilderFlags(ctx android.ModuleContext, deps deps) javaB // systemModules if deps.systemModules != nil { flags.systemModules = append(flags.systemModules, deps.systemModules) + flags.systemModulesDeps = append(flags.systemModulesDeps, deps.systemModulesDeps...) } // aidl flags. diff --git a/java/sdk_test.go b/java/sdk_test.go index 915333ec9..953c3722f 100644 --- a/java/sdk_test.go +++ b/java/sdk_test.go @@ -254,7 +254,7 @@ func TestClasspath(t *testing.T) { if testcase.system == "none" { system = "--system=none" } else if testcase.system != "" { - system = "--system=" + filepath.Join(buildDir, ".intermediates", testcase.system, "android_common", "system") + "/" + system = "--system=" + filepath.Join(buildDir, ".intermediates", testcase.system, "android_common", "system") } checkClasspath := func(t *testing.T, ctx *android.TestContext) { diff --git a/java/system_modules.go b/java/system_modules.go index 5a86f3c82..f71f4523f 100644 --- a/java/system_modules.go +++ b/java/system_modules.go @@ -61,7 +61,7 @@ var ( "moduleName", "classpath", "outDir", "workDir") ) -func TransformJarsToSystemModules(ctx android.ModuleContext, moduleName string, jars android.Paths) android.WritablePath { +func TransformJarsToSystemModules(ctx android.ModuleContext, moduleName string, jars android.Paths) (android.Path, android.Paths) { outDir := android.PathForModuleOut(ctx, "system") workDir := android.PathForModuleOut(ctx, "modules") outputFile := android.PathForModuleOut(ctx, "system/lib/modules") @@ -84,7 +84,7 @@ func TransformJarsToSystemModules(ctx android.ModuleContext, moduleName string, }, }) - return outputFile + return outDir, outputs.Paths() } func SystemModulesFactory() android.Module { @@ -101,7 +101,8 @@ type SystemModules struct { properties SystemModulesProperties - outputFile android.Path + outputDir android.Path + outputDeps android.Paths } type SystemModulesProperties struct { @@ -117,7 +118,7 @@ func (system *SystemModules) GenerateAndroidBuildActions(ctx android.ModuleConte jars = append(jars, dep.HeaderJars()...) }) - system.outputFile = TransformJarsToSystemModules(ctx, "java.base", jars) + system.outputDir, system.outputDeps = TransformJarsToSystemModules(ctx, "java.base", jars) } func (system *SystemModules) DepsMutator(ctx android.BottomUpMutatorContext) { @@ -127,16 +128,22 @@ func (system *SystemModules) DepsMutator(ctx android.BottomUpMutatorContext) { func (system *SystemModules) AndroidMk() android.AndroidMkData { return android.AndroidMkData{ Custom: func(w io.Writer, name, prefix, moduleDir string, data android.AndroidMkData) { - makevar := "SOONG_SYSTEM_MODULES_" + name fmt.Fprintln(w) - fmt.Fprintln(w, makevar, ":=", system.outputFile.String()) - fmt.Fprintln(w, ".KATI_READONLY", ":=", makevar) + + makevar := "SOONG_SYSTEM_MODULES_" + name + fmt.Fprintln(w, makevar, ":=$=", system.outputDir.String()) + fmt.Fprintln(w) + + makevar = "SOONG_SYSTEM_MODULES_LIBS_" + name + fmt.Fprintln(w, makevar, ":=$=", strings.Join(system.properties.Libs, " ")) + fmt.Fprintln(w) + + makevar = "SOONG_SYSTEM_MODULE_DEPS_" + name + fmt.Fprintln(w, makevar, ":=$=", strings.Join(system.outputDeps.Strings(), " ")) + fmt.Fprintln(w) + fmt.Fprintln(w, name+":", "$("+makevar+")") fmt.Fprintln(w, ".PHONY:", name) - fmt.Fprintln(w) - makevar = "SOONG_SYSTEM_MODULES_LIBS_" + name - fmt.Fprintln(w, makevar, ":=", strings.Join(system.properties.Libs, " ")) - fmt.Fprintln(w, ".KATI_READONLY :=", makevar) }, } } From 0f41678d008f433b5d88c2d3e211e8cdde08502c Mon Sep 17 00:00:00 2001 From: Dan Willemsen Date: Thu, 13 Jun 2019 21:44:53 +0000 Subject: [PATCH 2/2] Expand the dexpreopt image dependencies to entire image Instead of just depending on the main .art file (boot.art, etc), also expose the dependencies to the .oat/.vdex files (boot.oat/boot.vdex), and all of the module files that get implicitly loading (boot-ext.*, boot-framework.*, etc) This is necessary for RBE, where the rule only gets the files that it depends upon. Test: treehugger Test: build a system image with RBE Change-Id: I0c7051f18582f1891d3398b46763b1521e4326c8 --- dexpreopt/config.go | 8 +++-- dexpreopt/dexpreopt.go | 7 +++-- dexpreopt/dexpreopt_test.go | 1 + java/dexpreopt.go | 7 +++-- java/dexpreopt_bootjars.go | 61 ++++++++++++++++++++++--------------- java/dexpreopt_config.go | 48 +++++++++++++++++++---------- 6 files changed, 85 insertions(+), 47 deletions(-) diff --git a/dexpreopt/config.go b/dexpreopt/config.go index 5a1bd74a5..a2f1af4a6 100644 --- a/dexpreopt/config.go +++ b/dexpreopt/config.go @@ -121,8 +121,9 @@ type ModuleConfig struct { UsesLibraries []string LibraryPaths map[string]android.Path - Archs []android.ArchType - DexPreoptImages []android.Path + Archs []android.ArchType + DexPreoptImages []android.Path + DexPreoptImagesDeps []android.Paths PreoptBootClassPathDexFiles android.Paths // file paths of boot class path files PreoptBootClassPathDexLocations []string // virtual locations of boot class path files @@ -257,6 +258,9 @@ func LoadModuleConfig(ctx android.PathContext, path string) (ModuleConfig, error config.ModuleConfig.StripInputPath = constructPath(ctx, config.StripInputPath) config.ModuleConfig.StripOutputPath = constructWritablePath(ctx, config.StripOutputPath) + // This needs to exist, but dependencies are already handled in Make, so we don't need to pass them through JSON. + config.ModuleConfig.DexPreoptImagesDeps = make([]android.Paths, len(config.ModuleConfig.DexPreoptImages)) + return config.ModuleConfig, nil } diff --git a/dexpreopt/dexpreopt.go b/dexpreopt/dexpreopt.go index 0be37d0ff..e02e60f2f 100644 --- a/dexpreopt/dexpreopt.go +++ b/dexpreopt/dexpreopt.go @@ -125,7 +125,8 @@ func GenerateDexpreoptRule(ctx android.PathContext, for i, arch := range module.Archs { image := module.DexPreoptImages[i] - dexpreoptCommand(ctx, global, module, rule, arch, profile, image, appImage, generateDM) + imageDeps := module.DexPreoptImagesDeps[i] + dexpreoptCommand(ctx, global, module, rule, arch, profile, image, imageDeps, appImage, generateDM) } } } @@ -190,7 +191,7 @@ func profileCommand(ctx android.PathContext, global GlobalConfig, module ModuleC } func dexpreoptCommand(ctx android.PathContext, global GlobalConfig, module ModuleConfig, rule *android.RuleBuilder, - arch android.ArchType, profile, bootImage android.Path, appImage, generateDM bool) { + arch android.ArchType, profile, bootImage android.Path, bootImageDeps android.Paths, appImage, generateDM bool) { // HACK: make soname in Soong-generated .odex files match Make. base := filepath.Base(module.DexLocation) @@ -353,7 +354,7 @@ func dexpreoptCommand(ctx android.PathContext, global GlobalConfig, module Modul Flag("--runtime-arg").FlagWithList("-Xbootclasspath-locations:", module.PreoptBootClassPathDexLocations, ":"). Flag("${class_loader_context_arg}"). Flag("${stored_class_loader_context_arg}"). - FlagWithArg("--boot-image=", bootImageLocation).Implicit(bootImage). + FlagWithArg("--boot-image=", bootImageLocation).Implicits(bootImageDeps). FlagWithInput("--dex-file=", module.DexPath). FlagWithArg("--dex-location=", dexLocationArg). FlagWithOutput("--oat-file=", odexPath).ImplicitOutput(vdexPath). diff --git a/dexpreopt/dexpreopt_test.go b/dexpreopt/dexpreopt_test.go index 0402f8754..7f1fe4276 100644 --- a/dexpreopt/dexpreopt_test.go +++ b/dexpreopt/dexpreopt_test.go @@ -38,6 +38,7 @@ func testModuleConfig(ctx android.PathContext) ModuleConfig { LibraryPaths: nil, Archs: []android.ArchType{android.Arm}, DexPreoptImages: android.Paths{android.PathForTesting("system/framework/arm/boot.art")}, + DexPreoptImagesDeps: []android.Paths{android.Paths{}}, PreoptBootClassPathDexFiles: nil, PreoptBootClassPathDexLocations: nil, PreoptExtractedApk: false, diff --git a/java/dexpreopt.go b/java/dexpreopt.go index 23d2aa6e3..ed12fe6d2 100644 --- a/java/dexpreopt.go +++ b/java/dexpreopt.go @@ -132,8 +132,10 @@ func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, dexJarFile android.Mo } var images android.Paths + var imagesDeps []android.Paths for _, arch := range archs { images = append(images, bootImage.images[arch]) + imagesDeps = append(imagesDeps, bootImage.imagesDeps[arch]) } dexLocation := android.InstallPathToOnDevicePath(ctx, d.installPath) @@ -173,8 +175,9 @@ func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, dexJarFile android.Mo UsesLibraries: d.usesLibs, LibraryPaths: d.libraryPaths, - Archs: archs, - DexPreoptImages: images, + Archs: archs, + DexPreoptImages: images, + DexPreoptImagesDeps: imagesDeps, // We use the dex paths and dex locations of the default boot image, as it // contains the full dexpreopt boot classpath. Other images may just contain a subset of diff --git a/java/dexpreopt_bootjars.go b/java/dexpreopt_bootjars.go index 2a1a901b7..eb735c162 100644 --- a/java/dexpreopt_bootjars.go +++ b/java/dexpreopt_bootjars.go @@ -58,9 +58,32 @@ type bootImageConfig struct { symbolsDir android.OutputPath targets []android.Target images map[android.ArchType]android.OutputPath + imagesDeps map[android.ArchType]android.Paths zip android.WritablePath } +func (image bootImageConfig) moduleFiles(ctx android.PathContext, dir android.OutputPath, exts ...string) []android.OutputPath { + ret := make([]android.OutputPath, 0, len(image.modules)*len(exts)) + + // dex preopt on the bootclasspath produces multiple files. The first dex file + // is converted into to 'name'.art (to match the legacy assumption that 'name'.art + // exists), and the rest are converted to 'name'-.art. + // In addition, each .art file has an associated .oat and .vdex file, and an + // unstripped .oat file + for i, m := range image.modules { + name := image.name + if i != 0 { + name += "-" + m + } + + for _, ext := range exts { + ret = append(ret, dir.Join(ctx, name+ext)) + } + } + + return ret +} + type bootImage struct { bootImageConfig @@ -302,49 +325,38 @@ func buildBootImageRuleForArch(ctx android.SingletonContext, image *bootImage, installDir := filepath.Join("/system/framework", arch.String()) vdexInstallDir := filepath.Join("/system/framework") - var extraFiles android.WritablePaths var vdexInstalls android.RuleBuilderInstalls var unstrippedInstalls android.RuleBuilderInstalls var zipFiles android.WritablePaths - // dex preopt on the bootclasspath produces multiple files. The first dex file - // is converted into to 'name'.art (to match the legacy assumption that 'name'.art - // exists), and the rest are converted to 'name'-.art. - // In addition, each .art file has an associated .oat and .vdex file, and an - // unstripped .oat file - for i, m := range image.modules { - name := image.name - if i != 0 { - name += "-" + m - } + for _, artOrOat := range image.moduleFiles(ctx, outputDir, ".art", ".oat") { + cmd.ImplicitOutput(artOrOat) + zipFiles = append(zipFiles, artOrOat) - art := outputDir.Join(ctx, name+".art") - oat := outputDir.Join(ctx, name+".oat") - vdex := outputDir.Join(ctx, name+".vdex") - unstrippedOat := symbolsDir.Join(ctx, name+".oat") + // Install the .oat and .art files + rule.Install(artOrOat, filepath.Join(installDir, artOrOat.Base())) + } - extraFiles = append(extraFiles, art, oat, vdex, unstrippedOat) - - zipFiles = append(zipFiles, art, oat, vdex) - - // Install the .oat and .art files. - rule.Install(art, filepath.Join(installDir, art.Base())) - rule.Install(oat, filepath.Join(installDir, oat.Base())) + for _, vdex := range image.moduleFiles(ctx, outputDir, ".vdex") { + cmd.ImplicitOutput(vdex) + zipFiles = append(zipFiles, vdex) // The vdex files are identical between architectures, install them to a shared location. The Make rules will // only use the install rules for one architecture, and will create symlinks into the architecture-specific // directories. vdexInstalls = append(vdexInstalls, android.RuleBuilderInstall{vdex, filepath.Join(vdexInstallDir, vdex.Base())}) + } + + for _, unstrippedOat := range image.moduleFiles(ctx, symbolsDir, ".oat") { + cmd.ImplicitOutput(unstrippedOat) // Install the unstripped oat files. The Make rules will put these in $(TARGET_OUT_UNSTRIPPED) unstrippedInstalls = append(unstrippedInstalls, android.RuleBuilderInstall{unstrippedOat, filepath.Join(installDir, unstrippedOat.Base())}) } - cmd.ImplicitOutputs(extraFiles) - rule.Build(pctx, ctx, image.name+"JarsDexpreopt_"+arch.String(), "dexpreopt "+image.name+" jars "+arch.String()) // save output and installed files for makevars @@ -496,6 +508,7 @@ func (d *dexpreoptBootJars) MakeVars(ctx android.MakeVarsContext) { for _, arch := range arches { ctx.Strict("DEXPREOPT_IMAGE_VDEX_BUILT_INSTALLED_"+current.name+"_"+arch.String(), current.vdexInstalls[arch].String()) ctx.Strict("DEXPREOPT_IMAGE_"+current.name+"_"+arch.String(), current.images[arch].String()) + ctx.Strict("DEXPREOPT_IMAGE_DEPS_"+current.name+"_"+arch.String(), strings.Join(current.imagesDeps[arch].Strings(), " ")) ctx.Strict("DEXPREOPT_IMAGE_BUILT_INSTALLED_"+current.name+"_"+arch.String(), current.installs[arch].String()) ctx.Strict("DEXPREOPT_IMAGE_UNSTRIPPED_BUILT_INSTALLED_"+current.name+"_"+arch.String(), current.unstrippedInstalls[arch].String()) if current.zip != nil { diff --git a/java/dexpreopt_config.go b/java/dexpreopt_config.go index d903f456a..c396d3ea0 100644 --- a/java/dexpreopt_config.go +++ b/java/dexpreopt_config.go @@ -137,27 +137,35 @@ func defaultBootImageConfig(ctx android.PathContext) bootImageConfig { dir := android.PathForOutput(ctx, ctx.Config().DeviceName(), "dex_bootjars") symbolsDir := android.PathForOutput(ctx, ctx.Config().DeviceName(), "dex_bootjars_unstripped") - images := make(map[android.ArchType]android.OutputPath) zip := dir.Join(ctx, "boot.zip") targets := dexpreoptTargets(ctx) - for _, target := range targets { - images[target.Arch.ArchType] = dir.Join(ctx, - "system/framework", target.Arch.ArchType.String()).Join(ctx, "boot.art") - } - - return bootImageConfig{ + imageConfig := bootImageConfig{ name: "boot", modules: nonUpdatableBootModules, dexLocations: nonUpdatableBootLocations, dexPaths: nonUpdatableBootDexPaths, dir: dir, symbolsDir: symbolsDir, - images: images, + images: make(map[android.ArchType]android.OutputPath), + imagesDeps: make(map[android.ArchType]android.Paths), targets: targets, zip: zip, } + + for _, target := range targets { + imageDir := dir.Join(ctx, "system/framework", target.Arch.ArchType.String()) + imageConfig.images[target.Arch.ArchType] = imageDir.Join(ctx, "boot.art") + + imagesDeps := make([]android.Path, 0, len(imageConfig.modules)*3) + for _, dep := range imageConfig.moduleFiles(ctx, imageDir, ".art", ".oat", ".vdex") { + imagesDeps = append(imagesDeps, dep) + } + imageConfig.imagesDeps[target.Arch.ArchType] = imagesDeps + } + + return imageConfig }).(bootImageConfig) } @@ -196,16 +204,10 @@ func apexBootImageConfig(ctx android.PathContext) bootImageConfig { dir := android.PathForOutput(ctx, ctx.Config().DeviceName(), "dex_apexjars") symbolsDir := android.PathForOutput(ctx, ctx.Config().DeviceName(), "dex_apexjars_unstripped") - images := make(map[android.ArchType]android.OutputPath) targets := dexpreoptTargets(ctx) - for _, target := range targets { - images[target.Arch.ArchType] = dir.Join(ctx, - "system/framework", target.Arch.ArchType.String(), "apex.art") - } - - return bootImageConfig{ + imageConfig := bootImageConfig{ name: "apex", modules: imageModules, dexLocations: bootLocations, @@ -213,8 +215,22 @@ func apexBootImageConfig(ctx android.PathContext) bootImageConfig { dir: dir, symbolsDir: symbolsDir, targets: targets, - images: images, + images: make(map[android.ArchType]android.OutputPath), + imagesDeps: make(map[android.ArchType]android.Paths), } + + for _, target := range targets { + imageDir := dir.Join(ctx, "system/framework", target.Arch.ArchType.String()) + imageConfig.images[target.Arch.ArchType] = imageDir.Join(ctx, "apex.art") + + imagesDeps := make([]android.Path, 0, len(imageConfig.modules)*3) + for _, dep := range imageConfig.moduleFiles(ctx, imageDir, ".art", ".oat", ".vdex") { + imagesDeps = append(imagesDeps, dep) + } + imageConfig.imagesDeps[target.Arch.ArchType] = imagesDeps + } + + return imageConfig }).(bootImageConfig) }