CompatibilityMatrix::combine() combines everything correctly am: e7837b164c
am: dd4789a548
Change-Id: I6874463e29a07263b2e428a0c7b39fe0316918b7
diff --git a/AssembleVintf.cpp b/AssembleVintf.cpp
index 272301e..cc8db31 100644
--- a/AssembleVintf.cpp
+++ b/AssembleVintf.cpp
@@ -412,6 +412,7 @@
return true;
}
+ // Parse --kernel arguments and write to output matrix.
bool assembleFrameworkCompatibilityMatrixKernels(CompatibilityMatrix* matrix) {
for (auto& pair : mKernels) {
std::vector<ConditionedConfig> conditionedConfigs;
@@ -422,7 +423,11 @@
MatrixKernel kernel(KernelVersion{pair.first}, std::move(conditionedConfig.second));
if (conditionedConfig.first != nullptr)
kernel.mConditions.push_back(std::move(*conditionedConfig.first));
- matrix->framework.mKernels.push_back(std::move(kernel));
+ std::string error;
+ if (!matrix->addKernel(std::move(kernel), &error)) {
+ std::cerr << "Error:" << error << std::endl;
+ return false;
+ };
}
}
return true;
@@ -484,7 +489,24 @@
std::string error;
CompatibilityMatrix* matrix = nullptr;
std::unique_ptr<HalManifest> checkManifest;
+ std::unique_ptr<CompatibilityMatrix> builtMatrix;
+
+ if (mCheckFile != nullptr) {
+ checkManifest = std::make_unique<HalManifest>();
+ if (!gHalManifestConverter(checkManifest.get(), read(*mCheckFile), &error)) {
+ std::cerr << "Cannot parse check file as a HAL manifest: " << error << std::endl;
+ return false;
+ }
+ }
+
if (matrices->front().object.mType == SchemaType::DEVICE) {
+ if (matrices->size() > 1) {
+ std::cerr
+ << "assemble_vintf does not support merging device compatibilility matrices."
+ << std::endl;
+ return false;
+ }
+
matrix = &matrices->front().object;
auto vndkVersion = base::Trim(getEnv("REQUIRED_VNDK_VERSION"));
@@ -506,24 +528,20 @@
}
if (matrices->front().object.mType == SchemaType::FRAMEWORK) {
- Level deviceLevel = Level::UNSPECIFIED;
- if (mCheckFile != nullptr) {
- checkManifest = std::make_unique<HalManifest>();
- if (!gHalManifestConverter(checkManifest.get(), read(*mCheckFile), &error)) {
- std::cerr << "Cannot parse check file as a HAL manifest: " << error
- << std::endl;
- return false;
- }
- deviceLevel = checkManifest->level();
- }
-
+ Level deviceLevel =
+ checkManifest != nullptr ? checkManifest->level() : Level::UNSPECIFIED;
if (deviceLevel == Level::UNSPECIFIED) {
- // For GSI build, legacy devices that do not have a HAL manifest,
- // and devices in development, merge all compatibility matrices.
deviceLevel = getLowestFcmVersion(*matrices);
+ if (checkManifest != nullptr && deviceLevel != Level::UNSPECIFIED) {
+ std::cerr << "Warning: No Target FCM Version for device. Assuming \""
+ << to_string(deviceLevel)
+ << "\" when building final framework compatibility matrix."
+ << std::endl;
+ }
}
+ builtMatrix = CompatibilityMatrix::combine(deviceLevel, matrices, &error);
+ matrix = builtMatrix.get();
- matrix = CompatibilityMatrix::combine(deviceLevel, matrices, &error);
if (matrix == nullptr) {
std::cerr << error << std::endl;
return false;
@@ -558,9 +576,9 @@
}
getFlagIfUnset("POLICYVERS", &matrix->framework.mSepolicy.mKernelSepolicyVersion,
- deviceLevel == Level::UNSPECIFIED /* log */);
+ false /* log */);
getFlagIfUnset("FRAMEWORK_VBMETA_VERSION", &matrix->framework.mAvbMetaVersion,
- deviceLevel == Level::UNSPECIFIED /* log */);
+ false /* log */);
// Hard-override existing AVB version
getFlag("FRAMEWORK_VBMETA_VERSION_OVERRIDE", &matrix->framework.mAvbMetaVersion,
false /* log */);
diff --git a/CompatibilityMatrix.cpp b/CompatibilityMatrix.cpp
index 6a5645c..290ebf9 100644
--- a/CompatibilityMatrix.cpp
+++ b/CompatibilityMatrix.cpp
@@ -19,6 +19,8 @@
#include <iostream>
#include <utility>
+#include <android-base/strings.h>
+
#include "parse_string.h"
#include "parse_xml.h"
#include "utils.h"
@@ -26,14 +28,59 @@
namespace android {
namespace vintf {
-bool CompatibilityMatrix::add(MatrixHal &&hal) {
- return HalGroup<MatrixHal>::add(std::move(hal));
-}
-
-bool CompatibilityMatrix::add(MatrixKernel &&kernel) {
+bool CompatibilityMatrix::addKernel(MatrixKernel&& kernel, std::string* error) {
if (mType != SchemaType::FRAMEWORK) {
+ if (error) {
+ *error = "Cannot add <kernel> to a " + to_string(mType) + " compatibility matrix.";
+ }
return false;
}
+
+ auto it = framework.mKernels.begin();
+ for (; it != framework.mKernels.end(); ++it) {
+ if (it->minLts() == kernel.minLts()) {
+ break;
+ }
+ if (it->minLts().version == kernel.minLts().version &&
+ it->minLts().majorRev == kernel.minLts().majorRev) {
+ if (error) {
+ *error = "Kernel version mismatch; cannot add " + to_string(kernel.minLts()) +
+ " because " + to_string(it->minLts()) + " was added.";
+ }
+ return false;
+ }
+ }
+
+ bool seenVersion = it != framework.mKernels.end();
+
+ if (seenVersion) {
+ // If no conditions, must be the first among the same minLts
+ // because O libvintf only checks the first <kernel> tag that version matches.
+ if (kernel.conditions().empty()) {
+ // Found first <kernel> with the same minLts.
+ // Append config if it does not have <condition>s, else error.
+ if (it->conditions().empty()) {
+ const auto& configs = kernel.configs();
+ it->mConfigs.insert(it->mConfigs.end(), configs.begin(), configs.end());
+ } else {
+ if (error) {
+ *error =
+ "Base compatibility matrix has <condition> for the first <kernel> "
+ "with minlts " +
+ to_string(kernel.minLts()) + " for unknown reason.";
+ }
+ return false;
+ }
+ return true;
+ }
+ } else {
+ // First <kernel> of a minLts must not have <condition>'s for backwards compatibility
+ // with O libvintf.
+ if (!kernel.conditions().empty()) {
+ framework.mKernels.push_back(MatrixKernel(KernelVersion{kernel.minLts()}, {}));
+ }
+ }
+
framework.mKernels.push_back(std::move(kernel));
return true;
}
@@ -196,6 +243,27 @@
return true;
}
+// Merge Kernel.
+// Add <kernel> from exact "level", then optionally add <kernel> from high levels to low levels.
+// For example, (each letter is a kernel version x.y.z)
+// 1.xml: A1, B1
+// 2.xml: B2, C2, D2
+// 3.xml: D3, E3
+// Then the combined 1.xml should have
+// A1, B1 (from 1.xml, required), C2, D2, E3 (optional, use earliest possible).
+bool CompatibilityMatrix::addAllKernels(CompatibilityMatrix* other, std::string* error) {
+ for (MatrixKernel& kernel : other->framework.mKernels) {
+ KernelVersion ver = kernel.minLts();
+ if (!addKernel(std::move(kernel), error)) {
+ if (error) {
+ *error = "Cannot add kernel version " + to_string(ver) + ": " + *error;
+ }
+ return false;
+ }
+ }
+ return true;
+}
+
bool CompatibilityMatrix::addAllKernelsAsOptional(CompatibilityMatrix* other, std::string* error) {
if (other == nullptr || other->level() <= level()) {
return true;
@@ -216,9 +284,9 @@
}
KernelVersion minLts = kernelToAdd.minLts();
- if (!add(std::move(kernelToAdd))) {
+ if (!addKernel(std::move(kernelToAdd), error)) {
if (error) {
- *error = "Cannot add " + to_string(minLts) + " for unknown reason.";
+ *error = "Cannot add " + to_string(minLts) + ": " + *error;
}
return false;
}
@@ -226,6 +294,34 @@
return true;
}
+template <typename T>
+static bool mergeField(T* dst, T* src) {
+ static const T kEmpty{};
+ if (*dst == *src) {
+ return true; // no conflict
+ }
+ if (*src == kEmpty) {
+ return true;
+ }
+ if (*dst == kEmpty) {
+ *dst = std::move(*src);
+ return true;
+ }
+ return false;
+}
+
+bool CompatibilityMatrix::addSepolicy(CompatibilityMatrix* other, std::string* error) {
+ bool success = mergeField(&this->framework.mSepolicy, &other->framework.mSepolicy);
+ if (!success && error) *error = "<sepolicy> is already defined";
+ return success;
+}
+
+bool CompatibilityMatrix::addAvbMetaVersion(CompatibilityMatrix* other, std::string* error) {
+ bool success = mergeField(&this->framework.mAvbMetaVersion, &other->framework.mAvbMetaVersion);
+ if (!success && error) *error = "<avb><vbmeta-version> is already defined";
+ return success;
+}
+
bool operator==(const CompatibilityMatrix &lft, const CompatibilityMatrix &rgt) {
return lft.mType == rgt.mType && lft.mLevel == rgt.mLevel && lft.mHals == rgt.mHals &&
lft.mXmlFiles == rgt.mXmlFiles &&
@@ -243,175 +339,83 @@
lft.framework.mAvbMetaVersion == rgt.framework.mAvbMetaVersion));
}
-// Find compatibility_matrix.empty.xml (which has unspecified level) and use it
-// as a base matrix.
-CompatibilityMatrix* CompatibilityMatrix::findOrInsertBaseMatrix(
- std::vector<Named<CompatibilityMatrix>>* matrices, std::string* error) {
- std::vector<CompatibilityMatrix*> matricesUnspecified;
- std::vector<CompatibilityMatrix*> matricesEmpty;
+std::unique_ptr<CompatibilityMatrix> CompatibilityMatrix::combine(
+ Level deviceLevel, std::vector<Named<CompatibilityMatrix>>* matrices, std::string* error) {
+ // Check type.
+ for (const auto& e : *matrices) {
+ if (e.object.type() != SchemaType::FRAMEWORK) {
+ if (error) {
+ *error = "File \"" + e.name + "\" is not a framework compatibility matrix.";
+ return nullptr;
+ }
+ }
+ }
+
+ // Matrices with unspecified (empty) level are auto-filled with deviceLevel.
for (auto& e : *matrices) {
if (e.object.level() == Level::UNSPECIFIED) {
- matricesUnspecified.push_back(&e.object);
-
- if (!e.object.mHals.empty()) {
- continue;
- }
-
- if (!e.object.mXmlFiles.empty()) {
- continue;
- }
-
- matricesEmpty.push_back(&e.object);
+ e.object.mLevel = deviceLevel;
}
}
- if (matricesEmpty.size() > 1) {
- if (error) {
- *error =
- "Error: multiple framework compatibility matrix files have "
- "unspecified level; there should only be one such file.\n";
- for (auto& e : *matrices) {
- if (e.object.level() == Level::UNSPECIFIED) {
- *error += " " + e.name + "\n";
- }
- }
+ // Add from low to high FCM version so that optional <kernel> requirements are added correctly.
+ // See comment in addAllAsOptional.
+ std::sort(matrices->begin(), matrices->end(),
+ [](const auto& x, const auto& y) { return x.object.level() < y.object.level(); });
+
+ auto baseMatrix = std::make_unique<CompatibilityMatrix>();
+ baseMatrix->mLevel = deviceLevel;
+ baseMatrix->mType = SchemaType::FRAMEWORK;
+
+ std::vector<std::string> parsedFiles;
+ for (auto& e : *matrices) {
+ if (e.object.level() < deviceLevel) {
+ continue;
}
- return nullptr;
+
+ bool success = false;
+ if (e.object.level() == deviceLevel) {
+ success = baseMatrix->addAll(&e, error);
+ } else {
+ success = baseMatrix->addAllAsOptional(&e, error);
+ }
+ if (!success) {
+ if (error) {
+ *error = "Conflict when merging \"" + e.name + "\": " + *error + "\n" +
+ "Previous files:\n" + base::Join(parsedFiles, "\n");
+ }
+ return nullptr;
+ }
+ parsedFiles.push_back(e.name);
}
- if (matricesEmpty.size() == 1) {
- return matricesEmpty.front();
- }
- if (!matricesUnspecified.empty()) {
- return matricesUnspecified.front();
- }
- auto matrix = &matrices->emplace(matrices->end())->object;
- matrix->mType = SchemaType::FRAMEWORK;
- matrix->mLevel = Level::UNSPECIFIED;
- return matrix;
+
+ return baseMatrix;
}
-// Check if there are two files declaring level="1", for example, because
-// combine() use this assumption to simplify a lot of logic.
-static bool checkDuplicateLevels(const std::vector<Named<CompatibilityMatrix>>& matrices,
- std::string* error) {
- std::map<Level, const std::string*> existingLevels;
- for (const auto& e : matrices) {
- if (e.object.level() != Level::UNSPECIFIED &&
- existingLevels.find(e.object.level()) != existingLevels.end()) {
- if (error) {
- *error = "Conflict of levels: file \"" +
- *existingLevels.find(e.object.level())->second + "\" and \"" + e.name +
- " both have level " + to_string(e.object.level());
- }
- return false;
+bool CompatibilityMatrix::addAll(Named<CompatibilityMatrix>* inputMatrix, std::string* error) {
+ if (!addAllHals(&inputMatrix->object, error) || !addAllXmlFiles(&inputMatrix->object, error) ||
+ !addAllKernels(&inputMatrix->object, error) || !addSepolicy(&inputMatrix->object, error) ||
+ !addAvbMetaVersion(&inputMatrix->object, error)) {
+ if (error) {
+ *error = "File \"" + inputMatrix->name + "\" cannot be added: " + *error + ".";
}
- existingLevels.emplace(e.object.level(), &e.name);
}
return true;
}
-CompatibilityMatrix* CompatibilityMatrix::combine(Level deviceLevel,
- std::vector<Named<CompatibilityMatrix>>* matrices,
- std::string* error) {
- if (!checkDuplicateLevels(*matrices, error)) {
- return nullptr;
- }
-
- CompatibilityMatrix* matrix = findOrInsertBaseMatrix(matrices, error);
- if (matrix == nullptr) {
- return nullptr;
- }
-
- matrix->mLevel = deviceLevel;
-
- for (auto& e : *matrices) {
- if (&e.object != matrix &&
- (e.object.level() == deviceLevel || e.object.level() == Level::UNSPECIFIED)) {
- if (!matrix->addAllHals(&e.object, error)) {
- if (error) {
- *error = "File \"" + e.name + "\" cannot be added: HAL " + *error +
- " has a conflict.";
- }
- return nullptr;
- }
-
- if (!matrix->addAllXmlFiles(&e.object, error)) {
- if (error) {
- *error = "File \"" + e.name + "\" cannot be added: XML File entry " + *error +
- " has a conflict.";
- }
- return nullptr;
- }
+bool CompatibilityMatrix::addAllAsOptional(Named<CompatibilityMatrix>* inputMatrix,
+ std::string* error) {
+ if (!addAllHalsAsOptional(&inputMatrix->object, error) ||
+ !addAllXmlFilesAsOptional(&inputMatrix->object, error) ||
+ !addAllKernelsAsOptional(&inputMatrix->object, error)) {
+ if (error) {
+ *error = "File \"" + inputMatrix->name + "\" cannot be added: " + *error;
}
+ return false;
}
-
- for (auto& e : *matrices) {
- if (&e.object != matrix && e.object.level() != Level::UNSPECIFIED &&
- e.object.level() > deviceLevel) {
- if (!matrix->addAllHalsAsOptional(&e.object, error)) {
- if (error) {
- *error = "File \"" + e.name + "\" cannot be added: " + *error +
- ". See <hal> with the same name " +
- "in previously parsed files or previously declared in this file.";
- }
- return nullptr;
- }
-
- if (!matrix->addAllXmlFilesAsOptional(&e.object, error)) {
- if (error) {
- *error = "File \"" + e.name + "\" cannot be added: XML File entry " + *error +
- " has a conflict.";
- }
- return nullptr;
- }
- }
- }
-
- // Add <kernel> from exact "level", then optionally add <kernel> from high levels to low levels.
- // For example, (each letter is a kernel version x.y.z)
- // 1.xml: A1, B1
- // 2.xml: B2, C2, D2
- // 3.xml: D3, E3
- // Then the combined 1.xml should have
- // A1, B1 (from 1.xml, required), C2, D2, E3 (optional, use earliest possible).
- for (auto& e : *matrices) {
- if (&e.object != matrix && e.object.level() == deviceLevel &&
- e.object.type() == SchemaType::FRAMEWORK) {
- for (MatrixKernel& kernel : e.object.framework.mKernels) {
- KernelVersion ver = kernel.minLts();
- if (!matrix->add(std::move(kernel))) {
- if (error) {
- *error = "Cannot add kernel version " + to_string(ver) +
- " from FCM version " + to_string(deviceLevel);
- }
- return nullptr;
- }
- }
- }
- }
-
- // There is only one file per level, hence a map is used instead of a multimap. Also, there is
- // no good ordering (i.e. priority) for multiple files with the same level.
- std::map<Level, Named<CompatibilityMatrix>*> matricesMap;
- for (auto& e : *matrices) {
- if (&e.object != matrix && e.object.level() != Level::UNSPECIFIED &&
- e.object.level() > deviceLevel && e.object.type() == SchemaType::FRAMEWORK) {
- matricesMap.emplace(e.object.level(), &e);
- }
- }
-
- for (auto&& pair : matricesMap) {
- if (!matrix->addAllKernelsAsOptional(&pair.second->object, error)) {
- if (error) {
- *error = "Cannot add new kernel versions from FCM version " +
- to_string(pair.first) + " (" + pair.second->name + ")" +
- " to FCM version " + to_string(deviceLevel) + ": " + *error;
- }
- return nullptr;
- }
- }
-
- return matrix;
+ // ignore <sepolicy> requirement from higher level
+ // ignore <avb> requirement from higher level
+ return true;
}
bool CompatibilityMatrix::forEachInstanceOfVersion(
diff --git a/VintfObject.cpp b/VintfObject.cpp
index 1086590..aed9745 100644
--- a/VintfObject.cpp
+++ b/VintfObject.cpp
@@ -196,8 +196,7 @@
return NAME_NOT_FOUND;
}
- CompatibilityMatrix* combined =
- CompatibilityMatrix::combine(deviceLevel, &matrixFragments, error);
+ auto combined = CompatibilityMatrix::combine(deviceLevel, &matrixFragments, error);
if (combined == nullptr) {
return BAD_VALUE;
}
diff --git a/include/vintf/CompatibilityMatrix.h b/include/vintf/CompatibilityMatrix.h
index 512d278..feb8ac8 100644
--- a/include/vintf/CompatibilityMatrix.h
+++ b/include/vintf/CompatibilityMatrix.h
@@ -18,6 +18,7 @@
#define ANDROID_VINTF_COMPATIBILITY_MATRIX_H
#include <map>
+#include <memory>
#include <string>
#include <utils/Errors.h>
@@ -67,8 +68,23 @@
std::string getVendorNdkVersion() const;
private:
- bool add(MatrixHal &&hal);
- bool add(MatrixKernel &&kernel);
+ // Add everything in inputMatrix to "this" as requirements.
+ bool addAll(Named<CompatibilityMatrix>* inputMatrix, std::string* error);
+
+ // Add all <kernel> from other to "this". Error if there is a conflict.
+ bool addAllKernels(CompatibilityMatrix* other, std::string* error);
+
+ // Add a <kernel> tag to "this". Error if there is a conflict.
+ bool addKernel(MatrixKernel&& kernel, std::string* error);
+
+ // Merge <sepolicy> with other's <sepolicy>. Error if there is a conflict.
+ bool addSepolicy(CompatibilityMatrix* other, std::string* error);
+
+ // Merge <avb><vbmeta-version> with other's <avb><vbmeta-version>. Error if there is a conflict.
+ bool addAvbMetaVersion(CompatibilityMatrix* other, std::string* error);
+
+ // Add everything in inputMatrix to "this" as optional.
+ bool addAllAsOptional(Named<CompatibilityMatrix>* inputMatrix, std::string* error);
// Add all HALs as optional HALs from "other". This function moves MatrixHal objects
// from "other".
@@ -81,23 +97,23 @@
// Similar to addAllHalsAsOptional but on <kernel> entries.
bool addAllKernelsAsOptional(CompatibilityMatrix* other, std::string* error);
+ // Combine a set of framework compatibility matrices. For each CompatibilityMatrix in matrices
+ // (in the order of level(), where UNSPECIFIED (empty) is treated as deviceLevel)
+ // - If level() < deviceLevel, ignore
+ // - If level() == UNSPECIFIED or level() == deviceLevel,
+ // - Add as hard requirements. See combineSameFcmVersion
+ // - If level() > deviceLevel,
+ // - all <hal> versions and <xmlfile>s are added as optional.
+ // - <kernel minlts="x.y.z"> is added only if x.y does not exist in a file
+ // with lower level()
+ // - <sepolicy>, <avb><vbmeta-version> is ignored
+ // Return the combined matrix, nullptr if any error (e.g. conflict of information).
+ static std::unique_ptr<CompatibilityMatrix> combine(
+ Level deviceLevel, std::vector<Named<CompatibilityMatrix>>* matrices, std::string* error);
+
status_t fetchAllInformation(const FileSystem* fileSystem, const std::string& path,
std::string* error = nullptr);
- // Combine a subset of "matrices". For each CompatibilityMatrix in matrices,
- // - If level() == UNSPECIFIED, use it as the base matrix (for non-HAL, non-XML-file
- // requirements).
- // - If level() < deviceLevel, ignore
- // - If level() == deviceLevel, all HAL versions and XML files are added as is
- // (optionality is kept)
- // - If level() > deviceLevel, all HAL versions and XML files are added as optional.
- // Return a pointer into one of the elements in "matrices".
- static CompatibilityMatrix* combine(Level deviceLevel,
- std::vector<Named<CompatibilityMatrix>>* matrices,
- std::string* error);
- static CompatibilityMatrix* findOrInsertBaseMatrix(
- std::vector<Named<CompatibilityMatrix>>* matrices, std::string* error);
-
MatrixHal* splitInstance(MatrixHal* existingHal, const std::string& interface,
const std::string& instance, bool isRegex);
diff --git a/include/vintf/HalGroup.h b/include/vintf/HalGroup.h
index a0a14cd..5cad867 100644
--- a/include/vintf/HalGroup.h
+++ b/include/vintf/HalGroup.h
@@ -39,7 +39,7 @@
for (auto& pair : other->mHals) {
if (!add(std::move(pair.second))) {
if (error) {
- *error = pair.first;
+ *error = "HAL \"" + pair.first + "\" has a conflict.";
}
return false;
}
diff --git a/include/vintf/MatrixKernel.h b/include/vintf/MatrixKernel.h
index cee7e32..14cf5bf 100644
--- a/include/vintf/MatrixKernel.h
+++ b/include/vintf/MatrixKernel.h
@@ -58,6 +58,7 @@
private:
friend struct MatrixKernelConverter;
friend struct MatrixKernelConditionsConverter;
+ friend struct CompatibilityMatrix;
friend class AssembleVintfImpl;
KernelVersion mMinLts;
diff --git a/include/vintf/XmlFileGroup.h b/include/vintf/XmlFileGroup.h
index 524efc9..dd15647 100644
--- a/include/vintf/XmlFileGroup.h
+++ b/include/vintf/XmlFileGroup.h
@@ -62,7 +62,7 @@
for (auto& pair : other->mXmlFiles) {
if (!addXmlFile(std::move(pair.second))) {
if (error) {
- *error = pair.first;
+ *error = "XML File \"" + pair.first + "\" has a conflict.";
}
return false;
}
diff --git a/test/LibVintfTest.cpp b/test/LibVintfTest.cpp
index 68af9bb..a3a98b8 100644
--- a/test/LibVintfTest.cpp
+++ b/test/LibVintfTest.cpp
@@ -59,7 +59,10 @@
return cm.add(std::move(hal));
}
bool add(CompatibilityMatrix &cm, MatrixKernel &&kernel) {
- return cm.add(std::move(kernel));
+ std::string error;
+ bool success = cm.addKernel(std::move(kernel), &error);
+ EXPECT_EQ(success, error == "") << "success: " << success << ", error: " << error;
+ return success;
}
bool add(HalManifest &vm, ManifestHal &&hal) {
return vm.add(std::move(hal));