From b1692a3468cd7def123c3b7c58f6a60a47821841 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Mon, 25 Oct 2021 15:39:01 -0700 Subject: [PATCH 1/4] Remove PathForOutput from InstallPathToOnDevicePath The next patches will make more InstallPaths use the Make output directory, which is not inside PathForOutput. Fix the assumption that the InstallPath is relative to PathForOutput by storing enough information in the InstallPath to find the on-device path without PathForOutput. Bug: 204136549 Test: soong tests Change-Id: Icb3eeef3f1c72f773f333267f8a7dfc503feacb5 --- android/paths.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/android/paths.go b/android/paths.go index 2e378baa5..56f36ea08 100644 --- a/android/paths.go +++ b/android/paths.go @@ -1571,6 +1571,8 @@ type InstallPath struct { // For example, it is host/- for host modules, and target/product// for device modules. partitionDir string + partition string + // makePath indicates whether this path is for Soong (false) or Make (true). makePath bool } @@ -1716,6 +1718,7 @@ func pathForInstall(ctx PathContext, os OsType, arch ArchType, partition string, basePath: basePath{partionPath, ""}, soongOutDir: ctx.Config().soongOutDir, partitionDir: partionPath, + partition: partition, makePath: false, } @@ -1741,8 +1744,7 @@ func PathForMainlineSdksInstall(ctx PathContext, paths ...string) InstallPath { } func InstallPathToOnDevicePath(ctx PathContext, path InstallPath) string { - rel := Rel(ctx, PathForOutput(ctx, "target", "product", ctx.Config().DeviceName()).String(), path.String()) - + rel := Rel(ctx, strings.TrimSuffix(path.PartitionDir(), path.partition), path.String()) return "/" + rel } From a44551fec673cf1e851ea0423fe5b6e383de7d88 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Mon, 25 Oct 2021 15:36:21 -0700 Subject: [PATCH 2/4] Add PathForGoBinary Add PathForGoBinary that uses pathForInstall to return the install path of a GoBinaryTool. This will replace various places that used PathForOutput to reconstruct a path to a Go tool, and will support moving Go tools to the Make install directory outside of the PathForOutput directory in a future patch. Bug: 204136549 Test: m checkbuild Change-Id: I83a3be9f5c621975540f5ed601a0b9e2611c98b9 --- android/paths.go | 14 +++++++++----- apex/apex.go | 7 +------ genrule/genrule.go | 11 +++-------- 3 files changed, 13 insertions(+), 19 deletions(-) diff --git a/android/paths.go b/android/paths.go index 56f36ea08..69ab5f75c 100644 --- a/android/paths.go +++ b/android/paths.go @@ -462,6 +462,13 @@ func (p OutputPaths) Strings() []string { return ret } +// PathForGoBinary returns the path to the installed location of a bootstrap_go_binary module. +func PathForGoBinary(ctx PathContext, goBinary bootstrap.GoBinaryTool) Path { + goBinaryInstallDir := pathForInstall(ctx, ctx.Config().BuildOS, ctx.Config().BuildArch, "bin", false) + rel := Rel(ctx, goBinaryInstallDir.String(), goBinary.InstallPath()) + return goBinaryInstallDir.Join(ctx, rel) +} + // Expands Paths to a SourceFileProducer or OutputFileProducer module dependency referenced via ":name" or ":name{.tag}" syntax. // If the dependency is not found, a missingErrorDependency is returned. // If the module dependency is not a SourceFileProducer or OutputFileProducer, appropriate errors will be returned. @@ -482,11 +489,8 @@ func getPathsFromModuleDep(ctx ModuleWithDepsPathContext, path, moduleName, tag } 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 rel, err := filepath.Rel(PathForOutput(ctx).String(), goBinary.InstallPath()); err == nil { - return Paths{PathForOutput(ctx, rel).WithoutRel()}, nil - } else { - return nil, fmt.Errorf("cannot find output path for %q: %w", goBinary.InstallPath(), err) - } + goBinaryPath := PathForGoBinary(ctx, goBinary) + return Paths{goBinaryPath}, nil } else if srcProducer, ok := module.(SourceFileProducer); ok { return srcProducer.Srcs(), nil } else { diff --git a/apex/apex.go b/apex/apex.go index 1f0618750..a36a022f6 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -1454,12 +1454,7 @@ func apexFileForPyBinary(ctx android.BaseModuleContext, py *python.Module) apexF func apexFileForGoBinary(ctx android.BaseModuleContext, depName string, gb bootstrap.GoBinaryTool) apexFile { dirInApex := "bin" - s, err := filepath.Rel(android.PathForOutput(ctx).String(), gb.InstallPath()) - if err != nil { - ctx.ModuleErrorf("Unable to use compiled binary at %s", gb.InstallPath()) - return apexFile{} - } - fileToCopy := android.PathForOutput(ctx, s) + fileToCopy := android.PathForGoBinary(ctx, gb) // NB: Since go binaries are static we don't need the module for anything here, which is // good since the go tool is a blueprint.Module not an android.Module like we would // normally use. diff --git a/genrule/genrule.go b/genrule/genrule.go index b9c2b109d..c9bf958a3 100644 --- a/genrule/genrule.go +++ b/genrule/genrule.go @@ -336,14 +336,9 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) { } case bootstrap.GoBinaryTool: // A GoBinaryTool provides the install path to a tool, which will be copied. - if s, err := filepath.Rel(android.PathForOutput(ctx).String(), t.InstallPath()); err == nil { - toolPath := android.PathForOutput(ctx, s) - tools = append(tools, toolPath) - addLocationLabel(tag.label, toolLocation{android.Paths{toolPath}}) - } else { - ctx.ModuleErrorf("cannot find path for %q: %v", tool, err) - return - } + p := android.PathForGoBinary(ctx, t) + tools = append(tools, p) + addLocationLabel(tag.label, toolLocation{android.Paths{p}}) default: ctx.ModuleErrorf("%q is not a host tool provider", tool) return From 790ef35d1e7c87fd738cc1dcee45b031d21de600 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Mon, 25 Oct 2021 19:15:55 -0700 Subject: [PATCH 3/4] Make HostToolPath, HostJNIToolPath and sboxPathForToolRel use pathForInstall Use pathForInstall instead of PathForOutput for HostToolPath, HostJNIToolPath and sboxPathForToolRel so that they internally produce an InstallPath that can later support being converted to Make install path. Bug: 204136549 Test: m checkbuild Change-Id: Ie16a62641d113873daeec4d1dd4261251bc0d0eb --- android/config.go | 12 +++++++----- android/rule_builder.go | 9 +++++---- apex/builder.go | 2 +- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/android/config.go b/android/config.go index c8d7cfdc9..95d93c0de 100644 --- a/android/config.go +++ b/android/config.go @@ -355,14 +355,14 @@ func TestConfig(buildDir string, env map[string]string, bp string, fs map[string config.bp2buildModuleTypeConfig = map[string]bool{} + determineBuildOS(config) + return Config{config} } func modifyTestConfigToSupportArchMutator(testConfig Config) { config := testConfig.config - determineBuildOS(config) - config.Targets = map[OsType][]Target{ Android: []Target{ {Android, Arch{ArchType: Arm64, ArchVariant: "armv8-a", Abi: []string{"arm64-v8a"}}, NativeBridgeDisabled, "", "", false}, @@ -568,15 +568,17 @@ func (c *config) HostToolDir() string { } func (c *config) HostToolPath(ctx PathContext, tool string) Path { - return PathForOutput(ctx, "host", c.PrebuiltOS(), "bin", tool) + path := pathForInstall(ctx, ctx.Config().BuildOS, ctx.Config().BuildArch, "bin", false, tool) + return path } -func (c *config) HostJNIToolPath(ctx PathContext, path string) Path { +func (c *config) HostJNIToolPath(ctx PathContext, lib string) Path { ext := ".so" if runtime.GOOS == "darwin" { ext = ".dylib" } - return PathForOutput(ctx, "host", c.PrebuiltOS(), "lib64", path+ext) + path := pathForInstall(ctx, ctx.Config().BuildOS, ctx.Config().BuildArch, "lib64", false, lib+ext) + return path } func (c *config) HostJavaToolPath(ctx PathContext, path string) Path { diff --git a/android/rule_builder.go b/android/rule_builder.go index c9a9ddd31..1c6b1c086 100644 --- a/android/rule_builder.go +++ b/android/rule_builder.go @@ -833,10 +833,11 @@ func (c *RuleBuilderCommand) PathForOutput(path WritablePath) string { func sboxPathForToolRel(ctx BuilderContext, path Path) string { // Errors will be handled in RuleBuilder.Build where we have a context to report them - relOut, isRelOut, _ := maybeRelErr(PathForOutput(ctx, "host", ctx.Config().PrebuiltOS()).String(), path.String()) - if isRelOut { - // The tool is in the output directory, it will be copied to __SBOX_OUT_DIR__/tools/out - return filepath.Join(sboxToolsSubDir, "out", relOut) + toolDir := pathForInstall(ctx, ctx.Config().BuildOS, ctx.Config().BuildArch, "", false) + relOutSoong, isRelOutSoong, _ := maybeRelErr(toolDir.String(), path.String()) + if isRelOutSoong { + // The tool is in the Soong output directory, it will be copied to __SBOX_OUT_DIR__/tools/out + return filepath.Join(sboxToolsSubDir, "out", relOutSoong) } // The tool is in the source directory, it will be copied to __SBOX_OUT_DIR__/tools/src return filepath.Join(sboxToolsSubDir, "src", path.String()) diff --git a/apex/builder.go b/apex/builder.go index 772f7897a..0d984c0b2 100644 --- a/apex/builder.go +++ b/apex/builder.go @@ -524,7 +524,7 @@ func (a *apexBundle) buildUnflattenedApex(ctx android.ModuleContext) { } unsignedOutputFile := android.PathForModuleOut(ctx, a.Name()+suffix+".unsigned") - outHostBinDir := android.PathForOutput(ctx, "host", ctx.Config().PrebuiltOS(), "bin").String() + outHostBinDir := ctx.Config().HostToolPath(ctx, "").String() prebuiltSdkToolsBinDir := filepath.Join("prebuilts", "sdk", "tools", runtime.GOOS, "bin") // Figure out if need to compress apex. From f1f763a981dbdbd31d6cd0f2e8afa7f0952b3500 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Thu, 21 Oct 2021 16:14:19 -0700 Subject: [PATCH 4/4] Rename amod variable in AndroidMkEntries.fillInEntries to base AndroidMkEntries.fillInEntries calls its android.ModuleBase amod, despite also handling an android.Module. Rename amod to base to match other locations, and add a new amod for the android.Module. This will simplify the next patch that needs to access the android.Module. Bug: 204136549 Test: m checkbuild Change-Id: I04f2f558959def22e8b3f5b8c534b8d655b06a4e --- android/androidmk.go | 79 ++++++++++++++++++++++---------------------- 1 file changed, 40 insertions(+), 39 deletions(-) diff --git a/android/androidmk.go b/android/androidmk.go index f48c06bce..f5a9f874b 100644 --- a/android/androidmk.go +++ b/android/androidmk.go @@ -478,8 +478,9 @@ type fillInEntriesContext interface { func (a *AndroidMkEntries) fillInEntries(ctx fillInEntriesContext, mod blueprint.Module) { a.EntryMap = make(map[string][]string) - amod := mod.(Module).base() - name := amod.BaseModuleName() + amod := mod.(Module) + base := amod.base() + name := base.BaseModuleName() if a.OverrideName != "" { name = a.OverrideName } @@ -487,9 +488,9 @@ func (a *AndroidMkEntries) fillInEntries(ctx fillInEntriesContext, mod blueprint if a.Include == "" { a.Include = "$(BUILD_PREBUILT)" } - a.Required = append(a.Required, mod.(Module).RequiredModuleNames()...) - a.Host_required = append(a.Host_required, mod.(Module).HostRequiredModuleNames()...) - a.Target_required = append(a.Target_required, mod.(Module).TargetRequiredModuleNames()...) + a.Required = append(a.Required, amod.RequiredModuleNames()...) + a.Host_required = append(a.Host_required, amod.HostRequiredModuleNames()...) + a.Target_required = append(a.Target_required, amod.TargetRequiredModuleNames()...) for _, distString := range a.GetDistForGoals(mod) { fmt.Fprintf(&a.header, distString) @@ -500,14 +501,14 @@ func (a *AndroidMkEntries) fillInEntries(ctx fillInEntriesContext, mod blueprint // Collect make variable assignment entries. a.SetString("LOCAL_PATH", ctx.ModuleDir(mod)) a.SetString("LOCAL_MODULE", name+a.SubName) - a.AddStrings("LOCAL_LICENSE_KINDS", amod.commonProperties.Effective_license_kinds...) - a.AddStrings("LOCAL_LICENSE_CONDITIONS", amod.commonProperties.Effective_license_conditions...) - a.AddStrings("LOCAL_NOTICE_FILE", amod.commonProperties.Effective_license_text.Strings()...) + a.AddStrings("LOCAL_LICENSE_KINDS", base.commonProperties.Effective_license_kinds...) + a.AddStrings("LOCAL_LICENSE_CONDITIONS", base.commonProperties.Effective_license_conditions...) + a.AddStrings("LOCAL_NOTICE_FILE", base.commonProperties.Effective_license_text.Strings()...) // TODO(b/151177513): Does this code need to set LOCAL_MODULE_IS_CONTAINER ? - if amod.commonProperties.Effective_package_name != nil { - a.SetString("LOCAL_LICENSE_PACKAGE_NAME", *amod.commonProperties.Effective_package_name) - } else if len(amod.commonProperties.Effective_licenses) > 0 { - a.SetString("LOCAL_LICENSE_PACKAGE_NAME", strings.Join(amod.commonProperties.Effective_licenses, " ")) + if base.commonProperties.Effective_package_name != nil { + a.SetString("LOCAL_LICENSE_PACKAGE_NAME", *base.commonProperties.Effective_package_name) + } else if len(base.commonProperties.Effective_licenses) > 0 { + a.SetString("LOCAL_LICENSE_PACKAGE_NAME", strings.Join(base.commonProperties.Effective_licenses, " ")) } a.SetString("LOCAL_MODULE_CLASS", a.Class) a.SetString("LOCAL_PREBUILT_MODULE_FILE", a.OutputFile.String()) @@ -519,27 +520,27 @@ func (a *AndroidMkEntries) fillInEntries(ctx fillInEntriesContext, mod blueprint a.SetBoolIfTrue("LOCAL_NOT_AVAILABLE_FOR_PLATFORM", am.NotAvailableForPlatform()) } - archStr := amod.Arch().ArchType.String() + archStr := base.Arch().ArchType.String() host := false - switch amod.Os().Class { + switch base.Os().Class { case Host: - if amod.Target().HostCross { + if base.Target().HostCross { // Make cannot identify LOCAL_MODULE_HOST_CROSS_ARCH:= common. - if amod.Arch().ArchType != Common { + if base.Arch().ArchType != Common { a.SetString("LOCAL_MODULE_HOST_CROSS_ARCH", archStr) } } else { // Make cannot identify LOCAL_MODULE_HOST_ARCH:= common. - if amod.Arch().ArchType != Common { + if base.Arch().ArchType != Common { a.SetString("LOCAL_MODULE_HOST_ARCH", archStr) } } host = true case Device: // Make cannot identify LOCAL_MODULE_TARGET_ARCH:= common. - if amod.Arch().ArchType != Common { - if amod.Target().NativeBridge { - hostArchStr := amod.Target().NativeBridgeHostArchName + if base.Arch().ArchType != Common { + if base.Target().NativeBridge { + hostArchStr := base.Target().NativeBridgeHostArchName if hostArchStr != "" { a.SetString("LOCAL_MODULE_TARGET_ARCH", hostArchStr) } @@ -548,31 +549,31 @@ func (a *AndroidMkEntries) fillInEntries(ctx fillInEntriesContext, mod blueprint } } - if !amod.InRamdisk() && !amod.InVendorRamdisk() { - a.AddPaths("LOCAL_FULL_INIT_RC", amod.initRcPaths) + if !base.InRamdisk() && !base.InVendorRamdisk() { + a.AddPaths("LOCAL_FULL_INIT_RC", base.initRcPaths) } - if len(amod.vintfFragmentsPaths) > 0 { - a.AddPaths("LOCAL_FULL_VINTF_FRAGMENTS", amod.vintfFragmentsPaths) + if len(base.vintfFragmentsPaths) > 0 { + a.AddPaths("LOCAL_FULL_VINTF_FRAGMENTS", base.vintfFragmentsPaths) } - a.SetBoolIfTrue("LOCAL_PROPRIETARY_MODULE", Bool(amod.commonProperties.Proprietary)) - if Bool(amod.commonProperties.Vendor) || Bool(amod.commonProperties.Soc_specific) { + a.SetBoolIfTrue("LOCAL_PROPRIETARY_MODULE", Bool(base.commonProperties.Proprietary)) + if Bool(base.commonProperties.Vendor) || Bool(base.commonProperties.Soc_specific) { a.SetString("LOCAL_VENDOR_MODULE", "true") } - a.SetBoolIfTrue("LOCAL_ODM_MODULE", Bool(amod.commonProperties.Device_specific)) - a.SetBoolIfTrue("LOCAL_PRODUCT_MODULE", Bool(amod.commonProperties.Product_specific)) - a.SetBoolIfTrue("LOCAL_SYSTEM_EXT_MODULE", Bool(amod.commonProperties.System_ext_specific)) - if amod.commonProperties.Owner != nil { - a.SetString("LOCAL_MODULE_OWNER", *amod.commonProperties.Owner) + a.SetBoolIfTrue("LOCAL_ODM_MODULE", Bool(base.commonProperties.Device_specific)) + a.SetBoolIfTrue("LOCAL_PRODUCT_MODULE", Bool(base.commonProperties.Product_specific)) + a.SetBoolIfTrue("LOCAL_SYSTEM_EXT_MODULE", Bool(base.commonProperties.System_ext_specific)) + if base.commonProperties.Owner != nil { + a.SetString("LOCAL_MODULE_OWNER", *base.commonProperties.Owner) } } - if len(amod.noticeFiles) > 0 { - a.SetString("LOCAL_NOTICE_FILE", strings.Join(amod.noticeFiles.Strings(), " ")) + if len(base.noticeFiles) > 0 { + a.SetString("LOCAL_NOTICE_FILE", strings.Join(base.noticeFiles.Strings(), " ")) } if host { - makeOs := amod.Os().String() - if amod.Os() == Linux || amod.Os() == LinuxBionic || amod.Os() == LinuxMusl { + makeOs := base.Os().String() + if base.Os() == Linux || base.Os() == LinuxBionic || base.Os() == LinuxMusl { makeOs = "linux" } a.SetString("LOCAL_MODULE_HOST_OS", makeOs) @@ -580,10 +581,10 @@ func (a *AndroidMkEntries) fillInEntries(ctx fillInEntriesContext, mod blueprint } prefix := "" - if amod.ArchSpecific() { - switch amod.Os().Class { + if base.ArchSpecific() { + switch base.Os().Class { case Host: - if amod.Target().HostCross { + if base.Target().HostCross { prefix = "HOST_CROSS_" } else { prefix = "HOST_" @@ -593,7 +594,7 @@ func (a *AndroidMkEntries) fillInEntries(ctx fillInEntriesContext, mod blueprint } - if amod.Arch().ArchType != ctx.Config().Targets[amod.Os()][0].Arch.ArchType { + if base.Arch().ArchType != ctx.Config().Targets[base.Os()][0].Arch.ArchType { prefix = "2ND_" + prefix } }