From aada89ce19e36569cfe165753dc4cb6c09fe3b73 Mon Sep 17 00:00:00 2001 From: Ryan Mitchell Date: Tue, 12 Feb 2019 08:06:26 -0800 Subject: [PATCH] Correctly insert platformBuildVersionCode/Name AAPT(1) encodes platformBuildVersionCode and platformBuildVersionName into the manifest along with compileSdkVersion and compileSdkVersionCodename. AAPT2 was extracting the platform attributes from the app being created rather than the framework. This change inserts the correct platform attributes. Bug: 124227171 Test: aapt2_tests Change-Id: I4bc7986d25f065cecf01b9e988de4d37283ef7de --- tools/aapt2/link/ManifestFixer.cpp | 29 ++++---- tools/aapt2/link/ManifestFixer_test.cpp | 90 ++++++++++--------------- 2 files changed, 49 insertions(+), 70 deletions(-) diff --git a/tools/aapt2/link/ManifestFixer.cpp b/tools/aapt2/link/ManifestFixer.cpp index 85bf6f218ba07..582a5b869c658 100644 --- a/tools/aapt2/link/ManifestFixer.cpp +++ b/tools/aapt2/link/ManifestFixer.cpp @@ -294,22 +294,6 @@ bool ManifestFixer::BuildRules(xml::XmlActionExecutor* executor, } } - if (el->FindAttribute("", "platformBuildVersionCode") == nullptr) { - auto versionCode = el->FindAttribute(xml::kSchemaAndroid, "versionCode"); - if (versionCode != nullptr) { - el->attributes.push_back(xml::Attribute{"", "platformBuildVersionCode", - versionCode->value}); - } - } - - if (el->FindAttribute("", "platformBuildVersionName") == nullptr) { - auto versionName = el->FindAttribute(xml::kSchemaAndroid, "versionName"); - if (versionName != nullptr) { - el->attributes.push_back(xml::Attribute{"", "platformBuildVersionName", - versionName->value}); - } - } - return true; }); @@ -489,8 +473,14 @@ bool ManifestFixer::Consume(IAaptContext* context, xml::XmlResource* doc) { // Make sure we un-compile the value if it was set to something else. attr->compiled_value = {}; - attr->value = options_.compile_sdk_version.value(); + + attr = root->FindOrCreateAttribute("", "platformBuildVersionCode"); + + // Make sure we un-compile the value if it was set to something else. + attr->compiled_value = {}; + attr->value = options_.compile_sdk_version.value(); + } if (options_.compile_sdk_version_codename) { @@ -499,7 +489,12 @@ bool ManifestFixer::Consume(IAaptContext* context, xml::XmlResource* doc) { // Make sure we un-compile the value if it was set to something else. attr->compiled_value = {}; + attr->value = options_.compile_sdk_version_codename.value(); + attr = root->FindOrCreateAttribute("", "platformBuildVersionName"); + + // Make sure we un-compile the value if it was set to something else. + attr->compiled_value = {}; attr->value = options_.compile_sdk_version_codename.value(); } diff --git a/tools/aapt2/link/ManifestFixer_test.cpp b/tools/aapt2/link/ManifestFixer_test.cpp index adea6273bc8b6..4842f62e53b58 100644 --- a/tools/aapt2/link/ManifestFixer_test.cpp +++ b/tools/aapt2/link/ManifestFixer_test.cpp @@ -725,6 +725,43 @@ TEST_F(ManifestFixerTest, InsertCompileSdkVersions) { attr = manifest->root->FindAttribute(xml::kSchemaAndroid, "compileSdkVersionCodename"); ASSERT_THAT(attr, NotNull()); EXPECT_THAT(attr->value, StrEq("P")); + + attr = manifest->root->FindAttribute("", "platformBuildVersionCode"); + ASSERT_THAT(attr, NotNull()); + EXPECT_THAT(attr->value, StrEq("28")); + + attr = manifest->root->FindAttribute("", "platformBuildVersionName"); + ASSERT_THAT(attr, NotNull()); + EXPECT_THAT(attr->value, StrEq("P")); +} + +TEST_F(ManifestFixerTest, OverrideCompileSdkVersions) { + std::string input = R"( + )"; + ManifestFixerOptions options; + options.compile_sdk_version = {"28"}; + options.compile_sdk_version_codename = {"P"}; + + std::unique_ptr manifest = VerifyWithOptions(input, options); + ASSERT_THAT(manifest, NotNull()); + + xml::Attribute* attr = manifest->root->FindAttribute(xml::kSchemaAndroid, "compileSdkVersion"); + ASSERT_THAT(attr, NotNull()); + EXPECT_THAT(attr->value, StrEq("28")); + + attr = manifest->root->FindAttribute(xml::kSchemaAndroid, "compileSdkVersionCodename"); + ASSERT_THAT(attr, NotNull()); + EXPECT_THAT(attr->value, StrEq("P")); + + attr = manifest->root->FindAttribute("", "platformBuildVersionCode"); + ASSERT_THAT(attr, NotNull()); + EXPECT_THAT(attr->value, StrEq("28")); + + attr = manifest->root->FindAttribute("", "platformBuildVersionName"); + ASSERT_THAT(attr, NotNull()); + EXPECT_THAT(attr->value, StrEq("P")); } TEST_F(ManifestFixerTest, UnexpectedElementsInManifest) { @@ -750,59 +787,6 @@ TEST_F(ManifestFixerTest, UnexpectedElementsInManifest) { ASSERT_THAT(manifest, IsNull()); } -TEST_F(ManifestFixerTest, InsertPlatformBuildVersions) { - // Test for insertion when versionCode and versionName are included in the manifest - { - std::string input = R"( - )"; - std::unique_ptr manifest = Verify(input); - ASSERT_THAT(manifest, NotNull()); - - xml::Attribute* attr = manifest->root->FindAttribute("", "platformBuildVersionCode"); - ASSERT_THAT(attr, NotNull()); - EXPECT_THAT(attr->value, StrEq("27")); - attr = manifest->root->FindAttribute("", "platformBuildVersionName"); - ASSERT_THAT(attr, NotNull()); - EXPECT_THAT(attr->value, StrEq("O")); - } - - // Test for insertion when versionCode and versionName defaults are specified - { - std::string input = R"( - )"; - ManifestFixerOptions options; - options.version_code_default = {"27"}; - options.version_name_default = {"O"}; - std::unique_ptr manifest = VerifyWithOptions(input, options); - ASSERT_THAT(manifest, NotNull()); - - xml::Attribute* attr = manifest->root->FindAttribute("", "platformBuildVersionCode"); - ASSERT_THAT(attr, NotNull()); - EXPECT_THAT(attr->value, StrEq("27")); - attr = manifest->root->FindAttribute("", "platformBuildVersionName"); - ASSERT_THAT(attr, NotNull()); - EXPECT_THAT(attr->value, StrEq("O")); - } - - // Test that the platform build version attributes are not changed if they are currently present - { - std::string input = R"( - )"; - std::unique_ptr manifest = Verify(input); - ASSERT_THAT(manifest, NotNull()); - - xml::Attribute* attr = manifest->root->FindAttribute("", "platformBuildVersionCode"); - ASSERT_THAT(attr, NotNull()); - EXPECT_THAT(attr->value, StrEq("27")); - attr = manifest->root->FindAttribute("", "platformBuildVersionName"); - ASSERT_THAT(attr, NotNull()); - EXPECT_THAT(attr->value, StrEq("O")); - } -} - TEST_F(ManifestFixerTest, UsesLibraryMustHaveNonEmptyName) { std::string input = R"(