From 280bae6d204aa5720a4f42d265afd6173f1201b1 Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Tue, 20 Jul 2021 18:03:53 +0100 Subject: [PATCH] Filter blocked entries from modular flag files Previously, the sdk snapshot would include all the entries from the stub-flags.csv and all-flags.csv modular files generated by a single bootclasspath_fragment. That included implementation details, i.e. class members that are not part of a stable API or the hidden API which meant that the sdk snapshots were implementation dependent. This change removes the implementation details from the modular flag files, i.e. those entries that are only marked as "blocked". When comparing the files against the corresponding subset of the monolithic files it assumes that any entries missing from the modular flag files are blocked. Bug: 194063708 Test: atest --host verify_overlaps_test signature_patterns_test m out/soong/hiddenapi/hiddenapi-flags.csv - manually change files to cause difference in flags to check that it detects the differences. Change-Id: I6b67b2253cf029d6830b58a06ebb0c8fcaa0dd71 --- apex/platform_bootclasspath_test.go | 4 +-- java/hiddenapi_modular.go | 30 +++++++++++++++-- scripts/hiddenapi/verify_overlaps.py | 7 ++-- scripts/hiddenapi/verify_overlaps_test.py | 14 +++----- sdk/bootclasspath_fragment_sdk_test.go | 40 +++++++++++------------ 5 files changed, 59 insertions(+), 36 deletions(-) diff --git a/apex/platform_bootclasspath_test.go b/apex/platform_bootclasspath_test.go index 3066c790e..8cc837003 100644 --- a/apex/platform_bootclasspath_test.go +++ b/apex/platform_bootclasspath_test.go @@ -163,8 +163,8 @@ func TestPlatformBootclasspath_Fragments(t *testing.T) { android.AssertPathsRelativeToTopEquals(t, "metadata flags", []string{"out/soong/.intermediates/bar-fragment/android_common_apex10000/modular-hiddenapi/metadata.csv"}, info.MetadataPaths) android.AssertPathsRelativeToTopEquals(t, "index flags", []string{"out/soong/.intermediates/bar-fragment/android_common_apex10000/modular-hiddenapi/index.csv"}, info.IndexPaths) - android.AssertArrayString(t, "stub flags", []string{"out/soong/.intermediates/bar-fragment/android_common_apex10000/modular-hiddenapi/stub-flags.csv:out/soong/.intermediates/bar-fragment/android_common_apex10000/modular-hiddenapi/signature-patterns.csv"}, info.StubFlagSubsets.RelativeToTop()) - android.AssertArrayString(t, "all flags", []string{"out/soong/.intermediates/bar-fragment/android_common_apex10000/modular-hiddenapi/all-flags.csv:out/soong/.intermediates/bar-fragment/android_common_apex10000/modular-hiddenapi/signature-patterns.csv"}, info.FlagSubsets.RelativeToTop()) + android.AssertArrayString(t, "stub flags", []string{"out/soong/.intermediates/bar-fragment/android_common_apex10000/modular-hiddenapi/filtered-stub-flags.csv:out/soong/.intermediates/bar-fragment/android_common_apex10000/modular-hiddenapi/signature-patterns.csv"}, info.StubFlagSubsets.RelativeToTop()) + android.AssertArrayString(t, "all flags", []string{"out/soong/.intermediates/bar-fragment/android_common_apex10000/modular-hiddenapi/filtered-flags.csv:out/soong/.intermediates/bar-fragment/android_common_apex10000/modular-hiddenapi/signature-patterns.csv"}, info.FlagSubsets.RelativeToTop()) } func TestPlatformBootclasspathDependencies(t *testing.T) { diff --git a/java/hiddenapi_modular.go b/java/hiddenapi_modular.go index 8a06a9969..8e39f408e 100644 --- a/java/hiddenapi_modular.go +++ b/java/hiddenapi_modular.go @@ -948,6 +948,22 @@ func buildRuleSignaturePatternsFile(ctx android.ModuleContext, flagsPath android return patternsFile } +// buildRuleRemoveBlockedFlag creates a rule that will remove entries from the input path which +// only have blocked flags. It will not remove entries that have blocked as well as other flags, +// e.g. blocked,core-platform-api. +func buildRuleRemoveBlockedFlag(ctx android.BuilderContext, name string, desc string, inputPath android.Path, filteredPath android.WritablePath) { + rule := android.NewRuleBuilder(pctx, ctx) + rule.Command(). + Text(`grep -vE "^[^,]+,blocked$"`).Input(inputPath).Text(">").Output(filteredPath). + // Grep's exit code depends on whether it finds anything. It is 0 (build success) when it finds + // something and 1 (build failure) when it does not and 2 (when it encounters an error). + // However, while it is unlikely it is not an error if this does not find any matches. The + // following will only run if the grep does not find something and in that case it will treat + // an exit code of 1 as success and anything else as failure. + Text("|| test $? -eq 1") + rule.Build(name, desc) +} + // buildRuleValidateOverlappingCsvFiles checks that the modular CSV files, i.e. the files generated // by the individual bootclasspath_fragment modules are subsets of the monolithic CSV file. func buildRuleValidateOverlappingCsvFiles(ctx android.BuilderContext, name string, desc string, monolithicFilePath android.WritablePath, csvSubsets SignatureCsvSubsets) android.WritablePath { @@ -1036,14 +1052,24 @@ func hiddenAPIRulesForBootclasspathFragment(ctx android.ModuleContext, contents encodedBootDexJarsByModule[name] = encodedDex } + // Generate the filtered-stub-flags.csv file which contains the filtered stub flags that will be + // compared against the monolithic stub flags. + filteredStubFlagsCSV := android.PathForModuleOut(ctx, hiddenApiSubDir, "filtered-stub-flags.csv") + buildRuleRemoveBlockedFlag(ctx, "modularHiddenApiFilteredStubFlags", "modular hiddenapi filtered stub flags", stubFlagsCSV, filteredStubFlagsCSV) + + // Generate the filtered-flags.csv file which contains the filtered flags that will be compared + // against the monolithic flags. + filteredFlagsCSV := android.PathForModuleOut(ctx, hiddenApiSubDir, "filtered-flags.csv") + buildRuleRemoveBlockedFlag(ctx, "modularHiddenApiFilteredFlags", "modular hiddenapi filtered flags", allFlagsCSV, filteredFlagsCSV) + // Store the paths in the info for use by other modules and sdk snapshot generation. output := HiddenAPIOutput{ HiddenAPIFlagOutput: HiddenAPIFlagOutput{ - StubFlagsPath: stubFlagsCSV, + StubFlagsPath: filteredStubFlagsCSV, AnnotationFlagsPath: annotationFlagsCSV, MetadataPath: metadataCSV, IndexPath: indexCSV, - AllFlagsPath: allFlagsCSV, + AllFlagsPath: filteredFlagsCSV, }, EncodedBootDexFilesByModule: encodedBootDexJarsByModule, } diff --git a/scripts/hiddenapi/verify_overlaps.py b/scripts/hiddenapi/verify_overlaps.py index a4a423ed6..6432bf1a4 100755 --- a/scripts/hiddenapi/verify_overlaps.py +++ b/scripts/hiddenapi/verify_overlaps.py @@ -331,8 +331,11 @@ def compare_signature_flags(monolithicFlagsDict, modularFlagsDict): for signature in allSignatures: monolithicRow = monolithicFlagsDict.get(signature, {}) monolithicFlags = monolithicRow.get(None, []) - modularRow = modularFlagsDict.get(signature, {}) - modularFlags = modularRow.get(None, []) + if signature in modularFlagsDict: + modularRow = modularFlagsDict.get(signature, {}) + modularFlags = modularRow.get(None, []) + else: + modularFlags = ["blocked"] if monolithicFlags != modularFlags: mismatchingSignatures.append((signature, modularFlags, monolithicFlags)) return mismatchingSignatures diff --git a/scripts/hiddenapi/verify_overlaps_test.py b/scripts/hiddenapi/verify_overlaps_test.py index 747725458..00c0611d6 100755 --- a/scripts/hiddenapi/verify_overlaps_test.py +++ b/scripts/hiddenapi/verify_overlaps_test.py @@ -298,7 +298,7 @@ Ljava/lang/Object;->toString()Ljava/lang/String;,blocked ] self.assertEqual(expected, mismatches) - def test_missing_from_monolithic(self): + def test_match_treat_missing_from_modular_as_blocked(self): monolithic = self.read_signature_csv_from_string_as_dict('') modular = self.read_signature_csv_from_string_as_dict(''' Ljava/lang/Object;->toString()Ljava/lang/String;,public-api,system-api,test-api @@ -313,7 +313,7 @@ Ljava/lang/Object;->toString()Ljava/lang/String;,public-api,system-api,test-api ] self.assertEqual(expected, mismatches) - def test_missing_from_modular(self): + def test_mismatch_treat_missing_from_modular_as_blocked(self): monolithic = self.read_signature_csv_from_string_as_dict(''' Ljava/lang/Object;->hashCode()I,public-api,system-api,test-api ''') @@ -322,7 +322,7 @@ Ljava/lang/Object;->hashCode()I,public-api,system-api,test-api expected = [ ( 'Ljava/lang/Object;->hashCode()I', - [], + ['blocked'], ['public-api', 'system-api', 'test-api'], ), ] @@ -334,13 +334,7 @@ Ljava/lang/Object;->hashCode()I,blocked ''') modular = {} mismatches = compare_signature_flags(monolithic, modular) - expected = [ - ( - 'Ljava/lang/Object;->hashCode()I', - [], - ['blocked'], - ), - ] + expected = [] self.assertEqual(expected, mismatches) if __name__ == '__main__': diff --git a/sdk/bootclasspath_fragment_sdk_test.go b/sdk/bootclasspath_fragment_sdk_test.go index 916610957..618c735c4 100644 --- a/sdk/bootclasspath_fragment_sdk_test.go +++ b/sdk/bootclasspath_fragment_sdk_test.go @@ -138,8 +138,8 @@ prebuilt_bootclasspath_fragment { metadata: "hiddenapi/metadata.csv", index: "hiddenapi/index.csv", signature_patterns: "hiddenapi/signature-patterns.csv", - stub_flags: "hiddenapi/stub-flags.csv", - all_flags: "hiddenapi/all-flags.csv", + stub_flags: "hiddenapi/filtered-stub-flags.csv", + all_flags: "hiddenapi/filtered-flags.csv", }, } @@ -166,8 +166,8 @@ prebuilt_bootclasspath_fragment { metadata: "hiddenapi/metadata.csv", index: "hiddenapi/index.csv", signature_patterns: "hiddenapi/signature-patterns.csv", - stub_flags: "hiddenapi/stub-flags.csv", - all_flags: "hiddenapi/all-flags.csv", + stub_flags: "hiddenapi/filtered-stub-flags.csv", + all_flags: "hiddenapi/filtered-flags.csv", }, } @@ -191,8 +191,8 @@ sdk_snapshot { .intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/metadata.csv -> hiddenapi/metadata.csv .intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/index.csv -> hiddenapi/index.csv .intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/signature-patterns.csv -> hiddenapi/signature-patterns.csv -.intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/stub-flags.csv -> hiddenapi/stub-flags.csv -.intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/all-flags.csv -> hiddenapi/all-flags.csv +.intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/filtered-stub-flags.csv -> hiddenapi/filtered-stub-flags.csv +.intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/filtered-flags.csv -> hiddenapi/filtered-flags.csv .intermediates/mysdk/common_os/empty -> java_boot_libs/snapshot/jars/are/invalid/mybootlib.jar `), snapshotTestPreparer(checkSnapshotWithoutSource, preparerForSnapshot), @@ -339,8 +339,8 @@ prebuilt_bootclasspath_fragment { metadata: "hiddenapi/metadata.csv", index: "hiddenapi/index.csv", signature_patterns: "hiddenapi/signature-patterns.csv", - stub_flags: "hiddenapi/stub-flags.csv", - all_flags: "hiddenapi/all-flags.csv", + stub_flags: "hiddenapi/filtered-stub-flags.csv", + all_flags: "hiddenapi/filtered-flags.csv", }, } @@ -424,8 +424,8 @@ prebuilt_bootclasspath_fragment { metadata: "hiddenapi/metadata.csv", index: "hiddenapi/index.csv", signature_patterns: "hiddenapi/signature-patterns.csv", - stub_flags: "hiddenapi/stub-flags.csv", - all_flags: "hiddenapi/all-flags.csv", + stub_flags: "hiddenapi/filtered-stub-flags.csv", + all_flags: "hiddenapi/filtered-flags.csv", }, } @@ -503,8 +503,8 @@ sdk_snapshot { .intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/metadata.csv -> hiddenapi/metadata.csv .intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/index.csv -> hiddenapi/index.csv .intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/signature-patterns.csv -> hiddenapi/signature-patterns.csv -.intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/stub-flags.csv -> hiddenapi/stub-flags.csv -.intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/all-flags.csv -> hiddenapi/all-flags.csv +.intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/filtered-stub-flags.csv -> hiddenapi/filtered-stub-flags.csv +.intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/filtered-flags.csv -> hiddenapi/filtered-flags.csv .intermediates/mysdk/common_os/empty -> java_boot_libs/snapshot/jars/are/invalid/mybootlib.jar .intermediates/myothersdklibrary.stubs/android_common/javac/myothersdklibrary.stubs.jar -> sdk_library/public/myothersdklibrary-stubs.jar .intermediates/myothersdklibrary.stubs.source/android_common/metalava/myothersdklibrary.stubs.source_api.txt -> sdk_library/public/myothersdklibrary.txt @@ -546,13 +546,13 @@ sdk_snapshot { android.AssertStringEquals(t, "updatable-bcp-packages.txt", expectedContents, rule.Args["content"]) rule = module.Output("out/soong/hiddenapi/hiddenapi-flags.csv.valid") - android.AssertStringDoesContain(t, "verify-overlaps", rule.RuleParams.Command, " snapshot/hiddenapi/all-flags.csv:snapshot/hiddenapi/signature-patterns.csv ") + android.AssertStringDoesContain(t, "verify-overlaps", rule.RuleParams.Command, " snapshot/hiddenapi/filtered-flags.csv:snapshot/hiddenapi/signature-patterns.csv ") }), snapshotTestPreparer(checkSnapshotWithSourcePreferred, preparerForSnapshot), snapshotTestChecker(checkSnapshotWithSourcePreferred, func(t *testing.T, result *android.TestResult) { module := result.ModuleForTests("platform-bootclasspath", "android_common") rule := module.Output("out/soong/hiddenapi/hiddenapi-flags.csv.valid") - android.AssertStringDoesContain(t, "verify-overlaps", rule.RuleParams.Command, " out/soong/.intermediates/mybootclasspathfragment/android_common_myapex/modular-hiddenapi/all-flags.csv:out/soong/.intermediates/mybootclasspathfragment/android_common_myapex/modular-hiddenapi/signature-patterns.csv ") + android.AssertStringDoesContain(t, "verify-overlaps", rule.RuleParams.Command, " out/soong/.intermediates/mybootclasspathfragment/android_common_myapex/modular-hiddenapi/filtered-flags.csv:out/soong/.intermediates/mybootclasspathfragment/android_common_myapex/modular-hiddenapi/signature-patterns.csv ") }), snapshotTestPreparer(checkSnapshotPreferredWithSource, preparerForSnapshot), ) @@ -654,8 +654,8 @@ prebuilt_bootclasspath_fragment { metadata: "hiddenapi/metadata.csv", index: "hiddenapi/index.csv", signature_patterns: "hiddenapi/signature-patterns.csv", - stub_flags: "hiddenapi/stub-flags.csv", - all_flags: "hiddenapi/all-flags.csv", + stub_flags: "hiddenapi/filtered-stub-flags.csv", + all_flags: "hiddenapi/filtered-flags.csv", }, } @@ -857,8 +857,8 @@ prebuilt_bootclasspath_fragment { metadata: "hiddenapi/metadata.csv", index: "hiddenapi/index.csv", signature_patterns: "hiddenapi/signature-patterns.csv", - stub_flags: "hiddenapi/stub-flags.csv", - all_flags: "hiddenapi/all-flags.csv", + stub_flags: "hiddenapi/filtered-stub-flags.csv", + all_flags: "hiddenapi/filtered-flags.csv", }, } @@ -901,8 +901,8 @@ my-unsupported-packages.txt -> hiddenapi/my-unsupported-packages.txt .intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/metadata.csv -> hiddenapi/metadata.csv .intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/index.csv -> hiddenapi/index.csv .intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/signature-patterns.csv -> hiddenapi/signature-patterns.csv -.intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/stub-flags.csv -> hiddenapi/stub-flags.csv -.intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/all-flags.csv -> hiddenapi/all-flags.csv +.intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/filtered-stub-flags.csv -> hiddenapi/filtered-stub-flags.csv +.intermediates/mybootclasspathfragment/android_common/modular-hiddenapi/filtered-flags.csv -> hiddenapi/filtered-flags.csv .intermediates/mysdk/common_os/empty -> java_boot_libs/snapshot/jars/are/invalid/mybootlib.jar .intermediates/mysdklibrary.stubs/android_common/javac/mysdklibrary.stubs.jar -> sdk_library/public/mysdklibrary-stubs.jar .intermediates/mysdklibrary.stubs.source/android_common/metalava/mysdklibrary.stubs.source_api.txt -> sdk_library/public/mysdklibrary.txt