Merge remote-tracking branch 'aosp/upstream' into update

* aosp/upstream:
  Avoid exiting with errors in bpglob
  Correctly report errors in bpfmt
  Report creating modules in errors on created modules
  Move module and singleton godoc from the implementation to the interface
  Consolidate mutator contexts
  Fix panic message in SingletonContext.VisitAllModules
  Simplify pathtools.Match
  Add more pathtools.Match test cases
  Fix Ninja build system site link

Fixes: 113069050
Test: m checkbuild
Change-Id: I23e6d84e496a73bc3c9375f71f22d769dabe23e7
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..f72451b 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,113 @@
 	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
 }
 
 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 +808,10 @@
 
 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) 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 +842,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 +850,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 +860,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 +877,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 +888,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 +903,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 +914,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/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/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]))
 		}
 	}()