Avoid separate call to module factory to create conditional properties

Previously, the soong_config_module_type (and related module types)
would make a special call to the module factory of the module type
being customized to get the properties from which it created the
conditional properties. That caused problems when trying to customize
a singleton module. See the bug for more details.

This change avoids that special call by deferring the creation of the
conditional properties struct until just after the
soong_config_module_type's factory function calls the customized module
factory to create the module and properties. The properties are then
used to create the conditional properties struct avoiding the extra
factory call.

The conditional properties struct is only created once per module type
that soong_config_module_type (and related module types) creates.

Bug: 264876909
Test: m nothing
Change-Id: I55dc71d2553cb59d921a96c6458d0bc877512bbb

android/soong_config_modules.go

@@ -20,7 +20,9 @@ package android
 import (
 	"fmt"
 	"path/filepath"
+	"reflect"
 	"strings"
+	"sync"
 	"text/scanner"
 
 	"github.com/google/blueprint"
@@ -422,13 +424,43 @@ func loadSoongConfigModuleTypeDefinition(ctx LoadHookContext, from string) map[s
 // configModuleFactory takes an existing soongConfigModuleFactory and a
 // ModuleType to create a new ModuleFactory that uses a custom loadhook.
 func configModuleFactory(factory blueprint.ModuleFactory, moduleType *soongconfig.ModuleType, bp2build bool) blueprint.ModuleFactory {
-	conditionalFactoryProps := soongconfig.CreateProperties(factory, moduleType)
-	if !conditionalFactoryProps.IsValid() {
-		return factory
+	// Defer creation of conditional properties struct until the first call from the factory
+	// method. That avoids having to make a special call to the factory to create the properties
+	// structs from which the conditional properties struct is created. This is needed in order to
+	// allow singleton modules to be customized by soong_config_module_type as the
+	// SingletonModuleFactoryAdaptor factory registers a load hook for the singleton module
+	// everytime that it is called. Calling the factory twice causes a build failure as the load
+	// hook is called twice, the first time it updates the singleton module to indicate that it has
+	// been registered as a module, and the second time it fails because it thinks it has been
+	// registered again and a singleton module can only be registered once.
+	//
+	// This is an issue for singleton modules because:
+	// * Load hooks are registered on the module object and are only called when the module object
+	//   is created by Blueprint while processing the Android.bp file.
+	// * The module factory for a singleton module returns the same module object each time it is
+	//   called, and registers its load hook on that same module object.
+	// * When the module factory is called by Blueprint it then calls all the load hooks that have
+	//   been registered for every call to that module factory.
+	//
+	// It is not an issue for normal modules because they return a new module object each time the
+	// factory is called and so any load hooks registered on module objects which are discarded will
+	// not be run.
+	once := &sync.Once{}
+	conditionalFactoryProps := reflect.Value{}
+	getConditionalFactoryProps := func(props []interface{}) reflect.Value {
+		once.Do(func() {
+			conditionalFactoryProps = soongconfig.CreateProperties(props, moduleType)
+		})
+		return conditionalFactoryProps
 	}
 
 	return func() (blueprint.Module, []interface{}) {
 		module, props := factory()
+		conditionalFactoryProps := getConditionalFactoryProps(props)
+		if !conditionalFactoryProps.IsValid() {
+			return module, props
+		}
+
 		conditionalProps := proptools.CloneEmptyProperties(conditionalFactoryProps)
 		props = append(props, conditionalProps.Interface())
 

android/soong_config_modules_test.go

@@ -483,7 +483,7 @@ func TestSoongConfigModuleSingletonModule(t *testing.T) {
 		},
 	} {
 		t.Run(fmt.Sprintf("coyote:%t", test.coyote), func(t *testing.T) {
-			GroupFixturePreparers(
+			result := GroupFixturePreparers(
 				PrepareForTestWithSoongConfigModuleBuildComponents,
 				prepareForSoongConfigTestSingletonModule,
 				FixtureWithRootAndroidBp(bp),
@@ -494,9 +494,12 @@ func TestSoongConfigModuleSingletonModule(t *testing.T) {
 						},
 					}
 				}),
-			).ExtendWithErrorHandler(FixtureExpectsAllErrorsToMatchAPattern([]string{
-				`\QDuplicate SingletonModule "test_singleton", previously used in\E`,
-			})).RunTest(t)
+			).RunTest(t)
+
+			// Make sure that the singleton was created.
+			result.SingletonForTests("test_singleton")
+			m := result.ModuleForTests("wiley", "").module.(*soongConfigTestSingletonModule)
+			AssertStringEquals(t, "fragments", test.expectedFragments, fmt.Sprintf("%+v", m.props.Fragments))
 		})
 	}
 }

android/soongconfig/modules.go

@@ -22,7 +22,6 @@ import (
 	"strings"
 	"sync"
 
-	"github.com/google/blueprint"
 	"github.com/google/blueprint/parser"
 	"github.com/google/blueprint/proptools"
 
@@ -363,10 +362,9 @@ func (defs Bp2BuildSoongConfigDefinitions) String() string {
 //	        },
 //	    },
 //	}
-func CreateProperties(factory blueprint.ModuleFactory, moduleType *ModuleType) reflect.Value {
+func CreateProperties(factoryProps []interface{}, moduleType *ModuleType) reflect.Value {
 	var fields []reflect.StructField
 
-	_, factoryProps := factory()
 	affectablePropertiesType := createAffectablePropertiesType(moduleType.affectableProperties, factoryProps)
 	if affectablePropertiesType == nil {
 		return reflect.Value{}