From 2b9c5f0c95c74eb5dfbea9bacd57c56de83a16f7 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Mon, 6 Jul 2020 11:45:51 -0700 Subject: Support lint on unbundled builds Use prebuilts of the annotations.zip and api-versions.xml files when running lint in an unbundled build. Bug: 153485543 Test: m TARGET_BUILD_APPS=Gallery2 lint-check Change-Id: Idacf3758a2769678a635941486183673e95b43f8 Merged-In: Idacf3758a2769678a635941486183673e95b43f8 (cherry picked from commit 8a6ed3750df5ce7fb4a8f2b1b623bfa1071449cb) --- java/lint.go | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/java/lint.go b/java/lint.go index b73d6a51..6fbef18c 100644 --- a/java/lint.go +++ b/java/lint.go @@ -220,12 +220,21 @@ func (l *linter) lint(ctx android.ModuleContext) { rule.Command().Text("rm -rf").Flag(cacheDir.String()).Flag(homeDir.String()) rule.Command().Text("mkdir -p").Flag(cacheDir.String()).Flag(homeDir.String()) + var annotationsZipPath, apiVersionsXMLPath android.Path + if ctx.Config().UnbundledBuildUsePrebuiltSdks() { + annotationsZipPath = android.PathForSource(ctx, "prebuilts/sdk/current/public/data/annotations.zip") + apiVersionsXMLPath = android.PathForSource(ctx, "prebuilts/sdk/current/public/data/api-versions.xml") + } else { + annotationsZipPath = copiedAnnotationsZipPath(ctx) + apiVersionsXMLPath = copiedAPIVersionsXmlPath(ctx) + } + rule.Command(). Text("("). Flag("JAVA_OPTS=-Xmx2048m"). FlagWithArg("ANDROID_SDK_HOME=", homeDir.String()). - FlagWithInput("SDK_ANNOTATIONS=", annotationsZipPath(ctx)). - FlagWithInput("LINT_OPTS=-DLINT_API_DATABASE=", apiVersionsXmlPath(ctx)). + FlagWithInput("SDK_ANNOTATIONS=", annotationsZipPath). + FlagWithInput("LINT_OPTS=-DLINT_API_DATABASE=", apiVersionsXMLPath). Tool(android.PathForSource(ctx, "prebuilts/cmdline-tools/tools/bin/lint")). Implicit(android.PathForSource(ctx, "prebuilts/cmdline-tools/tools/lib/lint-classpath.jar")). Flag("--quiet"). @@ -271,7 +280,7 @@ func (l *lintSingleton) GenerateBuildActions(ctx android.SingletonContext) { } func (l *lintSingleton) copyLintDependencies(ctx android.SingletonContext) { - if ctx.Config().UnbundledBuild() { + if ctx.Config().UnbundledBuildUsePrebuiltSdks() { return } @@ -297,25 +306,29 @@ func (l *lintSingleton) copyLintDependencies(ctx android.SingletonContext) { ctx.Build(pctx, android.BuildParams{ Rule: android.Cp, Input: android.OutputFileForModule(ctx, frameworkDocStubs, ".annotations.zip"), - Output: annotationsZipPath(ctx), + Output: copiedAnnotationsZipPath(ctx), }) ctx.Build(pctx, android.BuildParams{ Rule: android.Cp, Input: android.OutputFileForModule(ctx, frameworkDocStubs, ".api_versions.xml"), - Output: apiVersionsXmlPath(ctx), + Output: copiedAPIVersionsXmlPath(ctx), }) } -func annotationsZipPath(ctx android.PathContext) android.WritablePath { +func copiedAnnotationsZipPath(ctx android.PathContext) android.WritablePath { return android.PathForOutput(ctx, "lint", "annotations.zip") } -func apiVersionsXmlPath(ctx android.PathContext) android.WritablePath { +func copiedAPIVersionsXmlPath(ctx android.PathContext) android.WritablePath { return android.PathForOutput(ctx, "lint", "api_versions.xml") } func (l *lintSingleton) generateLintReportZips(ctx android.SingletonContext) { + if ctx.Config().UnbundledBuild() { + return + } + var outputs []*lintOutputs var dirs []string ctx.VisitAllModules(func(m android.Module) { @@ -370,7 +383,9 @@ func (l *lintSingleton) generateLintReportZips(ctx android.SingletonContext) { } func (l *lintSingleton) MakeVars(ctx android.MakeVarsContext) { - ctx.DistForGoal("lint-check", l.htmlZip, l.textZip, l.xmlZip) + if !ctx.Config().UnbundledBuild() { + ctx.DistForGoal("lint-check", l.htmlZip, l.textZip, l.xmlZip) + } } var _ android.SingletonMakeVarsProvider = (*lintSingleton)(nil) -- cgit v1.2.3 From 70f590946be178907070a152a1daea2845f0a485 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Tue, 14 Jul 2020 15:02:16 -0700 Subject: Add DepSets Add DepSets to efficiently support tracking transitive Paths without copying, based on Bazel's depsets: https://docs.bazel.build/versions/master/skylark/depsets.html Bug: 153485543 Test: depset_test.go Change-Id: If744affdf1ed850113166ba611a79a891262040c Merged-In: If744affdf1ed850113166ba611a79a891262040c (cherry picked from commit 9e44e21e91cf38aa0a863bd87cffa6fe3534cc04) --- android/Android.bp | 2 + android/depset.go | 190 +++++++++++++++++++++++++++++++ android/depset_test.go | 304 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 496 insertions(+) create mode 100644 android/depset.go create mode 100644 android/depset_test.go diff --git a/android/Android.bp b/android/Android.bp index d904a86c..9712c46e 100644 --- a/android/Android.bp +++ b/android/Android.bp @@ -18,6 +18,7 @@ bootstrap_go_package { "csuite_config.go", "defaults.go", "defs.go", + "depset.go", "expand.go", "filegroup.go", "hooks.go", @@ -59,6 +60,7 @@ bootstrap_go_package { "arch_test.go", "config_test.go", "csuite_config_test.go", + "depset_test.go", "expand_test.go", "module_test.go", "mutator_test.go", diff --git a/android/depset.go b/android/depset.go new file mode 100644 index 00000000..f7070942 --- /dev/null +++ b/android/depset.go @@ -0,0 +1,190 @@ +// 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 "fmt" + +// DepSet is designed to be conceptually compatible with Bazel's depsets: +// https://docs.bazel.build/versions/master/skylark/depsets.html + +// A DepSet efficiently stores Paths from transitive dependencies without copying. It is stored +// as a DAG of DepSet nodes, each of which has some direct contents and a list of dependency +// DepSet nodes. +// +// A DepSet has an order that will be used to walk the DAG when ToList() is called. The order +// can be POSTORDER, PREORDER, or TOPOLOGICAL. POSTORDER and PREORDER orders return a postordered +// or preordered left to right flattened list. TOPOLOGICAL returns a list that guarantees that +// elements of children are listed after all of their parents (unless there are duplicate direct +// elements in the DepSet or any of its transitive dependencies, in which case the ordering of the +// duplicated element is not guaranteed). +// +// A DepSet is created by NewDepSet or NewDepSetBuilder.Build from the Paths for direct contents +// and the *DepSets of dependencies. A DepSet is immutable once created. +type DepSet struct { + preorder bool + reverse bool + order DepSetOrder + direct Paths + transitive []*DepSet +} + +// DepSetBuilder is used to create an immutable DepSet. +type DepSetBuilder struct { + order DepSetOrder + direct Paths + transitive []*DepSet +} + +type DepSetOrder int + +const ( + PREORDER DepSetOrder = iota + POSTORDER + TOPOLOGICAL +) + +func (o DepSetOrder) String() string { + switch o { + case PREORDER: + return "PREORDER" + case POSTORDER: + return "POSTORDER" + case TOPOLOGICAL: + return "TOPOLOGICAL" + default: + panic(fmt.Errorf("Invalid DepSetOrder %d", o)) + } +} + +// NewDepSet returns an immutable DepSet with the given order, direct and transitive contents. +func NewDepSet(order DepSetOrder, direct Paths, transitive []*DepSet) *DepSet { + var directCopy Paths + var transitiveCopy []*DepSet + if order == TOPOLOGICAL { + directCopy = ReversePaths(direct) + transitiveCopy = reverseDepSets(transitive) + } else { + // Use copy instead of append(nil, ...) to make a slice that is exactly the size of the input + // slice. The DepSet is immutable, there is no need for additional capacity. + directCopy = make(Paths, len(direct)) + copy(directCopy, direct) + transitiveCopy = make([]*DepSet, len(transitive)) + copy(transitiveCopy, transitive) + } + + for _, dep := range transitive { + if dep.order != order { + panic(fmt.Errorf("incompatible order, new DepSet is %s but transitive DepSet is %s", + order, dep.order)) + } + } + + return &DepSet{ + preorder: order == PREORDER, + reverse: order == TOPOLOGICAL, + order: order, + direct: directCopy, + transitive: transitiveCopy, + } +} + +// NewDepSetBuilder returns a DepSetBuilder to create an immutable DepSet with the given order. +func NewDepSetBuilder(order DepSetOrder) *DepSetBuilder { + return &DepSetBuilder{order: order} +} + +// Direct adds direct contents to the DepSet being built by a DepSetBuilder. Newly added direct +// contents are to the right of any existing direct contents. +func (b *DepSetBuilder) Direct(direct ...Path) *DepSetBuilder { + b.direct = append(b.direct, direct...) + return b +} + +// Transitive adds transitive contents to the DepSet being built by a DepSetBuilder. Newly added +// transitive contents are to the right of any existing transitive contents. +func (b *DepSetBuilder) Transitive(transitive ...*DepSet) *DepSetBuilder { + b.transitive = append(b.transitive, transitive...) + return b +} + +// Returns the DepSet being built by this DepSetBuilder. The DepSetBuilder retains its contents +// for creating more DepSets. +func (b *DepSetBuilder) Build() *DepSet { + return NewDepSet(b.order, b.direct, b.transitive) +} + +// walk calls the visit method in depth-first order on a DepSet, preordered if d.preorder is set, +// otherwise postordered. +func (d *DepSet) walk(visit func(Paths)) { + visited := make(map[*DepSet]bool) + + var dfs func(d *DepSet) + dfs = func(d *DepSet) { + visited[d] = true + if d.preorder { + visit(d.direct) + } + for _, dep := range d.transitive { + if !visited[dep] { + dfs(dep) + } + } + + if !d.preorder { + visit(d.direct) + } + } + + dfs(d) +} + +// ToList returns the DepSet flattened to a list. The order in the list is based on the order +// of the DepSet. POSTORDER and PREORDER orders return a postordered or preordered left to right +// flattened list. TOPOLOGICAL returns a list that guarantees that elements of children are listed +// after all of their parents (unless there are duplicate direct elements in the DepSet or any of +// its transitive dependencies, in which case the ordering of the duplicated element is not +// guaranteed). +func (d *DepSet) ToList() Paths { + var list Paths + d.walk(func(paths Paths) { + list = append(list, paths...) + }) + list = FirstUniquePaths(list) + if d.reverse { + reversePathsInPlace(list) + } + return list +} + +// ToSortedList returns the direct and transitive contents of a DepSet in lexically sorted order +// with duplicates removed. +func (d *DepSet) ToSortedList() Paths { + list := d.ToList() + return SortedUniquePaths(list) +} + +func reversePathsInPlace(list Paths) { + for i, j := 0, len(list)-1; i < j; i, j = i+1, j-1 { + list[i], list[j] = list[j], list[i] + } +} + +func reverseDepSets(list []*DepSet) []*DepSet { + ret := make([]*DepSet, len(list)) + for i := range list { + ret[i] = list[len(list)-1-i] + } + return ret +} diff --git a/android/depset_test.go b/android/depset_test.go new file mode 100644 index 00000000..c328127b --- /dev/null +++ b/android/depset_test.go @@ -0,0 +1,304 @@ +// 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 ( + "fmt" + "reflect" + "strings" + "testing" +) + +func ExampleDepSet_ToList_postordered() { + a := NewDepSetBuilder(POSTORDER).Direct(PathForTesting("a")).Build() + b := NewDepSetBuilder(POSTORDER).Direct(PathForTesting("b")).Transitive(a).Build() + c := NewDepSetBuilder(POSTORDER).Direct(PathForTesting("c")).Transitive(a).Build() + d := NewDepSetBuilder(POSTORDER).Direct(PathForTesting("d")).Transitive(b, c).Build() + + fmt.Println(d.ToList().Strings()) + // Output: [a b c d] +} + +func ExampleDepSet_ToList_preordered() { + a := NewDepSetBuilder(PREORDER).Direct(PathForTesting("a")).Build() + b := NewDepSetBuilder(PREORDER).Direct(PathForTesting("b")).Transitive(a).Build() + c := NewDepSetBuilder(PREORDER).Direct(PathForTesting("c")).Transitive(a).Build() + d := NewDepSetBuilder(PREORDER).Direct(PathForTesting("d")).Transitive(b, c).Build() + + fmt.Println(d.ToList().Strings()) + // Output: [d b a c] +} + +func ExampleDepSet_ToList_topological() { + a := NewDepSetBuilder(TOPOLOGICAL).Direct(PathForTesting("a")).Build() + b := NewDepSetBuilder(TOPOLOGICAL).Direct(PathForTesting("b")).Transitive(a).Build() + c := NewDepSetBuilder(TOPOLOGICAL).Direct(PathForTesting("c")).Transitive(a).Build() + d := NewDepSetBuilder(TOPOLOGICAL).Direct(PathForTesting("d")).Transitive(b, c).Build() + + fmt.Println(d.ToList().Strings()) + // Output: [d b c a] +} + +func ExampleDepSet_ToSortedList() { + a := NewDepSetBuilder(POSTORDER).Direct(PathForTesting("a")).Build() + b := NewDepSetBuilder(POSTORDER).Direct(PathForTesting("b")).Transitive(a).Build() + c := NewDepSetBuilder(POSTORDER).Direct(PathForTesting("c")).Transitive(a).Build() + d := NewDepSetBuilder(POSTORDER).Direct(PathForTesting("d")).Transitive(b, c).Build() + + fmt.Println(d.ToSortedList().Strings()) + // Output: [a b c d] +} + +// Tests based on Bazel's ExpanderTestBase.java to ensure compatibility +// https://github.com/bazelbuild/bazel/blob/master/src/test/java/com/google/devtools/build/lib/collect/nestedset/ExpanderTestBase.java +func TestDepSet(t *testing.T) { + a := PathForTesting("a") + b := PathForTesting("b") + c := PathForTesting("c") + c2 := PathForTesting("c2") + d := PathForTesting("d") + e := PathForTesting("e") + + tests := []struct { + name string + depSet func(t *testing.T, order DepSetOrder) *DepSet + postorder, preorder, topological []string + }{ + { + name: "simple", + depSet: func(t *testing.T, order DepSetOrder) *DepSet { + return NewDepSet(order, Paths{c, a, b}, nil) + }, + postorder: []string{"c", "a", "b"}, + preorder: []string{"c", "a", "b"}, + topological: []string{"c", "a", "b"}, + }, + { + name: "simpleNoDuplicates", + depSet: func(t *testing.T, order DepSetOrder) *DepSet { + return NewDepSet(order, Paths{c, a, a, a, b}, nil) + }, + postorder: []string{"c", "a", "b"}, + preorder: []string{"c", "a", "b"}, + topological: []string{"c", "a", "b"}, + }, + { + name: "nesting", + depSet: func(t *testing.T, order DepSetOrder) *DepSet { + subset := NewDepSet(order, Paths{c, a, e}, nil) + return NewDepSet(order, Paths{b, d}, []*DepSet{subset}) + }, + postorder: []string{"c", "a", "e", "b", "d"}, + preorder: []string{"b", "d", "c", "a", "e"}, + topological: []string{"b", "d", "c", "a", "e"}, + }, + { + name: "builderReuse", + depSet: func(t *testing.T, order DepSetOrder) *DepSet { + assertEquals := func(t *testing.T, w, g Paths) { + if !reflect.DeepEqual(w, g) { + t.Errorf("want %q, got %q", w, g) + } + } + builder := NewDepSetBuilder(order) + assertEquals(t, nil, builder.Build().ToList()) + + builder.Direct(b) + assertEquals(t, Paths{b}, builder.Build().ToList()) + + builder.Direct(d) + assertEquals(t, Paths{b, d}, builder.Build().ToList()) + + child := NewDepSetBuilder(order).Direct(c, a, e).Build() + builder.Transitive(child) + return builder.Build() + }, + postorder: []string{"c", "a", "e", "b", "d"}, + preorder: []string{"b", "d", "c", "a", "e"}, + topological: []string{"b", "d", "c", "a", "e"}, + }, + { + name: "builderChaining", + depSet: func(t *testing.T, order DepSetOrder) *DepSet { + return NewDepSetBuilder(order).Direct(b).Direct(d). + Transitive(NewDepSetBuilder(order).Direct(c, a, e).Build()).Build() + }, + postorder: []string{"c", "a", "e", "b", "d"}, + preorder: []string{"b", "d", "c", "a", "e"}, + topological: []string{"b", "d", "c", "a", "e"}, + }, + { + name: "transitiveDepsHandledSeparately", + depSet: func(t *testing.T, order DepSetOrder) *DepSet { + subset := NewDepSetBuilder(order).Direct(c, a, e).Build() + builder := NewDepSetBuilder(order) + // The fact that we add the transitive subset between the Direct(b) and Direct(d) + // calls should not change the result. + builder.Direct(b) + builder.Transitive(subset) + builder.Direct(d) + return builder.Build() + }, + postorder: []string{"c", "a", "e", "b", "d"}, + preorder: []string{"b", "d", "c", "a", "e"}, + topological: []string{"b", "d", "c", "a", "e"}, + }, + { + name: "nestingNoDuplicates", + depSet: func(t *testing.T, order DepSetOrder) *DepSet { + subset := NewDepSetBuilder(order).Direct(c, a, e).Build() + return NewDepSetBuilder(order).Direct(b, d, e).Transitive(subset).Build() + }, + postorder: []string{"c", "a", "e", "b", "d"}, + preorder: []string{"b", "d", "e", "c", "a"}, + topological: []string{"b", "d", "c", "a", "e"}, + }, + { + name: "chain", + depSet: func(t *testing.T, order DepSetOrder) *DepSet { + c := NewDepSetBuilder(order).Direct(c).Build() + b := NewDepSetBuilder(order).Direct(b).Transitive(c).Build() + a := NewDepSetBuilder(order).Direct(a).Transitive(b).Build() + + return a + }, + postorder: []string{"c", "b", "a"}, + preorder: []string{"a", "b", "c"}, + topological: []string{"a", "b", "c"}, + }, + { + name: "diamond", + depSet: func(t *testing.T, order DepSetOrder) *DepSet { + d := NewDepSetBuilder(order).Direct(d).Build() + c := NewDepSetBuilder(order).Direct(c).Transitive(d).Build() + b := NewDepSetBuilder(order).Direct(b).Transitive(d).Build() + a := NewDepSetBuilder(order).Direct(a).Transitive(b).Transitive(c).Build() + + return a + }, + postorder: []string{"d", "b", "c", "a"}, + preorder: []string{"a", "b", "d", "c"}, + topological: []string{"a", "b", "c", "d"}, + }, + { + name: "extendedDiamond", + depSet: func(t *testing.T, order DepSetOrder) *DepSet { + d := NewDepSetBuilder(order).Direct(d).Build() + e := NewDepSetBuilder(order).Direct(e).Build() + b := NewDepSetBuilder(order).Direct(b).Transitive(d).Transitive(e).Build() + c := NewDepSetBuilder(order).Direct(c).Transitive(e).Transitive(d).Build() + a := NewDepSetBuilder(order).Direct(a).Transitive(b).Transitive(c).Build() + return a + }, + postorder: []string{"d", "e", "b", "c", "a"}, + preorder: []string{"a", "b", "d", "e", "c"}, + topological: []string{"a", "b", "c", "e", "d"}, + }, + { + name: "extendedDiamondRightArm", + depSet: func(t *testing.T, order DepSetOrder) *DepSet { + d := NewDepSetBuilder(order).Direct(d).Build() + e := NewDepSetBuilder(order).Direct(e).Build() + b := NewDepSetBuilder(order).Direct(b).Transitive(d).Transitive(e).Build() + c2 := NewDepSetBuilder(order).Direct(c2).Transitive(e).Transitive(d).Build() + c := NewDepSetBuilder(order).Direct(c).Transitive(c2).Build() + a := NewDepSetBuilder(order).Direct(a).Transitive(b).Transitive(c).Build() + return a + }, + postorder: []string{"d", "e", "b", "c2", "c", "a"}, + preorder: []string{"a", "b", "d", "e", "c", "c2"}, + topological: []string{"a", "b", "c", "c2", "e", "d"}, + }, + { + name: "orderConflict", + depSet: func(t *testing.T, order DepSetOrder) *DepSet { + child1 := NewDepSetBuilder(order).Direct(a, b).Build() + child2 := NewDepSetBuilder(order).Direct(b, a).Build() + parent := NewDepSetBuilder(order).Transitive(child1).Transitive(child2).Build() + return parent + }, + postorder: []string{"a", "b"}, + preorder: []string{"a", "b"}, + topological: []string{"b", "a"}, + }, + { + name: "orderConflictNested", + depSet: func(t *testing.T, order DepSetOrder) *DepSet { + a := NewDepSetBuilder(order).Direct(a).Build() + b := NewDepSetBuilder(order).Direct(b).Build() + child1 := NewDepSetBuilder(order).Transitive(a).Transitive(b).Build() + child2 := NewDepSetBuilder(order).Transitive(b).Transitive(a).Build() + parent := NewDepSetBuilder(order).Transitive(child1).Transitive(child2).Build() + return parent + }, + postorder: []string{"a", "b"}, + preorder: []string{"a", "b"}, + topological: []string{"b", "a"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Run("postorder", func(t *testing.T) { + depSet := tt.depSet(t, POSTORDER) + if g, w := depSet.ToList().Strings(), tt.postorder; !reflect.DeepEqual(g, w) { + t.Errorf("expected ToList() = %q, got %q", w, g) + } + }) + t.Run("preorder", func(t *testing.T) { + depSet := tt.depSet(t, PREORDER) + if g, w := depSet.ToList().Strings(), tt.preorder; !reflect.DeepEqual(g, w) { + t.Errorf("expected ToList() = %q, got %q", w, g) + } + }) + t.Run("topological", func(t *testing.T) { + depSet := tt.depSet(t, TOPOLOGICAL) + if g, w := depSet.ToList().Strings(), tt.topological; !reflect.DeepEqual(g, w) { + t.Errorf("expected ToList() = %q, got %q", w, g) + } + }) + }) + } +} + +func TestDepSetInvalidOrder(t *testing.T) { + orders := []DepSetOrder{POSTORDER, PREORDER, TOPOLOGICAL} + + run := func(t *testing.T, order1, order2 DepSetOrder) { + defer func() { + if r := recover(); r != nil { + if err, ok := r.(error); !ok { + t.Fatalf("expected panic error, got %v", err) + } else if !strings.Contains(err.Error(), "incompatible order") { + t.Fatalf("expected incompatible order error, got %v", err) + } + } + }() + NewDepSet(order1, nil, []*DepSet{NewDepSet(order2, nil, nil)}) + t.Fatal("expected panic") + } + + for _, order1 := range orders { + t.Run(order1.String(), func(t *testing.T) { + for _, order2 := range orders { + t.Run(order2.String(), func(t *testing.T) { + if order1 != order2 { + run(t, order1, order2) + } + }) + } + }) + } +} -- cgit v1.2.3 From 1d11c871ae38a23212ced24ff23b8a5117433fe7 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Fri, 3 Jul 2020 11:56:24 -0700 Subject: Build a zip of transitive lint reports for apps Add a rule to build a zip containing the lint reports from transitive dependencies for apps, and pass it to Make. Bug: 153485543 Test: m TARGET_BUILD_APPS=Gallery2 lint-check Change-Id: I523c09016251377ff89d76084769be7401b95425 Merged-In: I523c09016251377ff89d76084769be7401b95425 (cherry picked from commit c0efd1db13e383a0b3ac96e3718a006dea0e1cfd) --- android/androidmk.go | 19 +++++++++ android/paths.go | 4 ++ java/androidmk.go | 8 ++++ java/app.go | 1 + java/lint.go | 107 ++++++++++++++++++++++++++++++++++++++------------- 5 files changed, 112 insertions(+), 27 deletions(-) diff --git a/android/androidmk.go b/android/androidmk.go index 6ba68af0..f86061af 100644 --- a/android/androidmk.go +++ b/android/androidmk.go @@ -107,6 +107,25 @@ func (a *AndroidMkEntries) SetPath(name string, path Path) { a.EntryMap[name] = []string{path.String()} } +func (a *AndroidMkEntries) SetOptionalPath(name string, path OptionalPath) { + if path.Valid() { + a.SetPath(name, path.Path()) + } +} + +func (a *AndroidMkEntries) AddPath(name string, path Path) { + if _, ok := a.EntryMap[name]; !ok { + a.entryOrder = append(a.entryOrder, name) + } + a.EntryMap[name] = append(a.EntryMap[name], path.String()) +} + +func (a *AndroidMkEntries) AddOptionalPath(name string, path OptionalPath) { + if path.Valid() { + a.AddPath(name, path.Path()) + } +} + func (a *AndroidMkEntries) SetBoolIfTrue(name string, flag bool) { if flag { if _, ok := a.EntryMap[name]; !ok { diff --git a/android/paths.go b/android/paths.go index 2a0a70a9..ddbeed3c 100644 --- a/android/paths.go +++ b/android/paths.go @@ -466,6 +466,10 @@ func (p Paths) Strings() []string { return ret } +func CopyOfPaths(paths Paths) Paths { + return append(Paths(nil), paths...) +} + // FirstUniquePaths returns all unique elements of a Paths, keeping the first copy of each. It // modifies the Paths slice contents in place, and returns a subslice of the original slice. func FirstUniquePaths(list Paths) Paths { diff --git a/java/androidmk.go b/java/androidmk.go index 8953c31c..48b9f868 100644 --- a/java/androidmk.go +++ b/java/androidmk.go @@ -131,6 +131,10 @@ func (library *Library) AndroidMkEntries() []android.AndroidMkEntries { entries.SetPath("LOCAL_SOONG_PROGUARD_DICT", library.proguardDictionary) } entries.SetString("LOCAL_MODULE_STEM", library.Stem()) + + entries.AddOptionalPath("LOCAL_SOONG_LINT_REPORTS", library.linter.outputs.transitiveHTMLZip) + entries.AddOptionalPath("LOCAL_SOONG_LINT_REPORTS", library.linter.outputs.transitiveTextZip) + entries.AddOptionalPath("LOCAL_SOONG_LINT_REPORTS", library.linter.outputs.transitiveXMLZip) }, }, } @@ -383,6 +387,10 @@ func (app *AndroidApp) AndroidMkEntries() []android.AndroidMkEntries { install := app.onDeviceDir + "/" + extra.Base() entries.AddStrings("LOCAL_SOONG_BUILT_INSTALLED", extra.String()+":"+install) } + + entries.AddOptionalPath("LOCAL_SOONG_LINT_REPORTS", app.linter.outputs.transitiveHTMLZip) + entries.AddOptionalPath("LOCAL_SOONG_LINT_REPORTS", app.linter.outputs.transitiveTextZip) + entries.AddOptionalPath("LOCAL_SOONG_LINT_REPORTS", app.linter.outputs.transitiveXMLZip) }, }, ExtraFooters: []android.AndroidMkExtraFootersFunc{ diff --git a/java/app.go b/java/app.go index 5af89b9c..fe703843 100755 --- a/java/app.go +++ b/java/app.go @@ -749,6 +749,7 @@ func (a *AndroidApp) generateAndroidBuildActions(ctx android.ModuleContext) { a.linter.mergedManifest = a.aapt.mergedManifestFile a.linter.manifest = a.aapt.manifestPath a.linter.resources = a.aapt.resourceFiles + a.linter.buildModuleReportZip = ctx.Config().UnbundledBuild() dexJarFile := a.dexBuildActions(ctx) diff --git a/java/lint.go b/java/lint.go index 6fbef18c..20a7dc49 100644 --- a/java/lint.go +++ b/java/lint.go @@ -67,12 +67,32 @@ type linter struct { kotlinLanguageLevel string outputs lintOutputs properties LintProperties + + buildModuleReportZip bool } type lintOutputs struct { html android.ModuleOutPath text android.ModuleOutPath xml android.ModuleOutPath + + transitiveHTML *android.DepSet + transitiveText *android.DepSet + transitiveXML *android.DepSet + + transitiveHTMLZip android.OptionalPath + transitiveTextZip android.OptionalPath + transitiveXMLZip android.OptionalPath +} + +type lintOutputIntf interface { + lintOutputs() *lintOutputs +} + +var _ lintOutputIntf = (*linter)(nil) + +func (l *linter) lintOutputs() *lintOutputs { + return &l.outputs } func (l *linter) enabled() bool { @@ -213,9 +233,22 @@ func (l *linter) lint(ctx android.ModuleContext) { projectXML, lintXML, cacheDir, homeDir, deps := l.writeLintProjectXML(ctx, rule) - l.outputs.html = android.PathForModuleOut(ctx, "lint-report.html") - l.outputs.text = android.PathForModuleOut(ctx, "lint-report.txt") - l.outputs.xml = android.PathForModuleOut(ctx, "lint-report.xml") + html := android.PathForModuleOut(ctx, "lint-report.html") + text := android.PathForModuleOut(ctx, "lint-report.txt") + xml := android.PathForModuleOut(ctx, "lint-report.xml") + + htmlDeps := android.NewDepSetBuilder(android.POSTORDER).Direct(html) + textDeps := android.NewDepSetBuilder(android.POSTORDER).Direct(text) + xmlDeps := android.NewDepSetBuilder(android.POSTORDER).Direct(xml) + + ctx.VisitDirectDepsWithTag(staticLibTag, func(dep android.Module) { + if depLint, ok := dep.(lintOutputIntf); ok { + depLintOutputs := depLint.lintOutputs() + htmlDeps.Transitive(depLintOutputs.transitiveHTML) + textDeps.Transitive(depLintOutputs.transitiveText) + xmlDeps.Transitive(depLintOutputs.transitiveXML) + } + }) rule.Command().Text("rm -rf").Flag(cacheDir.String()).Flag(homeDir.String()) rule.Command().Text("mkdir -p").Flag(cacheDir.String()).Flag(homeDir.String()) @@ -240,9 +273,9 @@ func (l *linter) lint(ctx android.ModuleContext) { Flag("--quiet"). FlagWithInput("--project ", projectXML). FlagWithInput("--config ", lintXML). - FlagWithOutput("--html ", l.outputs.html). - FlagWithOutput("--text ", l.outputs.text). - FlagWithOutput("--xml ", l.outputs.xml). + FlagWithOutput("--html ", html). + FlagWithOutput("--text ", text). + FlagWithOutput("--xml ", xml). FlagWithArg("--compile-sdk-version ", l.compileSdkVersion). FlagWithArg("--java-language-level ", l.javaLanguageLevel). FlagWithArg("--kotlin-language-level ", l.kotlinLanguageLevel). @@ -250,23 +283,37 @@ func (l *linter) lint(ctx android.ModuleContext) { Flag("--exitcode"). Flags(l.properties.Lint.Flags). Implicits(deps). - Text("|| (").Text("cat").Input(l.outputs.text).Text("; exit 7)"). + Text("|| (").Text("cat").Input(text).Text("; exit 7)"). Text(")") rule.Command().Text("rm -rf").Flag(cacheDir.String()).Flag(homeDir.String()) rule.Build(pctx, ctx, "lint", "lint") -} -func (l *linter) lintOutputs() *lintOutputs { - return &l.outputs -} + l.outputs = lintOutputs{ + html: html, + text: text, + xml: xml, -type lintOutputIntf interface { - lintOutputs() *lintOutputs -} + transitiveHTML: htmlDeps.Build(), + transitiveText: textDeps.Build(), + transitiveXML: xmlDeps.Build(), + } -var _ lintOutputIntf = (*linter)(nil) + if l.buildModuleReportZip { + htmlZip := android.PathForModuleOut(ctx, "lint-report-html.zip") + l.outputs.transitiveHTMLZip = android.OptionalPathForPath(htmlZip) + lintZip(ctx, l.outputs.transitiveHTML.ToSortedList(), htmlZip) + + textZip := android.PathForModuleOut(ctx, "lint-report-text.zip") + l.outputs.transitiveTextZip = android.OptionalPathForPath(textZip) + lintZip(ctx, l.outputs.transitiveText.ToSortedList(), textZip) + + xmlZip := android.PathForModuleOut(ctx, "lint-report-xml.zip") + l.outputs.transitiveXMLZip = android.OptionalPathForPath(xmlZip) + lintZip(ctx, l.outputs.transitiveXML.ToSortedList(), xmlZip) + } +} type lintSingleton struct { htmlZip android.WritablePath @@ -356,18 +403,7 @@ func (l *lintSingleton) generateLintReportZips(ctx android.SingletonContext) { paths = append(paths, get(output)) } - sort.Slice(paths, func(i, j int) bool { - return paths[i].String() < paths[j].String() - }) - - rule := android.NewRuleBuilder() - - rule.Command().BuiltTool(ctx, "soong_zip"). - FlagWithOutput("-o ", outputPath). - FlagWithArg("-C ", android.PathForIntermediates(ctx).String()). - FlagWithRspFileInputList("-l ", paths) - - rule.Build(pctx, ctx, outputPath.Base(), outputPath.Base()) + lintZip(ctx, paths, outputPath) } l.htmlZip = android.PathForOutput(ctx, "lint-report-html.zip") @@ -394,3 +430,20 @@ func init() { android.RegisterSingletonType("lint", func() android.Singleton { return &lintSingleton{} }) } + +func lintZip(ctx android.BuilderContext, paths android.Paths, outputPath android.WritablePath) { + paths = android.SortedUniquePaths(android.CopyOfPaths(paths)) + + sort.Slice(paths, func(i, j int) bool { + return paths[i].String() < paths[j].String() + }) + + rule := android.NewRuleBuilder() + + rule.Command().BuiltTool(ctx, "soong_zip"). + FlagWithOutput("-o ", outputPath). + FlagWithArg("-C ", android.PathForIntermediates(ctx).String()). + FlagWithRspFileInputList("-l ", paths) + + rule.Build(pctx, ctx, outputPath.Base(), outputPath.Base()) +} -- cgit v1.2.3 From b32b71222e2ae958f8c06e415c434e4b067a8527 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Mon, 6 Jul 2020 14:15:24 -0700 Subject: Pass unstripped JNI libraries to Make Pass a list of unstripped JNI libraries to Make so that they can be installed into the symbols directory. Bug: 159726429 Test: forrest Change-Id: Ieb4bffbb3d0a09f476da011399c5b8b1611929d7 Merged-In: Ieb4bffbb3d0a09f476da011399c5b8b1611929d7 (cherry picked from commit 403cc15f1b01a43902a77f9d136d96303ebbe7af) --- apex/androidmk.go | 15 +++++++++++---- java/androidmk.go | 10 ++++++++-- java/app.go | 43 ++++++++++++++++++++++++++++++++----------- java/java.go | 9 +++++---- 4 files changed, 56 insertions(+), 21 deletions(-) diff --git a/apex/androidmk.go b/apex/androidmk.go index e299588a..5879e56a 100644 --- a/apex/androidmk.go +++ b/apex/androidmk.go @@ -113,10 +113,11 @@ func (a *apexBundle) androidMkForFiles(w io.Writer, apexBundleName, apexName, mo } // /apex//{lib|framework|...} pathWhenActivated := filepath.Join("$(PRODUCT_OUT)", "apex", apexName, fi.installDir) + var modulePath string if apexType == flattenedApex { // /system/apex//{lib|framework|...} - fmt.Fprintln(w, "LOCAL_MODULE_PATH :=", filepath.Join(a.installDir.ToMakePath().String(), - apexBundleName, fi.installDir)) + modulePath = filepath.Join(a.installDir.ToMakePath().String(), apexBundleName, fi.installDir) + fmt.Fprintln(w, "LOCAL_MODULE_PATH :=", modulePath) if a.primaryApexType && !symbolFilesNotNeeded { fmt.Fprintln(w, "LOCAL_SOONG_SYMBOL_PATH :=", pathWhenActivated) } @@ -128,6 +129,7 @@ func (a *apexBundle) androidMkForFiles(w io.Writer, apexBundleName, apexName, mo fmt.Fprintln(w, "LOCAL_NOTICE_FILE :=", fi.module.NoticeFile().Path().String()) } } else { + modulePath = pathWhenActivated fmt.Fprintln(w, "LOCAL_MODULE_PATH :=", pathWhenActivated) // For non-flattend APEXes, the merged notice file is attached to the APEX itself. @@ -190,8 +192,13 @@ func (a *apexBundle) androidMkForFiles(w io.Writer, apexBundleName, apexName, mo // we need to remove the suffix from LOCAL_MODULE_STEM, otherwise // we will have foo.apk.apk fmt.Fprintln(w, "LOCAL_MODULE_STEM :=", strings.TrimSuffix(fi.Stem(), ".apk")) - if app, ok := fi.module.(*java.AndroidApp); ok && len(app.JniCoverageOutputs()) > 0 { - fmt.Fprintln(w, "LOCAL_PREBUILT_COVERAGE_ARCHIVE :=", strings.Join(app.JniCoverageOutputs().Strings(), " ")) + if app, ok := fi.module.(*java.AndroidApp); ok { + if jniCoverageOutputs := app.JniCoverageOutputs(); len(jniCoverageOutputs) > 0 { + fmt.Fprintln(w, "LOCAL_PREBUILT_COVERAGE_ARCHIVE :=", strings.Join(jniCoverageOutputs.Strings(), " ")) + } + if jniLibSymbols := app.JNISymbolsInstalls(modulePath); len(jniLibSymbols) > 0 { + fmt.Fprintln(w, "LOCAL_SOONG_JNI_LIBS_SYMBOLS :=", jniLibSymbols.String()) + } } fmt.Fprintln(w, "include $(BUILD_SYSTEM)/soong_app_prebuilt.mk") case appSet: diff --git a/java/androidmk.go b/java/androidmk.go index 8953c31c..c69b747a 100644 --- a/java/androidmk.go +++ b/java/androidmk.go @@ -370,9 +370,15 @@ func (app *AndroidApp) AndroidMkEntries() []android.AndroidMkEntries { entries.SetString("LOCAL_CERTIFICATE", app.certificate.AndroidMkString()) entries.AddStrings("LOCAL_OVERRIDES_PACKAGES", app.getOverriddenPackages()...) - for _, jniLib := range app.installJniLibs { - entries.AddStrings("LOCAL_SOONG_JNI_LIBS_"+jniLib.target.Arch.ArchType.String(), jniLib.name) + if app.embeddedJniLibs { + jniSymbols := app.JNISymbolsInstalls(app.installPathForJNISymbols.String()) + entries.SetString("LOCAL_SOONG_JNI_LIBS_SYMBOLS", jniSymbols.String()) + } else { + for _, jniLib := range app.jniLibs { + entries.AddStrings("LOCAL_SOONG_JNI_LIBS_"+jniLib.target.Arch.ArchType.String(), jniLib.name) + } } + if len(app.jniCoverageOutputs) > 0 { entries.AddStrings("LOCAL_PREBUILT_COVERAGE_ARCHIVE", app.jniCoverageOutputs.Strings()...) } diff --git a/java/app.go b/java/app.go index 5af89b9c..5f5a105f 100755 --- a/java/app.go +++ b/java/app.go @@ -291,8 +291,10 @@ type AndroidApp struct { overridableAppProperties overridableAppProperties - installJniLibs []jniLib - jniCoverageOutputs android.Paths + jniLibs []jniLib + installPathForJNISymbols android.Path + embeddedJniLibs bool + jniCoverageOutputs android.Paths bundleFile android.Path @@ -567,8 +569,7 @@ func (a *AndroidApp) proguardBuildActions(ctx android.ModuleContext) { a.Module.extraProguardFlagFiles = append(a.Module.extraProguardFlagFiles, a.proguardOptionsFile) } -func (a *AndroidApp) dexBuildActions(ctx android.ModuleContext) android.Path { - +func (a *AndroidApp) installPath(ctx android.ModuleContext) android.InstallPath { var installDir string if ctx.ModuleName() == "framework-res" { // framework-res.apk is installed as system/framework/framework-res.apk @@ -578,7 +579,12 @@ func (a *AndroidApp) dexBuildActions(ctx android.ModuleContext) android.Path { } else { installDir = filepath.Join("app", a.installApkName) } - a.dexpreopter.installPath = android.PathForModuleInstall(ctx, installDir, a.installApkName+".apk") + + return android.PathForModuleInstall(ctx, installDir, a.installApkName+".apk") +} + +func (a *AndroidApp) dexBuildActions(ctx android.ModuleContext) android.Path { + a.dexpreopter.installPath = a.installPath(ctx) if a.deviceProperties.Uncompress_dex == nil { // If the value was not force-set by the user, use reasonable default based on the module. a.deviceProperties.Uncompress_dex = proptools.BoolPtr(a.shouldUncompressDex(ctx)) @@ -600,8 +606,10 @@ func (a *AndroidApp) dexBuildActions(ctx android.ModuleContext) android.Path { func (a *AndroidApp) jniBuildActions(jniLibs []jniLib, ctx android.ModuleContext) android.WritablePath { var jniJarFile android.WritablePath if len(jniLibs) > 0 { + a.jniLibs = jniLibs if a.shouldEmbedJnis(ctx) { jniJarFile = android.PathForModuleOut(ctx, "jnilibs.zip") + a.installPathForJNISymbols = a.installPath(ctx).ToMakePath() TransformJniLibsToJar(ctx, jniJarFile, jniLibs, a.useEmbeddedNativeLibs(ctx)) for _, jni := range jniLibs { if jni.coverageFile.Valid() { @@ -619,13 +627,25 @@ func (a *AndroidApp) jniBuildActions(jniLibs []jniLib, ctx android.ModuleContext } } } - } else { - a.installJniLibs = jniLibs + a.embeddedJniLibs = true } } return jniJarFile } +func (a *AndroidApp) JNISymbolsInstalls(installPath string) android.RuleBuilderInstalls { + var jniSymbols android.RuleBuilderInstalls + for _, jniLib := range a.jniLibs { + if jniLib.unstrippedFile != nil { + jniSymbols = append(jniSymbols, android.RuleBuilderInstall{ + From: jniLib.unstrippedFile, + To: filepath.Join(installPath, targetToJniDir(jniLib.target), jniLib.unstrippedFile.Base()), + }) + } + } + return jniSymbols +} + func (a *AndroidApp) noticeBuildActions(ctx android.ModuleContext) { // Collect NOTICE files from all dependencies. seenModules := make(map[android.Module]bool) @@ -851,10 +871,11 @@ func collectAppDeps(ctx android.ModuleContext, app appDepsInterface, if lib.Valid() { jniLibs = append(jniLibs, jniLib{ - name: ctx.OtherModuleName(module), - path: path, - target: module.Target(), - coverageFile: dep.CoverageOutputFile(), + name: ctx.OtherModuleName(module), + path: path, + target: module.Target(), + coverageFile: dep.CoverageOutputFile(), + unstrippedFile: dep.UnstrippedOutputFile(), }) } else { ctx.ModuleErrorf("dependency %q missing output file", otherName) diff --git a/java/java.go b/java/java.go index 72c91a5e..0a764e63 100644 --- a/java/java.go +++ b/java/java.go @@ -626,10 +626,11 @@ func (s sdkDep) hasFrameworkLibs() bool { } type jniLib struct { - name string - path android.Path - target android.Target - coverageFile android.OptionalPath + name string + path android.Path + target android.Target + coverageFile android.OptionalPath + unstrippedFile android.Path } func (j *Module) shouldInstrument(ctx android.BaseModuleContext) bool { -- cgit v1.2.3