From 490595a5fc1c3618dbbc2da5d421d10c025fabac Mon Sep 17 00:00:00 2001 From: Adam Lesinski Date: Tue, 7 Nov 2017 17:08:07 -0800 Subject: [PATCH] AAPT2: Workaround for feature splits without namespacing Android Instant Apps in its current iteration does not use namespaces, but due to limitations on the Android resource runtime, needs to make it look like it does. This is due to a bug that treats any package ID that's not 0x01 or 0x7F as a shared library. Shared libraries require unique package names. As a workaround, and since Android Instant Apps can not have a feature depend on another feature, we can alter the resource package name of a feature split just before writing it out to disk. This avoids using a unique package name while linking, thereby avoiding namespace issues. Bug: 68820737 Test: manual Change-Id: Ic553ed42656436bbb949393d0248ee7bb9d37860 --- tests/FeatureSplit/feature1/Android.mk | 2 +- .../FeatureSplit/feature1/AndroidManifest.xml | 4 +- .../feature1/res/values/values.xml | 2 +- tests/FeatureSplit/feature2/Android.mk | 2 +- .../FeatureSplit/feature2/AndroidManifest.xml | 2 +- .../feature2/res/values/values.xml | 2 +- tools/aapt2/ResourceValues.cpp | 2 +- tools/aapt2/cmd/Link.cpp | 59 ++++++++++++++++--- tools/aapt2/format/binary/TableFlattener.cpp | 11 +++- 9 files changed, 70 insertions(+), 16 deletions(-) diff --git a/tests/FeatureSplit/feature1/Android.mk b/tests/FeatureSplit/feature1/Android.mk index b3ea97b03d5d6..e6ba5c2d04c97 100644 --- a/tests/FeatureSplit/feature1/Android.mk +++ b/tests/FeatureSplit/feature1/Android.mk @@ -26,6 +26,6 @@ LOCAL_APK_LIBRARIES := FeatureSplitBase LOCAL_RES_LIBRARIES := FeatureSplitBase LOCAL_AAPT_FLAGS += --package-id 0x80 -LOCAL_AAPT_FLAGS += --rename-manifest-package com.android.test.split.feature +LOCAL_AAPT_FLAGS += --custom-package com.android.test.split.feature.one include $(BUILD_PACKAGE) diff --git a/tests/FeatureSplit/feature1/AndroidManifest.xml b/tests/FeatureSplit/feature1/AndroidManifest.xml index 4e7d15102a6e0..b87361faac627 100644 --- a/tests/FeatureSplit/feature1/AndroidManifest.xml +++ b/tests/FeatureSplit/feature1/AndroidManifest.xml @@ -15,13 +15,13 @@ --> - + diff --git a/tests/FeatureSplit/feature1/res/values/values.xml b/tests/FeatureSplit/feature1/res/values/values.xml index 0e3e73c324807..7d58865b546ea 100644 --- a/tests/FeatureSplit/feature1/res/values/values.xml +++ b/tests/FeatureSplit/feature1/res/values/values.xml @@ -20,7 +20,7 @@ 200 #00ff00 - @*com.android.test.split.feature:string/app_title + @string/app_title diff --git a/tests/FeatureSplit/feature2/Android.mk b/tests/FeatureSplit/feature2/Android.mk index e2fd903845380..c8e860942fa3a 100644 --- a/tests/FeatureSplit/feature2/Android.mk +++ b/tests/FeatureSplit/feature2/Android.mk @@ -26,6 +26,6 @@ LOCAL_APK_LIBRARIES := FeatureSplitBase LOCAL_RES_LIBRARIES := FeatureSplitBase LOCAL_AAPT_FLAGS += --package-id 0x81 -LOCAL_AAPT_FLAGS += --rename-manifest-package com.android.test.split.feature +LOCAL_AAPT_FLAGS += --custom-package com.android.test.split.feature.two include $(BUILD_PACKAGE) diff --git a/tests/FeatureSplit/feature2/AndroidManifest.xml b/tests/FeatureSplit/feature2/AndroidManifest.xml index bfe6f38201bd9..abd0b5eb6933d 100644 --- a/tests/FeatureSplit/feature2/AndroidManifest.xml +++ b/tests/FeatureSplit/feature2/AndroidManifest.xml @@ -15,7 +15,7 @@ --> diff --git a/tests/FeatureSplit/feature2/res/values/values.xml b/tests/FeatureSplit/feature2/res/values/values.xml index 2fa6f907b8ab3..af5ed1b79b26d 100644 --- a/tests/FeatureSplit/feature2/res/values/values.xml +++ b/tests/FeatureSplit/feature2/res/values/values.xml @@ -18,7 +18,7 @@ 300 #0000ff - @*com.android.test.split.feature:string/app_title + @string/app_title diff --git a/tools/aapt2/ResourceValues.cpp b/tools/aapt2/ResourceValues.cpp index b38d259d6b25b..a782cd3672d19 100644 --- a/tools/aapt2/ResourceValues.cpp +++ b/tools/aapt2/ResourceValues.cpp @@ -118,7 +118,7 @@ bool Reference::Equals(const Value* value) const { bool Reference::Flatten(android::Res_value* out_value) const { const ResourceId resid = id.value_or_default(ResourceId(0)); const bool dynamic = resid.is_valid_dynamic() && resid.package_id() != kFrameworkPackageId && - resid.package_id() != kAppPackageId; + resid.package_id() < kAppPackageId; if (reference_type == Reference::Type::kResource) { if (dynamic) { diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp index b372923e40100..66b5a7af36779 100644 --- a/tools/aapt2/cmd/Link.cpp +++ b/tools/aapt2/cmd/Link.cpp @@ -795,6 +795,9 @@ class LinkCommand { for (auto& entry : asset_source->GetAssignedPackageIds()) { if (entry.first > kFrameworkPackageId && entry.first < kAppPackageId) { final_table_.included_packages_[entry.first] = entry.second; + } else if (entry.first == kAppPackageId) { + // Capture the included base feature package. + included_feature_base_ = entry.second; } } @@ -1465,11 +1468,48 @@ class LinkCommand { return false; } - if (!FlattenTable(table, options_.output_format, writer)) { - context_->GetDiagnostics()->Error(DiagMessage() << "failed to write resource table"); - return false; + // Hack to fix b/68820737. + // We need to modify the ResourceTable's package name, but that should NOT affect + // anything else being generated, which includes the Java classes. + // If required, the package name is modifed before flattening, and then modified back + // to its original name. + ResourceTablePackage* package_to_rewrite = nullptr; + if (context_->GetPackageId() > kAppPackageId && + included_feature_base_ == make_value(context_->GetCompilationPackage())) { + // The base APK is included, and this is a feature split. If the base package is + // the same as this package, then we are building an old style Android Instant Apps feature + // split and must apply this workaround to avoid requiring namespaces support. + package_to_rewrite = table->FindPackage(context_->GetCompilationPackage()); + if (package_to_rewrite != nullptr) { + CHECK_EQ(1u, table->packages.size()) << "can't change name of package when > 1 package"; + + std::string new_package_name = + StringPrintf("%s.%s", package_to_rewrite->name.c_str(), + app_info_.split_name.value_or_default("feature").c_str()); + + if (context_->IsVerbose()) { + context_->GetDiagnostics()->Note( + DiagMessage() << "rewriting resource package name for feature split to '" + << new_package_name << "'"); + } + package_to_rewrite->name = new_package_name; + } } - return true; + + bool success = FlattenTable(table, options_.output_format, writer); + + if (package_to_rewrite != nullptr) { + // Change the name back. + package_to_rewrite->name = context_->GetCompilationPackage(); + if (package_to_rewrite->id) { + table->included_packages_.erase(package_to_rewrite->id.value()); + } + } + + if (!success) { + context_->GetDiagnostics()->Error(DiagMessage() << "failed to write resource table"); + } + return success; } int Run(const std::vector& input_files) { @@ -1498,8 +1538,8 @@ class LinkCommand { return 1; } - const AppInfo& app_info = maybe_app_info.value(); - context_->SetMinSdkVersion(app_info.min_sdk_version.value_or_default(0)); + app_info_ = maybe_app_info.value(); + context_->SetMinSdkVersion(app_info_.min_sdk_version.value_or_default(0)); context_->SetNameManglerPolicy(NameManglerPolicy{context_->GetCompilationPackage()}); @@ -1699,7 +1739,7 @@ class LinkCommand { // Generate an AndroidManifest.xml for each split. std::unique_ptr split_manifest = - GenerateSplitManifest(app_info, *split_constraints_iter); + GenerateSplitManifest(app_info_, *split_constraints_iter); XmlReferenceLinker linker; if (!linker.Consume(context_, split_manifest.get())) { @@ -1799,6 +1839,8 @@ class LinkCommand { LinkContext* context_; ResourceTable final_table_; + AppInfo app_info_; + std::unique_ptr table_merger_; // A pointer to the FileCollection representing the filesystem (not archives). @@ -1816,6 +1858,9 @@ class LinkCommand { // The set of shared libraries being used, mapping their assigned package ID to package name. std::map shared_libs_; + + // The package name of the base application, if it is included. + Maybe included_feature_base_; }; int Link(const std::vector& args, IDiagnostics* diagnostics) { diff --git a/tools/aapt2/format/binary/TableFlattener.cpp b/tools/aapt2/format/binary/TableFlattener.cpp index 4a1b46c99d4bd..2a51df3912e30 100644 --- a/tools/aapt2/format/binary/TableFlattener.cpp +++ b/tools/aapt2/format/binary/TableFlattener.cpp @@ -23,6 +23,7 @@ #include "android-base/logging.h" #include "android-base/macros.h" +#include "android-base/stringprintf.h" #include "ResourceTable.h" #include "ResourceValues.h" @@ -579,7 +580,15 @@ bool TableFlattener::Consume(IAaptContext* context, ResourceTable* table) { // Write a self mapping entry for this package if the ID is non-standard (0x7f). const uint8_t package_id = package->id.value(); if (package_id != kFrameworkPackageId && package_id != kAppPackageId) { - table->included_packages_[package_id] = package->name; + auto result = table->included_packages_.insert({package_id, package->name}); + if (!result.second && result.first->second != package->name) { + // A mapping for this package ID already exists, and is a different package. Error! + context->GetDiagnostics()->Error( + DiagMessage() << android::base::StringPrintf( + "can't map package ID %02x to '%s'. Already mapped to '%s'", package_id, + package->name.c_str(), result.first->second.c_str())); + return false; + } } }