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
This commit is contained in:
Colin Cross 2016-04-11 15:41:52 -07:00
parent 97f6b9b61d
commit 910242b9a6
2 changed files with 61 additions and 39 deletions

View file

@ -200,6 +200,9 @@ type goPackage struct {
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 @@ type goPackage struct {
// The bootstrap Config
config *Config
// The stage in which this module should be built
buildStage Stage
}
var _ goPackageProducer = (*goPackage)(nil)
@ -224,9 +224,9 @@ var _ goPackageProducer = (*goPackage)(nil)
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) GoTestTarget() string {
}
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 @@ type goBinary struct {
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 @@ type goBinary struct {
// 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) GoTestTarget() string {
}
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) {

View file

@ -899,6 +899,37 @@ func getStringFromScope(scope *parser.Scope, v string) (string, scanner.Position
}
}
// 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 @@ func (c *Context) createVariations(origModule *moduleInfo, mutatorName string,
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 @@ func (c *Context) createVariations(origModule *moduleInfo, mutatorName string,
// 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 @@ func (c *Context) ResolveDependencies(config interface{}) []error {
return errs
}
c.cloneModules()
c.dependenciesReady = true
return nil
}
@ -1699,6 +1709,18 @@ func (c *Context) runBottomUpMutator(config interface{},
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 {