Allow implementation details to be excluded from signature patterns file

Previously, the signature-patterns.csv file (which was output in an sdk
snapshot) included implementation details, e.g. the names of classes
that are not part of any API, including the hidden API. This change
allows a bootclasspath_fragment module owner to customize the generated
signature patterns file to remove as many implementation details as
possible from the sdk snapshot. That means that implementation only
changes would not require that sdk snapshots be updated in sync with
the corresponding APEX.

Module owners can remove implementation in the following ways:
1) Limit the number of packages that are treated as split packages by
   explicitly specifying the split packages in the split_packages
   property. A split package is one whose classes are provided by
   separate bootclasspath_fragment modules and so the signature
   patterns has to include every class in that package provided by a
   specific bootclasspath_fragment module, including implementation
   classes, instead of just listing the package.

   The default is to treat all packages as being split, so if no
   split_packages is specified then it defaults to ["*"] which matches
   all packages. Assuming that no package was split unless specifically
   stated would require that all the modules that had split packages
   explicitly list them before this change could be submitted as
   without them this change would break the build.

   Once all existing modules with split packages have been updated to
   include them then the default may be changed.

2) Adding package prefixes for any hierarchy of packages that are
   owned solely by that bootclasspath_fragment. This removes the need
   to list the packages in that hierarchy, including implementation
   specific packages.

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: If2f90cfc41b6fff9fa4ac8b9d4973110938b9392

java/bootclasspath_fragment.go

@@ -139,6 +139,74 @@ type bootclasspathFragmentProperties struct {
 	BootclasspathFragmentsDepsProperties
 }
 
+type SourceOnlyBootclasspathProperties struct {
+	Hidden_api struct {
+		// Contains prefixes of a package hierarchy that is provided solely by this
+		// bootclasspath_fragment.
+		//
+		// This affects the signature patterns file that is used to select the subset of monolithic
+		// hidden API flags. See split_packages property for more details.
+		Package_prefixes []string
+
+		// The list of split packages provided by this bootclasspath_fragment.
+		//
+		// A split package is one that contains classes which are provided by multiple
+		// bootclasspath_fragment modules.
+		//
+		// This defaults to "*" - which treats all packages as being split. A module that has no split
+		// packages must specify an empty list.
+		//
+		// This affects the signature patterns file that is generated by a bootclasspath_fragment and
+		// used to select the subset of monolithic hidden API flags against which the flags generated
+		// by the bootclasspath_fragment are compared.
+		//
+		// The signature patterns file selects the subset of monolithic hidden API flags using a number
+		// of patterns, i.e.:
+		// * The qualified name (including package) of an outermost class, e.g. java/lang/Character.
+		//   This selects all the flags for all the members of this class and any nested classes.
+		// * A package wildcard, e.g. java/lang/*. This selects all the flags for all the members of all
+		//   the classes in this package (but not in sub-packages).
+		// * A recursive package wildcard, e.g. java/**. This selects all the flags for all the members
+		//   of all the classes in this package and sub-packages.
+		//
+		// The signature patterns file is constructed as follows:
+		// * All the signatures are retrieved from the all-flags.csv file.
+		// * The member and inner class names are removed.
+		// * If a class is in a split package then that is kept, otherwise the class part is removed
+		//   and replaced with a wildcard, i.e. *.
+		// * If a package matches a package prefix then the package is removed.
+		// * All the package prefixes are added with a recursive wildcard appended to each, i.e. **.
+		// * The resulting patterns are sorted.
+		//
+		// So, by default (i.e. without specifying any package_prefixes or split_packages) the signature
+		// patterns is a list of class names, because there are no package packages and all packages are
+		// assumed to be split.
+		//
+		// If any split packages are specified then only those packages are treated as split and all
+		// other packages are treated as belonging solely to the bootclasspath_fragment and so they use
+		// wildcard package patterns.
+		//
+		// So, if an empty list of split packages is specified then the signature patterns file just
+		// includes a wildcard package pattern for every package provided by the bootclasspath_fragment.
+		//
+		// If split_packages are specified and a package that is split is not listed then it could lead
+		// to build failures as it will select monolithic flags that are generated by another
+		// bootclasspath_fragment to compare against the flags provided by this fragment. The latter
+		// will obviously not contain those flags and that can cause the comparison and build to fail.
+		//
+		// If any package prefixes are specified then any matching packages are removed from the
+		// signature patterns and replaced with a single recursive package pattern.
+		//
+		// It is not strictly necessary to specify either package_prefixes or split_packages as the
+		// defaults will produce a valid set of signature patterns. However, those patterns may include
+		// implementation details, e.g. names of implementation classes or packages, which will be
+		// exported to the sdk snapshot in the signature patterns file. That is something that should be
+		// avoided where possible. Specifying package_prefixes and split_packages allows those
+		// implementation details to be excluded from the snapshot.
+		Split_packages []string
+	}
+}
+
 type BootclasspathFragmentModule struct {
 	android.ModuleBase
 	android.ApexModuleBase
@@ -147,6 +215,8 @@ type BootclasspathFragmentModule struct {
 
 	properties bootclasspathFragmentProperties
 
+	sourceOnlyProperties SourceOnlyBootclasspathProperties
+
 	// Collect the module directory for IDE info in java/jdeps.go.
 	modulePaths []string
 }
@@ -180,7 +250,7 @@ type bootImageFilesByArch map[android.ArchType]android.Paths
 
 func bootclasspathFragmentFactory() android.Module {
 	m := &BootclasspathFragmentModule{}
-	m.AddProperties(&m.properties)
+	m.AddProperties(&m.properties, &m.sourceOnlyProperties)
 	android.InitApexModule(m)
 	android.InitSdkAwareModule(m)
 	initClasspathFragment(m, BOOTCLASSPATH)
@@ -590,7 +660,7 @@ func (b *BootclasspathFragmentModule) generateHiddenAPIBuildActions(ctx android.
 	// TODO(b/192868581): Remove once the source and prebuilts provide a signature patterns file of
 	//  their own.
 	if output.SignaturePatternsPath == nil {
-		output.SignaturePatternsPath = buildRuleSignaturePatternsFile(ctx, output.AllFlagsPath)
+		output.SignaturePatternsPath = buildRuleSignaturePatternsFile(ctx, output.AllFlagsPath, []string{"*"}, nil)
 	}
 
 	// Initialize a HiddenAPIInfo structure.
@@ -659,7 +729,20 @@ func (b *BootclasspathFragmentModule) createHiddenAPIFlagInput(ctx android.Modul
 func (b *BootclasspathFragmentModule) produceHiddenAPIOutput(ctx android.ModuleContext, contents []android.Module, input HiddenAPIFlagInput) *HiddenAPIOutput {
 	// Generate the rules to create the hidden API flags and update the supplied hiddenAPIInfo with the
 	// paths to the created files.
-	return hiddenAPIRulesForBootclasspathFragment(ctx, contents, input)
+	output := hiddenAPIRulesForBootclasspathFragment(ctx, contents, input)
+
+	// If the module specifies split_packages or package_prefixes then use those to generate the
+	// signature patterns.
+	splitPackages := b.sourceOnlyProperties.Hidden_api.Split_packages
+	packagePrefixes := b.sourceOnlyProperties.Hidden_api.Package_prefixes
+	if splitPackages != nil || packagePrefixes != nil {
+		if splitPackages == nil {
+			splitPackages = []string{"*"}
+		}
+		output.SignaturePatternsPath = buildRuleSignaturePatternsFile(ctx, output.AllFlagsPath, splitPackages, packagePrefixes)
+	}
+
+	return output
 }
 
 // produceBootImageFiles builds the boot image files from the source if it is required.

java/hiddenapi_modular.go

@@ -943,13 +943,22 @@ func (s SignatureCsvSubsets) RelativeToTop() []string {
 
 // buildRuleSignaturePatternsFile creates a rule to generate a file containing the set of signature
 // patterns that will select a subset of the monolithic flags.
-func buildRuleSignaturePatternsFile(ctx android.ModuleContext, flagsPath android.Path) android.Path {
+func buildRuleSignaturePatternsFile(ctx android.ModuleContext, flagsPath android.Path, splitPackages []string, packagePrefixes []string) android.Path {
 	patternsFile := android.PathForModuleOut(ctx, "modular-hiddenapi", "signature-patterns.csv")
 	// Create a rule to validate the output from the following rule.
 	rule := android.NewRuleBuilder(pctx, ctx)
+
+	// Quote any * in the packages to avoid them being expanded by the shell.
+	quotedSplitPackages := []string{}
+	for _, pkg := range splitPackages {
+		quotedSplitPackages = append(quotedSplitPackages, strings.ReplaceAll(pkg, "*", "\\*"))
+	}
+
 	rule.Command().
 		BuiltTool("signature_patterns").
 		FlagWithInput("--flags ", flagsPath).
+		FlagForEachArg("--split-package ", quotedSplitPackages).
+		FlagForEachArg("--package-prefix ", packagePrefixes).
 		FlagWithOutput("--output ", patternsFile)
 	rule.Build("hiddenAPISignaturePatterns", "hidden API signature patterns")
 

scripts/hiddenapi/signature_patterns.py

@@ -13,8 +13,10 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
-"""Generate a set of signature patterns from the modular flags generated by a
+"""Generate a set of signature patterns for a bootclasspath_fragment.
-bootclasspath_fragment that can be used to select a subset of monolithic flags
+
+The patterns are generated from the modular flags produced by the
+bootclasspath_fragment and are used to select a subset of the monolithic flags
 against which the modular flags can be compared.
 """
 
@@ -22,33 +24,122 @@ import argparse
 import csv
 import sys
 
+
 def dict_reader(csvfile):
     return csv.DictReader(
-        csvfile, delimiter=',', quotechar='|', fieldnames=['signature']
+        csvfile, delimiter=',', quotechar='|', fieldnames=['signature'])
-    )
 
 
-def produce_patterns_from_file(file):
+def dotPackageToSlashPackage(pkg):
+    return pkg.replace('.', '/')
+
+
+def slashPackageToDotPackage(pkg):
+    return pkg.replace('/', '.')
+
+
+def isSplitPackage(splitPackages, pkg):
+    return splitPackages and (pkg in splitPackages or '*' in splitPackages)
+
+
+def matchedByPackagePrefixPattern(packagePrefixes, prefix):
+    for packagePrefix in packagePrefixes:
+        if prefix == packagePrefix:
+            return packagePrefix
+        elif prefix.startswith(packagePrefix) and prefix[len(
+                packagePrefix)] == '/':
+            return packagePrefix
+    return False
+
+
+def validate_package_prefixes(splitPackages, packagePrefixes):
+    # If there are no package prefixes then there is no possible conflict
+    # between them and the split packages.
+    if len(packagePrefixes) == 0:
+        return
+
+    # Check to make sure that the split packages and package prefixes do not
+    # overlap.
+    errors = []
+    for splitPackage in splitPackages:
+        if splitPackage == '*':
+            # A package prefix matches a split package.
+            packagePrefixesForOutput = ', '.join(
+                map(slashPackageToDotPackage, packagePrefixes))
+            errors.append(
+                'split package "*" conflicts with all package prefixes %s\n'
+                '    add split_packages:[] to fix' % packagePrefixesForOutput)
+        else:
+            packagePrefix = matchedByPackagePrefixPattern(
+                packagePrefixes, splitPackage)
+            if packagePrefix:
+                # A package prefix matches a split package.
+                splitPackageForOutput = slashPackageToDotPackage(splitPackage)
+                packagePrefixForOutput = slashPackageToDotPackage(packagePrefix)
+                errors.append(
+                    'split package %s is matched by package prefix %s' %
+                    (splitPackageForOutput, packagePrefixForOutput))
+    return errors
+
+
+def validate_split_packages(splitPackages):
+    errors = []
+    if '*' in splitPackages and len(splitPackages) > 1:
+        errors.append('split packages are invalid as they contain both the'
+                      ' wildcard (*) and specific packages, use the wildcard or'
+                      ' specific packages, not a mixture')
+    return errors
+
+
+def produce_patterns_from_file(file, splitPackages=None, packagePrefixes=None):
     with open(file, 'r') as f:
-        return produce_patterns_from_stream(f)
+        return produce_patterns_from_stream(f, splitPackages, packagePrefixes)
 
 
-def produce_patterns_from_stream(stream):
+def produce_patterns_from_stream(stream,
-    # Read in all the signatures into a list and remove member names.
+                                 splitPackages=None,
+                                 packagePrefixes=None):
+    splitPackages = set(splitPackages or [])
+    packagePrefixes = list(packagePrefixes or [])
+    # Read in all the signatures into a list and remove any unnecessary class
+    # and member names.
     patterns = set()
     for row in dict_reader(stream):
         signature = row['signature']
-        text = signature.removeprefix("L")
+        text = signature.removeprefix('L')
         # Remove the class specific member signature
-        pieces = text.split(";->")
+        pieces = text.split(';->')
         qualifiedClassName = pieces[0]
-        # Remove inner class names as they cannot be separated
+        pieces = qualifiedClassName.rsplit('/', maxsplit=1)
-        # from the containing outer class.
+        pkg = pieces[0]
-        pieces = qualifiedClassName.split("$", maxsplit=1)
+        # If the package is split across multiple modules then it cannot be used
+        # to select the subset of the monolithic flags that this module
+        # produces. In that case we need to keep the name of the class but can
+        # discard any nested class names as an outer class cannot be split
+        # across modules.
+        #
+        # If the package is not split then every class in the package must be
+        # provided by this module so there is no need to list the classes
+        # explicitly so just use the package name instead.
+        if isSplitPackage(splitPackages, pkg):
+            # Remove inner class names.
+            pieces = qualifiedClassName.split('$', maxsplit=1)
             pattern = pieces[0]
+        else:
+            # Add a * to ensure that the pattern matches the classes in that
+            # package.
+            pattern = pkg + '/*'
         patterns.add(pattern)
 
-    patterns = list(patterns) #pylint: disable=redefined-variable-type
+    # Remove any patterns that would be matched by a package prefix pattern.
+    patterns = list(
+        filter(lambda p: not matchedByPackagePrefixPattern(packagePrefixes, p),
+               patterns))
+    # Add the package prefix patterns to the list. Add a ** to ensure that each
+    # package prefix pattern will match the classes in that package and all
+    # sub-packages.
+    patterns = patterns + list(map(lambda x: x + '/**', packagePrefixes))
+    # Sort the patterns.
     patterns.sort()
     return patterns
 
@@ -56,24 +147,47 @@ def produce_patterns_from_stream(stream):
 def main(args):
     args_parser = argparse.ArgumentParser(
         description='Generate a set of signature patterns '
-        'that select a subset of monolithic hidden API files.'
+        'that select a subset of monolithic hidden API files.')
-    )
     args_parser.add_argument(
         '--flags',
         help='The stub flags file which contains an entry for every dex member',
     )
+    args_parser.add_argument(
+        '--split-package',
+        action='append',
+        help='A package that is split across multiple bootclasspath_fragment modules'
+    )
+    args_parser.add_argument(
+        '--package-prefix',
+        action='append',
+        help='A package prefix unique to this set of flags')
     args_parser.add_argument('--output', help='Generated signature prefixes')
     args = args_parser.parse_args(args)
 
+    splitPackages = set(map(dotPackageToSlashPackage, args.split_package or []))
+    errors = validate_split_packages(splitPackages)
+
+    packagePrefixes = list(
+        map(dotPackageToSlashPackage, args.package_prefix or []))
+
+    if not errors:
+        errors = validate_package_prefixes(splitPackages, packagePrefixes)
+
+    if errors:
+        for error in errors:
+            print(error)
+        sys.exit(1)
+
     # Read in all the patterns into a list.
-    patterns = produce_patterns_from_file(args.flags)
+    patterns = produce_patterns_from_file(args.flags, splitPackages,
+                                          packagePrefixes)
 
     # Write out all the patterns.
     with open(args.output, 'w') as outputFile:
         for pattern in patterns:
             outputFile.write(pattern)
-            outputFile.write("\n")
+            outputFile.write('\n')
 
 
-if __name__ == "__main__":
+if __name__ == '__main__':
     main(sys.argv[1:])

scripts/hiddenapi/signature_patterns_test.py

@@ -13,7 +13,6 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
-
 """Unit tests for signature_patterns.py."""
 import io
 import unittest
@@ -22,28 +21,91 @@ from signature_patterns import * #pylint: disable=unused-wildcard-import,wildcar
 
 
 class TestGeneratedPatterns(unittest.TestCase):
-    def produce_patterns_from_string(self, csvdata):
-        with io.StringIO(csvdata) as f:
-            return produce_patterns_from_stream(f)
 
-    def test_generate(self):
+    csvFlags = """
-        #pylint: disable=line-too-long
-        patterns = self.produce_patterns_from_string(
-            '''
 Ljava/lang/ProcessBuilder$Redirect$1;-><init>()V,blocked
 Ljava/lang/Character$UnicodeScript;->of(I)Ljava/lang/Character$UnicodeScript;,public-api
 Ljava/lang/Object;->hashCode()I,public-api,system-api,test-api
 Ljava/lang/Object;->toString()Ljava/lang/String;,blocked
-'''
+"""
-        )
+
-        #pylint: enable=line-too-long
+    def produce_patterns_from_string(self,
+                                     csv,
+                                     splitPackages=None,
+                                     packagePrefixes=None):
+        with io.StringIO(csv) as f:
+            return produce_patterns_from_stream(f, splitPackages,
+                                                packagePrefixes)
+
+    def test_generate_default(self):
+        patterns = self.produce_patterns_from_string(
+            TestGeneratedPatterns.csvFlags)
         expected = [
-            "java/lang/Character",
+            'java/lang/*',
-            "java/lang/Object",
-            "java/lang/ProcessBuilder",
         ]
         self.assertEqual(expected, patterns)
 
+    def test_generate_split_package(self):
+        patterns = self.produce_patterns_from_string(
+            TestGeneratedPatterns.csvFlags, splitPackages={'java/lang'})
+        expected = [
+            'java/lang/Character',
+            'java/lang/Object',
+            'java/lang/ProcessBuilder',
+        ]
+        self.assertEqual(expected, patterns)
+
+    def test_generate_split_package_wildcard(self):
+        patterns = self.produce_patterns_from_string(
+            TestGeneratedPatterns.csvFlags, splitPackages={'*'})
+        expected = [
+            'java/lang/Character',
+            'java/lang/Object',
+            'java/lang/ProcessBuilder',
+        ]
+        self.assertEqual(expected, patterns)
+
+    def test_generate_package_prefix(self):
+        patterns = self.produce_patterns_from_string(
+            TestGeneratedPatterns.csvFlags, packagePrefixes={'java/lang'})
+        expected = [
+            'java/lang/**',
+        ]
+        self.assertEqual(expected, patterns)
+
+    def test_generate_package_prefix_top_package(self):
+        patterns = self.produce_patterns_from_string(
+            TestGeneratedPatterns.csvFlags, packagePrefixes={'java'})
+        expected = [
+            'java/**',
+        ]
+        self.assertEqual(expected, patterns)
+
+    def test_split_package_wildcard_conflicts_with_other_split_packages(self):
+        errors = validate_split_packages({'*', 'java'})
+        expected = [
+            'split packages are invalid as they contain both the wildcard (*)'
+            ' and specific packages, use the wildcard or specific packages,'
+            ' not a mixture'
+        ]
+        self.assertEqual(expected, errors)
+
+    def test_split_package_wildcard_conflicts_with_package_prefixes(self):
+        errors = validate_package_prefixes({'*'}, packagePrefixes={'java'})
+        expected = [
+            'split package "*" conflicts with all package prefixes java\n'
+            '    add split_packages:[] to fix',
+        ]
+        self.assertEqual(expected, errors)
+
+    def test_split_package_conflict(self):
+        errors = validate_package_prefixes({'java/split'},
+                                           packagePrefixes={'java'})
+        expected = [
+            'split package java.split is matched by package prefix java',
+        ]
+        self.assertEqual(expected, errors)
+
 
 if __name__ == '__main__':
     unittest.main(verbosity=2)