From 7707b246efe17400aa27e843a507232bd02efd66 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Fri, 26 Jul 2024 12:02:36 -0700 Subject: [PATCH] Don't hold on to WritablePath Since only a single rule can write to a given WritablePath, it is unecessary to hold on to the WritablePath once the rule has been created. Keeping a WritablePath causes complications, as it prevents using the input Path as the output when no modifications to the input file are necessary. The normal pattern should be to create a WritablePath using PathForModuleOut, build the rule that writes to the WritablePath, and then store the WritablePath as a Path for use as an input to any future rules. WithoutRel previously only existed on OutputPath, which required keeping the output path of the module as an OutputPath for as long as possible in order to call WithoutRel on it at the end of the module. Add WithoutRel to Path, make it always return a Path type, and implement it on all the types that implement Path by using a helper in basePath. Replace long-lived WritablePaths with Paths. Test: all soong tests pass Flag: EXEMPT refactor Change-Id: I40f28075ce151e4be80d6cfc7ec173dfa46f9bbf --- android/paths.go | 37 ++++++++++++++++++++++++++++++-- java/aar.go | 54 +++++++++++++++++++++++++++-------------------- java/base.go | 49 ++++++++++++++++++------------------------ java/dex.go | 2 +- java/dexpreopt.go | 2 +- java/hiddenapi.go | 2 +- java/java.go | 2 +- 7 files changed, 91 insertions(+), 57 deletions(-) diff --git a/android/paths.go b/android/paths.go index dad70f770..d20b84a42 100644 --- a/android/paths.go +++ b/android/paths.go @@ -237,6 +237,9 @@ type Path interface { // directory, and OutputPath.Join("foo").Rel() would return "foo". Rel() string + // WithoutRel returns a new Path with no relative path, i.e. Rel() will return the same value as Base(). + WithoutRel() Path + // RelativeToTop returns a new path relative to the top, it is provided solely for use in tests. // // It is guaranteed to always return the same type as it is called on, e.g. if called on an @@ -1119,6 +1122,11 @@ func (p basePath) withRel(rel string) basePath { return p } +func (p basePath) withoutRel() basePath { + p.rel = filepath.Base(p.path) + return p +} + // SourcePath is a Path representing a file path rooted from SrcDir type SourcePath struct { basePath @@ -1278,6 +1286,11 @@ func (p SourcePath) String() string { return p.path } +func (p SourcePath) WithoutRel() Path { + p.basePath = p.basePath.withoutRel() + return p +} + // Join creates a new SourcePath with paths... joined with the current path. The // provided paths... may not use '..' to escape from the current path. func (p SourcePath) Join(ctx PathContext, paths ...string) SourcePath { @@ -1362,8 +1375,8 @@ func (p OutputPath) withRel(rel string) OutputPath { return p } -func (p OutputPath) WithoutRel() OutputPath { - p.basePath.rel = filepath.Base(p.basePath.path) +func (p OutputPath) WithoutRel() Path { + p.basePath = p.basePath.withoutRel() return p } @@ -1399,6 +1412,11 @@ type toolDepPath struct { basePath } +func (t toolDepPath) WithoutRel() Path { + t.basePath = t.basePath.withoutRel() + return t +} + func (t toolDepPath) RelativeToTop() Path { ensureTestOnly() return t @@ -1767,6 +1785,11 @@ func (p InstallPath) RelativeToTop() Path { return p } +func (p InstallPath) WithoutRel() Path { + p.basePath = p.basePath.withoutRel() + return p +} + func (p InstallPath) getSoongOutDir() string { return p.soongOutDir } @@ -2087,6 +2110,11 @@ func (p PhonyPath) RelativeToTop() Path { return p } +func (p PhonyPath) WithoutRel() Path { + p.basePath = p.basePath.withoutRel() + return p +} + func (p PhonyPath) ReplaceExtension(ctx PathContext, ext string) OutputPath { panic("Not implemented") } @@ -2103,6 +2131,11 @@ func (p testPath) RelativeToTop() Path { return p } +func (p testPath) WithoutRel() Path { + p.basePath = p.basePath.withoutRel() + return p +} + func (p testPath) String() string { return p.path } diff --git a/java/aar.go b/java/aar.go index b69b7c262..4a60f908a 100644 --- a/java/aar.go +++ b/java/aar.go @@ -991,17 +991,17 @@ type AARImport struct { properties AARImportProperties - headerJarFile android.WritablePath - implementationJarFile android.WritablePath - implementationAndResourcesJarFile android.WritablePath - proguardFlags android.WritablePath - exportPackage android.WritablePath + headerJarFile android.Path + implementationJarFile android.Path + implementationAndResourcesJarFile android.Path + proguardFlags android.Path + exportPackage android.Path transitiveAaptResourcePackagesFile android.Path - extraAaptPackagesFile android.WritablePath + extraAaptPackagesFile android.Path manifest android.Path - assetsPackage android.WritablePath - rTxt android.WritablePath - rJar android.WritablePath + assetsPackage android.Path + rTxt android.Path + rJar android.Path resourcesNodesDepSet *android.DepSet[*resourcesNode] manifestsDepSet *android.DepSet[android.Path] @@ -1166,14 +1166,14 @@ func (a *AARImport) GenerateAndroidBuildActions(ctx android.ModuleContext) { a.manifest = extractedManifest } - a.rTxt = extractedAARDir.Join(ctx, "R.txt") - a.assetsPackage = android.PathForModuleOut(ctx, "assets.zip") - a.proguardFlags = extractedAARDir.Join(ctx, "proguard.txt") + rTxt := extractedAARDir.Join(ctx, "R.txt") + assetsPackage := android.PathForModuleOut(ctx, "assets.zip") + proguardFlags := extractedAARDir.Join(ctx, "proguard.txt") transitiveProguardFlags, transitiveUnconditionalExportedFlags := collectDepProguardSpecInfo(ctx) android.SetProvider(ctx, ProguardSpecInfoProvider, ProguardSpecInfo{ ProguardFlagsFiles: android.NewDepSet[android.Path]( android.POSTORDER, - android.Paths{a.proguardFlags}, + android.Paths{proguardFlags}, transitiveProguardFlags, ), UnconditionallyExportedProguardFlags: android.NewDepSet[android.Path]( @@ -1186,15 +1186,19 @@ func (a *AARImport) GenerateAndroidBuildActions(ctx android.ModuleContext) { ctx.Build(pctx, android.BuildParams{ Rule: unzipAAR, Input: a.aarPath, - Outputs: android.WritablePaths{classpathFile, a.proguardFlags, extractedManifest, a.assetsPackage, a.rTxt}, + Outputs: android.WritablePaths{classpathFile, proguardFlags, extractedManifest, assetsPackage, rTxt}, Description: "unzip AAR", Args: map[string]string{ "outDir": extractedAARDir.String(), "combinedClassesJar": classpathFile.String(), - "assetsPackage": a.assetsPackage.String(), + "assetsPackage": assetsPackage.String(), }, }) + a.proguardFlags = proguardFlags + a.assetsPackage = assetsPackage + a.rTxt = rTxt + // Always set --pseudo-localize, it will be stripped out later for release // builds that don't want it. compileFlags := []string{"--pseudo-localize"} @@ -1202,10 +1206,10 @@ func (a *AARImport) GenerateAndroidBuildActions(ctx android.ModuleContext) { flata := compiledResDir.Join(ctx, "gen_res.flata") aapt2CompileZip(ctx, flata, a.aarPath, "res", compileFlags) - a.exportPackage = android.PathForModuleOut(ctx, "package-res.apk") + exportPackage := android.PathForModuleOut(ctx, "package-res.apk") proguardOptionsFile := android.PathForModuleGen(ctx, "proguard.options") aaptRTxt := android.PathForModuleOut(ctx, "R.txt") - a.extraAaptPackagesFile = android.PathForModuleOut(ctx, "extra_packages") + extraAaptPackagesFile := android.PathForModuleOut(ctx, "extra_packages") var linkDeps android.Paths @@ -1241,13 +1245,16 @@ func (a *AARImport) GenerateAndroidBuildActions(ctx android.ModuleContext) { } transitiveAssets := android.ReverseSliceInPlace(staticDeps.assets()) - aapt2Link(ctx, a.exportPackage, nil, proguardOptionsFile, aaptRTxt, + aapt2Link(ctx, exportPackage, nil, proguardOptionsFile, aaptRTxt, linkFlags, linkDeps, nil, overlayRes, transitiveAssets, nil, nil) + a.exportPackage = exportPackage - a.rJar = android.PathForModuleOut(ctx, "busybox/R.jar") - resourceProcessorBusyBoxGenerateBinaryR(ctx, a.rTxt, a.manifest, a.rJar, nil, true, nil, false) + rJar := android.PathForModuleOut(ctx, "busybox/R.jar") + resourceProcessorBusyBoxGenerateBinaryR(ctx, a.rTxt, a.manifest, rJar, nil, true, nil, false) + a.rJar = rJar - aapt2ExtractExtraPackages(ctx, a.extraAaptPackagesFile, a.rJar) + aapt2ExtractExtraPackages(ctx, extraAaptPackagesFile, a.rJar) + a.extraAaptPackagesFile = extraAaptPackagesFile resourcesNodesDepSetBuilder := android.NewDepSetBuilder[*resourcesNode](android.TOPOLOGICAL) resourcesNodesDepSetBuilder.Direct(&resourcesNode{ @@ -1330,8 +1337,9 @@ func (a *AARImport) GenerateAndroidBuildActions(ctx android.ModuleContext) { if len(staticHeaderJars) > 0 { combineJars := append(android.Paths{classpathFile}, staticHeaderJars...) - a.headerJarFile = android.PathForModuleOut(ctx, "turbine-combined", jarName) - TransformJarsToJar(ctx, a.headerJarFile, "combine header jars", combineJars, android.OptionalPath{}, false, nil, nil) + headerJarFile := android.PathForModuleOut(ctx, "turbine-combined", jarName) + TransformJarsToJar(ctx, headerJarFile, "combine header jars", combineJars, android.OptionalPath{}, false, nil, nil) + a.headerJarFile = headerJarFile } else { a.headerJarFile = classpathFile } diff --git a/java/base.go b/java/base.go index 02dc3e35b..44bc75946 100644 --- a/java/base.go +++ b/java/base.go @@ -1303,7 +1303,7 @@ func (j *Module) compile(ctx android.ModuleContext, extraSrcJars, extraClasspath return } - kotlinJarPath := j.repackageFlagsIfNecessary(ctx, kotlinJar.OutputPath, jarName, "kotlinc") + kotlinJarPath := j.repackageFlagsIfNecessary(ctx, kotlinJar, jarName, "kotlinc") // Make javac rule depend on the kotlinc rule flags.classpath = append(classpath{kotlinHeaderJar}, flags.classpath...) @@ -1505,7 +1505,7 @@ func (j *Module) compile(ctx android.ModuleContext, extraSrcJars, extraClasspath // Combine the classes built from sources, any manifests, and any static libraries into // classes.jar. If there is only one input jar this step will be skipped. - var outputFile android.OutputPath + var outputFile android.Path if len(jars) == 1 && !manifest.Valid() { // Optimization: skip the combine step as there is nothing to do @@ -1521,36 +1521,28 @@ func (j *Module) compile(ctx android.ModuleContext, extraSrcJars, extraClasspath // to the copy rules. stub, _ := moduleStubLinkType(ctx.ModuleName()) - // Transform the single path to the jar into an OutputPath as that is required by the following - // code. - if moduleOutPath, ok := jars[0].(android.ModuleOutPath); ok && !stub { - // The path contains an embedded OutputPath so reuse that. - outputFile = moduleOutPath.OutputPath - } else if outputPath, ok := jars[0].(android.OutputPath); ok && !stub { - // The path is an OutputPath so reuse it directly. - outputFile = outputPath - } else { - // The file is not in the out directory so create an OutputPath into which it can be copied - // and which the following code can use to refer to it. + if stub { combinedJar := android.PathForModuleOut(ctx, "combined", jarName) ctx.Build(pctx, android.BuildParams{ Rule: android.Cp, Input: jars[0], Output: combinedJar, }) - outputFile = combinedJar.OutputPath + outputFile = combinedJar + } else { + outputFile = jars[0] } } else { combinedJar := android.PathForModuleOut(ctx, "combined", jarName) TransformJarsToJar(ctx, combinedJar, "for javac", jars, manifest, false, nil, nil) - outputFile = combinedJar.OutputPath + outputFile = combinedJar } // jarjar implementation jar if necessary if j.expandJarjarRules != nil { // Transform classes.jar into classes-jarjar.jar - jarjarFile := android.PathForModuleOut(ctx, "jarjar", jarName).OutputPath + jarjarFile := android.PathForModuleOut(ctx, "jarjar", jarName) TransformJarJar(ctx, jarjarFile, outputFile, j.expandJarjarRules) outputFile = jarjarFile @@ -1575,15 +1567,16 @@ func (j *Module) compile(ctx android.ModuleContext, extraSrcJars, extraClasspath // will check that the jar only contains the permitted packages. The new location will become // the output file of this module. inputFile := outputFile - outputFile = android.PathForModuleOut(ctx, "package-check", jarName).OutputPath + packageCheckOutputFile := android.PathForModuleOut(ctx, "package-check", jarName) ctx.Build(pctx, android.BuildParams{ Rule: android.Cp, Input: inputFile, - Output: outputFile, + Output: packageCheckOutputFile, // Make sure that any dependency on the output file will cause ninja to run the package check // rule. Validation: pkgckFile, }) + outputFile = packageCheckOutputFile // Check packages and create a timestamp file when complete. CheckJarPackages(ctx, pkgckFile, outputFile, j.properties.Permitted_packages) @@ -1618,7 +1611,7 @@ func (j *Module) compile(ctx android.ModuleContext, extraSrcJars, extraClasspath implementationAndResourcesJar := outputFile if j.resourceJar != nil { jars := android.Paths{j.resourceJar, implementationAndResourcesJar} - combinedJar := android.PathForModuleOut(ctx, "withres", jarName).OutputPath + combinedJar := android.PathForModuleOut(ctx, "withres", jarName) TransformJarsToJar(ctx, combinedJar, "for resources", jars, manifest, false, nil, nil) implementationAndResourcesJar = combinedJar @@ -1645,7 +1638,7 @@ func (j *Module) compile(ctx android.ModuleContext, extraSrcJars, extraClasspath android.PathForSource(ctx, "build/make/core/proguard.jacoco.flags")) } // Dex compilation - var dexOutputFile android.OutputPath + var dexOutputFile android.Path params := &compileDexParams{ flags: flags, sdkVersion: j.SdkVersion(ctx), @@ -1672,17 +1665,17 @@ func (j *Module) compile(ctx android.ModuleContext, extraSrcJars, extraClasspath // If r8/d8 provides a profile that matches the optimized dex, use that for dexpreopt. if dexArtProfileOutput != nil { - j.dexpreopter.SetRewrittenProfile(*dexArtProfileOutput) + j.dexpreopter.SetRewrittenProfile(dexArtProfileOutput) } // merge dex jar with resources if necessary if j.resourceJar != nil { jars := android.Paths{dexOutputFile, j.resourceJar} - combinedJar := android.PathForModuleOut(ctx, "dex-withres", jarName).OutputPath + combinedJar := android.PathForModuleOut(ctx, "dex-withres", jarName) TransformJarsToJar(ctx, combinedJar, "for dex resources", jars, android.OptionalPath{}, false, nil, nil) if *j.dexProperties.Uncompress_dex { - combinedAlignedJar := android.PathForModuleOut(ctx, "dex-withres-aligned", jarName).OutputPath + combinedAlignedJar := android.PathForModuleOut(ctx, "dex-withres-aligned", jarName) TransformZipAlign(ctx, combinedAlignedJar, combinedJar, nil) dexOutputFile = combinedAlignedJar } else { @@ -1842,7 +1835,7 @@ func enableErrorproneFlags(flags javaBuilderFlags) javaBuilderFlags { } func (j *Module) compileJavaClasses(ctx android.ModuleContext, jarName string, idx int, - srcFiles, srcJars android.Paths, flags javaBuilderFlags, extraJarDeps android.Paths) android.WritablePath { + srcFiles, srcJars android.Paths, flags javaBuilderFlags, extraJarDeps android.Paths) android.Path { kzipName := pathtools.ReplaceExtension(jarName, "kzip") annoSrcJar := android.PathForModuleOut(ctx, "javac", "anno.srcjar") @@ -1852,7 +1845,7 @@ func (j *Module) compileJavaClasses(ctx android.ModuleContext, jarName string, i jarName += strconv.Itoa(idx) } - classes := android.PathForModuleOut(ctx, "javac", jarName).OutputPath + classes := android.PathForModuleOut(ctx, "javac", jarName) TransformJavaToClasses(ctx, classes, idx, srcFiles, srcJars, annoSrcJar, flags, extraJarDeps) if ctx.Config().EmitXrefRules() && ctx.Module() == ctx.PrimaryModule() { @@ -1947,10 +1940,10 @@ func (j *Module) compileJavaHeader(ctx android.ModuleContext, srcFiles, srcJars } func (j *Module) instrument(ctx android.ModuleContext, flags javaBuilderFlags, - classesJar android.Path, jarName string, specs string) android.OutputPath { + classesJar android.Path, jarName string, specs string) android.Path { jacocoReportClassesFile := android.PathForModuleOut(ctx, "jacoco-report-classes", jarName) - instrumentedJar := android.PathForModuleOut(ctx, "jacoco", jarName).OutputPath + instrumentedJar := android.PathForModuleOut(ctx, "jacoco", jarName) jacocoInstrumentJar(ctx, instrumentedJar, jacocoReportClassesFile, classesJar, specs) @@ -2714,7 +2707,7 @@ func getJarJarRuleText(provider *JarJarProviderData) string { } // Repackage the flags if the jarjar rule txt for the flags is generated -func (j *Module) repackageFlagsIfNecessary(ctx android.ModuleContext, infile android.WritablePath, jarName, info string) android.WritablePath { +func (j *Module) repackageFlagsIfNecessary(ctx android.ModuleContext, infile android.Path, jarName, info string) android.Path { if j.repackageJarjarRules == nil { return infile } diff --git a/java/dex.go b/java/dex.go index 7bb69257c..bb3687154 100644 --- a/java/dex.go +++ b/java/dex.go @@ -428,7 +428,7 @@ func (d *dexer) addArtProfile(ctx android.ModuleContext, dexParams *compileDexPa } // Return the compiled dex jar and (optional) profile _after_ r8 optimization -func (d *dexer) compileDex(ctx android.ModuleContext, dexParams *compileDexParams) (android.OutputPath, *android.OutputPath) { +func (d *dexer) compileDex(ctx android.ModuleContext, dexParams *compileDexParams) (android.Path, android.Path) { // Compile classes.jar into classes.dex and then javalib.jar javalibJar := android.PathForModuleOut(ctx, "dex", dexParams.jarName).OutputPath diff --git a/java/dexpreopt.go b/java/dexpreopt.go index 794924401..2980babe3 100644 --- a/java/dexpreopt.go +++ b/java/dexpreopt.go @@ -359,7 +359,7 @@ func (d *Dexpreopter) DexpreoptPrebuiltApexSystemServerJars(ctx android.ModuleCo d.dexpreopt(ctx, libraryName, dexJarFile) } -func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, libName string, dexJarFile android.WritablePath) { +func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, libName string, dexJarFile android.Path) { global := dexpreopt.GetGlobalConfig(ctx) // TODO(b/148690468): The check on d.installPath is to bail out in cases where diff --git a/java/hiddenapi.go b/java/hiddenapi.go index 5441a3b6e..689c78f6d 100644 --- a/java/hiddenapi.go +++ b/java/hiddenapi.go @@ -151,7 +151,7 @@ func isModuleInBootClassPath(ctx android.BaseModuleContext, module android.Modul // // Otherwise, it creates a copy of the supplied dex file into which it has encoded the hiddenapi // flags and returns this instead of the supplied dex jar. -func (h *hiddenAPI) hiddenAPIEncodeDex(ctx android.ModuleContext, dexJar android.OutputPath) android.OutputPath { +func (h *hiddenAPI) hiddenAPIEncodeDex(ctx android.ModuleContext, dexJar android.Path) android.Path { if !h.active { return dexJar diff --git a/java/java.go b/java/java.go index 88b31b586..263d0f333 100644 --- a/java/java.go +++ b/java/java.go @@ -2721,7 +2721,7 @@ func (j *Import) GenerateAndroidBuildActions(ctx android.ModuleContext) { setUncompressDex(ctx, &j.dexpreopter, &j.dexer) j.dexpreopter.uncompressedDex = *j.dexProperties.Uncompress_dex - var dexOutputFile android.OutputPath + var dexOutputFile android.Path dexParams := &compileDexParams{ flags: flags, sdkVersion: j.SdkVersion(ctx),