From 6301c3cffa727ceb72a38a5d0d7af429305ce573 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Tue, 28 Sep 2021 17:40:21 -0700 Subject: [PATCH] Export Soong install rules to Make Previously Soong's install rules have been disabled when embedded in Make (ctx.Config().KatiEnabled() == true). The primary blocker for moving installation into Soong has been the `required` proeprty, which is too vague to be easily handled in Soong. Keeping installation in Make has resulted in two host bin directories, the Make-owned directory (e.g. out/host/linux-x86/bin), and the Soong-owned directory (e.g. out/soong/host/linux-x86/bin). The lack of knowledge in Soong about the final, Make-owned installation location makes it hard to support NOTICE files entirely in Soong. This patch begins to solve this problem by supporting the creation of the installation rules into Soong, but rather than writing the rules to the ninja file it writes them to a Makefile and lets Kati convert them to ninja. This allows Kati to inject extra dependencies to handle the `required` property. Converting all modules to create their installation rules in Soong would be too complex, so only modules that return true from InstallBypassMake will use the Soong installation rules. This is currently only set for robolectric tests. Bug: 204136549 Test: m checkbuild Change-Id: I28af9fa7fadece8ea1f98f5efd140c823751cae7 --- android/androidmk.go | 10 +++ android/makevars.go | 94 +++++++++++++++++++++++++++ android/module.go | 147 ++++++++++++++++++++++++++++++++----------- java/robolectric.go | 12 ++-- 4 files changed, 220 insertions(+), 43 deletions(-) diff --git a/android/androidmk.go b/android/androidmk.go index f5a9f874b..38451b7da 100644 --- a/android/androidmk.go +++ b/android/androidmk.go @@ -516,6 +516,16 @@ func (a *AndroidMkEntries) fillInEntries(ctx fillInEntriesContext, mod blueprint a.AddStrings("LOCAL_HOST_REQUIRED_MODULES", a.Host_required...) a.AddStrings("LOCAL_TARGET_REQUIRED_MODULES", a.Target_required...) + // If the install rule was generated by Soong tell Make about it. + if amod.InstallBypassMake() && len(base.katiInstalls) > 0 { + // Assume the primary install file is last since it probably needs to depend on any other + // installed files. If that is not the case we can add a method to specify the primary + // installed file. + a.SetPath("LOCAL_SOONG_INSTALLED_MODULE", base.katiInstalls[len(base.katiInstalls)-1].to) + a.SetString("LOCAL_SOONG_INSTALL_PAIRS", base.katiInstalls.BuiltInstalled()) + a.SetPaths("LOCAL_SOONG_INSTALL_SYMLINKS", base.katiSymlinks.InstallPaths().Paths()) + } + if am, ok := mod.(ApexModule); ok { a.SetBoolIfTrue("LOCAL_NOT_AVAILABLE_FOR_PLATFORM", am.NotAvailableForPlatform()) } diff --git a/android/makevars.go b/android/makevars.go index 40c0ccded..20db65a50 100644 --- a/android/makevars.go +++ b/android/makevars.go @@ -17,6 +17,8 @@ package android import ( "bytes" "fmt" + "path/filepath" + "runtime" "sort" "strings" @@ -222,6 +224,9 @@ func (s *makeVarsSingleton) GenerateBuildActions(ctx SingletonContext) { lateOutFile := absolutePath(PathForOutput(ctx, "late"+proptools.String(ctx.Config().productVariables.Make_suffix)+".mk").String()) + installsFile := absolutePath(PathForOutput(ctx, + "installs"+proptools.String(ctx.Config().productVariables.Make_suffix)+".mk").String()) + if ctx.Failed() { return } @@ -229,6 +234,8 @@ func (s *makeVarsSingleton) GenerateBuildActions(ctx SingletonContext) { var vars []makeVarsVariable var dists []dist var phonies []phony + var katiInstalls []katiInstall + var katiSymlinks []katiInstall providers := append([]makeVarsProvider(nil), makeVarsInitProviders...) providers = append(providers, *ctx.Config().Get(singletonMakeVarsProvidersKey).(*[]makeVarsProvider)...) @@ -258,6 +265,11 @@ func (s *makeVarsSingleton) GenerateBuildActions(ctx SingletonContext) { phonies = append(phonies, mctx.phonies...) dists = append(dists, mctx.dists...) } + + if m.ExportedToMake() { + katiInstalls = append(katiInstalls, m.base().katiInstalls...) + katiSymlinks = append(katiSymlinks, m.base().katiSymlinks...) + } }) if ctx.Failed() { @@ -297,6 +309,10 @@ func (s *makeVarsSingleton) GenerateBuildActions(ctx SingletonContext) { ctx.Errorf(err.Error()) } + installsBytes := s.writeInstalls(katiInstalls, katiSymlinks) + if err := pathtools.WriteFileIfChanged(installsFile, installsBytes, 0666); err != nil { + ctx.Errorf(err.Error()) + } } func (s *makeVarsSingleton) writeVars(vars []makeVarsVariable) []byte { @@ -405,6 +421,84 @@ func (s *makeVarsSingleton) writeLate(phonies []phony, dists []dist) []byte { return buf.Bytes() } +// writeInstalls writes the list of install rules generated by Soong to a makefile. The rules +// are exported to Make instead of written directly to the ninja file so that main.mk can add +// the dependencies from the `required` property that are hard to resolve in Soong. +func (s *makeVarsSingleton) writeInstalls(installs, symlinks []katiInstall) []byte { + buf := &bytes.Buffer{} + + fmt.Fprint(buf, `# Autogenerated file + +# Values written by Soong to generate install rules that can be amended by Kati. + + +`) + + preserveSymlinksFlag := "-d" + if runtime.GOOS == "darwin" { + preserveSymlinksFlag = "-R" + } + + for _, install := range installs { + // Write a rule for each install request in the form: + // to: from [ deps ] [ | order only deps ] + // cp -f -d $< $@ [ && chmod +x $@ ] + fmt.Fprintf(buf, "%s: %s", install.to.String(), install.from.String()) + for _, dep := range install.implicitDeps { + fmt.Fprintf(buf, " %s", dep.String()) + } + if len(install.orderOnlyDeps) > 0 { + fmt.Fprintf(buf, " |") + } + for _, dep := range install.orderOnlyDeps { + fmt.Fprintf(buf, " %s", dep.String()) + } + fmt.Fprintln(buf) + + fmt.Fprintf(buf, "\trm -f $@ && cp -f %s $< $@", preserveSymlinksFlag) + if install.executable { + fmt.Fprintf(buf, " && chmod +x $@") + } + fmt.Fprintln(buf) + fmt.Fprintln(buf) + } + + for _, symlink := range symlinks { + fmt.Fprintf(buf, "%s:", symlink.to.String()) + for _, dep := range symlink.implicitDeps { + fmt.Fprintf(buf, " %s", dep.String()) + } + if symlink.from != nil || len(symlink.orderOnlyDeps) > 0 { + fmt.Fprintf(buf, " |") + } + if symlink.from != nil { + fmt.Fprintf(buf, " %s", symlink.from.String()) + } + for _, dep := range symlink.orderOnlyDeps { + fmt.Fprintf(buf, " %s", dep.String()) + } + fmt.Fprintln(buf) + + fromStr := "" + if symlink.from != nil { + rel, err := filepath.Rel(filepath.Dir(symlink.to.String()), symlink.from.String()) + if err != nil { + panic(fmt.Errorf("failed to find relative path for symlink from %q to %q: %w", + symlink.from.String(), symlink.to.String(), err)) + } + fromStr = rel + } else { + fromStr = symlink.absFrom + } + + fmt.Fprintf(buf, "\trm -f $@ && ln -sfn %s $@", fromStr) + fmt.Fprintln(buf) + fmt.Fprintln(buf) + } + + return buf.Bytes() +} + func (c *makeVarsContext) DeviceConfig() DeviceConfig { return DeviceConfig{c.Config().deviceConfig} } diff --git a/android/module.go b/android/module.go index 02706eccb..f464e301c 100644 --- a/android/module.go +++ b/android/module.go @@ -1194,7 +1194,10 @@ type ModuleBase struct { packagingSpecs []PackagingSpec packagingSpecsDepSet *packagingSpecsDepSet noticeFiles Paths - phonies map[string]Paths + // katiInstalls tracks the install rules that were created by Soong but are being exported + // to Make to convert to ninja rules so that Make can add additional dependencies. + katiInstalls katiInstalls + katiSymlinks katiInstalls // The files to copy to the dist as explicitly specified in the .bp file. distFiles TaggedDistFiles @@ -2010,9 +2013,8 @@ func (m *ModuleBase) GenerateBuildActions(blueprintCtx blueprint.ModuleContext) m.checkbuildFiles = append(m.checkbuildFiles, ctx.checkbuildFiles...) m.tidyFiles = append(m.tidyFiles, ctx.tidyFiles...) m.packagingSpecs = append(m.packagingSpecs, ctx.packagingSpecs...) - for k, v := range ctx.phonies { - m.phonies[k] = append(m.phonies[k], v...) - } + m.katiInstalls = append(m.katiInstalls, ctx.katiInstalls...) + m.katiSymlinks = append(m.katiSymlinks, ctx.katiSymlinks...) } else if ctx.Config().AllowMissingDependencies() { // If the module is not enabled it will not create any build rules, nothing will call // ctx.GetMissingDependencies(), and blueprint will consider the missing dependencies to be unhandled @@ -2211,12 +2213,52 @@ type moduleContext struct { module Module phonies map[string]Paths + katiInstalls []katiInstall + katiSymlinks []katiInstall + // For tests buildParams []BuildParams ruleParams map[blueprint.Rule]blueprint.RuleParams variables map[string]string } +// katiInstall stores a request from Soong to Make to create an install rule. +type katiInstall struct { + from Path + to InstallPath + implicitDeps Paths + orderOnlyDeps Paths + executable bool + + absFrom string +} + +type katiInstalls []katiInstall + +// BuiltInstalled returns the katiInstalls in the form used by $(call copy-many-files) in Make, a +// space separated list of from:to tuples. +func (installs katiInstalls) BuiltInstalled() string { + sb := strings.Builder{} + for i, install := range installs { + if i != 0 { + sb.WriteRune(' ') + } + sb.WriteString(install.from.String()) + sb.WriteRune(':') + sb.WriteString(install.to.String()) + } + return sb.String() +} + +// InstallPaths returns the install path of each entry. +func (installs katiInstalls) InstallPaths() InstallPaths { + paths := make(InstallPaths, 0, len(installs)) + for _, install := range installs { + paths = append(paths, install.to) + } + return paths +} + func (m *moduleContext) ninjaError(params BuildParams, err error) (PackageContext, BuildParams) { return pctx, BuildParams{ Rule: ErrorRule, @@ -2848,20 +2890,33 @@ func (m *moduleContext) installFile(installPath InstallPath, name string, srcPat orderOnlyDeps = deps } - rule := Cp - if executable { - rule = CpExecutable - } + if m.Config().KatiEnabled() && m.InstallBypassMake() { + // When creating the install rule in Soong but embedding in Make, write the rule to a + // makefile instead of directly to the ninja file so that main.mk can add the + // dependencies from the `required` property that are hard to resolve in Soong. + m.katiInstalls = append(m.katiInstalls, katiInstall{ + from: srcPath, + to: fullInstallPath, + implicitDeps: implicitDeps, + orderOnlyDeps: orderOnlyDeps, + executable: executable, + }) + } else { + rule := Cp + if executable { + rule = CpExecutable + } - m.Build(pctx, BuildParams{ - Rule: rule, - Description: "install " + fullInstallPath.Base(), - Output: fullInstallPath, - Input: srcPath, - Implicits: implicitDeps, - OrderOnly: orderOnlyDeps, - Default: !m.Config().KatiEnabled(), - }) + m.Build(pctx, BuildParams{ + Rule: rule, + Description: "install " + fullInstallPath.Base(), + Output: fullInstallPath, + Input: srcPath, + Implicits: implicitDeps, + OrderOnly: orderOnlyDeps, + Default: !m.Config().KatiEnabled(), + }) + } m.installFiles = append(m.installFiles, fullInstallPath) } @@ -2883,16 +2938,26 @@ func (m *moduleContext) InstallSymlink(installPath InstallPath, name string, src } if !m.skipInstall() { - m.Build(pctx, BuildParams{ - Rule: Symlink, - Description: "install symlink " + fullInstallPath.Base(), - Output: fullInstallPath, - Input: srcPath, - Default: !m.Config().KatiEnabled(), - Args: map[string]string{ - "fromPath": relPath, - }, - }) + if m.Config().KatiEnabled() && m.InstallBypassMake() { + // When creating the symlink rule in Soong but embedding in Make, write the rule to a + // makefile instead of directly to the ninja file so that main.mk can add the + // dependencies from the `required` property that are hard to resolve in Soong. + m.katiSymlinks = append(m.katiSymlinks, katiInstall{ + from: srcPath, + to: fullInstallPath, + }) + } else { + m.Build(pctx, BuildParams{ + Rule: Symlink, + Description: "install symlink " + fullInstallPath.Base(), + Output: fullInstallPath, + Input: srcPath, + Default: !m.Config().KatiEnabled(), + Args: map[string]string{ + "fromPath": relPath, + }, + }) + } m.installFiles = append(m.installFiles, fullInstallPath) m.checkbuildFiles = append(m.checkbuildFiles, srcPath) @@ -2915,15 +2980,25 @@ func (m *moduleContext) InstallAbsoluteSymlink(installPath InstallPath, name str m.module.base().hooks.runInstallHooks(m, nil, fullInstallPath, true) if !m.skipInstall() { - m.Build(pctx, BuildParams{ - Rule: Symlink, - Description: "install symlink " + fullInstallPath.Base() + " -> " + absPath, - Output: fullInstallPath, - Default: !m.Config().KatiEnabled(), - Args: map[string]string{ - "fromPath": absPath, - }, - }) + if m.Config().KatiEnabled() && m.InstallBypassMake() { + // When creating the symlink rule in Soong but embedding in Make, write the rule to a + // makefile instead of directly to the ninja file so that main.mk can add the + // dependencies from the `required` property that are hard to resolve in Soong. + m.katiSymlinks = append(m.katiSymlinks, katiInstall{ + absFrom: absPath, + to: fullInstallPath, + }) + } else { + m.Build(pctx, BuildParams{ + Rule: Symlink, + Description: "install symlink " + fullInstallPath.Base() + " -> " + absPath, + Output: fullInstallPath, + Default: !m.Config().KatiEnabled(), + Args: map[string]string{ + "fromPath": absPath, + }, + }) + } m.installFiles = append(m.installFiles, fullInstallPath) } diff --git a/java/robolectric.go b/java/robolectric.go index a3603adf8..16af546ba 100644 --- a/java/robolectric.go +++ b/java/robolectric.go @@ -212,13 +212,7 @@ func (r *robolectricTest) GenerateAndroidBuildActions(ctx android.ModuleContext) installDeps = append(installDeps, installedData) } - installed := ctx.InstallFile(installPath, ctx.ModuleName()+".jar", r.combinedJar, installDeps...) - - if r.ExportedToMake() { - // Soong handles installation here, but Make is usually what creates the phony rule that atest - // uses to build the module. Create it here for now. - ctx.Phony(ctx.ModuleName(), installed) - } + r.installFile = ctx.InstallFile(installPath, ctx.ModuleName()+".jar", r.combinedJar, installDeps...) } func generateRoboTestConfig(ctx android.ModuleContext, outputFile android.WritablePath, @@ -282,6 +276,10 @@ func (r *robolectricTest) generateRoboSrcJar(ctx android.ModuleContext, outputFi func (r *robolectricTest) AndroidMkEntries() []android.AndroidMkEntries { entriesList := r.Library.AndroidMkEntries() entries := &entriesList[0] + entries.ExtraEntries = append(entries.ExtraEntries, + func(ctx android.AndroidMkExtraEntriesContext, entries *android.AndroidMkEntries) { + entries.SetBool("LOCAL_UNINSTALLABLE_MODULE", true) + }) entries.ExtraFooters = []android.AndroidMkExtraFootersFunc{ func(w io.Writer, name, prefix, moduleDir string) {