From 104f51f70bb80dc61154bc04cb402cb712ed470f Mon Sep 17 00:00:00 2001 From: Chih-Hung Hsieh Date: Wed, 20 Apr 2022 15:48:41 -0700 Subject: [PATCH] add ALLOW_LOCAL_TIDY_TRUE and some tests * A new ALLOW_LOCAL_TIDY_TRUE variable, default is false. * If it is 0/false, local "tidy:true" is ignored. * If it is 1/true, local "tidy:true" is honored as it is now. Bug: 229779921 Test: make with and without ALLOW_LOCAL_TIDY_TRUE=1 Change-Id: I0323289a4d3bb2514982252a5a1339e94f2bbaab --- cc/Android.bp | 1 + cc/OWNERS | 2 +- cc/tidy.go | 5 ++- cc/tidy_test.go | 98 +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 103 insertions(+), 3 deletions(-) create mode 100644 cc/tidy_test.go diff --git a/cc/Android.bp b/cc/Android.bp index 9103a48b4..b105e7c18 100644 --- a/cc/Android.bp +++ b/cc/Android.bp @@ -100,6 +100,7 @@ bootstrap_go_package { "proto_test.go", "sanitize_test.go", "test_data_test.go", + "tidy_test.go", "vendor_public_library_test.go", "vendor_snapshot_test.go", ], diff --git a/cc/OWNERS b/cc/OWNERS index a438b1564..ffbf14abe 100644 --- a/cc/OWNERS +++ b/cc/OWNERS @@ -1,4 +1,4 @@ per-file ndk_*.go = danalbert@google.com -per-file tidy.go = srhines@google.com, chh@google.com +per-file tidy*.go = srhines@google.com, chh@google.com per-file afdo.go,afdo_test.go,lto.go,pgo.go = srhines@google.com, pirama@google.com, yikong@google.com per-file coverage.go = pirama@google.com, srhines@google.com, allenhair@google.com diff --git a/cc/tidy.go b/cc/tidy.go index 750e9de1e..03e967d7c 100644 --- a/cc/tidy.go +++ b/cc/tidy.go @@ -76,9 +76,10 @@ func (tidy *tidyFeature) flags(ctx ModuleContext, flags Flags) Flags { // the global WITH_TIDY or module 'tidy' property is true. flags.Tidy = true - // If explicitly enabled, by global default or local tidy property, + // If explicitly enabled, by global WITH_TIDY or local tidy:true property, // set flags.NeedTidyFiles to make this module depend on .tidy files. - if ctx.Config().ClangTidy() || Bool(tidy.Properties.Tidy) { + // Note that locally set tidy:true is ignored if ALLOW_LOCAL_TIDY_TRUE is not set to true. + if ctx.Config().IsEnvTrue("WITH_TIDY") || (ctx.Config().IsEnvTrue("ALLOW_LOCAL_TIDY_TRUE") && Bool(tidy.Properties.Tidy)) { flags.NeedTidyFiles = true } diff --git a/cc/tidy_test.go b/cc/tidy_test.go new file mode 100644 index 000000000..339b30281 --- /dev/null +++ b/cc/tidy_test.go @@ -0,0 +1,98 @@ +// Copyright 2022 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 cc + +import ( + "fmt" + "testing" + + "android/soong/android" +) + +func TestWithTidy(t *testing.T) { + // When WITH_TIDY=1 or (ALLOW_LOCAL_TIDY_TRUE=1 and local tidy:true) + // a C++ library should depend on .tidy files. + testCases := []struct { + withTidy, allowLocalTidyTrue string // "_" means undefined + needTidyFile []bool // for {libfoo_0, libfoo_1} and {libbar_0, libbar_1} + }{ + {"_", "_", []bool{false, false, false}}, + {"_", "0", []bool{false, false, false}}, + {"_", "1", []bool{false, true, false}}, + {"_", "true", []bool{false, true, false}}, + {"0", "_", []bool{false, false, false}}, + {"0", "1", []bool{false, true, false}}, + {"1", "_", []bool{true, true, false}}, + {"1", "false", []bool{true, true, false}}, + {"1", "1", []bool{true, true, false}}, + {"true", "_", []bool{true, true, false}}, + } + bp := ` + cc_library_shared { + name: "libfoo_0", // depends on .tidy if WITH_TIDY=1 + srcs: ["foo.c"], + } + cc_library_shared { // depends on .tidy if WITH_TIDY=1 or ALLOW_LOCAL_TIDY_TRUE=1 + name: "libfoo_1", + srcs: ["foo.c"], + tidy: true, + } + cc_library_shared { // no .tidy + name: "libfoo_2", + srcs: ["foo.c"], + tidy: false, + } + cc_library_static { + name: "libbar_0", // depends on .tidy if WITH_TIDY=1 + srcs: ["bar.c"], + } + cc_library_static { // depends on .tidy if WITH_TIDY=1 or ALLOW_LOCAL_TIDY_TRUE=1 + name: "libbar_1", + srcs: ["bar.c"], + tidy: true, + } + cc_library_static { // no .tidy + name: "libbar_2", + srcs: ["bar.c"], + tidy: false, + }` + for index, test := range testCases { + testName := fmt.Sprintf("case%d,%v,%v", index, test.withTidy, test.allowLocalTidyTrue) + t.Run(testName, func(t *testing.T) { + testEnv := map[string]string{} + if test.withTidy != "_" { + testEnv["WITH_TIDY"] = test.withTidy + } + if test.allowLocalTidyTrue != "_" { + testEnv["ALLOW_LOCAL_TIDY_TRUE"] = test.allowLocalTidyTrue + } + ctx := android.GroupFixturePreparers(prepareForCcTest, android.FixtureMergeEnv(testEnv)).RunTestWithBp(t, bp) + for n := 0; n < 3; n++ { + checkLibraryRule := func(foo, variant, ruleName string) { + libName := fmt.Sprintf("lib%s_%d", foo, n) + tidyFile := "out/soong/.intermediates/" + libName + "/" + variant + "/obj/" + foo + ".tidy" + depFiles := ctx.ModuleForTests(libName, variant).Rule(ruleName).Validations.Strings() + if test.needTidyFile[n] { + android.AssertStringListContains(t, libName+" needs .tidy file", depFiles, tidyFile) + } else { + android.AssertStringListDoesNotContain(t, libName+" does not need .tidy file", depFiles, tidyFile) + } + } + checkLibraryRule("foo", "android_arm64_armv8-a_shared", "ld") + checkLibraryRule("bar", "android_arm64_armv8-a_static", "ar") + } + }) + } +}