From 6134a5c66a32a15c48ef9f760e756febde0335e5 Mon Sep 17 00:00:00 2001
From: Colin Cross <ccross@android.com>
Date: Tue, 10 Feb 2015 11:26:26 -0800
Subject: [PATCH] Relax module naming restrictions

Forcing module names to be valid ninja names is an unnecessary
restraint on the project build logic.  Allow any string as a
module name, and sanitize and uniquify the module name for use
in module-scoped variables.

Also move the module scope to be per-module instead of per-group
so that modules can use the same local variable name for each variant.

Change-Id: If44cca20712305e2c0b6d6b39daa5eace335c148
---
 context.go       | 45 +++++++++++++++++++++++++--------------------
 doc.go           |  4 ++--
 ninja_strings.go | 21 +++++++++++++++++++++
 3 files changed, 48 insertions(+), 22 deletions(-)

diff --git a/context.go b/context.go
index 0062e36..96946d4 100644
--- a/context.go
+++ b/context.go
@@ -26,6 +26,7 @@ import (
 	"reflect"
 	"runtime"
 	"sort"
+	"strconv"
 	"strings"
 	"text/scanner"
 	"text/template"
@@ -67,6 +68,7 @@ type Context struct {
 	moduleGroupsSorted []*moduleGroup
 	singletonInfo      map[string]*singletonInfo
 	mutatorInfo        []*mutatorInfo
+	moduleNinjaNames   map[string]*moduleGroup
 
 	dependenciesReady bool // set to true on a successful ResolveDependencies
 	buildActionsReady bool // set to true on a successful PrepareBuildActions
@@ -106,6 +108,7 @@ type localBuildActions struct {
 type moduleGroup struct {
 	// set during Parse
 	typeName          string
+	ninjaName         string
 	relBlueprintsFile string
 	pos               scanner.Position
 	propertyPos       map[string]scanner.Position
@@ -182,10 +185,11 @@ func (e *Error) Error() string {
 // useful.
 func NewContext() *Context {
 	return &Context{
-		moduleFactories: make(map[string]ModuleFactory),
-		moduleGroups:    make(map[string]*moduleGroup),
-		moduleInfo:      make(map[Module]*moduleInfo),
-		singletonInfo:   make(map[string]*singletonInfo),
+		moduleFactories:  make(map[string]ModuleFactory),
+		moduleGroups:     make(map[string]*moduleGroup),
+		moduleInfo:       make(map[Module]*moduleInfo),
+		singletonInfo:    make(map[string]*singletonInfo),
+		moduleNinjaNames: make(map[string]*moduleGroup),
 	}
 }
 
@@ -790,23 +794,23 @@ func (c *Context) processModuleDef(moduleDef *parser.Module,
 		return nil, errs
 	}
 
+	ninjaName := toNinjaName(group.properties.Name)
+
+	// The sanitizing in toNinjaName can result in collisions, uniquify the name if it
+	// already exists
+	for i := 0; c.moduleNinjaNames[ninjaName] != nil; i++ {
+		ninjaName = toNinjaName(group.properties.Name) + strconv.Itoa(i)
+	}
+
+	c.moduleNinjaNames[ninjaName] = group
+	group.ninjaName = ninjaName
+
 	group.pos = moduleDef.Type.Pos
 	group.propertyPos = make(map[string]scanner.Position)
 	for name, propertyDef := range propertyMap {
 		group.propertyPos[name] = propertyDef.Pos
 	}
 
-	name := group.properties.Name
-	err := validateNinjaName(name)
-	if err != nil {
-		return nil, []error{
-			&Error{
-				Err: fmt.Errorf("invalid module name %q: %s", err),
-				Pos: group.propertyPos["name"],
-			},
-		}
-	}
-
 	module := &moduleInfo{
 		group:            group,
 		logicModule:      logicModule,
@@ -1317,12 +1321,13 @@ func (c *Context) generateModuleBuildActions(config interface{},
 	}()
 
 	c.parallelVisitAllBottomUp(func(group *moduleGroup) bool {
-		// The parent scope of the moduleContext's local scope gets overridden to be that of the
-		// calling Go package on a per-call basis.  Since the initial parent scope doesn't matter we
-		// just set it to nil.
-		scope := newLocalScope(nil, moduleNamespacePrefix(group.properties.Name))
-
 		for _, module := range group.modules {
+			// The parent scope of the moduleContext's local scope gets overridden to be that of the
+			// calling Go package on a per-call basis.  Since the initial parent scope doesn't matter we
+			// just set it to nil.
+			prefix := moduleNamespacePrefix(group.ninjaName + "_" + module.subName())
+			scope := newLocalScope(nil, prefix)
+
 			mctx := &moduleContext{
 				baseModuleContext: baseModuleContext{
 					context: c,
diff --git a/doc.go b/doc.go
index 2758dda..23968cb 100644
--- a/doc.go
+++ b/doc.go
@@ -13,7 +13,7 @@
 // limitations under the License.
 
 // Blueprint is a meta-build system that reads in Blueprints files that describe
-// modules that need to be built, and produces a Ninja 
+// modules that need to be built, and produces a Ninja
 // (http://martine.github.io/ninja/) manifest describing the commands that need
 // to be run and their dependencies.  Where most build systems use built-in
 // rules or a domain-specific langauge to describe the logic how modules are
@@ -25,7 +25,7 @@
 //
 // Blueprint uses a bootstrapping process to allow the code for Blueprint,
 // the code for the build logic, and the code for the project being compiled
-// to all live in the project.  Dependencies between the layers are fully 
+// to all live in the project.  Dependencies between the layers are fully
 // tracked - a change to the logic code will cause the logic to be recompiled,
 // regenerate the project build manifest, and run modified project rules.  A
 // change to Blueprint itself will cause Blueprint to rebuild, and then rebuild
diff --git a/ninja_strings.go b/ninja_strings.go
index 4cd7901..950518d 100644
--- a/ninja_strings.go
+++ b/ninja_strings.go
@@ -15,6 +15,7 @@
 package blueprint
 
 import (
+	"bytes"
 	"fmt"
 	"strings"
 )
@@ -288,6 +289,26 @@ func validateNinjaName(name string) error {
 	return nil
 }
 
+func toNinjaName(name string) string {
+	ret := bytes.Buffer{}
+	ret.Grow(len(name))
+	for _, r := range name {
+		valid := (r >= 'a' && r <= 'z') ||
+			(r >= 'A' && r <= 'Z') ||
+			(r >= '0' && r <= '9') ||
+			(r == '_') ||
+			(r == '-') ||
+			(r == '.')
+		if valid {
+			ret.WriteRune(r)
+		} else {
+			ret.WriteRune('_')
+		}
+	}
+
+	return ret.String()
+}
+
 var builtinRuleArgs = []string{"out", "in"}
 
 func validateArgName(argName string) error {