From 3d8d4a18c2bd4d697692ea30471852c90e7a3775 Mon Sep 17 00:00:00 2001 From: Jeremy Meyer Date: Fri, 23 Aug 2024 17:29:03 -0700 Subject: Error on duplicate resource with same disabled flag Also realized I hadn't handled flag negation so added that as well. Test: Automated Bug: 329436914 Flag: EXEMPT Aconfig not supported on host tools Change-Id: If90ae71070306f8e0c367be7e652da9c7bd0bb22 --- tools/aapt2/ResourceParser.cpp | 69 ++++++++++++++++++++---------------------- 1 file changed, 33 insertions(+), 36 deletions(-) (limited to 'tools/aapt2/ResourceParser.cpp') diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp index a5aecc855707..773edc3e3c22 100644 --- a/tools/aapt2/ResourceParser.cpp +++ b/tools/aapt2/ResourceParser.cpp @@ -107,9 +107,10 @@ struct ParsedResource { Visibility::Level visibility_level = Visibility::Level::kUndefined; bool staged_api = false; bool allow_new = false; - FlagStatus flag_status = FlagStatus::NoFlag; std::optional overlayable_item; std::optional staged_alias; + std::optional flag; + FlagStatus flag_status; std::string comment; std::unique_ptr value; @@ -151,6 +152,7 @@ static bool AddResourcesToTable(ResourceTable* table, android::IDiagnostics* dia } if (res->value != nullptr) { + res->value->SetFlag(res->flag); res->value->SetFlagStatus(res->flag_status); // Attach the comment, source and config to the value. res->value->SetComment(std::move(res->comment)); @@ -162,8 +164,6 @@ static bool AddResourcesToTable(ResourceTable* table, android::IDiagnostics* dia res_builder.SetStagedId(res->staged_alias.value()); } - res_builder.SetFlagStatus(res->flag_status); - bool error = false; if (!res->name.entry.empty()) { if (!table->AddResource(res_builder.Build(), diag)) { @@ -547,11 +547,15 @@ bool ResourceParser::ParseResource(xml::XmlPullParser* parser, }); std::string resource_type = parser->element_name(); - auto flag_status = GetFlagStatus(parser); - if (!flag_status) { + out_resource->flag = GetFlag(parser); + std::string error; + auto flag_status = GetFlagStatus(out_resource->flag, options_.feature_flag_values, &error); + if (flag_status) { + out_resource->flag_status = flag_status.value(); + } else { + diag_->Error(android::DiagMessage(source_.WithLine(parser->line_number())) << error); return false; } - out_resource->flag_status = flag_status.value(); // The value format accepted for this resource. uint32_t resource_format = 0u; @@ -733,31 +737,20 @@ bool ResourceParser::ParseResource(xml::XmlPullParser* parser, return false; } -std::optional ResourceParser::GetFlagStatus(xml::XmlPullParser* parser) { - auto flag_status = FlagStatus::NoFlag; - - std::optional flag = xml::FindAttribute(parser, xml::kSchemaAndroid, "featureFlag"); - if (flag) { - auto flag_it = options_.feature_flag_values.find(flag.value()); - if (flag_it == options_.feature_flag_values.end()) { - diag_->Error(android::DiagMessage(source_.WithLine(parser->line_number())) - << "Resource flag value undefined"); - return {}; - } - const auto& flag_properties = flag_it->second; - if (!flag_properties.read_only) { - diag_->Error(android::DiagMessage(source_.WithLine(parser->line_number())) - << "Only read only flags may be used with resources"); - return {}; - } - if (!flag_properties.enabled.has_value()) { - diag_->Error(android::DiagMessage(source_.WithLine(parser->line_number())) - << "Only flags with a value may be used with resources"); - return {}; +std::optional ResourceParser::GetFlag(xml::XmlPullParser* parser) { + auto name = xml::FindAttribute(parser, xml::kSchemaAndroid, "featureFlag"); + if (name) { + FeatureFlagAttribute flag; + if (name->starts_with('!')) { + flag.negated = true; + flag.name = name->substr(1); + } else { + flag.name = name.value(); } - flag_status = flag_properties.enabled.value() ? FlagStatus::Enabled : FlagStatus::Disabled; + return flag; + } else { + return {}; } - return flag_status; } bool ResourceParser::ParseItem(xml::XmlPullParser* parser, @@ -1666,21 +1659,25 @@ bool ResourceParser::ParseArrayImpl(xml::XmlPullParser* parser, const std::string& element_namespace = parser->element_namespace(); const std::string& element_name = parser->element_name(); if (element_namespace.empty() && element_name == "item") { - auto flag_status = GetFlagStatus(parser); - if (!flag_status) { - error = true; - continue; - } + auto flag = GetFlag(parser); std::unique_ptr item = ParseXml(parser, typeMask, kNoRawString); if (!item) { diag_->Error(android::DiagMessage(item_source) << "could not parse array item"); error = true; continue; } - item->SetFlagStatus(flag_status.value()); + item->SetFlag(flag); + std::string err; + auto status = GetFlagStatus(flag, options_.feature_flag_values, &err); + if (status) { + item->SetFlagStatus(status.value()); + } else { + diag_->Error(android::DiagMessage(item_source) << err); + error = true; + continue; + } item->SetSource(item_source); array->elements.emplace_back(std::move(item)); - } else if (!ShouldIgnoreElement(element_namespace, element_name)) { diag_->Error(android::DiagMessage(source_.WithLine(parser->line_number())) << "unknown tag <" << element_namespace << ":" << element_name << ">"); -- cgit v1.2.3-59-g8ed1b From 988be5d914334668ad25f8f993ec47442b3de33d Mon Sep 17 00:00:00 2001 From: Jeremy Meyer Date: Tue, 3 Sep 2024 16:31:54 -0700 Subject: Add support for flag in resource directory names This only applies it to the xml files with the top level element of resources. Other file support will be in a later CL. Test: Automated Bug: 329436914 Flag: EXEMPT Aconfig not supported on host tools Change-Id: I5e1e341e9de61073d05d9098b1b8b836025910b3 --- .../ResourceFlaggingTest.java | 20 ++++++++++ tools/aapt2/ResourceParser.cpp | 32 ++++++++++------ tools/aapt2/ResourceParser.h | 5 +++ tools/aapt2/cmd/Compile.cpp | 43 +++++++++++++++++++++- .../FlaggedResourcesTest/Android.bp | 8 ++++ .../flag(test.package.falseFlag)/values/bools.xml | 4 ++ .../FlaggedResourcesTest/res/values/bools.xml | 5 +++ .../values/flag(!test.package.falseFlag)/bools.xml | 4 ++ .../values/flag(!test.package.trueFlag)/bools.xml | 4 ++ .../values/flag(test.package.trueFlag)/bools.xml | 4 ++ 10 files changed, 117 insertions(+), 12 deletions(-) create mode 100644 tools/aapt2/integration-tests/FlaggedResourcesTest/res/flag(test.package.falseFlag)/values/bools.xml create mode 100644 tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/flag(!test.package.falseFlag)/bools.xml create mode 100644 tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/flag(!test.package.trueFlag)/bools.xml create mode 100644 tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/flag(test.package.trueFlag)/bools.xml (limited to 'tools/aapt2/ResourceParser.cpp') diff --git a/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java b/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java index 89c7582f4131..ee701f674715 100644 --- a/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java +++ b/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java @@ -103,6 +103,26 @@ public class ResourceFlaggingTest { assertThat(mResources.getIntArray(R.array.intarr1)).isEqualTo(new int[]{1, 2, 3}); } + @Test + public void testDirectoryEnabledFlag() { + assertThat(mResources.getBoolean(R.bool.bool8)).isTrue(); + } + + @Test + public void testDirectoryDisabledFlag() { + assertThat(mResources.getBoolean(R.bool.bool7)).isTrue(); + } + + @Test + public void testDirectoryNegatedEnabledFlag() { + assertThat(mResources.getBoolean(R.bool.bool9)).isTrue(); + } + + @Test + public void testDirectoryNegatedDisabledFlag() { + assertThat(mResources.getBoolean(R.bool.bool10)).isTrue(); + } + @Test public void testLayoutWithDisabledElements() { LinearLayout ll = (LinearLayout) getLayoutInflater().inflate(R.layout.layout1, null); diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp index 773edc3e3c22..a2383ac0285f 100644 --- a/tools/aapt2/ResourceParser.cpp +++ b/tools/aapt2/ResourceParser.cpp @@ -546,15 +546,25 @@ bool ResourceParser::ParseResource(xml::XmlPullParser* parser, {"symbol", std::mem_fn(&ResourceParser::ParseSymbol)}, }); - std::string resource_type = parser->element_name(); - out_resource->flag = GetFlag(parser); - std::string error; - auto flag_status = GetFlagStatus(out_resource->flag, options_.feature_flag_values, &error); - if (flag_status) { - out_resource->flag_status = flag_status.value(); - } else { - diag_->Error(android::DiagMessage(source_.WithLine(parser->line_number())) << error); - return false; + std::string_view resource_type = parser->element_name(); + if (auto flag = GetFlag(parser)) { + if (options_.flag) { + diag_->Error(android::DiagMessage(source_.WithLine(parser->line_number())) + << "Resource flag are not allowed both in the path and in the file"); + return false; + } + out_resource->flag = std::move(flag); + std::string error; + auto flag_status = GetFlagStatus(out_resource->flag, options_.feature_flag_values, &error); + if (flag_status) { + out_resource->flag_status = flag_status.value(); + } else { + diag_->Error(android::DiagMessage(source_.WithLine(parser->line_number())) << error); + return false; + } + } else if (options_.flag) { + out_resource->flag = options_.flag; + out_resource->flag_status = options_.flag_status; } // The value format accepted for this resource. @@ -571,7 +581,7 @@ bool ResourceParser::ParseResource(xml::XmlPullParser* parser, // Items have their type encoded in the type attribute. if (std::optional maybe_type = xml::FindNonEmptyAttribute(parser, "type")) { - resource_type = std::string(maybe_type.value()); + resource_type = maybe_type.value(); } else { diag_->Error(android::DiagMessage(source_.WithLine(parser->line_number())) << " must have a 'type' attribute"); @@ -594,7 +604,7 @@ bool ResourceParser::ParseResource(xml::XmlPullParser* parser, // Bags have their type encoded in the type attribute. if (std::optional maybe_type = xml::FindNonEmptyAttribute(parser, "type")) { - resource_type = std::string(maybe_type.value()); + resource_type = maybe_type.value(); } else { diag_->Error(android::DiagMessage(source_.WithLine(parser->line_number())) << " must have a 'type' attribute"); diff --git a/tools/aapt2/ResourceParser.h b/tools/aapt2/ResourceParser.h index a789d3e90770..4ad334c94518 100644 --- a/tools/aapt2/ResourceParser.h +++ b/tools/aapt2/ResourceParser.h @@ -57,6 +57,11 @@ struct ResourceParserOptions { std::optional visibility; FeatureFlagValues feature_flag_values; + + // The flag that should be applied to all resources parsed + std::optional flag; + + FlagStatus flag_status = FlagStatus::NoFlag; }; struct FlattenedXmlSubTree { diff --git a/tools/aapt2/cmd/Compile.cpp b/tools/aapt2/cmd/Compile.cpp index 2a978a5153ca..cd8c187c1e99 100644 --- a/tools/aapt2/cmd/Compile.cpp +++ b/tools/aapt2/cmd/Compile.cpp @@ -67,6 +67,7 @@ struct ResourcePathData { std::string resource_dir; std::string name; std::string extension; + std::string flag_name; // Original config str. We keep this because when we parse the config, we may add on // version qualifiers. We want to preserve the original input so the output is easily @@ -81,6 +82,22 @@ static std::optional ExtractResourcePathData(const std::string std::string* out_error, const CompileOptions& options) { std::vector parts = util::Split(path, dir_sep); + + std::string flag_name; + // Check for a flag + for (auto iter = parts.begin(); iter != parts.end();) { + if (iter->starts_with("flag(") && iter->ends_with(")")) { + if (!flag_name.empty()) { + if (out_error) *out_error = "resource path cannot contain more than one flag directory"; + return {}; + } + flag_name = iter->substr(5, iter->size() - 6); + iter = parts.erase(iter); + } else { + ++iter; + } + } + if (parts.size() < 2) { if (out_error) *out_error = "bad resource path"; return {}; @@ -131,6 +148,7 @@ static std::optional ExtractResourcePathData(const std::string std::string(dir_str), std::string(name), std::string(extension), + std::move(flag_name), std::string(config_str), config}; } @@ -142,6 +160,9 @@ static std::string BuildIntermediateContainerFilename(const ResourcePathData& da name << "-" << data.config_str; } name << "_" << data.name; + if (!data.flag_name.empty()) { + name << ".(" << data.flag_name << ")"; + } if (!data.extension.empty()) { name << "." << data.extension; } @@ -163,7 +184,6 @@ static bool CompileTable(IAaptContext* context, const CompileOptions& options, << "failed to open file: " << fin->GetError()); return false; } - // Parse the values file from XML. xml::XmlPullParser xml_parser(fin.get()); @@ -177,6 +197,27 @@ static bool CompileTable(IAaptContext* context, const CompileOptions& options, // we try to parse the , , or tags. parser_options.visibility = options.visibility; + if (!path_data.flag_name.empty()) { + FeatureFlagAttribute flag; + const auto& name = path_data.flag_name; + if (name.starts_with('!')) { + flag.negated = true; + flag.name = name.substr(1); + } else { + flag.name = name; + } + parser_options.flag = flag; + + std::string error; + auto flag_status = GetFlagStatus(flag, options.feature_flag_values, &error); + if (flag_status) { + parser_options.flag_status = std::move(flag_status.value()); + } else { + context->GetDiagnostics()->Error(android::DiagMessage(path_data.source) << error); + return false; + } + } + ResourceParser res_parser(context->GetDiagnostics(), &table, path_data.source, path_data.config, parser_options); if (!res_parser.Parse(&xml_parser)) { diff --git a/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp b/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp index c456e5c296d2..432b84e5ff90 100644 --- a/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp +++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp @@ -31,9 +31,17 @@ genrule { "res/values/ints.xml", "res/values/strings.xml", "res/layout/layout1.xml", + "res/flag(test.package.falseFlag)/values/bools.xml", + "res/values/flag(test.package.trueFlag)/bools.xml", + "res/values/flag(!test.package.trueFlag)/bools.xml", + "res/values/flag(!test.package.falseFlag)/bools.xml", ], out: [ "values_bools.arsc.flat", + "values_bools.(test.package.falseFlag).arsc.flat", + "values_bools.(test.package.trueFlag).arsc.flat", + "values_bools.(!test.package.falseFlag).arsc.flat", + "values_bools.(!test.package.trueFlag).arsc.flat", "values_bools2.arsc.flat", "values_ints.arsc.flat", "values_strings.arsc.flat", diff --git a/tools/aapt2/integration-tests/FlaggedResourcesTest/res/flag(test.package.falseFlag)/values/bools.xml b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/flag(test.package.falseFlag)/values/bools.xml new file mode 100644 index 000000000000..c46c4d4d8546 --- /dev/null +++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/flag(test.package.falseFlag)/values/bools.xml @@ -0,0 +1,4 @@ + + + false + \ No newline at end of file diff --git a/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools.xml b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools.xml index 7837e17b044b..35975ed1274a 100644 --- a/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools.xml +++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools.xml @@ -15,4 +15,9 @@ true false + + true + false + true + false \ No newline at end of file diff --git a/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/flag(!test.package.falseFlag)/bools.xml b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/flag(!test.package.falseFlag)/bools.xml new file mode 100644 index 000000000000..a63749c6ed7e --- /dev/null +++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/flag(!test.package.falseFlag)/bools.xml @@ -0,0 +1,4 @@ + + + true + \ No newline at end of file diff --git a/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/flag(!test.package.trueFlag)/bools.xml b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/flag(!test.package.trueFlag)/bools.xml new file mode 100644 index 000000000000..bb5526e69f97 --- /dev/null +++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/flag(!test.package.trueFlag)/bools.xml @@ -0,0 +1,4 @@ + + + false + \ No newline at end of file diff --git a/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/flag(test.package.trueFlag)/bools.xml b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/flag(test.package.trueFlag)/bools.xml new file mode 100644 index 000000000000..eba780e88c9a --- /dev/null +++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/flag(test.package.trueFlag)/bools.xml @@ -0,0 +1,4 @@ + + + true + \ No newline at end of file -- cgit v1.2.3-59-g8ed1b From e08e0379cc742c8b698fd020e565b7a6973d9264 Mon Sep 17 00:00:00 2001 From: Jeremy Meyer Date: Tue, 17 Sep 2024 12:45:26 -0700 Subject: Add support for flagging xml and png files This extends the previous change that added the ability to flag resource directories so that xml and png files are now supported. Test: Automated Bug: 329436914 Flag: EXEMPT Aconfig not supported on host tools Change-Id: I9f2b6b15ba0078ea33188f1a554377784cff9786 --- .../ResourceFlaggingTest.java | 19 ++++++++ tools/aapt2/Resource.h | 6 +++ tools/aapt2/ResourceParser.cpp | 20 +-------- tools/aapt2/ResourceParser.h | 2 - tools/aapt2/ResourcesInternal.proto | 5 +++ tools/aapt2/cmd/Compile.cpp | 48 +++++++++++++++------ tools/aapt2/cmd/Util.cpp | 14 ++++++ tools/aapt2/cmd/Util.h | 2 + tools/aapt2/format/proto/ProtoDeserialize.cpp | 6 +++ tools/aapt2/format/proto/ProtoSerialize.cpp | 5 +++ .../FlaggedResourcesTest/Android.bp | 8 ++++ .../drawable/removedpng.png | Bin 0 -> 5634 bytes .../layout/layout2.xml | 6 +++ .../flag(test.package.trueFlag)/layout/layout3.xml | 10 +++++ .../FlaggedResourcesTest/res/layout/layout3.xml | 6 +++ tools/aapt2/link/FlaggedResources_test.cpp | 6 +++ tools/aapt2/link/TableMerger.cpp | 2 + 17 files changed, 133 insertions(+), 32 deletions(-) create mode 100644 tools/aapt2/integration-tests/FlaggedResourcesTest/res/flag(test.package.falseFlag)/drawable/removedpng.png create mode 100644 tools/aapt2/integration-tests/FlaggedResourcesTest/res/flag(test.package.falseFlag)/layout/layout2.xml create mode 100644 tools/aapt2/integration-tests/FlaggedResourcesTest/res/flag(test.package.trueFlag)/layout/layout3.xml create mode 100644 tools/aapt2/integration-tests/FlaggedResourcesTest/res/layout/layout3.xml (limited to 'tools/aapt2/ResourceParser.cpp') diff --git a/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java b/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java index ee701f674715..d9e90fa001c5 100644 --- a/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java +++ b/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java @@ -29,6 +29,7 @@ import android.util.DisplayMetrics; import android.view.LayoutInflater; import android.view.View; import android.widget.LinearLayout; +import android.widget.TextView; import androidx.test.InstrumentationRegistry; import androidx.test.filters.SmallTest; @@ -132,6 +133,24 @@ public class ResourceFlaggingTest { assertThat((View) ll.findViewById(R.id.text2)).isNotNull(); } + @Test + public void testEnabledFlagLayoutOverrides() { + LinearLayout ll = (LinearLayout) getLayoutInflater().inflate(R.layout.layout3, null); + assertThat(ll).isNotNull(); + assertThat((View) ll.findViewById(R.id.text1)).isNotNull(); + assertThat(((TextView) ll.findViewById(R.id.text1)).getText()).isEqualTo("foobar"); + } + + @Test(expected = Resources.NotFoundException.class) + public void testDisabledLayout() { + getLayoutInflater().inflate(R.layout.layout2, null); + } + + @Test(expected = Resources.NotFoundException.class) + public void testDisabledDrawable() { + mResources.getDrawable(R.drawable.removedpng); + } + private LayoutInflater getLayoutInflater() { ContextWrapper c = new ContextWrapper(mContext) { private LayoutInflater mInflater; diff --git a/tools/aapt2/Resource.h b/tools/aapt2/Resource.h index 446fdd4dbc1b..0d261abd728d 100644 --- a/tools/aapt2/Resource.h +++ b/tools/aapt2/Resource.h @@ -243,6 +243,12 @@ struct ResourceFile { // Exported symbols std::vector exported_symbols; + + // Flag status + FlagStatus flag_status = FlagStatus::NoFlag; + + // Flag + std::optional flag; }; /** diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp index a2383ac0285f..fce6aa7c80d9 100644 --- a/tools/aapt2/ResourceParser.cpp +++ b/tools/aapt2/ResourceParser.cpp @@ -547,7 +547,7 @@ bool ResourceParser::ParseResource(xml::XmlPullParser* parser, }); std::string_view resource_type = parser->element_name(); - if (auto flag = GetFlag(parser)) { + if (auto flag = ParseFlag(xml::FindAttribute(parser, xml::kSchemaAndroid, "featureFlag"))) { if (options_.flag) { diag_->Error(android::DiagMessage(source_.WithLine(parser->line_number())) << "Resource flag are not allowed both in the path and in the file"); @@ -747,22 +747,6 @@ bool ResourceParser::ParseResource(xml::XmlPullParser* parser, return false; } -std::optional ResourceParser::GetFlag(xml::XmlPullParser* parser) { - auto name = xml::FindAttribute(parser, xml::kSchemaAndroid, "featureFlag"); - if (name) { - FeatureFlagAttribute flag; - if (name->starts_with('!')) { - flag.negated = true; - flag.name = name->substr(1); - } else { - flag.name = name.value(); - } - return flag; - } else { - return {}; - } -} - bool ResourceParser::ParseItem(xml::XmlPullParser* parser, ParsedResource* out_resource, const uint32_t format) { @@ -1669,7 +1653,7 @@ bool ResourceParser::ParseArrayImpl(xml::XmlPullParser* parser, const std::string& element_namespace = parser->element_namespace(); const std::string& element_name = parser->element_name(); if (element_namespace.empty() && element_name == "item") { - auto flag = GetFlag(parser); + auto flag = ParseFlag(xml::FindAttribute(parser, xml::kSchemaAndroid, "featureFlag")); std::unique_ptr item = ParseXml(parser, typeMask, kNoRawString); if (!item) { diag_->Error(android::DiagMessage(item_source) << "could not parse array item"); diff --git a/tools/aapt2/ResourceParser.h b/tools/aapt2/ResourceParser.h index 4ad334c94518..90690d522ef2 100644 --- a/tools/aapt2/ResourceParser.h +++ b/tools/aapt2/ResourceParser.h @@ -90,8 +90,6 @@ class ResourceParser { private: DISALLOW_COPY_AND_ASSIGN(ResourceParser); - std::optional GetFlag(xml::XmlPullParser* parser); - std::optional CreateFlattenSubTree(xml::XmlPullParser* parser); // Parses the XML subtree as a StyleString (flattened XML representation for strings with diff --git a/tools/aapt2/ResourcesInternal.proto b/tools/aapt2/ResourcesInternal.proto index b0ed3da33368..f4735a2f6ce7 100644 --- a/tools/aapt2/ResourcesInternal.proto +++ b/tools/aapt2/ResourcesInternal.proto @@ -49,4 +49,9 @@ message CompiledFile { // Any symbols this file auto-generates/exports (eg. @+id/foo in an XML file). repeated Symbol exported_symbol = 5; + + // The status of the flag the file is behind if any + uint32 flag_status = 6; + bool flag_negated = 7; + string flag_name = 8; } diff --git a/tools/aapt2/cmd/Compile.cpp b/tools/aapt2/cmd/Compile.cpp index cd8c187c1e99..52372fa38525 100644 --- a/tools/aapt2/cmd/Compile.cpp +++ b/tools/aapt2/cmd/Compile.cpp @@ -196,20 +196,11 @@ static bool CompileTable(IAaptContext* context, const CompileOptions& options, // If visibility was forced, we need to use it when creating a new resource and also error if // we try to parse the , , or tags. parser_options.visibility = options.visibility; + parser_options.flag = ParseFlag(path_data.flag_name); - if (!path_data.flag_name.empty()) { - FeatureFlagAttribute flag; - const auto& name = path_data.flag_name; - if (name.starts_with('!')) { - flag.negated = true; - flag.name = name.substr(1); - } else { - flag.name = name; - } - parser_options.flag = flag; - + if (parser_options.flag) { std::string error; - auto flag_status = GetFlagStatus(flag, options.feature_flag_values, &error); + auto flag_status = GetFlagStatus(parser_options.flag, options.feature_flag_values, &error); if (flag_status) { parser_options.flag_status = std::move(flag_status.value()); } else { @@ -443,6 +434,18 @@ static bool CompileXml(IAaptContext* context, const CompileOptions& options, xmlres->file.config = path_data.config; xmlres->file.source = path_data.source; xmlres->file.type = ResourceFile::Type::kProtoXml; + xmlres->file.flag = ParseFlag(path_data.flag_name); + + if (xmlres->file.flag) { + std::string error; + auto flag_status = GetFlagStatus(xmlres->file.flag, options.feature_flag_values, &error); + if (flag_status) { + xmlres->file.flag_status = flag_status.value(); + } else { + context->GetDiagnostics()->Error(android::DiagMessage(path_data.source) << error); + return false; + } + } // Collect IDs that are defined here. XmlIdCollector collector; @@ -532,6 +535,27 @@ static bool CompilePng(IAaptContext* context, const CompileOptions& options, res_file.source = path_data.source; res_file.type = ResourceFile::Type::kPng; + if (!path_data.flag_name.empty()) { + FeatureFlagAttribute flag; + auto name = path_data.flag_name; + if (name.starts_with('!')) { + flag.negated = true; + flag.name = name.substr(1); + } else { + flag.name = name; + } + res_file.flag = flag; + + std::string error; + auto flag_status = GetFlagStatus(flag, options.feature_flag_values, &error); + if (flag_status) { + res_file.flag_status = flag_status.value(); + } else { + context->GetDiagnostics()->Error(android::DiagMessage(path_data.source) << error); + return false; + } + } + { auto data = file->OpenAsData(); if (!data) { diff --git a/tools/aapt2/cmd/Util.cpp b/tools/aapt2/cmd/Util.cpp index 2177c345260c..08f8f0d85807 100644 --- a/tools/aapt2/cmd/Util.cpp +++ b/tools/aapt2/cmd/Util.cpp @@ -34,6 +34,20 @@ using ::android::base::StringPrintf; namespace aapt { +std::optional ParseFlag(std::optional flag_text) { + if (!flag_text || flag_text->empty()) { + return {}; + } + FeatureFlagAttribute flag; + if (flag_text->starts_with('!')) { + flag.negated = true; + flag.name = flag_text->substr(1); + } else { + flag.name = flag_text.value(); + } + return flag; +} + std::optional GetFlagStatus(const std::optional& flag, const FeatureFlagValues& feature_flag_values, std::string* out_err) { diff --git a/tools/aapt2/cmd/Util.h b/tools/aapt2/cmd/Util.h index f8e44b72247c..d32e532b86a8 100644 --- a/tools/aapt2/cmd/Util.h +++ b/tools/aapt2/cmd/Util.h @@ -49,6 +49,8 @@ struct FeatureFlagProperties { using FeatureFlagValues = std::map>; +std::optional ParseFlag(std::optional flag_text); + std::optional GetFlagStatus(const std::optional& flag, const FeatureFlagValues& feature_flag_values, std::string* out_err); diff --git a/tools/aapt2/format/proto/ProtoDeserialize.cpp b/tools/aapt2/format/proto/ProtoDeserialize.cpp index c6bd6dd3c4b2..8583cadff6d2 100644 --- a/tools/aapt2/format/proto/ProtoDeserialize.cpp +++ b/tools/aapt2/format/proto/ProtoDeserialize.cpp @@ -643,6 +643,12 @@ bool DeserializeCompiledFileFromPb(const pb::internal::CompiledFile& pb_file, out_file->source.path = pb_file.source_path(); out_file->type = DeserializeFileReferenceTypeFromPb(pb_file.type()); + out_file->flag_status = (FlagStatus)pb_file.flag_status(); + if (!pb_file.flag_name().empty()) { + out_file->flag = + FeatureFlagAttribute{.name = pb_file.flag_name(), .negated = pb_file.flag_negated()}; + } + std::string config_error; if (!DeserializeConfigFromPb(pb_file.config(), &out_file->config, &config_error)) { std::ostringstream error; diff --git a/tools/aapt2/format/proto/ProtoSerialize.cpp b/tools/aapt2/format/proto/ProtoSerialize.cpp index 9c28780e7880..d83fe916ee95 100644 --- a/tools/aapt2/format/proto/ProtoSerialize.cpp +++ b/tools/aapt2/format/proto/ProtoSerialize.cpp @@ -755,6 +755,11 @@ void SerializeCompiledFileToPb(const ResourceFile& file, pb::internal::CompiledF out_file->set_source_path(file.source.path); out_file->set_type(SerializeFileReferenceTypeToPb(file.type)); SerializeConfig(file.config, out_file->mutable_config()); + out_file->set_flag_status((uint32_t)file.flag_status); + if (file.flag) { + out_file->set_flag_negated(file.flag->negated); + out_file->set_flag_name(file.flag->name); + } for (const SourcedResourceName& exported : file.exported_symbols) { pb::internal::CompiledFile_Symbol* pb_symbol = out_file->add_exported_symbol(); diff --git a/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp b/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp index 432b84e5ff90..7160b35033da 100644 --- a/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp +++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp @@ -31,12 +31,17 @@ genrule { "res/values/ints.xml", "res/values/strings.xml", "res/layout/layout1.xml", + "res/layout/layout3.xml", "res/flag(test.package.falseFlag)/values/bools.xml", + "res/flag(test.package.falseFlag)/layout/layout2.xml", + "res/flag(test.package.falseFlag)/drawable/removedpng.png", + "res/flag(test.package.trueFlag)/layout/layout3.xml", "res/values/flag(test.package.trueFlag)/bools.xml", "res/values/flag(!test.package.trueFlag)/bools.xml", "res/values/flag(!test.package.falseFlag)/bools.xml", ], out: [ + "drawable_removedpng.(test.package.falseFlag).png.flat", "values_bools.arsc.flat", "values_bools.(test.package.falseFlag).arsc.flat", "values_bools.(test.package.trueFlag).arsc.flat", @@ -46,6 +51,9 @@ genrule { "values_ints.arsc.flat", "values_strings.arsc.flat", "layout_layout1.xml.flat", + "layout_layout2.(test.package.falseFlag).xml.flat", + "layout_layout3.xml.flat", + "layout_layout3.(test.package.trueFlag).xml.flat", ], cmd: "$(location aapt2) compile $(in) -o $(genDir) " + "--feature-flags test.package.falseFlag:ro=false,test.package.trueFlag:ro=true", diff --git a/tools/aapt2/integration-tests/FlaggedResourcesTest/res/flag(test.package.falseFlag)/drawable/removedpng.png b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/flag(test.package.falseFlag)/drawable/removedpng.png new file mode 100644 index 000000000000..8a9e6984be96 Binary files /dev/null and b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/flag(test.package.falseFlag)/drawable/removedpng.png differ diff --git a/tools/aapt2/integration-tests/FlaggedResourcesTest/res/flag(test.package.falseFlag)/layout/layout2.xml b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/flag(test.package.falseFlag)/layout/layout2.xml new file mode 100644 index 000000000000..dec5de72925a --- /dev/null +++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/flag(test.package.falseFlag)/layout/layout2.xml @@ -0,0 +1,6 @@ + + + \ No newline at end of file diff --git a/tools/aapt2/integration-tests/FlaggedResourcesTest/res/flag(test.package.trueFlag)/layout/layout3.xml b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/flag(test.package.trueFlag)/layout/layout3.xml new file mode 100644 index 000000000000..5aeee0ee1e28 --- /dev/null +++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/flag(test.package.trueFlag)/layout/layout3.xml @@ -0,0 +1,10 @@ + + + + \ No newline at end of file diff --git a/tools/aapt2/integration-tests/FlaggedResourcesTest/res/layout/layout3.xml b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/layout/layout3.xml new file mode 100644 index 000000000000..dec5de72925a --- /dev/null +++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/layout/layout3.xml @@ -0,0 +1,6 @@ + + + \ No newline at end of file diff --git a/tools/aapt2/link/FlaggedResources_test.cpp b/tools/aapt2/link/FlaggedResources_test.cpp index 67d0c41f6b6f..629300838bbe 100644 --- a/tools/aapt2/link/FlaggedResources_test.cpp +++ b/tools/aapt2/link/FlaggedResources_test.cpp @@ -76,6 +76,10 @@ TEST_F(FlaggedResourcesTest, DisabledResourcesRemovedFromTable) { std::string output; DumpResourceTableToString(loaded_apk.get(), &output); + ASSERT_EQ(output.find("bool4"), std::string::npos); + ASSERT_EQ(output.find("str1"), std::string::npos); + ASSERT_EQ(output.find("layout2"), std::string::npos); + ASSERT_EQ(output.find("removedpng"), std::string::npos); } TEST_F(FlaggedResourcesTest, DisabledResourcesRemovedFromTableChunks) { @@ -87,6 +91,8 @@ TEST_F(FlaggedResourcesTest, DisabledResourcesRemovedFromTableChunks) { ASSERT_EQ(output.find("bool4"), std::string::npos); ASSERT_EQ(output.find("str1"), std::string::npos); + ASSERT_EQ(output.find("layout2"), std::string::npos); + ASSERT_EQ(output.find("removedpng"), std::string::npos); } TEST_F(FlaggedResourcesTest, DisabledResourcesInRJava) { diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp index d21697938c62..1bef5f8b17f6 100644 --- a/tools/aapt2/link/TableMerger.cpp +++ b/tools/aapt2/link/TableMerger.cpp @@ -377,6 +377,8 @@ bool TableMerger::MergeFile(const ResourceFile& file_desc, bool overlay, io::IFi file_ref->SetSource(file_desc.source); file_ref->type = file_desc.type; file_ref->file = file; + file_ref->SetFlagStatus(file_desc.flag_status); + file_ref->SetFlag(file_desc.flag); ResourceTablePackage* pkg = table.FindOrCreatePackage(file_desc.name.package); pkg->FindOrCreateType(file_desc.name.type) -- cgit v1.2.3-59-g8ed1b From f53a027bfa9820821be0fdd51faec51ddedf381f Mon Sep 17 00:00:00 2001 From: Jeremy Meyer Date: Thu, 19 Sep 2024 09:47:47 -0700 Subject: Add flag support to styles elements Test: Automated Bug: 329436914 Flag: EXEMPT Aconfig not supported on host tools Change-Id: I3041ebbaf98a90527809b541df8885edfc70d035 --- .../ResourceFlaggingTest.java | 22 +++++++++++++++++++++ tools/aapt2/ResourceParser.cpp | 23 +++++++++++++++++++++- tools/aapt2/Resources.proto | 5 +++++ tools/aapt2/format/proto/ProtoDeserialize.cpp | 19 +++++++++--------- tools/aapt2/format/proto/ProtoSerialize.cpp | 7 +++++++ .../FlaggedResourcesTest/Android.bp | 2 ++ .../FlaggedResourcesTest/res/values/styles.xml | 21 ++++++++++++++++++++ 7 files changed, 88 insertions(+), 11 deletions(-) create mode 100644 tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/styles.xml (limited to 'tools/aapt2/ResourceParser.cpp') diff --git a/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java b/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java index d9e90fa001c5..a37000a40508 100644 --- a/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java +++ b/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java @@ -24,6 +24,7 @@ import android.content.res.ApkAssets; import android.content.res.AssetManager; import android.content.res.Configuration; import android.content.res.Resources; +import android.content.res.TypedArray; import android.os.FileUtils; import android.util.DisplayMetrics; import android.view.LayoutInflater; @@ -151,6 +152,27 @@ public class ResourceFlaggingTest { mResources.getDrawable(R.drawable.removedpng); } + @Test + public void testDisabledStyleDoesntOverride() { + TypedArray ta = mResources.newTheme().obtainStyledAttributes( + R.style.style1, new int[] { android.R.attr.windowIsTranslucent}); + assertThat(ta.getBoolean(0, false)).isTrue(); + } + + @Test + public void testEnabledStyleDoesOverride() { + TypedArray ta = mResources.newTheme().obtainStyledAttributes( + R.style.style2, new int[] { android.R.attr.windowIsTranslucent}); + assertThat(ta.getBoolean(0, false)).isTrue(); + } + + @Test + public void testEnabledStyleItemDoesOverride() { + TypedArray ta = mResources.newTheme().obtainStyledAttributes( + R.style.style3, new int[] { android.R.attr.windowIsTranslucent}); + assertThat(ta.getBoolean(0, false)).isTrue(); + } + private LayoutInflater getLayoutInflater() { ContextWrapper c = new ContextWrapper(mContext) { private LayoutInflater mInflater; diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp index fce6aa7c80d9..da092f43caa4 100644 --- a/tools/aapt2/ResourceParser.cpp +++ b/tools/aapt2/ResourceParser.cpp @@ -1529,13 +1529,34 @@ bool ResourceParser::ParseStyleItem(xml::XmlPullParser* parser, Style* style) { ResolvePackage(parser, &maybe_key.value()); maybe_key.value().SetSource(source); + auto flag = ParseFlag(xml::FindAttribute(parser, xml::kSchemaAndroid, "featureFlag")); + std::unique_ptr value = ParseXml(parser, 0, kAllowRawString); if (!value) { diag_->Error(android::DiagMessage(source) << "could not parse style item"); return false; } - style->entries.push_back(Style::Entry{std::move(maybe_key.value()), std::move(value)}); + if (flag) { + if (options_.flag) { + diag_->Error(android::DiagMessage(source_.WithLine(parser->line_number())) + << "Resource flag are not allowed both in the path and in the file"); + return false; + } + std::string error; + auto flag_status = GetFlagStatus(flag, options_.feature_flag_values, &error); + if (flag_status) { + value->SetFlagStatus(flag_status.value()); + value->SetFlag(std::move(flag)); + } else { + diag_->Error(android::DiagMessage(source_.WithLine(parser->line_number())) << error); + return false; + } + } + + if (value->GetFlagStatus() != FlagStatus::Disabled) { + style->entries.push_back(Style::Entry{std::move(maybe_key.value()), std::move(value)}); + } return true; } diff --git a/tools/aapt2/Resources.proto b/tools/aapt2/Resources.proto index a0f60b62db3a..fe9b4a8843ca 100644 --- a/tools/aapt2/Resources.proto +++ b/tools/aapt2/Resources.proto @@ -302,6 +302,11 @@ message CompoundValue { Plural plural = 5; MacroBody macro = 6; } + + // The status of the flag the value is behind if any + uint32 flag_status = 7; + bool flag_negated = 8; + string flag_name = 9; } // Message holding a boolean, so it can be optionally encoded. diff --git a/tools/aapt2/format/proto/ProtoDeserialize.cpp b/tools/aapt2/format/proto/ProtoDeserialize.cpp index 8583cadff6d2..91ec3485ac3b 100644 --- a/tools/aapt2/format/proto/ProtoDeserialize.cpp +++ b/tools/aapt2/format/proto/ProtoDeserialize.cpp @@ -551,11 +551,10 @@ static bool DeserializePackageFromPb(const pb::Package& pb_package, const ResStr return false; } - FeatureFlagAttribute flag; - flag.name = pb_config_value.value().item().flag_name(); - flag.negated = pb_config_value.value().item().flag_negated(); - ResourceConfigValue* config_value = - entry->FindOrCreateFlagDisabledValue(std::move(flag), config, pb_config.product()); + ResourceConfigValue* config_value = entry->FindOrCreateFlagDisabledValue( + FeatureFlagAttribute{.name = pb_config_value.value().item().flag_name(), + .negated = pb_config_value.value().item().flag_negated()}, + config, pb_config.product()); if (config_value->value != nullptr) { *out_error = "duplicate configuration in resource table"; return false; @@ -563,7 +562,6 @@ static bool DeserializePackageFromPb(const pb::Package& pb_package, const ResStr config_value->value = DeserializeValueFromPb(pb_config_value.value(), src_pool, config, &out_table->string_pool, files, out_error); - if (config_value->value == nullptr) { return false; } @@ -896,6 +894,9 @@ std::unique_ptr DeserializeValueFromPb(const pb::Value& pb_value, LOG(FATAL) << "unknown compound value: " << (int)pb_compound_value.value_case(); break; } + value->SetFlagStatus((FlagStatus)pb_compound_value.flag_status()); + value->SetFlag(FeatureFlagAttribute{.name = pb_compound_value.flag_name(), + .negated = pb_compound_value.flag_negated()}); } else { LOG(FATAL) << "unknown value: " << (int)pb_value.value_case(); return {}; @@ -1052,10 +1053,8 @@ std::unique_ptr DeserializeItemFromPb(const pb::Item& pb_item, if (item) { item->SetFlagStatus((FlagStatus)pb_item.flag_status()); if (!pb_item.flag_name().empty()) { - FeatureFlagAttribute flag; - flag.name = pb_item.flag_name(); - flag.negated = pb_item.flag_negated(); - item->SetFlag(std::move(flag)); + item->SetFlag( + FeatureFlagAttribute{.name = pb_item.flag_name(), .negated = pb_item.flag_negated()}); } } return item; diff --git a/tools/aapt2/format/proto/ProtoSerialize.cpp b/tools/aapt2/format/proto/ProtoSerialize.cpp index d83fe916ee95..fcc77d5a9d6d 100644 --- a/tools/aapt2/format/proto/ProtoSerialize.cpp +++ b/tools/aapt2/format/proto/ProtoSerialize.cpp @@ -734,6 +734,13 @@ void SerializeValueToPb(const Value& value, pb::Value* out_value, android::Strin out_value->mutable_item()->set_flag_negated(flag->negated); out_value->mutable_item()->set_flag_name(flag->name); } + } else if (out_value->has_compound_value()) { + out_value->mutable_compound_value()->set_flag_status((uint32_t)value.GetFlagStatus()); + if (value.GetFlag()) { + const auto& flag = value.GetFlag(); + out_value->mutable_compound_value()->set_flag_negated(flag->negated); + out_value->mutable_compound_value()->set_flag_name(flag->name); + } } } diff --git a/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp b/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp index 7160b35033da..1b0f99753ce6 100644 --- a/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp +++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp @@ -30,6 +30,7 @@ genrule { "res/values/bools2.xml", "res/values/ints.xml", "res/values/strings.xml", + "res/values/styles.xml", "res/layout/layout1.xml", "res/layout/layout3.xml", "res/flag(test.package.falseFlag)/values/bools.xml", @@ -50,6 +51,7 @@ genrule { "values_bools2.arsc.flat", "values_ints.arsc.flat", "values_strings.arsc.flat", + "values_styles.arsc.flat", "layout_layout1.xml.flat", "layout_layout2.(test.package.falseFlag).xml.flat", "layout_layout3.xml.flat", diff --git a/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/styles.xml b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/styles.xml new file mode 100644 index 000000000000..604129c26fef --- /dev/null +++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/styles.xml @@ -0,0 +1,21 @@ + + + + + + + + + + \ No newline at end of file -- cgit v1.2.3-59-g8ed1b From fe92e8d1e670ce8ed0ac77e7d05d5446eeecc5bb Mon Sep 17 00:00:00 2001 From: Jeremy Meyer Date: Tue, 1 Oct 2024 13:59:35 -0700 Subject: Set a default value for flagstatus Test: Automated Bug: 329436914 Flag: EXEMPT Aconfig not supported on host tools Change-Id: Id631e95180e50550b061ac235956aa8f47c242d4 --- tools/aapt2/ResourceParser.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools/aapt2/ResourceParser.cpp') diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp index fce6aa7c80d9..9df9ed5b1053 100644 --- a/tools/aapt2/ResourceParser.cpp +++ b/tools/aapt2/ResourceParser.cpp @@ -110,7 +110,7 @@ struct ParsedResource { std::optional overlayable_item; std::optional staged_alias; std::optional flag; - FlagStatus flag_status; + FlagStatus flag_status = FlagStatus::NoFlag; std::string comment; std::unique_ptr value; -- cgit v1.2.3-59-g8ed1b