From 80783774b913a8abe78675f81b23754cc67d4a4c Mon Sep 17 00:00:00 2001 From: Chih-Hung Hsieh Date: Mon, 11 Oct 2021 16:46:56 -0700 Subject: [PATCH] Add (obj|tidy)-*_os, and (obj|tidy)-*_subset targets * The obj-* targets only call clang or clang++ to compile C/C++ files, like the tidy-* targets only call clang-tidy. * Add (obj|tidy)-dir_os and module_os-(obj|tidy) phony targets to include only targets of the specified OS. * Add (obj|tidy)-dir_os_subset and module_os_subset-(obj|tidy) phony targets to include only a subset of that OS target group. * Most clang-tidy runs produce the same warnings for all variants of an OS. The *_subset targets reduce runs over multiple variants. * The obj-*subset targets are useful for a quick test to compile at least one variant of every C/C++ file for each OS. * The (obj|tidy)-soong phony target is changed to include only (obj|tidy)-top_level_directory targets instead of ALL module-(obj|tidy) targets in all subdirectories. * For aosp_arm64-eng make tidy-* targets; count NINJA commands: tidy-soong 381K tidy-soong_subset 106K tidy-soong_android 294K tidy-soong_android_subset 73K tidy-soong_linux_glibc 84K tidy-soong_windows 12K tidy-bionic 12K tidy-bionic_subset 2.4K tidy-bionic_android 11.5K tidy-bionic_android_subset 2.1K tidy-bionic_linux_glibc 1K tidy-device_android 7K tidy-device_android_subset 5K tidy-hardware_android 3K tidy-hardware_android_subset 9K libfec_rs-tidy 33 libfec_rs_subset-tidy 9 libfec_rs_android-tidy 18 libfec_rs_android_subset-tidy 3 * For aosp_arm64-eng make obj-* targets; count NINJA commands: obj-soong 399K obj-soong_subset 112K obj-soong_android 305K obj-soong_android_subset 75K obj-soong_linux_glibc 90K obj-soong_linux_glibc_subset 38K obj-soong_windows 12K obj-bionic 15K obj-bionic_subset 3K obj-bionic_android 14K obj-bionic_android_subset 2.3K obj-bionic_linux_glibc 1.3K obj-system-core 6K obj-system-core_subset 3K obj-frameworks-base 16K obj-frameworks-base_subset 6K libfec_rs-obj 33 libfec_rs_subset-obj 9 libfec_rs_android-obj 18 libfec_rs_android_subset-obj 3 Test: NINJA_ARGS="-n" WITH_TIDY=1 make some_obj_tidy_target; compare output commands from various phony targets Bug: 199169329 Bug: 202769240 Change-Id: I186c45dc07c5884888c1063dfc09cf212ffb0ebf --- android/androidmk.go | 4 ++ android/module.go | 58 +++-------------- cc/binary.go | 2 +- cc/builder.go | 17 +++-- cc/cc.go | 6 ++ cc/tidy.go | 152 +++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 179 insertions(+), 60 deletions(-) diff --git a/android/androidmk.go b/android/androidmk.go index 0adc2a6eb..1f03aa812 100644 --- a/android/androidmk.go +++ b/android/androidmk.go @@ -887,6 +887,10 @@ func translateAndroidMkEntriesModule(ctx SingletonContext, w io.Writer, mod blue return nil } +func ShouldSkipAndroidMkProcessing(module Module) bool { + return shouldSkipAndroidMkProcessing(module.base()) +} + func shouldSkipAndroidMkProcessing(module *ModuleBase) bool { if !module.commonProperties.NamespaceExportedToMake { // TODO(jeffrygaston) do we want to validate that there are no modules being diff --git a/android/module.go b/android/module.go index b500f01c6..c2cb6179a 100644 --- a/android/module.go +++ b/android/module.go @@ -419,7 +419,6 @@ type ModuleContext interface { PackageFile(installPath InstallPath, name string, srcPath Path) PackagingSpec CheckbuildFile(srcPath Path) - TidyFile(srcPath WritablePath) InstallInData() bool InstallInTestcases() bool @@ -1200,7 +1199,6 @@ type ModuleBase struct { installFiles InstallPaths installFilesDepSet *installPathsDepSet checkbuildFiles Paths - tidyFiles WritablePaths packagingSpecs []PackagingSpec packagingSpecsDepSet *packagingSpecsDepSet noticeFiles Paths @@ -1216,7 +1214,6 @@ type ModuleBase struct { // Only set on the final variant of each module installTarget WritablePath checkbuildTarget WritablePath - tidyTarget WritablePath blueprintDir string hooks hooks @@ -1779,17 +1776,15 @@ func (m *ModuleBase) VintfFragments() Paths { func (m *ModuleBase) generateModuleTarget(ctx ModuleContext) { var allInstalledFiles InstallPaths var allCheckbuildFiles Paths - var allTidyFiles WritablePaths ctx.VisitAllModuleVariants(func(module Module) { a := module.base() allInstalledFiles = append(allInstalledFiles, a.installFiles...) - // A module's -{checkbuild,tidy} phony targets should + // A module's -checkbuild phony targets should // not be created if the module is not exported to make. // Those could depend on the build target and fail to compile // for the current build target. if !ctx.Config().KatiEnabled() || !shouldSkipAndroidMkProcessing(a) { allCheckbuildFiles = append(allCheckbuildFiles, a.checkbuildFiles...) - allTidyFiles = append(allTidyFiles, a.tidyFiles...) } }) @@ -1814,13 +1809,6 @@ func (m *ModuleBase) generateModuleTarget(ctx ModuleContext) { deps = append(deps, m.checkbuildTarget) } - if len(allTidyFiles) > 0 { - name := namespacePrefix + ctx.ModuleName() + "-tidy" - ctx.Phony(name, allTidyFiles.Paths()...) - m.tidyTarget = PathForPhony(ctx, name) - deps = append(deps, m.tidyTarget) - } - if len(deps) > 0 { suffix := "" if ctx.Config().KatiEnabled() { @@ -2029,7 +2017,6 @@ func (m *ModuleBase) GenerateBuildActions(blueprintCtx blueprint.ModuleContext) m.installFiles = append(m.installFiles, ctx.installFiles...) m.checkbuildFiles = append(m.checkbuildFiles, ctx.checkbuildFiles...) - m.tidyFiles = append(m.tidyFiles, ctx.tidyFiles...) m.packagingSpecs = append(m.packagingSpecs, ctx.packagingSpecs...) m.katiInstalls = append(m.katiInstalls, ctx.katiInstalls...) m.katiSymlinks = append(m.katiSymlinks, ctx.katiSymlinks...) @@ -2227,7 +2214,6 @@ type moduleContext struct { packagingSpecs []PackagingSpec installFiles InstallPaths checkbuildFiles Paths - tidyFiles WritablePaths module Module phonies map[string]Paths @@ -3065,10 +3051,6 @@ func (m *moduleContext) CheckbuildFile(srcPath Path) { m.checkbuildFiles = append(m.checkbuildFiles, srcPath) } -func (m *moduleContext) TidyFile(srcPath WritablePath) { - m.tidyFiles = append(m.tidyFiles, srcPath) -} - func (m *moduleContext) blueprintModuleContext() blueprint.ModuleContext { return m.bp } @@ -3327,9 +3309,10 @@ func parentDir(dir string) string { type buildTargetSingleton struct{} -func addAncestors(ctx SingletonContext, dirMap map[string]Paths, mmName func(string) string) []string { +func AddAncestors(ctx SingletonContext, dirMap map[string]Paths, mmName func(string) string) ([]string, []string) { // Ensure ancestor directories are in dirMap // Make directories build their direct subdirectories + // Returns a slice of all directories and a slice of top-level directories. dirs := SortedStringKeys(dirMap) for _, dir := range dirs { dir := parentDir(dir) @@ -3342,34 +3325,31 @@ func addAncestors(ctx SingletonContext, dirMap map[string]Paths, mmName func(str } } dirs = SortedStringKeys(dirMap) + var topDirs []string for _, dir := range dirs { p := parentDir(dir) if p != "." && p != "/" { dirMap[p] = append(dirMap[p], PathForPhony(ctx, mmName(dir))) + } else if dir != "." && dir != "/" && dir != "" { + topDirs = append(topDirs, dir) } } - return SortedStringKeys(dirMap) + return SortedStringKeys(dirMap), topDirs } func (c *buildTargetSingleton) GenerateBuildActions(ctx SingletonContext) { var checkbuildDeps Paths - var tidyDeps Paths mmTarget := func(dir string) string { return "MODULES-IN-" + strings.Replace(filepath.Clean(dir), "/", "-", -1) } - mmTidyTarget := func(dir string) string { - return "tidy-" + strings.Replace(filepath.Clean(dir), "/", "-", -1) - } modulesInDir := make(map[string]Paths) - tidyModulesInDir := make(map[string]Paths) ctx.VisitAllModules(func(module Module) { blueprintDir := module.base().blueprintDir installTarget := module.base().installTarget checkbuildTarget := module.base().checkbuildTarget - tidyTarget := module.base().tidyTarget if checkbuildTarget != nil { checkbuildDeps = append(checkbuildDeps, checkbuildTarget) @@ -3379,16 +3359,6 @@ func (c *buildTargetSingleton) GenerateBuildActions(ctx SingletonContext) { if installTarget != nil { modulesInDir[blueprintDir] = append(modulesInDir[blueprintDir], installTarget) } - - if tidyTarget != nil { - tidyDeps = append(tidyDeps, tidyTarget) - // tidyTarget is in modulesInDir so it will be built with "mm". - modulesInDir[blueprintDir] = append(modulesInDir[blueprintDir], tidyTarget) - // tidyModulesInDir contains tidyTarget but not checkbuildTarget - // or installTarget, so tidy targets in a directory can be built - // without other checkbuild or install targets. - tidyModulesInDir[blueprintDir] = append(tidyModulesInDir[blueprintDir], tidyTarget) - } }) suffix := "" @@ -3399,24 +3369,12 @@ func (c *buildTargetSingleton) GenerateBuildActions(ctx SingletonContext) { // Create a top-level checkbuild target that depends on all modules ctx.Phony("checkbuild"+suffix, checkbuildDeps...) - // Create a top-level tidy target that depends on all modules - ctx.Phony("tidy"+suffix, tidyDeps...) - - dirs := addAncestors(ctx, tidyModulesInDir, mmTidyTarget) - - // Kati does not generate tidy-* phony targets yet. - // Create a tidy- target that depends on all subdirectories - // and modules in the directory. - for _, dir := range dirs { - ctx.Phony(mmTidyTarget(dir), tidyModulesInDir[dir]...) - } - // Make will generate the MODULES-IN-* targets if ctx.Config().KatiEnabled() { return } - dirs = addAncestors(ctx, modulesInDir, mmTarget) + dirs, _ := AddAncestors(ctx, modulesInDir, mmTarget) // Create a MODULES-IN- target that depends on all modules in a directory, and // depends on the MODULES-IN-* targets of all of its subdirectories that contain Android.bp diff --git a/cc/binary.go b/cc/binary.go index 0650bdf39..3f951eca5 100644 --- a/cc/binary.go +++ b/cc/binary.go @@ -389,7 +389,7 @@ func (binary *binaryDecorator) link(ctx ModuleContext, } } - var validations android.WritablePaths + var validations android.Paths // Handle host bionic linker symbols. if ctx.Os() == android.LinuxBionic && !binary.static() { diff --git a/cc/builder.go b/cc/builder.go index 72c2fa555..7161ccf83 100644 --- a/cc/builder.go +++ b/cc/builder.go @@ -413,7 +413,7 @@ type StripFlags struct { // Objects is a collection of file paths corresponding to outputs for C++ related build statements. type Objects struct { objFiles android.Paths - tidyFiles android.WritablePaths + tidyFiles android.Paths coverageFiles android.Paths sAbiDumpFiles android.Paths kytheFiles android.Paths @@ -422,7 +422,7 @@ type Objects struct { func (a Objects) Copy() Objects { return Objects{ objFiles: append(android.Paths{}, a.objFiles...), - tidyFiles: append(android.WritablePaths{}, a.tidyFiles...), + tidyFiles: append(android.Paths{}, a.tidyFiles...), coverageFiles: append(android.Paths{}, a.coverageFiles...), sAbiDumpFiles: append(android.Paths{}, a.sAbiDumpFiles...), kytheFiles: append(android.Paths{}, a.kytheFiles...), @@ -451,11 +451,11 @@ func transformSourceToObj(ctx android.ModuleContext, subdir string, srcFiles, no // Source files are one-to-one with tidy, coverage, or kythe files, if enabled. objFiles := make(android.Paths, len(srcFiles)) - var tidyFiles android.WritablePaths + var tidyFiles android.Paths noTidySrcsMap := make(map[android.Path]bool) var tidyVars string if flags.tidy { - tidyFiles = make(android.WritablePaths, 0, len(srcFiles)) + tidyFiles = make(android.Paths, 0, len(srcFiles)) for _, path := range noTidySrcs { noTidySrcsMap[path] = true } @@ -665,7 +665,6 @@ func transformSourceToObj(ctx android.ModuleContext, subdir string, srcFiles, no rule = clangTidyRE } - ctx.TidyFile(tidyFile) ctx.Build(pctx, android.BuildParams{ Rule: rule, Description: "clang-tidy " + srcFile.Rel(), @@ -719,7 +718,7 @@ func transformSourceToObj(ctx android.ModuleContext, subdir string, srcFiles, no // Generate a rule for compiling multiple .o files to a static library (.a) func transformObjToStaticLib(ctx android.ModuleContext, objFiles android.Paths, wholeStaticLibs android.Paths, - flags builderFlags, outputFile android.ModuleOutPath, deps android.Paths, validations android.WritablePaths) { + flags builderFlags, outputFile android.ModuleOutPath, deps android.Paths, validations android.Paths) { arCmd := "${config.ClangBin}/llvm-ar" arFlags := "" @@ -734,7 +733,7 @@ func transformObjToStaticLib(ctx android.ModuleContext, Output: outputFile, Inputs: objFiles, Implicits: deps, - Validations: validations.Paths(), + Validations: validations, Args: map[string]string{ "arFlags": "crsPD" + arFlags, "arCmd": arCmd, @@ -764,7 +763,7 @@ func transformObjToStaticLib(ctx android.ModuleContext, func transformObjToDynamicBinary(ctx android.ModuleContext, objFiles, sharedLibs, staticLibs, lateStaticLibs, wholeStaticLibs, deps, crtBegin, crtEnd android.Paths, groupLate bool, flags builderFlags, outputFile android.WritablePath, - implicitOutputs android.WritablePaths, validations android.WritablePaths) { + implicitOutputs android.WritablePaths, validations android.Paths) { ldCmd := "${config.ClangBin}/clang++" @@ -831,7 +830,7 @@ func transformObjToDynamicBinary(ctx android.ModuleContext, Inputs: objFiles, Implicits: deps, OrderOnly: sharedLibs, - Validations: validations.Paths(), + Validations: validations, Args: args, }) } diff --git a/cc/cc.go b/cc/cc.go index aeb342f4f..3bb0d6d83 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -815,6 +815,10 @@ type Module struct { makeLinkType string // Kythe (source file indexer) paths for this compilation module kytheFiles android.Paths + // Object .o file output paths for this compilation module + objFiles android.Paths + // Tidy .tidy file output paths for this compilation module + tidyFiles android.Paths // For apex variants, this is set as apex.min_sdk_version apexSdkVersion android.ApiLevel @@ -1835,6 +1839,8 @@ func (c *Module) GenerateAndroidBuildActions(actx android.ModuleContext) { return } c.kytheFiles = objs.kytheFiles + c.objFiles = objs.objFiles + c.tidyFiles = objs.tidyFiles } if c.linker != nil { diff --git a/cc/tidy.go b/cc/tidy.go index 53ff1564b..78a791faf 100644 --- a/cc/tidy.go +++ b/cc/tidy.go @@ -15,6 +15,7 @@ package cc import ( + "path/filepath" "regexp" "strings" @@ -183,3 +184,154 @@ func (tidy *tidyFeature) flags(ctx ModuleContext, flags Flags) Flags { } return flags } + +func init() { + android.RegisterSingletonType("tidy_phony_targets", TidyPhonySingleton) +} + +// This TidyPhonySingleton generates both tidy-* and obj-* phony targets for C/C++ files. +func TidyPhonySingleton() android.Singleton { + return &tidyPhonySingleton{} +} + +type tidyPhonySingleton struct{} + +// Given a final module, add its tidy/obj phony targets to tidy/objModulesInDirGroup. +func collectTidyObjModuleTargets(ctx android.SingletonContext, module android.Module, + tidyModulesInDirGroup, objModulesInDirGroup map[string]map[string]android.Paths) { + allObjFileGroups := make(map[string]android.Paths) // variant group name => obj file Paths + allTidyFileGroups := make(map[string]android.Paths) // variant group name => tidy file Paths + subsetObjFileGroups := make(map[string]android.Paths) // subset group name => obj file Paths + subsetTidyFileGroups := make(map[string]android.Paths) // subset group name => tidy file Paths + + // (1) Collect all obj/tidy files into OS-specific groups. + ctx.VisitAllModuleVariants(module, func(variant android.Module) { + if ctx.Config().KatiEnabled() && android.ShouldSkipAndroidMkProcessing(variant) { + return + } + if m, ok := variant.(*Module); ok { + osName := variant.Target().Os.Name + addToOSGroup(osName, m.objFiles, allObjFileGroups, subsetObjFileGroups) + addToOSGroup(osName, m.tidyFiles, allTidyFileGroups, subsetTidyFileGroups) + } + }) + + // (2) Add an all-OS group, with "" or "subset" name, to include all os-specific phony targets. + addAllOSGroup(ctx, module, allObjFileGroups, "", "obj") + addAllOSGroup(ctx, module, allTidyFileGroups, "", "tidy") + addAllOSGroup(ctx, module, subsetObjFileGroups, "subset", "obj") + addAllOSGroup(ctx, module, subsetTidyFileGroups, "subset", "tidy") + + tidyTargetGroups := make(map[string]android.Path) + objTargetGroups := make(map[string]android.Path) + genObjTidyPhonyTargets(ctx, module, "obj", allObjFileGroups, objTargetGroups) + genObjTidyPhonyTargets(ctx, module, "obj", subsetObjFileGroups, objTargetGroups) + genObjTidyPhonyTargets(ctx, module, "tidy", allTidyFileGroups, tidyTargetGroups) + genObjTidyPhonyTargets(ctx, module, "tidy", subsetTidyFileGroups, tidyTargetGroups) + + moduleDir := ctx.ModuleDir(module) + appendToModulesInDirGroup(tidyTargetGroups, moduleDir, tidyModulesInDirGroup) + appendToModulesInDirGroup(objTargetGroups, moduleDir, objModulesInDirGroup) +} + +func (m *tidyPhonySingleton) GenerateBuildActions(ctx android.SingletonContext) { + // For tidy-* directory phony targets, there are different variant groups. + // tidyModulesInDirGroup[G][D] is for group G, directory D, with Paths + // of all phony targets to be included into direct dependents of tidy-D_G. + tidyModulesInDirGroup := make(map[string]map[string]android.Paths) + // Also for obj-* directory phony targets. + objModulesInDirGroup := make(map[string]map[string]android.Paths) + + // Collect tidy/obj targets from the 'final' modules. + ctx.VisitAllModules(func(module android.Module) { + if module == ctx.FinalModule(module) { + collectTidyObjModuleTargets(ctx, module, tidyModulesInDirGroup, objModulesInDirGroup) + } + }) + + suffix := "" + if ctx.Config().KatiEnabled() { + suffix = "-soong" + } + generateObjTidyPhonyTargets(ctx, suffix, "obj", objModulesInDirGroup) + generateObjTidyPhonyTargets(ctx, suffix, "tidy", tidyModulesInDirGroup) +} + +// The name for an obj/tidy module variant group phony target is Name_group-obj/tidy, +func objTidyModuleGroupName(module android.Module, group string, suffix string) string { + if group == "" { + return module.Name() + "-" + suffix + } + return module.Name() + "_" + group + "-" + suffix +} + +// Generate obj-* or tidy-* phony targets. +func generateObjTidyPhonyTargets(ctx android.SingletonContext, suffix string, prefix string, objTidyModulesInDirGroup map[string]map[string]android.Paths) { + // For each variant group, create a -_group target that + // depends on all subdirectories and modules in the directory. + for group, modulesInDir := range objTidyModulesInDirGroup { + groupSuffix := "" + if group != "" { + groupSuffix = "_" + group + } + mmTarget := func(dir string) string { + return prefix + "-" + strings.Replace(filepath.Clean(dir), "/", "-", -1) + groupSuffix + } + dirs, topDirs := android.AddAncestors(ctx, modulesInDir, mmTarget) + // Create a -soong_group target that depends on all -dir_group of top level dirs. + var topDirPaths android.Paths + for _, dir := range topDirs { + topDirPaths = append(topDirPaths, android.PathForPhony(ctx, mmTarget(dir))) + } + ctx.Phony(prefix+suffix+groupSuffix, topDirPaths...) + // Create a -dir_group target that depends on all targets in modulesInDir[dir] + for _, dir := range dirs { + if dir != "." && dir != "" { + ctx.Phony(mmTarget(dir), modulesInDir[dir]...) + } + } + } +} + +// Append (obj|tidy)TargetGroups[group] into (obj|tidy)ModulesInDirGroups[group][moduleDir]. +func appendToModulesInDirGroup(targetGroups map[string]android.Path, moduleDir string, modulesInDirGroup map[string]map[string]android.Paths) { + for group, phonyPath := range targetGroups { + if _, found := modulesInDirGroup[group]; !found { + modulesInDirGroup[group] = make(map[string]android.Paths) + } + modulesInDirGroup[group][moduleDir] = append(modulesInDirGroup[group][moduleDir], phonyPath) + } +} + +// Add given files to the OS group and subset group. +func addToOSGroup(osName string, files android.Paths, allGroups, subsetGroups map[string]android.Paths) { + if len(files) > 0 { + subsetName := osName + "_subset" + allGroups[osName] = append(allGroups[osName], files...) + // Now include only the first variant in the subsetGroups. + // If clang and clang-tidy get faster, we might include more variants. + if _, found := subsetGroups[subsetName]; !found { + subsetGroups[subsetName] = files + } + } +} + +// Add an all-OS group, with groupName, to include all os-specific phony targets. +func addAllOSGroup(ctx android.SingletonContext, module android.Module, phonyTargetGroups map[string]android.Paths, groupName string, objTidyName string) { + if len(phonyTargetGroups) > 0 { + var targets android.Paths + for group, _ := range phonyTargetGroups { + targets = append(targets, android.PathForPhony(ctx, objTidyModuleGroupName(module, group, objTidyName))) + } + phonyTargetGroups[groupName] = targets + } +} + +// Create one phony targets for each group and add them to the targetGroups. +func genObjTidyPhonyTargets(ctx android.SingletonContext, module android.Module, objTidyName string, fileGroups map[string]android.Paths, targetGroups map[string]android.Path) { + for group, files := range fileGroups { + groupName := objTidyModuleGroupName(module, group, objTidyName) + ctx.Phony(groupName, files...) + targetGroups[group] = android.PathForPhony(ctx, groupName) + } +}