Snap for 5735642 from d6d76f814ed3bbfaf6681289d5e5763de7e427fc to sdk-release

Change-Id: Ib5b972cf279535d399f5cf930dfe2a6bf72ed4da
diff --git a/.travis.yml b/.travis.yml
index 6abd686..c39ad38 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,7 +1,6 @@
 language: go
 
 go:
-    - 1.9
     - "1.10"
     - "1.11"
     - "1.12"
diff --git a/Blueprints b/Blueprints
index 35e4190..d58169e 100644
--- a/Blueprints
+++ b/Blueprints
@@ -125,6 +125,8 @@
         "bootstrap/bpdoc/reader.go",
     ],
     testSrcs: [
+        "bootstrap/bpdoc/bpdoc_test.go",
+        "bootstrap/bpdoc/properties_test.go",
         "bootstrap/bpdoc/reader_test.go",
     ],
 }
diff --git a/bootstrap/bootstrap.go b/bootstrap/bootstrap.go
index 3464c0b..c5d4899 100644
--- a/bootstrap/bootstrap.go
+++ b/bootstrap/bootstrap.go
@@ -130,6 +130,7 @@
 			Deps:        blueprint.DepsGCC,
 			Depfile:     "$out.d",
 			Restat:      true,
+			Pool:        blueprint.Console,
 		},
 		"builder", "extra", "generator", "globFile")
 
diff --git a/bootstrap/bpdoc/bpdoc.go b/bootstrap/bpdoc/bpdoc.go
index 8ce41cf..4acfc5d 100644
--- a/bootstrap/bpdoc/bpdoc.go
+++ b/bootstrap/bpdoc/bpdoc.go
@@ -173,6 +173,9 @@
 				// The field is not exported so just skip it.
 				continue
 			}
+			if proptools.HasTag(field, "blueprint", "mutated") {
+				continue
+			}
 
 			fieldValue := structValue.Field(i)
 
diff --git a/bootstrap/bpdoc/bpdoc_test.go b/bootstrap/bpdoc/bpdoc_test.go
new file mode 100644
index 0000000..687d97b
--- /dev/null
+++ b/bootstrap/bpdoc/bpdoc_test.go
@@ -0,0 +1,46 @@
+package bpdoc
+
+import (
+	"reflect"
+	"testing"
+)
+
+type parentProps struct {
+	A string
+
+	Child *childProps
+
+	Mutated *mutatedProps `blueprint:"mutated"`
+}
+
+type childProps struct {
+	B int
+
+	Child *grandchildProps
+}
+
+type grandchildProps struct {
+	C bool
+}
+
+type mutatedProps struct {
+	D int
+}
+
+func TestNestedPropertyStructs(t *testing.T) {
+	parent := parentProps{Child: &childProps{Child: &grandchildProps{}}, Mutated: &mutatedProps{}}
+
+	allStructs := nestedPropertyStructs(reflect.ValueOf(parent))
+
+	// mutated shouldn't be found because it's a mutated property.
+	expected := []string{"child", "child.child"}
+	if len(allStructs) != len(expected) {
+		t.Errorf("expected %d structs, got %d, all entries: %q",
+			len(expected), len(allStructs), allStructs)
+	}
+	for _, e := range expected {
+		if _, ok := allStructs[e]; !ok {
+			t.Errorf("missing entry %q, all entries: %q", e, allStructs)
+		}
+	}
+}
diff --git a/bootstrap/bpdoc/properties.go b/bootstrap/bpdoc/properties.go
index 23b1ffd..9256d8e 100644
--- a/bootstrap/bpdoc/properties.go
+++ b/bootstrap/bpdoc/properties.go
@@ -250,17 +250,23 @@
 	// len(props) times to this slice will overwrite the original slice contents
 	filtered := (*props)[:0]
 	for _, x := range *props {
-		tag := x.Tag.Get(key)
-		for _, entry := range strings.Split(tag, ",") {
-			if (entry == value) == !exclude {
-				filtered = append(filtered, x)
-			}
+		if hasTag(x.Tag, key, value) == !exclude {
+			filtered = append(filtered, x)
 		}
 	}
 
 	*props = filtered
 }
 
+func hasTag(tag reflect.StructTag, key, value string) bool {
+	for _, entry := range strings.Split(tag.Get(key), ",") {
+		if entry == value {
+			return true
+		}
+	}
+	return false
+}
+
 func formatText(text string) template.HTML {
 	var html template.HTML
 	lines := strings.Split(text, "\n")
diff --git a/bootstrap/bpdoc/properties_test.go b/bootstrap/bpdoc/properties_test.go
new file mode 100644
index 0000000..4045cb1
--- /dev/null
+++ b/bootstrap/bpdoc/properties_test.go
@@ -0,0 +1,58 @@
+// Copyright 2019 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package bpdoc
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestExcludeByTag(t *testing.T) {
+	r := NewReader(pkgFiles)
+	ps, err := r.PropertyStruct(pkgPath, "tagTestProps", reflect.ValueOf(tagTestProps{}))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	ps.ExcludeByTag("tag1", "a")
+
+	expected := []string{"c"}
+	actual := []string{}
+	for _, p := range ps.Properties {
+		actual = append(actual, p.Name)
+	}
+	if !reflect.DeepEqual(expected, actual) {
+		t.Errorf("unexpected ExcludeByTag result, expected: %q, actual: %q", expected, actual)
+	}
+}
+
+func TestIncludeByTag(t *testing.T) {
+	r := NewReader(pkgFiles)
+	ps, err := r.PropertyStruct(pkgPath, "tagTestProps", reflect.ValueOf(tagTestProps{A: "B"}))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	ps.IncludeByTag("tag1", "c")
+
+	expected := []string{"b", "c"}
+	actual := []string{}
+	for _, p := range ps.Properties {
+		actual = append(actual, p.Name)
+	}
+	if !reflect.DeepEqual(expected, actual) {
+		t.Errorf("unexpected IncludeByTag result, expected: %q, actual: %q", expected, actual)
+	}
+}
diff --git a/bootstrap/bpdoc/reader_test.go b/bootstrap/bpdoc/reader_test.go
index b8ff109..0d608b3 100644
--- a/bootstrap/bpdoc/reader_test.go
+++ b/bootstrap/bpdoc/reader_test.go
@@ -34,6 +34,13 @@
 	A string
 }
 
+// for properties_test.go
+type tagTestProps struct {
+	A string `tag1:"a,b" tag2:"c"`
+	B string `tag1:"a,c"`
+	C string `tag1:"b,c"`
+}
+
 var pkgPath string
 var pkgFiles map[string][]string
 
diff --git a/bootstrap/bpglob/bpglob.go b/bootstrap/bpglob/bpglob.go
index 1097760..fe47b6f 100644
--- a/bootstrap/bpglob/bpglob.go
+++ b/bootstrap/bpglob/bpglob.go
@@ -21,7 +21,9 @@
 import (
 	"flag"
 	"fmt"
+	"io/ioutil"
 	"os"
+	"time"
 
 	"github.com/google/blueprint/pathtools"
 )
@@ -71,7 +73,15 @@
 
 	_, err := pathtools.GlobWithDepFile(flag.Arg(0), *out, *out+".d", excludes)
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "error: %s\n", err.Error())
-		os.Exit(1)
+		// Globs here were already run in the primary builder without error.  The only errors here should be if the glob
+		// pattern was made invalid by a change in the pathtools glob implementation, in which case the primary builder
+		// needs to be rerun anyways.  Update the output file with something that will always cause the primary builder
+		// to rerun.
+		s := fmt.Sprintf("%s: error: %s\n", time.Now().Format(time.StampNano), err.Error())
+		err := ioutil.WriteFile(*out, []byte(s), 0666)
+		if err != nil {
+			fmt.Fprintf(os.Stderr, "error: %s\n", err.Error())
+			os.Exit(1)
+		}
 	}
 }
diff --git a/bpfmt/bpfmt.go b/bpfmt/bpfmt.go
index 17fe513..c287ea2 100644
--- a/bpfmt/bpfmt.go
+++ b/bpfmt/bpfmt.go
@@ -125,6 +125,7 @@
 }
 
 func main() {
+	flag.Usage = usage
 	flag.Parse()
 
 	if !*writeToStout && !*overwriteSourceFile && !*doDiff && !*list {
@@ -135,8 +136,7 @@
 		// file to parse is stdin
 		if *overwriteSourceFile {
 			fmt.Fprintln(os.Stderr, "error: cannot use -w with standard input")
-			exitCode = 2
-			return
+			os.Exit(2)
 		}
 		if err := processReader("<standard input>", os.Stdin, os.Stdout); err != nil {
 			report(err)
@@ -157,6 +157,8 @@
 			}
 		}
 	}
+
+	os.Exit(exitCode)
 }
 
 func diff(b1, b2 []byte) (data []byte, err error) {
diff --git a/context.go b/context.go
index 324d1ec..6a61861 100644
--- a/context.go
+++ b/context.go
@@ -173,6 +173,7 @@
 	relBlueprintsFile string
 	pos               scanner.Position
 	propertyPos       map[string]scanner.Position
+	createdBy         *moduleInfo
 
 	variantName       string
 	variant           variationMap
@@ -183,12 +184,13 @@
 	properties  []interface{}
 
 	// set during ResolveDependencies
-	directDeps  []depInfo
-	missingDeps []string
+	missingDeps   []string
+	newDirectDeps []depInfo
 
 	// set during updateDependencies
 	reverseDeps []*moduleInfo
 	forwardDeps []*moduleInfo
+	directDeps  []depInfo
 
 	// used by parallelVisitAllBottomUp
 	waitingCount int
@@ -214,6 +216,10 @@
 	if module.variantName != "" {
 		s += fmt.Sprintf(" variant %q", module.variantName)
 	}
+	if module.createdBy != nil {
+		s += fmt.Sprintf(" (created by %s)", module.createdBy)
+	}
+
 	return s
 }
 
@@ -1427,7 +1433,7 @@
 	}
 
 	if m := c.findMatchingVariant(module, possibleDeps); m != nil {
-		module.directDeps = append(module.directDeps, depInfo{m, tag})
+		module.newDirectDeps = append(module.newDirectDeps, depInfo{m, tag})
 		atomic.AddUint32(&c.depsModified, 1)
 		return nil
 	}
@@ -1530,7 +1536,7 @@
 					Pos: module.pos,
 				}}
 			}
-			module.directDeps = append(module.directDeps, depInfo{m, tag})
+			module.newDirectDeps = append(module.newDirectDeps, depInfo{m, tag})
 			atomic.AddUint32(&c.depsModified, 1)
 			return nil
 		}
@@ -1575,7 +1581,7 @@
 			origModule.Name()))
 	}
 
-	fromInfo.directDeps = append(fromInfo.directDeps, depInfo{toInfo, tag})
+	fromInfo.newDirectDeps = append(fromInfo.newDirectDeps, depInfo{toInfo, tag})
 	atomic.AddUint32(&c.depsModified, 1)
 }
 
@@ -2149,9 +2155,18 @@
 					module.directDeps[j].module = dep.module.splitModules[0]
 				}
 			}
+
+			if module.createdBy != nil && module.createdBy.logicModule == nil {
+				module.createdBy = module.createdBy.splitModules[0]
+			}
+
+			// Add in any new direct dependencies that were added by the mutator
+			module.directDeps = append(module.directDeps, module.newDirectDeps...)
+			module.newDirectDeps = nil
 		}
 	}
 
+	// Add in any new reverse dependencies that were added by the mutator
 	for module, deps := range reverseDeps {
 		sort.Sort(depSorter(deps))
 		module.directDeps = append(module.directDeps, deps...)
diff --git a/doc.go b/doc.go
index f489bfd..4a9dfd7 100644
--- a/doc.go
+++ b/doc.go
@@ -14,7 +14,7 @@
 
 // Blueprint is a meta-build system that reads in Blueprints files that describe
 // modules that need to be built, and produces a Ninja
-// (http://martine.github.io/ninja/) manifest describing the commands that need
+// (https://ninja-build.org/) manifest describing the commands that need
 // to be run and their dependencies.  Where most build systems use built-in
 // rules or a domain-specific language to describe the logic how modules are
 // converted to build rules, Blueprint delegates this to per-project build logic
diff --git a/module_ctx.go b/module_ctx.go
index d127c0e..37079ad 100644
--- a/module_ctx.go
+++ b/module_ctx.go
@@ -121,15 +121,39 @@
 }
 
 type BaseModuleContext interface {
+	// Module returns the current module as a Module.  It should rarely be necessary, as the module already has a
+	// reference to itself.
+	Module() Module
+
+	// ModuleName returns the name of the module.  This is generally the value that was returned by Module.Name() when
+	// the module was created, but may have been modified by calls to BaseMutatorContext.Rename.
 	ModuleName() string
+
+	// ModuleDir returns the path to the directory that contains the defintion of the module.
 	ModuleDir() string
+
+	// ModuleType returns the name of the module type that was used to create the module, as specified in
+	// RegisterModuleType.
 	ModuleType() string
+
+	// Config returns the config object that was passed to Context.PrepareBuildActions.
 	Config() interface{}
 
+	// ContainsProperty returns true if the specified property name was set in the module definition.
 	ContainsProperty(name string) bool
+
+	// Errorf reports an error at the specified position of the module definition file.
 	Errorf(pos scanner.Position, fmt string, args ...interface{})
+
+	// ModuleErrorf reports an error at the line number of the module type in the module definition.
 	ModuleErrorf(fmt string, args ...interface{})
+
+	// PropertyErrorf reports an error at the line number of a property in the module definition.
 	PropertyErrorf(property, fmt string, args ...interface{})
+
+	// Failed returns true if any errors have been reported.  In most cases the module can continue with generating
+	// build rules after an error, allowing it to report additional errors in a single run, but in cases where the error
+	// has prevented the module from creating necessary data it can return early when Failed returns true.
 	Failed() bool
 
 	// GlobWithDeps returns a list of files and directories that match the
@@ -140,13 +164,103 @@
 	// does not match the pattern is added to a searched directory.
 	GlobWithDeps(pattern string, excludes []string) ([]string, error)
 
+	// Fs returns a pathtools.Filesystem that can be used to interact with files.  Using the Filesystem interface allows
+	// the module to be used in build system tests that run against a mock filesystem.
 	Fs() pathtools.FileSystem
+
+	// AddNinjaFileDeps adds dependencies on the specified files to the rule that creates the ninja manifest.  The
+	// primary builder will be rerun whenever the specified files are modified.
 	AddNinjaFileDeps(deps ...string)
 
 	moduleInfo() *moduleInfo
 	error(err error)
 
+	// Namespace returns the Namespace object provided by the NameInterface set by Context.SetNameInterface, or the
+	// default SimpleNameInterface if Context.SetNameInterface was not called.
 	Namespace() Namespace
+
+	// GetDirectDepWithTag returns the Module the direct dependency with the specified name, or nil if
+	// none exists.  It panics if the dependency does not have the specified tag.
+	GetDirectDepWithTag(name string, tag DependencyTag) Module
+
+	// GetDirectDep returns the Module and DependencyTag for the  direct dependency with the specified
+	// name, or nil if none exists.  If there are multiple dependencies on the same module it returns
+	// the first DependencyTag.
+	GetDirectDep(name string) (Module, DependencyTag)
+
+	// VisitDirectDeps calls visit for each direct dependency.  If there are multiple direct dependencies on the same
+	// module visit will be called multiple times on that module and OtherModuleDependencyTag will return a different
+	// tag for each.
+	//
+	// The Module passed to the visit function should not be retained outside of the visit function, it may be
+	// invalidated by future mutators.
+	VisitDirectDeps(visit func(Module))
+
+	// VisitDirectDepsIf calls pred for each direct dependency, and if pred returns true calls visit.  If there are
+	// multiple direct dependencies on the same module pred and visit will be called multiple times on that module and
+	// OtherModuleDependencyTag will return a different tag for each.
+	//
+	// The Module passed to the visit function should not be retained outside of the visit function, it may be
+	// invalidated by future mutators.
+	VisitDirectDepsIf(pred func(Module) bool, visit func(Module))
+
+	// VisitDepsDepthFirst calls visit for each transitive dependency, traversing the dependency tree in depth first
+	// order. visit will only be called once for any given module, even if there are multiple paths through the
+	// dependency tree to the module or multiple direct dependencies with different tags.  OtherModuleDependencyTag will
+	// return the tag for the first path found to the module.
+	//
+	// The Module passed to the visit function should not be retained outside of the visit function, it may be
+	// invalidated by future mutators.
+	VisitDepsDepthFirst(visit func(Module))
+
+	// VisitDepsDepthFirst calls pred for each transitive dependency, and if pred returns true calls visit, traversing
+	// the dependency tree in depth first order.  visit will only be called once for any given module, even if there are
+	// multiple paths through the dependency tree to the module or multiple direct dependencies with different tags.
+	// OtherModuleDependencyTag will return the tag for the first path found to the module.  The return value of pred
+	// does not affect which branches of the tree are traversed.
+	//
+	// The Module passed to the visit function should not be retained outside of the visit function, it may be
+	// invalidated by future mutators.
+	VisitDepsDepthFirstIf(pred func(Module) bool, visit func(Module))
+
+	// WalkDeps calls visit for each transitive dependency, traversing the dependency tree in top down order.  visit may
+	// be called multiple times for the same (child, parent) pair if there are multiple direct dependencies between the
+	// child and parent with different tags.  OtherModuleDependencyTag will return the tag for the currently visited
+	// (child, parent) pair.  If visit returns false WalkDeps will not continue recursing down to child.
+	//
+	// The Modules passed to the visit function should not be retained outside of the visit function, they may be
+	// invalidated by future mutators.
+	WalkDeps(visit func(Module, Module) bool)
+
+	// OtherModuleName returns the name of another Module.  See BaseModuleContext.ModuleName for more information.
+	// It is intended for use inside the visit functions of Visit* and WalkDeps.
+	OtherModuleName(m Module) string
+
+	// OtherModuleDir returns the directory of another Module.  See BaseModuleContext.ModuleDir for more information.
+	// It is intended for use inside the visit functions of Visit* and WalkDeps.
+	OtherModuleDir(m Module) string
+
+	// OtherModuleSubDir returns the unique subdirectory name of another Module.  See ModuleContext.ModuleSubDir for
+	// more information.
+	// It is intended for use inside the visit functions of Visit* and WalkDeps.
+	OtherModuleSubDir(m Module) string
+
+	// OtherModuleType returns the type of another Module.  See BaseModuleContext.ModuleType for more information.
+	// It is intended for use inside the visit functions of Visit* and WalkDeps.
+	OtherModuleType(m Module) string
+
+	// OtherModuleErrorf reports an error on another Module.  See BaseModuleContext.ModuleErrorf for more information.
+	// It is intended for use inside the visit functions of Visit* and WalkDeps.
+	OtherModuleErrorf(m Module, fmt string, args ...interface{})
+
+	// OtherModuleDependencyTag returns the dependency tag used to depend on a module, or nil if there is no dependency
+	// on the module.  When called inside a Visit* method with current module being visited, and there are multiple
+	// dependencies on the module being visited, it returns the dependency tag used for the current dependency.
+	OtherModuleDependencyTag(m Module) DependencyTag
+
+	// OtherModuleExists returns true if a module with the specified name exists, as determined by the NameInterface
+	// passed to Context.SetNameInterface, or SimpleNameInterface if it was not called.
+	OtherModuleExists(name string) bool
 }
 
 type DynamicDependerModuleContext BottomUpMutatorContext
@@ -154,32 +268,41 @@
 type ModuleContext interface {
 	BaseModuleContext
 
-	OtherModuleName(m Module) string
-	OtherModuleDir(m Module) string
-	OtherModuleSubDir(m Module) string
-	OtherModuleType(m Module) string
-	OtherModuleErrorf(m Module, fmt string, args ...interface{})
-	OtherModuleDependencyTag(m Module) DependencyTag
-
-	GetDirectDepWithTag(name string, tag DependencyTag) Module
-	GetDirectDep(name string) (Module, DependencyTag)
-
-	VisitDirectDeps(visit func(Module))
-	VisitDirectDepsIf(pred func(Module) bool, visit func(Module))
-	VisitDepsDepthFirst(visit func(Module))
-	VisitDepsDepthFirstIf(pred func(Module) bool, visit func(Module))
-	WalkDeps(visit func(child, parent Module) bool)
-
+	// ModuleSubDir returns a unique name for the current variant of a module that can be used as part of the path
+	// to ensure that each variant of a module gets its own intermediates directory to write to.
 	ModuleSubDir() string
 
+	// Variable creates a new ninja variable scoped to the module.  It can be referenced by calls to Rule and Build
+	// in the same module.
 	Variable(pctx PackageContext, name, value string)
+
+	// Rule creates a new ninja rule scoped to the module.  It can be referenced by calls to Build in the same module.
 	Rule(pctx PackageContext, name string, params RuleParams, argNames ...string) Rule
+
+	// Build creates a new ninja build statement.
 	Build(pctx PackageContext, params BuildParams)
 
+	// PrimaryModule returns the first variant of the current module.  Variants of a module are always visited in
+	// order by mutators and GenerateBuildActions, so the data created by the current mutator can be read from the
+	// Module returned by PrimaryModule without data races.  This can be used to perform singleton actions that are
+	// only done once for all variants of a module.
 	PrimaryModule() Module
+
+	// FinalModule returns the last variant of the current module.  Variants of a module are always visited in
+	// order by mutators and GenerateBuildActions, so the data created by the current mutator can be read from all
+	// variants using VisitAllModuleVariants if the current module == FinalModule().  This can be used to perform
+	// singleton actions that are only done once for all variants of a module.
 	FinalModule() Module
+
+	// VisitAllModuleVariants calls visit for each variant of the current module.  Variants of a module are always
+	// visited in order by mutators and GenerateBuildActions, so the data created by the current mutator can be read
+	// from all variants if the current module == FinalModule().  Otherwise, care must be taken to not access any
+	// data modified by the current mutator.
 	VisitAllModuleVariants(visit func(Module))
 
+	// GetMissingDependencies returns the list of dependencies that were passed to AddDependencies or related methods,
+	// but do not exist.  It can be used with Context.SetAllowMissingDependencies to allow the primary builder to
+	// handle missing dependencies on its own instead of having Blueprint treat them as an error.
 	GetMissingDependencies() []string
 }
 
@@ -199,6 +322,10 @@
 	return d.module
 }
 
+func (d *baseModuleContext) Module() Module {
+	return d.module.logicModule
+}
+
 func (d *baseModuleContext) ModuleName() string {
 	return d.module.Name()
 }
@@ -327,9 +454,6 @@
 	})
 }
 
-// OtherModuleDependencyTag returns the dependency tag used to depend on a module, or nil if there is no dependency
-// on the module.  When called inside a Visit* method with current module being visited, and there are multiple
-// dependencies on the module being visited, it returns the dependency tag used for the current dependency.
 func (m *baseModuleContext) OtherModuleDependencyTag(logicModule Module) DependencyTag {
 	// fast path for calling OtherModuleDependencyTag from inside VisitDirectDeps
 	if logicModule == m.visitingDep.module.logicModule {
@@ -345,9 +469,11 @@
 	return nil
 }
 
-// GetDirectDep returns the Module and DependencyTag for the  direct dependency with the specified
-// name, or nil if none exists.  If there are multiple dependencies on the same module it returns
-// the first DependencyTag.
+func (m *baseModuleContext) OtherModuleExists(name string) bool {
+	_, exists := m.context.nameInterface.ModuleFromName(name, m.module.namespace())
+	return exists
+}
+
 func (m *baseModuleContext) GetDirectDep(name string) (Module, DependencyTag) {
 	for _, dep := range m.module.directDeps {
 		if dep.module.Name() == name {
@@ -358,8 +484,6 @@
 	return nil, nil
 }
 
-// GetDirectDepWithTag returns the Module the direct dependency with the specified name, or nil if
-// none exists.  It panics if the dependency does not have the specified tag.
 func (m *baseModuleContext) GetDirectDepWithTag(name string, tag DependencyTag) Module {
 	var deps []depInfo
 	for _, dep := range m.module.directDeps {
@@ -378,8 +502,6 @@
 	return nil
 }
 
-// VisitDirectDeps calls visit for each direct dependency.  If there are multiple direct dependencies on the same module
-// visit will be called multiple times on that module and OtherModuleDependencyTag will return a different tag for each.
 func (m *baseModuleContext) VisitDirectDeps(visit func(Module)) {
 	defer func() {
 		if r := recover(); r != nil {
@@ -399,9 +521,6 @@
 	m.visitingDep = depInfo{}
 }
 
-// VisitDirectDepsIf calls pred for each direct dependency, and if pred returns true calls visit.  If there are multiple
-// direct dependencies on the same module pred and visit will be called multiple times on that module and
-// OtherModuleDependencyTag will return a different tag for each.
 func (m *baseModuleContext) VisitDirectDepsIf(pred func(Module) bool, visit func(Module)) {
 	defer func() {
 		if r := recover(); r != nil {
@@ -423,10 +542,6 @@
 	m.visitingDep = depInfo{}
 }
 
-// VisitDepsDepthFirst calls visit for each transitive dependency, traversing the dependency tree in depth first order.
-// visit will only be called once for any given module, even if there are multiple paths through the dependency tree
-// to the module or multiple direct dependencies with different tags.  OtherModuleDependencyTag will return the tag for
-// the first path found to the module.
 func (m *baseModuleContext) VisitDepsDepthFirst(visit func(Module)) {
 	defer func() {
 		if r := recover(); r != nil {
@@ -445,11 +560,6 @@
 	m.visitingDep = depInfo{}
 }
 
-// VisitDepsDepthFirst calls pred for each transitive dependency, and if pred returns true calls visit, traversing the
-// dependency tree in depth first order.  visit will only be called once for any given module, even if there are
-// multiple paths through the dependency tree to the module or multiple direct dependencies with different tags.
-// OtherModuleDependencyTag will return the tag for the first path found to the module.  The return value of pred does
-// not affect which branches of the tree are traversed.
 func (m *baseModuleContext) VisitDepsDepthFirstIf(pred func(Module) bool,
 	visit func(Module)) {
 
@@ -472,10 +582,6 @@
 	m.visitingDep = depInfo{}
 }
 
-// WalkDeps calls visit for each transitive dependency, traversing the dependency tree in top down order.  visit may be
-// called multiple times for the same (child, parent) pair if there are multiple direct dependencies between the
-// child and parent with different tags.  OtherModuleDependencyTag will return the tag for the currently visited
-// (child, parent) pair.  If visit returns false WalkDeps will not continue recursing down to child.
 func (m *baseModuleContext) WalkDeps(visit func(child, parent Module) bool) {
 	m.context.walkDeps(m.module, true, func(dep depInfo, parent *moduleInfo) bool {
 		m.visitingParent = parent
@@ -563,54 +669,116 @@
 	newModules    []*moduleInfo // brand new modules
 }
 
-type baseMutatorContext interface {
+type BaseMutatorContext interface {
 	BaseModuleContext
 
-	OtherModuleExists(name string) bool
+	// Rename all variants of a module.  The new name is not visible to calls to ModuleName,
+	// AddDependency or OtherModuleName until after this mutator pass is complete.
 	Rename(name string)
-	Module() Module
+
+	// MutatorName returns the name that this mutator was registered with.
+	MutatorName() string
 }
 
 type EarlyMutatorContext interface {
-	baseMutatorContext
+	BaseMutatorContext
 
+	// CreateVariations splits  a module into mulitple variants, one for each name in the variationNames
+	// parameter.  It returns a list of new modules in the same order as the variationNames
+	// list.
+	//
+	// If any of the dependencies of the module being operated on were already split
+	// by calling CreateVariations with the same name, the dependency will automatically
+	// be updated to point the matching variant.
+	//
+	// If a module is split, and then a module depending on the first module is not split
+	// when the Mutator is later called on it, the dependency of the depending module will
+	// automatically be updated to point to the first variant.
 	CreateVariations(...string) []Module
+
+	// CreateLocationVariations splits a module into mulitple variants, one for each name in the variantNames
+	// parameter.  It returns a list of new modules in the same order as the variantNames
+	// list.
+	//
+	// Local variations do not affect automatic dependency resolution - dependencies added
+	// to the split module via deps or DynamicDependerModule must exactly match a variant
+	// that contains all the non-local variations.
 	CreateLocalVariations(...string) []Module
 }
 
 type TopDownMutatorContext interface {
-	baseMutatorContext
+	BaseMutatorContext
 
-	OtherModuleName(m Module) string
-	OtherModuleDir(m Module) string
-	OtherModuleSubDir(m Module) string
-	OtherModuleType(m Module) string
-	OtherModuleErrorf(m Module, fmt string, args ...interface{})
-	OtherModuleDependencyTag(m Module) DependencyTag
-
+	// CreateModule creates a new module by calling the factory method for the specified moduleType, and applies
+	// the specified property structs to it as if the properties were set in a blueprint file.
 	CreateModule(ModuleFactory, ...interface{})
-
-	GetDirectDepWithTag(name string, tag DependencyTag) Module
-	GetDirectDep(name string) (Module, DependencyTag)
-
-	VisitDirectDeps(visit func(Module))
-	VisitDirectDepsIf(pred func(Module) bool, visit func(Module))
-	VisitDepsDepthFirst(visit func(Module))
-	VisitDepsDepthFirstIf(pred func(Module) bool, visit func(Module))
-	WalkDeps(visit func(Module, Module) bool)
 }
 
 type BottomUpMutatorContext interface {
-	baseMutatorContext
+	BaseMutatorContext
 
+	// AddDependency adds a dependency to the given module.
+	// Does not affect the ordering of the current mutator pass, but will be ordered
+	// correctly for all future mutator passes.
 	AddDependency(module Module, tag DependencyTag, name ...string)
+
+	// AddReverseDependency adds a dependency from the destination to the given module.
+	// Does not affect the ordering of the current mutator pass, but will be ordered
+	// correctly for all future mutator passes.  All reverse dependencies for a destination module are
+	// collected until the end of the mutator pass, sorted by name, and then appended to the destination
+	// module's dependency list.
 	AddReverseDependency(module Module, tag DependencyTag, name string)
+
+	// CreateVariations splits  a module into mulitple variants, one for each name in the variationNames
+	// parameter.  It returns a list of new modules in the same order as the variationNames
+	// list.
+	//
+	// If any of the dependencies of the module being operated on were already split
+	// by calling CreateVariations with the same name, the dependency will automatically
+	// be updated to point the matching variant.
+	//
+	// If a module is split, and then a module depending on the first module is not split
+	// when the Mutator is later called on it, the dependency of the depending module will
+	// automatically be updated to point to the first variant.
 	CreateVariations(...string) []Module
+
+	// CreateLocationVariations splits a module into mulitple variants, one for each name in the variantNames
+	// parameter.  It returns a list of new modules in the same order as the variantNames
+	// list.
+	//
+	// Local variations do not affect automatic dependency resolution - dependencies added
+	// to the split module via deps or DynamicDependerModule must exactly match a variant
+	// that contains all the non-local variations.
 	CreateLocalVariations(...string) []Module
+
+	// SetDependencyVariation sets all dangling dependencies on the current module to point to the variation
+	// with given name.
 	SetDependencyVariation(string)
+
+	// AddVariationDependencies adds deps as dependencies of the current module, but uses the variations
+	// argument to select which variant of the dependency to use.  A variant of the dependency must
+	// exist that matches the all of the non-local variations of the current module, plus the variations
+	// argument.
 	AddVariationDependencies([]Variation, DependencyTag, ...string)
+
+	// AddFarVariationDependencies adds deps as dependencies of the current module, but uses the
+	// variations argument to select which variant of the dependency to use.  A variant of the
+	// dependency must exist that matches the variations argument, but may also have other variations.
+	// For any unspecified variation the first variant will be used.
+	//
+	// Unlike AddVariationDependencies, the variations of the current module are ignored - the
+	// dependency only needs to match the supplied variations.
 	AddFarVariationDependencies([]Variation, DependencyTag, ...string)
+
+	// AddInterVariantDependency adds a dependency between two variants of the same module.  Variants are always
+	// ordered in the same orderas they were listed in CreateVariations, and AddInterVariantDependency does not change
+	// that ordering, but it associates a DependencyTag with the dependency and makes it visible to VisitDirectDeps,
+	// WalkDeps, etc.
 	AddInterVariantDependency(tag DependencyTag, from, to Module)
+
+	// ReplaceDependencies replaces all dependencies on the identical variant of the module with the
+	// specified name with the current variant of this module.  Replacements don't take effect until
+	// after the mutator pass is finished.
 	ReplaceDependencies(string)
 }
 
@@ -643,28 +811,14 @@
 
 var _ DependencyTag = BaseDependencyTag{}
 
-// Split a module into mulitple variants, one for each name in the variationNames
-// parameter.  It returns a list of new modules in the same order as the variationNames
-// list.
-//
-// If any of the dependencies of the module being operated on were already split
-// by calling CreateVariations with the same name, the dependency will automatically
-// be updated to point the matching variant.
-//
-// If a module is split, and then a module depending on the first module is not split
-// when the Mutator is later called on it, the dependency of the depending module will
-// automatically be updated to point to the first variant.
+func (mctx *mutatorContext) MutatorName() string {
+	return mctx.name
+}
+
 func (mctx *mutatorContext) CreateVariations(variationNames ...string) []Module {
 	return mctx.createVariations(variationNames, false)
 }
 
-// Split a module into mulitple variants, one for each name in the variantNames
-// parameter.  It returns a list of new modules in the same order as the variantNames
-// list.
-//
-// Local variations do not affect automatic dependency resolution - dependencies added
-// to the split module via deps or DynamicDependerModule must exactly match a variant
-// that contains all the non-local variations.
 func (mctx *mutatorContext) CreateLocalVariations(variationNames ...string) []Module {
 	return mctx.createVariations(variationNames, true)
 }
@@ -695,8 +849,6 @@
 	return ret
 }
 
-// Set all dangling dependencies on the current module to point to the variation
-// with given name.
 func (mctx *mutatorContext) SetDependencyVariation(variationName string) {
 	mctx.context.convertDepsToVariation(mctx.module, mctx.name, variationName)
 }
@@ -705,9 +857,6 @@
 	return mctx.module.logicModule
 }
 
-// Add a dependency to the given module.
-// Does not affect the ordering of the current mutator pass, but will be ordered
-// correctly for all future mutator passes.
 func (mctx *mutatorContext) AddDependency(module Module, tag DependencyTag, deps ...string) {
 	for _, dep := range deps {
 		modInfo := mctx.context.moduleInfo[module]
@@ -718,11 +867,6 @@
 	}
 }
 
-// Add a dependency from the destination to the given module.
-// Does not affect the ordering of the current mutator pass, but will be ordered
-// correctly for all future mutator passes.  All reverse dependencies for a destination module are
-// collected until the end of the mutator pass, sorted by name, and then appended to the destination
-// module's dependency list.
 func (mctx *mutatorContext) AddReverseDependency(module Module, tag DependencyTag, destName string) {
 	if _, ok := tag.(BaseDependencyTag); ok {
 		panic("BaseDependencyTag is not allowed to be used directly!")
@@ -740,10 +884,6 @@
 	})
 }
 
-// AddVariationDependencies adds deps as dependencies of the current module, but uses the variations
-// argument to select which variant of the dependency to use.  A variant of the dependency must
-// exist that matches the all of the non-local variations of the current module, plus the variations
-// argument.
 func (mctx *mutatorContext) AddVariationDependencies(variations []Variation, tag DependencyTag,
 	deps ...string) {
 
@@ -755,13 +895,6 @@
 	}
 }
 
-// AddFarVariationDependencies adds deps as dependencies of the current module, but uses the
-// variations argument to select which variant of the dependency to use.  A variant of the
-// dependency must exist that matches the variations argument, but may also have other variations.
-// For any unspecified variation the first variant will be used.
-//
-// Unlike AddVariationDependencies, the variations of the current module are ignored - the
-// depdendency only needs to match the supplied variations.
 func (mctx *mutatorContext) AddFarVariationDependencies(variations []Variation, tag DependencyTag,
 	deps ...string) {
 
@@ -777,9 +910,6 @@
 	mctx.context.addInterVariantDependency(mctx.module, tag, from, to)
 }
 
-// ReplaceDependencies replaces all dependencies on the identical variant of the module with the
-// specified name with the current variant of this module.  Replacements don't take effect until
-// after the mutator pass is finished.
 func (mctx *mutatorContext) ReplaceDependencies(name string) {
 	target := mctx.context.moduleMatchingVariant(mctx.module, name)
 
@@ -791,24 +921,17 @@
 	mctx.replace = append(mctx.replace, replace{target, mctx.module})
 }
 
-func (mctx *mutatorContext) OtherModuleExists(name string) bool {
-	_, exists := mctx.context.nameInterface.ModuleFromName(name, mctx.module.namespace())
-	return exists
-}
-
-// Rename all variants of a module.  The new name is not visible to calls to ModuleName,
-// AddDependency or OtherModuleName until after this mutator pass is complete.
 func (mctx *mutatorContext) Rename(name string) {
 	mctx.rename = append(mctx.rename, rename{mctx.module.group, name})
 }
 
-// Create a new module by calling the factory method for the specified moduleType, and apply
-// the specified property structs to it as if the properties were set in a blueprint file.
 func (mctx *mutatorContext) CreateModule(factory ModuleFactory, props ...interface{}) {
 	module := mctx.context.newModule(factory)
 
 	module.relBlueprintsFile = mctx.module.relBlueprintsFile
 	module.pos = mctx.module.pos
+	module.propertyPos = mctx.module.propertyPos
+	module.createdBy = mctx.module
 
 	for _, p := range props {
 		err := proptools.AppendMatchingProperties(module.properties, p, nil)
diff --git a/ninja_strings.go b/ninja_strings.go
index a13205f..5b8767d 100644
--- a/ninja_strings.go
+++ b/ninja_strings.go
@@ -277,12 +277,20 @@
 func (n *ninjaString) ValueWithEscaper(pkgNames map[*packageContext]string,
 	escaper *strings.Replacer) string {
 
-	str := escaper.Replace(n.strings[0])
-	for i, v := range n.variables {
-		str += "${" + v.fullName(pkgNames) + "}"
-		str += escaper.Replace(n.strings[i+1])
+	if len(n.strings) == 1 {
+		return escaper.Replace(n.strings[0])
 	}
-	return str
+
+	str := strings.Builder{}
+	str.WriteString(escaper.Replace(n.strings[0]))
+	for i, v := range n.variables {
+		str.WriteString("${")
+		str.WriteString(v.fullName(pkgNames))
+		str.WriteString("}")
+		str.WriteString(escaper.Replace(n.strings[i+1]))
+	}
+
+	return str.String()
 }
 
 func (n *ninjaString) Eval(variables map[Variable]*ninjaString) (string, error) {
diff --git a/ninja_strings_test.go b/ninja_strings_test.go
index 6227ea6..0e0de64 100644
--- a/ninja_strings_test.go
+++ b/ninja_strings_test.go
@@ -16,6 +16,8 @@
 
 import (
 	"reflect"
+	"strconv"
+	"strings"
 	"testing"
 )
 
@@ -161,3 +163,45 @@
 		t.Errorf("       got: %#v", output)
 	}
 }
+
+func BenchmarkNinjaString_Value(b *testing.B) {
+	b.Run("constant", func(b *testing.B) {
+		for _, l := range []int{1, 10, 100, 1000} {
+			ns := simpleNinjaString(strings.Repeat("a", l))
+			b.Run(strconv.Itoa(l), func(b *testing.B) {
+				for n := 0; n < b.N; n++ {
+					ns.Value(nil)
+				}
+			})
+		}
+	})
+	b.Run("variable", func(b *testing.B) {
+		for _, l := range []int{1, 10, 100, 1000} {
+			scope := newLocalScope(nil, "")
+			scope.AddLocalVariable("a", strings.Repeat("b", l/3))
+			ns, _ := parseNinjaString(scope, strings.Repeat("a", l/3)+"${a}"+strings.Repeat("a", l/3))
+			b.Run(strconv.Itoa(l), func(b *testing.B) {
+				for n := 0; n < b.N; n++ {
+					ns.Value(nil)
+				}
+			})
+		}
+	})
+	b.Run("variables", func(b *testing.B) {
+		for _, l := range []int{1, 2, 3, 4, 5, 10, 100, 1000} {
+			scope := newLocalScope(nil, "")
+			str := strings.Repeat("a", 10)
+			for i := 0; i < l; i++ {
+				scope.AddLocalVariable("a"+strconv.Itoa(i), strings.Repeat("b", 10))
+				str += "${a" + strconv.Itoa(i) + "}"
+			}
+			ns, _ := parseNinjaString(scope, str)
+			b.Run(strconv.Itoa(l), func(b *testing.B) {
+				for n := 0; n < b.N; n++ {
+					ns.Value(nil)
+				}
+			})
+		}
+	})
+
+}
diff --git a/pathtools/fs.go b/pathtools/fs.go
index 4217487..8329392 100644
--- a/pathtools/fs.go
+++ b/pathtools/fs.go
@@ -61,6 +61,9 @@
 		fs.dirs[dir] = true
 	}
 
+	fs.dirs["."] = true
+	fs.dirs["/"] = true
+
 	for f := range fs.files {
 		fs.all = append(fs.all, f)
 	}
@@ -330,8 +333,8 @@
 			if err != nil {
 				return nil, err
 			}
-			if f == "." && f != pattern {
-				// filepath.Glob won't return "." unless the pattern was "."
+			if (f == "." || f == "/") && f != pattern {
+				// filepath.Glob won't return "." or "/" unless the pattern was "." or "/"
 				match = false
 			}
 			if match {
diff --git a/pathtools/glob.go b/pathtools/glob.go
index 67394d2..727b725 100644
--- a/pathtools/glob.go
+++ b/pathtools/glob.go
@@ -25,8 +25,9 @@
 	"github.com/google/blueprint/deptools"
 )
 
-var GlobMultipleRecursiveErr = errors.New("pattern contains multiple **")
-var GlobLastRecursiveErr = errors.New("pattern ** as last path element")
+var GlobMultipleRecursiveErr = errors.New("pattern contains multiple '**'")
+var GlobLastRecursiveErr = errors.New("pattern has '**' as last path element")
+var GlobInvalidRecursiveErr = errors.New("pattern contains other characters between '**' and path separator")
 
 // Glob returns the list of files and directories that match the given pattern
 // but do not match the given exclude patterns, along with the list of
@@ -118,7 +119,7 @@
 			// as a dependency.
 			var matchDirs []string
 			for len(matchDirs) == 0 {
-				pattern, _ = saneSplit(pattern)
+				pattern = filepath.Dir(pattern)
 				matchDirs, err = fs.glob(pattern)
 				if err != nil {
 					return matches, dirs, err
@@ -136,6 +137,8 @@
 			return matches, dirs, GlobMultipleRecursiveErr
 		}
 		hasRecursive = true
+	} else if strings.Contains(file, "**") {
+		return matches, dirs, GlobInvalidRecursiveErr
 	}
 
 	dirMatches, dirs, err := glob(fs, dir, hasRecursive, follow)
@@ -242,7 +245,7 @@
 }
 
 // Match returns true if name matches pattern using the same rules as filepath.Match, but supporting
-// hierarchical patterns (a/*) and recursive globs (**).
+// recursive globs (**).
 func Match(pattern, name string) (bool, error) {
 	if filepath.Base(pattern) == "**" {
 		return false, GlobLastRecursiveErr
@@ -262,16 +265,35 @@
 
 	for {
 		var patternFile, nameFile string
-		pattern, patternFile = saneSplit(pattern)
-		name, nameFile = saneSplit(name)
+		pattern, patternFile = filepath.Dir(pattern), filepath.Base(pattern)
 
 		if patternFile == "**" {
-			return matchPrefix(pattern, filepath.Join(name, nameFile))
+			if strings.Contains(pattern, "**") {
+				return false, GlobMultipleRecursiveErr
+			}
+			// Test if the any prefix of name matches the part of the pattern before **
+			for {
+				if name == "." || name == "/" {
+					return name == pattern, nil
+				}
+				if match, err := filepath.Match(pattern, name); err != nil {
+					return false, err
+				} else if match {
+					return true, nil
+				}
+				name = filepath.Dir(name)
+			}
+		} else if strings.Contains(patternFile, "**") {
+			return false, GlobInvalidRecursiveErr
 		}
 
-		if nameFile == "" && patternFile == "" {
+		name, nameFile = filepath.Dir(name), filepath.Base(name)
+
+		if nameFile == "." && patternFile == "." {
 			return true, nil
-		} else if nameFile == "" || patternFile == "" {
+		} else if nameFile == "/" && patternFile == "/" {
+			return true, nil
+		} else if nameFile == "." || patternFile == "." || nameFile == "/" || patternFile == "/" {
 			return false, nil
 		}
 
@@ -282,56 +304,6 @@
 	}
 }
 
-// matchPrefix returns true if the beginning of name matches pattern using the same rules as
-// filepath.Match, but supporting hierarchical patterns (a/*).  Recursive globs (**) are not
-// supported, they should have been handled in Match().
-func matchPrefix(pattern, name string) (bool, error) {
-	if len(pattern) > 0 && pattern[0] == '/' {
-		if len(name) > 0 && name[0] == '/' {
-			pattern = pattern[1:]
-			name = name[1:]
-		} else {
-			return false, nil
-		}
-	}
-
-	for {
-		var patternElem, nameElem string
-		patternElem, pattern = saneSplitFirst(pattern)
-		nameElem, name = saneSplitFirst(name)
-
-		if patternElem == "." {
-			patternElem = ""
-		}
-		if nameElem == "." {
-			nameElem = ""
-		}
-
-		if patternElem == "**" {
-			return false, GlobMultipleRecursiveErr
-		}
-
-		if patternElem == "" {
-			return true, nil
-		} else if nameElem == "" {
-			return false, nil
-		}
-
-		match, err := filepath.Match(patternElem, nameElem)
-		if err != nil || !match {
-			return match, err
-		}
-	}
-}
-
-func saneSplitFirst(path string) (string, string) {
-	i := strings.IndexRune(path, filepath.Separator)
-	if i < 0 {
-		return path, ""
-	}
-	return path[:i], path[i+1:]
-}
-
 func GlobPatternList(patterns []string, prefix string) (globedList []string, depDirs []string, err error) {
 	var (
 		matches []string
diff --git a/pathtools/glob_test.go b/pathtools/glob_test.go
index 0265df6..a3a36ff 100644
--- a/pathtools/glob_test.go
+++ b/pathtools/glob_test.go
@@ -18,6 +18,7 @@
 	"os"
 	"path/filepath"
 	"reflect"
+	"strings"
 	"testing"
 )
 
@@ -215,6 +216,14 @@
 		pattern: "**/**",
 		err:     GlobLastRecursiveErr,
 	},
+	{
+		pattern: "a**/",
+		err:     GlobInvalidRecursiveErr,
+	},
+	{
+		pattern: "**a/",
+		err:     GlobInvalidRecursiveErr,
+	},
 
 	// exclude tests
 	{
@@ -778,6 +787,16 @@
 		{"a/**/*/", "a/b/", true},
 		{"a/**/*/", "a/b/c", false},
 
+		{"**/*", "a/", false},
+		{"**/*", "a/a", true},
+		{"**/*", "a/b/", false},
+		{"**/*", "a/b/c", true},
+
+		{"**/*/", "a/", true},
+		{"**/*/", "a/a", false},
+		{"**/*/", "a/b/", true},
+		{"**/*/", "a/b/c", false},
+
 		{`a/\*\*/\*`, `a/**/*`, true},
 		{`a/\*\*/\*`, `a/a/*`, false},
 		{`a/\*\*/\*`, `a/**/a`, false},
@@ -826,6 +845,38 @@
 		{`a/?`, `a/a`, true},
 		{`a/\?`, `a/?`, true},
 		{`a/\?`, `a/a`, false},
+
+		{"/a/*", "/a/", false},
+		{"/a/*", "/a/a", true},
+		{"/a/*", "/a/b/", false},
+		{"/a/*", "/a/b/c", false},
+
+		{"/a/*/", "/a/", false},
+		{"/a/*/", "/a/a", false},
+		{"/a/*/", "/a/b/", true},
+		{"/a/*/", "/a/b/c", false},
+
+		{"/a/**/*", "/a/", false},
+		{"/a/**/*", "/a/a", true},
+		{"/a/**/*", "/a/b/", false},
+		{"/a/**/*", "/a/b/c", true},
+
+		{"/**/*", "/a/", false},
+		{"/**/*", "/a/a", true},
+		{"/**/*", "/a/b/", false},
+		{"/**/*", "/a/b/c", true},
+
+		{"/**/*/", "/a/", true},
+		{"/**/*/", "/a/a", false},
+		{"/**/*/", "/a/b/", true},
+		{"/**/*/", "/a/b/c", false},
+
+		{`a`, `/a`, false},
+		{`/a`, `a`, false},
+		{`*`, `/a`, false},
+		{`/*`, `a`, false},
+		{`**/*`, `/a`, false},
+		{`/**/*`, `a`, false},
 	}
 
 	for _, test := range testCases {
@@ -839,4 +890,36 @@
 			}
 		})
 	}
+
+	// Run the same test cases through Glob
+	for _, test := range testCases {
+		// Glob and Match disagree on matching directories
+		if strings.HasSuffix(test.name, "/") || strings.HasSuffix(test.pattern, "/") {
+			continue
+		}
+		t.Run("glob:"+test.pattern+","+test.name, func(t *testing.T) {
+			mockFiles := map[string][]byte{
+				test.name: nil,
+			}
+
+			mock := MockFs(mockFiles)
+
+			matches, _, err := mock.Glob(test.pattern, nil, DontFollowSymlinks)
+			t.Log(test.name, test.pattern, matches)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			match := false
+			for _, x := range matches {
+				if x == test.name {
+					match = true
+				}
+			}
+
+			if match != test.match {
+				t.Errorf("want: %v, got %v", test.match, match)
+			}
+		})
+	}
 }
diff --git a/proptools/extend.go b/proptools/extend.go
index 1f323cf..f588ab2 100644
--- a/proptools/extend.go
+++ b/proptools/extend.go
@@ -34,7 +34,7 @@
 // embedded structs, pointers to structs, and interfaces containing
 // pointers to structs.  Appending the zero value of a property will always be a no-op.
 func AppendProperties(dst interface{}, src interface{}, filter ExtendPropertyFilterFunc) error {
-	return extendProperties(dst, src, filter, orderAppend)
+	return extendProperties(dst, src, filter, OrderAppend)
 }
 
 // PrependProperties prepends the values of properties in the property struct src to the property
@@ -52,7 +52,7 @@
 // embedded structs, pointers to structs, and interfaces containing
 // pointers to structs.  Prepending the zero value of a property will always be a no-op.
 func PrependProperties(dst interface{}, src interface{}, filter ExtendPropertyFilterFunc) error {
-	return extendProperties(dst, src, filter, orderPrepend)
+	return extendProperties(dst, src, filter, OrderPrepend)
 }
 
 // AppendMatchingProperties appends the values of properties in the property struct src to the
@@ -73,7 +73,7 @@
 // pointers to structs.  Appending the zero value of a property will always be a no-op.
 func AppendMatchingProperties(dst []interface{}, src interface{},
 	filter ExtendPropertyFilterFunc) error {
-	return extendMatchingProperties(dst, src, filter, orderAppend)
+	return extendMatchingProperties(dst, src, filter, OrderAppend)
 }
 
 // PrependMatchingProperties prepends the values of properties in the property struct src to the
@@ -94,7 +94,7 @@
 // pointers to structs.  Prepending the zero value of a property will always be a no-op.
 func PrependMatchingProperties(dst []interface{}, src interface{},
 	filter ExtendPropertyFilterFunc) error {
-	return extendMatchingProperties(dst, src, filter, orderPrepend)
+	return extendMatchingProperties(dst, src, filter, OrderPrepend)
 }
 
 // ExtendProperties appends or prepends the values of properties in the property struct src to the
@@ -160,13 +160,13 @@
 	dstField, srcField reflect.StructField,
 	dstValue, srcValue interface{}) (Order, error)
 
-func orderAppend(property string,
+func OrderAppend(property string,
 	dstField, srcField reflect.StructField,
 	dstValue, srcValue interface{}) (Order, error) {
 	return Append, nil
 }
 
-func orderPrepend(property string,
+func OrderPrepend(property string,
 	dstField, srcField reflect.StructField,
 	dstValue, srcValue interface{}) (Order, error) {
 	return Prepend, nil
diff --git a/singleton_ctx.go b/singleton_ctx.go
index 5ca8ee6..cc58e06 100644
--- a/singleton_ctx.go
+++ b/singleton_ctx.go
@@ -25,23 +25,51 @@
 }
 
 type SingletonContext interface {
+	// Config returns the config object that was passed to Context.PrepareBuildActions.
 	Config() interface{}
 
+	// Name returns the name of the current singleton passed to Context.RegisterSingletonType
 	Name() string
 
+	// ModuleName returns the name of the given Module.  See BaseModuleContext.ModuleName for more information.
 	ModuleName(module Module) string
+
+	// ModuleDir returns the directory of the given Module.  See BaseModuleContext.ModuleDir for more information.
 	ModuleDir(module Module) string
+
+	// ModuleSubDir returns the unique subdirectory name of the given Module.  See ModuleContext.ModuleSubDir for
+	// more information.
 	ModuleSubDir(module Module) string
+
+	// ModuleType returns the type of the given Module.  See BaseModuleContext.ModuleType for more information.
 	ModuleType(module Module) string
+
+	// BlueprintFile returns the path of the Blueprint file that defined the given module.
 	BlueprintFile(module Module) string
 
+	// ModuleErrorf reports an error at the line number of the module type in the module definition.
 	ModuleErrorf(module Module, format string, args ...interface{})
+
+	// Errorf reports an error at the specified position of the module definition file.
 	Errorf(format string, args ...interface{})
+
+	// Failed returns true if any errors have been reported.  In most cases the singleton can continue with generating
+	// build rules after an error, allowing it to report additional errors in a single run, but in cases where the error
+	// has prevented the singleton from creating necessary data it can return early when Failed returns true.
 	Failed() bool
 
+	// Variable creates a new ninja variable scoped to the singleton.  It can be referenced by calls to Rule and Build
+	// in the same singleton.
 	Variable(pctx PackageContext, name, value string)
+
+	// Rule creates a new ninja rule scoped to the singleton.  It can be referenced by calls to Build in the same
+	// singleton.
 	Rule(pctx PackageContext, name string, params RuleParams, argNames ...string) Rule
+
+	// Build creates a new ninja build statement.
 	Build(pctx PackageContext, params BuildParams)
+
+	// RequireNinjaVersion sets the generated ninja manifest to require at least the specified version of ninja.
 	RequireNinjaVersion(major, minor, micro int)
 
 	// SetNinjaBuildDir sets the value of the top-level "builddir" Ninja variable
@@ -58,17 +86,37 @@
 	// are expanded in the scope of the PackageContext.
 	Eval(pctx PackageContext, ninjaStr string) (string, error)
 
+	// VisitAllModules calls visit for each defined variant of each module in an unspecified order.
 	VisitAllModules(visit func(Module))
+
+	// VisitAllModules calls pred for each defined variant of each module in an unspecified order, and if pred returns
+	// true calls visit.
 	VisitAllModulesIf(pred func(Module) bool, visit func(Module))
+
+	// VisitDepsDepthFirst calls visit for each transitive dependency, traversing the dependency tree in depth first
+	// order. visit will only be called once for any given module, even if there are multiple paths through the
+	// dependency tree to the module or multiple direct dependencies with different tags.
 	VisitDepsDepthFirst(module Module, visit func(Module))
+
+	// VisitDepsDepthFirst calls pred for each transitive dependency, and if pred returns true calls visit, traversing
+	// the dependency tree in depth first order.  visit will only be called once for any given module, even if there are
+	// multiple paths through the dependency tree to the module or multiple direct dependencies with different tags.
 	VisitDepsDepthFirstIf(module Module, pred func(Module) bool,
 		visit func(Module))
 
+	// VisitAllModuleVariants calls visit for each variant of the given module.
 	VisitAllModuleVariants(module Module, visit func(Module))
 
+	// PrimaryModule returns the first variant of the given module.  This can be used to perform
+	//	// singleton actions that are only done once for all variants of a module.
 	PrimaryModule(module Module) Module
+
+	// FinalModule returns the last variant of the given module.  This can be used to perform
+	// singleton actions that are only done once for all variants of a module.
 	FinalModule(module Module) Module
 
+	// AddNinjaFileDeps adds dependencies on the specified files to the rule that creates the ninja manifest.  The
+	// primary builder will be rerun whenever the specified files are modified.
 	AddNinjaFileDeps(deps ...string)
 
 	// GlobWithDeps returns a list of files and directories that match the
@@ -79,6 +127,8 @@
 	// does not match the pattern is added to a searched directory.
 	GlobWithDeps(pattern string, excludes []string) ([]string, error)
 
+	// Fs returns a pathtools.Filesystem that can be used to interact with files.  Using the Filesystem interface allows
+	// the singleton to be used in build system tests that run against a mock filesystem.
 	Fs() pathtools.FileSystem
 }
 
@@ -223,7 +273,7 @@
 	defer func() {
 		if r := recover(); r != nil {
 			panic(newPanicErrorf(r, "VisitAllModules(%s) for module %s",
-				funcName(visit), visitingModule))
+				funcName(visit), s.context.moduleInfo[visitingModule]))
 		}
 	}()