From 7134e289a6f2da89bc6fc47e833bb600733a264d Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Wed, 1 Dec 2021 12:16:55 -0800 Subject: [PATCH] Deterministically report path errors to all callers of dexpreopt.GetGlobalConfig dexpreopt.GetGlobalConfig uses a OncePer to only parse the config once. If the config contains missing paths, which can happen in partial manifest branches, the missing path errors will only be reported on the first caller to GetGlobalConfig. Fix the nondeterminism by wrapping the PathContext and capturing any path errors and reporting them to all callers to GetGlobalConfig. Bug: 207813628 Test: tradefed branch builds Change-Id: I869c3ae49a819b1251b94b73e0487aa594cd5e22 --- dexpreopt/config.go | 44 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/dexpreopt/config.go b/dexpreopt/config.go index ab9e418a0..64cd46a76 100644 --- a/dexpreopt/config.go +++ b/dexpreopt/config.go @@ -250,8 +250,9 @@ func ParseGlobalConfig(ctx android.PathContext, data []byte) (*GlobalConfig, err } type globalConfigAndRaw struct { - global *GlobalConfig - data []byte + global *GlobalConfig + data []byte + pathErrors []error } // GetGlobalConfig returns the global dexpreopt.config that's created in the @@ -272,16 +273,26 @@ func GetGlobalConfigRawData(ctx android.PathContext) []byte { var globalConfigOnceKey = android.NewOnceKey("DexpreoptGlobalConfig") var testGlobalConfigOnceKey = android.NewOnceKey("TestDexpreoptGlobalConfig") +type pathContextErrorCollector struct { + android.PathContext + errors []error +} + +func (p *pathContextErrorCollector) Errorf(format string, args ...interface{}) { + p.errors = append(p.errors, fmt.Errorf(format, args...)) +} + func getGlobalConfigRaw(ctx android.PathContext) globalConfigAndRaw { - return ctx.Config().Once(globalConfigOnceKey, func() interface{} { + config := ctx.Config().Once(globalConfigOnceKey, func() interface{} { if data, err := ctx.Config().DexpreoptGlobalConfig(ctx); err != nil { panic(err) } else if data != nil { - globalConfig, err := ParseGlobalConfig(ctx, data) + pathErrorCollectorCtx := &pathContextErrorCollector{PathContext: ctx} + globalConfig, err := ParseGlobalConfig(pathErrorCollectorCtx, data) if err != nil { panic(err) } - return globalConfigAndRaw{globalConfig, data} + return globalConfigAndRaw{globalConfig, data, pathErrorCollectorCtx.errors} } // No global config filename set, see if there is a test config set @@ -291,16 +302,35 @@ func getGlobalConfigRaw(ctx android.PathContext) globalConfigAndRaw { DisablePreopt: true, DisablePreoptBootImages: true, DisableGenerateProfile: true, - }, nil} + }, nil, nil} }) }).(globalConfigAndRaw) + + // Avoid non-deterministic errors by reporting cached path errors on all callers. + for _, err := range config.pathErrors { + if ctx.Config().AllowMissingDependencies() { + // When AllowMissingDependencies it set, report errors through AddMissingDependencies. + // If AddMissingDependencies doesn't exist on the current context (for example when + // called with a SingletonContext), just swallow the errors since there is no way to + // report them. + if missingDepsCtx, ok := ctx.(interface { + AddMissingDependencies(missingDeps []string) + }); ok { + missingDepsCtx.AddMissingDependencies([]string{err.Error()}) + } + } else { + android.ReportPathErrorf(ctx, "%w", err) + } + } + + return config } // SetTestGlobalConfig sets a GlobalConfig that future calls to GetGlobalConfig // will return. It must be called before the first call to GetGlobalConfig for // the config. func SetTestGlobalConfig(config android.Config, globalConfig *GlobalConfig) { - config.Once(testGlobalConfigOnceKey, func() interface{} { return globalConfigAndRaw{globalConfig, nil} }) + config.Once(testGlobalConfigOnceKey, func() interface{} { return globalConfigAndRaw{globalConfig, nil, nil} }) } // This struct is required to convert ModuleConfig from/to JSON.