Merge "Make bootclasspath_fragment hidden API package checks exhaustive" am: 2cb89166df

Original change: https://android-review.googlesource.com/c/platform/build/soong/+/2028212

Change-Id: I0198164d0e4432570067f6c3f209627706710f2e

java/bootclasspath_fragment.go

@@ -140,7 +140,7 @@ type bootclasspathFragmentProperties struct {
 	BootclasspathFragmentsDepsProperties
 }
 
-type SourceOnlyBootclasspathProperties struct {
+type HiddenApiPackageProperties struct {
 	Hidden_api struct {
 		// Contains prefixes of a package hierarchy that is provided solely by this
 		// bootclasspath_fragment.
@@ -149,6 +149,14 @@ type SourceOnlyBootclasspathProperties struct {
 		// hidden API flags. See split_packages property for more details.
 		Package_prefixes []string
 
+		// A list of individual packages that are provided solely by this
+		// bootclasspath_fragment but which cannot be listed in package_prefixes
+		// because there are sub-packages which are provided by other modules.
+		//
+		// This should only be used for legacy packages. New packages should be
+		// covered by a package prefix.
+		Single_packages []string
+
 		// The list of split packages provided by this bootclasspath_fragment.
 		//
 		// A split package is one that contains classes which are provided by multiple
@@ -208,6 +216,11 @@ type SourceOnlyBootclasspathProperties struct {
 	}
 }
 
+type SourceOnlyBootclasspathProperties struct {
+	HiddenApiPackageProperties
+	Coverage HiddenApiPackageProperties
+}
+
 type BootclasspathFragmentModule struct {
 	android.ModuleBase
 	android.ApexModuleBase
@@ -271,6 +284,12 @@ func bootclasspathFragmentFactory() android.Module {
 				ctx.PropertyErrorf("coverage", "error trying to append coverage specific properties: %s", err)
 				return
 			}
+
+			err = proptools.AppendProperties(&m.sourceOnlyProperties.HiddenApiPackageProperties, &m.sourceOnlyProperties.Coverage, nil)
+			if err != nil {
+				ctx.PropertyErrorf("coverage", "error trying to append hidden api coverage specific properties: %s", err)
+				return
+			}
 		}
 
 		// Initialize the contents property from the image_name.
@@ -731,7 +750,8 @@ 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, []string{"*"}, nil)
+		output.SignaturePatternsPath = buildRuleSignaturePatternsFile(
+			ctx, output.AllFlagsPath, []string{"*"}, nil, nil)
 	}
 
 	// Initialize a HiddenAPIInfo structure.
@@ -806,11 +826,13 @@ func (b *BootclasspathFragmentModule) produceHiddenAPIOutput(ctx android.ModuleC
 	// signature patterns.
 	splitPackages := b.sourceOnlyProperties.Hidden_api.Split_packages
 	packagePrefixes := b.sourceOnlyProperties.Hidden_api.Package_prefixes
-	if splitPackages != nil || packagePrefixes != nil {
+	singlePackages := b.sourceOnlyProperties.Hidden_api.Single_packages
+	if splitPackages != nil || packagePrefixes != nil || singlePackages != nil {
 		if splitPackages == nil {
 			splitPackages = []string{"*"}
 		}
-		output.SignaturePatternsPath = buildRuleSignaturePatternsFile(ctx, output.AllFlagsPath, splitPackages, packagePrefixes)
+		output.SignaturePatternsPath = buildRuleSignaturePatternsFile(
+			ctx, output.AllFlagsPath, splitPackages, packagePrefixes, singlePackages)
 	}
 
 	return output

java/hiddenapi_modular.go

@@ -943,7 +943,9 @@ 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, splitPackages []string, packagePrefixes []string) android.Path {
+func buildRuleSignaturePatternsFile(
+	ctx android.ModuleContext, flagsPath android.Path,
+	splitPackages []string, packagePrefixes []string, singlePackages []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)
@@ -959,6 +961,7 @@ func buildRuleSignaturePatternsFile(ctx android.ModuleContext, flagsPath android
 		FlagWithInput("--flags ", flagsPath).
 		FlagForEachArg("--split-package ", quotedSplitPackages).
 		FlagForEachArg("--package-prefix ", packagePrefixes).
+		FlagForEachArg("--single-package ", singlePackages).
 		FlagWithOutput("--output ", patternsFile)
 	rule.Build("hiddenAPISignaturePatterns", "hidden API signature patterns")
 

scripts/hiddenapi/signature_patterns.py

@@ -60,7 +60,22 @@ def matched_by_package_prefix_pattern(package_prefixes, prefix):
     return False
 
 
-def validate_package_prefixes(split_packages, package_prefixes):
+def validate_package_is_not_matched_by_package_prefix(package_type, pkg,
+                                                      package_prefixes):
+    package_prefix = matched_by_package_prefix_pattern(package_prefixes, pkg)
+    if package_prefix:
+        # A package prefix matches the package.
+        package_for_output = slash_package_to_dot_package(pkg)
+        package_prefix_for_output = slash_package_to_dot_package(package_prefix)
+        return [
+            f'{package_type} {package_for_output} is matched by '
+            f'package prefix {package_prefix_for_output}'
+        ]
+    return []
+
+
+def validate_package_prefixes(split_packages, single_packages,
+                              package_prefixes):
     # If there are no package prefixes then there is no possible conflict
     # between them and the split packages.
     if len(package_prefixes) == 0:
@@ -79,17 +94,16 @@ def validate_package_prefixes(split_packages, package_prefixes):
                 f'{package_prefixes_for_output}\n'
                 '    add split_packages:[] to fix')
         else:
-            package_prefix = matched_by_package_prefix_pattern(
+            errs = validate_package_is_not_matched_by_package_prefix(
-                package_prefixes, split_package)
+                'split package', split_package, package_prefixes)
-            if package_prefix:
+            errors.extend(errs)
-                # A package prefix matches a split package.
+
-                split_package_for_output = slash_package_to_dot_package(
+    # Check to make sure that the single packages and package prefixes do not
-                    split_package)
+    # overlap.
-                package_prefix_for_output = slash_package_to_dot_package(
+    for single_package in single_packages:
-                    package_prefix)
+        errs = validate_package_is_not_matched_by_package_prefix(
-                errors.append(
+            'single package', single_package, package_prefixes)
-                    f'split package {split_package_for_output} is matched by '
+        errors.extend(errs)
-                    f'package prefix {package_prefix_for_output}')
     return errors
 
 
@@ -102,21 +116,40 @@ def validate_split_packages(split_packages):
     return errors
 
 
+def validate_single_packages(split_packages, single_packages):
+    overlaps = []
+    for single_package in single_packages:
+        if single_package in split_packages:
+            overlaps.append(single_package)
+    if overlaps:
+        indented = ''.join([f'\n    {o}' for o in overlaps])
+        return [
+            f'single_packages and split_packages overlap, please ensure the '
+            f'following packages are only present in one:{indented}'
+        ]
+    return []
+
+
 def produce_patterns_from_file(file,
                                split_packages=None,
+                               single_packages=None,
                                package_prefixes=None):
     with open(file, 'r', encoding='utf8') as f:
-        return produce_patterns_from_stream(f, split_packages, package_prefixes)
+        return produce_patterns_from_stream(f, split_packages, single_packages,
+                                            package_prefixes)
 
 
 def produce_patterns_from_stream(stream,
                                  split_packages=None,
+                                 single_packages=None,
                                  package_prefixes=None):
     split_packages = set(split_packages or [])
+    single_packages = set(single_packages or [])
     package_prefixes = list(package_prefixes or [])
     # Read in all the signatures into a list and remove any unnecessary class
     # and member names.
     patterns = set()
+    unmatched_packages = set()
     for row in dict_reader(stream):
         signature = row['signature']
         text = signature.removeprefix('L')
@@ -138,11 +171,31 @@ def produce_patterns_from_stream(stream,
             # Remove inner class names.
             pieces = qualified_class_name.split('$', maxsplit=1)
             pattern = pieces[0]
-        else:
+            patterns.add(pattern)
+        elif pkg in single_packages:
             # Add a * to ensure that the pattern matches the classes in that
             # package.
             pattern = pkg + '/*'
-        patterns.add(pattern)
+            patterns.add(pattern)
+        else:
+            unmatched_packages.add(pkg)
+
+    # Remove any unmatched packages that would be matched by a package prefix
+    # pattern.
+    unmatched_packages = [
+        p for p in unmatched_packages
+        if not matched_by_package_prefix_pattern(package_prefixes, p)
+    ]
+    errors = []
+    if unmatched_packages:
+        unmatched_packages.sort()
+        indented = ''.join([
+            f'\n    {slash_package_to_dot_package(p)}'
+            for p in unmatched_packages
+        ])
+        errors.append('The following packages were unexpected, please add them '
+                      'to one of the hidden_api properties, split_packages, '
+                      f'single_packages or package_prefixes:{indented}')
 
     # Remove any patterns that would be matched by a package prefix pattern.
     patterns = [
@@ -155,7 +208,13 @@ def produce_patterns_from_stream(stream,
     patterns = patterns + [f'{p}/**' for p in package_prefixes]
     # Sort the patterns.
     patterns.sort()
-    return patterns
+    return patterns, errors
+
+
+def print_and_exit(errors):
+    for error in errors:
+        print(error)
+    sys.exit(1)
 
 
 def main(args):
@@ -175,26 +234,41 @@ def main(args):
         '--package-prefix',
         action='append',
         help='A package prefix unique to this set of flags')
+    args_parser.add_argument(
+        '--single-package',
+        action='append',
+        help='A single package unique to this set of flags')
     args_parser.add_argument('--output', help='Generated signature prefixes')
     args = args_parser.parse_args(args)
 
     split_packages = set(
         dot_packages_to_slash_packages(args.split_package or []))
     errors = validate_split_packages(split_packages)
+    if errors:
+        print_and_exit(errors)
+
+    single_packages = list(
+        dot_packages_to_slash_packages(args.single_package or []))
+
+    errors = validate_single_packages(split_packages, single_packages)
+    if errors:
+        print_and_exit(errors)
 
     package_prefixes = dot_packages_to_slash_packages(args.package_prefix or [])
 
-    if not errors:
+    errors = validate_package_prefixes(split_packages, single_packages,
-        errors = validate_package_prefixes(split_packages, package_prefixes)
+                                       package_prefixes)
+    if errors:
+        print_and_exit(errors)
+
+    patterns = []
+    # Read in all the patterns into a list.
+    patterns, errors = produce_patterns_from_file(args.flags, split_packages,
+                                                  single_packages,
+                                                  package_prefixes)
 
     if errors:
-        for error in errors:
+        print_and_exit(errors)
-            print(error)
-        sys.exit(1)
-
-    # Read in all the patterns into a list.
-    patterns = produce_patterns_from_file(args.flags, split_packages,
-                                          package_prefixes)
 
     # Write out all the patterns.
     with open(args.output, 'w', encoding='utf8') as outputFile:

scripts/hiddenapi/signature_patterns_test.py

@@ -32,22 +32,37 @@ Ljava/lang/Object;->toString()Ljava/lang/String;,blocked
     @staticmethod
     def produce_patterns_from_string(csv_text,
                                      split_packages=None,
+                                     single_packages=None,
                                      package_prefixes=None):
         with io.StringIO(csv_text) as f:
             return signature_patterns.produce_patterns_from_stream(
-                f, split_packages, package_prefixes)
+                f, split_packages, single_packages, package_prefixes)
+
+    def test_generate_unmatched(self):
+        _, errors = self.produce_patterns_from_string(
+            TestGeneratedPatterns.csv_flags)
+        self.assertEqual([
+            'The following packages were unexpected, please add them to one of '
+            'the hidden_api properties, split_packages, single_packages or '
+            'package_prefixes:\n'
+            '    java.lang'
+        ], errors)
 
     def test_generate_default(self):
-        patterns = self.produce_patterns_from_string(
+        patterns, errors = self.produce_patterns_from_string(
-            TestGeneratedPatterns.csv_flags)
+            TestGeneratedPatterns.csv_flags, single_packages=['java/lang'])
+        self.assertEqual([], errors)
+
         expected = [
             'java/lang/*',
         ]
         self.assertEqual(expected, patterns)
 
     def test_generate_split_package(self):
-        patterns = self.produce_patterns_from_string(
+        patterns, errors = self.produce_patterns_from_string(
             TestGeneratedPatterns.csv_flags, split_packages={'java/lang'})
+        self.assertEqual([], errors)
+
         expected = [
             'java/lang/Character',
             'java/lang/Object',
@@ -56,8 +71,10 @@ Ljava/lang/Object;->toString()Ljava/lang/String;,blocked
         self.assertEqual(expected, patterns)
 
     def test_generate_split_package_wildcard(self):
-        patterns = self.produce_patterns_from_string(
+        patterns, errors = self.produce_patterns_from_string(
             TestGeneratedPatterns.csv_flags, split_packages={'*'})
+        self.assertEqual([], errors)
+
         expected = [
             'java/lang/Character',
             'java/lang/Object',
@@ -66,16 +83,20 @@ Ljava/lang/Object;->toString()Ljava/lang/String;,blocked
         self.assertEqual(expected, patterns)
 
     def test_generate_package_prefix(self):
-        patterns = self.produce_patterns_from_string(
+        patterns, errors = self.produce_patterns_from_string(
             TestGeneratedPatterns.csv_flags, package_prefixes={'java/lang'})
+        self.assertEqual([], errors)
+
         expected = [
             'java/lang/**',
         ]
         self.assertEqual(expected, patterns)
 
     def test_generate_package_prefix_top_package(self):
-        patterns = self.produce_patterns_from_string(
+        patterns, errors = self.produce_patterns_from_string(
             TestGeneratedPatterns.csv_flags, package_prefixes={'java'})
+        self.assertEqual([], errors)
+
         expected = [
             'java/**',
         ]
@@ -92,21 +113,38 @@ Ljava/lang/Object;->toString()Ljava/lang/String;,blocked
 
     def test_split_package_wildcard_conflicts_with_package_prefixes(self):
         errors = signature_patterns.validate_package_prefixes(
-            {'*'}, package_prefixes={'java'})
+            {'*'}, [], package_prefixes={'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):
+    def test_split_package_conflicts_with_package_prefixes(self):
         errors = signature_patterns.validate_package_prefixes(
-            {'java/split'}, package_prefixes={'java'})
+            {'java/split'}, [], package_prefixes={'java'})
         expected = [
             'split package java.split is matched by package prefix java',
         ]
         self.assertEqual(expected, errors)
 
+    def test_single_package_conflicts_with_package_prefixes(self):
+        errors = signature_patterns.validate_package_prefixes(
+            {}, ['java/single'], package_prefixes={'java'})
+        expected = [
+            'single package java.single is matched by package prefix java',
+        ]
+        self.assertEqual(expected, errors)
+
+    def test_single_package_conflicts_with_split_packages(self):
+        errors = signature_patterns.validate_single_packages({'java/pkg'},
+                                                             ['java/pkg'])
+        expected = [
+            'single_packages and split_packages overlap, please ensure the '
+            'following packages are only present in one:\n    java/pkg'
+        ]
+        self.assertEqual(expected, errors)
+
 
 if __name__ == '__main__':
     unittest.main(verbosity=2)