Support uses-permission merging in manifest merger

src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java

@@ -980,6 +980,15 @@ public static class Options extends FragmentOptions {
                 + " transition` with changed options to avoid potential action conflicts.")
     public boolean androidPlatformsTransitionsUpdateAffected;
 
+    @Option(
+        name = "merge_android_manifest_permissions",
+        defaultValue = "false",
+        documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+        effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
+        help = "If enabled, the manifest merger will merge uses-permission and "
+                + "uses-permission-sdk-23 attributes.")
+    public boolean mergeAndroidManifestPermissions;
+
     @Override
     public FragmentOptions getHost() {
       Options host = (Options) super.getHost();
@@ -1065,6 +1074,7 @@ public FragmentOptions getHost() {
   private final boolean hwasan;
   private final boolean getJavaResourcesFromOptimizedJar;
   private final boolean includeProguardLocationReferences;
+  private final boolean mergeAndroidManifestPermissions;
 
   public AndroidConfiguration(BuildOptions buildOptions) throws InvalidConfigurationException {
     Options options = buildOptions.get(Options.class);
@@ -1126,6 +1136,7 @@ public AndroidConfiguration(BuildOptions buildOptions) throws InvalidConfigurati
     this.hwasan = options.hwasan;
     this.getJavaResourcesFromOptimizedJar = options.getJavaResourcesFromOptimizedJar;
     this.includeProguardLocationReferences = options.includeProguardLocationReferences;
+    this.mergeAndroidManifestPermissions = options.mergeAndroidManifestPermissions;
 
     if (incrementalDexingShardsAfterProguard < 0) {
       throw new InvalidConfigurationException(
@@ -1411,6 +1422,10 @@ public boolean includeProguardLocationReferences() {
     return includeProguardLocationReferences;
   }
 
+  public boolean mergeAndroidManifestPermissions() {
+    return mergeAndroidManifestPermissions;
+  }
+
   /** Returns the label provided with --legacy_main_dex_list_generator, if any. */
   // TODO(b/147692286): Move R8's main dex list tool into tool repository.
   @StarlarkConfigurationField(

src/main/java/com/google/devtools/build/lib/rules/android/AndroidManifest.java

@@ -191,6 +191,7 @@ public StampedAndroidManifest stamp(AndroidDataContext dataContext) {
       new ManifestMergerActionBuilder()
           .setManifest(manifest)
           .setLibrary(true)
+          .setMergeManifestPermissions(dataContext.getAndroidConfig().mergeAndroidManifestPermissions())
           .setCustomPackage(pkg)
           .setManifestOutput(outputManifest)
           .build(dataContext);
@@ -235,6 +236,7 @@ public StampedAndroidManifest mergeWithDeps(
           .setManifest(manifest)
           .setMergeeManifests(mergeeManifests)
           .setLibrary(false)
+          .setMergeManifestPermissions(dataContext.getAndroidConfig().mergeAndroidManifestPermissions())
           .setManifestValues(manifestValues)
           .setCustomPackage(pkg)
           .setManifestOutput(newManifest)

src/main/java/com/google/devtools/build/lib/rules/android/ManifestMergerActionBuilder.java

@@ -27,6 +27,7 @@ public class ManifestMergerActionBuilder {
   private Artifact manifest;
   private Map<Artifact, Label> mergeeManifests;
   private boolean isLibrary;
+  private boolean mergeManifestPermissions;
   private Map<String, String> manifestValues;
   private String customPackage;
   private Artifact manifestOutput;
@@ -47,6 +48,11 @@ public ManifestMergerActionBuilder setLibrary(boolean isLibrary) {
     return this;
   }
 
+  public ManifestMergerActionBuilder setMergeManifestPermissions(boolean mergeManifestPermissions) {
+    this.mergeManifestPermissions = mergeManifestPermissions;
+    return this;
+  }
+
   public ManifestMergerActionBuilder setManifestValues(Map<String, String> manifestValues) {
     this.manifestValues = manifestValues;
     return this;
@@ -80,6 +86,10 @@ public void build(AndroidDataContext dataContext) {
           mergeeManifests.keySet());
     }
 
+    if (mergeManifestPermissions) {
+      builder.addFlag("--mergeManifestPermissions");
+    }
+
     builder
         .maybeAddFlag("--mergeType", isLibrary)
         .maybeAddFlag("LIBRARY", isLibrary)

src/test/java/com/google/devtools/build/android/ManifestMergerActionTest.java

@@ -159,7 +159,62 @@ public void testMerge_GenerateDummyManifest() throws Exception {
             false, /* isLibrary */
             ImmutableMap.of("applicationId", "com.google.android.apps.testapp"),
             "", /* custom_package */
-            mergedManifest);
+            mergedManifest,
+            /* mergeManifestPermissions */false);
+    ManifestMergerAction.main(args.toArray(new String[0]));
+
+    assertThat(
+        Joiner.on(" ")
+            .join(Files.readAllLines(mergedManifest, UTF_8))
+            .replaceAll("\\s+", " ")
+            .trim())
+        .isEqualTo(
+            Joiner.on(" ")
+                .join(Files.readAllLines(expectedManifest, UTF_8))
+                .replaceAll("\\s+", " ")
+                .trim());
+  }
+
+  @Test public void testMergeWithMergePermissionsEnabled() throws Exception {
+    String dataDir =
+        Paths.get(System.getenv("TEST_WORKSPACE"), System.getenv("TEST_BINARY"))
+            .resolveSibling("testing/manifestmerge")
+            .toString()
+            .replace("\\", "/");
+
+    final Path mergerManifest = rlocation(dataDir + "/merger/AndroidManifest.xml");
+    final Path mergeeManifestOne = rlocation(dataDir + "/mergeeOne/AndroidManifest.xml");
+    final Path mergeeManifestTwo = rlocation(dataDir + "/mergeeTwo/AndroidManifest.xml");
+    assertThat(mergerManifest.toFile().exists()).isTrue();
+    assertThat(mergeeManifestOne.toFile().exists()).isTrue();
+    assertThat(mergeeManifestTwo.toFile().exists()).isTrue();
+
+    // The following code retrieves the path of the only AndroidManifest.xml in the expected/
+    // manifests directory. Unfortunately, this test runs internally and externally and the files
+    // have different names.
+    final File expectedManifestDirectory =
+        mergerManifest.getParent().resolveSibling("expected-merged-permissions").toFile();
+    final String[] debug =
+        expectedManifestDirectory.list(new PatternFilenameFilter(".*AndroidManifest\\.xml$"));
+    assertThat(debug).isNotNull();
+    final File[] expectedManifestDirectoryManifests =
+        expectedManifestDirectory.listFiles((File dir, String name) -> true);
+    assertThat(expectedManifestDirectoryManifests).isNotNull();
+    assertThat(expectedManifestDirectoryManifests).hasLength(1);
+    final Path expectedManifest = expectedManifestDirectoryManifests[0].toPath();
+
+    Files.createDirectories(working.resolve("output"));
+    final Path mergedManifest = working.resolve("output/mergedManifest.xml");
+
+    List<String> args =
+        generateArgs(
+            mergerManifest,
+            ImmutableMap.of(mergeeManifestOne, "mergeeOne", mergeeManifestTwo, "mergeeTwo"),
+            false, /* isLibrary */
+            ImmutableMap.of("applicationId", "com.google.android.apps.testapp"),
+            "", /* custom_package */
+            mergedManifest,
+            /* mergeManifestPermissions */true);
     ManifestMergerAction.main(args.toArray(new String[0]));
 
     assertThat(
@@ -199,7 +254,7 @@ public void testMerge_GenerateDummyManifest() throws Exception {
 
     // libFoo manifest merging
     List<String> args = generateArgs(libFooManifest, ImmutableMap.<Path, String>of(), true,
-        ImmutableMap.<String, String>of(), "", libFooOutput);
+        ImmutableMap.<String, String>of(), "", libFooOutput, false);
     ManifestMergerAction.main(args.toArray(new String[0]));
     assertThat(Joiner.on(" ")
         .join(Files.readAllLines(libFooOutput, UTF_8))
@@ -212,7 +267,7 @@ public void testMerge_GenerateDummyManifest() throws Exception {
 
     // libBar manifest merging
     args = generateArgs(libBarManifest, ImmutableMap.<Path, String>of(), true,
-        ImmutableMap.<String, String>of(), "com.google.libbar", libBarOutput);
+        ImmutableMap.<String, String>of(), "com.google.libbar", libBarOutput, false);
     ManifestMergerAction.main(args.toArray(new String[0]));
     assertThat(Joiner.on(" ")
         .join(Files.readAllLines(libBarOutput, UTF_8))
@@ -235,7 +290,8 @@ public void testMerge_GenerateDummyManifest() throws Exception {
                 "applicationId", "com.google.android.app",
                 "foo", "this \\\\: is \"a, \"bad string"),
             /* customPackage= */ "",
-            binaryOutput);
+            binaryOutput,
+            /* mergeManifestPermissions */false);
     ManifestMergerAction.main(args.toArray(new String[0]));
     assertThat(Joiner.on(" ")
         .join(Files.readAllLines(binaryOutput, UTF_8))
@@ -255,14 +311,22 @@ private List<String> generateArgs(
       boolean library,
       Map<String, String> manifestValues,
       String customPackage,
-      Path manifestOutput) {
-    return ImmutableList.of(
+      Path manifestOutput,
+      boolean mergeManifestPermissions) {
+    ImmutableList.Builder<String> builder = ImmutableList.builder();
+    builder.add(
         "--manifest", manifest.toString(),
-        "--mergeeManifests", mapToDictionaryString(mergeeManifests),
+        "--mergeeManifests", mapToDictionaryString(mergeeManifests));
+    if (mergeManifestPermissions) {
+      builder.add("--mergeManifestPermissions");
+    }
+
+    builder.add(
         "--mergeType", library ? "LIBRARY" : "APPLICATION",
         "--manifestValues", mapToDictionaryString(manifestValues),
         "--customPackage", customPackage,
         "--manifestOutput", manifestOutput.toString());
+    return builder.build();
   }
 
   private <K, V> String mapToDictionaryString(Map<K, V> map) {

src/test/java/com/google/devtools/build/android/testing/manifestmerge/BUILD

@@ -13,6 +13,7 @@ filegroup(
 filegroup(
     name = "test_data",
     srcs = [
+        "expected-merged-permissions/AndroidManifest.xml",
         "expected/AndroidManifest.xml",
         "mergeeOne/AndroidManifest.xml",
         "mergeeTwo/AndroidManifest.xml",

src/test/java/com/google/devtools/build/android/testing/manifestmerge/expected-merged-permissions/AndroidManifest.xml

@@ -0,0 +1,107 @@
+<?xml version="1.0" encoding="utf-8"?>
+<manifest
+        xmlns:android="http://schemas.android.com/apk/res/android"
+        package="com.google.android.apps.testapp"
+        android:versionCode="70"
+        android:versionName="1.0" >
+  <uses-sdk android:minSdkVersion="10" />
+  <uses-feature android:name="android.hardware.nfc" android:required="true" />
+  <uses-permission android:name="android.permission.ACCESS_COARSE_LOCATION" />
+  <permission android:name="com.google.android.apps.foo.C2D_MESSAGE" android:protectionLevel="signature" />
+  <uses-permission android:name="android.permission.READ_LOGS" />
+  <uses-permission android:name="android.permission.INTERNET" />
+  <uses-permission android:name="android.permission.ACCESS_NETWORK_STATE" />
+  <!-- Comment for permission android.permission.GET_ACCOUNTS. This is just to make sure the comment is being merged correctly. -->
+  <uses-permission android:name="android.permission.GET_ACCOUNTS" />
+  <android:uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />
+  <android:uses-permission android:name="android.permission.READ_PHONE_STATE" />
+  <android:uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE" />
+  <application
+          android:name="com.google.android.apps.testapp.TestApplication"
+          android:backupAgent="com.google.android.apps.testapp2.FooBar"
+          android:icon="@drawable/icon"
+          android:label="@string/app_name"
+          android:theme="@style/Theme.Test" >
+    <!--  START LIBRARIES (Maintain Alphabetic order) -->
+    <!-- NFC extras -->
+    <uses-library android:name="com.google.android.nfc_extras" android:required="false" />
+    <!--  END LIBRARIES -->
+    <!--  START ACTIVITIES (Maintain Alphabetic order) -->
+    <!-- Entry point activity - navigation and title bar. -->
+    <activity
+            android:name="com.google.android.apps.testapp.entrypoint.EntryPointActivityGroup"
+            android:launchMode="singleTop"
+            android:screenOrientation="portrait" />
+    <activity android:name="com.google.android.apps.testapp.ui.topup.TopUpActivity" />
+    <service android:name="com.google.android.apps.testapp.nfcevent.NfcEventService" />
+    <receiver
+            android:name="com.receiver.TestReceiver"
+            android:process="@string/receiver_service_name" >
+      <!-- Receive the actual message -->
+      <intent-filter>
+        <action
+                android:name="android.intent.action.USER_PRESENT" />
+      </intent-filter>
+    </receiver>
+    <provider
+            android:name="com.google.android.apps.testapp.dataaccess.persistence.ContentProvider"
+            android:authorities="com.google.android.apps.testapp"
+            android:exported="false" />
+    <activity android:name="com.google.android.apps.testapp2.ui.home.HomeActivity" android:label="@string/app_name" >
+      <intent-filter>
+        <action android:name="android.intent.action.MAIN" />
+        <category android:name="android.intent.category.LAUNCHER" />
+      </intent-filter>
+    </activity>
+    <activity android:name="com.google.android.apps.testapp2.TestActivity2" />
+    <activity android:name="com.google.android.apps.testapp2.PreviewActivity" />
+    <activity android:name="com.google.android.apps.testapp2.ShowTextActivity"
+              android:excludeFromRecents="true" />
+    <activity android:name="com.google.android.apps.testapp2.ShowStringListActivity"
+              android:excludeFromRecents="true"
+              android:parentActivityName="com.google.android.apps.testapp2.ui.home.HomeActivity" >
+    </activity>
+    <service android:name="com.google.android.apps.testapp2.TestService" >
+      <meta-data android:name="param" android:value="value" />
+    </service>
+    <service android:name="com.google.android.apps.testapp2.nfcevent.NfcEventService" />
+    <receiver android:name="com.google.android.apps.testapp2.ConnectivityReceiver"
+              android:enabled="false" >
+      <intent-filter>
+        <action android:name="android.net.conn.CONNECTIVITY_CHANGE" />
+      </intent-filter>
+    </receiver>
+    <activity-alias android:name="com.google.android.apps.testapp2.BarFoo"
+                    android:targetActivity="com.google.android.apps.testapp2.FooBar" />
+    <provider
+            android:name="some.package.with.inner.class$AnInnerClass" />
+    <provider
+            android:name="com.google.android.apps.testapp"
+            android:authorities="com.google.android.apps.testapp.com.google.android.apps.testapp"
+            android:exported="false" />
+    <provider
+            android:name="com.google.android.apps.testapp.PlaceHolderProviderName"
+            android:authorities="PlaceHolderProviderAuthorities.com.google.android.apps.testapp"
+            android:exported="false" />
+    <activity
+            android:name="activityPrefix.com.google.android.apps.testapp.activitySuffix" >
+      <intent-filter>
+        <action android:name="actionPrefix.com.google.android.apps.testapp.actionSuffix" />
+      </intent-filter>
+    </activity>
+    <activity android:name="com.google.android.apps.testapp3.ui.home.HomeActivity" android:label="@string/app_name" >
+      <intent-filter>
+        <action android:name="android.intent.action.MAIN" />
+        <category android:name="android.intent.category.LAUNCHER" />
+      </intent-filter>
+    </activity>
+    <activity android:name="com.google.android.apps.testapp3.TestActivity" />
+    <service android:name="com.google.android.apps.testapp3.TestService" />
+    <receiver android:name="com.google.android.apps.testapp3.ConnectivityReceiver"
+              android:enabled="true" >
+      <intent-filter>
+        <action android:name="android.net.conn.CONNECTIVITY_CHANGER" />
+      </intent-filter>
+    </receiver>
+  </application>
+</manifest>

src/tools/android/java/com/google/devtools/build/android/ManifestMergerAction.java

@@ -62,6 +62,7 @@
  *       --manifestValues key value pairs of manifest overrides
  *       --customPackage package to write for library manifest
  *       --manifestOutput path to write output manifest
+ *       --mergeManifestPermissions merge manifest uses-permissions
  * </pre>
  */
 public class ManifestMergerAction {
@@ -152,6 +153,16 @@ public static final class Options extends OptionsBase {
       help = "Path to where the merger log should be written."
     )
     public Path log;
+
+    @Option(
+      name = "mergeManifestPermissions",
+      defaultValue = "false",
+      category = "output",
+      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+      effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
+      help = "If enabled, manifest permissions will be merged."
+    )
+    public boolean mergeManifestPermissions;
   }
 
   private static final String[] PERMISSION_TAGS =
@@ -194,13 +205,17 @@ public static void main(String[] args) throws Exception {
       Path mergedManifest;
       AndroidManifestProcessor manifestProcessor = AndroidManifestProcessor.with(stdLogger);
 
-      // Remove uses-permission tags from mergees before the merge.
       Path tmp = Files.createTempDirectory("manifest_merge_tmp");
       tmp.toFile().deleteOnExit();
       ImmutableMap.Builder<Path, String> mergeeManifests = ImmutableMap.builder();
       for (Map.Entry<Path, String> mergeeManifest : options.mergeeManifests.entrySet()) {
-        mergeeManifests.put(
-            removePermissions(mergeeManifest.getKey(), tmp), mergeeManifest.getValue());
+        if (!options.mergeManifestPermissions) {
+          // Remove uses-permission tags from mergees before the merge.
+          mergeeManifests.put(
+                  removePermissions(mergeeManifest.getKey(), tmp), mergeeManifest.getValue());
+        } else {
+          mergeeManifests.put(mergeeManifest);
+        }
       }
 
       Path manifest = options.manifest;