Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 67118217 authored by Kiyoung Kim's avatar Kiyoung Kim
Browse files

Use common interface to build linker configuration

Current filesystem logic is quite separated with systemimage, so it can
end up generating linker configuration twice which can end up an error
case. This change creates a common interface and let both filesystem and
systemimage can implement their own function to generate linker
configuration.

Bug: 324995772
Test: Compared linker.config.pb of generic_system_image with this patch
Change-Id: Ic515e54deeafbae74fd02bb023661606aa7e0b7c
parent 45aeb5f9
Loading
Loading
Loading
Loading
+4 −1
Original line number Diff line number Diff line
@@ -10426,7 +10426,10 @@ func TestFileSystemShouldSkipApexLibraries(t *testing.T) {
			deps: [
				"libfoo",
			],
			linker_config_src: "linker.config.json",
			linkerconfig: {
				gen_linker_config: true,
				linker_config_srcs: ["linker.config.json"],
			},
		}

		cc_library {
+16 −31
Original line number Diff line number Diff line
@@ -52,12 +52,6 @@ type filesystem struct {

	properties FilesystemProperties

	// Function that builds extra files under the root directory and returns the files
	buildExtraFiles func(ctx android.ModuleContext, root android.OutputPath) android.OutputPaths

	// Function that filters PackagingSpec in PackagingBase.GatherPackagingSpecs()
	filterPackagingSpec func(spec android.PackagingSpec) bool

	output     android.OutputPath
	installDir android.InstallPath

@@ -65,8 +59,18 @@ type filesystem struct {

	// Keeps the entries installed from this filesystem
	entries []string

	filesystemBuilder filesystemBuilder
}

type filesystemBuilder interface {
	BuildLinkerConfigFile(ctx android.ModuleContext, builder *android.RuleBuilder, rebasedDir android.OutputPath)
	// Function that filters PackagingSpec in PackagingBase.GatherPackagingSpecs()
	FilterPackagingSpec(spec android.PackagingSpec) bool
}

var _ filesystemBuilder = (*filesystem)(nil)

type SymlinkDefinition struct {
	Target *string
	Name   *string
@@ -183,7 +187,7 @@ type LinkerConfigProperties struct {
// partitions like system.img. For example, cc_library modules are placed under ./lib[64] directory.
func FilesystemFactory() android.Module {
	module := &filesystem{}
	module.filterPackagingSpec = module.filterInstallablePackagingSpec
	module.filesystemBuilder = module
	initFilesystemModule(module, module)
	return module
}
@@ -275,7 +279,7 @@ func (f *filesystem) partitionName() string {
	return proptools.StringDefault(f.properties.Partition_name, f.Name())
}

func (f *filesystem) filterInstallablePackagingSpec(ps android.PackagingSpec) bool {
func (f *filesystem) FilterPackagingSpec(ps android.PackagingSpec) bool {
	// Filesystem module respects the installation semantic. A PackagingSpec from a module with
	// IsSkipInstall() is skipped.
	if proptools.Bool(f.properties.Is_auto_generated) { // TODO (spandandas): Remove this.
@@ -377,25 +381,6 @@ func (f *filesystem) buildNonDepsFiles(ctx android.ModuleContext, builder *andro
		builder.Command().Text("ln -sf").Text(proptools.ShellEscape(target)).Text(dst.String())
		f.appendToEntry(ctx, dst)
	}

	// create extra files if there's any
	if f.buildExtraFiles != nil {
		rootForExtraFiles := android.PathForModuleGen(ctx, "root-extra").OutputPath
		extraFiles := f.buildExtraFiles(ctx, rootForExtraFiles)
		for _, extraFile := range extraFiles {
			rel, err := filepath.Rel(rootForExtraFiles.String(), extraFile.String())
			if err != nil || strings.HasPrefix(rel, "..") {
				ctx.ModuleErrorf("can't make %q relative to %q", extraFile, rootForExtraFiles)
			}
			f.appendToEntry(ctx, rootDir.Join(ctx, rel))
		}
		if len(extraFiles) > 0 {
			builder.Command().BuiltTool("merge_directories").
				Implicits(extraFiles.Paths()).
				Text(rootDir.String()).
				Text(rootForExtraFiles.String())
		}
	}
}

func (f *filesystem) copyPackagingSpecs(ctx android.ModuleContext, builder *android.RuleBuilder, specs map[string]android.PackagingSpec, rootDir, rebasedDir android.WritablePath) []string {
@@ -442,7 +427,7 @@ func (f *filesystem) buildImageUsingBuildImage(ctx android.ModuleContext) androi
	f.buildFsverityMetadataFiles(ctx, builder, specs, rootDir, rebasedDir)
	f.buildEventLogtagsFile(ctx, builder, rebasedDir)
	f.buildAconfigFlagsFiles(ctx, builder, specs, rebasedDir)
	f.buildLinkerConfigFile(ctx, builder, rebasedDir)
	f.filesystemBuilder.BuildLinkerConfigFile(ctx, builder, rebasedDir)
	f.copyFilesToProductOut(ctx, builder, rebasedDir)

	// run host_init_verifier
@@ -606,7 +591,7 @@ func (f *filesystem) buildCpioImage(ctx android.ModuleContext, compressed bool)
	f.buildFsverityMetadataFiles(ctx, builder, specs, rootDir, rebasedDir)
	f.buildEventLogtagsFile(ctx, builder, rebasedDir)
	f.buildAconfigFlagsFiles(ctx, builder, specs, rebasedDir)
	f.buildLinkerConfigFile(ctx, builder, rebasedDir)
	f.filesystemBuilder.BuildLinkerConfigFile(ctx, builder, rebasedDir)
	f.copyFilesToProductOut(ctx, builder, rebasedDir)

	output := android.PathForModuleOut(ctx, f.installFileName()).OutputPath
@@ -698,7 +683,7 @@ func (f *filesystem) buildEventLogtagsFile(ctx android.ModuleContext, builder *a
	f.appendToEntry(ctx, eventLogtagsPath)
}

func (f *filesystem) buildLinkerConfigFile(ctx android.ModuleContext, builder *android.RuleBuilder, rebasedDir android.OutputPath) {
func (f *filesystem) BuildLinkerConfigFile(ctx android.ModuleContext, builder *android.RuleBuilder, rebasedDir android.OutputPath) {
	if !proptools.Bool(f.properties.Linkerconfig.Gen_linker_config) {
		return
	}
@@ -765,7 +750,7 @@ func (f *filesystem) SignedOutputPath() android.Path {
// Note that "apex" module installs its contents to "apex"(fake partition) as well
// for symbol lookup by imitating "activated" paths.
func (f *filesystem) gatherFilteredPackagingSpecs(ctx android.ModuleContext) map[string]android.PackagingSpec {
	specs := f.PackagingBase.GatherPackagingSpecsWithFilter(ctx, f.filterPackagingSpec)
	specs := f.PackagingBase.GatherPackagingSpecsWithFilter(ctx, f.filesystemBuilder.FilterPackagingSpec)
	return specs
}

+16 −6
Original line number Diff line number Diff line
@@ -156,11 +156,15 @@ func TestFileSystemFillsLinkerConfigWithStubLibs(t *testing.T) {
	result := fixture.RunTestWithBp(t, `
		android_system_image {
			name: "myfilesystem",
			base_dir: "system",
			deps: [
				"libfoo",
				"libbar",
			],
			linker_config_src: "linker.config.json",
			linkerconfig: {
				gen_linker_config: true,
				linker_config_srcs: ["linker.config.json"],
			},
		}

		cc_library {
@@ -176,7 +180,7 @@ func TestFileSystemFillsLinkerConfigWithStubLibs(t *testing.T) {
	`)

	module := result.ModuleForTests("myfilesystem", "android_common")
	output := module.Output("system/etc/linker.config.pb")
	output := module.Output("out/soong/.intermediates/myfilesystem/android_common/root/system/etc/linker.config.pb")

	android.AssertStringDoesContain(t, "linker.config.pb should have libfoo",
		output.RuleParams.Command, "libfoo.so")
@@ -223,7 +227,10 @@ func TestFileSystemGathersItemsOnlyInSystemPartition(t *testing.T) {
					deps: ["foo"],
				},
			},
			linker_config_src: "linker.config.json",
			linkerconfig: {
				gen_linker_config: true,
				linker_config_srcs: ["linker.config.json"],
			},
		}
		component {
			name: "foo",
@@ -318,7 +325,10 @@ func TestFileSystemWithCoverageVariants(t *testing.T) {
			deps: [
				"libfoo",
			],
			linker_config_src: "linker.config.json",
			linkerconfig: {
				gen_linker_config: true,
				linker_config_srcs: ["linker.config.json"],
			},
		}

		cc_library {
+12 −25
Original line number Diff line number Diff line
@@ -17,27 +17,22 @@ package filesystem
import (
	"android/soong/android"
	"android/soong/linkerconfig"

	"github.com/google/blueprint/proptools"
)

type systemImage struct {
	filesystem

	properties systemImageProperties
}

type systemImageProperties struct {
	// Path to the input linker config json file.
	Linker_config_src *string `android:"path"`
}
var _ filesystemBuilder = (*systemImage)(nil)

// android_system_image is a specialization of android_filesystem for the 'system' partition.
// Currently, the only difference is the inclusion of linker.config.pb file which specifies
// the provided and the required libraries to and from APEXes.
func SystemImageFactory() android.Module {
	module := &systemImage{}
	module.AddProperties(&module.properties)
	module.filesystem.buildExtraFiles = module.buildExtraFiles
	module.filesystem.filterPackagingSpec = module.filterPackagingSpec
	module.filesystemBuilder = module
	initFilesystemModule(module, &module.filesystem)
	return module
}
@@ -46,30 +41,22 @@ func (s systemImage) FsProps() FilesystemProperties {
	return s.filesystem.properties
}

func (s *systemImage) buildExtraFiles(ctx android.ModuleContext, root android.OutputPath) android.OutputPaths {
	if s.filesystem.properties.Partition_type != nil {
		ctx.PropertyErrorf("partition_type", "partition_type must be unset on an android_system_image module. It is assumed to be 'system'.")
func (s *systemImage) BuildLinkerConfigFile(ctx android.ModuleContext, builder *android.RuleBuilder, rebasedDir android.OutputPath) {
	if !proptools.Bool(s.filesystem.properties.Linkerconfig.Gen_linker_config) {
		return
	}
	lc := s.buildLinkerConfigFile(ctx, root)
	// Add more files if needed
	return []android.OutputPath{lc}
}

func (s *systemImage) buildLinkerConfigFile(ctx android.ModuleContext, root android.OutputPath) android.OutputPath {
	input := android.PathForModuleSrc(ctx, android.String(s.properties.Linker_config_src))
	output := root.Join(ctx, "system", "etc", "linker.config.pb")

	provideModules, requireModules := s.getLibsForLinkerConfig(ctx)
	builder := android.NewRuleBuilder(pctx, ctx)
	linkerconfig.BuildLinkerConfig(ctx, builder, android.Paths{input}, provideModules, requireModules, output)
	builder.Build("conv_linker_config", "Generate linker config protobuf "+output.String())
	return output
	output := rebasedDir.Join(ctx, "etc", "linker.config.pb")
	linkerconfig.BuildLinkerConfig(ctx, builder, android.PathsForModuleSrc(ctx, s.filesystem.properties.Linkerconfig.Linker_config_srcs), provideModules, requireModules, output)

	s.appendToEntry(ctx, output)
}

// Filter the result of GatherPackagingSpecs to discard items targeting outside "system" / "root"
// partition.  Note that "apex" module installs its contents to "apex"(fake partition) as well
// for symbol lookup by imitating "activated" paths.
func (s *systemImage) filterPackagingSpec(ps android.PackagingSpec) bool {
func (s *systemImage) FilterPackagingSpec(ps android.PackagingSpec) bool {
	return !ps.SkipInstall() &&
		(ps.Partition() == "system" || ps.Partition() == "root")
}