Clone all modules between mutators and GenerateBuildActions
A common pattern in mutators is to set a property on a module to be used
later in GenerateBuildActions. A common anti-pattern is to set a member
variable instead of a property. Setting member variables will appear to
work until a mutator calls CreateVariations after the member variable is
set. The first variant will have the member variables, but any other
variants will have zero values for all member variables.
To catch this common case early, replace all modules with clones after
running all the mutators but before GenerateBuildActions.
This catches the anti-pattern used in bootstrap, replace the buildStage
member variable with a property.
Change-Id: I6ff37580783a6227ebba2b46d577b443566a79bb
diff --git a/bootstrap/bootstrap.go b/bootstrap/bootstrap.go
index c952c85..c596ac4 100644
--- a/bootstrap/bootstrap.go
+++ b/bootstrap/bootstrap.go
@@ -200,6 +200,9 @@
Srcs []string
TestSrcs []string
PluginFor []string
+
+ // The stage in which this module should be built
+ BuildStage Stage `blueprint:"mutated"`
}
// The root dir in which the package .a file is located. The full .a file
@@ -214,9 +217,6 @@
// The bootstrap Config
config *Config
-
- // The stage in which this module should be built
- buildStage Stage
}
var _ goPackageProducer = (*goPackage)(nil)
@@ -224,9 +224,9 @@
func newGoPackageModuleFactory(config *Config) func() (blueprint.Module, []interface{}) {
return func() (blueprint.Module, []interface{}) {
module := &goPackage{
- buildStage: StagePrimary,
- config: config,
+ config: config,
}
+ module.properties.BuildStage = StagePrimary
return module, []interface{}{&module.properties}
}
}
@@ -248,11 +248,11 @@
}
func (g *goPackage) BuildStage() Stage {
- return g.buildStage
+ return g.properties.BuildStage
}
func (g *goPackage) SetBuildStage(buildStage Stage) {
- g.buildStage = buildStage
+ g.properties.BuildStage = buildStage
}
func (g *goPackage) IsPluginFor(name string) bool {
@@ -326,6 +326,9 @@
Srcs []string
TestSrcs []string
PrimaryBuilder bool
+
+ // The stage in which this module should be built
+ BuildStage Stage `blueprint:"mutated"`
}
// The path of the test .a file that is to be built.
@@ -333,17 +336,14 @@
// The bootstrap Config
config *Config
-
- // The stage in which this module should be built
- buildStage Stage
}
func newGoBinaryModuleFactory(config *Config, buildStage Stage) func() (blueprint.Module, []interface{}) {
return func() (blueprint.Module, []interface{}) {
module := &goBinary{
- config: config,
- buildStage: buildStage,
+ config: config,
}
+ module.properties.BuildStage = buildStage
return module, []interface{}{&module.properties}
}
}
@@ -353,11 +353,11 @@
}
func (g *goBinary) BuildStage() Stage {
- return g.buildStage
+ return g.properties.BuildStage
}
func (g *goBinary) SetBuildStage(buildStage Stage) {
- g.buildStage = buildStage
+ g.properties.BuildStage = buildStage
}
func (g *goBinary) GenerateBuildActions(ctx blueprint.ModuleContext) {
diff --git a/context.go b/context.go
index ca3f919..0dc84a2 100644
--- a/context.go
+++ b/context.go
@@ -899,6 +899,37 @@
}
}
+// Clones a build logic module by calling the factory method for its module type, and then cloning
+// property values. Any values stored in the module object that are not stored in properties
+// structs will be lost.
+func (c *Context) cloneLogicModule(origModule *moduleInfo) (Module, []interface{}) {
+ typeName := origModule.typeName
+ factory, ok := c.moduleFactories[typeName]
+ if !ok {
+ panic(fmt.Sprintf("unrecognized module type %q during cloning", typeName))
+ }
+
+ props := []interface{}{
+ &origModule.properties,
+ }
+ newLogicModule, newProperties := factory()
+
+ newProperties = append(props, newProperties...)
+
+ if len(newProperties) != len(origModule.moduleProperties) {
+ panic("mismatched properties array length in " + origModule.properties.Name)
+ }
+
+ for i := range newProperties {
+ dst := reflect.ValueOf(newProperties[i]).Elem()
+ src := reflect.ValueOf(origModule.moduleProperties[i]).Elem()
+
+ proptools.CopyProperties(dst, src)
+ }
+
+ return newLogicModule, newProperties
+}
+
func (c *Context) createVariations(origModule *moduleInfo, mutatorName string,
variationNames []string) ([]*moduleInfo, []error) {
@@ -912,12 +943,6 @@
var errs []error
for i, variationName := range variationNames {
- typeName := origModule.typeName
- factory, ok := c.moduleFactories[typeName]
- if !ok {
- panic(fmt.Sprintf("unrecognized module type %q during cloning", typeName))
- }
-
var newLogicModule Module
var newProperties []interface{}
@@ -925,26 +950,9 @@
// Reuse the existing module for the first new variant
// This both saves creating a new module, and causes the insertion in c.moduleInfo below
// with logicModule as the key to replace the original entry in c.moduleInfo
- newLogicModule = origModule.logicModule
- newProperties = origModule.moduleProperties
+ newLogicModule, newProperties = origModule.logicModule, origModule.moduleProperties
} else {
- props := []interface{}{
- &origModule.properties,
- }
- newLogicModule, newProperties = factory()
-
- newProperties = append(props, newProperties...)
-
- if len(newProperties) != len(origModule.moduleProperties) {
- panic("mismatched properties array length in " + origModule.properties.Name)
- }
-
- for i := range newProperties {
- dst := reflect.ValueOf(newProperties[i]).Elem()
- src := reflect.ValueOf(origModule.moduleProperties[i]).Elem()
-
- proptools.CopyProperties(dst, src)
- }
+ newLogicModule, newProperties = c.cloneLogicModule(origModule)
}
newVariant := origModule.variant.clone()
@@ -1116,6 +1124,8 @@
return errs
}
+ c.cloneModules()
+
c.dependenciesReady = true
return nil
}
@@ -1699,6 +1709,18 @@
return errs
}
+// Replaces every build logic module with a clone of itself. Prevents introducing problems where
+// a mutator sets a non-property member variable on a module, which works until a later mutator
+// creates variants of that module.
+func (c *Context) cloneModules() {
+ for _, m := range c.modulesSorted {
+ origLogicModule := m.logicModule
+ m.logicModule, m.moduleProperties = c.cloneLogicModule(m)
+ delete(c.moduleInfo, origLogicModule)
+ c.moduleInfo[m.logicModule] = m
+ }
+}
+
func spliceModules(modules []*moduleInfo, origModule *moduleInfo,
newModules []*moduleInfo) []*moduleInfo {
for i, m := range modules {
