AAPT2: Fix product support
Previously the default product wasn't tried if 'default' wasn't specified on the command line.
Also adds support for multiple products.
Change-Id: I1e4872b34bb8d609b6444841a4e7e4dbb3bbb76b
diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp
index 1e879a0..e1f9642 100644
--- a/tools/aapt2/ResourceParser.cpp
+++ b/tools/aapt2/ResourceParser.cpp
@@ -19,6 +19,7 @@
#include "ResourceUtils.h"
#include "ResourceValues.h"
#include "ValueVisitor.h"
+#include "util/Comparators.h"
#include "util/ImmutableMap.h"
#include "util/Util.h"
#include "xml/XmlPullParser.h"
@@ -64,26 +65,6 @@
return mask;
}
-static bool shouldStripResource(const xml::XmlPullParser* parser,
- const Maybe<std::u16string> productToMatch) {
- assert(parser->getEvent() == xml::XmlPullParser::Event::kStartElement);
-
- if (Maybe<StringPiece16> maybeProduct = xml::findNonEmptyAttribute(parser, u"product")) {
- if (!productToMatch) {
- if (maybeProduct.value() != u"default" && maybeProduct.value() != u"phone") {
- // We didn't specify a product and this is not a default product, so skip.
- return true;
- }
- } else {
- if (productToMatch && maybeProduct.value() != productToMatch.value()) {
- // We specified a product, but they don't match.
- return true;
- }
- }
- }
- return false;
-}
-
/**
* A parsed resource ready to be added to the ResourceTable.
*/
@@ -97,6 +78,35 @@
std::list<ParsedResource> childResources;
};
+bool ResourceParser::shouldStripResource(const ResourceNameRef& name,
+ const Maybe<std::u16string>& product) const {
+ if (product) {
+ for (const std::u16string& productToMatch : mOptions.products) {
+ if (product.value() == productToMatch) {
+ // We specified a product, and it is in the list, so don't strip.
+ return false;
+ }
+ }
+ }
+
+ // Nothing matched, try 'default'. Default only matches if we didn't already use another
+ // product variant.
+ if (!product || product.value() == u"default") {
+ if (Maybe<ResourceTable::SearchResult> result = mTable->findResource(name)) {
+ const ResourceEntry* entry = result.value().entry;
+ auto iter = std::lower_bound(entry->values.begin(), entry->values.end(), mConfig,
+ cmp::lessThanConfig);
+ if (iter != entry->values.end() && iter->config == mConfig && !iter->value->isWeak()) {
+ // We have a value for this config already, and it is not weak,
+ // so filter out this default.
+ return true;
+ }
+ }
+ return false;
+ }
+ return true;
+}
+
// Recursively adds resources to the ResourceTable.
static bool addResourcesToTable(ResourceTable* table, const ConfigDescription& config,
IDiagnostics* diag, ParsedResource* res) {
@@ -283,17 +293,20 @@
parsedResource.source = mSource.withLine(parser->getLineNumber());
parsedResource.comment = std::move(comment);
- // Check if we should skip this product. We still need to parse it for correctness.
- const bool stripResource = shouldStripResource(parser, mOptions.product);
+ // Extract the product name if it exists.
+ Maybe<std::u16string> product;
+ if (Maybe<StringPiece16> maybeProduct = xml::findNonEmptyAttribute(parser, u"product")) {
+ product = maybeProduct.value().toString();
+ }
+ // Parse the resource regardless of product.
if (!parseResource(parser, &parsedResource)) {
error = true;
continue;
}
- // We successfully parsed the resource.
-
- if (stripResource) {
+ // We successfully parsed the resource. Check if we should include it or strip it.
+ if (shouldStripResource(parsedResource.name, product)) {
// Record that we stripped out this resource name.
// We will check that at least one variant of this resource was included.
strippedResources.insert(parsedResource.name);
diff --git a/tools/aapt2/ResourceParser.h b/tools/aapt2/ResourceParser.h
index 29b1bc1..9ad749e 100644
--- a/tools/aapt2/ResourceParser.h
+++ b/tools/aapt2/ResourceParser.h
@@ -34,11 +34,11 @@
struct ResourceParserOptions {
/**
- * Optional product name by which to filter resources.
+ * Optional product names by which to filter resources.
* This is like a preprocessor definition in that we strip out resources
* that don't match before we compile them.
*/
- Maybe<std::u16string> product;
+ std::vector<std::u16string> products;
/**
* Whether the default setting for this parser is to allow translation.
@@ -101,6 +101,9 @@
bool parseArrayImpl(xml::XmlPullParser* parser, ParsedResource* outResource, uint32_t typeMask);
bool parsePlural(xml::XmlPullParser* parser, ParsedResource* outResource);
+ bool shouldStripResource(const ResourceNameRef& name,
+ const Maybe<std::u16string>& product) const;
+
IDiagnostics* mDiag;
ResourceTable* mTable;
Source mSource;
diff --git a/tools/aapt2/ResourceParser_test.cpp b/tools/aapt2/ResourceParser_test.cpp
index ab44a06..8d10ba1 100644
--- a/tools/aapt2/ResourceParser_test.cpp
+++ b/tools/aapt2/ResourceParser_test.cpp
@@ -48,11 +48,11 @@
}
::testing::AssertionResult testParse(const StringPiece& str,
- Maybe<std::u16string> product = {}) {
+ std::initializer_list<std::u16string> products = {}) {
std::stringstream input(kXmlPreamble);
input << "<resources>\n" << str << "\n</resources>" << std::endl;
ResourceParserOptions parserOptions;
- parserOptions.product = product;
+ parserOptions.products = products;
ResourceParser parser(mContext->getDiagnostics(), &mTable, Source{ "test" }, {},
parserOptions);
xml::XmlPullParser xmlParser(input);
@@ -514,11 +514,15 @@
}
TEST_F(ResourceParserTest, FilterProductsThatDontMatch) {
- std::string input = "<string name=\"foo\" product=\"phone\">hi</string>\n"
- "<string name=\"foo\" product=\"no-sdcard\">ho</string>\n"
- "<string name=\"bar\" product=\"\">wee</string>\n"
- "<string name=\"baz\">woo</string>\n";
- ASSERT_TRUE(testParse(input, std::u16string(u"no-sdcard")));
+ std::string input = R"EOF(
+ <string name="foo" product="phone">hi</string>
+ <string name="foo" product="no-sdcard">ho</string>
+ <string name="bar" product="">wee</string>
+ <string name="baz">woo</string>
+ <string name="bit" product="phablet">hoot</string>
+ <string name="bot" product="default">yes</string>
+ )EOF";
+ ASSERT_TRUE(testParse(input, { std::u16string(u"no-sdcard"), std::u16string(u"phablet") }));
String* fooStr = test::getValue<String>(&mTable, u"@string/foo");
ASSERT_NE(nullptr, fooStr);
@@ -526,11 +530,25 @@
EXPECT_NE(nullptr, test::getValue<String>(&mTable, u"@string/bar"));
EXPECT_NE(nullptr, test::getValue<String>(&mTable, u"@string/baz"));
+ EXPECT_NE(nullptr, test::getValue<String>(&mTable, u"@string/bit"));
+ EXPECT_NE(nullptr, test::getValue<String>(&mTable, u"@string/bot"));
+}
+
+TEST_F(ResourceParserTest, FilterProductsThatBothMatchInOrder) {
+ std::string input = R"EOF(
+ <string name="foo" product="phone">phone</string>
+ <string name="foo" product="default">default</string>
+ )EOF";
+ ASSERT_TRUE(testParse(input, { std::u16string(u"phone") }));
+
+ String* foo = test::getValue<String>(&mTable, u"@string/foo");
+ ASSERT_NE(nullptr, foo);
+ EXPECT_EQ(std::u16string(u"phone"), *foo->value);
}
TEST_F(ResourceParserTest, FailWhenProductFilterStripsOutAllVersionsOfResource) {
std::string input = "<string name=\"foo\" product=\"tablet\">hello</string>\n";
- ASSERT_FALSE(testParse(input, std::u16string(u"phone")));
+ ASSERT_FALSE(testParse(input, { std::u16string(u"phone") }));
}
TEST_F(ResourceParserTest, AutoIncrementIdsInPublicGroup) {
diff --git a/tools/aapt2/compile/Compile.cpp b/tools/aapt2/compile/Compile.cpp
index 967e236..b3b0f65 100644
--- a/tools/aapt2/compile/Compile.cpp
+++ b/tools/aapt2/compile/Compile.cpp
@@ -105,7 +105,7 @@
struct CompileOptions {
std::string outputPath;
Maybe<std::string> resDir;
- Maybe<std::u16string> product;
+ std::vector<std::u16string> products;
bool pseudolocalize = false;
bool verbose = false;
};
@@ -191,7 +191,7 @@
xml::XmlPullParser xmlParser(fin);
ResourceParserOptions parserOptions;
- parserOptions.product = options.product;
+ parserOptions.products = options.products;
// If the filename includes donottranslate, then the default translatable is false.
parserOptions.translatable = pathData.name.find(u"donottranslate") == std::string::npos;
@@ -430,10 +430,11 @@
int compile(const std::vector<StringPiece>& args) {
CompileOptions options;
- Maybe<std::string> product;
+ Maybe<std::string> productList;
Flags flags = Flags()
.requiredFlag("-o", "Output path", &options.outputPath)
- .optionalFlag("--product", "Product type to compile", &product)
+ .optionalFlag("--product", "Comma separated list of product types to compile",
+ &productList)
.optionalFlag("--dir", "Directory to scan for resources", &options.resDir)
.optionalSwitch("--pseudo-localize", "Generate resources for pseudo-locales "
"(en-XA and ar-XB)", &options.pseudolocalize)
@@ -442,8 +443,10 @@
return 1;
}
- if (product) {
- options.product = util::utf8ToUtf16(product.value());
+ if (productList) {
+ for (StringPiece part : util::tokenize<char>(productList.value(), ',')) {
+ options.products.push_back(util::utf8ToUtf16(part));
+ }
}
CompileContext context;