HalManifest::checkIncompatibleHals uses instances API.
Now that <hal> is not the smallest unit of a manifest,
the compatibility check logic is updated accordingly.
Test: libvintf_test
Test: vintf_object_test
Bug: 73556059
Change-Id: Ief73afc61bace0dcc3d02410c16c7e261fa63315
diff --git a/HalManifest.cpp b/HalManifest.cpp
index a27e935..0e3be91 100644
--- a/HalManifest.cpp
+++ b/HalManifest.cpp
@@ -199,89 +199,12 @@
return true;
}
-static bool satisfyVersion(const MatrixHal& matrixHal, const Version& manifestHalVersion) {
- for (const VersionRange &matrixVersionRange : matrixHal.versionRanges) {
- // If Compatibility Matrix says 2.5-2.7, the "2.7" is purely informational;
- // the framework can work with all 2.5-2.infinity.
- if (matrixVersionRange.supportedBy(manifestHalVersion)) {
- return true;
- }
- }
- return false;
-}
-
-// Check if matrixHal.interfaces is a subset of instancesOfVersion
-static bool satisfyAllInstances(const MatrixHal& matrixHal,
- const InstancesOfVersion &instancesOfVersion) {
- for (const auto& matrixHalInterfacePair : matrixHal.interfaces) {
- const std::string& interface = matrixHalInterfacePair.first;
- auto it = instancesOfVersion.find(interface);
- if (it == instancesOfVersion.end()) {
- return false;
- }
- const std::set<std::string>& manifestInterfaceInstances = it->second;
- const std::set<std::string>& matrixInterfaceInstances =
- matrixHalInterfacePair.second.instances;
- if (!std::includes(manifestInterfaceInstances.begin(), manifestInterfaceInstances.end(),
- matrixInterfaceInstances.begin(), matrixInterfaceInstances.end())) {
- return false;
- }
- }
- return true;
-}
-
-Instances HalManifest::expandInstances(const std::string& name) const {
- Instances instances;
- // Do the cross product version x interface x instance and sort them,
- // because interfaces / instances can span in multiple HALs.
- // This is efficient for small <hal> entries.
- for (const ManifestHal* manifestHal : getHals(name)) {
- for (const Version& manifestHalVersion : manifestHal->versions) {
- instances[manifestHalVersion] = {};
- for (const auto& halInterfacePair : manifestHal->interfaces) {
- const std::string& interface = halInterfacePair.first;
- const auto& toAdd = halInterfacePair.second.instances;
- instances[manifestHalVersion][interface].insert(toAdd.begin(), toAdd.end());
- }
- }
- }
- return instances;
-}
-
-bool HalManifest::isCompatible(const Instances& instances, const MatrixHal& matrixHal) const {
- for (const auto& instanceMapPair : instances) {
- const Version& manifestHalVersion = instanceMapPair.first;
- const InstancesOfVersion& instancesOfVersion = instanceMapPair.second;
- if (!satisfyVersion(matrixHal, manifestHalVersion)) {
- continue;
- }
- if (!satisfyAllInstances(matrixHal, instancesOfVersion)) {
- continue;
- }
- return true; // match!
- }
- return false;
-}
-
-static std::vector<std::string> toLines(const Instances& allInstances) {
- std::vector<std::string> lines;
- for (const auto& pair : allInstances) {
- const auto& version = pair.first;
- for (const auto& ifacePair : pair.second) {
- const auto& interface = ifacePair.first;
- for (const auto& instance : ifacePair.second) {
- lines.push_back(toFQNameString(version, interface, instance));
- }
- }
- }
- return lines;
-}
-
// indent = 2, {"foo"} => "foo"
// indent = 2, {"foo", "bar"} => "\n foo\n bar";
-void multilineIndent(std::ostream& os, size_t indent, const std::vector<std::string>& lines) {
+template <typename Container>
+void multilineIndent(std::ostream& os, size_t indent, const Container& lines) {
if (lines.size() == 1) {
- os << lines.front();
+ os << *lines.begin();
return;
}
for (const auto& line : lines) {
@@ -298,13 +221,29 @@
if (matrixHal.optional) {
continue;
}
- auto manifestInstances = expandInstances(matrixHal.name);
- if (!isCompatible(manifestInstances, matrixHal)) {
+
+ std::set<FqInstance> manifestInstances;
+ std::set<FqInstance> manifestInstancesNoPackage;
+ std::set<Version> versions;
+ for (const ManifestHal* manifestHal : getHals(matrixHal.name)) {
+ manifestHal->forEachInstance([&](const auto& manifestInstance) {
+ manifestInstances.insert(manifestInstance.getFqInstance());
+ manifestInstancesNoPackage.insert(manifestInstance.getFqInstanceNoPackage());
+ return true;
+ });
+ manifestHal->appendAllVersions(&versions);
+ }
+
+ if (!matrixHal.isCompatible(manifestInstances, versions)) {
std::ostringstream oss;
oss << matrixHal.name << ":\n required: ";
multilineIndent(oss, 8, android::vintf::expandInstances(matrixHal));
oss << "\n provided: ";
- multilineIndent(oss, 8, toLines(manifestInstances));
+ if (manifestInstances.empty()) {
+ multilineIndent(oss, 8, versions);
+ } else {
+ multilineIndent(oss, 8, manifestInstancesNoPackage);
+ }
ret.insert(ret.end(), oss.str());
}
diff --git a/ManifestHal.cpp b/ManifestHal.cpp
index 7d687fc..cd30f16 100644
--- a/ManifestHal.cpp
+++ b/ManifestHal.cpp
@@ -75,5 +75,13 @@
return !hasInstance;
}
+void ManifestHal::appendAllVersions(std::set<Version>* ret) const {
+ ret->insert(versions.begin(), versions.end());
+ forEachInstance([&](const auto& e) {
+ ret->insert(e.version());
+ return true;
+ });
+}
+
} // namespace vintf
} // namespace android
diff --git a/ManifestInstance.cpp b/ManifestInstance.cpp
index 37662fc..9679a6e 100644
--- a/ManifestInstance.cpp
+++ b/ManifestInstance.cpp
@@ -60,5 +60,9 @@
return mTransportArch.arch;
}
+const FqInstance& ManifestInstance::getFqInstance() const {
+ return mFqInstance;
+}
+
} // namespace vintf
} // namespace android
diff --git a/MatrixHal.cpp b/MatrixHal.cpp
index fbe9a5f..9788545 100644
--- a/MatrixHal.cpp
+++ b/MatrixHal.cpp
@@ -70,14 +70,22 @@
bool MatrixHal::forEachInstance(const std::function<bool(const MatrixInstance&)>& func) const {
for (const auto& vr : versionRanges) {
- for (const auto& intf : iterateValues(interfaces)) {
- for (const auto& instance : intf.instances) {
- // TODO(b/73556059): Store MatrixInstance as well to avoid creating temps
- FqInstance fqInstance;
- if (fqInstance.setTo(getName(), vr.majorVer, vr.minMinor, intf.name, instance)) {
- if (!func(MatrixInstance(std::move(fqInstance), VersionRange(vr), optional))) {
- return false;
- }
+ if (!forEachInstance(vr, func)) {
+ return false;
+ }
+ }
+ return true;
+}
+
+bool MatrixHal::forEachInstance(const VersionRange& vr,
+ const std::function<bool(const MatrixInstance&)>& func) const {
+ for (const auto& intf : iterateValues(interfaces)) {
+ for (const auto& instance : intf.instances) {
+ // TODO(b/73556059): Store MatrixInstance as well to avoid creating temps
+ FqInstance fqInstance;
+ if (fqInstance.setTo(getName(), vr.majorVer, vr.minMinor, intf.name, instance)) {
+ if (!func(MatrixInstance(std::move(fqInstance), VersionRange(vr), optional))) {
+ return false;
}
}
}
@@ -85,5 +93,42 @@
return true;
}
+bool MatrixHal::isCompatible(const std::set<FqInstance>& providedInstances,
+ const std::set<Version>& providedVersions) const {
+ // <version>'s are related by OR.
+ return std::any_of(versionRanges.begin(), versionRanges.end(), [&](const VersionRange& vr) {
+ return isCompatible(vr, providedInstances, providedVersions);
+ });
+}
+
+bool MatrixHal::isCompatible(const VersionRange& vr, const std::set<FqInstance>& providedInstances,
+ const std::set<Version>& providedVersions) const {
+ bool hasAnyInstance = false;
+ bool versionUnsatisfied = false;
+
+ // Look at each interface/instance, and ensure that they are in providedInstances.
+ forEachInstance(vr, [&](const MatrixInstance& matrixInstance) {
+ hasAnyInstance = true;
+
+ versionUnsatisfied |=
+ !std::any_of(providedInstances.begin(), providedInstances.end(),
+ [&](const FqInstance& providedInstance) {
+ return matrixInstance.isSatisfiedBy(providedInstance);
+ });
+
+ return !versionUnsatisfied; // if any interface/instance is unsatisfied, break
+ });
+
+ if (hasAnyInstance) {
+ return !versionUnsatisfied;
+ }
+
+ // In some cases (e.g. tests and native HALs), compatibility matrix doesn't specify
+ // any instances. Check versions only.
+ return std::any_of(
+ providedVersions.begin(), providedVersions.end(),
+ [&](const auto& providedVersion) { return vr.supportedBy(providedVersion); });
+}
+
} // namespace vintf
} // namespace android
diff --git a/MatrixInstance.cpp b/MatrixInstance.cpp
index e411fe2..0ac3681 100644
--- a/MatrixInstance.cpp
+++ b/MatrixInstance.cpp
@@ -58,5 +58,11 @@
return mOptional;
}
+bool MatrixInstance::isSatisfiedBy(const FqInstance& provided) const {
+ return package() == provided.getPackage() &&
+ versionRange().supportedBy(provided.getVersion()) &&
+ interface() == provided.getInterface() && instance() == provided.getInstance();
+}
+
} // namespace vintf
} // namespace android
diff --git a/include/vintf/ManifestHal.h b/include/vintf/ManifestHal.h
index 9fca823..91ae570 100644
--- a/include/vintf/ManifestHal.h
+++ b/include/vintf/ManifestHal.h
@@ -78,6 +78,9 @@
// (constructed via ManifestHal()) is valid.
bool isValid() const;
+ // Return all versions mentioned by <version>s and <fqname>s.
+ void appendAllVersions(std::set<Version>* ret) const;
+
bool mIsOverride = false;
};
diff --git a/include/vintf/ManifestInstance.h b/include/vintf/ManifestInstance.h
index 6aea665..abe1d71 100644
--- a/include/vintf/ManifestInstance.h
+++ b/include/vintf/ManifestInstance.h
@@ -45,6 +45,10 @@
Transport transport() const;
Arch arch() const;
+ // Convenience methods.
+ // return package@version::interface/instance
+ const FqInstance& getFqInstance() const;
+
private:
FqInstance mFqInstance;
TransportArch mTransportArch;
diff --git a/include/vintf/MatrixHal.h b/include/vintf/MatrixHal.h
index 1959467..301e1ee 100644
--- a/include/vintf/MatrixHal.h
+++ b/include/vintf/MatrixHal.h
@@ -52,6 +52,15 @@
bool containsInstances(const MatrixHal& other) const;
bool forEachInstance(const std::function<bool(const MatrixInstance&)>& func) const;
+
+ private:
+ friend struct HalManifest;
+ bool forEachInstance(const VersionRange& vr,
+ const std::function<bool(const MatrixInstance&)>& 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;
};
} // namespace vintf
diff --git a/include/vintf/MatrixInstance.h b/include/vintf/MatrixInstance.h
index 4e64268..c3d2a0b 100644
--- a/include/vintf/MatrixInstance.h
+++ b/include/vintf/MatrixInstance.h
@@ -44,6 +44,8 @@
const std::string& instance() const;
bool optional() const;
+ bool isSatisfiedBy(const FqInstance& provided) const;
+
private:
FqInstance mFqInstance;
VersionRange mRange;
diff --git a/parse_string.cpp b/parse_string.cpp
index 2aa3c1b..51ca009 100644
--- a/parse_string.cpp
+++ b/parse_string.cpp
@@ -395,16 +395,22 @@
<< (req.optional ? kOptional : kRequired);
}
-static std::string expandInstances(const MatrixHal& req, const VersionRange& vr) {
+static std::string expandInstances(const MatrixHal& req, const VersionRange& vr, bool brace) {
std::string s;
- bool first = true;
+ size_t count = 0;
for (const auto& interface : iterateValues(req.interfaces)) {
for (const auto& instance : interface.instances) {
- if (!first) s += " AND ";
+ if (count > 0) s += " AND ";
s += toFQNameString(vr, interface.name, instance);
- first = false;
+ count++;
}
}
+ if (count == 0) {
+ s += "@" + to_string(vr);
+ }
+ if (count >= 2 && brace) {
+ s = "(" + s + ")";
+ }
return s;
}
@@ -414,14 +420,14 @@
}
if (req.versionRanges.size() == 1 && req.interfaces.size() == 1 &&
req.interfaces.begin()->second.instances.size() == 1) {
- return {expandInstances(req, req.versionRanges.front())};
+ return {expandInstances(req, req.versionRanges.front(), false /* brace */)};
}
std::vector<std::string> ss;
for (const auto& vr : req.versionRanges) {
if (!ss.empty()) {
ss.back() += " OR";
}
- ss.push_back("(" + expandInstances(req, vr) + ")");
+ ss.push_back(expandInstances(req, vr, true /* brace */));
}
return ss;
}
diff --git a/test/LibVintfTest.cpp b/test/LibVintfTest.cpp
index 0177467..9a243f4 100644
--- a/test/LibVintfTest.cpp
+++ b/test/LibVintfTest.cpp
@@ -1042,7 +1042,7 @@
HalManifest manifest;
EXPECT_TRUE(gHalManifestConverter(&manifest, manifestXml));
EXPECT_FALSE(manifest.checkCompatibility(matrix, &error))
- << "should not be compatible because IFoo/default is missing";
+ << "should not be compatible because IFoo/specific is missing";
}
{