diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp index 1e879a0e5f498..e1f9642d27d72 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 @@ static uint32_t parseFormatAttribute(const StringPiece16& str) { return mask; } -static bool shouldStripResource(const xml::XmlPullParser* parser, - const Maybe productToMatch) { - assert(parser->getEvent() == xml::XmlPullParser::Event::kStartElement); - - if (Maybe 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 @@ struct ParsedResource { std::list childResources; }; +bool ResourceParser::shouldStripResource(const ResourceNameRef& name, + const Maybe& 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 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 @@ bool ResourceParser::parseResources(xml::XmlPullParser* parser) { 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 product; + if (Maybe 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 29b1bc188b56e..9ad749e27dbce 100644 --- a/tools/aapt2/ResourceParser.h +++ b/tools/aapt2/ResourceParser.h @@ -34,11 +34,11 @@ struct ParsedResource; 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 product; + std::vector products; /** * Whether the default setting for this parser is to allow translation. @@ -101,6 +101,9 @@ private: bool parseArrayImpl(xml::XmlPullParser* parser, ParsedResource* outResource, uint32_t typeMask); bool parsePlural(xml::XmlPullParser* parser, ParsedResource* outResource); + bool shouldStripResource(const ResourceNameRef& name, + const Maybe& product) const; + IDiagnostics* mDiag; ResourceTable* mTable; Source mSource; diff --git a/tools/aapt2/ResourceParser_test.cpp b/tools/aapt2/ResourceParser_test.cpp index ab44a064a9533..8d10ba14924b3 100644 --- a/tools/aapt2/ResourceParser_test.cpp +++ b/tools/aapt2/ResourceParser_test.cpp @@ -48,11 +48,11 @@ struct ResourceParserTest : public ::testing::Test { } ::testing::AssertionResult testParse(const StringPiece& str, - Maybe product = {}) { + std::initializer_list products = {}) { std::stringstream input(kXmlPreamble); input << "\n" << str << "\n" << 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, ParsePublicIdAsDefinition) { } TEST_F(ResourceParserTest, FilterProductsThatDontMatch) { - std::string input = "hi\n" - "ho\n" - "wee\n" - "woo\n"; - ASSERT_TRUE(testParse(input, std::u16string(u"no-sdcard"))); + std::string input = R"EOF( + hi + ho + wee + woo + hoot + yes + )EOF"; + ASSERT_TRUE(testParse(input, { std::u16string(u"no-sdcard"), std::u16string(u"phablet") })); String* fooStr = test::getValue(&mTable, u"@string/foo"); ASSERT_NE(nullptr, fooStr); @@ -526,11 +530,25 @@ TEST_F(ResourceParserTest, FilterProductsThatDontMatch) { EXPECT_NE(nullptr, test::getValue(&mTable, u"@string/bar")); EXPECT_NE(nullptr, test::getValue(&mTable, u"@string/baz")); + EXPECT_NE(nullptr, test::getValue(&mTable, u"@string/bit")); + EXPECT_NE(nullptr, test::getValue(&mTable, u"@string/bot")); +} + +TEST_F(ResourceParserTest, FilterProductsThatBothMatchInOrder) { + std::string input = R"EOF( + phone + default + )EOF"; + ASSERT_TRUE(testParse(input, { std::u16string(u"phone") })); + + String* foo = test::getValue(&mTable, u"@string/foo"); + ASSERT_NE(nullptr, foo); + EXPECT_EQ(std::u16string(u"phone"), *foo->value); } TEST_F(ResourceParserTest, FailWhenProductFilterStripsOutAllVersionsOfResource) { std::string input = "hello\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 967e2363ffc07..b3b0f65e54da9 100644 --- a/tools/aapt2/compile/Compile.cpp +++ b/tools/aapt2/compile/Compile.cpp @@ -105,7 +105,7 @@ static Maybe extractResourcePathData(const std::string& path, struct CompileOptions { std::string outputPath; Maybe resDir; - Maybe product; + std::vector products; bool pseudolocalize = false; bool verbose = false; }; @@ -191,7 +191,7 @@ static bool compileTable(IAaptContext* context, const CompileOptions& options, 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 @@ public: int compile(const std::vector& args) { CompileOptions options; - Maybe product; + Maybe 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 @@ int compile(const std::vector& args) { return 1; } - if (product) { - options.product = util::utf8ToUtf16(product.value()); + if (productList) { + for (StringPiece part : util::tokenize(productList.value(), ',')) { + options.products.push_back(util::utf8ToUtf16(part)); + } } CompileContext context;