From f8f4af8f1a3045da2cb3b13c1dd575bd9f4a06f7 Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Fri, 12 Feb 2021 15:42:20 +0000 Subject: [PATCH 1/3] Remove implicit dependency from -> -hiddenapi It can be removed as all implicit dependencies have been replaced with explicit ones so it is no longer needed. Test: m droid Verified that hiddenapi files (both aggregated ones and for the individual modules) are not affected by this change. Change-Id: I3da5fcb3b244a295502f2cacc94a504250d4d616 --- java/hiddenapi.go | 43 ++++++++++---------------------- java/hiddenapi_singleton_test.go | 7 ------ 2 files changed, 13 insertions(+), 37 deletions(-) diff --git a/java/hiddenapi.go b/java/hiddenapi.go index f8e41c458..9caea944b 100644 --- a/java/hiddenapi.go +++ b/java/hiddenapi.go @@ -15,8 +15,6 @@ package java import ( - "strings" - "github.com/google/blueprint" "android/soong/android" @@ -29,8 +27,8 @@ var hiddenAPIGenerateCSVRule = pctx.AndroidStaticRule("hiddenAPIGenerateCSV", bl type hiddenAPI struct { // The name of the module as it would be used in the boot jars configuration, e.g. without any - // prebuilt_ prefix (if it is a prebuilt), without any "-hiddenapi" suffix if it just provides - // annotations and without any ".impl" suffix if it is a java_sdk_library implementation library. + // prebuilt_ prefix (if it is a prebuilt) and without any ".impl" suffix if it is a + // java_sdk_library implementation library. configurationName string // True if the module containing this structure contributes to the hiddenapi information or has @@ -49,11 +47,6 @@ type hiddenAPI struct { // annotation information. primary bool - // True if the module only contains additional annotations and so does not require hiddenapi - // information to be encoded in its dex file and should not be used to generate the - // hiddenAPISingletonPathsStruct.stubFlags file. - annotationsOnly bool - // The path to the dex jar that is in the boot class path. If this is nil then the associated // module is not a boot jar, but could be one of the -hiddenapi modules that provide additional // annotations for the boot dex jar but which do not actually provide a boot dex jar @@ -119,16 +112,12 @@ type hiddenAPIIntf interface { var _ hiddenAPIIntf = (*hiddenAPI)(nil) // Initialize the hiddenapi structure -func (h *hiddenAPI) initHiddenAPI(ctx android.BaseModuleContext, name string) { +func (h *hiddenAPI) initHiddenAPI(ctx android.BaseModuleContext, configurationName string) { // If hiddenapi processing is disabled treat this as inactive. if ctx.Config().IsEnvTrue("UNSAFE_DISABLE_HIDDENAPI_FLAGS") { return } - // Modules whose names are of the format -hiddenapi provide hiddenapi information for the boot - // jar module . Otherwise, the module provides information for itself. Either way extract the - // configurationName of the boot jar module. - configurationName := strings.TrimSuffix(name, "-hiddenapi") h.configurationName = configurationName // It is important that hiddenapi information is only gathered for/from modules that are actually @@ -141,10 +130,6 @@ func (h *hiddenAPI) initHiddenAPI(ctx android.BaseModuleContext, name string) { return } - // If this module has a suffix of -hiddenapi then it only provides additional annotation - // information for a module on the boot jars list. - h.annotationsOnly = strings.HasSuffix(name, "-hiddenapi") - // Determine whether this module is the primary module or not. primary := true @@ -156,11 +141,11 @@ func (h *hiddenAPI) initHiddenAPI(ctx android.BaseModuleContext, name string) { primary = p.UsePrebuilt() } } else { - // The only module that will pass a different name to its module name to this method is the - // implementation library of a java_sdk_library. It has a configuration name of the same - // as its parent java_sdk_library but a module name of .impl. It is not the primary module, - // the java_sdk_library with the name of is. - primary = name == ctx.ModuleName() + // The only module that will pass a different configurationName to its module name to this + // method is the implementation library of a java_sdk_library. It has a configuration name of + // the same as its parent java_sdk_library but a module name of .impl. It is not the + // primary module, the java_sdk_library with the name of is. + primary = configurationName == ctx.ModuleName() // A source module that has been replaced by a prebuilt can never be the primary module. primary = primary && !module.IsReplacedByPrebuilt() @@ -191,15 +176,13 @@ func (h *hiddenAPI) hiddenAPIExtractAndEncode(ctx android.ModuleContext, dexJar h.hiddenAPIExtractInformation(ctx, dexJar, implementationJar) - if !h.annotationsOnly { - hiddenAPIJar := android.PathForModuleOut(ctx, "hiddenapi", h.configurationName+".jar").OutputPath + hiddenAPIJar := android.PathForModuleOut(ctx, "hiddenapi", h.configurationName+".jar").OutputPath - // Create a copy of the dex jar which has been encoded with hiddenapi flags. - hiddenAPIEncodeDex(ctx, hiddenAPIJar, dexJar, uncompressDex) + // Create a copy of the dex jar which has been encoded with hiddenapi flags. + hiddenAPIEncodeDex(ctx, hiddenAPIJar, dexJar, uncompressDex) - // Use the encoded dex jar from here onwards. - dexJar = hiddenAPIJar - } + // Use the encoded dex jar from here onwards. + dexJar = hiddenAPIJar return dexJar } diff --git a/java/hiddenapi_singleton_test.go b/java/hiddenapi_singleton_test.go index 4670d0311..c0f0e381c 100644 --- a/java/hiddenapi_singleton_test.go +++ b/java/hiddenapi_singleton_test.go @@ -88,12 +88,6 @@ func TestHiddenAPIIndexSingleton(t *testing.T) { ], } - java_library { - name: "foo-hiddenapi", - srcs: ["a.java"], - compile_dex: true, - } - java_library { name: "foo-hiddenapi-annotations", srcs: ["a.java"], @@ -118,7 +112,6 @@ func TestHiddenAPIIndexSingleton(t *testing.T) { indexRule := hiddenAPIIndex.Rule("singleton-merged-hiddenapi-index") CheckHiddenAPIRuleInputs(t, ` .intermediates/bar/android_common/hiddenapi/index.csv -.intermediates/foo-hiddenapi/android_common/hiddenapi/index.csv .intermediates/foo/android_common/hiddenapi/index.csv `, indexRule) From 82b3fcf12334a9d3bc257eecc948cc6fbc6ac473 Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Fri, 12 Feb 2021 15:42:46 +0000 Subject: [PATCH 2/3] Remove duplicates in monolithic hidden API files Previously, multiple APEX variants of some boot jars were being processed by hiddenapi to extract information which resulted in duplicate entries in the monolithic hidden API files. This change applies the same filter that was previously used to ensure that the hiddenapi-flags.csv file did not include any duplicates to all sources of hidden API information. Bug: 180102243 Test: m droid Verified that hiddenapi files (both aggregated ones and for the individual modules) are not affected by this change other than removing some duplicates entries. Change-Id: I9ffc8586d5d6efea4e3440be2dfd5424790665c8 --- java/hiddenapi.go | 13 +++++++++++-- java/hiddenapi_singleton.go | 17 ++++------------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/java/hiddenapi.go b/java/hiddenapi.go index 9caea944b..2d94ac491 100644 --- a/java/hiddenapi.go +++ b/java/hiddenapi.go @@ -124,7 +124,8 @@ func (h *hiddenAPI) initHiddenAPI(ctx android.BaseModuleContext, configurationNa // on the boot jars list because the runtime only enforces access to the hidden API for the // bootclassloader. If information is gathered for modules not on the list then that will cause // failures in the CtsHiddenApiBlocklist... tests. - h.active = inList(configurationName, ctx.Config().BootJars()) + module := ctx.Module() + h.active = isModuleInBootClassPath(ctx, module) if !h.active { // The rest of the properties will be ignored if active is false. return @@ -135,7 +136,6 @@ func (h *hiddenAPI) initHiddenAPI(ctx android.BaseModuleContext, configurationNa // A prebuilt module is only primary if it is preferred and conversely a source module is only // primary if it has not been replaced by a prebuilt module. - module := ctx.Module() if pi, ok := module.(android.PrebuiltInterface); ok { if p := pi.Prebuilt(); p != nil { primary = p.UsePrebuilt() @@ -153,6 +153,15 @@ func (h *hiddenAPI) initHiddenAPI(ctx android.BaseModuleContext, configurationNa h.primary = primary } +func isModuleInBootClassPath(ctx android.BaseModuleContext, module android.Module) bool { + // Get the configured non-updatable and updatable boot jars. + nonUpdatableBootJars := ctx.Config().NonUpdatableBootJars() + updatableBootJars := ctx.Config().UpdatableBootJars() + active := isModuleInConfiguredList(ctx, module, nonUpdatableBootJars) || + isModuleInConfiguredList(ctx, module, updatableBootJars) + return active +} + // hiddenAPIExtractAndEncode is called by any module that could contribute to the hiddenapi // processing. // diff --git a/java/hiddenapi_singleton.go b/java/hiddenapi_singleton.go index 6341a3406..25d39f346 100644 --- a/java/hiddenapi_singleton.go +++ b/java/hiddenapi_singleton.go @@ -217,10 +217,6 @@ func stubFlagsRule(ctx android.SingletonContext) { var bootDexJars android.Paths - // Get the configured non-updatable and updatable boot jars. - nonUpdatableBootJars := ctx.Config().NonUpdatableBootJars() - updatableBootJars := ctx.Config().UpdatableBootJars() - ctx.VisitAllModules(func(module android.Module) { // Collect dex jar paths for the modules listed above. if j, ok := module.(UsesLibraryDependency); ok { @@ -235,11 +231,6 @@ func stubFlagsRule(ctx android.SingletonContext) { // Collect dex jar paths for modules that had hiddenapi encode called on them. if h, ok := module.(hiddenAPIIntf); ok { if jar := h.bootDexJar(); jar != nil { - if !isModuleInConfiguredList(ctx, module, nonUpdatableBootJars) && - !isModuleInConfiguredList(ctx, module, updatableBootJars) { - return - } - bootDexJars = append(bootDexJars, jar) } } @@ -291,8 +282,8 @@ func stubFlagsRule(ctx android.SingletonContext) { // there too. // // TODO(b/179354495): Avoid having to perform this type of check or if necessary dedup it. -func isModuleInConfiguredList(ctx android.SingletonContext, module android.Module, configuredBootJars android.ConfiguredJarList) bool { - name := ctx.ModuleName(module) +func isModuleInConfiguredList(ctx android.BaseModuleContext, module android.Module, configuredBootJars android.ConfiguredJarList) bool { + name := ctx.OtherModuleName(module) // Strip a prebuilt_ prefix so that this can match a prebuilt module that has not been renamed. name = android.RemoveOptionalPrebuiltPrefix(name) @@ -305,11 +296,11 @@ func isModuleInConfiguredList(ctx android.SingletonContext, module android.Modul // It is an error if the module is not an ApexModule. if _, ok := module.(android.ApexModule); !ok { - ctx.Errorf("module %q configured in boot jars does not support being added to an apex", module) + ctx.ModuleErrorf("is configured in boot jars but does not support being added to an apex") return false } - apexInfo := ctx.ModuleProvider(module, android.ApexInfoProvider).(android.ApexInfo) + apexInfo := ctx.OtherModuleProvider(module, android.ApexInfoProvider).(android.ApexInfo) // Now match the apex part of the boot image configuration. requiredApex := configuredBootJars.Apex(index) From 2c36f240828eaad1904d231b8809e212aecd6708 Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Tue, 16 Feb 2021 16:57:06 +0000 Subject: [PATCH 3/3] Sort hiddenapi monolithic files by signature Adds a new --key_field option to merge_csv.py which specifies the name of the field that should be used to sort the input. If specified it causes that field to be the first in each row and performs the merge operation of a merge sort on the input files. That assumes that each input file is already sorted into the same order. Modifies the rules that use merge_csv.py to pass in: --key_field signature to sort the rows by signature. Bug: 180387396 Test: Verified that hiddenapi files (both aggregated ones and for the individual modules) are not affected by this change other than changing the order. Change-Id: Idcd5f0fea373b520b604889e1c280f21ed495660 --- java/hiddenapi.go | 1 + java/hiddenapi_singleton.go | 2 ++ scripts/hiddenapi/merge_csv.py | 34 ++++++++++++++++++++++++++++++---- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/java/hiddenapi.go b/java/hiddenapi.go index 2d94ac491..da2c48f0c 100644 --- a/java/hiddenapi.go +++ b/java/hiddenapi.go @@ -254,6 +254,7 @@ func (h *hiddenAPI) hiddenAPIExtractInformation(ctx android.ModuleContext, dexJa rule.Command(). BuiltTool("merge_csv"). Flag("--zip_input"). + Flag("--key_field signature"). FlagWithOutput("--output=", indexCSV). Inputs(classesJars) rule.Build("merged-hiddenapi-index", "Merged Hidden API index") diff --git a/java/hiddenapi_singleton.go b/java/hiddenapi_singleton.go index 25d39f346..82e8b3f75 100644 --- a/java/hiddenapi_singleton.go +++ b/java/hiddenapi_singleton.go @@ -424,6 +424,7 @@ func metadataRule(ctx android.SingletonContext) android.Path { rule.Command(). BuiltTool("merge_csv"). + Flag("--key_field signature"). FlagWithOutput("--output=", outputPath). Inputs(metadataCSV) @@ -535,6 +536,7 @@ func (h *hiddenAPIIndexSingleton) GenerateBuildActions(ctx android.SingletonCont rule := android.NewRuleBuilder(pctx, ctx) rule.Command(). BuiltTool("merge_csv"). + Flag("--key_field signature"). FlagWithArg("--header=", "signature,file,startline,startcol,endline,endcol,properties"). FlagWithOutput("--output=", hiddenAPISingletonPaths(ctx).index). Inputs(indexes) diff --git a/scripts/hiddenapi/merge_csv.py b/scripts/hiddenapi/merge_csv.py index 5ad61b2f6..b047aab71 100755 --- a/scripts/hiddenapi/merge_csv.py +++ b/scripts/hiddenapi/merge_csv.py @@ -20,6 +20,9 @@ Merge multiple CSV files, possibly with different columns. import argparse import csv import io +import heapq +import itertools +import operator from zipfile import ZipFile @@ -28,6 +31,10 @@ args_parser.add_argument('--header', help='Comma separated field names; ' 'if missing determines the header from input files.') args_parser.add_argument('--zip_input', help='Treat files as ZIP archives containing CSV files to merge.', action="store_true") +args_parser.add_argument('--key_field', help='The name of the field by which the rows should be sorted. ' + 'Must be in the field names. ' + 'Will be the first field in the output. ' + 'All input files must be sorted by that field.') args_parser.add_argument('--output', help='Output file for merged CSV.', default='-', type=argparse.FileType('w')) args_parser.add_argument('files', nargs=argparse.REMAINDER) @@ -57,10 +64,29 @@ else: headers = headers.union(reader.fieldnames) fieldnames = sorted(headers) -# Concatenate all files to output: +# By default chain the csv readers together so that the resulting output is +# the concatenation of the rows from each of them: +all_rows = itertools.chain.from_iterable(csv_readers) + +if len(csv_readers) > 0: + keyField = args.key_field + if keyField: + assert keyField in fieldnames, ( + "--key_field {} not found, must be one of {}\n").format( + keyField, ",".join(fieldnames)) + # Make the key field the first field in the output + keyFieldIndex = fieldnames.index(args.key_field) + fieldnames.insert(0, fieldnames.pop(keyFieldIndex)) + # Create an iterable that performs a lazy merge sort on the csv readers + # sorting the rows by the key field. + all_rows = heapq.merge(*csv_readers, key=operator.itemgetter(keyField)) + +# Write all rows from the input files to the output: writer = csv.DictWriter(args.output, delimiter=',', quotechar='|', quoting=csv.QUOTE_MINIMAL, dialect='unix', fieldnames=fieldnames) writer.writeheader() -for reader in csv_readers: - for row in reader: - writer.writerow(row) + +# Read all the rows from the input and write them to the output in the correct +# order: +for row in all_rows: + writer.writerow(row)