From abdb293492018ba8fe29a94817e089fc8174fa56 Mon Sep 17 00:00:00 2001 From: mrziwang Date: Tue, 18 Jun 2024 12:43:41 -0700 Subject: [PATCH] Update outputFilesForModuleFromProvider This CL includes following changes: 1. Added the ability to differentiate the cases that module never sets OutputFilesProvider and that module sets the provider with a nil value. 2. Updated GenerateTaggedDistFiles to use outputFilesForModuleFromProvider. 3. Updated on cc module to use OutputFilesProvider. Test: CI Bug: 339477385 Change-Id: Ib5663a947315f6a90a81b7f073cf8dd22fbb1e05 --- android/module.go | 56 +++++++++++++++++++++++++++++----------------- android/testing.go | 9 +++++--- cc/binary.go | 2 +- cc/cc.go | 37 ++++++++++++------------------ cc/cc_test.go | 34 +++++++++------------------- cc/object.go | 2 +- 6 files changed, 68 insertions(+), 72 deletions(-) diff --git a/android/module.go b/android/module.go index e2c24bfdf..c08d2f42e 100644 --- a/android/module.go +++ b/android/module.go @@ -1234,17 +1234,28 @@ func (m *ModuleBase) GenerateTaggedDistFiles(ctx BaseModuleContext) TaggedDistFi // the special tag name which represents that. tag := proptools.StringDefault(dist.Tag, DefaultDistTag) + distFileForTagFromProvider, err := outputFilesForModuleFromProvider(ctx, m.module, tag) + if err != OutputFilesProviderNotSet { + if err != nil && tag != DefaultDistTag { + ctx.PropertyErrorf("dist.tag", "%s", err.Error()) + } else { + distFiles = distFiles.addPathsForTag(tag, distFileForTagFromProvider...) + continue + } + } + + // if the tagged dist file cannot be obtained from OutputFilesProvider, + // fall back to use OutputFileProducer + // TODO: remove this part after OutputFilesProvider fully replaces OutputFileProducer if outputFileProducer, ok := m.module.(OutputFileProducer); ok { // Call the OutputFiles(tag) method to get the paths associated with the tag. distFilesForTag, err := outputFileProducer.OutputFiles(tag) - // If the tag was not supported and is not DefaultDistTag then it is an error. // Failing to find paths for DefaultDistTag is not an error. It just means // that the module type requires the legacy behavior. if err != nil && tag != DefaultDistTag { ctx.PropertyErrorf("dist.tag", "%s", err.Error()) } - distFiles = distFiles.addPathsForTag(tag, distFilesForTag...) } else if tag != DefaultDistTag { // If the tag was specified then it is an error if the module does not @@ -2513,7 +2524,7 @@ 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 { + if outputFilesFromProvider != nil || err != OutputFilesProviderNotSet { return outputFilesFromProvider, err } if outputFileProducer, ok := module.(OutputFileProducer); ok { @@ -2541,38 +2552,38 @@ func outputFilesForModule(ctx PathContext, module blueprint.Module, tag string) // reading OutputFilesProvider before GenerateBuildActions is finished. // If a module doesn't have the OutputFilesProvider, nil is returned. func outputFilesForModuleFromProvider(ctx PathContext, module blueprint.Module, tag string) (Paths, error) { - var outputFilesProvider OutputFilesInfo + var outputFiles OutputFilesInfo + fromProperty := false if mctx, isMctx := ctx.(ModuleContext); isMctx { if mctx.Module() != module { - outputFilesProvider, _ = OtherModuleProvider(mctx, module, OutputFilesProvider) + outputFiles, _ = OtherModuleProvider(mctx, module, OutputFilesProvider) } else { - if tag == "" { - return mctx.Module().base().outputFiles.DefaultOutputFiles, nil - } else if taggedOutputFiles, hasTag := mctx.Module().base().outputFiles.TaggedOutputFiles[tag]; hasTag { - return taggedOutputFiles, nil - } else { - return nil, fmt.Errorf("unsupported tag %q for module getting its own output files", tag) - } + outputFiles = mctx.Module().base().outputFiles + fromProperty = true } } else if cta, isCta := ctx.(*singletonContextAdaptor); isCta { providerData, _ := cta.moduleProvider(module, OutputFilesProvider) - outputFilesProvider, _ = providerData.(OutputFilesInfo) + outputFiles, _ = providerData.(OutputFilesInfo) } // TODO: Add a check for skipped context - if !outputFilesProvider.isEmpty() { - if tag == "" { - return outputFilesProvider.DefaultOutputFiles, nil - } else if taggedOutputFiles, hasTag := outputFilesProvider.TaggedOutputFiles[tag]; hasTag { - return taggedOutputFiles, nil + if outputFiles.isEmpty() { + // TODO: Add a check for param module not having OutputFilesProvider set + return nil, OutputFilesProviderNotSet + } + + if tag == "" { + return outputFiles.DefaultOutputFiles, nil + } else if taggedOutputFiles, hasTag := outputFiles.TaggedOutputFiles[tag]; hasTag { + return taggedOutputFiles, nil + } else { + if fromProperty { + return nil, fmt.Errorf("unsupported tag %q for module getting its own output files", tag) } else { return nil, fmt.Errorf("unsupported module reference tag %q", tag) } } - - // TODO: Add a check for param module not having OutputFilesProvider set - return nil, nil } func (o OutputFilesInfo) isEmpty() bool { @@ -2589,6 +2600,9 @@ type OutputFilesInfo struct { var OutputFilesProvider = blueprint.NewProvider[OutputFilesInfo]() +// This is used to mark the case where OutputFilesProvider is not set on some modules. +var OutputFilesProviderNotSet = fmt.Errorf("No output files from provider") + // 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/testing.go b/android/testing.go index 8dd467dce..18fd3b31c 100644 --- a/android/testing.go +++ b/android/testing.go @@ -1025,9 +1025,12 @@ func (m TestingModule) VariablesForTestsRelativeToTop() map[string]string { // otherwise returns the result of calling Paths.RelativeToTop // on the returned Paths. func (m TestingModule) OutputFiles(t *testing.T, tag string) Paths { - // TODO: add non-empty-string tag case and remove OutputFileProducer part - if tag == "" && m.module.base().outputFiles.DefaultOutputFiles != nil { - return m.module.base().outputFiles.DefaultOutputFiles.RelativeToTop() + // TODO: remove OutputFileProducer part + outputFiles := m.Module().base().outputFiles + if tag == "" && outputFiles.DefaultOutputFiles != nil { + return outputFiles.DefaultOutputFiles.RelativeToTop() + } else if taggedOutputFiles, hasTag := outputFiles.TaggedOutputFiles[tag]; hasTag { + return taggedOutputFiles } producer, ok := m.module.(OutputFileProducer) diff --git a/cc/binary.go b/cc/binary.go index 3ff35de56..2ac9a45bc 100644 --- a/cc/binary.go +++ b/cc/binary.go @@ -451,7 +451,7 @@ func (binary *binaryDecorator) unstrippedOutputFilePath() android.Path { } func (binary *binaryDecorator) strippedAllOutputFilePath() android.Path { - panic("Not implemented.") + return nil } func (binary *binaryDecorator) setSymlinkList(ctx ModuleContext) { diff --git a/cc/cc.go b/cc/cc.go index d8fe3199b..3a8932cf7 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -2119,10 +2119,23 @@ func (c *Module) GenerateAndroidBuildActions(actx android.ModuleContext) { if c.Properties.IsSdkVariant && c.Properties.SdkAndPlatformVariantVisibleToMake { moduleInfoJSON.Uninstallable = true } - } buildComplianceMetadataInfo(ctx, c, deps) + + c.setOutputFiles(ctx) +} + +func (c *Module) setOutputFiles(ctx ModuleContext) { + if c.outputFile.Valid() { + ctx.SetOutputFiles(android.Paths{c.outputFile.Path()}, "") + } else { + ctx.SetOutputFiles(android.Paths{}, "") + } + if c.linker != nil { + ctx.SetOutputFiles(android.PathsIfNonNil(c.linker.unstrippedOutputFilePath()), "unstripped") + ctx.SetOutputFiles(android.PathsIfNonNil(c.linker.strippedAllOutputFilePath()), "stripped_all") + } } func buildComplianceMetadataInfo(ctx ModuleContext, c *Module, deps PathDeps) { @@ -3615,28 +3628,6 @@ func (c *Module) IntermPathForModuleOut() android.OptionalPath { return c.outputFile } -func (c *Module) OutputFiles(tag string) (android.Paths, error) { - switch tag { - case "": - if c.outputFile.Valid() { - return android.Paths{c.outputFile.Path()}, nil - } - return android.Paths{}, nil - case "unstripped": - if c.linker != nil { - return android.PathsIfNonNil(c.linker.unstrippedOutputFilePath()), nil - } - return nil, nil - case "stripped_all": - if c.linker != nil { - return android.PathsIfNonNil(c.linker.strippedAllOutputFilePath()), nil - } - return nil, nil - default: - return nil, fmt.Errorf("unsupported module reference tag %q", tag) - } -} - func (c *Module) static() bool { if static, ok := c.linker.(interface { static() bool diff --git a/cc/cc_test.go b/cc/cc_test.go index 776d13334..ccdaae58f 100644 --- a/cc/cc_test.go +++ b/cc/cc_test.go @@ -300,13 +300,9 @@ func TestDataLibs(t *testing.T) { config := TestConfig(t.TempDir(), android.Android, nil, bp, nil) ctx := testCcWithConfig(t, config) - module := ctx.ModuleForTests("main_test", "android_arm_armv7-a-neon").Module() - testBinary := module.(*Module).linker.(*testBinary) - outputFiles, err := module.(android.OutputFileProducer).OutputFiles("") - if err != nil { - t.Errorf("Expected cc_test to produce output files, error: %s", err) - return - } + testingModule := ctx.ModuleForTests("main_test", "android_arm_armv7-a-neon") + testBinary := testingModule.Module().(*Module).linker.(*testBinary) + outputFiles := testingModule.OutputFiles(t, "") if len(outputFiles) != 1 { t.Errorf("expected exactly one output file. output files: [%s]", outputFiles) return @@ -356,12 +352,10 @@ func TestDataLibsRelativeInstallPath(t *testing.T) { config := TestConfig(t.TempDir(), android.Android, nil, bp, nil) ctx := testCcWithConfig(t, config) - module := ctx.ModuleForTests("main_test", "android_arm_armv7-a-neon").Module() + testingModule := ctx.ModuleForTests("main_test", "android_arm_armv7-a-neon") + module := testingModule.Module() testBinary := module.(*Module).linker.(*testBinary) - outputFiles, err := module.(android.OutputFileProducer).OutputFiles("") - if err != nil { - t.Fatalf("Expected cc_test to produce output files, error: %s", err) - } + outputFiles := testingModule.OutputFiles(t, "") if len(outputFiles) != 1 { t.Fatalf("expected exactly one output file. output files: [%s]", outputFiles) } @@ -1407,12 +1401,10 @@ func TestDataLibsPrebuiltSharedTestLibrary(t *testing.T) { config := TestConfig(t.TempDir(), android.Android, nil, bp, nil) ctx := testCcWithConfig(t, config) - module := ctx.ModuleForTests("main_test", "android_arm_armv7-a-neon").Module() + testingModule := ctx.ModuleForTests("main_test", "android_arm_armv7-a-neon") + module := testingModule.Module() testBinary := module.(*Module).linker.(*testBinary) - outputFiles, err := module.(android.OutputFileProducer).OutputFiles("") - if err != nil { - t.Fatalf("Expected cc_test to produce output files, error: %s", err) - } + outputFiles := testingModule.OutputFiles(t, "") if len(outputFiles) != 1 { t.Errorf("expected exactly one output file. output files: [%s]", outputFiles) } @@ -3118,12 +3110,8 @@ func TestStrippedAllOutputFile(t *testing.T) { ` config := TestConfig(t.TempDir(), android.Android, nil, bp, nil) ctx := testCcWithConfig(t, config) - module := ctx.ModuleForTests("test_lib", "android_arm_armv7-a-neon_shared").Module() - outputFile, err := module.(android.OutputFileProducer).OutputFiles("stripped_all") - if err != nil { - t.Errorf("Expected cc_library to produce output files, error: %s", err) - return - } + testingModule := ctx.ModuleForTests("test_lib", "android_arm_armv7-a-neon_shared") + outputFile := testingModule.OutputFiles(t, "stripped_all") if !strings.HasSuffix(outputFile.Strings()[0], "/stripped_all/test_lib.so") { t.Errorf("Unexpected output file: %s", outputFile.Strings()[0]) return diff --git a/cc/object.go b/cc/object.go index 6c0391f3b..8b23295c7 100644 --- a/cc/object.go +++ b/cc/object.go @@ -220,7 +220,7 @@ func (object *objectLinker) unstrippedOutputFilePath() android.Path { } func (object *objectLinker) strippedAllOutputFilePath() android.Path { - panic("Not implemented.") + return nil } func (object *objectLinker) nativeCoverage() bool {