From 5e6a79798280bec894ffe8ab5b7e9f30e2eca0eb Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Sun, 7 Jun 2020 16:56:32 -0700 Subject: [PATCH 1/5] Allow tests to bypass PathForSource existence checks Forcing every test to specify every file it wants to pass to PathForSource or PathForModuleSrc is painful to maintain and doesn't add any value. Allow tests to reference paths through PathForSource and PathForModuleSrc without specifying them in the mock FS. Test: all soong tests Change-Id: Ia8a8fd965a338d0645b3721314bf91f50146ad21 --- android/config.go | 8 ++++++++ android/paths.go | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/android/config.go b/android/config.go index 59118ce86..a37afe897 100644 --- a/android/config.go +++ b/android/config.go @@ -111,6 +111,10 @@ type config struct { fs pathtools.FileSystem mockBpList string + // If testAllowNonExistentPaths is true then PathForSource and PathForModuleSrc won't error + // in tests when a path doesn't exist. + testAllowNonExistentPaths bool + OncePer } @@ -230,6 +234,10 @@ func TestConfig(buildDir string, env map[string]string, bp string, fs map[string buildDir: buildDir, captureBuild: true, env: envCopy, + + // Set testAllowNonExistentPaths so that test contexts don't need to specify every path + // passed to PathForSource or PathForModuleSrc. + testAllowNonExistentPaths: true, } config.deviceConfig = &deviceConfig{ config: config, diff --git a/android/paths.go b/android/paths.go index 3ad27acbe..bed6f3f58 100644 --- a/android/paths.go +++ b/android/paths.go @@ -405,7 +405,7 @@ func expandOneSrcPath(ctx ModuleContext, s string, expandedExcludes []string) (P p := pathForModuleSrc(ctx, s) if exists, _, err := ctx.Config().fs.Exists(p.String()); err != nil { reportPathErrorf(ctx, "%s: %s", p, err.Error()) - } else if !exists { + } else if !exists && !ctx.Config().testAllowNonExistentPaths { reportPathErrorf(ctx, "module source path %q does not exist", p) } @@ -798,7 +798,7 @@ func PathForSource(ctx PathContext, pathComponents ...string) SourcePath { } } else if exists, _, err := ctx.Config().fs.Exists(path.String()); err != nil { reportPathErrorf(ctx, "%s: %s", path, err.Error()) - } else if !exists { + } else if !exists && !ctx.Config().testAllowNonExistentPaths { reportPathErrorf(ctx, "source path %q does not exist", path) } return path From 238c1f3903eef027c7f1f9448bb6bcc6d4c669cd Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Sun, 7 Jun 2020 16:58:18 -0700 Subject: [PATCH 2/5] Remove most paths from java.TestConfig Now that tests don't need to specify every path passed to PathForSource or PathForModuleSrc, remove most of them from java.TestConfig. Leave a few that are globbed by lots of tests, and move a few that are globbed by a single test into the tests. Test: all soong tests Change-Id: Ica91d7203a6a7dbca0fd4fed84c78f149b8699e1 --- java/app_test.go | 9 +++++-- java/droiddoc.go | 2 +- java/java_test.go | 46 ++++++++++++++++++++++++++--------- java/testing.go | 62 ----------------------------------------------- 4 files changed, 42 insertions(+), 77 deletions(-) diff --git a/java/app_test.go b/java/app_test.go index eeba161b3..e45ba70d5 100644 --- a/java/app_test.go +++ b/java/app_test.go @@ -2657,7 +2657,7 @@ func TestCodelessApp(t *testing.T) { } func TestEmbedNotice(t *testing.T) { - ctx, _ := testJava(t, cc.GatherRequiredDepsForTest(android.Android)+` + ctx, _ := testJavaWithFS(t, cc.GatherRequiredDepsForTest(android.Android)+` android_app { name: "foo", srcs: ["a.java"], @@ -2713,7 +2713,12 @@ func TestEmbedNotice(t *testing.T) { srcs: ["b.java"], notice: "TOOL_NOTICE", } - `) + `, map[string][]byte{ + "APP_NOTICE": nil, + "GENRULE_NOTICE": nil, + "LIB_NOTICE": nil, + "TOOL_NOTICE": nil, + }) // foo has NOTICE files to process, and embed_notices is true. foo := ctx.ModuleForTests("foo", "android_common") diff --git a/java/droiddoc.go b/java/droiddoc.go index 4c3e11236..68cfe9fde 100644 --- a/java/droiddoc.go +++ b/java/droiddoc.go @@ -1470,7 +1470,7 @@ func metalavaCmd(ctx android.ModuleContext, rule *android.RuleBuilder, javaVersi FlagWithInput("@", srcJarList). FlagWithOutput("--strict-input-files:warn ", android.PathForModuleOut(ctx, ctx.ModuleName()+"-"+"violations.txt")) - if implicitsRsp.String() != "" { + if implicitsRsp != nil { cmd.FlagWithArg("--strict-input-files-exempt ", "@"+implicitsRsp.String()) } diff --git a/java/java_test.go b/java/java_test.go index 8ea34d975..215070e6f 100644 --- a/java/java_test.go +++ b/java/java_test.go @@ -142,9 +142,14 @@ func testJavaErrorWithConfig(t *testing.T, pattern string, config android.Config return ctx, config } +func testJavaWithFS(t *testing.T, bp string, fs map[string][]byte) (*android.TestContext, android.Config) { + t.Helper() + return testJavaWithConfig(t, testConfig(nil, bp, fs)) +} + func testJava(t *testing.T, bp string) (*android.TestContext, android.Config) { t.Helper() - return testJavaWithConfig(t, testConfig(nil, bp, nil)) + return testJavaWithFS(t, bp, nil) } func testJavaWithConfig(t *testing.T, config android.Config) (*android.TestContext, android.Config) { @@ -740,7 +745,7 @@ func TestResources(t *testing.T) { for _, test := range table { t.Run(test.name, func(t *testing.T) { - ctx, _ := testJava(t, ` + ctx, _ := testJavaWithFS(t, ` java_library { name: "foo", srcs: [ @@ -750,7 +755,13 @@ func TestResources(t *testing.T) { ], `+test.prop+`, } - `+test.extra) + `+test.extra, + map[string][]byte{ + "java-res/a/a": nil, + "java-res/b/b": nil, + "java-res2/a": nil, + }, + ) foo := ctx.ModuleForTests("foo", "android_common").Output("withres/foo.jar") fooRes := ctx.ModuleForTests("foo", "android_common").Output("res/foo.jar") @@ -769,7 +780,7 @@ func TestResources(t *testing.T) { } func TestIncludeSrcs(t *testing.T) { - ctx, _ := testJava(t, ` + ctx, _ := testJavaWithFS(t, ` java_library { name: "foo", srcs: [ @@ -790,7 +801,11 @@ func TestIncludeSrcs(t *testing.T) { java_resource_dirs: ["java-res"], include_srcs: true, } - `) + `, map[string][]byte{ + "java-res/a/a": nil, + "java-res/b/b": nil, + "java-res2/a": nil, + }) // Test a library with include_srcs: true foo := ctx.ModuleForTests("foo", "android_common").Output("withres/foo.jar") @@ -832,7 +847,7 @@ func TestIncludeSrcs(t *testing.T) { } func TestGeneratedSources(t *testing.T) { - ctx, _ := testJava(t, ` + ctx, _ := testJavaWithFS(t, ` java_library { name: "foo", srcs: [ @@ -847,7 +862,10 @@ func TestGeneratedSources(t *testing.T) { tool_files: ["java-res/a"], out: ["gen.java"], } - `) + `, map[string][]byte{ + "a.java": nil, + "b.java": nil, + }) javac := ctx.ModuleForTests("foo", "android_common").Rule("javac") genrule := ctx.ModuleForTests("gen", "").Rule("generator") @@ -932,7 +950,7 @@ func TestSharding(t *testing.T) { } func TestDroiddoc(t *testing.T) { - ctx, _ := testJava(t, ` + ctx, _ := testJavaWithFS(t, ` droiddoc_exported_dir { name: "droiddoc-templates-sdk", path: ".", @@ -945,7 +963,7 @@ func TestDroiddoc(t *testing.T) { droiddoc { name: "bar-doc", srcs: [ - "bar-doc/*.java", + "bar-doc/a.java", "bar-doc/IFoo.aidl", ":bar-doc-aidl-srcs", ], @@ -963,7 +981,11 @@ func TestDroiddoc(t *testing.T) { todo_file: "libcore-docs-todo.html", args: "-offlinemode -title \"libcore\"", } - `) + `, + map[string][]byte{ + "bar-doc/a.java": nil, + "bar-doc/b.java": nil, + }) barDoc := ctx.ModuleForTests("bar-doc", "android_common").Rule("javadoc") var javaSrcs []string @@ -989,7 +1011,7 @@ func TestDroidstubsWithSystemModules(t *testing.T) { droidstubs { name: "stubs-source-system-modules", srcs: [ - "bar-doc/*.java", + "bar-doc/a.java", ], sdk_version: "none", system_modules: "source-system-modules", @@ -1010,7 +1032,7 @@ func TestDroidstubsWithSystemModules(t *testing.T) { droidstubs { name: "stubs-prebuilt-system-modules", srcs: [ - "bar-doc/*.java", + "bar-doc/a.java", ], sdk_version: "none", system_modules: "prebuilt-system-modules", diff --git a/java/testing.go b/java/testing.go index f993f56b3..f34c64af6 100644 --- a/java/testing.go +++ b/java/testing.go @@ -25,35 +25,12 @@ func TestConfig(buildDir string, env map[string]string, bp string, fs map[string bp += GatherRequiredDepsForTest() mockFS := map[string][]byte{ - "a.java": nil, - "b.java": nil, - "c.java": nil, - "b.kt": nil, - "a.jar": nil, - "b.jar": nil, - "c.jar": nil, - "APP_NOTICE": nil, - "GENRULE_NOTICE": nil, - "LIB_NOTICE": nil, - "TOOL_NOTICE": nil, - "AndroidTest.xml": nil, - "java-res/a/a": nil, - "java-res/b/b": nil, - "java-res2/a": nil, - "java-fg/a.java": nil, - "java-fg/b.java": nil, - "java-fg/c.java": nil, "api/current.txt": nil, "api/removed.txt": nil, "api/system-current.txt": nil, "api/system-removed.txt": nil, "api/test-current.txt": nil, "api/test-removed.txt": nil, - "framework/aidl/a.aidl": nil, - "aidl/foo/IFoo.aidl": nil, - "aidl/bar/IBar.aidl": nil, - "assets_a/a": nil, - "assets_b/b": nil, "prebuilts/sdk/14/public/android.jar": nil, "prebuilts/sdk/14/public/framework.aidl": nil, @@ -103,45 +80,6 @@ func TestConfig(buildDir string, env map[string]string, bp string, fs map[string "prebuilts/sdk/30/test/api/bar-removed.txt": nil, "prebuilts/sdk/tools/core-lambda-stubs.jar": nil, "prebuilts/sdk/Android.bp": []byte(`prebuilt_apis { name: "sdk", api_dirs: ["14", "28", "30", "current"],}`), - - "prebuilts/apk/app.apk": nil, - "prebuilts/apk/app_arm.apk": nil, - "prebuilts/apk/app_arm64.apk": nil, - "prebuilts/apk/app_xhdpi.apk": nil, - "prebuilts/apk/app_xxhdpi.apk": nil, - - "prebuilts/apks/app.apks": nil, - - // For framework-res, which is an implicit dependency for framework - "AndroidManifest.xml": nil, - "build/make/target/product/security/testkey": nil, - - "build/soong/scripts/jar-wrapper.sh": nil, - - "build/make/core/verify_uses_libraries.sh": nil, - - "build/make/core/proguard.flags": nil, - "build/make/core/proguard_basic_keeps.flags": nil, - - "jdk8/jre/lib/jce.jar": nil, - "jdk8/jre/lib/rt.jar": nil, - "jdk8/lib/tools.jar": nil, - - "bar-doc/a.java": nil, - "bar-doc/b.java": nil, - "bar-doc/IFoo.aidl": nil, - "bar-doc/IBar.aidl": nil, - "bar-doc/known_oj_tags.txt": nil, - "external/doclava/templates-sdk": nil, - - "cert/new_cert.x509.pem": nil, - "cert/new_cert.pk8": nil, - "lineage.bin": nil, - - "testdata/data": nil, - - "stubs-sources/foo/Foo.java": nil, - "stubs/sources/foo/Foo.java": nil, } cc.GatherRequiredFilesForTest(mockFS) From 2fce23ae6d0e66c00940fd249845c4880b478092 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Sun, 7 Jun 2020 17:02:48 -0700 Subject: [PATCH 3/5] Remove paths from cc.TestConfig Now that tests don't need to specify every path passed to PathForSource or PathForModuleSrc, remove them from cc.TestConfig. Test: all soong tests Change-Id: I90cd7b4dfc49c156afbb0eea9a77159c3e1860fa --- cc/testing.go | 24 +----------------------- 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/cc/testing.go b/cc/testing.go index edbb24d13..479b424ab 100644 --- a/cc/testing.go +++ b/cc/testing.go @@ -467,15 +467,6 @@ func GatherRequiredDepsForTest(oses ...android.OsType) string { } func GatherRequiredFilesForTest(fs map[string][]byte) { - fs["prebuilts/ndk/current/sources/cxx-stl/llvm-libc++/libs/arm64-v8a/libc++_shared.so"] = nil - fs["prebuilts/ndk/current/platforms/android-27/arch-arm/usr/lib/crtbegin_so.o"] = nil - fs["prebuilts/ndk/current/platforms/android-27/arch-arm/usr/lib/crtend_so.o"] = nil - fs["prebuilts/ndk/current/platforms/android-27/arch-arm64/usr/lib/crtbegin_so.o"] = nil - fs["prebuilts/ndk/current/platforms/android-27/arch-arm64/usr/lib/crtend_so.o"] = nil - fs["prebuilts/ndk/current/platforms/android-27/arch-x86/usr/lib/crtbegin_so.o"] = nil - fs["prebuilts/ndk/current/platforms/android-27/arch-x86/usr/lib/crtend_so.o"] = nil - fs["prebuilts/ndk/current/platforms/android-27/arch-x86_64/usr/lib64/crtbegin_so.o"] = nil - fs["prebuilts/ndk/current/platforms/android-27/arch-x86_64/usr/lib64/crtend_so.o"] = nil } func TestConfig(buildDir string, os android.OsType, env map[string]string, @@ -484,20 +475,7 @@ func TestConfig(buildDir string, os android.OsType, env map[string]string, // add some modules that are required by the compiler and/or linker bp = bp + GatherRequiredDepsForTest(os) - mockFS := map[string][]byte{ - "foo.c": nil, - "foo.lds": nil, - "bar.c": nil, - "baz.c": nil, - "baz.o": nil, - "a.proto": nil, - "b.aidl": nil, - "sub/c.aidl": nil, - "my_include": nil, - "foo.map.txt": nil, - "liba.so": nil, - "libb.a": nil, - } + mockFS := map[string][]byte{} GatherRequiredFilesForTest(mockFS) From c3d87d31129405975f9a889e3c1c96cdcbe275a4 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Thu, 4 Jun 2020 13:25:17 -0700 Subject: [PATCH 4/5] Define Soong phony rules in Make To support dist-for-goals in Soong, we need to define all phony rules in Make so that dist-for-goals can insert additional dependencies on them. Collect all the phony rules in phonySingleton and write them out as Make rules when Soong is embedded in Make, or as blueprint.Phony rules when Soong is run standalone. Test: m checkbuild Change-Id: I68201eff30744b0f487fc4f11f033767b53a627d --- android/Android.bp | 1 + android/makevars.go | 70 ++++++++++++++++++++++++++++-------- android/module.go | 84 +++++++++++++++++--------------------------- android/phony.go | 75 +++++++++++++++++++++++++++++++++++++++ android/register.go | 3 ++ android/singleton.go | 10 ++++++ android/writedocs.go | 6 +--- cc/cc.go | 7 +--- java/java.go | 6 +--- 9 files changed, 180 insertions(+), 82 deletions(-) create mode 100644 android/phony.go diff --git a/android/Android.bp b/android/Android.bp index 47dbc5d5f..487372bd8 100644 --- a/android/Android.bp +++ b/android/Android.bp @@ -36,6 +36,7 @@ bootstrap_go_package { "package_ctx.go", "path_properties.go", "paths.go", + "phony.go", "prebuilt.go", "proto.go", "register.go", diff --git a/android/makevars.go b/android/makevars.go index aba4ccec3..0acd0f665 100644 --- a/android/makevars.go +++ b/android/makevars.go @@ -17,12 +17,11 @@ package android import ( "bytes" "fmt" - "io/ioutil" - "os" "strconv" "strings" "github.com/google/blueprint" + "github.com/google/blueprint/pathtools" "github.com/google/blueprint/proptools" ) @@ -84,6 +83,11 @@ type MakeVarsContext interface { // builder whenever a file matching the pattern as added or removed, without rerunning if a // file that does not match the pattern is added to a searched directory. GlobWithDeps(pattern string, excludes []string) ([]string, error) + + // Phony creates a phony rule in Make, which will allow additional DistForGoal + // dependencies to be added to it. Phony can be called on the same name multiple + // times to add additional dependencies. + Phony(names string, deps ...Path) } var _ PathContext = MakeVarsContext(nil) @@ -130,9 +134,10 @@ var makeVarsProviders []makeVarsProvider type makeVarsContext struct { SingletonContext - config Config - pctx PackageContext - vars []makeVarsVariable + config Config + pctx PackageContext + vars []makeVarsVariable + phonies []phony } var _ MakeVarsContext = &makeVarsContext{} @@ -144,6 +149,11 @@ type makeVarsVariable struct { strict bool } +type phony struct { + name string + deps []string +} + func (s *makeVarsSingleton) GenerateBuildActions(ctx SingletonContext) { if !ctx.Config().EmbeddedInMake() { return @@ -152,11 +162,15 @@ func (s *makeVarsSingleton) GenerateBuildActions(ctx SingletonContext) { outFile := absolutePath(PathForOutput(ctx, "make_vars"+proptools.String(ctx.Config().productVariables.Make_suffix)+".mk").String()) + lateOutFile := absolutePath(PathForOutput(ctx, + "late"+proptools.String(ctx.Config().productVariables.Make_suffix)+".mk").String()) + if ctx.Failed() { return } vars := []makeVarsVariable{} + var phonies []phony for _, provider := range makeVarsProviders { mctx := &makeVarsContext{ SingletonContext: ctx, @@ -166,6 +180,7 @@ func (s *makeVarsSingleton) GenerateBuildActions(ctx SingletonContext) { provider.call(mctx) vars = append(vars, mctx.vars...) + phonies = append(phonies, mctx.phonies...) } if ctx.Failed() { @@ -174,17 +189,16 @@ func (s *makeVarsSingleton) GenerateBuildActions(ctx SingletonContext) { outBytes := s.writeVars(vars) - if _, err := os.Stat(absolutePath(outFile)); err == nil { - if data, err := ioutil.ReadFile(absolutePath(outFile)); err == nil { - if bytes.Equal(data, outBytes) { - return - } - } - } - - if err := ioutil.WriteFile(absolutePath(outFile), outBytes, 0666); err != nil { + if err := pathtools.WriteFileIfChanged(outFile, outBytes, 0666); err != nil { ctx.Errorf(err.Error()) } + + lateOutBytes := s.writeLate(phonies) + + if err := pathtools.WriteFileIfChanged(lateOutFile, lateOutBytes, 0666); err != nil { + ctx.Errorf(err.Error()) + } + } func (s *makeVarsSingleton) writeVars(vars []makeVarsVariable) []byte { @@ -263,6 +277,26 @@ my_check_failed := fmt.Fprintln(buf, "\nsoong-compare-var :=") + fmt.Fprintln(buf) + + return buf.Bytes() +} + +func (s *makeVarsSingleton) writeLate(phonies []phony) []byte { + buf := &bytes.Buffer{} + + fmt.Fprint(buf, `# Autogenerated file + +# Values written by Soong read after parsing all Android.mk files. + + +`) + + for _, phony := range phonies { + fmt.Fprintf(buf, ".PHONY: %s\n", phony.name) + fmt.Fprintf(buf, "%s: %s\n", phony.name, strings.Join(phony.deps, "\\\n ")) + } + return buf.Bytes() } @@ -299,6 +333,10 @@ func (c *makeVarsContext) addVariable(name, ninjaStr string, strict, sort bool) c.addVariableRaw(name, value, strict, sort) } +func (c *makeVarsContext) addPhony(name string, deps []string) { + c.phonies = append(c.phonies, phony{name, deps}) +} + func (c *makeVarsContext) Strict(name, ninjaStr string) { c.addVariable(name, ninjaStr, true, false) } @@ -318,3 +356,7 @@ func (c *makeVarsContext) CheckSorted(name, ninjaStr string) { func (c *makeVarsContext) CheckRaw(name, value string) { c.addVariableRaw(name, value, false, false) } + +func (c *makeVarsContext) Phony(name string, deps ...Path) { + c.addPhony(name, Paths(deps).Strings()) +} diff --git a/android/module.go b/android/module.go index 82321f417..a488c0d77 100644 --- a/android/module.go +++ b/android/module.go @@ -207,6 +207,10 @@ type ModuleContext interface { // Similar to blueprint.ModuleContext.Build, but takes Paths instead of []string, // and performs more verification. Build(pctx PackageContext, params BuildParams) + // Phony creates a Make-style phony rule, a rule with no commands that can depend on other + // phony rules or real files. Phony can be called on the same name multiple times to add + // additional dependencies. + Phony(phony string, deps ...Path) PrimaryModule() Module FinalModule() Module @@ -722,6 +726,7 @@ type ModuleBase struct { installFiles InstallPaths checkbuildFiles Paths noticeFiles Paths + phonies map[string]Paths // Used by buildTargetSingleton to create checkbuild and per-directory build targets // Only set on the final variant of each module @@ -1093,26 +1098,17 @@ func (m *ModuleBase) generateModuleTarget(ctx ModuleContext) { } if len(allInstalledFiles) > 0 { - name := PathForPhony(ctx, namespacePrefix+ctx.ModuleName()+"-install") - ctx.Build(pctx, BuildParams{ - Rule: blueprint.Phony, - Output: name, - Implicits: allInstalledFiles.Paths(), - Default: !ctx.Config().EmbeddedInMake(), - }) - deps = append(deps, name) - m.installTarget = name + name := namespacePrefix + ctx.ModuleName() + "-install" + ctx.Phony(name, allInstalledFiles.Paths()...) + m.installTarget = PathForPhony(ctx, name) + deps = append(deps, m.installTarget) } if len(allCheckbuildFiles) > 0 { - name := PathForPhony(ctx, namespacePrefix+ctx.ModuleName()+"-checkbuild") - ctx.Build(pctx, BuildParams{ - Rule: blueprint.Phony, - Output: name, - Implicits: allCheckbuildFiles, - }) - deps = append(deps, name) - m.checkbuildTarget = name + name := namespacePrefix + ctx.ModuleName() + "-checkbuild" + ctx.Phony(name, allCheckbuildFiles...) + m.checkbuildTarget = PathForPhony(ctx, name) + deps = append(deps, m.checkbuildTarget) } if len(deps) > 0 { @@ -1121,12 +1117,7 @@ func (m *ModuleBase) generateModuleTarget(ctx ModuleContext) { suffix = "-soong" } - name := PathForPhony(ctx, namespacePrefix+ctx.ModuleName()+suffix) - ctx.Build(pctx, BuildParams{ - Rule: blueprint.Phony, - Outputs: []WritablePath{name}, - Implicits: deps, - }) + ctx.Phony(namespacePrefix+ctx.ModuleName()+suffix, deps...) m.blueprintDir = ctx.ModuleDir() } @@ -1313,6 +1304,9 @@ func (m *ModuleBase) GenerateBuildActions(blueprintCtx blueprint.ModuleContext) m.checkbuildFiles = append(m.checkbuildFiles, ctx.checkbuildFiles...) m.initRcPaths = PathsForModuleSrc(ctx, m.commonProperties.Init_rc) m.vintfFragmentsPaths = PathsForModuleSrc(ctx, m.commonProperties.Vintf_fragments) + for k, v := range ctx.phonies { + m.phonies[k] = append(m.phonies[k], v...) + } } 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 @@ -1460,6 +1454,7 @@ type moduleContext struct { installFiles InstallPaths checkbuildFiles Paths module Module + phonies map[string]Paths // For tests buildParams []BuildParams @@ -1574,6 +1569,11 @@ func (m *moduleContext) Build(pctx PackageContext, params BuildParams) { m.bp.Build(pctx.PackageContext, convertBuildParams(params)) } + +func (m *moduleContext) Phony(name string, deps ...Path) { + addPhony(m.config, name, deps...) +} + func (m *moduleContext) GetMissingDependencies() []string { var missingDeps []string missingDeps = append(missingDeps, m.Module().base().commonProperties.MissingDeps...) @@ -2233,9 +2233,8 @@ type buildTargetSingleton struct{} func (c *buildTargetSingleton) GenerateBuildActions(ctx SingletonContext) { var checkbuildDeps Paths - mmTarget := func(dir string) WritablePath { - return PathForPhony(ctx, - "MODULES-IN-"+strings.Replace(filepath.Clean(dir), "/", "-", -1)) + mmTarget := func(dir string) string { + return "MODULES-IN-" + strings.Replace(filepath.Clean(dir), "/", "-", -1) } modulesInDir := make(map[string]Paths) @@ -2261,11 +2260,7 @@ func (c *buildTargetSingleton) GenerateBuildActions(ctx SingletonContext) { } // Create a top-level checkbuild target that depends on all modules - ctx.Build(pctx, BuildParams{ - Rule: blueprint.Phony, - Output: PathForPhony(ctx, "checkbuild"+suffix), - Implicits: checkbuildDeps, - }) + ctx.Phony("checkbuild"+suffix, checkbuildDeps...) // Make will generate the MODULES-IN-* targets if ctx.Config().EmbeddedInMake() { @@ -2289,7 +2284,7 @@ func (c *buildTargetSingleton) GenerateBuildActions(ctx SingletonContext) { for _, dir := range dirs { p := parentDir(dir) if p != "." && p != "/" { - modulesInDir[p] = append(modulesInDir[p], mmTarget(dir)) + modulesInDir[p] = append(modulesInDir[p], PathForPhony(ctx, mmTarget(dir))) } } @@ -2297,14 +2292,7 @@ func (c *buildTargetSingleton) GenerateBuildActions(ctx SingletonContext) { // depends on the MODULES-IN-* targets of all of its subdirectories that contain Android.bp // files. for _, dir := range dirs { - ctx.Build(pctx, BuildParams{ - Rule: blueprint.Phony, - Output: mmTarget(dir), - Implicits: modulesInDir[dir], - // HACK: checkbuild should be an optional build, but force it - // enabled for now in standalone builds - Default: !ctx.Config().EmbeddedInMake(), - }) + ctx.Phony(mmTarget(dir), modulesInDir[dir]...) } // Create (host|host-cross|target)- phony rules to build a reduced checkbuild. @@ -2331,23 +2319,15 @@ func (c *buildTargetSingleton) GenerateBuildActions(ctx SingletonContext) { continue } - name := PathForPhony(ctx, className+"-"+os.Name) - osClass[className] = append(osClass[className], name) + name := className + "-" + os.Name + osClass[className] = append(osClass[className], PathForPhony(ctx, name)) - ctx.Build(pctx, BuildParams{ - Rule: blueprint.Phony, - Output: name, - Implicits: deps, - }) + ctx.Phony(name, deps...) } // Wrap those into host|host-cross|target phony rules for _, class := range SortedStringKeys(osClass) { - ctx.Build(pctx, BuildParams{ - Rule: blueprint.Phony, - Output: PathForPhony(ctx, class), - Implicits: osClass[class], - }) + ctx.Phony(class, osClass[class]...) } } diff --git a/android/phony.go b/android/phony.go new file mode 100644 index 000000000..f8e5a4401 --- /dev/null +++ b/android/phony.go @@ -0,0 +1,75 @@ +// Copyright 2020 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package android + +import ( + "sync" + + "github.com/google/blueprint" +) + +var phonyMapOnceKey = NewOnceKey("phony") + +type phonyMap map[string]Paths + +var phonyMapLock sync.Mutex + +func getPhonyMap(config Config) phonyMap { + return config.Once(phonyMapOnceKey, func() interface{} { + return make(phonyMap) + }).(phonyMap) +} + +func addPhony(config Config, name string, deps ...Path) { + phonyMap := getPhonyMap(config) + phonyMapLock.Lock() + defer phonyMapLock.Unlock() + phonyMap[name] = append(phonyMap[name], deps...) +} + +type phonySingleton struct { + phonyMap phonyMap + phonyList []string +} + +var _ SingletonMakeVarsProvider = (*phonySingleton)(nil) + +func (p *phonySingleton) GenerateBuildActions(ctx SingletonContext) { + p.phonyMap = getPhonyMap(ctx.Config()) + p.phonyList = SortedStringKeys(p.phonyMap) + for _, phony := range p.phonyList { + p.phonyMap[phony] = SortedUniquePaths(p.phonyMap[phony]) + } + + if !ctx.Config().EmbeddedInMake() { + for _, phony := range p.phonyList { + ctx.Build(pctx, BuildParams{ + Rule: blueprint.Phony, + Outputs: []WritablePath{PathForPhony(ctx, phony)}, + Implicits: p.phonyMap[phony], + }) + } + } +} + +func (p phonySingleton) MakeVars(ctx MakeVarsContext) { + for _, phony := range p.phonyList { + ctx.Phony(phony, p.phonyMap[phony]...) + } +} + +func phonySingletonFactory() Singleton { + return &phonySingleton{} +} diff --git a/android/register.go b/android/register.go index ccfe01e74..036a8113f 100644 --- a/android/register.go +++ b/android/register.go @@ -104,6 +104,9 @@ func (ctx *Context) Register() { registerMutators(ctx.Context, preArch, preDeps, postDeps, finalDeps) + // Register phony just before makevars so it can write out its phony rules as Make rules + ctx.RegisterSingletonType("phony", SingletonFactoryAdaptor(phonySingletonFactory)) + // Register makevars after other singletons so they can export values through makevars ctx.RegisterSingletonType("makevars", SingletonFactoryAdaptor(makeVarsSingletonFunc)) diff --git a/android/singleton.go b/android/singleton.go index 568398cdd..2c51c6c48 100644 --- a/android/singleton.go +++ b/android/singleton.go @@ -36,6 +36,12 @@ type SingletonContext interface { Variable(pctx PackageContext, name, value string) Rule(pctx PackageContext, name string, params blueprint.RuleParams, argNames ...string) blueprint.Rule Build(pctx PackageContext, params BuildParams) + + // Phony creates a Make-style phony rule, a rule with no commands that can depend on other + // phony rules or real files. Phony can be called on the same name multiple times to add + // additional dependencies. + Phony(name string, deps ...Path) + RequireNinjaVersion(major, minor, micro int) // SetNinjaBuildDir sets the value of the top-level "builddir" Ninja variable @@ -156,6 +162,10 @@ func (s *singletonContextAdaptor) Build(pctx PackageContext, params BuildParams) } +func (s *singletonContextAdaptor) Phony(name string, deps ...Path) { + addPhony(s.Config(), name, deps...) +} + func (s *singletonContextAdaptor) SetNinjaBuildDir(pctx PackageContext, value string) { s.SingletonContext.SetNinjaBuildDir(pctx.PackageContext, value) } diff --git a/android/writedocs.go b/android/writedocs.go index 7262ad810..9e43e80a6 100644 --- a/android/writedocs.go +++ b/android/writedocs.go @@ -69,9 +69,5 @@ func (c *docsSingleton) GenerateBuildActions(ctx SingletonContext) { }) // Add a phony target for building the documentation - ctx.Build(pctx, BuildParams{ - Rule: blueprint.Phony, - Output: PathForPhony(ctx, "soong_docs"), - Input: docsFile, - }) + ctx.Phony("soong_docs", docsFile) } diff --git a/cc/cc.go b/cc/cc.go index 8eabff516..31d35e1bb 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -3230,12 +3230,7 @@ func (ks *kytheExtractAllSingleton) GenerateBuildActions(ctx android.SingletonCo }) // TODO(asmundak): Perhaps emit a rule to output a warning if there were no xrefTargets if len(xrefTargets) > 0 { - ctx.Build(pctx, android.BuildParams{ - Rule: blueprint.Phony, - Output: android.PathForPhony(ctx, "xref_cxx"), - Inputs: xrefTargets, - //Default: true, - }) + ctx.Phony("xref_cxx", xrefTargets...) } } diff --git a/java/java.go b/java/java.go index af68e56b7..9a352dbe0 100644 --- a/java/java.go +++ b/java/java.go @@ -2863,11 +2863,7 @@ func (ks *kytheExtractJavaSingleton) GenerateBuildActions(ctx android.SingletonC }) // TODO(asmundak): perhaps emit a rule to output a warning if there were no xrefTargets if len(xrefTargets) > 0 { - ctx.Build(pctx, android.BuildParams{ - Rule: blueprint.Phony, - Output: android.PathForPhony(ctx, "xref_java"), - Inputs: xrefTargets, - }) + ctx.Phony("xref_java", xrefTargets...) } } From 3cda0d8df9d3979cbb516a49f15b47e116ab095c Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Tue, 24 Sep 2019 13:40:07 -0700 Subject: [PATCH 5/5] Add DistForGoal to MakeVarsContext Add methods to MakeVarsContext to allow Singletons to dist artifacts without manually adding $(dist-for-goals) in Make. Test: m checkbuild Change-Id: Ia5ddb31afe29329f2df0ae1297ed963c8c28e590 --- android/makevars.go | 62 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 59 insertions(+), 3 deletions(-) diff --git a/android/makevars.go b/android/makevars.go index 0acd0f665..ff7c8e4a7 100644 --- a/android/makevars.go +++ b/android/makevars.go @@ -88,6 +88,24 @@ type MakeVarsContext interface { // dependencies to be added to it. Phony can be called on the same name multiple // times to add additional dependencies. Phony(names string, deps ...Path) + + // DistForGoal creates a rule to copy one or more Paths to the artifacts + // directory on the build server when the specified goal is built. + DistForGoal(goal string, paths ...Path) + + // DistForGoalWithFilename creates a rule to copy a Path to the artifacts + // directory on the build server with the given filename when the specified + // goal is built. + DistForGoalWithFilename(goal string, path Path, filename string) + + // DistForGoals creates a rule to copy one or more Paths to the artifacts + // directory on the build server when any of the specified goals are built. + DistForGoals(goals []string, paths ...Path) + + // DistForGoalsWithFilename creates a rule to copy a Path to the artifacts + // directory on the build server with the given filename when any of the + // specified goals are built. + DistForGoalsWithFilename(goals []string, path Path, filename string) } var _ PathContext = MakeVarsContext(nil) @@ -138,6 +156,7 @@ type makeVarsContext struct { pctx PackageContext vars []makeVarsVariable phonies []phony + dists []dist } var _ MakeVarsContext = &makeVarsContext{} @@ -154,6 +173,11 @@ type phony struct { deps []string } +type dist struct { + goals []string + paths []string +} + func (s *makeVarsSingleton) GenerateBuildActions(ctx SingletonContext) { if !ctx.Config().EmbeddedInMake() { return @@ -169,7 +193,8 @@ func (s *makeVarsSingleton) GenerateBuildActions(ctx SingletonContext) { return } - vars := []makeVarsVariable{} + var vars []makeVarsVariable + var dists []dist var phonies []phony for _, provider := range makeVarsProviders { mctx := &makeVarsContext{ @@ -181,6 +206,7 @@ func (s *makeVarsSingleton) GenerateBuildActions(ctx SingletonContext) { vars = append(vars, mctx.vars...) phonies = append(phonies, mctx.phonies...) + dists = append(dists, mctx.dists...) } if ctx.Failed() { @@ -193,7 +219,7 @@ func (s *makeVarsSingleton) GenerateBuildActions(ctx SingletonContext) { ctx.Errorf(err.Error()) } - lateOutBytes := s.writeLate(phonies) + lateOutBytes := s.writeLate(phonies, dists) if err := pathtools.WriteFileIfChanged(lateOutFile, lateOutBytes, 0666); err != nil { ctx.Errorf(err.Error()) @@ -282,7 +308,7 @@ my_check_failed := return buf.Bytes() } -func (s *makeVarsSingleton) writeLate(phonies []phony) []byte { +func (s *makeVarsSingleton) writeLate(phonies []phony, dists []dist) []byte { buf := &bytes.Buffer{} fmt.Fprint(buf, `# Autogenerated file @@ -297,6 +323,13 @@ func (s *makeVarsSingleton) writeLate(phonies []phony) []byte { fmt.Fprintf(buf, "%s: %s\n", phony.name, strings.Join(phony.deps, "\\\n ")) } + fmt.Fprintln(buf) + + for _, dist := range dists { + fmt.Fprintf(buf, "$(call dist-for-goals,%s,%s)\n", + strings.Join(dist.goals, " "), strings.Join(dist.paths, " ")) + } + return buf.Bytes() } @@ -337,6 +370,13 @@ func (c *makeVarsContext) addPhony(name string, deps []string) { c.phonies = append(c.phonies, phony{name, deps}) } +func (c *makeVarsContext) addDist(goals []string, paths []string) { + c.dists = append(c.dists, dist{ + goals: goals, + paths: paths, + }) +} + func (c *makeVarsContext) Strict(name, ninjaStr string) { c.addVariable(name, ninjaStr, true, false) } @@ -360,3 +400,19 @@ func (c *makeVarsContext) CheckRaw(name, value string) { func (c *makeVarsContext) Phony(name string, deps ...Path) { c.addPhony(name, Paths(deps).Strings()) } + +func (c *makeVarsContext) DistForGoal(goal string, paths ...Path) { + c.DistForGoals([]string{goal}, paths...) +} + +func (c *makeVarsContext) DistForGoalWithFilename(goal string, path Path, filename string) { + c.DistForGoalsWithFilename([]string{goal}, path, filename) +} + +func (c *makeVarsContext) DistForGoals(goals []string, paths ...Path) { + c.addDist(goals, Paths(paths).Strings()) +} + +func (c *makeVarsContext) DistForGoalsWithFilename(goals []string, path Path, filename string) { + c.addDist(goals, []string{path.String() + ":" + filename}) +}