diff options
| -rw-r--r-- | tools/aapt2/configuration/ConfigurationParser.cpp | 20 | ||||
| -rw-r--r-- | tools/aapt2/configuration/ConfigurationParser.h | 16 | ||||
| -rw-r--r-- | tools/aapt2/configuration/ConfigurationParser_test.cpp | 122 | ||||
| -rw-r--r-- | tools/aapt2/configuration/aapt2.xsd | 1 | ||||
| -rw-r--r-- | tools/aapt2/optimize/MultiApkGenerator.cpp | 109 | ||||
| -rw-r--r-- | tools/aapt2/optimize/MultiApkGenerator.h | 4 |
6 files changed, 208 insertions, 64 deletions
diff --git a/tools/aapt2/configuration/ConfigurationParser.cpp b/tools/aapt2/configuration/ConfigurationParser.cpp index d2b4d725c416..a79a577c663b 100644 --- a/tools/aapt2/configuration/ConfigurationParser.cpp +++ b/tools/aapt2/configuration/ConfigurationParser.cpp @@ -330,15 +330,32 @@ Maybe<PostProcessingConfiguration> ConfigurationParser::Parse() { // TODO: Validate all references in the configuration are valid. It should be safe to assume from // this point on that any references from one section to another will be present. + // TODO: Automatically arrange artifacts so that they match Play Store multi-APK requirements. + // see: https://developer.android.com/google/play/publishing/multiple-apks.html + // + // For now, make sure the version codes are unique. + std::vector<Artifact>& artifacts = config.artifacts; + std::sort(artifacts.begin(), artifacts.end()); + if (std::adjacent_find(artifacts.begin(), artifacts.end()) != artifacts.end()) { + diag_->Error(DiagMessage() << "Configuration has duplicate versions"); + return {}; + } + return {config}; } ConfigurationParser::ActionHandler ConfigurationParser::artifact_handler_ = [](PostProcessingConfiguration* config, Element* root_element, IDiagnostics* diag) -> bool { + // This will be incremented later so the first version will always be different to the base APK. + int current_version = (config->artifacts.empty()) ? 0 : config->artifacts.back().version; + Artifact artifact{}; + Maybe<int> version; for (const auto& attr : root_element->attributes) { if (attr.name == "name") { artifact.name = attr.value; + } else if (attr.name == "version") { + version = std::stoi(attr.value); } else if (attr.name == "abi-group") { artifact.abi_group = {attr.value}; } else if (attr.name == "screen-density-group") { @@ -356,6 +373,9 @@ ConfigurationParser::ActionHandler ConfigurationParser::artifact_handler_ = << attr.value); } } + + artifact.version = (version) ? version.value() : current_version + 1; + config->artifacts.push_back(artifact); return true; }; diff --git a/tools/aapt2/configuration/ConfigurationParser.h b/tools/aapt2/configuration/ConfigurationParser.h index ebf1c986a980..c5d3284c33f4 100644 --- a/tools/aapt2/configuration/ConfigurationParser.h +++ b/tools/aapt2/configuration/ConfigurationParser.h @@ -17,6 +17,7 @@ #ifndef AAPT2_CONFIGURATION_H #define AAPT2_CONFIGURATION_H +#include <set> #include <string> #include <unordered_map> #include <vector> @@ -41,6 +42,12 @@ using Entry = std::unordered_map<std::string, T>; struct Artifact { /** Name to use for output of processing foo.apk -> foo.<name>.apk. */ Maybe<std::string> name; + /** + * Value to add to the base Android manifest versionCode. If it is not present in the + * configuration file, it is set to the previous artifact + 1. If the first artifact does not have + * a value, artifacts are a 1 based index. + */ + int version; /** If present, uses the ABI group with this name. */ Maybe<std::string> abi_group; /** If present, uses the screen density group with this name. */ @@ -60,6 +67,15 @@ struct Artifact { /** Convert an artifact name template into a name string based on configuration contents. */ Maybe<std::string> Name(const android::StringPiece& apk_name, IDiagnostics* diag) const; + + bool operator<(const Artifact& rhs) const { + // TODO(safarmer): Order by play store multi-APK requirements. + return version < rhs.version; + } + + bool operator==(const Artifact& rhs) const { + return version == rhs.version; + } }; /** Enumeration of currently supported ABIs. */ diff --git a/tools/aapt2/configuration/ConfigurationParser_test.cpp b/tools/aapt2/configuration/ConfigurationParser_test.cpp index 6bb168f15b1e..3654901e3e02 100644 --- a/tools/aapt2/configuration/ConfigurationParser_test.cpp +++ b/tools/aapt2/configuration/ConfigurationParser_test.cpp @@ -183,55 +183,117 @@ TEST_F(ConfigurationParserTest, InvalidNamespace) { } TEST_F(ConfigurationParserTest, ArtifactAction) { - static constexpr const char* xml = R"xml( + PostProcessingConfiguration config; + { + const auto doc = test::BuildXmlDom(R"xml( + <artifact + abi-group="arm" + screen-density-group="large" + locale-group="europe" + android-sdk-group="v19" + gl-texture-group="dxt1" + device-feature-group="low-latency"/>)xml"); + + ASSERT_TRUE(artifact_handler_(&config, NodeCast<Element>(doc->root.get()), &diag_)); + + EXPECT_EQ(1ul, config.artifacts.size()); + + auto& artifact = config.artifacts.back(); + EXPECT_FALSE(artifact.name); // TODO: make this fail. + EXPECT_EQ(1, artifact.version); + EXPECT_EQ("arm", artifact.abi_group.value()); + EXPECT_EQ("large", artifact.screen_density_group.value()); + EXPECT_EQ("europe", artifact.locale_group.value()); + EXPECT_EQ("v19", artifact.android_sdk_group.value()); + EXPECT_EQ("dxt1", artifact.gl_texture_group.value()); + EXPECT_EQ("low-latency", artifact.device_feature_group.value()); + } + + { + // Perform a second action to ensure we get 2 artifacts. + const auto doc = test::BuildXmlDom(R"xml( + <artifact + abi-group="other" + screen-density-group="large" + locale-group="europe" + android-sdk-group="v19" + gl-texture-group="dxt1" + device-feature-group="low-latency"/>)xml"); + + ASSERT_TRUE(artifact_handler_(&config, NodeCast<Element>(doc.get()->root.get()), &diag_)); + EXPECT_EQ(2ul, config.artifacts.size()); + EXPECT_EQ(2, config.artifacts.back().version); + } + + { + // Perform a third action with a set version code. + const auto doc = test::BuildXmlDom(R"xml( <artifact - abi-group="arm" + version="5" + abi-group="other" screen-density-group="large" locale-group="europe" android-sdk-group="v19" gl-texture-group="dxt1" - device-feature-group="low-latency"/>)xml"; + device-feature-group="low-latency"/>)xml"); - auto doc = test::BuildXmlDom(xml); - - PostProcessingConfiguration config; - bool ok = artifact_handler_(&config, NodeCast<Element>(doc->root.get()), &diag_); - ASSERT_TRUE(ok); - - EXPECT_EQ(1ul, config.artifacts.size()); - - auto& artifact = config.artifacts.front(); - EXPECT_FALSE(artifact.name); // TODO: make this fail. - EXPECT_EQ("arm", artifact.abi_group.value()); - EXPECT_EQ("large", artifact.screen_density_group.value()); - EXPECT_EQ("europe", artifact.locale_group.value()); - EXPECT_EQ("v19", artifact.android_sdk_group.value()); - EXPECT_EQ("dxt1", artifact.gl_texture_group.value()); - EXPECT_EQ("low-latency", artifact.device_feature_group.value()); + ASSERT_TRUE(artifact_handler_(&config, NodeCast<Element>(doc.get()->root.get()), &diag_)); + EXPECT_EQ(3ul, config.artifacts.size()); + EXPECT_EQ(5, config.artifacts.back().version); + } - // Perform a second action to ensure we get 2 artifacts. - static constexpr const char* second = R"xml( + { + // Perform a fourth action to ensure the version code still increments. + const auto doc = test::BuildXmlDom(R"xml( <artifact abi-group="other" screen-density-group="large" locale-group="europe" android-sdk-group="v19" gl-texture-group="dxt1" - device-feature-group="low-latency"/>)xml"; - doc = test::BuildXmlDom(second); + device-feature-group="low-latency"/>)xml"); - ok = artifact_handler_(&config, NodeCast<Element>(doc.get()->root.get()), &diag_); - ASSERT_TRUE(ok); - EXPECT_EQ(2ul, config.artifacts.size()); + ASSERT_TRUE(artifact_handler_(&config, NodeCast<Element>(doc.get()->root.get()), &diag_)); + EXPECT_EQ(4ul, config.artifacts.size()); + EXPECT_EQ(6, config.artifacts.back().version); + } +} + +TEST_F(ConfigurationParserTest, DuplicateArtifactVersion) { + static constexpr const char* configuration = R"xml(<?xml version="1.0" encoding="utf-8" ?> + <pst-process xmlns="http://schemas.android.com/tools/aapt">> + <artifacts> + <artifact-format> + ${base}.${abi}.${screen-density}.${locale}.${sdk}.${gl}.${feature}.release + </artifact-format> + <artifact + name="art1" + abi-group="arm" + screen-density-group="large" + locale-group="europe" + android-sdk-group="v19" + gl-texture-group="dxt1" + device-feature-group="low-latency"/> + <artifact + name="art2" + version = "1" + abi-group="other" + screen-density-group="alldpi" + locale-group="north-america" + android-sdk-group="v19" + gl-texture-group="dxt1" + device-feature-group="low-latency"/> + </artifacts> + </post-process>)xml"; + auto result = ConfigurationParser::ForContents(configuration).Parse(); + ASSERT_FALSE(result); } TEST_F(ConfigurationParserTest, ArtifactFormatAction) { - static constexpr const char* xml = R"xml( + const auto doc = test::BuildXmlDom(R"xml( <artifact-format> ${base}.${abi}.${screen-density}.${locale}.${sdk}.${gl}.${feature}.release - </artifact-format>)xml"; - - auto doc = test::BuildXmlDom(xml); + </artifact-format>)xml"); PostProcessingConfiguration config; bool ok = artifact_format_handler_(&config, NodeCast<Element>(doc.get()->root.get()), &diag_); diff --git a/tools/aapt2/configuration/aapt2.xsd b/tools/aapt2/configuration/aapt2.xsd index 47bf99e28089..134153a017f8 100644 --- a/tools/aapt2/configuration/aapt2.xsd +++ b/tools/aapt2/configuration/aapt2.xsd @@ -39,6 +39,7 @@ <!-- Groups output artifacts together by dimension labels. --> <xsd:complexType name="artifact"> <xsd:attribute name="name" type="xsd:string"/> + <xsd:attribute name="version" type="xsd:integer"/> <xsd:attribute name="abi-group" type="xsd:string"/> <xsd:attribute name="android-sdk-group" type="xsd:string"/> <xsd:attribute name="device-feature-group" type="xsd:string"/> diff --git a/tools/aapt2/optimize/MultiApkGenerator.cpp b/tools/aapt2/optimize/MultiApkGenerator.cpp index 563c46df26d1..fb150e204167 100644 --- a/tools/aapt2/optimize/MultiApkGenerator.cpp +++ b/tools/aapt2/optimize/MultiApkGenerator.cpp @@ -23,6 +23,7 @@ #include "LoadedApk.h" #include "ResourceUtils.h" +#include "ValueVisitor.h" #include "configuration/ConfigurationParser.h" #include "filter/AbiFilter.h" #include "filter/Filter.h" @@ -33,12 +34,14 @@ #include "split/TableSplitter.h" #include "util/Files.h" #include "xml/XmlDom.h" +#include "xml/XmlUtil.h" namespace aapt { using ::aapt::configuration::AndroidSdk; using ::aapt::configuration::Artifact; using ::aapt::configuration::PostProcessingConfiguration; +using ::aapt::xml::kSchemaAndroid; using ::aapt::xml::XmlResource; using ::android::StringPiece; @@ -153,40 +156,10 @@ bool MultiApkGenerator::FromBaseApk(const MultiApkGeneratorOptions& options) { } std::unique_ptr<XmlResource> manifest; - - Maybe<AndroidSdk> maybe_sdk = GetAndroidSdk(artifact, config, diag); - if (maybe_sdk) { - // TODO(safarmer): Handle the rest of the Android SDK. - const AndroidSdk& android_sdk = maybe_sdk.value(); - - manifest = apk_->InflateManifest(context_); - if (!manifest) { - return false; - } - - // Make sure the first element is <manifest> with package attribute. - xml::Element* manifest_el = manifest->root.get(); - if (manifest_el == nullptr) { - return {}; - } - - if (!manifest_el->namespace_uri.empty() || manifest_el->name != "manifest") { - diag->Error(DiagMessage(manifest->file.source) << "root tag must be <manifest>"); - return {}; - } - - if (xml::Element* uses_sdk_el = manifest_el->FindChild({}, "uses-sdk")) { - if (xml::Attribute* min_sdk_attr = - uses_sdk_el->FindAttribute(xml::kSchemaAndroid, "minSdkVersion")) { - if (min_sdk_attr == nullptr) { - diag->Error(DiagMessage(manifest->file.source.WithLine(uses_sdk_el->line_number)) - << "missing android:minSdkVersion"); - return {}; - } - const std::string& min_sdk_str = std::to_string(android_sdk.min_sdk_version.value()); - min_sdk_attr->compiled_value = ResourceUtils::TryParseInt(min_sdk_str); - } - } + if (!UpdateManifest(artifact, config, &manifest, diag)) { + diag->Error(DiagMessage() << "could not update AndroidManifest.xml for " + << artifact_name.value()); + return false; } std::string out = options.out_dir; @@ -288,4 +261,72 @@ std::unique_ptr<ResourceTable> MultiApkGenerator::FilterTable( return table; } +bool MultiApkGenerator::UpdateManifest(const Artifact& artifact, + const PostProcessingConfiguration& config, + std::unique_ptr<XmlResource>* updated_manifest, + IDiagnostics* diag) { + *updated_manifest = apk_->InflateManifest(context_); + XmlResource* manifest = updated_manifest->get(); + if (manifest == nullptr) { + return false; + } + + // Make sure the first element is <manifest> with package attribute. + xml::Element* manifest_el = manifest->root.get(); + if (manifest_el == nullptr) { + return false; + } + + if (!manifest_el->namespace_uri.empty() || manifest_el->name != "manifest") { + diag->Error(DiagMessage(manifest->file.source) << "root tag must be <manifest>"); + return false; + } + + // Update the versionCode attribute. + xml::Attribute* versionCode = manifest_el->FindAttribute(kSchemaAndroid, "versionCode"); + if (versionCode == nullptr) { + diag->Error(DiagMessage(manifest->file.source) << "manifest must have a versionCode attribute"); + return false; + } + + auto* compiled_version = ValueCast<BinaryPrimitive>(versionCode->compiled_value.get()); + if (compiled_version == nullptr) { + diag->Error(DiagMessage(manifest->file.source) << "versionCode is invalid"); + return false; + } + + int new_version = compiled_version->value.data + artifact.version; + versionCode->compiled_value = ResourceUtils::TryParseInt(std::to_string(new_version)); + + // Check to see if the minSdkVersion needs to be updated. + Maybe<AndroidSdk> maybe_sdk = GetAndroidSdk(artifact, config, diag); + if (maybe_sdk) { + // TODO(safarmer): Handle the rest of the Android SDK. + const AndroidSdk& android_sdk = maybe_sdk.value(); + + if (xml::Element* uses_sdk_el = manifest_el->FindChild({}, "uses-sdk")) { + if (xml::Attribute* min_sdk_attr = + uses_sdk_el->FindAttribute(xml::kSchemaAndroid, "minSdkVersion")) { + // Populate with a pre-compiles attribute to we don't need to relink etc. + const std::string& min_sdk_str = std::to_string(android_sdk.min_sdk_version.value()); + min_sdk_attr->compiled_value = ResourceUtils::TryParseInt(min_sdk_str); + } else { + // There was no minSdkVersion. This is strange since at this point we should have been + // through the manifest fixer which sets the default minSdkVersion. + diag->Error(DiagMessage(manifest->file.source) << "missing minSdkVersion from <uses-sdk>"); + return false; + } + } else { + // No uses-sdk present. This is strange since at this point we should have been + // through the manifest fixer which should have added it. + diag->Error(DiagMessage(manifest->file.source) << "missing <uses-sdk> from <manifest>"); + return false; + } + } + + // TODO(safarmer): Check if we changed the supported screens and update the manifest. + + return true; +} + } // namespace aapt diff --git a/tools/aapt2/optimize/MultiApkGenerator.h b/tools/aapt2/optimize/MultiApkGenerator.h index b06440014be6..e6546eec584e 100644 --- a/tools/aapt2/optimize/MultiApkGenerator.h +++ b/tools/aapt2/optimize/MultiApkGenerator.h @@ -54,6 +54,10 @@ class MultiApkGenerator { return context_->GetDiagnostics(); } + bool UpdateManifest(const configuration::Artifact& artifact, + const configuration::PostProcessingConfiguration& config, + std::unique_ptr<xml::XmlResource>* updated_manifest, IDiagnostics* diag); + LoadedApk* apk_; IAaptContext* context_; }; |