CompatibilityMatrix::combine handles multiple instances.
Fix an edge case:
1.xml states 1.0/default AND 1.0/custom
2.xml states 2.0/default AND 2.0/strong
The old code generates:
(1.0/default AND 1.0/custom) OR
(2.0/default AND 2.0/custom)
The new code correctly generates:
(1.0/default OR 2.0/default) AND
(1.0/custom) AND
(Optional 2.0/strong)
Test: libvintf_test
Test: vintf_object_test
Bug: 74341777
Change-Id: I07f0529b9bc1d046c269fc41f33be307f599422a
Merged-In: I07f0529b9bc1d046c269fc41f33be307f599422a
diff --git a/CompatibilityMatrix.cpp b/CompatibilityMatrix.cpp
index 3f05881..5b216f7 100644
--- a/CompatibilityMatrix.cpp
+++ b/CompatibilityMatrix.cpp
@@ -75,6 +75,30 @@
return "";
}
+// Split existingHal into a HAL that contains only interface/instance and a HAL
+// that does not contain it. Return the HAL that contains only interface/instance.
+// - Return nullptr if existingHal does not contain interface/instance
+// - Return existingHal if existingHal contains only interface/instance
+// - Remove interface/instance from existingHal, and return a new MatrixHal (that is added
+// to "this") that contains only interface/instance.
+MatrixHal* CompatibilityMatrix::splitInstance(MatrixHal* existingHal, const std::string& interface,
+ const std::string& instance) {
+ if (!existingHal->hasInstance(interface, instance)) {
+ return nullptr;
+ }
+
+ if (existingHal->hasOnlyInstance(interface, instance)) {
+ return existingHal;
+ }
+
+ existingHal->removeInstance(interface, instance);
+ MatrixHal copy = *existingHal;
+ copy.clearInstances();
+ copy.insertInstance(interface, instance);
+
+ return addInternal(std::move(copy));
+}
+
// Add all package@other_version::interface/instance as an optional instance.
// If package@this_version::interface/instance is in this (that is, some instance
// with the same package and interface and instance exists), then other_version is
@@ -89,16 +113,27 @@
const std::string& name = pair.first;
MatrixHal& halToAdd = pair.second;
- bool added = false;
- for (auto* existingHal : getHals(name)) {
- if (existingHal->containsInstances(halToAdd)) {
- existingHal->insertVersionRanges(halToAdd);
- added = true;
- // Do not break here; try other <hal> with the same name as well.
+ std::set<std::pair<std::string, std::string>> insertedInstances;
+ auto existingHals = getHals(name);
+
+ halToAdd.forEachInstance([&](const std::vector<VersionRange>& versionRanges,
+ const std::string& interface, const std::string& instance) {
+ for (auto* existingHal : existingHals) {
+ MatrixHal* splitInstance = this->splitInstance(existingHal, interface, instance);
+ if (splitInstance != nullptr) {
+ splitInstance->insertVersionRanges(versionRanges);
+ insertedInstances.insert(std::make_pair(interface, instance));
+ }
}
+ return true;
+ });
+
+ // Add the remaining instances.
+ for (const auto& pair : insertedInstances) {
+ halToAdd.removeInstance(pair.first, pair.second);
}
- if (!added) {
+ if (halToAdd.hasAnyInstance()) {
halToAdd.setOptional(true);
if (!add(std::move(halToAdd))) {
if (error) {
diff --git a/MatrixHal.cpp b/MatrixHal.cpp
index 504265e..62e0cd8 100644
--- a/MatrixHal.cpp
+++ b/MatrixHal.cpp
@@ -52,24 +52,6 @@
return ret;
}
-bool MatrixHal::containsInstances(const MatrixHal& other) const {
- for (const auto& pair : other.interfaces) {
- const std::string& interfaceName = pair.first;
- auto thisIt = interfaces.find(interfaceName);
- if (thisIt == interfaces.end()) {
- return false;
- }
-
- const std::set<std::string>& thisInstances = thisIt->second.instances;
- const std::set<std::string>& otherInstances = pair.second.instances;
- if (!std::includes(thisInstances.begin(), thisInstances.end(), otherInstances.begin(),
- otherInstances.end())) {
- return false;
- }
- }
- return true;
-}
-
bool MatrixHal::forEachInstance(const std::function<bool(const MatrixInstance&)>& func) const {
for (const auto& vr : versionRanges) {
if (!forEachInstance(vr, func)) {
@@ -95,6 +77,19 @@
return true;
}
+bool MatrixHal::forEachInstance(
+ const std::function<bool(const std::vector<VersionRange>&, const std::string&,
+ const std::string&)>& func) const {
+ for (const auto& intf : iterateValues(interfaces)) {
+ for (const auto& instance : intf.instances) {
+ if (!func(versionRanges, intf.name, instance)) {
+ return false;
+ }
+ }
+ }
+ return true;
+}
+
bool MatrixHal::isCompatible(const std::set<FqInstance>& providedInstances,
const std::set<Version>& providedVersions) const {
// <version>'s are related by OR.
@@ -136,8 +131,8 @@
this->optional = o;
}
-void MatrixHal::insertVersionRanges(const MatrixHal& other) {
- for (const VersionRange& otherVr : other.versionRanges) {
+void MatrixHal::insertVersionRanges(const std::vector<VersionRange>& other) {
+ for (const VersionRange& otherVr : other) {
auto existingVr = std::find_if(this->versionRanges.begin(), this->versionRanges.end(),
[&](const auto& e) { return e.overlaps(otherVr); });
@@ -150,5 +145,59 @@
}
}
+void MatrixHal::insertInstance(const std::string& interface, const std::string& instance) {
+ auto it = interfaces.find(interface);
+ if (it == interfaces.end())
+ it = interfaces.emplace(interface, HalInterface{interface, {}}).first;
+ it->second.instances.insert(instance);
+}
+
+bool MatrixHal::hasAnyInstance() const {
+ bool found = false;
+ forEachInstance([&](const auto&) {
+ found = true;
+ return false; // break if any instance
+ });
+ return found;
+}
+
+bool MatrixHal::hasInstance(const std::string& interface, const std::string& instance) const {
+ bool found = false;
+ forEachInstance([&](const auto& matrixInstance) {
+ found |= matrixInstance.interface() == interface && matrixInstance.instance() == instance;
+ return !found; // continue if not match
+ });
+ return found;
+}
+
+bool MatrixHal::hasOnlyInstance(const std::string& interface, const std::string& instance) const {
+ bool found = false;
+ bool foundOthers = false;
+
+ forEachInstance([&](const auto& matrixInstance) {
+ bool match =
+ matrixInstance.interface() == interface && matrixInstance.instance() == instance;
+
+ found |= match;
+ foundOthers |= (!match);
+
+ return !foundOthers;
+ });
+
+ return found && !foundOthers;
+}
+
+bool MatrixHal::removeInstance(const std::string& interface, const std::string& instance) {
+ auto it = interfaces.find(interface);
+ if (it == interfaces.end()) return false;
+ it->second.instances.erase(instance);
+ if (it->second.instances.empty()) interfaces.erase(it);
+ return true;
+}
+
+void MatrixHal::clearInstances() {
+ this->interfaces.clear();
+}
+
} // namespace vintf
} // namespace android
diff --git a/include/vintf/CompatibilityMatrix.h b/include/vintf/CompatibilityMatrix.h
index 6431836..02326ae 100644
--- a/include/vintf/CompatibilityMatrix.h
+++ b/include/vintf/CompatibilityMatrix.h
@@ -91,6 +91,9 @@
static CompatibilityMatrix* findOrInsertBaseMatrix(
std::vector<Named<CompatibilityMatrix>>* matrices, std::string* error);
+ MatrixHal* splitInstance(MatrixHal* existingHal, const std::string& interface,
+ const std::string& instance);
+
friend struct HalManifest;
friend struct RuntimeInfo;
friend struct CompatibilityMatrixConverter;
diff --git a/include/vintf/HalGroup.h b/include/vintf/HalGroup.h
index 033adf0..2a1e1c3 100644
--- a/include/vintf/HalGroup.h
+++ b/include/vintf/HalGroup.h
@@ -48,14 +48,7 @@
}
// Add an hal to this HalGroup so that it can be constructed programatically.
- virtual bool add(Hal&& hal) {
- if (!shouldAdd(hal)) {
- return false;
- }
- std::string name = hal.getName();
- mHals.emplace(std::move(name), std::move(hal)); // always succeed
- return true;
- }
+ virtual bool add(Hal&& hal) { return addInternal(std::move(hal)) != nullptr; }
// Get all hals with the given name (e.g "android.hardware.camera").
// There could be multiple hals that matches the same given name.
@@ -191,6 +184,15 @@
}
return &(it->second);
}
+
+ Hal* addInternal(Hal&& hal) {
+ if (!shouldAdd(hal)) {
+ return nullptr;
+ }
+ std::string name = hal.getName();
+ auto it = mHals.emplace(std::move(name), std::move(hal)); // always succeeds
+ return &it->second;
+ }
};
} // namespace vintf
diff --git a/include/vintf/MatrixHal.h b/include/vintf/MatrixHal.h
index 6044792..595c57a 100644
--- a/include/vintf/MatrixHal.h
+++ b/include/vintf/MatrixHal.h
@@ -48,23 +48,35 @@
inline const std::string& getName() const { return name; }
- // Return true if "this" contains all interface/instance instances in "other".
- bool containsInstances(const MatrixHal& other) const;
-
bool forEachInstance(const std::function<bool(const MatrixInstance&)>& func) const;
private:
friend struct HalManifest;
friend struct CompatibilityMatrix;
+ // Loop over interface/instance for a specific VersionRange.
bool forEachInstance(const VersionRange& vr,
const std::function<bool(const MatrixInstance&)>& func) const;
+ // Loop over interface/instance. VersionRange is supplied to the function as a vector.
+ bool forEachInstance(
+ const std::function<bool(const std::vector<VersionRange>&, const std::string&,
+ const std::string&)>& func) const;
bool isCompatible(const std::set<FqInstance>& providedInstances,
const std::set<Version>& providedVersions) const;
bool isCompatible(const VersionRange& vr, const std::set<FqInstance>& providedInstances,
const std::set<Version>& providedVersions) const;
void setOptional(bool o);
- void insertVersionRanges(const MatrixHal& other);
+ void insertVersionRanges(const std::vector<VersionRange>& other);
+ void insertInstance(const std::string& interface, const std::string& instance);
+ // Return true if it has any interface/instance tags.
+ bool hasAnyInstance() const;
+ bool hasInstance(const std::string& interface, const std::string& instance) const;
+ // Return true if it contains only interface/instance.
+ bool hasOnlyInstance(const std::string& interface, const std::string& instance) const;
+ // Remove a specific interface/instances. Return true if removed, false otherwise.
+ bool removeInstance(const std::string& interface, const std::string& instance);
+ // Remove all <interface> tags.
+ void clearInstances();
};
} // namespace vintf
diff --git a/test/LibVintfTest.cpp b/test/LibVintfTest.cpp
index fc7ebc9..8c42d77 100644
--- a/test/LibVintfTest.cpp
+++ b/test/LibVintfTest.cpp
@@ -2275,6 +2275,203 @@
"</compatibility-matrix>\n");
}
+TEST_F(LibVintfTest, AddRequiredHalOverlapInstance) {
+ CompatibilityMatrix cm1;
+ std::string error;
+ std::string xml;
+
+ xml =
+ "<compatibility-matrix version=\"1.0\" type=\"framework\" level=\"1\">\n"
+ " <hal format=\"hidl\" optional=\"false\">\n"
+ " <name>android.hardware.foo</name>\n"
+ " <version>1.0</version>\n"
+ " <interface>\n"
+ " <name>IFoo</name>\n"
+ " <instance>default</instance>\n"
+ " <instance>custom</instance>\n"
+ " </interface>\n"
+ " </hal>\n"
+ "</compatibility-matrix>\n";
+ EXPECT_TRUE(gCompatibilityMatrixConverter(&cm1, xml))
+ << gCompatibilityMatrixConverter.lastError();
+
+ {
+ // Test that 2.0 should be added to IFoo/default, so 1.0::IFoo/custom
+ // should be in a new <hal> tag
+ CompatibilityMatrix cm2;
+ xml =
+ "<compatibility-matrix version=\"1.0\" type=\"framework\" level=\"2\">\n"
+ " <hal format=\"hidl\" optional=\"false\">\n"
+ " <name>android.hardware.foo</name>\n"
+ " <version>2.0</version>\n"
+ " <interface>\n"
+ " <name>IFoo</name>\n"
+ " <instance>default</instance>\n"
+ " </interface>\n"
+ " </hal>\n"
+ "</compatibility-matrix>\n";
+ EXPECT_TRUE(gCompatibilityMatrixConverter(&cm2, xml))
+ << gCompatibilityMatrixConverter.lastError();
+
+ EXPECT_TRUE(addAllHalsAsOptional(&cm1, &cm2, &error)) << error;
+
+ xml = gCompatibilityMatrixConverter(cm1, SerializeFlag::HALS_ONLY);
+ EXPECT_EQ(xml,
+ "<compatibility-matrix version=\"1.0\" type=\"framework\" level=\"1\">\n"
+ " <hal format=\"hidl\" optional=\"false\">\n"
+ " <name>android.hardware.foo</name>\n"
+ " <version>1.0</version>\n"
+ " <interface>\n"
+ " <name>IFoo</name>\n"
+ " <instance>custom</instance>\n"
+ " </interface>\n"
+ " </hal>\n"
+ " <hal format=\"hidl\" optional=\"false\">\n"
+ " <name>android.hardware.foo</name>\n"
+ " <version>1.0</version>\n"
+ " <version>2.0</version>\n"
+ " <interface>\n"
+ " <name>IFoo</name>\n"
+ " <instance>default</instance>\n"
+ " </interface>\n"
+ " </hal>\n"
+ "</compatibility-matrix>\n");
+ }
+
+ {
+ // Test that 2.0::IFoo/strong should be added as an optional <hal> tag.
+ CompatibilityMatrix cm2;
+ xml =
+ "<compatibility-matrix version=\"1.0\" type=\"framework\" level=\"2\">\n"
+ " <hal format=\"hidl\" optional=\"false\">\n"
+ " <name>android.hardware.foo</name>\n"
+ " <version>2.0</version>\n"
+ " <interface>\n"
+ " <name>IFoo</name>\n"
+ " <instance>default</instance>\n"
+ " <instance>strong</instance>\n"
+ " </interface>\n"
+ " </hal>\n"
+ "</compatibility-matrix>\n";
+ EXPECT_TRUE(gCompatibilityMatrixConverter(&cm2, xml))
+ << gCompatibilityMatrixConverter.lastError();
+
+ EXPECT_TRUE(addAllHalsAsOptional(&cm1, &cm2, &error)) << error;
+
+ xml = gCompatibilityMatrixConverter(cm1, SerializeFlag::HALS_ONLY);
+ EXPECT_EQ(xml,
+ "<compatibility-matrix version=\"1.0\" type=\"framework\" level=\"1\">\n"
+ " <hal format=\"hidl\" optional=\"false\">\n"
+ " <name>android.hardware.foo</name>\n"
+ " <version>1.0</version>\n"
+ " <interface>\n"
+ " <name>IFoo</name>\n"
+ " <instance>custom</instance>\n"
+ " </interface>\n"
+ " </hal>\n"
+ " <hal format=\"hidl\" optional=\"false\">\n"
+ " <name>android.hardware.foo</name>\n"
+ " <version>1.0</version>\n"
+ " <version>2.0</version>\n"
+ " <interface>\n"
+ " <name>IFoo</name>\n"
+ " <instance>default</instance>\n"
+ " </interface>\n"
+ " </hal>\n"
+ " <hal format=\"hidl\" optional=\"true\">\n"
+ " <name>android.hardware.foo</name>\n"
+ " <version>2.0</version>\n"
+ " <interface>\n"
+ " <name>IFoo</name>\n"
+ " <instance>strong</instance>\n"
+ " </interface>\n"
+ " </hal>\n"
+ "</compatibility-matrix>\n");
+ }
+}
+
+TEST_F(LibVintfTest, AddRequiredHalOverlapInstanceSplit) {
+ CompatibilityMatrix cm1;
+ CompatibilityMatrix cm2;
+ std::string error;
+ std::string xml;
+
+ xml =
+ "<compatibility-matrix version=\"1.0\" type=\"framework\" level=\"1\">\n"
+ " <hal format=\"hidl\" optional=\"false\">\n"
+ " <name>android.hardware.foo</name>\n"
+ " <version>1.0</version>\n"
+ " <interface>\n"
+ " <name>IFoo</name>\n"
+ " <instance>default</instance>\n"
+ " </interface>\n"
+ " </hal>\n"
+ " <hal format=\"hidl\" optional=\"false\">\n"
+ " <name>android.hardware.foo</name>\n"
+ " <version>1.0</version>\n"
+ " <interface>\n"
+ " <name>IFoo</name>\n"
+ " <instance>custom</instance>\n"
+ " </interface>\n"
+ " </hal>\n"
+ "</compatibility-matrix>\n";
+ EXPECT_TRUE(gCompatibilityMatrixConverter(&cm1, xml))
+ << gCompatibilityMatrixConverter.lastError();
+
+ xml =
+ "<compatibility-matrix version=\"1.0\" type=\"framework\" level=\"2\">\n"
+ " <hal format=\"hidl\" optional=\"false\">\n"
+ " <name>android.hardware.foo</name>\n"
+ " <version>2.0</version>\n"
+ " <interface>\n"
+ " <name>IFoo</name>\n"
+ " <instance>default</instance>\n"
+ " </interface>\n"
+ " </hal>\n"
+ " <hal format=\"hidl\" optional=\"false\">\n"
+ " <name>android.hardware.foo</name>\n"
+ " <version>2.0</version>\n"
+ " <interface>\n"
+ " <name>IFoo</name>\n"
+ " <instance>strong</instance>\n"
+ " </interface>\n"
+ " </hal>\n"
+ "</compatibility-matrix>\n";
+ EXPECT_TRUE(gCompatibilityMatrixConverter(&cm2, xml))
+ << gCompatibilityMatrixConverter.lastError();
+
+ EXPECT_TRUE(addAllHalsAsOptional(&cm1, &cm2, &error)) << error;
+ xml = gCompatibilityMatrixConverter(cm1, SerializeFlag::HALS_ONLY);
+ EXPECT_EQ(
+ "<compatibility-matrix version=\"1.0\" type=\"framework\" level=\"1\">\n"
+ " <hal format=\"hidl\" optional=\"false\">\n"
+ " <name>android.hardware.foo</name>\n"
+ " <version>1.0</version>\n"
+ " <version>2.0</version>\n"
+ " <interface>\n"
+ " <name>IFoo</name>\n"
+ " <instance>default</instance>\n"
+ " </interface>\n"
+ " </hal>\n"
+ " <hal format=\"hidl\" optional=\"false\">\n"
+ " <name>android.hardware.foo</name>\n"
+ " <version>1.0</version>\n"
+ " <interface>\n"
+ " <name>IFoo</name>\n"
+ " <instance>custom</instance>\n"
+ " </interface>\n"
+ " </hal>\n"
+ " <hal format=\"hidl\" optional=\"true\">\n"
+ " <name>android.hardware.foo</name>\n"
+ " <version>2.0</version>\n"
+ " <interface>\n"
+ " <name>IFoo</name>\n"
+ " <instance>strong</instance>\n"
+ " </interface>\n"
+ " </hal>\n"
+ "</compatibility-matrix>\n",
+ xml);
+}
TEST_F(LibVintfTest, AddOptionalXmlFile) {
CompatibilityMatrix cm1;
CompatibilityMatrix cm2;