AAPT2: Push more configuration code into the parser
When parsing is complete, we now have a list of output artifacts that
have their referential integrity validated. This means that once the
configuration file is parsed, the only errors that can occur are related
to APK processing, and not the configuration itself.
This reduces the number of errors that could cause a partial output of
APK artifacts. It simplifies the public API and reduces the complexity of
the code to generate multiple APKs.
Test: Ran unit tests
Test: manually ran the optimize command to ensure it still works
Change-Id: I3f2d885b207a84c958f5348a4baa6718598184a4
diff --git a/tools/aapt2/optimize/MultiApkGenerator.cpp b/tools/aapt2/optimize/MultiApkGenerator.cpp
index e2d738a..16898d6 100644
--- a/tools/aapt2/optimize/MultiApkGenerator.cpp
+++ b/tools/aapt2/optimize/MultiApkGenerator.cpp
@@ -40,33 +40,11 @@
namespace aapt {
using ::aapt::configuration::AndroidSdk;
-using ::aapt::configuration::Artifact;
-using ::aapt::configuration::PostProcessingConfiguration;
+using ::aapt::configuration::OutputArtifact;
using ::aapt::xml::kSchemaAndroid;
using ::aapt::xml::XmlResource;
using ::android::StringPiece;
-namespace {
-
-Maybe<AndroidSdk> GetAndroidSdk(const Artifact& artifact, const PostProcessingConfiguration& config,
- IDiagnostics* diag) {
- if (!artifact.android_sdk_group) {
- return {};
- }
-
- const std::string& group_name = artifact.android_sdk_group.value();
- auto group = config.android_sdk_groups.find(group_name);
- // TODO: Remove validation when configuration parser ensures referential integrity.
- if (group == config.android_sdk_groups.end()) {
- diag->Error(DiagMessage() << "could not find referenced group '" << group_name << "'");
- return {};
- }
-
- return group->second;
-}
-
-} // namespace
-
/**
* Context wrapper that allows the min Android SDK value to be overridden.
*/
@@ -115,8 +93,9 @@
min_sdk_ = min_sdk;
}
- void SetSource(const Source& source) {
- source_diag_ = util::make_unique<SourcePathDiagnostics>(source, context_->GetDiagnostics());
+ void SetSource(const std::string& source) {
+ source_diag_ =
+ util::make_unique<SourcePathDiagnostics>(Source{source}, context_->GetDiagnostics());
}
private:
@@ -142,82 +121,60 @@
bool MultiApkGenerator::FromBaseApk(const MultiApkGeneratorOptions& options) {
// TODO(safarmer): Handle APK version codes for the generated APKs.
- const PostProcessingConfiguration& config = options.config;
-
- const std::string& apk_name = file::GetFilename(apk_->GetSource().path).to_string();
- const StringPiece ext = file::GetExtension(apk_name);
- const std::string base_name = apk_name.substr(0, apk_name.rfind(ext.to_string()));
std::unordered_set<std::string> artifacts_to_keep = options.kept_artifacts;
std::unordered_set<std::string> filtered_artifacts;
std::unordered_set<std::string> kept_artifacts;
// For now, just write out the stripped APK since ABI splitting doesn't modify anything else.
- for (const Artifact& artifact : config.artifacts) {
- SourcePathDiagnostics diag{{apk_name}, context_->GetDiagnostics()};
-
+ for (const OutputArtifact& artifact : options.apk_artifacts) {
FilterChain filters;
- if (!artifact.name && !config.artifact_format) {
- diag.Error(
- DiagMessage() << "Artifact does not have a name and no global name template defined");
- return false;
- }
-
- Maybe<std::string> maybe_artifact_name =
- (artifact.name) ? artifact.Name(apk_name, &diag)
- : artifact.ToArtifactName(config.artifact_format.value(), apk_name, &diag);
-
- if (!maybe_artifact_name) {
- diag.Error(DiagMessage() << "Could not determine split APK artifact name");
- return false;
- }
-
- const std::string& artifact_name = maybe_artifact_name.value();
-
ContextWrapper wrapped_context{context_};
- wrapped_context.SetSource({artifact_name});
+ wrapped_context.SetSource(artifact.name);
if (!options.kept_artifacts.empty()) {
- const auto& it = artifacts_to_keep.find(artifact_name);
+ const auto& it = artifacts_to_keep.find(artifact.name);
if (it == artifacts_to_keep.end()) {
- filtered_artifacts.insert(artifact_name);
+ filtered_artifacts.insert(artifact.name);
if (context_->IsVerbose()) {
- context_->GetDiagnostics()->Note(DiagMessage(artifact_name) << "skipping artifact");
+ context_->GetDiagnostics()->Note(DiagMessage(artifact.name) << "skipping artifact");
}
continue;
} else {
artifacts_to_keep.erase(it);
- kept_artifacts.insert(artifact_name);
+ kept_artifacts.insert(artifact.name);
}
}
std::unique_ptr<ResourceTable> table =
- FilterTable(artifact, config, *apk_->GetResourceTable(), &wrapped_context, &filters);
+ FilterTable(context_, artifact, *apk_->GetResourceTable(), &filters);
if (!table) {
return false;
}
+ IDiagnostics* diag = wrapped_context.GetDiagnostics();
+
std::unique_ptr<XmlResource> manifest;
- if (!UpdateManifest(artifact, config, &manifest, &diag)) {
- diag.Error(DiagMessage() << "could not update AndroidManifest.xml for output artifact");
+ if (!UpdateManifest(artifact, &manifest, diag)) {
+ diag->Error(DiagMessage() << "could not update AndroidManifest.xml for output artifact");
return false;
}
std::string out = options.out_dir;
if (!file::mkdirs(out)) {
- diag.Warn(DiagMessage() << "could not create out dir: " << out);
+ diag->Warn(DiagMessage() << "could not create out dir: " << out);
}
- file::AppendPath(&out, artifact_name);
+ file::AppendPath(&out, artifact.name);
if (context_->IsVerbose()) {
- diag.Note(DiagMessage() << "Generating split: " << out);
+ diag->Note(DiagMessage() << "Generating split: " << out);
}
- std::unique_ptr<IArchiveWriter> writer = CreateZipFileArchiveWriter(&diag, out);
+ std::unique_ptr<IArchiveWriter> writer = CreateZipFileArchiveWriter(diag, out);
if (context_->IsVerbose()) {
- diag.Note(DiagMessage() << "Writing output: " << out);
+ diag->Note(DiagMessage() << "Writing output: " << out);
}
filters.AddFilter(util::make_unique<SignatureFilter>());
@@ -254,64 +211,34 @@
return true;
}
-std::unique_ptr<ResourceTable> MultiApkGenerator::FilterTable(
- const configuration::Artifact& artifact,
- const configuration::PostProcessingConfiguration& config, const ResourceTable& old_table,
- IAaptContext* context, FilterChain* filters) {
+std::unique_ptr<ResourceTable> MultiApkGenerator::FilterTable(IAaptContext* context,
+ const OutputArtifact& artifact,
+ const ResourceTable& old_table,
+ FilterChain* filters) {
TableSplitterOptions splits;
AxisConfigFilter axis_filter;
ContextWrapper wrapped_context{context};
+ wrapped_context.SetSource(artifact.name);
- if (artifact.abi_group) {
- const std::string& group_name = artifact.abi_group.value();
-
- auto group = config.abi_groups.find(group_name);
- // TODO: Remove validation when configuration parser ensures referential integrity.
- if (group == config.abi_groups.end()) {
- context->GetDiagnostics()->Error(DiagMessage() << "could not find referenced ABI group '"
- << group_name << "'");
- return {};
- }
- filters->AddFilter(AbiFilter::FromAbiList(group->second));
+ if (!artifact.abis.empty()) {
+ filters->AddFilter(AbiFilter::FromAbiList(artifact.abis));
}
- if (artifact.screen_density_group) {
- const std::string& group_name = artifact.screen_density_group.value();
-
- auto group = config.screen_density_groups.find(group_name);
- // TODO: Remove validation when configuration parser ensures referential integrity.
- if (group == config.screen_density_groups.end()) {
- context->GetDiagnostics()->Error(DiagMessage()
- << "could not find referenced group '" << group_name << "'");
- return {};
- }
-
- const std::vector<ConfigDescription>& densities = group->second;
- for (const auto& density_config : densities) {
+ if (!artifact.screen_densities.empty()) {
+ for (const auto& density_config : artifact.screen_densities) {
splits.preferred_densities.push_back(density_config.density);
}
}
- if (artifact.locale_group) {
- const std::string& group_name = artifact.locale_group.value();
- auto group = config.locale_groups.find(group_name);
- // TODO: Remove validation when configuration parser ensures referential integrity.
- if (group == config.locale_groups.end()) {
- context->GetDiagnostics()->Error(DiagMessage()
- << "could not find referenced group '" << group_name << "'");
- return {};
- }
-
- const std::vector<ConfigDescription>& locales = group->second;
- for (const auto& locale : locales) {
+ if (!artifact.locales.empty()) {
+ for (const auto& locale : artifact.locales) {
axis_filter.AddConfig(locale);
}
splits.config_filter = &axis_filter;
}
- Maybe<AndroidSdk> sdk = GetAndroidSdk(artifact, config, context->GetDiagnostics());
- if (sdk && sdk.value().min_sdk_version) {
- wrapped_context.SetMinSdkVersion(sdk.value().min_sdk_version.value());
+ if (artifact.android_sdk && artifact.android_sdk.value().min_sdk_version) {
+ wrapped_context.SetMinSdkVersion(artifact.android_sdk.value().min_sdk_version.value());
}
std::unique_ptr<ResourceTable> table = old_table.Clone();
@@ -327,8 +254,7 @@
return table;
}
-bool MultiApkGenerator::UpdateManifest(const Artifact& artifact,
- const PostProcessingConfiguration& config,
+bool MultiApkGenerator::UpdateManifest(const OutputArtifact& artifact,
std::unique_ptr<XmlResource>* updated_manifest,
IDiagnostics* diag) {
const xml::XmlResource* apk_manifest = apk_->GetManifest();
@@ -367,10 +293,9 @@
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) {
+ if (artifact.android_sdk) {
// TODO(safarmer): Handle the rest of the Android SDK.
- const AndroidSdk& android_sdk = maybe_sdk.value();
+ const AndroidSdk& android_sdk = artifact.android_sdk.value();
if (xml::Element* uses_sdk_el = manifest_el->FindChild({}, "uses-sdk")) {
if (xml::Attribute* min_sdk_attr =
@@ -392,10 +317,7 @@
}
}
- if (artifact.screen_density_group) {
- auto densities = config.screen_density_groups.find(artifact.screen_density_group.value());
- CHECK(densities != config.screen_density_groups.end()) << "Missing density group";
-
+ if (!artifact.screen_densities.empty()) {
xml::Element* screens_el = manifest_el->FindChild({}, "compatible-screens");
if (!screens_el) {
// create a new element.
@@ -408,7 +330,7 @@
screens_el->GetChildElements().clear();
}
- for (const auto& density : densities->second) {
+ for (const auto& density : artifact.screen_densities) {
std::unique_ptr<xml::Element> screen_el = util::make_unique<xml::Element>();
screen_el->name = "screen";
const char* density_str = density.toString().string();