From 95b07f2b59049dd4a539780ed0c91b8809ce97a9 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Wed, 16 Dec 2020 11:06:50 -0800 Subject: [PATCH 1/3] Don't copy uninstallable variants of NDK libraries to sysroot After the next patch libraryDecorator.install will be called for uninstallable variants of modules, manually filter them out when copying to the NDK sysroot. Bug: 124313442 Test: m checkbuild Change-Id: I28b538d4ae271dc5e27c386d7cfa538ac0ed841b --- cc/cc.go | 5 +++++ cc/library.go | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/cc/cc.go b/cc/cc.go index 89f32f163..afc96bacb 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -432,6 +432,7 @@ type ModuleContextIntf interface { mustUseVendorVariant() bool nativeCoverage() bool directlyInAnyApex() bool + isPreventInstall() bool } type ModuleContext interface { @@ -1325,6 +1326,10 @@ func (ctx *moduleContextImpl) directlyInAnyApex() bool { return ctx.mod.DirectlyInAnyApex() } +func (ctx *moduleContextImpl) isPreventInstall() bool { + return ctx.mod.Properties.PreventInstall +} + func newBaseModule(hod android.HostOrDeviceSupported, multilib android.Multilib) *Module { return &Module{ hod: hod, diff --git a/cc/library.go b/cc/library.go index 959d67086..73ff75425 100644 --- a/cc/library.go +++ b/cc/library.go @@ -1345,7 +1345,7 @@ func (library *libraryDecorator) install(ctx ModuleContext, file android.Path) { if Bool(library.Properties.Static_ndk_lib) && library.static() && !ctx.useVndk() && !ctx.inRamdisk() && !ctx.inVendorRamdisk() && !ctx.inRecovery() && ctx.Device() && library.baseLinker.sanitize.isUnsanitizedVariant() && - !library.buildStubs() && ctx.sdkVersion() == "" { + ctx.isForPlatform() && !ctx.isPreventInstall() { installPath := getNdkSysrootBase(ctx).Join( ctx, "usr/lib", config.NDKTriple(ctx.toolchain()), file.Base()) From a9c8c9f1457a3848ff0d2c6bcb3dfbe256c1b8d4 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Wed, 16 Dec 2020 10:20:23 -0800 Subject: [PATCH 2/3] Call ctx.InstallFile for uninstallable cc modules SkipInstall is actually primarily used to prevent making a module visible to Make, rename it and add new SkipInstall that actually skips installation without affecting Make. Call c.SkipInstall() for uninstallable cc modules to allow calling c.installer.install, which will collect PackagingSpecs for uninstallable cc modules, allowing them to be used by genrules. Bug: 124313442 Test: m checkbuild Change-Id: I8038ed5c6f05c989ac21ec06c4552fb3136b9a7a --- android/androidmk.go | 2 +- android/module.go | 45 +++++++++++++++++++++++++++----------- android/override_module.go | 4 ++-- android/prebuilt.go | 2 +- apex/apex_test.go | 2 +- apex/prebuilt.go | 4 ++-- cc/androidmk.go | 2 +- cc/cc.go | 22 ++++++++++++------- cc/image.go | 2 +- cc/library.go | 2 +- cc/sabi.go | 2 +- cc/snapshot_prebuilt.go | 2 +- cc/vendor_snapshot.go | 4 ++-- cc/vndk_prebuilt.go | 4 ++-- rust/image.go | 2 +- 15 files changed, 63 insertions(+), 38 deletions(-) diff --git a/android/androidmk.go b/android/androidmk.go index b32048aab..062dcb352 100644 --- a/android/androidmk.go +++ b/android/androidmk.go @@ -820,7 +820,7 @@ func shouldSkipAndroidMkProcessing(module *ModuleBase) bool { } return !module.Enabled() || - module.commonProperties.SkipInstall || + module.commonProperties.HideFromMake || // Make does not understand LinuxBionic module.Os() == LinuxBionic } diff --git a/android/module.go b/android/module.go index 429f31192..83f5494f5 100644 --- a/android/module.go +++ b/android/module.go @@ -452,8 +452,8 @@ type Module interface { InstallInRoot() bool InstallBypassMake() bool InstallForceOS() (*OsType, *ArchType) - SkipInstall() - IsSkipInstall() bool + HideFromMake() + IsHideFromMake() bool MakeUninstallable() ReplacedByPrebuilt() IsReplacedByPrebuilt() bool @@ -751,6 +751,13 @@ type commonProperties struct { // Set by osMutator. CommonOSVariant bool `blueprint:"mutated"` + // When HideFromMake is set to true, no entry for this variant will be emitted in the + // generated Android.mk file. + HideFromMake bool `blueprint:"mutated"` + + // When SkipInstall is set to true, calls to ctx.InstallFile, ctx.InstallExecutable, + // ctx.InstallSymlink and ctx.InstallAbsoluteSymlink act like calls to ctx.PackageFile + // and don't create a rule to install the file. SkipInstall bool `blueprint:"mutated"` // Whether the module has been replaced by a prebuilt @@ -1355,26 +1362,33 @@ func (m *ModuleBase) Disable() { m.commonProperties.ForcedDisabled = true } +// HideFromMake marks this variant so that it is not emitted in the generated Android.mk file. +func (m *ModuleBase) HideFromMake() { + m.commonProperties.HideFromMake = true +} + +// IsHideFromMake returns true if HideFromMake was previously called. +func (m *ModuleBase) IsHideFromMake() bool { + return m.commonProperties.HideFromMake == true +} + +// SkipInstall marks this variant to not create install rules when ctx.Install* are called. func (m *ModuleBase) SkipInstall() { m.commonProperties.SkipInstall = true } -func (m *ModuleBase) IsSkipInstall() bool { - return m.commonProperties.SkipInstall == true -} - -// Similar to SkipInstall, but if the AndroidMk entry would set +// Similar to HideFromMake, but if the AndroidMk entry would set // LOCAL_UNINSTALLABLE_MODULE then this variant may still output that entry // rather than leaving it out altogether. That happens in cases where it would // have other side effects, in particular when it adds a NOTICE file target, // which other install targets might depend on. func (m *ModuleBase) MakeUninstallable() { - m.SkipInstall() + m.HideFromMake() } func (m *ModuleBase) ReplacedByPrebuilt() { m.commonProperties.ReplacedByPrebuilt = true - m.SkipInstall() + m.HideFromMake() } func (m *ModuleBase) IsReplacedByPrebuilt() bool { @@ -2440,11 +2454,15 @@ func (m *moduleContext) InstallForceOS() (*OsType, *ArchType) { return m.module.InstallForceOS() } -func (m *moduleContext) skipInstall(fullInstallPath InstallPath) bool { +func (m *moduleContext) skipInstall() bool { if m.module.base().commonProperties.SkipInstall { return true } + if m.module.base().commonProperties.HideFromMake { + return true + } + // We'll need a solution for choosing which of modules with the same name in different // namespaces to install. For now, reuse the list of namespaces exported to Make as the // list of namespaces to install in a Soong-only build. @@ -2492,7 +2510,7 @@ func (m *moduleContext) installFile(installPath InstallPath, name string, srcPat fullInstallPath := installPath.Join(m, name) m.module.base().hooks.runInstallHooks(m, srcPath, fullInstallPath, false) - if !m.skipInstall(fullInstallPath) { + if !m.skipInstall() { deps = append(deps, m.module.base().installFilesDepSet.ToList().Paths()...) var implicitDeps, orderOnlyDeps Paths @@ -2526,6 +2544,7 @@ func (m *moduleContext) installFile(installPath InstallPath, name string, srcPat m.packageFile(fullInstallPath, srcPath, executable) m.checkbuildFiles = append(m.checkbuildFiles, srcPath) + return fullInstallPath } @@ -2537,7 +2556,7 @@ func (m *moduleContext) InstallSymlink(installPath InstallPath, name string, src if err != nil { panic(fmt.Sprintf("Unable to generate symlink between %q and %q: %s", fullInstallPath.Base(), srcPath.Base(), err)) } - if !m.skipInstall(fullInstallPath) { + if !m.skipInstall() { m.Build(pctx, BuildParams{ Rule: Symlink, @@ -2570,7 +2589,7 @@ func (m *moduleContext) InstallAbsoluteSymlink(installPath InstallPath, name str fullInstallPath := installPath.Join(m, name) m.module.base().hooks.runInstallHooks(m, nil, fullInstallPath, true) - if !m.skipInstall(fullInstallPath) { + if !m.skipInstall() { m.Build(pctx, BuildParams{ Rule: Symlink, Description: "install symlink " + fullInstallPath.Base() + " -> " + absPath, diff --git a/android/override_module.go b/android/override_module.go index f8342d52d..fa0856631 100644 --- a/android/override_module.go +++ b/android/override_module.go @@ -235,7 +235,7 @@ func overrideModuleDepsMutator(ctx BottomUpMutatorContext) { return } // See if there's a prebuilt module that overrides this override module with prefer flag, - // in which case we call SkipInstall on the corresponding variant later. + // in which case we call HideFromMake on the corresponding variant later. ctx.VisitDirectDepsWithTag(PrebuiltDepTag, func(dep Module) { prebuilt, ok := dep.(PrebuiltInterface) if !ok { @@ -284,7 +284,7 @@ func performOverrideMutator(ctx BottomUpMutatorContext) { mods[i+1].(OverridableModule).override(ctx, o) if o.getOverriddenByPrebuilt() { // The overriding module itself, too, is overridden by a prebuilt. Skip its installation. - mods[i+1].SkipInstall() + mods[i+1].HideFromMake() } } } else if o, ok := ctx.Module().(OverrideModule); ok { diff --git a/android/prebuilt.go b/android/prebuilt.go index 8114a65a5..39d30c5ec 100644 --- a/android/prebuilt.go +++ b/android/prebuilt.go @@ -289,7 +289,7 @@ func PrebuiltPostDepsMutator(ctx BottomUpMutatorContext) { }) } } else { - m.SkipInstall() + m.HideFromMake() } } } diff --git a/apex/apex_test.go b/apex/apex_test.go index 69b1dbbb5..4a6aecfdf 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -6578,7 +6578,7 @@ func TestPrebuiltStubLibDep(t *testing.T) { continue } mod := ctx.ModuleForTests(modName, variant).Module().(*cc.Module) - if !mod.Enabled() || mod.IsSkipInstall() { + if !mod.Enabled() || mod.IsHideFromMake() { continue } for _, ent := range android.AndroidMkEntriesForTest(t, config, "", mod) { diff --git a/apex/prebuilt.go b/apex/prebuilt.go index ce16d7340..7931e9e25 100644 --- a/apex/prebuilt.go +++ b/apex/prebuilt.go @@ -218,7 +218,7 @@ func (p *Prebuilt) GenerateAndroidBuildActions(ctx android.ModuleContext) { }) if p.prebuiltCommon.checkForceDisable(ctx) { - p.SkipInstall() + p.HideFromMake() return } @@ -392,7 +392,7 @@ func (a *ApexSet) GenerateAndroidBuildActions(ctx android.ModuleContext) { }) if a.prebuiltCommon.checkForceDisable(ctx) { - a.SkipInstall() + a.HideFromMake() return } diff --git a/cc/androidmk.go b/cc/androidmk.go index 187a2ff16..ddacb70f3 100644 --- a/cc/androidmk.go +++ b/cc/androidmk.go @@ -273,7 +273,7 @@ func (library *libraryDecorator) AndroidMkEntries(ctx AndroidMkContext, entries entries.SubName = "." + library.stubsVersion() } entries.ExtraEntries = append(entries.ExtraEntries, func(entries *android.AndroidMkEntries) { - // library.makeUninstallable() depends on this to bypass SkipInstall() for + // library.makeUninstallable() depends on this to bypass HideFromMake() for // static libraries. entries.SetBool("LOCAL_UNINSTALLABLE_MODULE", true) if library.buildStubs() { diff --git a/cc/cc.go b/cc/cc.go index afc96bacb..9f90332a3 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -1580,18 +1580,24 @@ func (c *Module) GenerateAndroidBuildActions(actx android.ModuleContext) { } } - if c.installable(apexInfo) { + if !proptools.BoolDefault(c.Properties.Installable, true) { + // If the module has been specifically configure to not be installed then + // hide from make as otherwise it will break when running inside make + // as the output path to install will not be specified. Not all uninstallable + // modules can be hidden from make as some are needed for resolving make side + // dependencies. + c.HideFromMake() + } else if !c.installable(apexInfo) { + c.SkipInstall() + } + + // Still call c.installer.install though, the installs will be stored as PackageSpecs + // to allow using the outputs in a genrule. + if c.installer != nil && c.outputFile.Valid() { c.installer.install(ctx, c.outputFile.Path()) if ctx.Failed() { return } - } else if !proptools.BoolDefault(c.Properties.Installable, true) { - // If the module has been specifically configure to not be installed then - // skip the installation as otherwise it will break when running inside make - // as the output path to install will not be specified. Not all uninstallable - // modules can skip installation as some are needed for resolving make side - // dependencies. - c.SkipInstall() } } diff --git a/cc/image.go b/cc/image.go index 32325b910..cca454aa9 100644 --- a/cc/image.go +++ b/cc/image.go @@ -454,7 +454,7 @@ func (c *Module) SetImageVariation(ctx android.BaseModuleContext, variant string vndkVersion := ctx.DeviceConfig().VndkVersion() if vndkVersion != "current" && vndkVersion != "" && vndkVersion != m.Properties.VndkVersion { m.Properties.HideFromMake = true - m.SkipInstall() + m.HideFromMake() } } else if strings.HasPrefix(variant, ProductVariationPrefix) { m.Properties.ImageVariationPrefix = ProductVariationPrefix diff --git a/cc/library.go b/cc/library.go index 73ff75425..11ee7ddc4 100644 --- a/cc/library.go +++ b/cc/library.go @@ -1336,7 +1336,7 @@ func (library *libraryDecorator) install(ctx ModuleContext, file android.Path) { } } else if ctx.directlyInAnyApex() && ctx.isLlndk(ctx.Config()) && !isBionic(ctx.baseModuleName()) { // Skip installing LLNDK (non-bionic) libraries moved to APEX. - ctx.Module().SkipInstall() + ctx.Module().HideFromMake() } library.baseInstaller.install(ctx, file) diff --git a/cc/sabi.go b/cc/sabi.go index 99e718e60..c357f8dab 100644 --- a/cc/sabi.go +++ b/cc/sabi.go @@ -131,7 +131,7 @@ func shouldCreateSourceAbiDumpForLibrary(ctx android.BaseModuleContext) bool { // Module is shared library type. // Don't check uninstallable modules. - if m.IsSkipInstall() { + if m.IsHideFromMake() { return false } diff --git a/cc/snapshot_prebuilt.go b/cc/snapshot_prebuilt.go index b291bd0d3..f5f812146 100644 --- a/cc/snapshot_prebuilt.go +++ b/cc/snapshot_prebuilt.go @@ -836,7 +836,7 @@ func VendorSnapshotSourceMutator(ctx android.BottomUpMutatorContext) { // Disables source modules if corresponding snapshot exists. if lib, ok := module.linker.(libraryInterface); ok && lib.buildStatic() && lib.buildShared() { // But do not disable because the shared variant depends on the static variant. - module.SkipInstall() + module.HideFromMake() module.Properties.HideFromMake = true } else { module.Disable() diff --git a/cc/vendor_snapshot.go b/cc/vendor_snapshot.go index da37d0f1a..bca76dc19 100644 --- a/cc/vendor_snapshot.go +++ b/cc/vendor_snapshot.go @@ -181,8 +181,8 @@ func isSnapshotAware(m *Module, inProprietaryPath bool, apexInfo android.ApexInf return false } // When android/prebuilt.go selects between source and prebuilt, it sets - // SkipInstall on the other one to avoid duplicate install rules in make. - if m.IsSkipInstall() { + // HideFromMake on the other one to avoid duplicate install rules in make. + if m.IsHideFromMake() { return false } // skip proprietary modules, but (for the vendor snapshot only) diff --git a/cc/vndk_prebuilt.go b/cc/vndk_prebuilt.go index e6e2ad85d..04162cdce 100644 --- a/cc/vndk_prebuilt.go +++ b/cc/vndk_prebuilt.go @@ -130,7 +130,7 @@ func (p *vndkPrebuiltLibraryDecorator) link(ctx ModuleContext, flags Flags, deps PathDeps, objs Objects) android.Path { if !p.matchesWithDevice(ctx.DeviceConfig()) { - ctx.Module().SkipInstall() + ctx.Module().HideFromMake() return nil } @@ -175,7 +175,7 @@ func (p *vndkPrebuiltLibraryDecorator) link(ctx ModuleContext, return in } - ctx.Module().SkipInstall() + ctx.Module().HideFromMake() return nil } diff --git a/rust/image.go b/rust/image.go index 4951d2b7b..af8c3b2fd 100644 --- a/rust/image.go +++ b/rust/image.go @@ -90,7 +90,7 @@ func (mod *Module) SetImageVariation(ctx android.BaseModuleContext, variant stri vndkVersion := ctx.DeviceConfig().VndkVersion() if vndkVersion != "current" && vndkVersion != "" && vndkVersion != m.Properties.VndkVersion { m.Properties.HideFromMake = true - m.SkipInstall() + m.HideFromMake() } } } From ba9e403703f3601ed55fa665d35aad973218d36d Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Tue, 24 Nov 2020 16:32:22 -0800 Subject: [PATCH 3/3] Sandbox genrule tools This relands I38393900677c5dbe8e72fe06a7dd7d71f3c09f82 after I8038ed5c6f05c989ac21ec06c4552fb3136b9a7a, which makes the ASAN variants of libraries have PackagingSpecs so they can be copied into sandboxed genrules. Copy tools used by genrules into the sandbox directory. This ensures correct dependencies on all files used by tools, and is one step closer to enabling genrules inside unselected namespaces. Bug: 124313442 Test: genrule_test.go Test: rule_builder_test.go Test: m checkbuild Change-Id: I57c0d5fc8bba216fac4deb972d0d2098593e8963 --- android/module.go | 1 + android/rule_builder.go | 126 ++++++++++++++++++++++++++++++++++- android/rule_builder_test.go | 38 +++++++++++ genrule/genrule.go | 70 ++++++++++++------- genrule/genrule_test.go | 28 ++++---- 5 files changed, 223 insertions(+), 40 deletions(-) diff --git a/android/module.go b/android/module.go index 83f5494f5..cfb32c126 100644 --- a/android/module.go +++ b/android/module.go @@ -2757,6 +2757,7 @@ func outputFilesForModule(ctx PathContext, module blueprint.Module, tag string) // 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 { + Module // HostToolPath returns the path to the host tool for the module if it is one, or an invalid // OptionalPath. HostToolPath() OptionalPath diff --git a/android/rule_builder.go b/android/rule_builder.go index e2d81877c..84501fe9f 100644 --- a/android/rule_builder.go +++ b/android/rule_builder.go @@ -32,6 +32,7 @@ import ( const sboxSandboxBaseDir = "__SBOX_SANDBOX_DIR__" const sboxOutSubDir = "out" +const sboxToolsSubDir = "tools" const sboxOutDir = sboxSandboxBaseDir + "/" + sboxOutSubDir // RuleBuilder provides an alternative to ModuleContext.Rule and ModuleContext.Build to add a command line to the build @@ -48,6 +49,7 @@ type RuleBuilder struct { highmem bool remoteable RemoteRuleSupports outDir WritablePath + sboxTools bool sboxManifestPath WritablePath missingDeps []string } @@ -140,6 +142,19 @@ func (r *RuleBuilder) Sbox(outputDir WritablePath, manifestPath WritablePath) *R return r } +// SandboxTools enables tool sandboxing for the rule by copying any referenced tools into the +// sandbox. +func (r *RuleBuilder) SandboxTools() *RuleBuilder { + if !r.sbox { + panic("SandboxTools() must be called after Sbox()") + } + if len(r.commands) > 0 { + panic("SandboxTools() may not be called after Command()") + } + r.sboxTools = true + return r +} + // Install associates an output of the rule with an install location, which can be retrieved later using // RuleBuilder.Installs. func (r *RuleBuilder) Install(from Path, to string) { @@ -468,8 +483,29 @@ func (r *RuleBuilder) Build(name string, desc string) { manifest.OutputDepfile = proto.String(depFile.String()) } + // If sandboxing tools is enabled, add copy rules to the manifest to copy each tool + // into the sbox directory. + if r.sboxTools { + for _, tool := range tools { + command.CopyBefore = append(command.CopyBefore, &sbox_proto.Copy{ + From: proto.String(tool.String()), + To: proto.String(sboxPathForToolRel(r.ctx, tool)), + }) + } + for _, c := range r.commands { + for _, tool := range c.packagedTools { + command.CopyBefore = append(command.CopyBefore, &sbox_proto.Copy{ + From: proto.String(tool.srcPath.String()), + To: proto.String(sboxPathForPackagedToolRel(tool)), + Executable: proto.Bool(tool.executable), + }) + tools = append(tools, tool.srcPath) + } + } + } + // Add copy rules to the manifest to copy each output file from the sbox directory. - // to the output directory. + // to the output directory after running the commands. sboxOutputs := make([]string, len(outputs)) for i, output := range outputs { rel := Rel(r.ctx, r.outDir.String(), output.String()) @@ -582,6 +618,7 @@ type RuleBuilderCommand struct { symlinkOutputs WritablePaths depFiles WritablePaths tools Paths + packagedTools []PackagingSpec rspFileInputs Paths // spans [start,end) of the command that should not be ninja escaped @@ -625,6 +662,79 @@ func (c *RuleBuilderCommand) PathForOutput(path WritablePath) string { return path.String() } +// SboxPathForTool takes a path to a tool, which may be an output file or a source file, and returns +// the corresponding path for the tool in the sbox sandbox. It assumes that sandboxing and tool +// sandboxing are enabled. +func SboxPathForTool(ctx BuilderContext, path Path) string { + return filepath.Join(sboxSandboxBaseDir, sboxPathForToolRel(ctx, path)) +} + +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) + } + // The tool is in the source directory, it will be copied to __SBOX_OUT_DIR__/tools/src + return filepath.Join(sboxToolsSubDir, "src", path.String()) +} + +// SboxPathForPackagedTool takes a PackageSpec for a tool and returns the corresponding path for the +// tool after copying it into the sandbox. This can be used on the RuleBuilder command line to +// reference the tool. +func SboxPathForPackagedTool(spec PackagingSpec) string { + return filepath.Join(sboxSandboxBaseDir, sboxPathForPackagedToolRel(spec)) +} + +func sboxPathForPackagedToolRel(spec PackagingSpec) string { + return filepath.Join(sboxToolsSubDir, "out", spec.relPathInPackage) +} + +// PathForTool takes a path to a tool, which may be an output file or a source file, and returns +// the corresponding path for the tool in the sbox sandbox if sbox is enabled, or the original path +// if it is not. This can be used on the RuleBuilder command line to reference the tool. +func (c *RuleBuilderCommand) PathForTool(path Path) string { + if c.rule.sbox && c.rule.sboxTools { + return filepath.Join(sboxSandboxBaseDir, sboxPathForToolRel(c.rule.ctx, path)) + } + return path.String() +} + +// PackagedTool adds the specified tool path to the command line. It can only be used with tool +// sandboxing enabled by SandboxTools(), and will copy the tool into the sandbox. +func (c *RuleBuilderCommand) PackagedTool(spec PackagingSpec) *RuleBuilderCommand { + if !c.rule.sboxTools { + panic("PackagedTool() requires SandboxTools()") + } + + c.packagedTools = append(c.packagedTools, spec) + c.Text(sboxPathForPackagedToolRel(spec)) + return c +} + +// ImplicitPackagedTool copies the specified tool into the sandbox without modifying the command +// line. It can only be used with tool sandboxing enabled by SandboxTools(). +func (c *RuleBuilderCommand) ImplicitPackagedTool(spec PackagingSpec) *RuleBuilderCommand { + if !c.rule.sboxTools { + panic("ImplicitPackagedTool() requires SandboxTools()") + } + + c.packagedTools = append(c.packagedTools, spec) + return c +} + +// ImplicitPackagedTools copies the specified tools into the sandbox without modifying the command +// line. It can only be used with tool sandboxing enabled by SandboxTools(). +func (c *RuleBuilderCommand) ImplicitPackagedTools(specs []PackagingSpec) *RuleBuilderCommand { + if !c.rule.sboxTools { + panic("ImplicitPackagedTools() requires SandboxTools()") + } + + c.packagedTools = append(c.packagedTools, specs...) + return c +} + // Text adds the specified raw text to the command line. The text should not contain input or output paths or the // rule will not have them listed in its dependencies or outputs. func (c *RuleBuilderCommand) Text(text string) *RuleBuilderCommand { @@ -693,7 +803,19 @@ func (c *RuleBuilderCommand) FlagWithList(flag string, list []string, sep string // RuleBuilder.Tools. func (c *RuleBuilderCommand) Tool(path Path) *RuleBuilderCommand { c.tools = append(c.tools, path) - return c.Text(path.String()) + return c.Text(c.PathForTool(path)) +} + +// Tool adds the specified tool path to the dependencies returned by RuleBuilder.Tools. +func (c *RuleBuilderCommand) ImplicitTool(path Path) *RuleBuilderCommand { + c.tools = append(c.tools, path) + return c +} + +// Tool adds the specified tool path to the dependencies returned by RuleBuilder.Tools. +func (c *RuleBuilderCommand) ImplicitTools(paths Paths) *RuleBuilderCommand { + c.tools = append(c.tools, paths...) + return c } // BuiltTool adds the specified tool path that was built using a host Soong module to the command line. The path will diff --git a/android/rule_builder_test.go b/android/rule_builder_test.go index e676e4a92..06ea1242d 100644 --- a/android/rule_builder_test.go +++ b/android/rule_builder_test.go @@ -436,6 +436,44 @@ func TestRuleBuilder(t *testing.T) { t.Errorf("\nwant rule.depFileMergerCmd() = %#v\n got %#v", w, g) } }) + + t.Run("sbox tools", func(t *testing.T) { + rule := NewRuleBuilder(pctx, ctx).Sbox(PathForOutput(ctx, ""), + PathForOutput(ctx, "sbox.textproto")).SandboxTools() + addCommands(rule) + + wantCommands := []string{ + "__SBOX_SANDBOX_DIR__/out/DepFile Flag FlagWithArg=arg FlagWithDepFile=__SBOX_SANDBOX_DIR__/out/depfile FlagWithInput=input FlagWithOutput=__SBOX_SANDBOX_DIR__/out/output Input __SBOX_SANDBOX_DIR__/out/Output __SBOX_SANDBOX_DIR__/out/SymlinkOutput Text __SBOX_SANDBOX_DIR__/tools/src/Tool after command2 old cmd", + "command2 __SBOX_SANDBOX_DIR__/out/depfile2 input2 __SBOX_SANDBOX_DIR__/out/output2 __SBOX_SANDBOX_DIR__/tools/src/tool2", + "command3 input3 __SBOX_SANDBOX_DIR__/out/output2 __SBOX_SANDBOX_DIR__/out/output3", + } + + wantDepMergerCommand := "__SBOX_SANDBOX_DIR__/tools/out/bin/dep_fixer __SBOX_SANDBOX_DIR__/out/DepFile __SBOX_SANDBOX_DIR__/out/depfile __SBOX_SANDBOX_DIR__/out/ImplicitDepFile __SBOX_SANDBOX_DIR__/out/depfile2" + + if g, w := rule.Commands(), wantCommands; !reflect.DeepEqual(g, w) { + t.Errorf("\nwant rule.Commands() = %#v\n got %#v", w, g) + } + + if g, w := rule.Inputs(), wantInputs; !reflect.DeepEqual(w, g) { + t.Errorf("\nwant rule.Inputs() = %#v\n got %#v", w, g) + } + if g, w := rule.Outputs(), wantOutputs; !reflect.DeepEqual(w, g) { + t.Errorf("\nwant rule.Outputs() = %#v\n got %#v", w, g) + } + if g, w := rule.DepFiles(), wantDepFiles; !reflect.DeepEqual(w, g) { + t.Errorf("\nwant rule.DepFiles() = %#v\n got %#v", w, g) + } + if g, w := rule.Tools(), wantTools; !reflect.DeepEqual(w, g) { + t.Errorf("\nwant rule.Tools() = %#v\n got %#v", w, g) + } + if g, w := rule.OrderOnlys(), wantOrderOnlys; !reflect.DeepEqual(w, g) { + t.Errorf("\nwant rule.OrderOnlys() = %#v\n got %#v", w, g) + } + + if g, w := rule.depFileMergerCmd(rule.DepFiles()).String(), wantDepMergerCommand; g != w { + t.Errorf("\nwant rule.depFileMergerCmd() = %#v\n got %#v", w, g) + } + }) } func testRuleBuilderFactory() Module { diff --git a/genrule/genrule.go b/genrule/genrule.go index 905401796..1d1d96c01 100644 --- a/genrule/genrule.go +++ b/genrule/genrule.go @@ -139,7 +139,6 @@ type Module struct { // number of shards the input files are sharded into. taskGenerator taskFunc - deps android.Paths rule blueprint.Rule rawCommands []string @@ -244,6 +243,8 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) { } } + var tools android.Paths + var packagedTools []android.PackagingSpec if len(g.properties.Tools) > 0 { seenTools := make(map[string]bool) @@ -251,37 +252,52 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) { switch tag := ctx.OtherModuleDependencyTag(module).(type) { case hostToolDependencyTag: tool := ctx.OtherModuleName(module) - var path android.OptionalPath - if t, ok := module.(android.HostToolProvider); ok { + switch t := module.(type) { + case android.HostToolProvider: + // A HostToolProvider provides the path to a tool, which will be copied + // into the sandbox. if !t.(android.Module).Enabled() { if ctx.Config().AllowMissingDependencies() { ctx.AddMissingDependencies([]string{tool}) } else { ctx.ModuleErrorf("depends on disabled module %q", tool) } - break + return } - path = t.HostToolPath() - } else if t, ok := module.(bootstrap.GoBinaryTool); ok { + path := t.HostToolPath() + if !path.Valid() { + ctx.ModuleErrorf("host tool %q missing output file", tool) + return + } + if specs := t.TransitivePackagingSpecs(); specs != nil { + // If the HostToolProvider has PackgingSpecs, which are definitions of the + // required relative locations of the tool and its dependencies, use those + // instead. They will be copied to those relative locations in the sbox + // sandbox. + packagedTools = append(packagedTools, specs...) + // Assume that the first PackagingSpec of the module is the tool. + addLocationLabel(tag.label, []string{android.SboxPathForPackagedTool(specs[0])}) + } else { + tools = append(tools, path.Path()) + addLocationLabel(tag.label, []string{android.SboxPathForTool(ctx, path.Path())}) + } + 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 { - path = android.OptionalPathForPath(android.PathForOutput(ctx, s)) + toolPath := android.PathForOutput(ctx, s) + tools = append(tools, toolPath) + addLocationLabel(tag.label, []string{android.SboxPathForTool(ctx, toolPath)}) } else { ctx.ModuleErrorf("cannot find path for %q: %v", tool, err) - break + return } - } else { + default: ctx.ModuleErrorf("%q is not a host tool provider", tool) - break + return } - if path.Valid() { - g.deps = append(g.deps, path.Path()) - addLocationLabel(tag.label, []string{path.Path().String()}) - seenTools[tag.label] = true - } else { - ctx.ModuleErrorf("host tool %q missing output file", tool) - } + seenTools[tag.label] = true } }) @@ -305,8 +321,12 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) { for _, toolFile := range g.properties.Tool_files { paths := android.PathsForModuleSrc(ctx, []string{toolFile}) - g.deps = append(g.deps, paths...) - addLocationLabel(toolFile, paths.Strings()) + tools = append(tools, paths...) + var sandboxPaths []string + for _, path := range paths { + sandboxPaths = append(sandboxPaths, android.SboxPathForTool(ctx, path)) + } + addLocationLabel(toolFile, sandboxPaths) } var srcFiles android.Paths @@ -358,7 +378,7 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) { manifestPath := android.PathForModuleOut(ctx, manifestName) // Use a RuleBuilder to create a rule that runs the command inside an sbox sandbox. - rule := android.NewRuleBuilder(pctx, ctx).Sbox(task.genDir, manifestPath) + rule := android.NewRuleBuilder(pctx, ctx).Sbox(task.genDir, manifestPath).SandboxTools() cmd := rule.Command() for _, out := range task.out { @@ -448,8 +468,9 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) { cmd.Text(rawCommand) cmd.ImplicitOutputs(task.out) cmd.Implicits(task.in) - cmd.Implicits(g.deps) - cmd.Implicits(task.extraTools) + cmd.ImplicitTools(tools) + cmd.ImplicitTools(task.extraTools) + cmd.ImplicitPackagedTools(packagedTools) if Bool(g.properties.Depfile) { cmd.ImplicitDepFile(task.depFile) } @@ -616,7 +637,7 @@ func NewGenSrcs() *Module { // TODO(ccross): this RuleBuilder is a hack to be able to call // rule.Command().PathForOutput. Replace this with passing the rule into the // generator. - rule := android.NewRuleBuilder(pctx, ctx).Sbox(genDir, nil) + rule := android.NewRuleBuilder(pctx, ctx).Sbox(genDir, nil).SandboxTools() for _, in := range shard { outFile := android.GenPathWithExt(ctx, finalSubDir, in, String(properties.Output_extension)) @@ -669,7 +690,8 @@ func NewGenSrcs() *Module { outputDepfile = android.PathForModuleGen(ctx, genSubDir, "gensrcs.d") depFixerTool := ctx.Config().HostToolPath(ctx, "dep_fixer") fullCommand += fmt.Sprintf(" && %s -o $(depfile) %s", - depFixerTool.String(), strings.Join(commandDepFiles, " ")) + android.SboxPathForTool(ctx, depFixerTool), + strings.Join(commandDepFiles, " ")) extraTools = append(extraTools, depFixerTool) } diff --git a/genrule/genrule_test.go b/genrule/genrule_test.go index 6ae9a18dd..2f5605e94 100644 --- a/genrule/genrule_test.go +++ b/genrule/genrule_test.go @@ -141,7 +141,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location) > $(out)", `, - expect: "out/tool > __SBOX_SANDBOX_DIR__/out/out", + expect: "__SBOX_SANDBOX_DIR__/tools/out/bin/tool > __SBOX_SANDBOX_DIR__/out/out", }, { name: "empty location tool2", @@ -150,7 +150,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location) > $(out)", `, - expect: "out/tool > __SBOX_SANDBOX_DIR__/out/out", + expect: "__SBOX_SANDBOX_DIR__/tools/out/bin/tool > __SBOX_SANDBOX_DIR__/out/out", }, { name: "empty location tool file", @@ -159,7 +159,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location) > $(out)", `, - expect: "tool_file1 > __SBOX_SANDBOX_DIR__/out/out", + expect: "__SBOX_SANDBOX_DIR__/tools/src/tool_file1 > __SBOX_SANDBOX_DIR__/out/out", }, { name: "empty location tool file fg", @@ -168,7 +168,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location) > $(out)", `, - expect: "tool_file1 > __SBOX_SANDBOX_DIR__/out/out", + expect: "__SBOX_SANDBOX_DIR__/tools/src/tool_file1 > __SBOX_SANDBOX_DIR__/out/out", }, { name: "empty location tool and tool file", @@ -178,7 +178,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location) > $(out)", `, - expect: "out/tool > __SBOX_SANDBOX_DIR__/out/out", + expect: "__SBOX_SANDBOX_DIR__/tools/out/bin/tool > __SBOX_SANDBOX_DIR__/out/out", }, { name: "tool", @@ -187,7 +187,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location tool) > $(out)", `, - expect: "out/tool > __SBOX_SANDBOX_DIR__/out/out", + expect: "__SBOX_SANDBOX_DIR__/tools/out/bin/tool > __SBOX_SANDBOX_DIR__/out/out", }, { name: "tool2", @@ -196,7 +196,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location :tool) > $(out)", `, - expect: "out/tool > __SBOX_SANDBOX_DIR__/out/out", + expect: "__SBOX_SANDBOX_DIR__/tools/out/bin/tool > __SBOX_SANDBOX_DIR__/out/out", }, { name: "tool file", @@ -205,7 +205,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location tool_file1) > $(out)", `, - expect: "tool_file1 > __SBOX_SANDBOX_DIR__/out/out", + expect: "__SBOX_SANDBOX_DIR__/tools/src/tool_file1 > __SBOX_SANDBOX_DIR__/out/out", }, { name: "tool file fg", @@ -214,7 +214,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location :1tool_file) > $(out)", `, - expect: "tool_file1 > __SBOX_SANDBOX_DIR__/out/out", + expect: "__SBOX_SANDBOX_DIR__/tools/src/tool_file1 > __SBOX_SANDBOX_DIR__/out/out", }, { name: "tool files", @@ -223,7 +223,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(locations :tool_files) > $(out)", `, - expect: "tool_file1 tool_file2 > __SBOX_SANDBOX_DIR__/out/out", + expect: "__SBOX_SANDBOX_DIR__/tools/src/tool_file1 __SBOX_SANDBOX_DIR__/tools/src/tool_file2 > __SBOX_SANDBOX_DIR__/out/out", }, { name: "in1", @@ -600,7 +600,7 @@ func TestGenSrcs(t *testing.T) { cmd: "$(location) $(in) > $(out)", `, cmds: []string{ - "bash -c 'out/tool in1.txt > __SBOX_SANDBOX_DIR__/out/in1.h' && bash -c 'out/tool in2.txt > __SBOX_SANDBOX_DIR__/out/in2.h'", + "bash -c '__SBOX_SANDBOX_DIR__/tools/out/bin/tool in1.txt > __SBOX_SANDBOX_DIR__/out/in1.h' && bash -c '__SBOX_SANDBOX_DIR__/tools/out/bin/tool in2.txt > __SBOX_SANDBOX_DIR__/out/in2.h'", }, deps: []string{buildDir + "/.intermediates/gen/gen/gensrcs/in1.h", buildDir + "/.intermediates/gen/gen/gensrcs/in2.h"}, files: []string{buildDir + "/.intermediates/gen/gen/gensrcs/in1.h", buildDir + "/.intermediates/gen/gen/gensrcs/in2.h"}, @@ -614,8 +614,8 @@ func TestGenSrcs(t *testing.T) { shard_size: 2, `, cmds: []string{ - "bash -c 'out/tool in1.txt > __SBOX_SANDBOX_DIR__/out/in1.h' && bash -c 'out/tool in2.txt > __SBOX_SANDBOX_DIR__/out/in2.h'", - "bash -c 'out/tool in3.txt > __SBOX_SANDBOX_DIR__/out/in3.h'", + "bash -c '__SBOX_SANDBOX_DIR__/tools/out/bin/tool in1.txt > __SBOX_SANDBOX_DIR__/out/in1.h' && bash -c '__SBOX_SANDBOX_DIR__/tools/out/bin/tool in2.txt > __SBOX_SANDBOX_DIR__/out/in2.h'", + "bash -c '__SBOX_SANDBOX_DIR__/tools/out/bin/tool in3.txt > __SBOX_SANDBOX_DIR__/out/in3.h'", }, deps: []string{buildDir + "/.intermediates/gen/gen/gensrcs/in1.h", buildDir + "/.intermediates/gen/gen/gensrcs/in2.h", buildDir + "/.intermediates/gen/gen/gensrcs/in3.h"}, files: []string{buildDir + "/.intermediates/gen/gen/gensrcs/in1.h", buildDir + "/.intermediates/gen/gen/gensrcs/in2.h", buildDir + "/.intermediates/gen/gen/gensrcs/in3.h"}, @@ -758,7 +758,7 @@ func toolFactory() android.Module { } func (t *testTool) GenerateAndroidBuildActions(ctx android.ModuleContext) { - t.outputFile = android.PathForTesting("out", ctx.ModuleName()) + t.outputFile = ctx.InstallFile(android.PathForModuleInstall(ctx, "bin"), ctx.ModuleName(), android.PathForOutput(ctx, ctx.ModuleName())) } func (t *testTool) HostToolPath() android.OptionalPath {