From cdfcec98bb7a7ca63e1b943125bce628ff6c7a37 Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Fri, 1 May 2020 11:57:12 +0100 Subject: [PATCH] Simplify package by using LoadHook instead of mutators This was not previously possible due to LoadHooks being run after the module was registered. Bug: 155462403 Test: m nothing Change-Id: Ia8383b9d1272bb12c8a83948753a0e4b0d98a650 --- android/mutator.go | 8 --- android/package.go | 149 +++------------------------------------- android/package_test.go | 4 +- 3 files changed, 12 insertions(+), 149 deletions(-) diff --git a/android/mutator.go b/android/mutator.go index 247eb4d14..79a85062a 100644 --- a/android/mutator.go +++ b/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, diff --git a/android/package.go b/android/package.go index bb5f4e79e..182b3ed6e 100644 --- a/android/package.go +++ b/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. -// -// 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. +// Register the package module type. 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 - packageInfo *packageInfo + properties packageProperties } 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) - } -} diff --git a/android/package_test.go b/android/package_test.go index 4710e3954..04dfc08a9 100644 --- a/android/package_test.go +++ b/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`, }, }, }