Merge "Remove support for stripping dex."

dexpreopt/config.go

@@ -24,8 +24,6 @@ import (
 
 // GlobalConfig stores the configuration for dex preopting set by the product
 type GlobalConfig struct {
-	DefaultNoStripping bool // don't strip dex files by default
-
 	DisablePreopt        bool     // disable preopt for all modules
 	DisablePreoptModules []string // modules with preopt disabled by product-specific config
 
@@ -56,7 +54,6 @@ type GlobalConfig struct {
 	SystemServerCompilerFilter string // default compiler filter to pass to dex2oat for system server jars
 
 	GenerateDMFiles bool // generate Dex Metadata files
-	NeverAllowStripping bool // whether stripping should not be done - used as build time check to make sure dex files are always available
 
 	NoDebugInfo                 bool // don't generate debug info by default
 	DontResolveStartupStrings   bool // don't resolve string literals loaded during application startup.
@@ -133,10 +130,6 @@ type ModuleConfig struct {
 	ForceCreateAppImage bool
 
 	PresignedPrebuilt bool
-
-	NoStripping     bool
-	StripInputPath  android.Path
-	StripOutputPath android.WritablePath
 }
 
 func constructPath(ctx android.PathContext, path string) android.Path {
@@ -233,8 +226,6 @@ func LoadModuleConfig(ctx android.PathContext, path string) (ModuleConfig, error
 		LibraryPaths                map[string]string
 		DexPreoptImages             []string
 		PreoptBootClassPathDexFiles []string
-		StripInputPath              string
-		StripOutputPath             string
 	}
 
 	config := ModuleJSONConfig{}
@@ -252,8 +243,6 @@ func LoadModuleConfig(ctx android.PathContext, path string) (ModuleConfig, error
 	config.ModuleConfig.LibraryPaths = constructPathMap(ctx, config.LibraryPaths)
 	config.ModuleConfig.DexPreoptImages = constructPaths(ctx, config.DexPreoptImages)
 	config.ModuleConfig.PreoptBootClassPathDexFiles = constructPaths(ctx, config.PreoptBootClassPathDexFiles)
-	config.ModuleConfig.StripInputPath = constructPath(ctx, config.StripInputPath)
-	config.ModuleConfig.StripOutputPath = constructWritablePath(ctx, config.StripOutputPath)
 
 	// This needs to exist, but dependencies are already handled in Make, so we don't need to pass them through JSON.
 	config.ModuleConfig.DexPreoptImagesDeps = make([]android.Paths, len(config.ModuleConfig.DexPreoptImages))
@@ -283,7 +272,6 @@ func loadConfig(ctx android.PathContext, path string, config interface{}) ([]byt
 
 func GlobalConfigForTests(ctx android.PathContext) GlobalConfig {
 	return GlobalConfig{
-		DefaultNoStripping:                 false,
 		DisablePreopt:                      false,
 		DisablePreoptModules:               nil,
 		OnlyPreoptBootImageAndSystemServer: false,
@@ -302,7 +290,6 @@ func GlobalConfigForTests(ctx android.PathContext) GlobalConfig {
 		DefaultCompilerFilter:              "",
 		SystemServerCompilerFilter:         "",
 		GenerateDMFiles:                    false,
-		NeverAllowStripping:                false,
 		NoDebugInfo:                        false,
 		DontResolveStartupStrings:          false,
 		AlwaysSystemServerDebugInfo:        false,

dexpreopt/dexpreopt.go

@@ -13,7 +13,7 @@
 // limitations under the License.
 
 // The dexpreopt package converts a global dexpreopt config and a module dexpreopt config into rules to perform
-// dexpreopting and to strip the dex files from the APK or JAR.
+// dexpreopting.
 //
 // It is used in two places; in the dexpeopt_gen binary for modules defined in Make, and directly linked into Soong.
 //
@@ -22,8 +22,7 @@
 // changed.  One script takes an APK or JAR as an input and produces a zip file containing any outputs of preopting,
 // in the location they should be on the device.  The Make build rules will unzip the zip file into $(PRODUCT_OUT) when
 // installing the APK, which will install the preopt outputs into $(PRODUCT_OUT)/system or $(PRODUCT_OUT)/system_other
-// as necessary.  The zip file may be empty if preopting was disabled for any reason.  The second script takes an APK or
+// as necessary.  The zip file may be empty if preopting was disabled for any reason.
-// JAR as an input and strips the dex files in it as necessary.
 //
 // The intermediate shell scripts allow changes to this package or to the global config to regenerate the shell scripts
 // but only require re-executing preopting if the script has changed.
@@ -48,45 +47,6 @@ import (
 const SystemPartition = "/system/"
 const SystemOtherPartition = "/system_other/"
 
-// GenerateStripRule generates a set of commands that will take an APK or JAR as an input and strip the dex files if
-// they are no longer necessary after preopting.
-func GenerateStripRule(global GlobalConfig, module ModuleConfig) (rule *android.RuleBuilder, err error) {
-	defer func() {
-		if r := recover(); r != nil {
-			if _, ok := r.(runtime.Error); ok {
-				panic(r)
-			} else if e, ok := r.(error); ok {
-				err = e
-				rule = nil
-			} else {
-				panic(r)
-			}
-		}
-	}()
-
-	tools := global.Tools
-
-	rule = android.NewRuleBuilder()
-
-	strip := shouldStripDex(module, global)
-
-	if strip {
-		if global.NeverAllowStripping {
-			panic(fmt.Errorf("Stripping requested on %q, though the product does not allow it", module.DexLocation))
-		}
-		// Only strips if the dex files are not already uncompressed
-		rule.Command().
-			Textf(`if (zipinfo %s '*.dex' 2>/dev/null | grep -v ' stor ' >/dev/null) ; then`, module.StripInputPath).
-			Tool(tools.Zip2zip).FlagWithInput("-i ", module.StripInputPath).FlagWithOutput("-o ", module.StripOutputPath).
-			FlagWithArg("-x ", `"classes*.dex"`).
-			Textf(`; else cp -f %s %s; fi`, module.StripInputPath, module.StripOutputPath)
-	} else {
-		rule.Command().Text("cp -f").Input(module.StripInputPath).Output(module.StripOutputPath)
-	}
-
-	return rule, nil
-}
-
 // GenerateDexpreoptRule generates a set of commands that will preopt a module based on a GlobalConfig and a
 // ModuleConfig.  The produced files and their install locations will be available through rule.Installs().
 func GenerateDexpreoptRule(ctx android.PathContext,
@@ -120,7 +80,6 @@ func GenerateDexpreoptRule(ctx android.PathContext,
 
 	if !dexpreoptDisabled(global, module) {
 		// Don't preopt individual boot jars, they will be preopted together.
-		// This check is outside dexpreoptDisabled because they still need to be stripped.
 		if !contains(global.BootJars, module.Name) {
 			appImage := (generateProfile || module.ForceCreateAppImage || global.DefaultAppImages) &&
 				!module.NoCreateAppImage
@@ -515,51 +474,6 @@ func dexpreoptCommand(ctx android.PathContext, global GlobalConfig, module Modul
 	rule.Install(vdexPath, vdexInstallPath)
 }
 
-// Return if the dex file in the APK should be stripped.  If an APK is found to contain uncompressed dex files at
-// dex2oat time it will not be stripped even if strip=true.
-func shouldStripDex(module ModuleConfig, global GlobalConfig) bool {
-	strip := !global.DefaultNoStripping
-
-	if dexpreoptDisabled(global, module) {
-		strip = false
-	}
-
-	if module.NoStripping {
-		strip = false
-	}
-
-	// Don't strip modules that are not on the system partition in case the oat/vdex version in system ROM
-	// doesn't match the one in other partitions. It needs to be able to fall back to the APK for that case.
-	if !strings.HasPrefix(module.DexLocation, SystemPartition) {
-		strip = false
-	}
-
-	// system_other isn't there for an OTA, so don't strip if module is on system, and odex is on system_other.
-	if odexOnSystemOther(module, global) {
-		strip = false
-	}
-
-	if module.HasApkLibraries {
-		strip = false
-	}
-
-	// Don't strip with dex files we explicitly uncompress (dexopt will not store the dex code).
-	if module.UncompressedDex {
-		strip = false
-	}
-
-	if shouldGenerateDM(module, global) {
-		strip = false
-	}
-
-	if module.PresignedPrebuilt {
-		// Only strip out files if we can re-sign the package.
-		strip = false
-	}
-
-	return strip
-}
-
 func shouldGenerateDM(module ModuleConfig, global GlobalConfig) bool {
 	// Generating DM files only makes sense for verify, avoid doing for non verify compiler filter APKs.
 	// No reason to use a dm file if the dex is already uncompressed.

dexpreopt/dexpreopt_gen/dexpreopt_gen.go

@@ -31,7 +31,6 @@ import (
 
 var (
 	dexpreoptScriptPath = flag.String("dexpreopt_script", "", "path to output dexpreopt script")
-	stripScriptPath     = flag.String("strip_script", "", "path to output strip script")
 	globalConfigPath    = flag.String("global", "", "path to global configuration file")
 	moduleConfigPath    = flag.String("module", "", "path to module configuration file")
 	outDir              = flag.String("out_dir", "", "path to output directory")
@@ -64,10 +63,6 @@ func main() {
 		usage("path to output dexpreopt script is required")
 	}
 
-	if *stripScriptPath == "" {
-		usage("path to output strip script is required")
-	}
-
 	if *globalConfigPath == "" {
 		usage("path to global configuration file is required")
 	}
@@ -90,10 +85,6 @@ func main() {
 		os.Exit(2)
 	}
 
-	// This shouldn't be using *PathForTesting, but it's outside of soong_build so its OK for now.
-	moduleConfig.StripInputPath = android.PathForTesting("$1")
-	moduleConfig.StripOutputPath = android.WritablePathForTesting("$2")
-
 	moduleConfig.DexPath = android.PathForTesting("$1")
 
 	defer func() {
@@ -110,11 +101,11 @@ func main() {
 		}
 	}()
 
-	writeScripts(ctx, globalConfig, moduleConfig, *dexpreoptScriptPath, *stripScriptPath)
+	writeScripts(ctx, globalConfig, moduleConfig, *dexpreoptScriptPath)
 }
 
 func writeScripts(ctx android.PathContext, global dexpreopt.GlobalConfig, module dexpreopt.ModuleConfig,
-	dexpreoptScriptPath, stripScriptPath string) {
+	dexpreoptScriptPath string) {
 	dexpreoptRule, err := dexpreopt.GenerateDexpreoptRule(ctx, global, module)
 	if err != nil {
 		panic(err)
@@ -135,11 +126,6 @@ func writeScripts(ctx android.PathContext, global dexpreopt.GlobalConfig, module
 		FlagWithArg("-C ", installDir.String()).
 		FlagWithArg("-D ", installDir.String())
 
-	stripRule, err := dexpreopt.GenerateStripRule(global, module)
-	if err != nil {
-		panic(err)
-	}
-
 	write := func(rule *android.RuleBuilder, file string) {
 		script := &bytes.Buffer{}
 		script.WriteString(scriptHeader)
@@ -180,15 +166,8 @@ func writeScripts(ctx android.PathContext, global dexpreopt.GlobalConfig, module
 	if module.DexPath.String() != "$1" {
 		panic(fmt.Errorf("module.DexPath must be '$1', was %q", module.DexPath))
 	}
-	if module.StripInputPath.String() != "$1" {
-		panic(fmt.Errorf("module.StripInputPath must be '$1', was %q", module.StripInputPath))
-	}
-	if module.StripOutputPath.String() != "$2" {
-		panic(fmt.Errorf("module.StripOutputPath must be '$2', was %q", module.StripOutputPath))
-	}
 
 	write(dexpreoptRule, dexpreoptScriptPath)
-	write(stripRule, stripScriptPath)
 }
 
 const scriptHeader = `#!/bin/bash

dexpreopt/dexpreopt_test.go

@@ -17,8 +17,6 @@ package dexpreopt
 import (
 	"android/soong/android"
 	"fmt"
-	"reflect"
-	"strings"
 	"testing"
 )
 
@@ -58,9 +56,6 @@ func testModuleConfig(ctx android.PathContext, name, partition string) ModuleCon
 		NoCreateAppImage:                false,
 		ForceCreateAppImage:             false,
 		PresignedPrebuilt:               false,
-		NoStripping:                     false,
-		StripInputPath:                  android.PathForOutput(ctx, fmt.Sprintf("unstripped/%s.apk", name)),
-		StripOutputPath:                 android.PathForOutput(ctx, fmt.Sprintf("stripped/%s.apk", name)),
 	}
 }
 
@@ -83,20 +78,6 @@ func TestDexPreopt(t *testing.T) {
 	}
 }
 
-func TestDexPreoptStrip(t *testing.T) {
-	// Test that we panic if we strip in a configuration where stripping is not allowed.
-	ctx := android.PathContextForTesting(android.TestConfig("out", nil), nil)
-	global, module := GlobalConfigForTests(ctx), testSystemModuleConfig(ctx, "test")
-
-	global.NeverAllowStripping = true
-	module.NoStripping = false
-
-	_, err := GenerateStripRule(global, module)
-	if err == nil {
-		t.Errorf("Expected an error when calling GenerateStripRule on a stripped module")
-	}
-}
-
 func TestDexPreoptSystemOther(t *testing.T) {
 	ctx := android.PathContextForTesting(android.TestConfig("out", nil), nil)
 	global := GlobalConfigForTests(ctx)
@@ -176,56 +157,3 @@ func TestDexPreoptProfile(t *testing.T) {
 		t.Errorf("\nwant installs:\n   %v\ngot:\n   %v", wantInstalls, rule.Installs())
 	}
 }
-
-func TestStripDex(t *testing.T) {
-	tests := []struct {
-		name  string
-		setup func(global *GlobalConfig, module *ModuleConfig)
-		strip bool
-	}{
-		{
-			name:  "default strip",
-			setup: func(global *GlobalConfig, module *ModuleConfig) {},
-			strip: true,
-		},
-		{
-			name:  "global no stripping",
-			setup: func(global *GlobalConfig, module *ModuleConfig) { global.DefaultNoStripping = true },
-			strip: false,
-		},
-		{
-			name:  "module no stripping",
-			setup: func(global *GlobalConfig, module *ModuleConfig) { module.NoStripping = true },
-			strip: false,
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-
-			ctx := android.PathContextForTesting(android.TestConfig("out", nil), nil)
-			global, module := GlobalConfigForTests(ctx), testSystemModuleConfig(ctx, "test")
-
-			test.setup(&global, &module)
-
-			rule, err := GenerateStripRule(global, module)
-			if err != nil {
-				t.Fatal(err)
-			}
-
-			if test.strip {
-				want := `zip2zip -i out/unstripped/test.apk -o out/stripped/test.apk -x "classes*.dex"`
-				if len(rule.Commands()) < 1 || !strings.Contains(rule.Commands()[0], want) {
-					t.Errorf("\nwant commands[0] to have:\n   %v\ngot:\n   %v", want, rule.Commands()[0])
-				}
-			} else {
-				wantCommands := []string{
-					"cp -f out/unstripped/test.apk out/stripped/test.apk",
-				}
-				if !reflect.DeepEqual(rule.Commands(), wantCommands) {
-					t.Errorf("\nwant commands:\n   %v\ngot:\n   %v", wantCommands, rule.Commands())
-				}
-			}
-		})
-	}
-}

java/app_test.go

@@ -1149,13 +1149,7 @@ func TestAndroidAppImport_Presigned(t *testing.T) {
 		variant.MaybeOutput("dexpreopt/oat/arm64/package.odex").Rule == nil {
 		t.Errorf("can't find dexpreopt outputs")
 	}
-	// Make sure stripping wasn't done.
+	// Make sure signing was skipped and aligning was done.
-	stripRule := variant.Output("dexpreopt/foo.apk")
-	if !strings.HasPrefix(stripRule.RuleParams.Command, "cp -f") {
-		t.Errorf("unexpected, non-skipping strip command: %q", stripRule.RuleParams.Command)
-	}
-
-	// Make sure signing was skipped and aligning was done instead.
 	if variant.MaybeOutput("signed/foo.apk").Rule != nil {
 		t.Errorf("signing rule shouldn't be included.")
 	}

java/dexpreopt.go

@@ -40,13 +40,9 @@ type dexpreopter struct {
 
 type DexpreoptProperties struct {
 	Dex_preopt struct {
-		// If false, prevent dexpreopting and stripping the dex file from the final jar.  Defaults to
+		// If false, prevent dexpreopting.  Defaults to true.
-		// true.
 		Enabled *bool
 
-		// If true, never strip the dex files from the final jar when dexpreopting.  Defaults to false.
-		No_stripping *bool
-
 		// If true, generate an app image (.art file) for this module.
 		App_image *bool
 
@@ -136,8 +132,6 @@ func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, dexJarFile android.Mo
 
 	dexLocation := android.InstallPathToOnDevicePath(ctx, d.installPath)
 
-	strippedDexJarFile := android.PathForModuleOut(ctx, "dexpreopt", dexJarFile.Base())
-
 	var profileClassListing android.OptionalPath
 	var profileBootListing android.OptionalPath
 	profileIsTextListing := false
@@ -191,10 +185,6 @@ func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, dexJarFile android.Mo
 		ForceCreateAppImage: BoolDefault(d.dexpreoptProperties.Dex_preopt.App_image, false),
 
 		PresignedPrebuilt: d.isPresignedPrebuilt,
-
-		NoStripping:     Bool(d.dexpreoptProperties.Dex_preopt.No_stripping),
-		StripInputPath:  dexJarFile,
-		StripOutputPath: strippedDexJarFile.OutputPath,
 	}
 
 	dexpreoptRule, err := dexpreopt.GenerateDexpreoptRule(ctx, global, dexpreoptConfig)
@@ -207,13 +197,5 @@ func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, dexJarFile android.Mo
 
 	d.builtInstalled = dexpreoptRule.Installs().String()
 
-	stripRule, err := dexpreopt.GenerateStripRule(global, dexpreoptConfig)
-	if err != nil {
-		ctx.ModuleErrorf("error generating dexpreopt strip rule: %s", err.Error())
 	return dexJarFile
-	}
-
-	stripRule.Build(pctx, ctx, "dexpreopt_strip", "dexpreopt strip")
-
-	return strippedDexJarFile
 }