From d976af0cb47b78a7b4dbc60452d1eff4f0296df6 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Mon, 16 Nov 2020 22:44:30 -0800 Subject: [PATCH] Skip creating variants for disabled OSes The documentation java_genrule_host states that it creates a single variant, which would make it work with the single variant fallback in AddDependency used by the data property, but it actually has a host and a host-cross windows variant. Modify osMutator to take the OS-specific enabled properties into account to skip creating variants that will immediately be disabled so there is a single variant. Test: m checkbuild Change-Id: Ic2daab29f4fa3a3797d7a08348fbfcf1036ec5dc --- android/arch.go | 12 ++++++++++- android/defaults.go | 4 +++- android/module.go | 43 +++++++++++++++++++++++++++++++++----- android/path_properties.go | 2 +- genrule/genrule.go | 3 +++ 5 files changed, 56 insertions(+), 8 deletions(-) diff --git a/android/arch.go b/android/arch.go index afb9c7fc9..a5d416c12 100644 --- a/android/arch.go +++ b/android/arch.go @@ -757,7 +757,7 @@ func osMutator(bpctx blueprint.BottomUpMutatorContext) { for _, os := range OsTypeList { for _, t := range mctx.Config().Targets[os] { - if base.supportsTarget(t) { + if base.supportsTarget(t) && base.osEnabled(os) { moduleOSList = append(moduleOSList, os) break } @@ -1183,6 +1183,16 @@ func InitArchModule(m Module) { } base.archProperties = append(base.archProperties, archProperties) m.AddProperties(archProperties...) + + // Special case the enabled property so the osMutator can skip creating variants that + // are disabled. + if properties == &base.enabledProperties { + if len(archProperties) != 1 { + panic(fmt.Errorf("expected a single arch-specific enabledProperties type, found %d", + len(archProperties))) + } + base.archEnabledProperties = archProperties[0].(*archPropRoot) + } } base.customizableProperties = m.GetProperties() diff --git a/android/defaults.go b/android/defaults.go index eb013d7ab..be5359f12 100644 --- a/android/defaults.go +++ b/android/defaults.go @@ -174,20 +174,22 @@ func (d *DefaultsModuleBase) GenerateAndroidBuildActions(ctx ModuleContext) { func InitDefaultsModule(module DefaultsModule) { commonProperties := &commonProperties{} + enabledProperties := &enabledProperties{} module.AddProperties( &hostAndDeviceProperties{}, + enabledProperties, commonProperties, &ApexProperties{}, &distProperties{}) + base := module.base() initAndroidModuleBase(module) initProductVariableModule(module) InitArchModule(module) InitDefaultableModule(module) // Add properties that will not have defaults applied to them. - base := module.base() defaultsVisibility := &DefaultsVisibilityProperties{} module.AddProperties(&base.nameProperties, defaultsVisibility) diff --git a/android/module.go b/android/module.go index 9bcb22f04..fa2f108b4 100644 --- a/android/module.go +++ b/android/module.go @@ -19,6 +19,7 @@ import ( "os" "path" "path/filepath" + "reflect" "regexp" "strings" "text/scanner" @@ -512,7 +513,7 @@ type nameProperties struct { Name *string } -type commonProperties struct { +type enabledProperties struct { // emit build rules for this module // // Disabling a module should only be done for those modules that cannot be built @@ -521,7 +522,9 @@ type commonProperties struct { // disabled as that will prevent them from being built by the checkbuild target // and so prevent early detection of changes that have broken those modules. Enabled *bool `android:"arch_variant"` +} +type commonProperties struct { // Controls the visibility of this module to other modules. Allowable values are one or more of // these formats: // @@ -830,6 +833,7 @@ func InitAndroidModule(m Module) { m.AddProperties( &base.nameProperties, + &base.enabledProperties, &base.commonProperties, &base.distProperties) @@ -928,6 +932,11 @@ type ModuleBase struct { archProperties [][]interface{} customizableProperties []interface{} + // The enabled property is special-cased so that the osMutator can skip creating variants + // that are disabled. + enabledProperties enabledProperties + archEnabledProperties *archPropRoot + // Information about all the properties on the module that contains visibility rules that need // checking. visibilityPropertyInfo []visibilityProperty @@ -1118,6 +1127,33 @@ func (m *ModuleBase) supportsTarget(target Target) bool { } } +// osEnabled returns true if the given OS is enabled for the current module. +func (m *ModuleBase) osEnabled(os OsType) bool { + targetStruct := reflect.ValueOf(m.archEnabledProperties.Target) + + if targetStruct.Kind() != reflect.Ptr || targetStruct.Type().Elem().Kind() != reflect.Struct { + panic(fmt.Errorf("expected a pointer to a struct, found %s", targetStruct.Type())) + } + + if targetStruct.IsNil() { + return !os.DefaultDisabled + } + + osStruct := targetStruct.Elem().FieldByName(os.Field) + + if targetStruct.Kind() != reflect.Ptr || targetStruct.Type().Elem().Kind() != reflect.Struct { + panic(fmt.Errorf("expected a pointer to a struct, found %s", targetStruct.Type())) + } + + if osStruct.IsNil() { + return !os.DefaultDisabled + } + + enabledField := osStruct.Elem().FieldByName("Enabled") + + return proptools.BoolDefault(enabledField.Interface().(*bool), !os.DefaultDisabled) +} + // DeviceSupported returns true if the current module is supported and enabled for device targets, // i.e. the factory method set the HostOrDeviceSupported value to include device support and // the device support is enabled by default or enabled by the device_supported property. @@ -1217,10 +1253,7 @@ func (m *ModuleBase) Enabled() bool { if m.commonProperties.ForcedDisabled { return false } - if m.commonProperties.Enabled == nil { - return !m.Os().DefaultDisabled - } - return *m.commonProperties.Enabled + return proptools.BoolDefault(m.enabledProperties.Enabled, !m.Os().DefaultDisabled) } func (m *ModuleBase) Disable() { diff --git a/android/path_properties.go b/android/path_properties.go index 6b1cdb308..ee84c676f 100644 --- a/android/path_properties.go +++ b/android/path_properties.go @@ -29,7 +29,7 @@ func registerPathDepsMutator(ctx RegisterMutatorsContext) { // property that is tagged with android:"path". func pathDepsMutator(ctx BottomUpMutatorContext) { m := ctx.Module().(Module) - if m == nil { + if m == nil || !m.Enabled() { return } diff --git a/genrule/genrule.go b/genrule/genrule.go index 53b9dbeb2..978a8ffda 100644 --- a/genrule/genrule.go +++ b/genrule/genrule.go @@ -184,6 +184,9 @@ func (g *Module) GeneratedDeps() android.Paths { func toolDepsMutator(ctx android.BottomUpMutatorContext) { if g, ok := ctx.Module().(*Module); ok { + if !g.Enabled() { + return + } for _, tool := range g.properties.Tools { tag := hostToolDependencyTag{label: tool} if m := android.SrcIsModule(tool); m != "" {