AAPT2: Honor --custom-package when writing Manifest.java

Bug: 62826426
Test: manual
Change-Id: I517ff5da2ef2a9147ddbfb13b074a1f60550f189

diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp
index 740a401..e6bf3a6 100644 (file)
--- a/tools/aapt2/cmd/Link.cpp
+++ b/tools/aapt2/cmd/Link.cpp
@@ -989,7 +989,8 @@ class LinkCommand {
       manifest_class->GetCommentBuilder()->AppendComment(proper_annotation);
     }
 
-    const std::string& package_utf8 = context_->GetCompilationPackage();
+    const std::string package_utf8 =
+        options_.custom_java_package.value_or_default(context_->GetCompilationPackage());
 
     std::string out_path = options_.generate_java_class_path.value();
     file::AppendPath(&out_path, file::PackageToPath(package_utf8));

diff --git a/tools/aapt2/java/ManifestClassGenerator_test.cpp b/tools/aapt2/java/ManifestClassGenerator_test.cpp
index 5ebf508..9f6ec21 100644 (file)
--- a/tools/aapt2/java/ManifestClassGenerator_test.cpp
+++ b/tools/aapt2/java/ManifestClassGenerator_test.cpp
@@ -18,124 +18,115 @@
 
 #include "test/Test.h"
 
-namespace aapt {
+using ::testing::HasSubstr;
+using ::testing::Not;
 
-static ::testing::AssertionResult GetManifestClassText(IAaptContext* context,
-                                                       xml::XmlResource* res,
-                                                       std::string* out_str) {
-  std::unique_ptr<ClassDefinition> manifest_class =
-      GenerateManifestClass(context->GetDiagnostics(), res);
-  if (!manifest_class) {
-    return ::testing::AssertionFailure() << "manifest_class == nullptr";
-  }
-
-  std::stringstream out;
-  if (!manifest_class->WriteJavaFile(manifest_class.get(), "android", true,
-                                     &out)) {
-    return ::testing::AssertionFailure() << "failed to write java file";
-  }
+namespace aapt {
 
-  *out_str = out.str();
-  return ::testing::AssertionSuccess();
-}
+static ::testing::AssertionResult GetManifestClassText(IAaptContext* context, xml::XmlResource* res,
+                                                       std::string* out_str);
 
 TEST(ManifestClassGeneratorTest, NameIsProperlyGeneratedFromSymbol) {
   std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
-  std::unique_ptr<xml::XmlResource> manifest = test::BuildXmlDom(R"EOF(
-        <manifest xmlns:android="http://schemas.android.com/apk/res/android">
-          <permission android:name="android.permission.ACCESS_INTERNET" />
-          <permission android:name="android.DO_DANGEROUS_THINGS" />
-          <permission android:name="com.test.sample.permission.HUH" />
-          <permission-group android:name="foo.bar.PERMISSION" />
-        </manifest>)EOF");
+  std::unique_ptr<xml::XmlResource> manifest = test::BuildXmlDom(R"(
+      <manifest xmlns:android="http://schemas.android.com/apk/res/android">
+        <permission android:name="android.permission.ACCESS_INTERNET" />
+        <permission android:name="android.DO_DANGEROUS_THINGS" />
+        <permission android:name="com.test.sample.permission.HUH" />
+        <permission-group android:name="foo.bar.PERMISSION" />
+      </manifest>)");
 
   std::string actual;
   ASSERT_TRUE(GetManifestClassText(context.get(), manifest.get(), &actual));
 
-  const size_t permission_class_pos =
-      actual.find("public static final class permission {");
-  const size_t permission_croup_class_pos =
+  ASSERT_THAT(actual, HasSubstr("public static final class permission {"));
+  ASSERT_THAT(actual, HasSubstr("public static final class permission_group {"));
+
+  const size_t permission_start_pos = actual.find("public static final class permission {");
+  const size_t permission_group_start_pos =
       actual.find("public static final class permission_group {");
-  ASSERT_NE(std::string::npos, permission_class_pos);
-  ASSERT_NE(std::string::npos, permission_croup_class_pos);
 
   //
   // Make sure these permissions are in the permission class.
   //
-
-  size_t pos = actual.find(
-      "public static final String ACCESS_INTERNET="
-      "\"android.permission.ACCESS_INTERNET\";");
-  EXPECT_GT(pos, permission_class_pos);
-  EXPECT_LT(pos, permission_croup_class_pos);
-
-  pos = actual.find(
-      "public static final String DO_DANGEROUS_THINGS="
-      "\"android.DO_DANGEROUS_THINGS\";");
-  EXPECT_GT(pos, permission_class_pos);
-  EXPECT_LT(pos, permission_croup_class_pos);
-
-  pos = actual.find(
-      "public static final String HUH=\"com.test.sample.permission.HUH\";");
-  EXPECT_GT(pos, permission_class_pos);
-  EXPECT_LT(pos, permission_croup_class_pos);
+  const std::string permission_class =
+      actual.substr(permission_start_pos, permission_group_start_pos - permission_start_pos);
+
+  EXPECT_THAT(
+      permission_class,
+      HasSubstr(
+          "public static final String ACCESS_INTERNET=\"android.permission.ACCESS_INTERNET\";"));
+  EXPECT_THAT(
+      permission_class,
+      HasSubstr("public static final String DO_DANGEROUS_THINGS=\"android.DO_DANGEROUS_THINGS\";"));
+  EXPECT_THAT(permission_class,
+              HasSubstr("public static final String HUH=\"com.test.sample.permission.HUH\";"));
 
   //
   // Make sure these permissions are in the permission_group class
   //
+  const std::string permission_group_class = actual.substr(permission_group_start_pos);
 
-  pos = actual.find(
-      "public static final String PERMISSION="
-      "\"foo.bar.PERMISSION\";");
-  EXPECT_GT(pos, permission_croup_class_pos);
-  EXPECT_LT(pos, std::string::npos);
+  EXPECT_THAT(permission_group_class,
+              HasSubstr("public static final String PERMISSION=\"foo.bar.PERMISSION\";"));
 }
 
 TEST(ManifestClassGeneratorTest, CommentsAndAnnotationsArePresent) {
   std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
-  std::unique_ptr<xml::XmlResource> manifest = test::BuildXmlDom(R"EOF(
-        <manifest xmlns:android="http://schemas.android.com/apk/res/android">
-          <!-- Required to access the internet.
-               Added in API 1. -->
-          <permission android:name="android.permission.ACCESS_INTERNET" />
-          <!-- @deprecated This permission is for playing outside. -->
-          <permission android:name="android.permission.PLAY_OUTSIDE" />
-          <!-- This is a private permission for system only!
-               @hide
-               @SystemApi -->
-          <permission android:name="android.permission.SECRET" />
-        </manifest>)EOF");
+  std::unique_ptr<xml::XmlResource> manifest = test::BuildXmlDom(R"(
+      <manifest xmlns:android="http://schemas.android.com/apk/res/android">
+        <!-- Required to access the internet.
+             Added in API 1. -->
+        <permission android:name="android.permission.ACCESS_INTERNET" />
+        <!-- @deprecated This permission is for playing outside. -->
+        <permission android:name="android.permission.PLAY_OUTSIDE" />
+        <!-- This is a private permission for system only!
+             @hide
+             @SystemApi -->
+        <permission android:name="android.permission.SECRET" />
+      </manifest>)");
 
   std::string actual;
   ASSERT_TRUE(GetManifestClassText(context.get(), manifest.get(), &actual));
 
-  const char* expected_access_internet =
-      R"EOF(    /**
+  const char* expected_access_internet = R"(    /**
      * Required to access the internet.
      * Added in API 1.
      */
-    public static final String ACCESS_INTERNET="android.permission.ACCESS_INTERNET";)EOF";
-
-  EXPECT_NE(std::string::npos, actual.find(expected_access_internet));
+    public static final String ACCESS_INTERNET="android.permission.ACCESS_INTERNET";)";
+  EXPECT_THAT(actual, HasSubstr(expected_access_internet));
 
-  const char* expected_play_outside =
-      R"EOF(    /**
+  const char* expected_play_outside = R"(    /**
      * @deprecated This permission is for playing outside.
      */
     @Deprecated
-    public static final String PLAY_OUTSIDE="android.permission.PLAY_OUTSIDE";)EOF";
-
-  EXPECT_NE(std::string::npos, actual.find(expected_play_outside));
+    public static final String PLAY_OUTSIDE="android.permission.PLAY_OUTSIDE";)";
+  EXPECT_THAT(actual, HasSubstr(expected_play_outside));
 
-  const char* expected_secret =
-      R"EOF(    /**
+  const char* expected_secret = R"(    /**
      * This is a private permission for system only!
      * @hide
      */
     @android.annotation.SystemApi
-    public static final String SECRET="android.permission.SECRET";)EOF";
+    public static final String SECRET="android.permission.SECRET";)";
+  EXPECT_THAT(actual, HasSubstr(expected_secret));
+}
 
-  EXPECT_NE(std::string::npos, actual.find(expected_secret));
+static ::testing::AssertionResult GetManifestClassText(IAaptContext* context, xml::XmlResource* res,
+                                                       std::string* out_str) {
+  std::unique_ptr<ClassDefinition> manifest_class =
+      GenerateManifestClass(context->GetDiagnostics(), res);
+  if (!manifest_class) {
+    return ::testing::AssertionFailure() << "manifest_class == nullptr";
+  }
+
+  std::stringstream out;
+  if (!manifest_class->WriteJavaFile(manifest_class.get(), "android", true, &out)) {
+    return ::testing::AssertionFailure() << "failed to write java file";
+  }
+
+  *out_str = out.str();
+  return ::testing::AssertionSuccess();
 }
 
 }  // namespace aapt

diff --git a/tools/aapt2/readme.md b/tools/aapt2/readme.md
index d6483ec..9b2e0a4 100644 (file)
--- a/tools/aapt2/readme.md
+++ b/tools/aapt2/readme.md
@@ -4,6 +4,7 @@
 ### `aapt2 ...`
 - Fixed issue where enum values were interpreted as integers and range checked. (bug 62358540)
 - Fixed issue where ints and floats with trailing whitespace would not be parsed. (bug 62902869)
+- Fixed issue where `--custom-package` was not honored when writing Manifest.java. (bug 62826426)
 
 ## Version 2.17
 ### `aapt2 ...`