From bc1399271117e056fa5bbe72fba63fa7e500f123 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Thu, 25 Mar 2021 18:33:16 -0700 Subject: [PATCH 1/2] Support sandboxing droiddoc and droidstubs with args properties args properties can access arbitrary files with $(location) expansions, so they need to pass them through RuleBuilderCommand.PathsForInputs to produce a path inside the sandbox. Extract the arg expansion out of collectDeps into a new expandArgs method that takes the RuleBuilderCommand. Test: TestDroidstubsSandbox Change-Id: I9022d17bf3cb64c97b2008c4c1b733bf48edca95 --- java/droiddoc.go | 22 +++++++++++++--------- java/droidstubs.go | 10 ++-------- java/droidstubs_test.go | 14 ++++++++++++++ 3 files changed, 29 insertions(+), 17 deletions(-) diff --git a/java/droiddoc.go b/java/droiddoc.go index a8e2b0e48..f7595b1fc 100644 --- a/java/droiddoc.go +++ b/java/droiddoc.go @@ -226,11 +226,8 @@ type Javadoc struct { srcJars android.Paths srcFiles android.Paths sourcepaths android.Paths - argFiles android.Paths implicits android.Paths - args []string - docZip android.WritablePath stubsSrcJar android.WritablePath } @@ -480,15 +477,20 @@ func (j *Javadoc) collectDeps(ctx android.ModuleContext) deps { j.sourcepaths = android.PathsForModuleSrc(ctx, []string{"."}) } - j.argFiles = android.PathsForModuleSrc(ctx, j.properties.Arg_files) + return deps +} + +func (j *Javadoc) expandArgs(ctx android.ModuleContext, cmd *android.RuleBuilderCommand) { + var argFiles android.Paths argFilesMap := map[string]string{} argFileLabels := []string{} for _, label := range j.properties.Arg_files { var paths = android.PathsForModuleSrc(ctx, []string{label}) if _, exists := argFilesMap[label]; !exists { - argFilesMap[label] = strings.Join(paths.Strings(), " ") + argFilesMap[label] = strings.Join(cmd.PathsForInputs(paths), " ") argFileLabels = append(argFileLabels, label) + argFiles = append(argFiles, paths...) } else { ctx.ModuleErrorf("multiple arg_files for %q, %q and %q", label, argFilesMap[label], paths) @@ -508,7 +510,7 @@ func (j *Javadoc) collectDeps(ctx android.ModuleContext) deps { } for _, flag := range flags { - args, err := android.Expand(flag, func(name string) (string, error) { + expanded, err := android.Expand(flag, func(name string) (string, error) { if strings.HasPrefix(name, "location ") { label := strings.TrimSpace(strings.TrimPrefix(name, "location ")) if paths, ok := argFilesMap[label]; ok { @@ -526,10 +528,10 @@ func (j *Javadoc) collectDeps(ctx android.ModuleContext) deps { if err != nil { ctx.PropertyErrorf(argsPropertyName, "%s", err.Error()) } - j.args = append(j.args, args) + cmd.Flag(expanded) } - return deps + cmd.Implicits(argFiles) } func (j *Javadoc) DepsMutator(ctx android.BottomUpMutatorContext) { @@ -563,6 +565,8 @@ func (j *Javadoc) GenerateAndroidBuildActions(ctx android.ModuleContext) { Flag("-XDignore.symbol.file"). Flag("-Xdoclint:none") + j.expandArgs(ctx, cmd) + rule.Command(). BuiltTool("soong_zip"). Flag("-write_if_changed"). @@ -821,7 +825,7 @@ func (d *Droiddoc) GenerateAndroidBuildActions(ctx android.ModuleContext) { deps.bootClasspath, deps.classpath, d.Javadoc.sourcepaths) } - cmd.Flag(strings.Join(d.Javadoc.args, " ")).Implicits(d.Javadoc.argFiles) + d.expandArgs(ctx, cmd) if d.properties.Compat_config != nil { compatConfig := android.PathForModuleSrc(ctx, String(d.properties.Compat_config)) diff --git a/java/droidstubs.go b/java/droidstubs.go index e453e62d6..b42cb66fe 100644 --- a/java/droidstubs.go +++ b/java/droidstubs.go @@ -284,7 +284,7 @@ func (d *Droidstubs) annotationsFlags(ctx android.ModuleContext, cmd *android.Ru cmd.Flag("--include-annotations") validatingNullability := - android.InList("--validate-nullability-from-merged-stubs", d.Javadoc.args) || + strings.Contains(String(d.Javadoc.properties.Args), "--validate-nullability-from-merged-stubs") || String(d.properties.Validate_nullability_from_list) != "" migratingNullability := String(d.properties.Previous_api) != "" @@ -509,14 +509,8 @@ func (d *Droidstubs) GenerateAndroidBuildActions(ctx android.ModuleContext) { d.inclusionAnnotationsFlags(ctx, cmd) d.apiLevelsAnnotationsFlags(ctx, cmd) - if android.InList("--generate-documentation", d.Javadoc.args) { - // Currently Metalava have the ability to invoke Javadoc in a separate process. - // Pass "-nodocs" to suppress the Javadoc invocation when Metalava receives - // "--generate-documentation" arg. This is not needed when Metalava removes this feature. - d.Javadoc.args = append(d.Javadoc.args, "-nodocs") - } + d.expandArgs(ctx, cmd) - cmd.Flag(strings.Join(d.Javadoc.args, " ")).Implicits(d.Javadoc.argFiles) for _, o := range d.Javadoc.properties.Out { cmd.ImplicitOutput(android.PathForModuleGen(ctx, o)) } diff --git a/java/droidstubs_test.go b/java/droidstubs_test.go index c6db97901..6ad932329 100644 --- a/java/droidstubs_test.go +++ b/java/droidstubs_test.go @@ -81,10 +81,19 @@ func TestDroidstubs(t *testing.T) { func TestDroidstubsSandbox(t *testing.T) { ctx, _ := testJavaWithFS(t, ` + genrule { + name: "foo", + out: ["foo.txt"], + cmd: "touch $(out)", + } + droidstubs { name: "bar-stubs", srcs: ["bar-doc/a.java"], sandbox: true, + + args: "--reference $(location :foo)", + arg_files: [":foo"], } `, map[string][]byte{ @@ -96,6 +105,11 @@ func TestDroidstubsSandbox(t *testing.T) { if g, w := metalava.Inputs.Strings(), []string{"bar-doc/a.java"}; !reflect.DeepEqual(w, g) { t.Errorf("Expected inputs %q, got %q", w, g) } + + manifest := android.RuleBuilderSboxProtoForTests(t, m.Output("metalava.sbox.textproto")) + if g, w := manifest.Commands[0].GetCommand(), "reference __SBOX_SANDBOX_DIR__/out/.intermediates/foo/gen/foo.txt"; !strings.Contains(g, w) { + t.Errorf("Expected command to contain %q, got %q", w, g) + } } func TestDroidstubsWithSystemModules(t *testing.T) { From 5f6ffc72f7e1587e7344635bde8febcaeb51d19e Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Mon, 29 Mar 2021 21:54:45 -0700 Subject: [PATCH 2/2] Add dependencies for Metalava's implicit android.jar references Metalava implicitly searches prebuilts/tools/common/api-versions/android-%/android.jar and prebuilts/sdk/%/public/android.jar when looking for --android-jar-patterns matches, and fails if it can't find a match for an API level between 1 and 28 in at least one pattern. Add android.jar files from the api_levels_annotations_dirs directories to try to satisfy these patterns. Bug: 153703940 Test: m docs Change-Id: I866850b33d9a5cd82cc5135bd8f9e9400ed65439 --- java/droidstubs.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/java/droidstubs.go b/java/droidstubs.go index b42cb66fe..be4d8f848 100644 --- a/java/droidstubs.go +++ b/java/droidstubs.go @@ -360,7 +360,16 @@ func (d *Droidstubs) apiLevelsAnnotationsFlags(ctx android.ModuleContext, cmd *a ctx.VisitDirectDepsWithTag(metalavaAPILevelsAnnotationsDirTag, func(m android.Module) { if t, ok := m.(*ExportedDroiddocDir); ok { for _, dep := range t.deps { - if strings.HasSuffix(dep.String(), filename) { + if dep.Base() == filename { + cmd.Implicit(dep) + } + if filename != "android.jar" && dep.Base() == "android.jar" { + // Metalava implicitly searches these patterns: + // prebuilts/tools/common/api-versions/android-%/android.jar + // prebuilts/sdk/%/public/android.jar + // Add android.jar files from the api_levels_annotations_dirs directories to try + // to satisfy these patterns. If Metalava can't find a match for an API level + // between 1 and 28 in at least one pattern it will fail. cmd.Implicit(dep) } }