From fb1271a52b723ff62f53a09e83a988d7aee4a00e Mon Sep 17 00:00:00 2001 From: Dan Willemsen Date: Wed, 26 Sep 2018 15:00:42 -0700 Subject: [PATCH] Add a Kati-based packaging step The idea is that we'd move the installation and packaging tasks over to it, using data from Soong & the Kati reading Android.mk files. This would allow us to make more fundamental changes about how we package things without having to adjust makefiles throughout the tree. Possible use cases: * Moving some information from Soong's Android.mk output to a file read by the packaging step may allow us to read the Android.mk files less often, speeding up builds. * Refactoring our current two-stage ASAN builds to run the Kati build step twice, writing into different object directories, then have a single packaging step that reads both outputs. Soong already has the capability of writing out a single ninja file with all the asan combinations. * Running two build steps, one building the system-related modules using a "generic" device configuration, and one building the vendor modules using a specific device configuration. This could enforce a GSI/mainline system vs vendor split in a single build invocation. * If all installation is through this tool, it will be much easier to track what should no longer be installed on an incremental build, reducing the need for installclean. * Changing PRODUCT_PACKAGES should be a much faster operation, which means we could keep track of local additions to the images. Then `mma` would be more persistent, instead of installing something once, then never updating it again. Eventually we plan on switching from Kati to something Go-based, but this is a more incremental approach while we clean up everything else. Currently, this just moves the dist-for-goal handling over to the packaging step, so that we don't need to read Android.mk files when DIST_DIR changes, or we switch between dist vs not. Bug: 116968624 Bug: 117463001 Test: m nothing Change-Id: Idec5ac6f7c7475397ba0fb65bd3785128a7517df --- ui/build/build.go | 6 ++-- ui/build/config.go | 8 +++++ ui/build/dumpvars.go | 1 - ui/build/environment.go | 11 +++++++ ui/build/environment_test.go | 9 +++++ ui/build/kati.go | 64 +++++++++++++++++++++++++++++++----- ui/status/kati.go | 2 +- 7 files changed, 89 insertions(+), 12 deletions(-) diff --git a/ui/build/build.go b/ui/build/build.go index 377481be9..c902a0f8e 100644 --- a/ui/build/build.go +++ b/ui/build/build.go @@ -40,9 +40,10 @@ builddir = {{.OutDir}} pool local_pool depth = {{.Parallel}} build _kati_always_build_: phony -{{if .HasKatiSuffix}}include {{.KatiBuildNinjaFile}} +{{if .HasKatiSuffix}}subninja {{.KatiBuildNinjaFile}} +subninja {{.KatiPackageNinjaFile}} {{end -}} -include {{.SoongNinjaFile}} +subninja {{.SoongNinjaFile}} `)) func createCombinedBuildNinjaFile(ctx Context, config Config) { @@ -180,6 +181,7 @@ func Build(ctx Context, config Config, what int) { genKatiSuffix(ctx, config) runKatiCleanSpec(ctx, config) runKatiBuild(ctx, config) + runKatiPackage(ctx, config) ioutil.WriteFile(config.LastKatiSuffixFile(), []byte(config.KatiSuffix()), 0777) } else { diff --git a/ui/build/config.go b/ui/build/config.go index d65d97f28..1008b2e13 100644 --- a/ui/build/config.go +++ b/ui/build/config.go @@ -508,6 +508,10 @@ func (c *configImpl) KatiBuildNinjaFile() string { return filepath.Join(c.OutDir(), "build"+c.KatiSuffix()+katiBuildSuffix+".ninja") } +func (c *configImpl) KatiPackageNinjaFile() string { + return filepath.Join(c.OutDir(), "build"+c.KatiSuffix()+katiPackageSuffix+".ninja") +} + func (c *configImpl) SoongNinjaFile() string { return filepath.Join(c.SoongOutDir(), "build.ninja") } @@ -535,6 +539,10 @@ func (c *configImpl) DevicePreviousProductConfig() string { return filepath.Join(c.ProductOut(), "previous_build_config.mk") } +func (c *configImpl) KatiPackageMkDir() string { + return filepath.Join(c.ProductOut(), "obj", "CONFIG", "kati_packaging") +} + func (c *configImpl) hostOutRoot() string { return filepath.Join(c.OutDir(), "host") } diff --git a/ui/build/dumpvars.go b/ui/build/dumpvars.go index ffea841a4..bea08b67c 100644 --- a/ui/build/dumpvars.go +++ b/ui/build/dumpvars.go @@ -48,7 +48,6 @@ func dumpMakeVars(ctx Context, config Config, goals, vars []string, write_soong_ "dump-many-vars", "MAKECMDGOALS="+strings.Join(goals, " ")) cmd.Environment.Set("CALLED_FROM_SETUP", "true") - cmd.Environment.Set("BUILD_SYSTEM", "build/make/core") if write_soong_vars { cmd.Environment.Set("WRITE_SOONG_VARIABLES", "true") } diff --git a/ui/build/environment.go b/ui/build/environment.go index cbeeb4bc4..d8ff7f207 100644 --- a/ui/build/environment.go +++ b/ui/build/environment.go @@ -75,6 +75,17 @@ func (e *Environment) UnsetWithPrefix(prefix string) { *e = out } +// Allow removes all keys that are not present in the input list +func (e *Environment) Allow(keys ...string) { + out := (*e)[:0] + for _, env := range *e { + if key, _, ok := decodeKeyValue(env); ok && inList(key, keys) { + out = append(out, env) + } + } + *e = out +} + // Environ returns the []string required for exec.Cmd.Env func (e *Environment) Environ() []string { return []string(*e) diff --git a/ui/build/environment_test.go b/ui/build/environment_test.go index 0294dac46..37f500f30 100644 --- a/ui/build/environment_test.go +++ b/ui/build/environment_test.go @@ -56,6 +56,15 @@ func TestEnvSetDup(t *testing.T) { } } +func TestEnvAllow(t *testing.T) { + initial := &Environment{"TEST=1", "TEST2=0", "TEST3=2"} + initial.Allow("TEST3", "TEST") + got := initial.Environ() + if len(got) != 2 || got[0] != "TEST=1" || got[1] != "TEST3=2" { + t.Errorf("Expected [TEST=1 TEST3=2], got: %v", got) + } +} + const testKatiEnvFileContents = `#!/bin/sh # Generated by kati unknown diff --git a/ui/build/kati.go b/ui/build/kati.go index 546fd1ac1..b3e820e2c 100644 --- a/ui/build/kati.go +++ b/ui/build/kati.go @@ -28,6 +28,7 @@ var spaceSlashReplacer = strings.NewReplacer("/", "_", " ", "_") const katiBuildSuffix = "" const katiCleanspecSuffix = "-cleanspec" +const katiPackageSuffix = "-package" // genKatiSuffix creates a suffix for kati-generated files so that we can cache // them based on their inputs. So this should encode all common changes to Kati @@ -59,7 +60,7 @@ func genKatiSuffix(ctx Context, config Config) { } } -func runKati(ctx Context, config Config, extraSuffix string, args []string) { +func runKati(ctx Context, config Config, extraSuffix string, args []string, envFunc func(*Environment)) { executable := config.PrebuiltBuildTool("ckati") args = append([]string{ "--ninja", @@ -80,10 +81,6 @@ func runKati(ctx Context, config Config, extraSuffix string, args []string) { "--kati_stats", }, args...) - args = append(args, - "SOONG_MAKEVARS_MK="+config.SoongMakeVarsMk(), - "TARGET_DEVICE_DIR="+config.TargetDeviceDir()) - cmd := Command(ctx, config, "ckati", executable, args...) cmd.Sandbox = katiSandbox pipe, err := cmd.StdoutPipe() @@ -92,6 +89,8 @@ func runKati(ctx Context, config Config, extraSuffix string, args []string) { } cmd.Stderr = cmd.Stdout + envFunc(cmd.Environment) + cmd.StartOrFatal() status.KatiReader(ctx.Status.StartTool(), pipe) cmd.WaitOrFatal() @@ -103,7 +102,6 @@ func runKatiBuild(ctx Context, config Config) { args := []string{ "--writable", config.OutDir() + "/", - "--writable", config.DistDir() + "/", "-f", "build/make/core/main.mk", } @@ -125,9 +123,55 @@ func runKatiBuild(ctx Context, config Config) { args = append(args, config.KatiArgs()...) - args = append(args, "SOONG_ANDROID_MK="+config.SoongAndroidMk()) + args = append(args, + "SOONG_MAKEVARS_MK="+config.SoongMakeVarsMk(), + "SOONG_ANDROID_MK="+config.SoongAndroidMk(), + "TARGET_DEVICE_DIR="+config.TargetDeviceDir(), + "KATI_PACKAGE_MK_DIR="+config.KatiPackageMkDir()) - runKati(ctx, config, katiBuildSuffix, args) + runKati(ctx, config, katiBuildSuffix, args, func(env *Environment) { + env.Unset("DIST_DIR") + }) +} + +func runKatiPackage(ctx Context, config Config) { + ctx.BeginTrace("kati package") + defer ctx.EndTrace() + + args := []string{ + "--writable", config.DistDir() + "/", + "--werror_writable", + "--werror_implicit_rules", + "--werror_overriding_commands", + "--werror_real_to_phony", + "--werror_phony_looks_real", + "-f", "build/make/packaging/main.mk", + "KATI_PACKAGE_MK_DIR=" + config.KatiPackageMkDir(), + } + + runKati(ctx, config, katiPackageSuffix, args, func(env *Environment) { + env.Allow([]string{ + // Some generic basics + "LANG", + "LC_MESSAGES", + "PATH", + "PWD", + "TMPDIR", + + // Tool configs + "JAVA_HOME", + "PYTHONDONTWRITEBYTECODE", + + // Build configuration + "ANDROID_BUILD_SHELL", + "DIST_DIR", + "OUT_DIR", + }...) + + if config.Dist() { + env.Set("DIST", "true") + } + }) } func runKatiCleanSpec(ctx Context, config Config) { @@ -138,5 +182,9 @@ func runKatiCleanSpec(ctx Context, config Config) { "--werror_implicit_rules", "--werror_overriding_commands", "-f", "build/make/core/cleanbuild.mk", + "SOONG_MAKEVARS_MK=" + config.SoongMakeVarsMk(), + "TARGET_DEVICE_DIR=" + config.TargetDeviceDir(), + }, func(env *Environment) { + env.Unset("DIST_DIR") }) } diff --git a/ui/status/kati.go b/ui/status/kati.go index 552a9e970..7c26d427d 100644 --- a/ui/status/kati.go +++ b/ui/status/kati.go @@ -24,7 +24,7 @@ import ( ) var katiError = regexp.MustCompile(`^(\033\[1m)?[^ ]+:[0-9]+: (\033\[31m)?error:`) -var katiIncludeRe = regexp.MustCompile(`^(\[(\d+)/(\d+)] )?((including [^ ]+|initializing build system|finishing build rules|writing build rules) ...)$`) +var katiIncludeRe = regexp.MustCompile(`^(\[(\d+)/(\d+)] )?((including [^ ]+|initializing (build|packaging) system|finishing (build|packaging) rules|writing (build|packaging) rules) ...)$`) var katiLogRe = regexp.MustCompile(`^\*kati\*: `) var katiNinjaMissing = regexp.MustCompile("^[^ ]+ is missing, regenerating...$")