Merge "Allow implementation details to be excluded from signature patterns file"

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
-        pattern = pieces[0]
+        # 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,37 +13,99 @@
 # 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
 
-from signature_patterns import * #pylint: disable=unused-wildcard-import,wildcard-import
+from signature_patterns import *  #pylint: disable=unused-wildcard-import,wildcard-import
 
 
 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)