Merge "Allow using include_top to filter results even when there is a constant prefix"
diff --git a/android/bazel_handler.go b/android/bazel_handler.go
index 8ab003c..8834c11 100644
--- a/android/bazel_handler.go
+++ b/android/bazel_handler.go
@@ -111,7 +111,7 @@
// Issues commands to Bazel to receive results for all cquery requests
// queued in the BazelContext.
- InvokeBazel() error
+ InvokeBazel(config Config) error
// Returns true if bazel is enabled for the given configuration.
BazelEnabled() bool
@@ -191,7 +191,7 @@
return result, nil
}
-func (m MockBazelContext) InvokeBazel() error {
+func (m MockBazelContext) InvokeBazel(config Config) error {
panic("unimplemented")
}
@@ -261,7 +261,7 @@
panic("unimplemented")
}
-func (n noopBazelContext) InvokeBazel() error {
+func (n noopBazelContext) InvokeBazel(config Config) error {
panic("unimplemented")
}
@@ -361,6 +361,7 @@
type mockBazelRunner struct {
bazelCommandResults map[bazelCommand]string
commands []bazelCommand
+ extraFlags []string
}
func (r *mockBazelRunner) issueBazelCommand(paths *bazelPaths,
@@ -368,6 +369,7 @@
command bazelCommand,
extraFlags ...string) (string, string, error) {
r.commands = append(r.commands, command)
+ r.extraFlags = append(r.extraFlags, strings.Join(extraFlags, " "))
if ret, ok := r.bazelCommandResults[command]; ok {
return ret, "", nil
}
@@ -676,7 +678,7 @@
// Issues commands to Bazel to receive results for all cquery requests
// queued in the BazelContext.
-func (context *bazelContext) InvokeBazel() error {
+func (context *bazelContext) InvokeBazel(config Config) error {
context.results = make(map[cqueryKey]string)
var cqueryOutput string
@@ -759,15 +761,31 @@
// Issue an aquery command to retrieve action information about the bazel build tree.
//
- // TODO(cparsons): Use --target_pattern_file to avoid command line limits.
var aqueryOutput string
+ var coverageFlags []string
+ if Bool(config.productVariables.ClangCoverage) {
+ coverageFlags = append(coverageFlags, "--collect_code_coverage")
+ if len(config.productVariables.NativeCoveragePaths) > 0 ||
+ len(config.productVariables.NativeCoverageExcludePaths) > 0 {
+ includePaths := JoinWithPrefixAndSeparator(config.productVariables.NativeCoveragePaths, "+", ",")
+ excludePaths := JoinWithPrefixAndSeparator(config.productVariables.NativeCoverageExcludePaths, "-", ",")
+ if len(includePaths) > 0 && len(excludePaths) > 0 {
+ includePaths += ","
+ }
+ coverageFlags = append(coverageFlags, fmt.Sprintf(`--instrumentation_filter=%s`,
+ includePaths+excludePaths))
+ }
+ }
+
+ extraFlags := append([]string{"--output=jsonproto"}, coverageFlags...)
+
aqueryOutput, _, err = context.issueBazelCommand(
context.paths,
bazel.AqueryBuildRootRunName,
bazelCommand{"aquery", fmt.Sprintf("deps(%s)", buildrootLabel)},
// Use jsonproto instead of proto; actual proto parsing would require a dependency on Bazel's
// proto sources, which would add a number of unnecessary dependencies.
- "--output=jsonproto")
+ extraFlags...)
if err != nil {
return err
diff --git a/android/bazel_handler_test.go b/android/bazel_handler_test.go
index cfdccd7..dd9a7ed 100644
--- a/android/bazel_handler_test.go
+++ b/android/bazel_handler_test.go
@@ -4,11 +4,14 @@
"os"
"path/filepath"
"reflect"
+ "strings"
"testing"
"android/soong/bazel/cquery"
)
+var testConfig = TestConfig("out", nil, "", nil)
+
func TestRequestResultsAfterInvokeBazel(t *testing.T) {
label := "//foo:bar"
cfg := configKey{"arm64_armv8-a", Android}
@@ -16,7 +19,7 @@
bazelCommand{command: "cquery", expression: "deps(@soong_injection//mixed_builds:buildroot, 2)"}: `//foo:bar|arm64_armv8-a|android>>out/foo/bar.txt`,
})
bazelContext.QueueBazelRequest(label, cquery.GetOutputFiles, cfg)
- err := bazelContext.InvokeBazel()
+ err := bazelContext.InvokeBazel(testConfig)
if err != nil {
t.Fatalf("Did not expect error invoking Bazel, but got %s", err)
}
@@ -30,7 +33,7 @@
func TestInvokeBazelWritesBazelFiles(t *testing.T) {
bazelContext, baseDir := testBazelContext(t, map[bazelCommand]string{})
- err := bazelContext.InvokeBazel()
+ err := bazelContext.InvokeBazel(testConfig)
if err != nil {
t.Fatalf("Did not expect error invoking Bazel, but got %s", err)
}
@@ -86,7 +89,7 @@
}]
}`,
})
- err := bazelContext.InvokeBazel()
+ err := bazelContext.InvokeBazel(testConfig)
if err != nil {
t.Fatalf("Did not expect error invoking Bazel, but got %s", err)
}
@@ -97,6 +100,54 @@
}
}
+func TestCoverageFlagsAfterInvokeBazel(t *testing.T) {
+ testConfig.productVariables.ClangCoverage = boolPtr(true)
+
+ testConfig.productVariables.NativeCoveragePaths = []string{"foo1", "foo2"}
+ testConfig.productVariables.NativeCoverageExcludePaths = []string{"bar1", "bar2"}
+ verifyExtraFlags(t, testConfig, `--collect_code_coverage --instrumentation_filter=+foo1,+foo2,-bar1,-bar2`)
+
+ testConfig.productVariables.NativeCoveragePaths = []string{"foo1"}
+ testConfig.productVariables.NativeCoverageExcludePaths = []string{"bar1"}
+ verifyExtraFlags(t, testConfig, `--collect_code_coverage --instrumentation_filter=+foo1,-bar1`)
+
+ testConfig.productVariables.NativeCoveragePaths = []string{"foo1"}
+ testConfig.productVariables.NativeCoverageExcludePaths = nil
+ verifyExtraFlags(t, testConfig, `--collect_code_coverage --instrumentation_filter=+foo1`)
+
+ testConfig.productVariables.NativeCoveragePaths = nil
+ testConfig.productVariables.NativeCoverageExcludePaths = []string{"bar1"}
+ verifyExtraFlags(t, testConfig, `--collect_code_coverage --instrumentation_filter=-bar1`)
+
+ testConfig.productVariables.ClangCoverage = boolPtr(false)
+ actual := verifyExtraFlags(t, testConfig, ``)
+ if strings.Contains(actual, "--collect_code_coverage") ||
+ strings.Contains(actual, "--instrumentation_filter=") {
+ t.Errorf("Expected code coverage disabled, but got %#v", actual)
+ }
+}
+
+func verifyExtraFlags(t *testing.T, config Config, expected string) string {
+ bazelContext, _ := testBazelContext(t, map[bazelCommand]string{})
+
+ err := bazelContext.InvokeBazel(config)
+ if err != nil {
+ t.Fatalf("Did not expect error invoking Bazel, but got %s", err)
+ }
+
+ flags := bazelContext.bazelRunner.(*mockBazelRunner).extraFlags
+ if expected := 3; len(flags) != expected {
+ t.Errorf("Expected %d extra flags got %#v", expected, flags)
+ }
+
+ actual := flags[1]
+ if !strings.Contains(actual, expected) {
+ t.Errorf("Expected %#v got %#v", expected, actual)
+ }
+
+ return actual
+}
+
func testBazelContext(t *testing.T, bazelCommandResults map[bazelCommand]string) (*bazelContext, string) {
t.Helper()
p := bazelPaths{
diff --git a/android/deapexer.go b/android/deapexer.go
index 265f531..6a93f60 100644
--- a/android/deapexer.go
+++ b/android/deapexer.go
@@ -15,6 +15,8 @@
package android
import (
+ "strings"
+
"github.com/google/blueprint"
)
@@ -148,12 +150,19 @@
func FindDeapexerProviderForModule(ctx ModuleContext) *DeapexerInfo {
var di *DeapexerInfo
ctx.VisitDirectDepsWithTag(DeapexerTag, func(m Module) {
- p := ctx.OtherModuleProvider(m, DeapexerProvider).(DeapexerInfo)
+ c := ctx.OtherModuleProvider(m, DeapexerProvider).(DeapexerInfo)
+ p := &c
if di != nil {
+ // If two DeapexerInfo providers have been found then check if they are
+ // equivalent. If they are then use the selected one, otherwise fail.
+ if selected := equivalentDeapexerInfoProviders(di, p); selected != nil {
+ di = selected
+ return
+ }
ctx.ModuleErrorf("Multiple installable prebuilt APEXes provide ambiguous deapexers: %s and %s",
di.ApexModuleName(), p.ApexModuleName())
}
- di = &p
+ di = p
})
if di != nil {
return di
@@ -162,3 +171,33 @@
ctx.ModuleErrorf("No prebuilt APEX provides a deapexer module for APEX variant %s", ai.ApexVariationName)
return nil
}
+
+// removeCompressedApexSuffix removes the _compressed suffix from the name if present.
+func removeCompressedApexSuffix(name string) string {
+ return strings.TrimSuffix(name, "_compressed")
+}
+
+// equivalentDeapexerInfoProviders checks to make sure that the two DeapexerInfo structures are
+// equivalent.
+//
+// At the moment <x> and <x>_compressed APEXes are treated as being equivalent.
+//
+// If they are not equivalent then this returns nil, otherwise, this returns the DeapexerInfo that
+// should be used by the build, which is always the uncompressed one. That ensures that the behavior
+// of the build is not dependent on which prebuilt APEX is visited first.
+func equivalentDeapexerInfoProviders(p1 *DeapexerInfo, p2 *DeapexerInfo) *DeapexerInfo {
+ n1 := removeCompressedApexSuffix(p1.ApexModuleName())
+ n2 := removeCompressedApexSuffix(p2.ApexModuleName())
+
+ // If the names don't match then they are not equivalent.
+ if n1 != n2 {
+ return nil
+ }
+
+ // Select the uncompressed APEX.
+ if n1 == removeCompressedApexSuffix(n1) {
+ return p1
+ } else {
+ return p2
+ }
+}
diff --git a/android/util.go b/android/util.go
index 47c4583..a0f7160 100644
--- a/android/util.go
+++ b/android/util.go
@@ -32,6 +32,12 @@
// JoinWithPrefix prepends the prefix to each string in the list and
// returns them joined together with " " as separator.
func JoinWithPrefix(strs []string, prefix string) string {
+ return JoinWithPrefixAndSeparator(strs, prefix, " ")
+}
+
+// JoinWithPrefixAndSeparator prepends the prefix to each string in the list and
+// returns them joined together with the given separator.
+func JoinWithPrefixAndSeparator(strs []string, prefix string, sep string) string {
if len(strs) == 0 {
return ""
}
@@ -40,7 +46,7 @@
buf.WriteString(prefix)
buf.WriteString(strs[0])
for i := 1; i < len(strs); i++ {
- buf.WriteString(" ")
+ buf.WriteString(sep)
buf.WriteString(prefix)
buf.WriteString(strs[i])
}
diff --git a/apex/apex_test.go b/apex/apex_test.go
index 7905710..dbe9180 100644
--- a/apex/apex_test.go
+++ b/apex/apex_test.go
@@ -7440,7 +7440,7 @@
return result.TestContext
}
-func TestDuplicateDeapexeresFromPrebuiltApexes(t *testing.T) {
+func TestDuplicateDeapexersFromPrebuiltApexes(t *testing.T) {
preparers := android.GroupFixturePreparers(
java.PrepareForTestWithJavaDefaultModules,
PrepareForTestWithApexBuildComponents,
@@ -7509,6 +7509,107 @@
})
}
+func TestDuplicateButEquivalentDeapexersFromPrebuiltApexes(t *testing.T) {
+ preparers := android.GroupFixturePreparers(
+ java.PrepareForTestWithJavaDefaultModules,
+ PrepareForTestWithApexBuildComponents,
+ )
+
+ bpBase := `
+ apex_set {
+ name: "com.android.myapex",
+ installable: true,
+ exported_bootclasspath_fragments: ["my-bootclasspath-fragment"],
+ set: "myapex.apks",
+ }
+
+ apex_set {
+ name: "com.android.myapex_compressed",
+ apex_name: "com.android.myapex",
+ installable: true,
+ exported_bootclasspath_fragments: ["my-bootclasspath-fragment"],
+ set: "myapex_compressed.apks",
+ }
+
+ prebuilt_bootclasspath_fragment {
+ name: "my-bootclasspath-fragment",
+ apex_available: [
+ "com.android.myapex",
+ "com.android.myapex_compressed",
+ ],
+ hidden_api: {
+ annotation_flags: "annotation-flags.csv",
+ metadata: "metadata.csv",
+ index: "index.csv",
+ signature_patterns: "signature_patterns.csv",
+ },
+ %s
+ }
+ `
+
+ t.Run("java_import", func(t *testing.T) {
+ result := preparers.RunTestWithBp(t,
+ fmt.Sprintf(bpBase, `contents: ["libfoo"]`)+`
+ java_import {
+ name: "libfoo",
+ jars: ["libfoo.jar"],
+ apex_available: [
+ "com.android.myapex",
+ "com.android.myapex_compressed",
+ ],
+ }
+ `)
+
+ module := result.Module("libfoo", "android_common_com.android.myapex")
+ usesLibraryDep := module.(java.UsesLibraryDependency)
+ android.AssertPathRelativeToTopEquals(t, "dex jar path",
+ "out/soong/.intermediates/com.android.myapex.deapexer/android_common/deapexer/javalib/libfoo.jar",
+ usesLibraryDep.DexJarBuildPath().Path())
+ })
+
+ t.Run("java_sdk_library_import", func(t *testing.T) {
+ result := preparers.RunTestWithBp(t,
+ fmt.Sprintf(bpBase, `contents: ["libfoo"]`)+`
+ java_sdk_library_import {
+ name: "libfoo",
+ public: {
+ jars: ["libbar.jar"],
+ },
+ apex_available: [
+ "com.android.myapex",
+ "com.android.myapex_compressed",
+ ],
+ compile_dex: true,
+ }
+ `)
+
+ module := result.Module("libfoo", "android_common_com.android.myapex")
+ usesLibraryDep := module.(java.UsesLibraryDependency)
+ android.AssertPathRelativeToTopEquals(t, "dex jar path",
+ "out/soong/.intermediates/com.android.myapex.deapexer/android_common/deapexer/javalib/libfoo.jar",
+ usesLibraryDep.DexJarBuildPath().Path())
+ })
+
+ t.Run("prebuilt_bootclasspath_fragment", func(t *testing.T) {
+ _ = preparers.RunTestWithBp(t, fmt.Sprintf(bpBase, `
+ image_name: "art",
+ contents: ["libfoo"],
+ `)+`
+ java_sdk_library_import {
+ name: "libfoo",
+ public: {
+ jars: ["libbar.jar"],
+ },
+ apex_available: [
+ "com.android.myapex",
+ "com.android.myapex_compressed",
+ ],
+ compile_dex: true,
+ }
+ `)
+ })
+}
+
func TestUpdatable_should_set_min_sdk_version(t *testing.T) {
testApexError(t, `"myapex" .*: updatable: updatable APEXes should set min_sdk_version`, `
apex {
diff --git a/bazel/aquery.go b/bazel/aquery.go
index 433d502..5e4ebf8 100644
--- a/bazel/aquery.go
+++ b/bazel/aquery.go
@@ -664,6 +664,9 @@
if a.Mnemonic == "FileWrite" {
return true
}
+ if a.Mnemonic == "BaselineCoverage" {
+ return true
+ }
return false
}
diff --git a/bazel/cquery/request_type.go b/bazel/cquery/request_type.go
index 5d00b0b..f5435f2 100644
--- a/bazel/cquery/request_type.go
+++ b/bazel/cquery/request_type.go
@@ -132,7 +132,7 @@
sharedLibraries = []
rootSharedLibraries = []
-shared_info_tag = "@rules_cc//examples:experimental_cc_shared_library.bzl%CcSharedLibraryInfo"
+shared_info_tag = "@_builtins//:common/cc/experimental_cc_shared_library.bzl%CcSharedLibraryInfo"
if shared_info_tag in providers(target):
shared_info = providers(target)[shared_info_tag]
for lib in shared_info.linker_input.libraries:
diff --git a/bp2build/cc_library_conversion_test.go b/bp2build/cc_library_conversion_test.go
index 555f5a7..2cc2207 100644
--- a/bp2build/cc_library_conversion_test.go
+++ b/bp2build/cc_library_conversion_test.go
@@ -959,11 +959,12 @@
"features": `[
"disable_pack_relocations",
"-no_undefined_symbols",
+ "-coverage",
]`,
"srcs": `["a.cpp"]`,
})...)
expected_targets = append(expected_targets, makeCcLibraryTargets("b", attrNameToString{
- "features": `select({
+ "features": `["-coverage"] + select({
"//build/bazel/platforms/arch:x86_64": [
"disable_pack_relocations",
"-no_undefined_symbols",
@@ -994,6 +995,7 @@
pack_relocations: false,
allow_undefined_symbols: true,
include_build_directory: false,
+ native_coverage: false,
}
cc_library {
@@ -1006,6 +1008,7 @@
},
},
include_build_directory: false,
+ native_coverage: false,
}
cc_library {
diff --git a/cc/bp2build.go b/cc/bp2build.go
index 70dcf40..d891007 100644
--- a/cc/bp2build.go
+++ b/cc/bp2build.go
@@ -55,6 +55,8 @@
Enabled bazel.BoolAttribute
+ Native_coverage bazel.BoolAttribute
+
sdkAttributes
}
@@ -568,10 +570,15 @@
}
}
}
-
compilerAttrs.convertStlProps(ctx, module)
(&linkerAttrs).convertStripProps(ctx, module)
+ if module.coverage != nil && module.coverage.Properties.Native_coverage != nil &&
+ !Bool(module.coverage.Properties.Native_coverage) {
+ // Native_coverage is arch neutral
+ (&linkerAttrs).features.Append(bazel.MakeStringListAttribute([]string{"-coverage"}))
+ }
+
productVariableProps := android.ProductVariableProperties(ctx)
(&compilerAttrs).convertProductVariables(ctx, productVariableProps)
diff --git a/cc/config/tidy.go b/cc/config/tidy.go
index f96f3ed..674edad 100644
--- a/cc/config/tidy.go
+++ b/cc/config/tidy.go
@@ -19,6 +19,37 @@
"strings"
)
+var (
+ // Some clang-tidy checks have bugs or don't work for Android.
+ // They are disabled here, overriding any locally selected checks.
+ globalNoCheckList = []string{
+ // https://b.corp.google.com/issues/153464409
+ // many local projects enable cert-* checks, which
+ // trigger bugprone-reserved-identifier.
+ "-bugprone-reserved-identifier*,-cert-dcl51-cpp,-cert-dcl37-c",
+ // http://b/153757728
+ "-readability-qualified-auto",
+ // http://b/193716442
+ "-bugprone-implicit-widening-of-multiplication-result",
+ // Too many existing functions trigger this rule, and fixing it requires large code
+ // refactoring. The cost of maintaining this tidy rule outweighs the benefit it brings.
+ "-bugprone-easily-swappable-parameters",
+ // http://b/216364337 - TODO: Follow-up after compiler update to
+ // disable or fix individual instances.
+ "-cert-err33-c",
+ }
+
+ // Some clang-tidy checks are included in some tidy_checks_as_errors lists,
+ // but not all warnings are fixed/suppressed yet. These checks are not
+ // disabled in the TidyGlobalNoChecks list, so we can see them and fix/suppress them.
+ globalNoErrorCheckList = []string{
+ // http://b/155034563
+ "-bugprone-signed-char-misuse",
+ // http://b/155034972
+ "-bugprone-branch-clone",
+ }
+)
+
func init() {
// Many clang-tidy checks like altera-*, llvm-*, modernize-*
// are not designed for Android source code or creating too
@@ -94,29 +125,12 @@
}, ",")
})
- // Some clang-tidy checks have bugs or not work for Android.
- // They are disabled here, overriding any locally selected checks.
pctx.VariableFunc("TidyGlobalNoChecks", func(ctx android.PackageVarContext) string {
- return strings.Join([]string{
- // https://b.corp.google.com/issues/153464409
- // many local projects enable cert-* checks, which
- // trigger bugprone-reserved-identifier.
- "-bugprone-reserved-identifier*,-cert-dcl51-cpp,-cert-dcl37-c",
- // http://b/153757728
- "-readability-qualified-auto",
- // http://b/155034563
- "-bugprone-signed-char-misuse",
- // http://b/155034972
- "-bugprone-branch-clone",
- // http://b/193716442
- "-bugprone-implicit-widening-of-multiplication-result",
- // Too many existing functions trigger this rule, and fixing it requires large code
- // refactoring. The cost of maintaining this tidy rule outweighs the benefit it brings.
- "-bugprone-easily-swappable-parameters",
- // http://b/216364337 - TODO: Follow-up after compiler update to
- // disable or fix individual instances.
- "-cert-err33-c",
- }, ",")
+ return strings.Join(globalNoCheckList, ",")
+ })
+
+ pctx.VariableFunc("TidyGlobalNoErrorChecks", func(ctx android.PackageVarContext) string {
+ return strings.Join(globalNoErrorCheckList, ",")
})
// To reduce duplicate warnings from the same header files,
@@ -140,7 +154,6 @@
const tidyDefault = "${config.TidyDefaultGlobalChecks}"
const tidyExternalVendor = "${config.TidyExternalVendorChecks}"
const tidyDefaultNoAnalyzer = "${config.TidyDefaultGlobalChecks},-clang-analyzer-*"
-const tidyGlobalNoChecks = "${config.TidyGlobalNoChecks}"
// This is a map of local path prefixes to the set of default clang-tidy checks
// to be used.
@@ -180,7 +193,18 @@
// Returns a globally disabled tidy checks, overriding locally selected checks.
func TidyGlobalNoChecks() string {
- return tidyGlobalNoChecks
+ if len(globalNoCheckList) > 0 {
+ return ",${config.TidyGlobalNoChecks}"
+ }
+ return ""
+}
+
+// Returns a globally allowed/no-error tidy checks, appended to -warnings-as-errors.
+func TidyGlobalNoErrorChecks() string {
+ if len(globalNoErrorCheckList) > 0 {
+ return ",${config.TidyGlobalNoErrorChecks}"
+ }
+ return ""
}
func TidyFlagsForSrcFile(srcFile android.Path, flags string) string {
diff --git a/cc/tidy.go b/cc/tidy.go
index 75c038d..6b5d572 100644
--- a/cc/tidy.go
+++ b/cc/tidy.go
@@ -15,6 +15,7 @@
package cc
import (
+ "fmt"
"path/filepath"
"regexp"
"strings"
@@ -62,6 +63,11 @@
return []interface{}{&tidy.Properties}
}
+// Set this const to true when all -warnings-as-errors in tidy_flags
+// are replaced with tidy_checks_as_errors.
+// Then, that old style usage will be obsolete and an error.
+const NoWarningsAsErrorsInTidyFlags = true
+
func (tidy *tidyFeature) flags(ctx ModuleContext, flags Flags) Flags {
CheckBadTidyFlags(ctx, "tidy_flags", tidy.Properties.Tidy_flags)
CheckBadTidyChecks(ctx, "tidy_checks", tidy.Properties.Tidy_checks)
@@ -161,42 +167,37 @@
config.ClangRewriteTidyChecks(localChecks)), ",")
}
}
- tidyChecks = tidyChecks + "," + config.TidyGlobalNoChecks()
+ tidyChecks = tidyChecks + config.TidyGlobalNoChecks()
if ctx.Windows() {
// https://b.corp.google.com/issues/120614316
// mingw32 has cert-dcl16-c warning in NO_ERROR,
// which is used in many Android files.
- tidyChecks = tidyChecks + ",-cert-dcl16-c"
+ tidyChecks += ",-cert-dcl16-c"
}
flags.TidyFlags = append(flags.TidyFlags, tidyChecks)
- if ctx.Config().IsEnvTrue("WITH_TIDY") {
- // WITH_TIDY=1 enables clang-tidy globally. There could be many unexpected
- // warnings from new checks and many local tidy_checks_as_errors and
- // -warnings-as-errors can break a global build.
- // So allow all clang-tidy warnings.
- inserted := false
- for i, s := range flags.TidyFlags {
- if strings.Contains(s, "-warnings-as-errors=") {
- // clang-tidy accepts only one -warnings-as-errors
- // replace the old one
- re := regexp.MustCompile(`'?-?-warnings-as-errors=[^ ]* *`)
- newFlag := re.ReplaceAllString(s, "")
- if newFlag == "" {
- flags.TidyFlags[i] = "-warnings-as-errors=-*"
- } else {
- flags.TidyFlags[i] = newFlag + " -warnings-as-errors=-*"
- }
- inserted = true
- break
+ // Embedding -warnings-as-errors in tidy_flags is error-prone.
+ // It should be replaced with the tidy_checks_as_errors list.
+ for i, s := range flags.TidyFlags {
+ if strings.Contains(s, "-warnings-as-errors=") {
+ if NoWarningsAsErrorsInTidyFlags {
+ ctx.PropertyErrorf("tidy_flags", "should not contain "+s+"; use tidy_checks_as_errors instead.")
+ } else {
+ fmt.Printf("%s: warning: module %s's tidy_flags should not contain %s, which is replaced with -warnings-as-errors=-*; use tidy_checks_as_errors for your own as-error warnings instead.\n",
+ ctx.BlueprintsFile(), ctx.ModuleName(), s)
+ flags.TidyFlags[i] = "-warnings-as-errors=-*"
}
+ break // there is at most one -warnings-as-errors
}
- if !inserted {
- flags.TidyFlags = append(flags.TidyFlags, "-warnings-as-errors=-*")
- }
- } else if len(tidy.Properties.Tidy_checks_as_errors) > 0 {
- tidyChecksAsErrors := "-warnings-as-errors=" + strings.Join(esc(ctx, "tidy_checks_as_errors", tidy.Properties.Tidy_checks_as_errors), ",")
+ }
+ // Default clang-tidy flags does not contain -warning-as-errors.
+ // If a module has tidy_checks_as_errors, add the list to -warnings-as-errors
+ // and then append the TidyGlobalNoErrorChecks.
+ if len(tidy.Properties.Tidy_checks_as_errors) > 0 {
+ tidyChecksAsErrors := "-warnings-as-errors=" +
+ strings.Join(esc(ctx, "tidy_checks_as_errors", tidy.Properties.Tidy_checks_as_errors), ",") +
+ config.TidyGlobalNoErrorChecks()
flags.TidyFlags = append(flags.TidyFlags, tidyChecksAsErrors)
}
return flags
diff --git a/cc/tidy_test.go b/cc/tidy_test.go
index 5c15fac..7036ecb 100644
--- a/cc/tidy_test.go
+++ b/cc/tidy_test.go
@@ -22,6 +22,82 @@
"android/soong/android"
)
+func TestTidyFlagsWarningsAsErrors(t *testing.T) {
+ // The "tidy_flags" property should not contain -warnings-as-errors.
+ type testCase struct {
+ libName, bp string
+ errorMsg string // a negative test; must have error message
+ flags []string // must have substrings in tidyFlags
+ noFlags []string // must not have substrings in tidyFlags
+ }
+
+ testCases := []testCase{
+ {
+ "libfoo1",
+ `cc_library_shared { // no warnings-as-errors, good tidy_flags
+ name: "libfoo1",
+ srcs: ["foo.c"],
+ tidy_flags: ["-header-filter=dir1/"],
+ }`,
+ "",
+ []string{"-header-filter=dir1/"},
+ []string{"-warnings-as-errors"},
+ },
+ {
+ "libfoo2",
+ `cc_library_shared { // good use of tidy_checks_as_errors
+ name: "libfoo2",
+ srcs: ["foo.c"],
+ tidy_checks_as_errors: ["xyz-*", "abc"],
+ }`,
+ "",
+ []string{
+ "-header-filter=^", // there is a default header filter
+ "-warnings-as-errors='xyz-*',abc,${config.TidyGlobalNoErrorChecks}",
+ },
+ []string{},
+ },
+ }
+ if NoWarningsAsErrorsInTidyFlags {
+ testCases = append(testCases, testCase{
+ "libfoo3",
+ `cc_library_shared { // bad use of -warnings-as-errors in tidy_flags
+ name: "libfoo3",
+ srcs: ["foo.c"],
+ tidy_flags: [
+ "-header-filters=.*",
+ "-warnings-as-errors=xyz-*",
+ ],
+ }`,
+ `module "libfoo3" .*: tidy_flags: should not contain .*;` +
+ ` use tidy_checks_as_errors instead`,
+ []string{},
+ []string{},
+ })
+ }
+ for _, test := range testCases {
+ if test.errorMsg != "" {
+ testCcError(t, test.errorMsg, test.bp)
+ continue
+ }
+ variant := "android_arm64_armv8-a_shared"
+ ctx := testCc(t, test.bp)
+ t.Run("caseTidyFlags", func(t *testing.T) {
+ flags := ctx.ModuleForTests(test.libName, variant).Rule("clangTidy").Args["tidyFlags"]
+ for _, flag := range test.flags {
+ if !strings.Contains(flags, flag) {
+ t.Errorf("tidyFlags %v for %s does not contain %s.", flags, test.libName, flag)
+ }
+ }
+ for _, flag := range test.noFlags {
+ if strings.Contains(flags, flag) {
+ t.Errorf("tidyFlags %v for %s should not contain %s.", flags, test.libName, flag)
+ }
+ }
+ })
+ }
+}
+
func TestTidyChecks(t *testing.T) {
// The "tidy_checks" property defines additional checks appended
// to global default. But there are some checks disabled after
diff --git a/cmd/soong_build/main.go b/cmd/soong_build/main.go
index c548ef8..8a3d6e0 100644
--- a/cmd/soong_build/main.go
+++ b/cmd/soong_build/main.go
@@ -139,7 +139,7 @@
bazelHook := func() error {
ctx.EventHandler.Begin("bazel")
defer ctx.EventHandler.End("bazel")
- return configuration.BazelContext.InvokeBazel()
+ return configuration.BazelContext.InvokeBazel(configuration)
}
ctx.SetBeforePrepareBuildActionsHook(bazelHook)
diff --git a/rust/config/global.go b/rust/config/global.go
index 647a7cf..e9751fd 100644
--- a/rust/config/global.go
+++ b/rust/config/global.go
@@ -24,7 +24,7 @@
var pctx = android.NewPackageContext("android/soong/rust/config")
var (
- RustDefaultVersion = "1.61.0.p1"
+ RustDefaultVersion = "1.61.0.p2"
RustDefaultBase = "prebuilts/rust/"
DefaultEdition = "2021"
Stdlibs = []string{
diff --git a/scripts/manifest_check.py b/scripts/manifest_check.py
index 0216fc0..c8d4f76 100755
--- a/scripts/manifest_check.py
+++ b/scripts/manifest_check.py
@@ -20,11 +20,9 @@
import argparse
import json
-import os
import re
import subprocess
import sys
-from collections import OrderedDict
from xml.dom import minidom
from manifest import android_ns
@@ -45,13 +43,11 @@
'--uses-library',
dest='uses_libraries',
action='append',
- default=[],
help='specify uses-library entries known to the build system')
parser.add_argument(
'--optional-uses-library',
dest='optional_uses_libraries',
action='append',
- default=[],
help='specify uses-library entries known to the build system with '
'required:false'
)
@@ -78,14 +74,9 @@
help='print the targetSdkVersion from the manifest')
parser.add_argument(
'--dexpreopt-config',
- dest='dexpreopt_config',
- help='a path to dexpreopt.config file for this library/app')
- parser.add_argument(
- '--dexpreopt-dep-config',
- dest='dexpreopt_dep_configs',
+ dest='dexpreopt_configs',
action='append',
- default=[],
- help='a path to dexpreopt.config file for a dependency library')
+ help='a paths to a dexpreopt.config of some library')
parser.add_argument('--aapt', dest='aapt', help='path to aapt executable')
parser.add_argument(
'--output', '-o', dest='output', help='output AndroidManifest.xml file')
@@ -304,53 +295,25 @@
return target_attr.value
-def remove_duplicates(l):
- return list(OrderedDict.fromkeys(l))
-
-
-def load_dexpreopt_configs(args):
+def load_dexpreopt_configs(configs):
"""Load dexpreopt.config files and map module names to library names."""
module_to_libname = {}
- # Go over dexpreopt.config files for uses-library dependencies and create
- # a mapping from module name to real library name (they may differ).
- for config in args.dexpreopt_dep_configs:
- # Empty dexpreopt.config files are expected for some dependencies.
- if os.stat(config).st_size != 0:
- with open(config, 'r') as f:
- contents = json.load(f)
- module_to_libname[contents['Name']] = contents['ProvidesUsesLibrary']
+ if configs is None:
+ configs = []
- required = translate_libnames(args.uses_libraries, module_to_libname)
- optional = translate_libnames(args.optional_uses_libraries, module_to_libname)
-
- # Add extra uses-libraries from the library/app's own dexpreopt.config.
- # Extra libraries may be propagated via dependencies' dexpreopt.config files
- # (not only uses-library ones, but also transitively via static libraries).
- if args.dexpreopt_config:
- with open(args.dexpreopt_config, 'r') as f:
+ for config in configs:
+ with open(config, 'r') as f:
contents = json.load(f)
- for clc in contents['ClassLoaderContexts']['any']:
- ulib = clc['Name']
- if clc['Optional']:
- optional.append(ulib)
- else:
- required.append(ulib)
+ module_to_libname[contents['Name']] = contents['ProvidesUsesLibrary']
- required = remove_duplicates(required)
- optional = remove_duplicates(optional)
-
- # If the same library is both in optional and required, prefer required.
- # This may happen for compatibility libraries, e.g. org.apache.http.legacy.
- for lib in required:
- if lib in optional:
- optional.remove(lib)
-
- return required, optional
+ return module_to_libname
def translate_libnames(modules, module_to_libname):
"""Translate module names into library names using the mapping."""
+ if modules is None:
+ modules = []
libnames = []
for name in modules:
@@ -383,7 +346,10 @@
# `optional_uses_libs`, `LOCAL_USES_LIBRARIES`,
# `LOCAL_OPTIONAL_LIBRARY_NAMES` all contain module names), while
# the manifest addresses libraries by their name.
- required, optional = load_dexpreopt_configs(args)
+ mod_to_lib = load_dexpreopt_configs(args.dexpreopt_configs)
+ required = translate_libnames(args.uses_libraries, mod_to_lib)
+ optional = translate_libnames(args.optional_uses_libraries,
+ mod_to_lib)
# Check if the <uses-library> lists in the build system agree with
# those in the manifest. Raise an exception on mismatch, unless the