From e6c8581fbef9ee7d9cd859b61ba07564ea1299c2 Mon Sep 17 00:00:00 2001 From: mrziwang Date: Wed, 22 May 2024 14:36:09 -0700 Subject: [PATCH] Use OutputFilesProvider on bpf This changes makes bpf module type uses OutputFilesProvider, instead of current OutputFileProducer for inter-module- communication. Test: CI Bug: 339477385 Bug: 342406930 Change-Id: I85d1141e9f6583cc5427756107da99f56b0c7ea1 --- android/module.go | 62 +++++++++++++++++++++++++++++++++++++-- android/module_context.go | 19 ++++++++++++ android/paths.go | 18 ++++-------- apex/apex.go | 2 +- bpf/bpf.go | 17 ++--------- java/sdk_library_test.go | 2 +- 6 files changed, 88 insertions(+), 32 deletions(-) diff --git a/android/module.go b/android/module.go index fed1d0e7b..dc585d295 100644 --- a/android/module.go +++ b/android/module.go @@ -915,6 +915,10 @@ type ModuleBase struct { // moduleInfoJSON can be filled out by GenerateAndroidBuildActions to write a JSON file that will // be included in the final module-info.json produced by Make. moduleInfoJSON *ModuleInfoJSON + + // outputFiles stores the output of a module by tag and is used to set + // the OutputFilesProvider in GenerateBuildActions + outputFiles OutputFilesInfo } func (m *ModuleBase) AddJSONData(d *map[string]interface{}) { @@ -1996,6 +2000,10 @@ func (m *ModuleBase) GenerateBuildActions(blueprintCtx blueprint.ModuleContext) m.buildParams = ctx.buildParams m.ruleParams = ctx.ruleParams m.variables = ctx.variables + + if m.outputFiles.DefaultOutputFiles != nil || m.outputFiles.TaggedOutputFiles != nil { + SetProvider(ctx, OutputFilesProvider, m.outputFiles) + } } func SetJarJarPrefixHandler(handler func(ModuleContext)) { @@ -2445,11 +2453,15 @@ func OutputFileForModule(ctx PathContext, module blueprint.Module, tag string) P } func outputFilesForModule(ctx PathContext, module blueprint.Module, tag string) (Paths, error) { + outputFilesFromProvider, err := outputFilesForModuleFromProvider(ctx, module, tag) + if outputFilesFromProvider != nil || err != nil { + return outputFilesFromProvider, err + } if outputFileProducer, ok := module.(OutputFileProducer); ok { paths, err := outputFileProducer.OutputFiles(tag) if err != nil { - return nil, fmt.Errorf("failed to get output file from module %q: %s", - pathContextName(ctx, module), err.Error()) + return nil, fmt.Errorf("failed to get output file from module %q at tag %q: %s", + pathContextName(ctx, module), tag, err.Error()) } return paths, nil } else if sourceFileProducer, ok := module.(SourceFileProducer); ok { @@ -2459,10 +2471,54 @@ func outputFilesForModule(ctx PathContext, module blueprint.Module, tag string) paths := sourceFileProducer.Srcs() return paths, nil } else { - return nil, fmt.Errorf("module %q is not an OutputFileProducer", pathContextName(ctx, module)) + return nil, fmt.Errorf("module %q is not an OutputFileProducer or SourceFileProducer", pathContextName(ctx, module)) } } +// This method uses OutputFilesProvider for output files +// *inter-module-communication*. +// If mctx module is the same as the param module the output files are obtained +// from outputFiles property of module base, to avoid both setting and +// reading OutputFilesProvider before GenerateBuildActions is finished. Also +// only empty-string-tag is supported in this case. +// If a module doesn't have the OutputFilesProvider, nil is returned. +func outputFilesForModuleFromProvider(ctx PathContext, module blueprint.Module, tag string) (Paths, error) { + // TODO: support OutputFilesProvider for singletons + mctx, ok := ctx.(ModuleContext) + if !ok { + return nil, nil + } + if mctx.Module() != module { + if outputFilesProvider, ok := OtherModuleProvider(mctx, module, OutputFilesProvider); ok { + if tag == "" { + return outputFilesProvider.DefaultOutputFiles, nil + } else if taggedOutputFiles, hasTag := outputFilesProvider.TaggedOutputFiles[tag]; hasTag { + return taggedOutputFiles, nil + } else { + return nil, fmt.Errorf("unsupported module reference tag %q", tag) + } + } + } else { + if tag == "" { + return mctx.Module().base().outputFiles.DefaultOutputFiles, nil + } else { + return nil, fmt.Errorf("unsupported tag %q for module getting its own output files", tag) + } + } + // TODO: Add a check for param module not having OutputFilesProvider set + return nil, nil +} + +type OutputFilesInfo struct { + // default output files when tag is an empty string "" + DefaultOutputFiles Paths + + // the corresponding output files for given tags + TaggedOutputFiles map[string]Paths +} + +var OutputFilesProvider = blueprint.NewProvider[OutputFilesInfo]() + // Modules can implement HostToolProvider and return a valid OptionalPath from HostToolPath() to // specify that they can be used as a tool by a genrule module. type HostToolProvider interface { diff --git a/android/module_context.go b/android/module_context.go index 18adb3002..bc089114f 100644 --- a/android/module_context.go +++ b/android/module_context.go @@ -212,6 +212,10 @@ type ModuleContext interface { // GenerateAndroidBuildActions. If it is called then the struct will be written out and included in // the module-info.json generated by Make, and Make will not generate its own data for this module. ModuleInfoJSON() *ModuleInfoJSON + + // SetOutputFiles stores the outputFiles to outputFiles property, which is used + // to set the OutputFilesProvider later. + SetOutputFiles(outputFiles Paths, tag string) } type moduleContext struct { @@ -707,6 +711,21 @@ func (m *moduleContext) ModuleInfoJSON() *ModuleInfoJSON { return moduleInfoJSON } +func (m *moduleContext) SetOutputFiles(outputFiles Paths, tag string) { + if tag == "" { + if len(m.module.base().outputFiles.DefaultOutputFiles) > 0 { + m.ModuleErrorf("Module %s default OutputFiles cannot be overwritten", m.ModuleName()) + } + m.module.base().outputFiles.DefaultOutputFiles = outputFiles + } else { + if _, exists := m.module.base().outputFiles.TaggedOutputFiles[tag]; exists { + m.ModuleErrorf("Module %s OutputFiles at tag %s cannot be overwritten", m.ModuleName(), tag) + } else { + m.module.base().outputFiles.TaggedOutputFiles[tag] = outputFiles + } + } +} + // Returns a list of paths expanded from globs and modules referenced using ":module" syntax. The property must // be tagged with `android:"path" to support automatic source module dependency resolution. // diff --git a/android/paths.go b/android/paths.go index 8d92aa4a9..edc07000c 100644 --- a/android/paths.go +++ b/android/paths.go @@ -565,21 +565,15 @@ func getPathsFromModuleDep(ctx ModuleWithDepsPathContext, path, moduleName, tag if aModule, ok := module.(Module); ok && !aModule.Enabled(ctx) { return nil, missingDependencyError{[]string{moduleName}} } - if outProducer, ok := module.(OutputFileProducer); ok { - outputFiles, err := outProducer.OutputFiles(tag) - if err != nil { - return nil, fmt.Errorf("path dependency %q: %s", path, err) - } - return outputFiles, nil - } else if tag != "" { - return nil, fmt.Errorf("path dependency %q is not an output file producing module", path) - } else if goBinary, ok := module.(bootstrap.GoBinaryTool); ok { + if goBinary, ok := module.(bootstrap.GoBinaryTool); ok && tag == "" { goBinaryPath := PathForGoBinary(ctx, goBinary) return Paths{goBinaryPath}, nil - } else if srcProducer, ok := module.(SourceFileProducer); ok { - return srcProducer.Srcs(), nil + } + outputFiles, err := outputFilesForModule(ctx, module, tag) + if outputFiles != nil && err == nil { + return outputFiles, nil } else { - return nil, fmt.Errorf("path dependency %q is not a source file producing module", path) + return nil, err } } diff --git a/apex/apex.go b/apex/apex.go index 1dec61b6d..e79afadc1 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -2112,7 +2112,7 @@ func (a *apexBundle) depVisitor(vctx *visitorContext, ctx android.ModuleContext, } case bpfTag: if bpfProgram, ok := child.(bpf.BpfModule); ok { - filesToCopy, _ := bpfProgram.OutputFiles("") + filesToCopy := android.OutputFilesForModule(ctx, bpfProgram, "") apex_sub_dir := bpfProgram.SubDir() for _, bpfFile := range filesToCopy { vctx.filesInfo = append(vctx.filesInfo, apexFileForBpfProgram(ctx, bpfFile, apex_sub_dir, bpfProgram)) diff --git a/bpf/bpf.go b/bpf/bpf.go index 38fbd8804..2eb869ebe 100644 --- a/bpf/bpf.go +++ b/bpf/bpf.go @@ -65,8 +65,6 @@ var PrepareForTestWithBpf = android.FixtureRegisterWithContext(registerBpfBuildC type BpfModule interface { android.Module - OutputFiles(tag string) (android.Paths, error) - // Returns the sub install directory if the bpf module is included by apex. SubDir() string } @@ -213,6 +211,8 @@ func (bpf *bpf) GenerateAndroidBuildActions(ctx android.ModuleContext) { } android.SetProvider(ctx, blueprint.SrcsFileProviderKey, blueprint.SrcsFileProviderData{SrcPaths: srcs.Strings()}) + + ctx.SetOutputFiles(bpf.objs, "") } func (bpf *bpf) AndroidMk() android.AndroidMkData { @@ -255,23 +255,10 @@ func (bpf *bpf) AndroidMk() android.AndroidMkData { } } -// Implements OutputFileFileProducer interface so that the obj output can be used in the data property -// of other modules. -func (bpf *bpf) OutputFiles(tag string) (android.Paths, error) { - switch tag { - case "": - return bpf.objs, nil - default: - return nil, fmt.Errorf("unsupported module reference tag %q", tag) - } -} - func (bpf *bpf) SubDir() string { return bpf.properties.Sub_dir } -var _ android.OutputFileProducer = (*bpf)(nil) - func BpfFactory() android.Module { module := &bpf{} diff --git a/java/sdk_library_test.go b/java/sdk_library_test.go index d240e701b..39f8c760b 100644 --- a/java/sdk_library_test.go +++ b/java/sdk_library_test.go @@ -492,7 +492,7 @@ func TestJavaSdkLibrary_AccessOutputFiles_NoAnnotations(t *testing.T) { PrepareForTestWithJavaSdkLibraryFiles, FixtureWithLastReleaseApis("foo"), ). - ExtendWithErrorHandler(android.FixtureExpectsAtLeastOneErrorMatchingPattern(`module "bar" variant "android_common": path dependency ":foo{.public.annotations.zip}": annotations.zip not available for api scope public`)). + ExtendWithErrorHandler(android.FixtureExpectsAtLeastOneErrorMatchingPattern(`module "bar" variant "android_common": failed to get output file from module "foo" at tag ".public.annotations.zip": annotations.zip not available for api scope public`)). RunTestWithBp(t, ` java_sdk_library { name: "foo",