diff --git a/java/bootclasspath_fragment.go b/java/bootclasspath_fragment.go index ced004850..8f18790ea 100644 --- a/java/bootclasspath_fragment.go +++ b/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. diff --git a/java/hiddenapi_modular.go b/java/hiddenapi_modular.go index f1e30f320..0cc960d5c 100644 --- a/java/hiddenapi_modular.go +++ b/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") diff --git a/scripts/hiddenapi/signature_patterns.py b/scripts/hiddenapi/signature_patterns.py index 0acb2a004..e75ee9566 100755 --- a/scripts/hiddenapi/signature_patterns.py +++ b/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 -bootclasspath_fragment that can be used to select a subset of monolithic flags +"""Generate a set of signature patterns for a bootclasspath_fragment. + +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): - # Read in all the signatures into a list and remove member names. +def produce_patterns_from_stream(stream, + 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 - # from the containing outer class. - pieces = qualifiedClassName.split("$", maxsplit=1) - pattern = pieces[0] + pieces = qualifiedClassName.rsplit('/', maxsplit=1) + pkg = pieces[0] + # 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:]) diff --git a/scripts/hiddenapi/signature_patterns_test.py b/scripts/hiddenapi/signature_patterns_test.py index 3babe5420..b59dfd71b 100755 --- a/scripts/hiddenapi/signature_patterns_test.py +++ b/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): - #pylint: disable=line-too-long - patterns = self.produce_patterns_from_string( - ''' + csvFlags = """ Ljava/lang/ProcessBuilder$Redirect$1;->()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/Object", - "java/lang/ProcessBuilder", + 'java/lang/*', ] 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)