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

@@ -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

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-<directory> 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-<directory> 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

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() {

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,
 	})
 }

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 {

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 <prefix>-<directory>_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 <prefix>-soong_group target that depends on all <prefix>-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 <prefix>-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)
+	}
+}