Add <fqname> to manifests.

The format of <fqname> is:
<hal>
    <name>android.hardware.foo</name>
    <!-- transport, etc. -->
    <fqname>@1.0::IFoo/default</fqname>
</hal>

It is possible to mix <fqname> and <version>
x <interface> x <instance> ; see tests for details.

This allows instances at different versions. For example,
it is now allowed to serve @1.0::IFoo/default, @1.1::IFoo/custom
simultaneously without serving @1.1::IFoo/default.

If override="true" and no <version>x<interface>x<instance> nor
<fqname>, the HAL tag is disabled. (Previously, the HAL tag
is disabled iff no <version> exists).

Bug: 73556059
Test: libvintf_test
Test: vintf_object_test

Change-Id: I80cb9ccdeec708c2c5530812913b37f8b3cc3ffa
diff --git a/ManifestHal.cpp b/ManifestHal.cpp
index cd30f16..1576c80 100644
--- a/ManifestHal.cpp
+++ b/ManifestHal.cpp
@@ -18,6 +18,7 @@
 #include <unordered_set>
 
 #include "MapValueIterator.h"
+#include "parse_string.h"
 
 namespace android {
 namespace vintf {
@@ -43,11 +44,11 @@
     if (!(transportArch == other.transportArch)) return false;
     if (interfaces != other.interfaces) return false;
     if (isOverride() != other.isOverride()) return false;
+    if (mAdditionalInstances != other.mAdditionalInstances) return false;
     return true;
 }
 
 bool ManifestHal::forEachInstance(const std::function<bool(const ManifestInstance&)>& func) const {
-    // TODO(b/73556059): support <fqname> as well.
     for (const auto& v : versions) {
         for (const auto& intf : iterateValues(interfaces)) {
             for (const auto& instance : intf.instances) {
@@ -62,6 +63,13 @@
             }
         }
     }
+
+    for (const auto& manifestInstance : mAdditionalInstances) {
+        if (!func(manifestInstance)) {
+            return false;
+        }
+    }
+
     return true;
 }
 
@@ -83,5 +91,49 @@
     });
 }
 
+static bool verifyInstances(const std::set<FqInstance>& fqInstances, std::string* error) {
+    for (const FqInstance& fqInstance : fqInstances) {
+        if (fqInstance.hasPackage()) {
+            if (error) *error = "Should not specify package: \"" + fqInstance.string() + "\"";
+            return false;
+        }
+        if (!fqInstance.hasVersion()) {
+            if (error) *error = "Should specify version: \"" + fqInstance.string() + "\"";
+            return false;
+        }
+        if (!fqInstance.hasInterface()) {
+            if (error) *error = "Should specify interface: \"" + fqInstance.string() + "\"";
+            return false;
+        }
+        if (!fqInstance.hasInstance()) {
+            if (error) *error = "Should specify instance: \"" + fqInstance.string() + "\"";
+            return false;
+        }
+    }
+    return true;
+}
+
+bool ManifestHal::insertInstances(const std::set<FqInstance>& fqInstances, std::string* error) {
+    if (!verifyInstances(fqInstances, error)) {
+        return false;
+    }
+
+    for (const FqInstance& e : fqInstances) {
+        FqInstance withPackage;
+        if (!withPackage.setTo(this->getName(), e.getMajorVersion(), e.getMinorVersion(),
+                               e.getInterface(), e.getInstance())) {
+            if (error) {
+                *error = "Cannot create FqInstance with package='" + this->getName() +
+                         "', version='" + to_string(Version(e.getVersion())) + "', interface='" +
+                         e.getInterface() + "', instance='" + e.getInstance() + "'";
+            }
+            return false;
+        }
+        mAdditionalInstances.emplace(std::move(withPackage), this->transportArch);
+    }
+
+    return true;
+}
+
 } // namespace vintf
 } // namespace android
diff --git a/ManifestInstance.cpp b/ManifestInstance.cpp
index 9679a6e..7a216eb 100644
--- a/ManifestInstance.cpp
+++ b/ManifestInstance.cpp
@@ -14,6 +14,11 @@
  * limitations under the License.
  */
 
+#ifndef LIBVINTF_TARGET
+#define LOG_TAG "libvintf"
+#include <android-base/logging.h>
+#endif
+
 #include "ManifestInstance.h"
 
 #include <utility>
@@ -64,5 +69,25 @@
     return mFqInstance;
 }
 
+bool ManifestInstance::operator==(const ManifestInstance& other) const {
+    return mFqInstance == other.mFqInstance && mTransportArch == other.mTransportArch;
+}
+bool ManifestInstance::operator<(const ManifestInstance& other) const {
+    if (mFqInstance < other.mFqInstance) return true;
+    if (other.mFqInstance < mFqInstance) return false;
+    return mTransportArch < other.mTransportArch;
+}
+
+FqInstance ManifestInstance::getFqInstanceNoPackage() const {
+    FqInstance e;
+    bool success = e.setTo(version().majorVer, version().minorVer, interface(), instance());
+#ifndef LIBVINTF_TARGET
+    CHECK(success) << "Cannot remove package from '" << mFqInstance.string() << "'";
+#else
+    (void)success;
+#endif
+    return e;
+}
+
 }  // namespace vintf
 }  // namespace android
diff --git a/include/vintf/ManifestHal.h b/include/vintf/ManifestHal.h
index 91ae570..026c7a5 100644
--- a/include/vintf/ManifestHal.h
+++ b/include/vintf/ManifestHal.h
@@ -23,6 +23,8 @@
 #include <string>
 #include <vector>
 
+#include <hidl-util/FqInstance.h>
+
 #include "HalFormat.h"
 #include "HalInterface.h"
 #include "ManifestInstance.h"
@@ -82,6 +84,14 @@
     void appendAllVersions(std::set<Version>* ret) const;
 
     bool mIsOverride = false;
+    // Additional instances to <version> x <interface> x <instance>.
+    std::set<ManifestInstance> mAdditionalInstances;
+
+    // insert instances to mAdditionalInstances.
+    // Existing instances will be ignored.
+    // Pre: all instances to be inserted must satisfy
+    // !hasPackage() && hasVersion() && hasInterface() && hasInstance()
+    bool insertInstances(const std::set<FqInstance>& fqInstances, std::string* error = nullptr);
 };
 
 } // namespace vintf
diff --git a/include/vintf/ManifestInstance.h b/include/vintf/ManifestInstance.h
index abe1d71..61abbf1 100644
--- a/include/vintf/ManifestInstance.h
+++ b/include/vintf/ManifestInstance.h
@@ -45,9 +45,14 @@
     Transport transport() const;
     Arch arch() const;
 
+    bool operator==(const ManifestInstance& other) const;
+    bool operator<(const ManifestInstance& other) const;
+
     // Convenience methods.
     // return package@version::interface/instance
     const FqInstance& getFqInstance() const;
+    // return @version::interface/instance
+    FqInstance getFqInstanceNoPackage() const;
 
    private:
     FqInstance mFqInstance;
diff --git a/include/vintf/TransportArch.h b/include/vintf/TransportArch.h
index 1bd6604..b1d9352 100644
--- a/include/vintf/TransportArch.h
+++ b/include/vintf/TransportArch.h
@@ -31,6 +31,11 @@
     inline bool operator==(const TransportArch& other) const {
         return transport == other.transport && arch == other.arch;
     }
+    inline bool operator<(const TransportArch& other) const {
+        if (transport < other.transport) return true;
+        if (transport > other.transport) return false;
+        return arch < other.arch;
+    }
 
    private:
     friend struct TransportArchConverter;
diff --git a/include/vintf/parse_string.h b/include/vintf/parse_string.h
index 8820639..1094795 100644
--- a/include/vintf/parse_string.h
+++ b/include/vintf/parse_string.h
@@ -51,6 +51,7 @@
 std::ostream &operator<<(std::ostream &os, const ManifestHal &hal);
 std::ostream &operator<<(std::ostream &os, const MatrixHal &req);
 std::ostream &operator<<(std::ostream &os, const KernelConfigTypedValue &kcv);
+std::ostream& operator<<(std::ostream& os, const FqInstance& fqInstance);
 
 template <typename T>
 std::string to_string(const T &obj) {
@@ -83,6 +84,7 @@
 // if return true, hal->isValid() must be true.
 bool parse(const std::string &s, ManifestHal *hal);
 bool parse(const std::string &s, MatrixHal *req);
+bool parse(const std::string& s, FqInstance* fqInstance);
 
 bool parseKernelConfigInt(const std::string &s, int64_t *i);
 bool parseKernelConfigInt(const std::string &s, uint64_t *i);
diff --git a/include/vintf/parse_xml.h b/include/vintf/parse_xml.h
index 59db3bf..578e15b 100644
--- a/include/vintf/parse_xml.h
+++ b/include/vintf/parse_xml.h
@@ -31,10 +31,14 @@
     NO_KERNEL = 1 << 4,
     NO_XMLFILES = 1 << 5,
     NO_SSDK = 1 << 6,
+    NO_FQNAME = 1 << 7,
 
     EVERYTHING = 0,
-    HALS_ONLY = ~NO_HALS,
+    HALS_ONLY = ~(NO_HALS | NO_FQNAME),  // <hal> with <fqname>
     XMLFILES_ONLY = ~NO_XMLFILES,
+    SEPOLICY_ONLY = ~NO_SEPOLICY,
+    VNDK_ONLY = ~NO_VNDK,
+    HALS_NO_FQNAME = ~NO_HALS,  // <hal> without <fqname>
 };
 using SerializeFlags = uint32_t;
 
diff --git a/parse_string.cpp b/parse_string.cpp
index 51ca009..0aff5ea 100644
--- a/parse_string.cpp
+++ b/parse_string.cpp
@@ -511,5 +511,13 @@
     return toFQNameString("", range, interface, instance);
 }
 
+std::ostream& operator<<(std::ostream& os, const FqInstance& fqInstance) {
+    return os << fqInstance.string();
+}
+
+bool parse(const std::string& s, FqInstance* fqInstance) {
+    return fqInstance->setTo(s);
+}
+
 } // namespace vintf
 } // namespace android
diff --git a/parse_xml.cpp b/parse_xml.cpp
index bf8889f..38a09b4 100644
--- a/parse_xml.cpp
+++ b/parse_xml.cpp
@@ -230,11 +230,12 @@
         }
     }
 
-    template<typename T, typename Array>
-    inline void appendChildren(NodeType *parent, const XmlNodeConverter<T> &conv,
-            const Array &array, DocType *d) const {
+    template <typename T, typename Array>
+    inline void appendChildren(NodeType* parent, const XmlNodeConverter<T>& conv,
+                               const Array& array, DocType* d,
+                               SerializeFlags flags = SerializeFlag::EVERYTHING) const {
         for (const T &t : array) {
-            appendChild(parent, conv(t, d));
+            appendChild(parent, conv.serialize(t, d, flags));
         }
     }
 
@@ -600,9 +601,15 @@
 
 MatrixKernelConverter matrixKernelConverter{};
 
+XmlTextConverter<FqInstance> fqInstanceConverter{"fqname"};
+
 struct ManifestHalConverter : public XmlNodeConverter<ManifestHal> {
     std::string elementName() const override { return "hal"; }
-    void mutateNode(const ManifestHal &hal, NodeType *root, DocType *d) const override {
+    void mutateNode(const ManifestHal& m, NodeType* root, DocType* d) const override {
+        mutateNode(m, root, d, SerializeFlag::EVERYTHING);
+    }
+    void mutateNode(const ManifestHal& hal, NodeType* root, DocType* d,
+                    SerializeFlags flags) const override {
         appendAttr(root, "format", hal.format);
         appendTextElement(root, "name", hal.name, d);
         appendChild(root, transportArchConverter(hal.transportArch, d));
@@ -611,6 +618,15 @@
         if (hal.isOverride()) {
             appendAttr(root, "override", hal.isOverride());
         }
+
+        if (!(flags & SerializeFlag::NO_FQNAME)) {
+            std::set<FqInstance> fqInstances;
+            hal.forEachInstance([&fqInstances](const auto& manifestInstance) {
+                fqInstances.emplace(manifestInstance.getFqInstanceNoPackage());
+                return true;
+            });
+            appendChildren(root, fqInstanceConverter, fqInstances, d);
+        }
     }
     bool buildObject(ManifestHal* object, NodeType* root, std::string* error) const override {
         std::vector<HalInterface> interfaces;
@@ -666,6 +682,15 @@
             return false;
         }
 #endif
+
+        std::set<FqInstance> fqInstances;
+        if (!parseChildren(root, fqInstanceConverter, &fqInstances, error)) {
+            return false;
+        }
+        if (!object->insertInstances(fqInstances, error)) {
+            return false;
+        }
+
         return true;
     }
 
@@ -808,7 +833,7 @@
         appendAttr(root, "type", m.mType);
 
         if (!(flags & SerializeFlag::NO_HALS)) {
-            appendChildren(root, manifestHalConverter, m.getHals(), d);
+            appendChildren(root, manifestHalConverter, m.getHals(), d, flags);
         }
         if (m.mType == SchemaType::DEVICE) {
             if (!(flags & SerializeFlag::NO_SEPOLICY)) {
diff --git a/test/LibVintfTest.cpp b/test/LibVintfTest.cpp
index 9a243f4..7fc4e2a 100644
--- a/test/LibVintfTest.cpp
+++ b/test/LibVintfTest.cpp
@@ -251,7 +251,8 @@
 
 TEST_F(LibVintfTest, HalManifestConverter) {
     HalManifest vm = testDeviceManifest();
-    std::string xml = gHalManifestConverter(vm);
+    std::string xml =
+        gHalManifestConverter(vm, SerializeFlag::HALS_NO_FQNAME & SerializeFlag::SEPOLICY_ONLY);
     EXPECT_EQ(xml,
         "<manifest version=\"1.0\" type=\"device\">\n"
         "    <hal format=\"hidl\">\n"
@@ -288,7 +289,8 @@
 
 TEST_F(LibVintfTest, HalManifestConverterFramework) {
     HalManifest vm = testFrameworkManfiest();
-    std::string xml = gHalManifestConverter(vm);
+    std::string xml =
+        gHalManifestConverter(vm, SerializeFlag::HALS_NO_FQNAME & SerializeFlag::VNDK_ONLY);
     EXPECT_EQ(xml,
         "<manifest version=\"1.0\" type=\"framework\">\n"
         "    <hal format=\"hidl\">\n"
@@ -1275,7 +1277,9 @@
 
 TEST_F(LibVintfTest, HalManifestConverterXmlFile) {
     HalManifest vm = testDeviceManifestWithXmlFile();
-    std::string xml = gHalManifestConverter(vm);
+    std::string xml =
+        gHalManifestConverter(vm, SerializeFlag::HALS_NO_FQNAME & SerializeFlag::SEPOLICY_ONLY &
+                                      SerializeFlag::XMLFILES_ONLY);
     EXPECT_EQ(xml,
               "<manifest version=\"1.0\" type=\"device\">\n"
               "    <hal format=\"hidl\">\n"
@@ -2435,7 +2439,7 @@
     EXPECT_TRUE(gHalManifestConverter(&newManifest, xml)) << gHalManifestConverter.lastError();
 
     manifest.addAllHals(&newManifest);
-    EXPECT_EQ(xml, gHalManifestConverter(manifest, SerializeFlag::HALS_ONLY));
+    EXPECT_EQ(xml, gHalManifestConverter(manifest, SerializeFlag::HALS_NO_FQNAME));
 }
 
 TEST_F(LibVintfTest, ManifestAddOverrideHalSimpleOverride) {
@@ -2466,7 +2470,7 @@
     EXPECT_TRUE(gHalManifestConverter(&newManifest, xml)) << gHalManifestConverter.lastError();
 
     manifest.addAllHals(&newManifest);
-    EXPECT_EQ(xml, gHalManifestConverter(manifest, SerializeFlag::HALS_ONLY));
+    EXPECT_EQ(xml, gHalManifestConverter(manifest, SerializeFlag::HALS_NO_FQNAME));
 }
 
 // Existing major versions should be removed.
@@ -2536,7 +2540,7 @@
         "        </interface>\n"
         "    </hal>\n"
         "</manifest>\n",
-        gHalManifestConverter(manifest, SerializeFlag::HALS_ONLY));
+        gHalManifestConverter(manifest, SerializeFlag::HALS_NO_FQNAME));
 }
 
 TEST_F(LibVintfTest, ManifestAddOverrideHalMultiVersion2) {
@@ -2573,7 +2577,7 @@
     EXPECT_TRUE(gHalManifestConverter(&newManifest, xml)) << gHalManifestConverter.lastError();
 
     manifest.addAllHals(&newManifest);
-    EXPECT_EQ(xml, gHalManifestConverter(manifest, SerializeFlag::HALS_ONLY));
+    EXPECT_EQ(xml, gHalManifestConverter(manifest, SerializeFlag::HALS_NO_FQNAME));
 }
 
 // if no <versions>, remove all existing <hal> with given <name>.
@@ -2843,6 +2847,7 @@
     xml =
         "<manifest version=\"1.0\" type=\"framework\">\n"
         "    <hal format=\"hidl\" override=\"true\">\n"
+        "        <transport>hwbinder</transport>\n"
         "        <name>android.hardware.foo</name>\n"
         "        <transport>hwbinder</transport>\n"
         "    </hal>\n"
@@ -2869,6 +2874,169 @@
     EXPECT_FALSE(baz.front()->isDisabledHal());
 }
 
+TEST_F(LibVintfTest, FqNameValid) {
+    std::string error;
+    std::string xml;
+
+    CompatibilityMatrix cm;
+    xml =
+        "<compatibility-matrix version=\"1.0\" type=\"device\">\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.1</version>\n"
+        "        <interface>\n"
+        "            <name>IFoo</name>\n"
+        "            <instance>custom</instance>\n"
+        "        </interface>\n"
+        "    </hal>\n"
+        "</compatibility-matrix>\n";
+    EXPECT_TRUE(gCompatibilityMatrixConverter(&cm, xml, &error)) << error;
+
+    {
+        HalManifest manifest;
+        xml =
+            "<manifest version=\"1.0\" type=\"framework\">\n"
+            "    <hal format=\"hidl\">\n"
+            "        <name>android.hardware.foo</name>\n"
+            "        <transport>hwbinder</transport>\n"
+            "        <version>1.0</version>\n"
+            "        <interface>\n"
+            "            <name>IFoo</name>\n"
+            "            <instance>default</instance>\n"
+            "            <instance>custom</instance>\n"
+            "        </interface>\n"
+            "        <fqname>@1.1::IFoo/custom</fqname>\n"
+            "    </hal>\n"
+            "</manifest>\n";
+        ASSERT_TRUE(gHalManifestConverter(&manifest, xml, &error)) << error;
+        EXPECT_TRUE(manifest.checkCompatibility(cm, &error)) << error;
+    }
+
+    {
+        HalManifest manifest;
+        xml =
+            "<manifest version=\"1.0\" type=\"framework\">\n"
+            "    <hal format=\"hidl\">\n"
+            "        <name>android.hardware.foo</name>\n"
+            "        <transport>hwbinder</transport>\n"
+            "        <fqname>@1.0::IFoo/default</fqname>\n"
+            "        <fqname>@1.1::IFoo/custom</fqname>\n"
+            "    </hal>\n"
+            "</manifest>\n";
+        ASSERT_TRUE(gHalManifestConverter(&manifest, xml, &error)) << error;
+        EXPECT_TRUE(manifest.checkCompatibility(cm, &error)) << error;
+    }
+
+    {
+        HalManifest manifest;
+        xml =
+            "<manifest version=\"1.0\" type=\"framework\">\n"
+            "    <hal format=\"hidl\">\n"
+            "        <name>android.hardware.foo</name>\n"
+            "        <transport>hwbinder</transport>\n"
+            "        <version>1.0</version>\n"
+            "        <interface>\n"
+            "            <name>IFoo</name>\n"
+            "            <instance>default</instance>\n"
+            "            <instance>custom</instance>\n"
+            "        </interface>\n"
+            "    </hal>\n"
+            "</manifest>\n";
+        ASSERT_TRUE(gHalManifestConverter(&manifest, xml, &error)) << error;
+        EXPECT_FALSE(manifest.checkCompatibility(cm, &error));
+        EXPECT_IN(
+            "android.hardware.foo:\n"
+            "    required: @1.1::IFoo/custom\n"
+            "    provided: \n"
+            "        @1.0::IFoo/custom\n"
+            "        @1.0::IFoo/default",
+            error);
+    }
+
+    {
+        HalManifest manifest;
+        xml =
+            "<manifest version=\"1.0\" type=\"framework\">\n"
+            "    <hal format=\"hidl\">\n"
+            "        <name>android.hardware.foo</name>\n"
+            "        <transport>hwbinder</transport>\n"
+            "        <fqname>@1.0::IFoo/default</fqname>\n"
+            "        <fqname>@1.0::IFoo/custom</fqname>\n"
+            "    </hal>\n"
+            "</manifest>\n";
+        ASSERT_TRUE(gHalManifestConverter(&manifest, xml, &error)) << error;
+        EXPECT_IN(
+            "android.hardware.foo:\n"
+            "    required: @1.1::IFoo/custom\n"
+            "    provided: \n"
+            "        @1.0::IFoo/custom\n"
+            "        @1.0::IFoo/default",
+            error);
+    }
+}
+
+TEST_F(LibVintfTest, FqNameInvalid) {
+    std::string error;
+    std::string xml;
+    ManifestHal hal;
+    xml =
+        "<hal format=\"hidl\">\n"
+        "    <name>android.hardware.foo</name>\n"
+        "    <transport>hwbinder</transport>\n"
+        "    <fqname>@1.1::IFoo/custom</fqname>\n"
+        "</hal>\n";
+    EXPECT_TRUE(gManifestHalConverter(&hal, xml, &error)) << error;
+    xml =
+        "<hal format=\"hidl\">\n"
+        "    <name>android.hardware.foo</name>\n"
+        "    <transport>hwbinder</transport>\n"
+        "    <fqname>1.1::IFoo/custom</fqname>\n"
+        "</hal>\n";
+    ASSERT_FALSE(gManifestHalConverter(&hal, xml, &error));
+    EXPECT_IN("Could not parse text \"1.1::IFoo/custom\" in element <fqname>", error);
+    xml =
+        "<hal format=\"hidl\">\n"
+        "    <name>android.hardware.foo</name>\n"
+        "    <transport>hwbinder</transport>\n"
+        "    <fqname>android.hardware.foo@1.1::IFoo/custom</fqname>\n"
+        "</hal>\n";
+    ASSERT_FALSE(gManifestHalConverter(&hal, xml, &error));
+    EXPECT_IN("Should not specify package", error);
+    xml =
+        "<hal format=\"hidl\">\n"
+        "    <name>android.hardware.foo</name>\n"
+        "    <transport>hwbinder</transport>\n"
+        "    <fqname>IFoo/custom</fqname>\n"
+        "</hal>\n";
+    ASSERT_FALSE(gManifestHalConverter(&hal, xml, &error));
+    EXPECT_IN("Should specify version", error);
+    xml =
+        "<hal format=\"hidl\">\n"
+        "    <name>android.hardware.foo</name>\n"
+        "    <transport>hwbinder</transport>\n"
+        "    <fqname>@1.0::IFoo</fqname>\n"
+        "</hal>\n";
+    ASSERT_FALSE(gManifestHalConverter(&hal, xml, &error));
+    EXPECT_IN("Should specify instance", error);
+    xml =
+        "<hal format=\"hidl\">\n"
+        "    <name>n07 4 v4l1d 1n73rf4c3</name>\n"
+        "    <transport>hwbinder</transport>\n"
+        "    <fqname>@1.0::IFoo/custom</fqname>\n"
+        "</hal>\n";
+    ASSERT_FALSE(gManifestHalConverter(&hal, xml, &error));
+    EXPECT_IN("Cannot create FqInstance", error);
+    EXPECT_IN("n07 4 v4l1d 1n73rf4c3", error);
+}
+
 } // namespace vintf
 } // namespace android