Merge "Simplify package by using LoadHook instead of mutators" am: 00789cc58b am: c11d79fe52

Change-Id: I6e6a45b00b8f3cbc8967d2474958ae555adf8dc1

android/mutator.go

@@ -82,14 +82,6 @@ type RegisterMutatorFunc func(RegisterMutatorsContext)
 var preArch = []RegisterMutatorFunc{
 	RegisterNamespaceMutator,
 
-	// Rename package module types.
-	//
-	// The package module type does not have a name property, instead its name is determined
-	// by the location of the containing .bp file relative to the root of the file structure
-	// being built by Soong. Unfortunately, due to limitations in LoadHook the module has to
-	// be given a synthetic temporary name which is then fixed up by these mutators.
-	RegisterPackageRenamer,
-
 	// Create an association between prebuilt modules and their corresponding source
 	// modules (if any).
 	RegisterPrebuiltsPreArchMutators,

android/package.go

@@ -15,43 +15,20 @@
 package android
 
 import (
-	"fmt"
-	"sync/atomic"
-
 	"github.com/google/blueprint"
+	"github.com/google/blueprint/proptools"
 )
 
 func init() {
 	RegisterPackageBuildComponents(InitRegistrationContext)
 }
 
-// Register the package module type and supporting mutators.
+// Register the package module type.
-//
-// This must be called in the correct order (relative to other methods that also
-// register mutators) to match the order of mutator registration in mutator.go.
-// Failing to do so will result in an unrealistic test environment.
 func RegisterPackageBuildComponents(ctx RegistrationContext) {
 	ctx.RegisterModuleType("package", PackageFactory)
-
-	// Register mutators that are hard coded in to mutator.go.
-	ctx.HardCodedPreArchMutators(RegisterPackageRenamer)
-}
-
-// The information maintained about each package.
-type packageInfo struct {
-	// The module from which this information was populated. If `duplicated` = true then this is the
-	// module that has been renamed and must be used to report errors.
-	module *packageModule
-
-	// If true this indicates that there are two package statements in the same package which is not
-	// allowed and will cause the build to fail. This flag is set by packageRenamer and checked in
-	// packageErrorReporter
-	duplicated bool
 }
 
 type packageProperties struct {
-	Name string `blueprint:"mutated"`
-
 	// Specifies the default visibility for all modules defined in this package.
 	Default_visibility []string
 }
@@ -59,8 +36,7 @@ type packageProperties struct {
 type packageModule struct {
 	ModuleBase
 
-	properties  packageProperties
+	properties packageProperties
-	packageInfo *packageInfo
 }
 
 func (p *packageModule) GenerateAndroidBuildActions(ModuleContext) {
@@ -76,126 +52,21 @@ func (p *packageModule) qualifiedModuleId(ctx BaseModuleContext) qualifiedModule
 	return newPackageId(ctx.ModuleDir())
 }
 
-func (p *packageModule) Name() string {
-	return p.properties.Name
-}
-
-func (p *packageModule) setName(name string) {
-	p.properties.Name = name
-}
-
-// Counter to ensure package modules are created with a unique name within whatever namespace they
-// belong.
-var packageCount uint32 = 0
-
 func PackageFactory() Module {
 	module := &packageModule{}
 
-	// Get a unique if for the package. Has to be done atomically as the creation of the modules are
-	// done in parallel.
-	id := atomic.AddUint32(&packageCount, 1)
-	name := fmt.Sprintf("soong_package_%d", id)
-
-	module.properties.Name = name
-
 	module.AddProperties(&module.properties)
 
+	// The name is the relative path from build root to the directory containing this
+	// module. Set that name at the earliest possible moment that information is available
+	// which is in a LoadHook.
+	AddLoadHook(module, func(ctx LoadHookContext) {
+		module.nameProperties.Name = proptools.StringPtr("//" + ctx.ModuleDir())
+	})
+
 	// The default_visibility property needs to be checked and parsed by the visibility module during
 	// its checking and parsing phases so make it the primary visibility property.
 	setPrimaryVisibilityProperty(module, "default_visibility", &module.properties.Default_visibility)
 
 	return module
 }
-
-// Registers the function that renames the packages.
-func RegisterPackageRenamer(ctx RegisterMutatorsContext) {
-	ctx.BottomUp("packageRenamer", packageRenamer).Parallel()
-	ctx.BottomUp("packageErrorReporter", packageErrorReporter).Parallel()
-}
-
-// Renames the package to match the package directory.
-//
-// This also creates a PackageInfo object for each package and uses that to detect and remember
-// duplicates for later error reporting.
-func packageRenamer(ctx BottomUpMutatorContext) {
-	m, ok := ctx.Module().(*packageModule)
-	if !ok {
-		return
-	}
-
-	packageName := "//" + ctx.ModuleDir()
-
-	pi := newPackageInfo(ctx, packageName, m)
-	if pi.module != m {
-		// Remember that the package was duplicated but do not rename as that will cause an error to
-		// be logged with the generated name. Similarly, reporting the error here will use the generated
-		// name as renames are only processed after this phase.
-		pi.duplicated = true
-	} else {
-		// This is the first package module in this package so rename it to match the package name.
-		m.setName(packageName)
-		ctx.Rename(packageName)
-
-		// Store a package info reference in the module.
-		m.packageInfo = pi
-	}
-}
-
-// Logs any deferred errors.
-func packageErrorReporter(ctx BottomUpMutatorContext) {
-	m, ok := ctx.Module().(*packageModule)
-	if !ok {
-		return
-	}
-
-	packageDir := ctx.ModuleDir()
-	packageName := "//" + packageDir
-
-	// Get the PackageInfo for the package. Should have been populated in the packageRenamer phase.
-	pi := findPackageInfo(ctx, packageName)
-	if pi == nil {
-		ctx.ModuleErrorf("internal error, expected package info to be present for package '%s'",
-			packageName)
-		return
-	}
-
-	if pi.module != m {
-		// The package module has been duplicated but this is not the module that has been renamed so
-		// ignore it. An error will be logged for the renamed module which will ensure that the error
-		// message uses the correct name.
-		return
-	}
-
-	// Check to see whether there are duplicate package modules in the package.
-	if pi.duplicated {
-		ctx.ModuleErrorf("package {...} specified multiple times")
-		return
-	}
-}
-
-type defaultPackageInfoKey string
-
-func newPackageInfo(
-	ctx BaseModuleContext, packageName string, module *packageModule) *packageInfo {
-	key := NewCustomOnceKey(defaultPackageInfoKey(packageName))
-
-	return ctx.Config().Once(key, func() interface{} {
-		return &packageInfo{module: module}
-	}).(*packageInfo)
-}
-
-// Get the PackageInfo for the package name (starts with //, no trailing /), is nil if no package
-// module type was specified.
-func findPackageInfo(ctx BaseModuleContext, packageName string) *packageInfo {
-	key := NewCustomOnceKey(defaultPackageInfoKey(packageName))
-
-	pi := ctx.Config().Once(key, func() interface{} {
-		return nil
-	})
-
-	if pi == nil {
-		return nil
-	} else {
-		return pi.(*packageInfo)
-	}
-}

android/package_test.go

@@ -20,7 +20,7 @@ var packageTests = []struct {
 				}`),
 		},
 		expectedErrors: []string{
-			`top/Blueprints:3:10: mutated field name cannot be set in a Blueprint file`,
+			`top/Blueprints:3:10: unrecognized property "name"`,
 			`top/Blueprints:4:16: unrecognized property "visibility"`,
 		},
 	},
@@ -50,7 +50,7 @@ var packageTests = []struct {
 				}`),
 		},
 		expectedErrors: []string{
-			`module "//top": package {...} specified multiple times`,
+			`module "//top" already defined`,
 		},
 	},
 }