diff --git a/android/bazel_handler.go b/android/bazel_handler.go index 10cf60a0a..7e81792a1 100644 --- a/android/bazel_handler.go +++ b/android/bazel_handler.go @@ -505,7 +505,7 @@ func (m noopBazelContext) AqueryDepsets() []bazel.AqueryDepset { return []bazel.AqueryDepset{} } -func addToStringSet(set map[string]bool, items []string) { +func AddToStringSet(set map[string]bool, items []string) { for _, item := range items { set[item] = true } @@ -517,19 +517,19 @@ func GetBazelEnabledAndDisabledModules(buildMode SoongBuildMode, forceEnabled ma switch buildMode { case BazelProdMode: - addToStringSet(enabledModules, allowlists.ProdMixedBuildsEnabledList) + AddToStringSet(enabledModules, allowlists.ProdMixedBuildsEnabledList) for enabledAdHocModule := range forceEnabled { enabledModules[enabledAdHocModule] = true } case BazelStagingMode: // Staging mode includes all prod modules plus all staging modules. - addToStringSet(enabledModules, allowlists.ProdMixedBuildsEnabledList) - addToStringSet(enabledModules, allowlists.StagingMixedBuildsEnabledList) + AddToStringSet(enabledModules, allowlists.ProdMixedBuildsEnabledList) + AddToStringSet(enabledModules, allowlists.StagingMixedBuildsEnabledList) for enabledAdHocModule := range forceEnabled { enabledModules[enabledAdHocModule] = true } case BazelDevMode: - addToStringSet(disabledModules, allowlists.MixedBuildsDisabledList) + AddToStringSet(disabledModules, allowlists.MixedBuildsDisabledList) default: panic("Expected BazelProdMode, BazelStagingMode, or BazelDevMode") } @@ -609,7 +609,7 @@ func NewBazelContext(c *config) (BazelContext, error) { allowlists.StagingDclaMixedBuildsEnabledList...) } dclaEnabledModules := map[string]bool{} - addToStringSet(dclaEnabledModules, dclaMixedBuildsEnabledList) + AddToStringSet(dclaEnabledModules, dclaMixedBuildsEnabledList) return &mixedBuildBazelContext{ bazelRunner: &builtinBazelRunner{c.UseBazelProxy, absolutePath(c.outDir)}, paths: &paths, diff --git a/android/config.go b/android/config.go index acadb3ff3..58ad4c5c6 100644 --- a/android/config.go +++ b/android/config.go @@ -1886,8 +1886,8 @@ func (c *deviceConfig) BuildBrokenInputDir(name string) bool { return InList(name, c.config.productVariables.BuildBrokenInputDirModules) } -func (c *deviceConfig) BuildBrokenDepfile() bool { - return Bool(c.config.productVariables.BuildBrokenDepfile) +func (c *deviceConfig) GenruleSandboxing() bool { + return Bool(c.config.productVariables.GenruleSandboxing) } func (c *deviceConfig) RequiresInsecureExecmemForSwiftshader() bool { diff --git a/android/variable.go b/android/variable.go index 97171e7b9..a6822c6a4 100644 --- a/android/variable.go +++ b/android/variable.go @@ -443,7 +443,7 @@ type productVariables struct { BuildBrokenClangAsFlags bool `json:",omitempty"` BuildBrokenClangCFlags bool `json:",omitempty"` BuildBrokenClangProperty bool `json:",omitempty"` - BuildBrokenDepfile *bool `json:",omitempty"` + GenruleSandboxing *bool `json:",omitempty"` BuildBrokenEnforceSyspropOwner bool `json:",omitempty"` BuildBrokenTrebleSyspropNeverallow bool `json:",omitempty"` BuildBrokenUsesSoongPython2Modules bool `json:",omitempty"` diff --git a/genrule/Android.bp b/genrule/Android.bp index 8fb5c402c..b201cae9a 100644 --- a/genrule/Android.bp +++ b/genrule/Android.bp @@ -15,6 +15,7 @@ bootstrap_go_package { "soong-shared", ], srcs: [ + "allowlists.go", "genrule.go", "locations.go", ], diff --git a/genrule/allowlists.go b/genrule/allowlists.go new file mode 100644 index 000000000..875dbabed --- /dev/null +++ b/genrule/allowlists.go @@ -0,0 +1,147 @@ +// Copyright 2023 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 genrule + +import ( + "android/soong/android" +) + +var ( + DepfileAllowList = []string{ + "depfile_allowed_for_test", + "tflite_support_spm_config", + "tflite_support_spm_encoder_config", + "gen_uwb_core_proto", + "libtextclassifier_fbgen_utils_flatbuffers_flatbuffers_test", + "libtextclassifier_fbgen_utils_lua_utils_tests", + "libtextclassifier_fbgen_lang_id_common_flatbuffers_model", + "libtextclassifier_fbgen_lang_id_common_flatbuffers_embedding-network", + "libtextclassifier_fbgen_annotator_datetime_datetime", + "libtextclassifier_fbgen_annotator_model", + "libtextclassifier_fbgen_annotator_experimental_experimental", + "libtextclassifier_fbgen_annotator_entity-data", + "libtextclassifier_fbgen_annotator_person_name_person_name_model", + "libtextclassifier_fbgen_utils_tflite_text_encoder_config", + "libtextclassifier_fbgen_utils_codepoint-range", + "libtextclassifier_fbgen_utils_intents_intent-config", + "libtextclassifier_fbgen_utils_flatbuffers_flatbuffers", + "libtextclassifier_fbgen_utils_zlib_buffer", + "libtextclassifier_fbgen_utils_tokenizer", + "libtextclassifier_fbgen_utils_grammar_rules", + "libtextclassifier_fbgen_utils_grammar_semantics_expression", + "libtextclassifier_fbgen_utils_resources", + "libtextclassifier_fbgen_utils_i18n_language-tag", + "libtextclassifier_fbgen_utils_normalization", + "libtextclassifier_fbgen_utils_container_bit-vector", + "libtextclassifier_fbgen_actions_actions-entity-data", + "libtextclassifier_fbgen_actions_actions_model", + "libtextclassifier_fbgen_utils_grammar_testing_value", + } + + SandboxingDenyModuleList = []string{ + "framework-javastream-protos", + "RsBalls-rscript", + "CtsRsBlasTestCases-rscript", + "pvmfw_fdt_template_rs", + "RSTest_v14-rscript", + "com.android.apex.test.bar_stripped", + "com.android.apex.test.sharedlibs_secondary_generated", + "ImageProcessingJB-rscript", + "RSTest-rscript", + "BluetoothGeneratedDumpsysBinarySchema_bfbs", + "WmediumdServerProto_h", + "TracingVMProtoStub_h", + "FrontendStub_h", + "VehicleServerProtoStub_cc", + "AudioFocusControlProtoStub_cc", + "AudioFocusControlProtoStub_h", + "TracingVMProtoStub_cc", + "VehicleServerProtoStub_h", + "hidl2aidl_translate_cpp_test_gen_headers", + "hidl2aidl_translate_cpp_test_gen_src", + "hidl2aidl_translate_java_test_gen_src", + "hidl2aidl_translate_ndk_test_gen_headers", + "hidl2aidl_translate_ndk_test_gen_src", + "hidl_hash_test_gen", + "nos_app_avb_service_genc++", + "nos_app_avb_service_genc++_headers", + "nos_app_avb_service_genc++_mock", + "nos_app_identity_service_genc++", + "nos_app_keymaster_service_genc++", + "nos_generator_test_service_genc++_headers", + "nos_generator_test_service_genc++_mock", + "r8retrace-run-retrace", + "ltp_config_arm", + "ltp_config_arm_64_hwasan", + "ltp_config_arm_lowmem", + "ltp_config_arm_64", + "ltp_config_riscv_64", + "ltp_config_x86_64", + "vm-tests-tf-lib", + "hidl_cpp_impl_test_gen-headers", + "pandora_experimental-python-gen-src", + "framework-cppstream-protos", + "Refocus-rscript", + "RSTest_v11-rscript", + "RSTest_v16-rscript", + "ScriptGroupTest-rscript", + "ImageProcessing2-rscript", + "ImageProcessing-rscript", + "com.android.apex.test.pony_stripped", + "com.android.apex.test.baz_stripped", + "com.android.apex.test.foo_stripped", + "com.android.apex.test.sharedlibs_generated", + "CtsRenderscriptTestCases-rscript", + "BlueberryFacadeAndCertGeneratedStub_py", + "BlueberryFacadeGeneratedStub_cc", + "BlueberryFacadeGeneratedStub_h", + "BluetoothGeneratedDumpsysDataSchema_h", + "FrontendStub_cc", + "OpenwrtControlServerProto_cc", + "OpenwrtControlServerProto_h", + "WmediumdServerProto_cc", + "c2hal_test_genc++", + "c2hal_test_genc++_headers", + "hidl2aidl_test_gen_aidl", + "hidl_error_test_gen", + "hidl_export_test_gen-headers", + "hidl_format_test_diff", + "hidl_hash_version_gen", + "libbt_topshim_facade_py_proto", + "nos_app_identity_service_genc++_headers", + "nos_app_identity_service_genc++_mock", + "nos_app_keymaster_service_genc++_headers", + "nos_app_keymaster_service_genc++_mock", + "nos_app_weaver_service_genc++", + "nos_app_weaver_service_genc++_headers", + "nos_app_weaver_service_genc++_mock", + "nos_generator_test_service_genc++", + "pandora-python-gen-src", + } + + SandboxingDenyPathList = []string{ + "art/test", + "external/perfetto", + } +) +var DepfileAllowSet = map[string]bool{} +var SandboxingDenyModuleSet = map[string]bool{} +var SandboxingDenyPathSet = map[string]bool{} + +func init() { + android.AddToStringSet(DepfileAllowSet, DepfileAllowList) + android.AddToStringSet(SandboxingDenyModuleSet, append(DepfileAllowList, SandboxingDenyModuleList...)) + android.AddToStringSet(SandboxingDenyPathSet, SandboxingDenyPathList) +} diff --git a/genrule/genrule.go b/genrule/genrule.go index 00adb7025..c830fcc43 100644 --- a/genrule/genrule.go +++ b/genrule/genrule.go @@ -452,7 +452,7 @@ func (g *Module) generateCommonBuildActions(ctx android.ModuleContext) { manifestPath := android.PathForModuleOut(ctx, manifestName) // Use a RuleBuilder to create a rule that runs the command inside an sbox sandbox. - rule := android.NewRuleBuilder(pctx, ctx).Sbox(task.genDir, manifestPath).SandboxTools() + rule := getSandboxedRuleBuilder(ctx, android.NewRuleBuilder(pctx, ctx).Sbox(task.genDir, manifestPath)) cmd := rule.Command() for _, out := range task.out { @@ -594,14 +594,16 @@ func (g *Module) generateCommonBuildActions(ctx android.ModuleContext) { func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) { // Allowlist genrule to use depfile until we have a solution to remove it. // TODO(b/235582219): Remove allowlist for genrule - if ctx.ModuleType() == "gensrcs" && - !ctx.DeviceConfig().BuildBrokenDepfile() && - Bool(g.properties.Depfile) { - ctx.PropertyErrorf( - "depfile", - "Deprecated to ensure the module type is convertible to Bazel. "+ - "Try specifying the dependencies explicitly so that there is no need to use depfile. "+ - "If not possible, the escape hatch is to use BUILD_BROKEN_DEPFILE to bypass the error.") + if Bool(g.properties.Depfile) { + // TODO(b/283852474): Checking the GenruleSandboxing flag is temporary in + // order to pass the presubmit before internal master is updated. + if ctx.DeviceConfig().GenruleSandboxing() && !DepfileAllowSet[g.Name()] { + ctx.PropertyErrorf( + "depfile", + "Deprecated to ensure the module type is convertible to Bazel. "+ + "Try specifying the dependencies explicitly so that there is no need to use depfile. "+ + "If not possible, the escape hatch is to add the module to allowlists.go to bypass the error.") + } } g.generateCommonBuildActions(ctx) @@ -737,7 +739,7 @@ func NewGenSrcs() *Module { // TODO(ccross): this RuleBuilder is a hack to be able to call // rule.Command().PathForOutput. Replace this with passing the rule into the // generator. - rule := android.NewRuleBuilder(pctx, ctx).Sbox(genDir, nil).SandboxTools() + rule := getSandboxedRuleBuilder(ctx, android.NewRuleBuilder(pctx, ctx).Sbox(genDir, nil)) for _, in := range shard { outFile := android.GenPathWithExt(ctx, finalSubDir, in, String(properties.Output_extension)) @@ -1020,3 +1022,11 @@ func DefaultsFactory(props ...interface{}) android.Module { return module } + +func getSandboxedRuleBuilder(ctx android.ModuleContext, r *android.RuleBuilder) *android.RuleBuilder { + if !ctx.DeviceConfig().GenruleSandboxing() || SandboxingDenyPathSet[ctx.ModuleDir()] || + SandboxingDenyModuleSet[ctx.ModuleName()] { + return r.SandboxTools() + } + return r.SandboxInputs() +} diff --git a/genrule/genrule_test.go b/genrule/genrule_test.go index 63f8fa9b8..370fe3d46 100644 --- a/genrule/genrule_test.go +++ b/genrule/genrule_test.go @@ -97,8 +97,9 @@ func testGenruleBp() string { func TestGenruleCmd(t *testing.T) { testcases := []struct { - name string - prop string + name string + moduleName string + prop string allowMissingDependencies bool @@ -285,7 +286,8 @@ func TestGenruleCmd(t *testing.T) { expect: "echo foo > __SBOX_SANDBOX_DIR__/out/out2", }, { - name: "depfile", + name: "depfile", + moduleName: "depfile_allowed_for_test", prop: ` out: ["out"], depfile: true, @@ -397,7 +399,8 @@ func TestGenruleCmd(t *testing.T) { err: "$(depfile) used without depfile property", }, { - name: "error no depfile", + name: "error no depfile", + moduleName: "depfile_allowed_for_test", prop: ` out: ["out"], depfile: true, @@ -440,11 +443,15 @@ func TestGenruleCmd(t *testing.T) { for _, test := range testcases { t.Run(test.name, func(t *testing.T) { - bp := "genrule {\n" - bp += "name: \"gen\",\n" - bp += test.prop - bp += "}\n" - + moduleName := "gen" + if test.moduleName != "" { + moduleName = test.moduleName + } + bp := fmt.Sprintf(` + genrule { + name: "%s", + %s + }`, moduleName, test.prop) var expectedErrors []string if test.err != "" { expectedErrors = append(expectedErrors, regexp.QuoteMeta(test.err)) @@ -455,6 +462,9 @@ func TestGenruleCmd(t *testing.T) { android.FixtureModifyProductVariables(func(variables android.FixtureProductVariables) { variables.Allow_missing_dependencies = proptools.BoolPtr(test.allowMissingDependencies) }), + android.FixtureModifyProductVariables(func(variables android.FixtureProductVariables) { + variables.GenruleSandboxing = proptools.BoolPtr(true) + }), android.FixtureModifyContext(func(ctx *android.TestContext) { ctx.SetAllowMissingDependencies(test.allowMissingDependencies) }), @@ -466,7 +476,7 @@ func TestGenruleCmd(t *testing.T) { return } - gen := result.Module("gen", "").(*Module) + gen := result.Module(moduleName, "").(*Module) android.AssertStringEquals(t, "raw commands", test.expect, gen.rawCommands[0]) }) } @@ -627,53 +637,42 @@ func TestGenSrcs(t *testing.T) { } } -func TestGensrcsBuildBrokenDepfile(t *testing.T) { +func TestGenruleAllowlistingDepfile(t *testing.T) { tests := []struct { - name string - prop string - BuildBrokenDepfile *bool - err string + name string + prop string + err string + moduleName string }{ { - name: `error when BuildBrokenDepfile is set to false`, + name: `error when module is not allowlisted`, prop: ` depfile: true, cmd: "cat $(in) > $(out) && cat $(depfile)", `, - BuildBrokenDepfile: proptools.BoolPtr(false), - err: "depfile: Deprecated to ensure the module type is convertible to Bazel", + err: "depfile: Deprecated to ensure the module type is convertible to Bazel", }, { - name: `error when BuildBrokenDepfile is not set`, + name: `no error when module is allowlisted`, prop: ` depfile: true, cmd: "cat $(in) > $(out) && cat $(depfile)", `, - err: "depfile: Deprecated to ensure the module type is convertible to Bazel.", - }, - { - name: `no error when BuildBrokenDepfile is explicitly set to true`, - prop: ` - depfile: true, - cmd: "cat $(in) > $(out) && cat $(depfile)", - `, - BuildBrokenDepfile: proptools.BoolPtr(true), - }, - { - name: `no error if depfile is not set`, - prop: ` - cmd: "cat $(in) > $(out)", - `, + moduleName: `depfile_allowed_for_test`, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { + moduleName := "foo" + if test.moduleName != "" { + moduleName = test.moduleName + } bp := fmt.Sprintf(` gensrcs { - name: "foo", + name: "%s", srcs: ["data.txt"], %s - }`, test.prop) + }`, moduleName, test.prop) var expectedErrors []string if test.err != "" { @@ -682,9 +681,7 @@ func TestGensrcsBuildBrokenDepfile(t *testing.T) { android.GroupFixturePreparers( prepareForGenRuleTest, android.FixtureModifyProductVariables(func(variables android.FixtureProductVariables) { - if test.BuildBrokenDepfile != nil { - variables.BuildBrokenDepfile = test.BuildBrokenDepfile - } + variables.GenruleSandboxing = proptools.BoolPtr(true) }), ). ExtendWithErrorHandler(android.FixtureExpectsAllErrorsToMatchAPattern(expectedErrors)).