From 8130c482abf16bb1fdb34250ece258ccd7423780 Mon Sep 17 00:00:00 2001 From: Ulya Trafimovich Date: Wed, 7 Oct 2020 15:17:13 +0100 Subject: [PATCH] Move part of logic from construct_context.py to Soong. construct_context.py is a script that runs at Ninja stage and constructs class loader context arguments for dex2oat. Previously it accepted lists of library paths and concatenated them into one class loader context string. The script also handled the dependency of "android.hidl.manager" on "android.hidl.base" that is not tracked by the build system and must be handled in a special way. Now that class loader context representation is going to change from flat lists to trees, passing paths to individual libraries as script arguments is no longer possible, because a list of paths cannot represent a class loader context tree. Passing the trees in a serialized form is also inconvenient, because the script would have to parse them, which would complicate it a lot. Therefore this patch ports all the concatenation and "android.hidl.base" handling to Soong. It is not possible to port the remaining script to Soong because Soong has no information about the targetSdkVersiion of the dexpreopted apps (it is in the manifest and sometimes in an APK, and has to be extracted at Ninja time). Test: construct_context_test.py Test: new subtests in TestUsesLibs Test: lunch aosp_cf_x86_phone-userdebug && m Bug: 132357300 Change-Id: Icdb03cf00d1e27e4cff3844b89bfaec4de502dd7 --- dexpreopt/dexpreopt.go | 94 +++++++++++++++++++++++++++---- java/app_test.go | 43 +++++++------- scripts/construct_context.py | 43 ++++++-------- scripts/construct_context_test.py | 26 ++++----- 4 files changed, 131 insertions(+), 75 deletions(-) diff --git a/dexpreopt/dexpreopt.go b/dexpreopt/dexpreopt.go index 3605d9d8a..66e765f5a 100644 --- a/dexpreopt/dexpreopt.go +++ b/dexpreopt/dexpreopt.go @@ -328,9 +328,42 @@ func genClassLoaderContext(ctx android.PathContext, global *GlobalConfig, module return &classLoaderContexts } +// Find build and install paths to "android.hidl.base". The library must be present in conditional +// class loader context for SDK version 29, because it's one of the compatibility libraries. +func findHidlBasePaths(ctx android.PathContext, clcMap classLoaderContextMap) (android.Path, string) { + var hostPath android.Path + targetPath := UnknownInstallLibraryPath + + if clc, ok := clcMap[29]; ok { + for i, lib := range clc.Names { + if lib == AndroidHidlBase { + hostPath = clc.Host[i] + targetPath = clc.Target[i] + break + } + } + } + + // Fail if the library paths were not found. This may happen if the function is called at the + // wrong time (either before the compatibility libraries were added to context, or after they + // have been removed for some reason). + if hostPath == nil { + android.ReportPathErrorf(ctx, "dexpreopt cannot find build path to '%s'", AndroidHidlBase) + } else if targetPath == UnknownInstallLibraryPath { + android.ReportPathErrorf(ctx, "dexpreopt cannot find install path to '%s'", AndroidHidlBase) + } + + return hostPath, targetPath +} + // Now that the full unconditional context is known, reconstruct conditional context. // Apply filters for individual libraries, mirroring what the PackageManager does when it // constructs class loader context on device. +// +// TODO(b/132357300): +// - move handling of android.hidl.manager -> android.hidl.base dependency here +// - remove android.hidl.manager and android.hidl.base unless the app is a system app. +// func fixConditionalClassLoaderContext(clcMap classLoaderContextMap) { usesLibs := clcMap.usesLibs() @@ -352,6 +385,52 @@ func fixConditionalClassLoaderContext(clcMap classLoaderContextMap) { } } +// Return the class loader context as a string and a slice of build paths for all dependencies. +func computeClassLoaderContext(ctx android.PathContext, clcMap classLoaderContextMap) (clcStr string, paths android.Paths) { + hidlBaseHostPath, hidlBaseTargetPath := findHidlBasePaths(ctx, clcMap) + + for _, ver := range android.SortedIntKeys(clcMap) { + clc := clcMap.getValue(ver) + + clcLen := len(clc.Names) + if clcLen != len(clc.Host) || clcLen != len(clc.Target) { + android.ReportPathErrorf(ctx, "ill-formed class loader context") + } + + var hostClc, targetClc []string + var hostPaths android.Paths + + for i := 0; i < clcLen; i++ { + hostStr := "PCL[" + clc.Host[i].String() + "]" + targetStr := "PCL[" + clc.Target[i] + "]" + + // Add dependency of android.hidl.manager on android.hidl.base (it is not tracked as + // a regular dependency by the build system, so it needs special handling). + if clc.Names[i] == AndroidHidlManager { + hostStr += "{PCL[" + hidlBaseHostPath.String() + "]}" + targetStr += "{PCL[" + hidlBaseTargetPath + "]}" + hostPaths = append(hostPaths, hidlBaseHostPath) + } + + hostClc = append(hostClc, hostStr) + targetClc = append(targetClc, targetStr) + hostPaths = append(hostPaths, clc.Host[i]) + } + + if hostPaths != nil { + sdkVerStr := fmt.Sprintf("%d", ver) + if ver == AnySdkVersion { + sdkVerStr = "any" // a special keyword that means any SDK version + } + clcStr += fmt.Sprintf(" --host-context-for-sdk %s %s", sdkVerStr, strings.Join(hostClc, "#")) + clcStr += fmt.Sprintf(" --target-context-for-sdk %s %s", sdkVerStr, strings.Join(targetClc, "#")) + paths = append(paths, hostPaths...) + } + } + + return clcStr, paths +} + func dexpreoptCommand(ctx android.PathContext, globalSoong *GlobalSoongConfig, global *GlobalConfig, module *ModuleConfig, rule *android.RuleBuilder, archIdx int, classLoaderContexts classLoaderContextMap, profile android.WritablePath, appImage bool, generateDM bool) { @@ -422,20 +501,11 @@ func dexpreoptCommand(ctx android.PathContext, globalSoong *GlobalSoongConfig, g } // Generate command that saves host and target class loader context in shell variables. + clc, paths := computeClassLoaderContext(ctx, classLoaderContexts) cmd := rule.Command(). Text(`eval "$(`).Tool(globalSoong.ConstructContext). - Text(` --target-sdk-version ${target_sdk_version}`) - for _, ver := range android.SortedIntKeys(classLoaderContexts) { - if clc := classLoaderContexts.getValue(ver); len(clc.Host) > 0 { - verString := fmt.Sprintf("%d", ver) - if ver == AnySdkVersion { - verString = "any" // a special keyword that means any SDK version - } - cmd.Textf(`--host-classpath-for-sdk %s %s`, verString, strings.Join(clc.Host.Strings(), ":")). - Implicits(clc.Host). - Textf(`--target-classpath-for-sdk %s %s`, verString, strings.Join(clc.Target, ":")) - } - } + Text(` --target-sdk-version ${target_sdk_version}`). + Text(clc).Implicits(paths) cmd.Text(`)"`) } else { // Pass special class loader context to skip the classpath and collision check. diff --git a/java/app_test.go b/java/app_test.go index f2e434957..8c7757336 100644 --- a/java/app_test.go +++ b/java/app_test.go @@ -2812,51 +2812,52 @@ func TestUsesLibraries(t *testing.T) { t.Errorf("wanted %q in %q", w, cmd) } - // Test that all present libraries are preopted, including implicit SDK dependencies, possibly stubs. + // Test that all present libraries are preopted, including implicit SDK dependencies, possibly stubs cmd = app.Rule("dexpreopt").RuleParams.Command - w := `--target-classpath-for-sdk any` + - ` /system/framework/foo.jar` + - `:/system/framework/quuz.jar` + - `:/system/framework/qux.jar` + - `:/system/framework/runtime-library.jar` + - `:/system/framework/bar.jar ` + w := `--target-context-for-sdk any ` + + `PCL[/system/framework/foo.jar]#` + + `PCL[/system/framework/quuz.jar]#` + + `PCL[/system/framework/qux.jar]#` + + `PCL[/system/framework/runtime-library.jar]#` + + `PCL[/system/framework/bar.jar]` if !strings.Contains(cmd, w) { t.Errorf("wanted %q in %q", w, cmd) } // Test conditional context for target SDK version 28. - if w := `--target-classpath-for-sdk 28` + - ` /system/framework/org.apache.http.legacy.jar `; !strings.Contains(cmd, w) { + if w := `--target-context-for-sdk 28` + + ` PCL[/system/framework/org.apache.http.legacy.jar] `; !strings.Contains(cmd, w) { t.Errorf("wanted %q in %q", w, cmd) } // Test conditional context for target SDK version 29. - if w := `--target-classpath-for-sdk 29` + - ` /system/framework/android.hidl.base-V1.0-java.jar` + - `:/system/framework/android.hidl.manager-V1.0-java.jar `; !strings.Contains(cmd, w) { + // Hardcoded dependency "android.hidl.manager" -> "android.hidl.base" is present. + if w := `--target-context-for-sdk 29` + + ` PCL[/system/framework/android.hidl.base-V1.0-java.jar]` + + `#PCL[/system/framework/android.hidl.manager-V1.0-java.jar]{PCL[/system/framework/android.hidl.base-V1.0-java.jar]} `; !strings.Contains(cmd, w) { t.Errorf("wanted %q in %q", w, cmd) } // Test conditional context for target SDK version 30. // "android.test.mock" is absent because "android.test.runner" is not used. - if w := `--target-classpath-for-sdk 30` + - ` /system/framework/android.test.base.jar `; !strings.Contains(cmd, w) { + if w := `--target-context-for-sdk 30` + + ` PCL[/system/framework/android.test.base.jar] `; !strings.Contains(cmd, w) { t.Errorf("wanted %q in %q", w, cmd) } cmd = prebuilt.Rule("dexpreopt").RuleParams.Command - if w := `--target-classpath-for-sdk any` + - ` /system/framework/foo.jar` + - `:/system/framework/android.test.runner.jar` + - `:/system/framework/bar.jar `; !strings.Contains(cmd, w) { + if w := `--target-context-for-sdk any` + + ` PCL[/system/framework/foo.jar]` + + `#PCL[/system/framework/android.test.runner.jar]` + + `#PCL[/system/framework/bar.jar] `; !strings.Contains(cmd, w) { t.Errorf("wanted %q in %q", w, cmd) } // Test conditional context for target SDK version 30. // "android.test.mock" is present because "android.test.runner" is used. - if w := `--target-classpath-for-sdk 30` + - ` /system/framework/android.test.base.jar` + - `:/system/framework/android.test.mock.jar `; !strings.Contains(cmd, w) { + if w := `--target-context-for-sdk 30` + + ` PCL[/system/framework/android.test.base.jar]` + + `#PCL[/system/framework/android.test.mock.jar] `; !strings.Contains(cmd, w) { t.Errorf("wanted %q in %q", w, cmd) } } diff --git a/scripts/construct_context.py b/scripts/construct_context.py index 8717fe305..6f9edc429 100755 --- a/scripts/construct_context.py +++ b/scripts/construct_context.py @@ -29,41 +29,32 @@ def parse_args(args): parser = argparse.ArgumentParser() parser.add_argument('--target-sdk-version', default='', dest='sdk', help='specify target SDK version (as it appears in the manifest)') - parser.add_argument('--host-classpath-for-sdk', dest='host_classpaths', - action='append', nargs=2, metavar=('sdk','classpath'), - help='specify classpath on host for a given SDK version or "any" version') - parser.add_argument('--target-classpath-for-sdk', dest='target_classpaths', - action='append', nargs=2, metavar=('sdk','classpath'), - help='specify classpath on target for a given SDK version or "any" version') + parser.add_argument('--host-context-for-sdk', dest='host_contexts', + action='append', nargs=2, metavar=('sdk','context'), + help='specify context on host for a given SDK version or "any" version') + parser.add_argument('--target-context-for-sdk', dest='target_contexts', + action='append', nargs=2, metavar=('sdk','context'), + help='specify context on target for a given SDK version or "any" version') return parser.parse_args(args) -# The hidl.manager shared library has a dependency on hidl.base. We manually -# add that information to the class loader context if we see those libraries. -HIDL_MANAGER = 'android.hidl.manager-V1.0-java' -HIDL_BASE = 'android.hidl.base-V1.0-java' - -# Special keyword that means that the classpath should be added to class loader +# Special keyword that means that the context should be added to class loader # context regardless of the target SDK version. any_sdk = 'any' -# We assume that the order of classpath arguments passed to this script is +# We assume that the order of context arguments passed to this script is # correct (matches the order computed by package manager). It is possible to # sort them here, but Soong needs to use deterministic order anyway, so it can # as well use the correct order. -def construct_context(versioned_classpaths, target_sdk): +def construct_context(versioned_contexts, target_sdk): context = [] - for [sdk, classpath] in versioned_classpaths: + for [sdk, ctx] in versioned_contexts: if sdk == any_sdk or compare_version_gt(sdk, target_sdk): - for jar in classpath.split(':'): - pcl = 'PCL[%s]' % jar - if HIDL_MANAGER in jar: - pcl += '{PCL[%s]}' % jar.replace(HIDL_MANAGER, HIDL_BASE, 1) - context.append(pcl) + context.append(ctx) return context def construct_contexts(args): - host_context = construct_context(args.host_classpaths, args.sdk) - target_context = construct_context(args.target_classpaths, args.sdk) + host_context = construct_context(args.host_contexts, args.sdk) + target_context = construct_context(args.target_contexts, args.sdk) context_sep = '#' return ('class_loader_context_arg=--class-loader-context=PCL[]{%s} ; ' % context_sep.join(host_context) + 'stored_class_loader_context_arg=--stored-class-loader-context=PCL[]{%s}' % context_sep.join(target_context)) @@ -74,10 +65,10 @@ def main(): args = parse_args(sys.argv[1:]) if not args.sdk: raise SystemExit('target sdk version is not set') - if not args.host_classpaths: - raise SystemExit('host classpath is not set') - if not args.target_classpaths: - raise SystemExit('target classpath is not set') + if not args.host_contexts: + raise SystemExit('host context is not set') + if not args.target_contexts: + raise SystemExit('target context is not set') print(construct_contexts(args)) diff --git a/scripts/construct_context_test.py b/scripts/construct_context_test.py index 0b0b0a315..3b05f9013 100755 --- a/scripts/construct_context_test.py +++ b/scripts/construct_context_test.py @@ -27,53 +27,47 @@ def construct_contexts(arglist): args = cc.parse_args(arglist) return cc.construct_contexts(args) -classpaths = [ - '--host-classpath-for-sdk', '28', 'out/zdir/z.jar', - '--target-classpath-for-sdk', '28', '/system/z.jar', - '--host-classpath-for-sdk', '29', 'out/xdir/x.jar:out/ydir/y.jar', - '--target-classpath-for-sdk', '29', '/system/x.jar:/product/y.jar', - '--host-classpath-for-sdk', 'any', 'out/adir/a.jar:out/android.hidl.manager-V1.0-java.jar:out/bdir/b.jar', - '--target-classpath-for-sdk', 'any', '/system/a.jar:/system/android.hidl.manager-V1.0-java.jar:/product/b.jar', +contexts = [ + '--host-context-for-sdk', '28', 'PCL[out/zdir/z.jar]', + '--target-context-for-sdk', '28', 'PCL[/system/z.jar]', + '--host-context-for-sdk', '29', 'PCL[out/xdir/x.jar]#PCL[out/ydir/y.jar]', + '--target-context-for-sdk', '29', 'PCL[/system/x.jar]#PCL[/product/y.jar]', + '--host-context-for-sdk', 'any', 'PCL[out/adir/a.jar]#PCL[out/bdir/b.jar]', + '--target-context-for-sdk', 'any', 'PCL[/system/a.jar]#PCL[/product/b.jar]', ] class ConstructContextTest(unittest.TestCase): def test_construct_context_28(self): - args = ['--target-sdk-version', '28'] + classpaths + args = ['--target-sdk-version', '28'] + contexts result = construct_contexts(args) expect = ('class_loader_context_arg=--class-loader-context=PCL[]{PCL[out/xdir/x.jar]' '#PCL[out/ydir/y.jar]' '#PCL[out/adir/a.jar]' - '#PCL[out/android.hidl.manager-V1.0-java.jar]{PCL[out/android.hidl.base-V1.0-java.jar]}' '#PCL[out/bdir/b.jar]}' ' ; ' 'stored_class_loader_context_arg=--stored-class-loader-context=PCL[]{PCL[/system/x.jar]' '#PCL[/product/y.jar]' '#PCL[/system/a.jar]' - '#PCL[/system/android.hidl.manager-V1.0-java.jar]{PCL[/system/android.hidl.base-V1.0-java.jar]}' '#PCL[/product/b.jar]}') self.assertEqual(result, expect) def test_construct_context_29(self): - args = ['--target-sdk-version', '29'] + classpaths + args = ['--target-sdk-version', '29'] + contexts result = construct_contexts(args) expect = ('class_loader_context_arg=--class-loader-context=PCL[]{PCL[out/adir/a.jar]' - '#PCL[out/android.hidl.manager-V1.0-java.jar]{PCL[out/android.hidl.base-V1.0-java.jar]}' '#PCL[out/bdir/b.jar]}' ' ; ' 'stored_class_loader_context_arg=--stored-class-loader-context=PCL[]{PCL[/system/a.jar]' - '#PCL[/system/android.hidl.manager-V1.0-java.jar]{PCL[/system/android.hidl.base-V1.0-java.jar]}' '#PCL[/product/b.jar]}') self.assertEqual(result, expect) def test_construct_context_S(self): - args = ['--target-sdk-version', 'S'] + classpaths + args = ['--target-sdk-version', 'S'] + contexts result = construct_contexts(args) expect = ('class_loader_context_arg=--class-loader-context=PCL[]{PCL[out/adir/a.jar]' - '#PCL[out/android.hidl.manager-V1.0-java.jar]{PCL[out/android.hidl.base-V1.0-java.jar]}' '#PCL[out/bdir/b.jar]}' ' ; ' 'stored_class_loader_context_arg=--stored-class-loader-context=PCL[]{PCL[/system/a.jar]' - '#PCL[/system/android.hidl.manager-V1.0-java.jar]{PCL[/system/android.hidl.base-V1.0-java.jar]}' '#PCL[/product/b.jar]}') self.assertEqual(result, expect)