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
diff --git a/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java b/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java
index 005538a..89c7582 100644
--- a/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java
+++ b/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java
@@ -78,6 +78,16 @@
}
@Test
+ public void testNegatedDisabledFlag() {
+ assertThat(mResources.getBoolean(R.bool.bool5)).isTrue();
+ }
+
+ @Test
+ public void testNegatedEnabledFlag() {
+ assertThat(mResources.getBoolean(R.bool.bool6)).isTrue();
+ }
+
+ @Test
public void testFlagEnabledDifferentCompilationUnit() {
assertThat(mResources.getBoolean(R.bool.bool3)).isTrue();
}
diff --git a/tools/aapt2/Debug.cpp b/tools/aapt2/Debug.cpp
index df1d51e..064b461 100644
--- a/tools/aapt2/Debug.cpp
+++ b/tools/aapt2/Debug.cpp
@@ -346,6 +346,21 @@
value->value->Accept(&body_printer);
printer->Undent();
}
+ printer->Println("Flag disabled values:");
+ for (const auto& value : entry.flag_disabled_values) {
+ printer->Print("(");
+ printer->Print(value->config.to_string());
+ printer->Print(") ");
+ value->value->Accept(&headline_printer);
+ if (options.show_sources && !value->value->GetSource().path.empty()) {
+ printer->Print(" src=");
+ printer->Print(value->value->GetSource().to_string());
+ }
+ printer->Println();
+ printer->Indent();
+ value->value->Accept(&body_printer);
+ printer->Undent();
+ }
printer->Undent();
}
}
diff --git a/tools/aapt2/Resource.h b/tools/aapt2/Resource.h
index a274f04..446fdd4 100644
--- a/tools/aapt2/Resource.h
+++ b/tools/aapt2/Resource.h
@@ -71,6 +71,17 @@
enum class FlagStatus { NoFlag = 0, Disabled = 1, Enabled = 2 };
+struct FeatureFlagAttribute {
+ std::string name;
+ bool negated = false;
+
+ std::string ToString() {
+ return (negated ? "!" : "") + name;
+ }
+
+ bool operator==(const FeatureFlagAttribute& o) const = default;
+};
+
android::StringPiece to_string(ResourceType type);
/**
diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp
index a5aecc8..773edc3 100644
--- a/tools/aapt2/ResourceParser.cpp
+++ b/tools/aapt2/ResourceParser.cpp
@@ -107,9 +107,10 @@
Visibility::Level visibility_level = Visibility::Level::kUndefined;
bool staged_api = false;
bool allow_new = false;
- FlagStatus flag_status = FlagStatus::NoFlag;
std::optional<OverlayableItem> overlayable_item;
std::optional<StagedId> staged_alias;
+ std::optional<FeatureFlagAttribute> flag;
+ FlagStatus flag_status;
std::string comment;
std::unique_ptr<Value> value;
@@ -151,6 +152,7 @@
}
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 @@
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 @@
});
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 @@
return false;
}
-std::optional<FlagStatus> ResourceParser::GetFlagStatus(xml::XmlPullParser* parser) {
- auto flag_status = FlagStatus::NoFlag;
-
- std::optional<StringPiece> 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 {};
+std::optional<FeatureFlagAttribute> 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();
}
- 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 {};
- }
- 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 @@
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> 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 << ">");
diff --git a/tools/aapt2/ResourceParser.h b/tools/aapt2/ResourceParser.h
index 442dea8..a789d3e 100644
--- a/tools/aapt2/ResourceParser.h
+++ b/tools/aapt2/ResourceParser.h
@@ -85,7 +85,7 @@
private:
DISALLOW_COPY_AND_ASSIGN(ResourceParser);
- std::optional<FlagStatus> GetFlagStatus(xml::XmlPullParser* parser);
+ std::optional<FeatureFlagAttribute> GetFlag(xml::XmlPullParser* parser);
std::optional<FlattenedXmlSubTree> CreateFlattenSubTree(xml::XmlPullParser* parser);
diff --git a/tools/aapt2/ResourceTable.cpp b/tools/aapt2/ResourceTable.cpp
index 9751459..5435cba2 100644
--- a/tools/aapt2/ResourceTable.cpp
+++ b/tools/aapt2/ResourceTable.cpp
@@ -101,6 +101,21 @@
}
};
+struct ConfigFlagKey {
+ const ConfigDescription* config;
+ StringPiece product;
+ const FeatureFlagAttribute& flag;
+};
+
+struct lt_config_flag_key_ref {
+ template <typename T>
+ bool operator()(const T& lhs, const ConfigFlagKey& rhs) const noexcept {
+ return std::tie(lhs->config, lhs->product, lhs->value->GetFlag()->name,
+ lhs->value->GetFlag()->negated) <
+ std::tie(*rhs.config, rhs.product, rhs.flag.name, rhs.flag.negated);
+ }
+};
+
} // namespace
ResourceTable::ResourceTable(ResourceTable::Validation validation) : validation_(validation) {
@@ -213,6 +228,25 @@
return results;
}
+ResourceConfigValue* ResourceEntry::FindOrCreateFlagDisabledValue(
+ const FeatureFlagAttribute& flag, const android::ConfigDescription& config,
+ android::StringPiece product) {
+ auto iter = std::lower_bound(flag_disabled_values.begin(), flag_disabled_values.end(),
+ ConfigFlagKey{&config, product, flag}, lt_config_flag_key_ref());
+ if (iter != flag_disabled_values.end()) {
+ ResourceConfigValue* value = iter->get();
+ const auto value_flag = value->value->GetFlag().value();
+ if (value_flag.name == flag.name && value_flag.negated == flag.negated &&
+ value->config == config && value->product == product) {
+ return value;
+ }
+ }
+ ResourceConfigValue* newValue =
+ flag_disabled_values.insert(iter, util::make_unique<ResourceConfigValue>(config, product))
+ ->get();
+ return newValue;
+}
+
bool ResourceEntry::HasDefaultValue() const {
// The default config should be at the top of the list, since the list is sorted.
return !values.empty() && values.front()->config == ConfigDescription::DefaultConfig();
@@ -375,13 +409,14 @@
}
};
-void InsertEntryIntoTableView(ResourceTableView& table, const ResourceTablePackage* package,
- const ResourceTableType* type, const std::string& entry_name,
- const std::optional<ResourceId>& id, const Visibility& visibility,
- const std::optional<AllowNew>& allow_new,
- const std::optional<OverlayableItem>& overlayable_item,
- const std::optional<StagedId>& staged_id,
- const std::vector<std::unique_ptr<ResourceConfigValue>>& values) {
+void InsertEntryIntoTableView(
+ ResourceTableView& table, const ResourceTablePackage* package, const ResourceTableType* type,
+ const std::string& entry_name, const std::optional<ResourceId>& id,
+ const Visibility& visibility, const std::optional<AllowNew>& allow_new,
+ const std::optional<OverlayableItem>& overlayable_item,
+ const std::optional<StagedId>& staged_id,
+ const std::vector<std::unique_ptr<ResourceConfigValue>>& values,
+ const std::vector<std::unique_ptr<ResourceConfigValue>>& flag_disabled_values) {
SortedVectorInserter<ResourceTablePackageView, PackageViewComparer> package_inserter;
SortedVectorInserter<ResourceTableTypeView, TypeViewComparer> type_inserter;
SortedVectorInserter<ResourceTableEntryView, EntryViewComparer> entry_inserter;
@@ -408,6 +443,9 @@
for (auto& value : values) {
new_entry.values.emplace_back(value.get());
}
+ for (auto& value : flag_disabled_values) {
+ new_entry.flag_disabled_values.emplace_back(value.get());
+ }
entry_inserter.Insert(view_type->entries, std::move(new_entry));
}
@@ -426,6 +464,21 @@
return nullptr;
}
+const ResourceConfigValue* ResourceTableEntryView::FindFlagDisabledValue(
+ const FeatureFlagAttribute& flag, const ConfigDescription& config,
+ android::StringPiece product) const {
+ auto iter = std::lower_bound(flag_disabled_values.begin(), flag_disabled_values.end(),
+ ConfigFlagKey{&config, product, flag}, lt_config_flag_key_ref());
+ if (iter != values.end()) {
+ const ResourceConfigValue* value = *iter;
+ if (value->value->GetFlag() == flag && value->config == config &&
+ StringPiece(value->product) == product) {
+ return value;
+ }
+ }
+ return nullptr;
+}
+
ResourceTableView ResourceTable::GetPartitionedView(const ResourceTableViewOptions& options) const {
ResourceTableView view;
for (const auto& package : packages) {
@@ -433,13 +486,13 @@
for (const auto& entry : type->entries) {
InsertEntryIntoTableView(view, package.get(), type.get(), entry->name, entry->id,
entry->visibility, entry->allow_new, entry->overlayable_item,
- entry->staged_id, entry->values);
+ entry->staged_id, entry->values, entry->flag_disabled_values);
if (options.create_alias_entries && entry->staged_id) {
auto alias_id = entry->staged_id.value().id;
InsertEntryIntoTableView(view, package.get(), type.get(), entry->name, alias_id,
entry->visibility, entry->allow_new, entry->overlayable_item, {},
- entry->values);
+ entry->values, entry->flag_disabled_values);
}
}
}
@@ -587,6 +640,25 @@
entry->staged_id = res.staged_id.value();
}
+ if (res.value != nullptr && res.value->GetFlagStatus() == FlagStatus::Disabled) {
+ auto disabled_config_value =
+ entry->FindOrCreateFlagDisabledValue(res.value->GetFlag().value(), res.config, res.product);
+ if (!disabled_config_value->value) {
+ // Resource does not exist, add it now.
+ // Must clone the value since it might be in the values vector as well
+ CloningValueTransformer cloner(&string_pool);
+ disabled_config_value->value = res.value->Transform(cloner);
+ } else {
+ diag->Error(android::DiagMessage(source)
+ << "duplicate value for resource '" << res.name << "' " << "with config '"
+ << res.config << "' and flag '"
+ << (res.value->GetFlag().value().negated ? "!" : "")
+ << res.value->GetFlag().value().name << "'");
+ diag->Error(android::DiagMessage(source) << "resource previously defined here");
+ return false;
+ }
+ }
+
if (res.value != nullptr) {
auto config_value = entry->FindOrCreateValue(res.config, res.product);
if (!config_value->value) {
@@ -595,9 +667,9 @@
} else {
// When validation is enabled, ensure that a resource cannot have multiple values defined for
// the same configuration unless protected by flags.
- auto result =
- validate ? ResolveFlagCollision(config_value->value->GetFlagStatus(), res.flag_status)
- : CollisionResult::kKeepBoth;
+ auto result = validate ? ResolveFlagCollision(config_value->value->GetFlagStatus(),
+ res.value->GetFlagStatus())
+ : CollisionResult::kKeepBoth;
if (result == CollisionResult::kConflict) {
result = ResolveValueCollision(config_value->value.get(), res.value.get());
}
@@ -771,11 +843,6 @@
return *this;
}
-NewResourceBuilder& NewResourceBuilder::SetFlagStatus(FlagStatus flag_status) {
- res_.flag_status = flag_status;
- return *this;
-}
-
NewResource NewResourceBuilder::Build() {
return std::move(res_);
}
diff --git a/tools/aapt2/ResourceTable.h b/tools/aapt2/ResourceTable.h
index cba6b70..b0e1855 100644
--- a/tools/aapt2/ResourceTable.h
+++ b/tools/aapt2/ResourceTable.h
@@ -136,6 +136,9 @@
// The resource's values for each configuration.
std::vector<std::unique_ptr<ResourceConfigValue>> values;
+ // The resource's values that are behind disabled flags.
+ std::vector<std::unique_ptr<ResourceConfigValue>> flag_disabled_values;
+
explicit ResourceEntry(android::StringPiece name) : name(name) {
}
@@ -148,6 +151,13 @@
android::StringPiece product);
std::vector<ResourceConfigValue*> FindAllValues(const android::ConfigDescription& config);
+ // Either returns the existing ResourceConfigValue in the disabled list with the given flag,
+ // config, and product or creates a new one and returns that. In either case the returned value
+ // does not have the flag set on the value so it must be set by the caller.
+ ResourceConfigValue* FindOrCreateFlagDisabledValue(const FeatureFlagAttribute& flag,
+ const android::ConfigDescription& config,
+ android::StringPiece product = {});
+
template <typename Func>
std::vector<ResourceConfigValue*> FindValuesIf(Func f) {
std::vector<ResourceConfigValue*> results;
@@ -215,9 +225,14 @@
std::optional<OverlayableItem> overlayable_item;
std::optional<StagedId> staged_id;
std::vector<const ResourceConfigValue*> values;
+ std::vector<const ResourceConfigValue*> flag_disabled_values;
const ResourceConfigValue* FindValue(const android::ConfigDescription& config,
android::StringPiece product = {}) const;
+
+ const ResourceConfigValue* FindFlagDisabledValue(const FeatureFlagAttribute& flag,
+ const android::ConfigDescription& config,
+ android::StringPiece product = {}) const;
};
struct ResourceTableTypeView {
@@ -269,7 +284,6 @@
std::optional<AllowNew> allow_new;
std::optional<StagedId> staged_id;
bool allow_mangled = false;
- FlagStatus flag_status = FlagStatus::NoFlag;
};
struct NewResourceBuilder {
@@ -283,7 +297,6 @@
NewResourceBuilder& SetAllowNew(AllowNew allow_new);
NewResourceBuilder& SetStagedId(StagedId id);
NewResourceBuilder& SetAllowMangled(bool allow_mangled);
- NewResourceBuilder& SetFlagStatus(FlagStatus flag_status);
NewResource Build();
private:
diff --git a/tools/aapt2/ResourceValues.cpp b/tools/aapt2/ResourceValues.cpp
index b75e87c..723cfc0 100644
--- a/tools/aapt2/ResourceValues.cpp
+++ b/tools/aapt2/ResourceValues.cpp
@@ -1102,6 +1102,7 @@
std::unique_ptr<T> CopyValueFields(std::unique_ptr<T> new_value, const T* value) {
new_value->SetSource(value->GetSource());
new_value->SetComment(value->GetComment());
+ new_value->SetFlag(value->GetFlag());
new_value->SetFlagStatus(value->GetFlagStatus());
return new_value;
}
diff --git a/tools/aapt2/ResourceValues.h b/tools/aapt2/ResourceValues.h
index a1b1839..e000c65 100644
--- a/tools/aapt2/ResourceValues.h
+++ b/tools/aapt2/ResourceValues.h
@@ -65,10 +65,21 @@
return translatable_;
}
+ void SetFlag(std::optional<FeatureFlagAttribute> val) {
+ flag_ = val;
+ }
+
+ std::optional<FeatureFlagAttribute> GetFlag() const {
+ return flag_;
+ }
+
void SetFlagStatus(FlagStatus val) {
flag_status_ = val;
}
+ // If the value is behind a flag this returns whether that flag was enabled when the value was
+ // parsed by comparing it to the flags passed on the command line to aapt2 (taking into account
+ // negation if necessary). If there was no flag, FlagStatus::NoFlag is returned instead.
FlagStatus GetFlagStatus() const {
return flag_status_;
}
@@ -128,6 +139,7 @@
std::string comment_;
bool weak_ = false;
bool translatable_ = true;
+ std::optional<FeatureFlagAttribute> flag_;
FlagStatus flag_status_ = FlagStatus::NoFlag;
private:
diff --git a/tools/aapt2/Resources.proto b/tools/aapt2/Resources.proto
index 5c64089..a0f60b6 100644
--- a/tools/aapt2/Resources.proto
+++ b/tools/aapt2/Resources.proto
@@ -240,6 +240,9 @@
// The staged resource ID of this finalized resource.
StagedId staged_id = 7;
+
+ // The set of values defined for this entry which are behind disabled flags
+ repeated ConfigValue flag_disabled_config_value = 8;
}
// A Configuration/Value pair.
@@ -283,6 +286,8 @@
// The status of the flag the value is behind if any
uint32 flag_status = 8;
+ bool flag_negated = 9;
+ string flag_name = 10;
}
// A CompoundValue is an abstract type. It represents a value that is a made of other values.
diff --git a/tools/aapt2/cmd/Diff.cpp b/tools/aapt2/cmd/Diff.cpp
index 6da3176..d3750a6 100644
--- a/tools/aapt2/cmd/Diff.cpp
+++ b/tools/aapt2/cmd/Diff.cpp
@@ -138,6 +138,22 @@
}
}
+ for (const ResourceConfigValue* config_value_a : entry_a.flag_disabled_values) {
+ auto config_value_b = entry_b.FindFlagDisabledValue(config_value_a->value->GetFlag().value(),
+ config_value_a->config);
+ if (!config_value_b) {
+ std::stringstream str_stream;
+ str_stream << "missing disabled value " << pkg_a.name << ":" << type_a.named_type << "/"
+ << entry_a.name << " config=" << config_value_a->config
+ << " flag=" << config_value_a->value->GetFlag()->ToString();
+ EmitDiffLine(apk_b->GetSource(), str_stream.str());
+ diff = true;
+ } else {
+ diff |= EmitResourceConfigValueDiff(context, apk_a, pkg_a, type_a, entry_a, config_value_a,
+ apk_b, pkg_b, type_b, entry_b, config_value_b);
+ }
+ }
+
// Check for any newly added config values.
for (const ResourceConfigValue* config_value_b : entry_b.values) {
auto config_value_a = entry_a.FindValue(config_value_b->config);
@@ -149,6 +165,18 @@
diff = true;
}
}
+ for (const ResourceConfigValue* config_value_b : entry_b.flag_disabled_values) {
+ auto config_value_a = entry_a.FindFlagDisabledValue(config_value_b->value->GetFlag().value(),
+ config_value_b->config);
+ if (!config_value_a) {
+ std::stringstream str_stream;
+ str_stream << "new disabled config " << pkg_b.name << ":" << type_b.named_type << "/"
+ << entry_b.name << " config=" << config_value_b->config
+ << " flag=" << config_value_b->value->GetFlag()->ToString();
+ EmitDiffLine(apk_b->GetSource(), str_stream.str());
+ diff = true;
+ }
+ }
return diff;
}
diff --git a/tools/aapt2/cmd/Util.cpp b/tools/aapt2/cmd/Util.cpp
index 7739171..2177c34 100644
--- a/tools/aapt2/cmd/Util.cpp
+++ b/tools/aapt2/cmd/Util.cpp
@@ -34,6 +34,30 @@
namespace aapt {
+std::optional<FlagStatus> GetFlagStatus(const std::optional<FeatureFlagAttribute>& flag,
+ const FeatureFlagValues& feature_flag_values,
+ std::string* out_err) {
+ if (!flag) {
+ return FlagStatus::NoFlag;
+ }
+ auto flag_it = feature_flag_values.find(flag->name);
+ if (flag_it == feature_flag_values.end()) {
+ *out_err = "Resource flag value undefined: " + flag->name;
+ return {};
+ }
+ const auto& flag_properties = flag_it->second;
+ if (!flag_properties.read_only) {
+ *out_err = "Only read only flags may be used with resources: " + flag->name;
+ return {};
+ }
+ if (!flag_properties.enabled.has_value()) {
+ *out_err = "Only flags with a value may be used with resources: " + flag->name;
+ return {};
+ }
+ return (flag_properties.enabled.value() != flag->negated) ? FlagStatus::Enabled
+ : FlagStatus::Disabled;
+}
+
std::optional<uint16_t> ParseTargetDensityParameter(StringPiece arg, android::IDiagnostics* diag) {
ConfigDescription preferred_density_config;
if (!ConfigDescription::Parse(arg, &preferred_density_config)) {
diff --git a/tools/aapt2/cmd/Util.h b/tools/aapt2/cmd/Util.h
index 6b8813b..f8e44b7 100644
--- a/tools/aapt2/cmd/Util.h
+++ b/tools/aapt2/cmd/Util.h
@@ -49,6 +49,10 @@
using FeatureFlagValues = std::map<std::string, FeatureFlagProperties, std::less<>>;
+std::optional<FlagStatus> GetFlagStatus(const std::optional<FeatureFlagAttribute>& flag,
+ const FeatureFlagValues& feature_flag_values,
+ std::string* out_err);
+
// Parses a configuration density (ex. hdpi, xxhdpi, 234dpi, anydpi, etc).
// Returns Nothing and logs a human friendly error message if the string was not legal.
std::optional<uint16_t> ParseTargetDensityParameter(android::StringPiece arg,
diff --git a/tools/aapt2/format/proto/ProtoDeserialize.cpp b/tools/aapt2/format/proto/ProtoDeserialize.cpp
index 55f5e56..c6bd6dd 100644
--- a/tools/aapt2/format/proto/ProtoDeserialize.cpp
+++ b/tools/aapt2/format/proto/ProtoDeserialize.cpp
@@ -536,6 +536,34 @@
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;
+ }
+ }
+
+ // flag disabled
+ for (const pb::ConfigValue& pb_config_value : pb_entry.flag_disabled_config_value()) {
+ const pb::Configuration& pb_config = pb_config_value.config();
+
+ ConfigDescription config;
+ if (!DeserializeConfigFromPb(pb_config, &config, out_error)) {
+ 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());
+ if (config_value->value != nullptr) {
+ *out_error = "duplicate configuration in resource table";
+ return false;
+ }
+
+ 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;
}
@@ -748,7 +776,6 @@
if (value == nullptr) {
return {};
}
-
} else if (pb_value.has_compound_value()) {
const pb::CompoundValue& pb_compound_value = pb_value.compound_value();
switch (pb_compound_value.value_case()) {
@@ -1018,6 +1045,12 @@
DeserializeItemFromPbInternal(pb_item, src_pool, config, value_pool, files, out_error);
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));
+ }
}
return item;
}
diff --git a/tools/aapt2/format/proto/ProtoSerialize.cpp b/tools/aapt2/format/proto/ProtoSerialize.cpp
index 5772b3b..9c28780 100644
--- a/tools/aapt2/format/proto/ProtoSerialize.cpp
+++ b/tools/aapt2/format/proto/ProtoSerialize.cpp
@@ -427,6 +427,14 @@
SerializeValueToPb(*config_value->value, pb_config_value->mutable_value(),
source_pool.get());
}
+
+ for (const ResourceConfigValue* config_value : entry.flag_disabled_values) {
+ pb::ConfigValue* pb_config_value = pb_entry->add_flag_disabled_config_value();
+ SerializeConfig(config_value->config, pb_config_value->mutable_config());
+ pb_config_value->mutable_config()->set_product(config_value->product);
+ SerializeValueToPb(*config_value->value, pb_config_value->mutable_value(),
+ source_pool.get());
+ }
}
}
}
@@ -721,6 +729,11 @@
}
if (out_value->has_item()) {
out_value->mutable_item()->set_flag_status((uint32_t)value.GetFlagStatus());
+ if (value.GetFlag()) {
+ const auto& flag = value.GetFlag();
+ out_value->mutable_item()->set_flag_negated(flag->negated);
+ out_value->mutable_item()->set_flag_name(flag->name);
+ }
}
}
@@ -730,6 +743,11 @@
item.Accept(&serializer);
out_item->MergeFrom(value.item());
out_item->set_flag_status((uint32_t)item.GetFlagStatus());
+ if (item.GetFlag()) {
+ const auto& flag = item.GetFlag();
+ out_item->set_flag_negated(flag->negated);
+ out_item->set_flag_name(flag->name);
+ }
}
void SerializeCompiledFileToPb(const ResourceFile& file, pb::internal::CompiledFile* out_file) {
diff --git a/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools.xml b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools.xml
index 1ed0c8a..7837e17 100644
--- a/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools.xml
+++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools.xml
@@ -9,4 +9,10 @@
<bool name="bool3">false</bool>
<bool name="bool4" android:featureFlag="test.package.falseFlag">true</bool>
+
+ <bool name="bool5">false</bool>
+ <bool name="bool5" android:featureFlag="!test.package.falseFlag">true</bool>
+
+ <bool name="bool6">true</bool>
+ <bool name="bool6" android:featureFlag="!test.package.trueFlag">false</bool>
</resources>
\ No newline at end of file
diff --git a/tools/aapt2/link/FlaggedResources_test.cpp b/tools/aapt2/link/FlaggedResources_test.cpp
index 3db37c2..67d0c41 100644
--- a/tools/aapt2/link/FlaggedResources_test.cpp
+++ b/tools/aapt2/link/FlaggedResources_test.cpp
@@ -17,6 +17,7 @@
#include "LoadedApk.h"
#include "cmd/Dump.h"
#include "io/StringStream.h"
+#include "test/Common.h"
#include "test/Test.h"
#include "text/Printer.h"
@@ -98,4 +99,47 @@
ASSERT_NE(r_contents.find("public static final int str1"), std::string::npos);
}
+TEST_F(FlaggedResourcesTest, TwoValuesSameDisabledFlag) {
+ test::TestDiagnosticsImpl diag;
+ const std::string compiled_files_dir = GetTestPath("compiled");
+ ASSERT_FALSE(CompileFile(
+ GetTestPath("res/values/values.xml"),
+ R"(<resources xmlns:android="http://schemas.android.com/apk/res/android">
+ <bool name="bool1" android:featureFlag="test.package.falseFlag">false</bool>
+ <bool name="bool1" android:featureFlag="test.package.falseFlag">true</bool>
+ </resources>)",
+ compiled_files_dir, &diag,
+ {"--feature-flags", "test.package.falseFlag:ro=false,test.package.trueFlag:ro=true"}));
+ ASSERT_TRUE(diag.GetLog().contains("duplicate value for resource 'bool/bool1'"));
+}
+
+TEST_F(FlaggedResourcesTest, TwoValuesSameDisabledFlagDifferentFiles) {
+ test::TestDiagnosticsImpl diag;
+ const std::string compiled_files_dir = GetTestPath("compiled");
+ ASSERT_TRUE(CompileFile(
+ GetTestPath("res/values/values1.xml"),
+ R"(<resources xmlns:android="http://schemas.android.com/apk/res/android">
+ <bool name="bool1" android:featureFlag="test.package.falseFlag">false</bool>
+ </resources>)",
+ compiled_files_dir, &diag,
+ {"--feature-flags", "test.package.falseFlag:ro=false,test.package.trueFlag:ro=true"}));
+ ASSERT_TRUE(CompileFile(
+ GetTestPath("res/values/values2.xml"),
+ R"(<resources xmlns:android="http://schemas.android.com/apk/res/android">
+ <bool name="bool1" android:featureFlag="test.package.falseFlag">true</bool>
+ </resources>)",
+ compiled_files_dir, &diag,
+ {"--feature-flags", "test.package.falseFlag:ro=false,test.package.trueFlag:ro=true"}));
+ const std::string out_apk = GetTestPath("out.apk");
+ std::vector<std::string> link_args = {
+ "--manifest",
+ GetDefaultManifest(),
+ "-o",
+ out_apk,
+ };
+
+ ASSERT_FALSE(Link(link_args, compiled_files_dir, &diag));
+ ASSERT_TRUE(diag.GetLog().contains("duplicate value for resource 'bool1'"));
+}
+
} // namespace aapt
diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp
index 37a039e..d216979 100644
--- a/tools/aapt2/link/TableMerger.cpp
+++ b/tools/aapt2/link/TableMerger.cpp
@@ -321,6 +321,30 @@
}
}
}
+
+ // disabled values
+ for (auto& src_config_value : src_entry->flag_disabled_values) {
+ auto dst_config_value = dst_entry->FindOrCreateFlagDisabledValue(
+ src_config_value->value->GetFlag().value(), src_config_value->config,
+ src_config_value->product);
+ if (!dst_config_value->value) {
+ // Resource does not exist, add it now.
+ // Must clone the value since it might be in the values vector as well
+ CloningValueTransformer cloner(&main_table_->string_pool);
+ dst_config_value->value = src_config_value->value->Transform(cloner);
+ } else {
+ error = true;
+ context_->GetDiagnostics()->Error(
+ android::DiagMessage(src_config_value->value->GetSource())
+ << "duplicate value for resource '" << src_entry->name << "' " << "with config '"
+ << src_config_value->config << "' and flag '"
+ << (src_config_value->value->GetFlag()->negated ? "!" : "")
+ << src_config_value->value->GetFlag()->name << "'");
+ context_->GetDiagnostics()->Note(
+ android::DiagMessage(dst_config_value->value->GetSource())
+ << "resource previously defined here");
+ }
+ }
}
}
return !error;
diff --git a/tools/aapt2/test/Common.cpp b/tools/aapt2/test/Common.cpp
index cdf24534..c7dd4c9 100644
--- a/tools/aapt2/test/Common.cpp
+++ b/tools/aapt2/test/Common.cpp
@@ -21,23 +21,6 @@
namespace aapt {
namespace test {
-struct TestDiagnosticsImpl : public android::IDiagnostics {
- void Log(Level level, android::DiagMessageActual& actual_msg) override {
- switch (level) {
- case Level::Note:
- return;
-
- case Level::Warn:
- std::cerr << actual_msg.source << ": warn: " << actual_msg.message << "." << std::endl;
- break;
-
- case Level::Error:
- std::cerr << actual_msg.source << ": error: " << actual_msg.message << "." << std::endl;
- break;
- }
- }
-};
-
android::IDiagnostics* GetDiagnostics() {
static TestDiagnosticsImpl diag;
return &diag;
diff --git a/tools/aapt2/test/Common.h b/tools/aapt2/test/Common.h
index 0437980..b06c432 100644
--- a/tools/aapt2/test/Common.h
+++ b/tools/aapt2/test/Common.h
@@ -37,6 +37,32 @@
namespace aapt {
namespace test {
+struct TestDiagnosticsImpl : public android::IDiagnostics {
+ void Log(Level level, android::DiagMessageActual& actual_msg) override {
+ switch (level) {
+ case Level::Note:
+ return;
+
+ case Level::Warn:
+ std::cerr << actual_msg.source << ": warn: " << actual_msg.message << "." << std::endl;
+ log << actual_msg.source << ": warn: " << actual_msg.message << "." << std::endl;
+ break;
+
+ case Level::Error:
+ std::cerr << actual_msg.source << ": error: " << actual_msg.message << "." << std::endl;
+ log << actual_msg.source << ": error: " << actual_msg.message << "." << std::endl;
+ break;
+ }
+ }
+
+ std::string GetLog() {
+ return log.str();
+ }
+
+ private:
+ std::ostringstream log;
+};
+
android::IDiagnostics* GetDiagnostics();
inline ResourceName ParseNameOrDie(android::StringPiece str) {
diff --git a/tools/aapt2/test/Fixture.cpp b/tools/aapt2/test/Fixture.cpp
index b91abe5..570bcf1 100644
--- a/tools/aapt2/test/Fixture.cpp
+++ b/tools/aapt2/test/Fixture.cpp
@@ -91,10 +91,13 @@
}
bool CommandTestFixture::CompileFile(const std::string& path, const std::string& contents,
- android::StringPiece out_dir, android::IDiagnostics* diag) {
+ android::StringPiece out_dir, android::IDiagnostics* diag,
+ const std::vector<android::StringPiece>& additional_args) {
WriteFile(path, contents);
CHECK(file::mkdirs(out_dir.data()));
- return CompileCommand(diag).Execute({path, "-o", out_dir, "-v"}, &std::cerr) == 0;
+ std::vector<android::StringPiece> args = {path, "-o", out_dir, "-v"};
+ args.insert(args.end(), additional_args.begin(), additional_args.end());
+ return CompileCommand(diag).Execute(args, &std::cerr) == 0;
}
bool CommandTestFixture::Link(const std::vector<std::string>& args, android::IDiagnostics* diag) {
diff --git a/tools/aapt2/test/Fixture.h b/tools/aapt2/test/Fixture.h
index 14298d16..178d011 100644
--- a/tools/aapt2/test/Fixture.h
+++ b/tools/aapt2/test/Fixture.h
@@ -73,7 +73,8 @@
// Wries the contents of the file to the specified path. The file is compiled and the flattened
// file is written to the out directory.
bool CompileFile(const std::string& path, const std::string& contents,
- android::StringPiece flat_out_dir, android::IDiagnostics* diag);
+ android::StringPiece flat_out_dir, android::IDiagnostics* diag,
+ const std::vector<android::StringPiece>& additional_args = {});
// Executes the link command with the specified arguments.
bool Link(const std::vector<std::string>& args, android::IDiagnostics* diag);