From 02dd6e564078fe7f4088a2f158239260d8acebfa Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Wed, 3 Apr 2024 17:04:57 -0700 Subject: [PATCH 1/2] Add tests for android:replace_instead_of_append Bug: 323382414 Test: m nothing --no-skip-soong-tests Change-Id: Ideb739b3f1a6a5854453db7d51bdee73a3979fd4 --- android/arch.go | 2 +- android/selects_test.go | 98 +++++++++++++++++++++++++++++++++++------ 2 files changed, 86 insertions(+), 14 deletions(-) diff --git a/android/arch.go b/android/arch.go index 9e79e3175..27ce4d464 100644 --- a/android/arch.go +++ b/android/arch.go @@ -980,7 +980,7 @@ func filterArchStruct(field reflect.StructField, prefix string) (bool, reflect.S panic(fmt.Errorf("unexpected tag format %q", field.Tag)) } // these tags don't need to be present in the runtime generated struct type. - values = RemoveListFromList(values, []string{"arch_variant", "variant_prepend", "path"}) + values = RemoveListFromList(values, []string{"arch_variant", "variant_prepend", "path", "replace_instead_of_append"}) if len(values) > 0 { panic(fmt.Errorf("unknown tags %q in field %q", values, prefix+field.Name)) } diff --git a/android/selects_test.go b/android/selects_test.go index a54459c10..ec0847ffe 100644 --- a/android/selects_test.go +++ b/android/selects_test.go @@ -368,14 +368,62 @@ func TestSelects(t *testing.T) { my_bool: proptools.BoolPtr(true), }, }, + { + name: "defaults with lists are appended", + bp: ` + my_module_type { + name: "foo", + defaults: ["bar"], + my_string_list: select(soong_config_variable("my_namespace", "my_variable"), { + "a": ["a1"], + default: ["b1"], + }), + } + my_defaults { + name: "bar", + my_string_list: select(soong_config_variable("my_namespace", "my_variable2"), { + "a": ["a2"], + default: ["b2"], + }), + } + `, + provider: selectsTestProvider{ + my_string_list: &[]string{"b2", "b1"}, + }, + }, + { + name: "Replacing string list", + bp: ` + my_module_type { + name: "foo", + defaults: ["bar"], + replacing_string_list: select(soong_config_variable("my_namespace", "my_variable"), { + "a": ["a1"], + default: ["b1"], + }), + } + my_defaults { + name: "bar", + replacing_string_list: select(soong_config_variable("my_namespace", "my_variable2"), { + "a": ["a2"], + default: ["b2"], + }), + } + `, + provider: selectsTestProvider{ + replacing_string_list: &[]string{"b1"}, + }, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { fixtures := GroupFixturePreparers( + PrepareForTestWithDefaults, PrepareForTestWithArchMutator, FixtureRegisterWithContext(func(ctx RegistrationContext) { ctx.RegisterModuleType("my_module_type", newSelectsMockModule) + ctx.RegisterModuleType("my_defaults", newSelectsMockModuleDefaults) }), FixtureModifyProductVariables(func(variables FixtureProductVariables) { variables.VendorVars = tc.vendorVars @@ -398,10 +446,11 @@ func TestSelects(t *testing.T) { } type selectsTestProvider struct { - my_bool *bool - my_string *string - my_string_list *[]string - my_paths *[]string + my_bool *bool + my_string *string + my_string_list *[]string + my_paths *[]string + replacing_string_list *[]string } func (p *selectsTestProvider) String() string { @@ -418,16 +467,18 @@ func (p *selectsTestProvider) String() string { my_string: %s, my_string_list: %s, my_paths: %s, -}`, myBoolStr, myStringStr, p.my_string_list, p.my_paths) + replacing_string_list %s, +}`, myBoolStr, myStringStr, p.my_string_list, p.my_paths, p.replacing_string_list) } var selectsTestProviderKey = blueprint.NewProvider[selectsTestProvider]() type selectsMockModuleProperties struct { - My_bool proptools.Configurable[bool] - My_string proptools.Configurable[string] - My_string_list proptools.Configurable[[]string] - My_paths proptools.Configurable[[]string] `android:"path"` + My_bool proptools.Configurable[bool] + My_string proptools.Configurable[string] + My_string_list proptools.Configurable[[]string] + My_paths proptools.Configurable[[]string] `android:"path"` + Replacing_string_list proptools.Configurable[[]string] `android:"replace_instead_of_append,arch_variant"` } type selectsMockModule struct { @@ -438,10 +489,11 @@ type selectsMockModule struct { func (p *selectsMockModule) GenerateAndroidBuildActions(ctx ModuleContext) { SetProvider(ctx, selectsTestProviderKey, selectsTestProvider{ - my_bool: p.properties.My_bool.Evaluate(ctx), - my_string: p.properties.My_string.Evaluate(ctx), - my_string_list: p.properties.My_string_list.Evaluate(ctx), - my_paths: p.properties.My_paths.Evaluate(ctx), + my_bool: p.properties.My_bool.Evaluate(ctx), + my_string: p.properties.My_string.Evaluate(ctx), + my_string_list: p.properties.My_string_list.Evaluate(ctx), + my_paths: p.properties.My_paths.Evaluate(ctx), + replacing_string_list: p.properties.Replacing_string_list.Evaluate(ctx), }) } @@ -452,3 +504,23 @@ func newSelectsMockModule() Module { InitDefaultableModule(m) return m } + +type selectsMockModuleDefaults struct { + ModuleBase + DefaultsModuleBase +} + +func (d *selectsMockModuleDefaults) GenerateAndroidBuildActions(ctx ModuleContext) { +} + +func newSelectsMockModuleDefaults() Module { + module := &selectsMockModuleDefaults{} + + module.AddProperties( + &selectsMockModuleProperties{}, + ) + + InitDefaultsModule(module) + + return module +} From b78ce43ae30fbce4e20f93d2b9ca072846f406f3 Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Thu, 4 Apr 2024 10:55:19 -0700 Subject: [PATCH 2/2] Rename Evaluate() to Get() and add GetDefault() Part of the design of property structs is that they were easy to access. In keeping with that spirit, use a shorter and easier to spell name for the getter, and add GetDefault() so that you don't need to pass the result of Get() to one of the proptools.StringDefault/BoolDefault/etc functions. Bug: 323382414 Test: m nothing --no-skip-soong-tests Change-Id: I0b8df151de59f2cd9463425c6666cbfda6bf47f2 --- android/path_properties.go | 4 ++-- android/selects_test.go | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/android/path_properties.go b/android/path_properties.go index ea925654e..6210aee91 100644 --- a/android/path_properties.go +++ b/android/path_properties.go @@ -109,9 +109,9 @@ func pathPropertiesForPropertyStruct(ctx BottomUpMutatorContext, ps interface{}) case reflect.Struct: intf := sv.Interface() if configurable, ok := intf.(proptools.Configurable[string]); ok { - ret = append(ret, proptools.String(configurable.Evaluate(ctx))) + ret = append(ret, configurable.GetOrDefault(ctx, "")) } else if configurable, ok := intf.(proptools.Configurable[[]string]); ok { - ret = append(ret, proptools.Slice(configurable.Evaluate(ctx))...) + ret = append(ret, configurable.GetOrDefault(ctx, nil)...) } else { panic(fmt.Errorf(`field %s in type %s has tag android:"path" but is not a string or slice of strings, it is a %s`, v.Type().FieldByIndex(i).Name, v.Type(), sv.Type())) diff --git a/android/selects_test.go b/android/selects_test.go index ec0847ffe..e59b3e65d 100644 --- a/android/selects_test.go +++ b/android/selects_test.go @@ -489,11 +489,11 @@ type selectsMockModule struct { func (p *selectsMockModule) GenerateAndroidBuildActions(ctx ModuleContext) { SetProvider(ctx, selectsTestProviderKey, selectsTestProvider{ - my_bool: p.properties.My_bool.Evaluate(ctx), - my_string: p.properties.My_string.Evaluate(ctx), - my_string_list: p.properties.My_string_list.Evaluate(ctx), - my_paths: p.properties.My_paths.Evaluate(ctx), - replacing_string_list: p.properties.Replacing_string_list.Evaluate(ctx), + my_bool: p.properties.My_bool.Get(ctx), + my_string: p.properties.My_string.Get(ctx), + my_string_list: p.properties.My_string_list.Get(ctx), + my_paths: p.properties.My_paths.Get(ctx), + replacing_string_list: p.properties.Replacing_string_list.Get(ctx), }) }