regex-instance: Fix check deprecate test
Deprecation check should check <regex-instance> as well
as <instance>.
An instance is considered deprecated if it matches
some <instance> or <regex-instance> in "old" matrix
but a higher major version / higher minor version matchese
some <instance> or <regex-instance> in "new" matrix.
For example:
1.xml: @1.0::IFoo/legacy/[0-9]+
2.xml: @2.0::IFoo/leg[a-z]+/[0-9]+
And a device manifest provides @1.0::IFoo/legacy/1, then
this instance is deprecated.
Caveat:
VintfObject::IsInstanceInUse is changed to VintfObject::ListInstances
because the matrix no longer provides an exact list of instances
that it checks, but rather a "match rule"; hence it needs to list
all instances from IServiceManager or manifest to work. However,
this doesn't work with passthrough service manager. Hence, if a HAL
is a passthrough HAL, only <instance>s can be checked.
Bug: 73738616
Test: libvintf_test
Test: vintf_object_test
Test: vts_treble_vintf_test
Change-Id: Ibb7585b157c3c1148ba78b8fcd27a113d398297a
diff --git a/VintfObject.cpp b/VintfObject.cpp
index 367dc17..604f7cb 100644
--- a/VintfObject.cpp
+++ b/VintfObject.cpp
@@ -577,10 +577,10 @@
bool VintfObject::isHalDeprecated(const MatrixHal& oldMatrixHal,
const CompatibilityMatrix& targetMatrix,
- const IsInstanceInUse& isInstanceInUse, std::string* error) {
+ const ListInstances& listInstances, std::string* error) {
bool isDeprecated = false;
oldMatrixHal.forEachInstance([&](const MatrixInstance& oldMatrixInstance) {
- if (isInstanceDeprecated(oldMatrixInstance, targetMatrix, isInstanceInUse, error)) {
+ if (isInstanceDeprecated(oldMatrixInstance, targetMatrix, listInstances, error)) {
isDeprecated = true;
}
return !isDeprecated; // continue if no deprecated instance is found.
@@ -588,30 +588,39 @@
return isDeprecated;
}
-// If isInstanceInUse(package@x.y::interface/instance), return true iff:
-// 1. package@x.?::interface/instance is not in targetMatrix; OR
-// 2. package@x.z::interface/instance is in targetMatrix but
-// !isInstanceInUse(package@x.z::interface/instance)
+// Let oldMatrixInstance = package@x.y-w::interface with instancePattern.
+// If any "servedInstance" in listInstances(package@x.y::interface) matches instancePattern, return
+// true iff:
+// 1. package@x.?::interface/servedInstance is not in targetMatrix; OR
+// 2. package@x.z::interface/servedInstance is in targetMatrix but
+// servedInstance is not in listInstances(package@x.z::interface)
bool VintfObject::isInstanceDeprecated(const MatrixInstance& oldMatrixInstance,
const CompatibilityMatrix& targetMatrix,
- const IsInstanceInUse& isInstanceInUse, std::string* error) {
+ const ListInstances& listInstances, std::string* error) {
const std::string& package = oldMatrixInstance.package();
const Version& version = oldMatrixInstance.versionRange().minVer();
const std::string& interface = oldMatrixInstance.interface();
- const std::string& instance = oldMatrixInstance.instance();
- bool oldVersionIsServed;
- Version servedVersion;
- std::tie(oldVersionIsServed, servedVersion) =
- isInstanceInUse(package, version, interface, instance);
- if (oldVersionIsServed) {
+ std::vector<std::string> instanceHint;
+ if (!oldMatrixInstance.isRegex()) {
+ instanceHint.push_back(oldMatrixInstance.exactInstance());
+ }
+
+ auto list = listInstances(package, version, interface, instanceHint);
+ for (const auto& pair : list) {
+ const std::string& servedInstance = pair.first;
+ Version servedVersion = pair.second;
+ if (!oldMatrixInstance.matchInstance(servedInstance)) {
+ continue;
+ }
+
// Find any package@x.? in target matrix, and check if instance is in target matrix.
bool foundInstance = false;
Version targetMatrixMinVer;
targetMatrix.forEachInstanceOfPackage(package, [&](const auto& targetMatrixInstance) {
if (targetMatrixInstance.versionRange().majorVer == version.majorVer &&
targetMatrixInstance.interface() == interface &&
- targetMatrixInstance.instance() == instance) {
+ targetMatrixInstance.matchInstance(servedInstance)) {
targetMatrixMinVer = targetMatrixInstance.versionRange().minVer();
foundInstance = true;
}
@@ -619,7 +628,7 @@
});
if (!foundInstance) {
if (error) {
- *error = toFQNameString(package, servedVersion, interface, instance) +
+ *error = toFQNameString(package, servedVersion, interface, servedInstance) +
" is deprecated in compatibility matrix at FCM Version " +
to_string(targetMatrix.level()) + "; it should not be served.";
}
@@ -627,23 +636,29 @@
}
// Assuming that targetMatrix requires @x.u-v, require that at least @x.u is served.
- bool targetVersionServed;
- std::tie(targetVersionServed, std::ignore) =
- isInstanceInUse(package, targetMatrixMinVer, interface, instance);
+ bool targetVersionServed = false;
+ for (const auto& newPair :
+ listInstances(package, targetMatrixMinVer, interface, instanceHint)) {
+ if (newPair.first == servedInstance) {
+ targetVersionServed = true;
+ break;
+ }
+ }
if (!targetVersionServed) {
if (error) {
- *error += toFQNameString(package, servedVersion) + " is deprecated; " +
- "require at least " + to_string(targetMatrixMinVer) + "\n";
+ *error += toFQNameString(package, servedVersion, interface, servedInstance) +
+ " is deprecated; requires at least " + to_string(targetMatrixMinVer) +
+ "\n";
}
return true;
}
}
+
return false;
}
-int32_t VintfObject::CheckDeprecation(const IsInstanceInUse& isInstanceInUse,
- std::string* error) {
+int32_t VintfObject::CheckDeprecation(const ListInstances& listInstances, std::string* error) {
auto matrixFragments = GetAllFrameworkMatrixLevels(error);
if (matrixFragments.empty()) {
if (error && error->empty())
@@ -680,7 +695,7 @@
const auto& oldMatrix = namedMatrix.object;
for (const MatrixHal& hal : oldMatrix.getHals()) {
- hasDeprecatedHals |= isHalDeprecated(hal, *targetMatrix, isInstanceInUse, error);
+ hasDeprecatedHals |= isHalDeprecated(hal, *targetMatrix, listInstances, error);
}
}
@@ -690,22 +705,18 @@
int32_t VintfObject::CheckDeprecation(std::string* error) {
using namespace std::placeholders;
auto deviceManifest = GetDeviceHalManifest();
- IsInstanceInUse inManifest = [&deviceManifest](const std::string& package, Version version,
- const std::string& interface,
- const std::string& instance) {
- std::pair<bool, Version> ret(false, Version{});
- deviceManifest->forEachInstanceOfInterface(
- package, version, interface,
- [&instance, &ret](const ManifestInstance& manifestInstance) {
- if (manifestInstance.instance() == instance) {
- ret.first = true;
- ret.second = manifestInstance.version();
- return false;
- }
- return true;
- });
- return ret;
- };
+ ListInstances inManifest =
+ [&deviceManifest](const std::string& package, Version version, const std::string& interface,
+ const std::vector<std::string>& /* hintInstances */) {
+ std::vector<std::pair<std::string, Version>> ret;
+ deviceManifest->forEachInstanceOfInterface(
+ package, version, interface, [&ret](const ManifestInstance& manifestInstance) {
+ ret.push_back(
+ std::make_pair(manifestInstance.instance(), manifestInstance.version()));
+ return true;
+ });
+ return ret;
+ };
return CheckDeprecation(inManifest, error);
}
diff --git a/include/vintf/VintfObject.h b/include/vintf/VintfObject.h
index 811c110..56581f8 100644
--- a/include/vintf/VintfObject.h
+++ b/include/vintf/VintfObject.h
@@ -110,22 +110,32 @@
std::string* error = nullptr,
DisabledChecks disabledChecks = ENABLE_ALL_CHECKS);
- using IsInstanceInUse = std::function<std::pair<bool, Version>(
+ /**
+ * A std::function that abstracts a list of "provided" instance names. Given package, version
+ * and interface, the function returns a list of instance names that matches.
+ * This function can represent a manifest, an IServiceManager, etc.
+ * If the source is passthrough service manager, a list of instance names cannot be provided.
+ * Instead, the function should call getService on each of the "hintInstances", and
+ * return those instances for which getService does not return a nullptr. This means that for
+ * passthrough HALs, the deprecation on <regex-instance>s cannot be enforced; only <instance>s
+ * can be enforced.
+ */
+ using ListInstances = std::function<std::vector<std::pair<std::string, Version>>(
const std::string& package, Version version, const std::string& interface,
- const std::string& instance)>;
+ const std::vector<std::string>& hintInstances)>;
/**
* Check deprecation on framework matrices with a provided predicate.
*
- * @param isInstanceInUse predicate that takes parameter in this format:
- * android.hardware.foo@1.0::IFoo/instance
- * and returns {true, version} if HAL is in use, where version =
+ * @param listInstances predicate that takes parameter in this format:
+ * android.hardware.foo@1.0::IFoo
+ * and returns {{"default", version}...} if HAL is in use, where version =
* first version in interfaceChain where package + major version matches.
*
* @return = 0 if success (no deprecated HALs)
* > 0 if there is at least one deprecated HAL
* < 0 if any error (mount partition fails, illformed XML, etc.)
*/
- static int32_t CheckDeprecation(const IsInstanceInUse& isInstanceInUse,
+ static int32_t CheckDeprecation(const ListInstances& listInstances,
std::string* error = nullptr);
/**
@@ -153,10 +163,10 @@
static bool isHalDeprecated(const MatrixHal& oldMatrixHal,
const CompatibilityMatrix& targetMatrix,
- const IsInstanceInUse& isInstanceInUse, std::string* error);
+ const ListInstances& listInstances, std::string* error);
static bool isInstanceDeprecated(const MatrixInstance& oldMatrixInstance,
const CompatibilityMatrix& targetMatrix,
- const IsInstanceInUse& isInstanceInUse, std::string* error);
+ const ListInstances& listInstances, std::string* error);
};
enum : int32_t {
diff --git a/test/vintf_object_tests.cpp b/test/vintf_object_tests.cpp
index 0a08b26..27c04a5 100644
--- a/test/vintf_object_tests.cpp
+++ b/test/vintf_object_tests.cpp
@@ -32,7 +32,7 @@
using namespace ::android::vintf;
using namespace ::android::vintf::details;
-using android::FQName;
+using android::FqInstance;
static bool In(const std::string& sub, const std::string& str) {
return str.find(sub) != std::string::npos;
@@ -837,23 +837,29 @@
EXPECT_TRUE(containsOdmManifest(p));
}
-struct FQInstance {
- FQName fqName;
- std::string instance;
+struct CheckedFqInstance : FqInstance {
+ CheckedFqInstance(const char* s) : CheckedFqInstance(std::string(s)) {}
+ CheckedFqInstance(const std::string& s) { CHECK(setTo(s)) << s; }
- FQInstance(const char* s) : FQInstance(std::string(s)) {}
- FQInstance(const std::string& s) {
- auto tokens = android::base::Split(s, "/");
- CHECK(2u == tokens.size());
- CHECK(FQName::parse(tokens[0], &fqName));
- instance = tokens[1];
- }
-
- Version getVersion() const {
- return Version{fqName.getPackageMajorVersion(), fqName.getPackageMinorVersion()};
- }
+ Version getVersion() const { return FqInstance::getVersion(); }
};
+static VintfObject::ListInstances getInstanceListFunc(
+ const std::vector<CheckedFqInstance>& instances) {
+ return [instances](const std::string& package, Version version, const std::string& interface,
+ const auto& /* instanceHint */) {
+ std::vector<std::pair<std::string, Version>> ret;
+ for (auto&& existing : instances) {
+ if (existing.getPackage() == package && existing.getVersion().minorAtLeast(version) &&
+ existing.getInterface() == interface) {
+ ret.push_back(std::make_pair(existing.getInstance(), existing.getVersion()));
+ }
+ }
+
+ return ret;
+ };
+}
+
class DeprecateTest : public VintfObjectTestBase {
protected:
virtual void SetUp() override {
@@ -878,24 +884,10 @@
VintfObject::GetDeviceHalManifest(true /* skipCache */);
}
- VintfObject::IsInstanceInUse getPredicate(const std::vector<FQInstance>& instances) {
- return [instances](const std::string& package, Version version,
- const std::string& interface, const std::string& instance) {
- for (auto&& existing : instances) {
- if (existing.fqName.package() == package &&
- existing.getVersion().minorAtLeast(version) &&
- existing.fqName.name() == interface && existing.instance == instance) {
- return std::make_pair(true, existing.getVersion());
- }
- }
-
- return std::make_pair(false, Version{});
- };
- }
};
TEST_F(DeprecateTest, CheckNoDeprecate) {
- auto pred = getPredicate({
+ auto pred = getInstanceListFunc({
"android.hardware.minor@1.1::IMinor/default",
"android.hardware.major@2.0::IMajor/default",
});
@@ -904,7 +896,7 @@
}
TEST_F(DeprecateTest, CheckRemoved) {
- auto pred = getPredicate({
+ auto pred = getInstanceListFunc({
"android.hardware.removed@1.0::IRemoved/default",
"android.hardware.minor@1.1::IMinor/default",
"android.hardware.major@2.0::IMajor/default",
@@ -915,7 +907,7 @@
}
TEST_F(DeprecateTest, CheckMinor) {
- auto pred = getPredicate({
+ auto pred = getInstanceListFunc({
"android.hardware.minor@1.0::IMinor/default",
"android.hardware.major@2.0::IMajor/default",
});
@@ -925,7 +917,7 @@
}
TEST_F(DeprecateTest, CheckMinorDeprecatedInstance1) {
- auto pred = getPredicate({
+ auto pred = getInstanceListFunc({
"android.hardware.minor@1.0::IMinor/legacy",
"android.hardware.minor@1.1::IMinor/default",
"android.hardware.major@2.0::IMajor/default",
@@ -936,7 +928,7 @@
}
TEST_F(DeprecateTest, CheckMinorDeprecatedInstance2) {
- auto pred = getPredicate({
+ auto pred = getInstanceListFunc({
"android.hardware.minor@1.1::IMinor/default",
"android.hardware.minor@1.1::IMinor/legacy",
"android.hardware.major@2.0::IMajor/default",
@@ -947,7 +939,7 @@
}
TEST_F(DeprecateTest, CheckMajor1) {
- auto pred = getPredicate({
+ auto pred = getInstanceListFunc({
"android.hardware.minor@1.1::IMinor/default",
"android.hardware.major@1.0::IMajor/default",
"android.hardware.major@2.0::IMajor/default",
@@ -958,7 +950,7 @@
}
TEST_F(DeprecateTest, CheckMajor2) {
- auto pred = getPredicate({
+ auto pred = getInstanceListFunc({
"android.hardware.minor@1.1::IMinor/default",
"android.hardware.major@1.0::IMajor/default",
});
