compare version improved

plugins/zenoh-backend-traits/src/config.rs

@@ -68,12 +68,12 @@ pub struct ReplicaConfig {
     pub delta: Duration,
 }
 
-const VOLUME_CONFIG_VERSION: &str = "1";
-
 impl CompatibilityVersion for VolumeConfig {
-    fn version() -> &'static str {
+    fn version() -> u64 {
+        1
+    }
+    fn features() -> &'static str {
         concat_enabled_features!(
-            VOLUME_CONFIG_VERSION,
             "auth_pubkey",
             "auth_usrpwd",
             "complete_n",

plugins/zenoh-backend-traits/src/lib.rs

@@ -132,13 +132,13 @@
 //! ```
 
 use async_trait::async_trait;
-use zenoh_plugin_trait::{CompatibilityVersion, concat_enabled_features};
 use std::sync::Arc;
 use zenoh::prelude::{KeyExpr, OwnedKeyExpr, Sample, Selector};
 use zenoh::queryable::ReplyBuilder;
 use zenoh::time::Timestamp;
 use zenoh::value::Value;
 pub use zenoh::Result as ZResult;
+use zenoh_plugin_trait::{concat_enabled_features, CompatibilityVersion};
 
 pub mod config;
 use config::{StorageConfig, VolumeConfig};
@@ -217,12 +217,12 @@ pub trait Volume: Send + Sync {
 
 pub type VolumePlugin = Box<dyn Volume + 'static>;
 
-const VOLUME_PLUGIN_VERSION: &str = "1";
-
 impl CompatibilityVersion for VolumePlugin {
-    fn version() -> &'static str {
+    fn version() -> u64 {
+        1
+    }
+    fn features() -> &'static str {
         concat_enabled_features!(
-            VOLUME_PLUGIN_VERSION,
             "auth_pubkey",
             "auth_usrpwd",
             "complete_n",

plugins/zenoh-plugin-trait/Cargo.toml

@@ -37,3 +37,4 @@ serde_json = { workspace = true }
 zenoh-macros = { workspace = true }
 zenoh-result = { workspace = true }
 zenoh-util = { workspace = true }
+const_format = { workspace = true } 

plugins/zenoh-plugin-trait/src/lib.rs

@@ -24,22 +24,29 @@ pub mod vtable;
 use zenoh_result::ZResult;
 
 pub mod prelude {
-    pub use crate::{loading::*, vtable::*, CompatibilityVersion, Plugin, concat_enabled_features};
+    pub use crate::{concat_enabled_features, loading::*, vtable::*, CompatibilityVersion, Plugin};
 }
 
 #[macro_export]
 macro_rules! concat_enabled_features {
-    ($version:ident, $($feature:literal),*) => {
+    ($($feature:literal),*) => {
         {
             use const_format::concatcp;
-            const_format::concatcp!($version $(,
+            const_format::concatcp!("" $(,
                 if cfg!(feature = $feature) { concatcp!(" ", $feature) } else { "" }
             )*)
         }
     };
 }
+
 pub trait CompatibilityVersion {
-    fn version() -> &'static str;
+    /// The version of the structure implementing this trait. After any channge in the structure or it's dependencies
+    /// whcich may affect the ABI, this version should be incremented.
+    fn version() -> u64;
+    /// The features enabled when the structure implementing this trait was compiled.
+    /// Different features between the plugin and the host may cuase ABI incompatibility even if the structure version is the same.
+    /// Use `concat_enabled_features!` to generate this string.
+    fn features() -> &'static str;
 }
 
 pub trait Plugin: Sized + 'static {

plugins/zenoh-plugin-trait/src/loading.rs

@@ -224,14 +224,16 @@ impl<StartArgs: 'static + CompatibilityVersion, RunningPlugin: 'static + Compati
 
     /// Tries to load a plugin with the name `defaukt_lib_prefix` + `backend_name` + `.so | .dll | .dylib`
     /// in lib_loader's search paths.
+    /// Returns a tuple of (retval, plugin_record)
+    /// where `retval`` is true if the plugin was successfully loaded, false if pluginw with this name it was already loaded
     pub fn load_plugin_by_backend_name<T: AsRef<str>, T1: AsRef<str>>(
         &mut self,
         name: T,
         backend_name: T1,
-    ) -> ZResult<&mut PluginRecord<StartArgs, RunningPlugin>> {
+    ) -> ZResult<(bool, &mut PluginRecord<StartArgs, RunningPlugin>)> {
         let name = name.as_ref();
-        if self.get_plugin_index(name).is_some() {
-            bail!("Plugin `{}` already loaded", name);
+        if let Some(index) = self.get_plugin_index(name) {
+            return Ok((false, &mut self.plugins[index]));
         }
         let backend_name = backend_name.as_ref();
         let (lib, p) = match &mut self.loader {
@@ -243,25 +245,27 @@ impl<StartArgs: 'static + CompatibilityVersion, RunningPlugin: 'static + Compati
             Err(e) => bail!("After loading `{:?}`: {}", &p, e),
         };
         self.plugins.push(PluginRecord::new(plugin));
-        Ok(self.plugins.last_mut().unwrap())
+        Ok((true, self.plugins.last_mut().unwrap()))
     }
     /// Tries to load a plugin from the list of path to plugin (absolute or relative to the current working directory)
+    /// Returns a tuple of (retval, plugin_record)
+    /// where `retval`` is true if the plugin was successfully loaded, false if pluginw with this name it was already loaded
     pub fn load_plugin_by_paths<T: AsRef<str>, P: AsRef<str> + std::fmt::Debug>(
         &mut self,
         name: T,
         paths: &[P],
-    ) -> ZResult<&mut PluginRecord<StartArgs, RunningPlugin>> {
+    ) -> ZResult<(bool, &mut PluginRecord<StartArgs, RunningPlugin>)> {
         let name = name.as_ref();
-        if self.get_plugin_index(name).is_some() {
-            bail!("Plugin `{}` already loaded", name);
+        if let Some(index) = self.get_plugin_index(name) {
+            return Ok((false, &mut self.plugins[index]));
         }
         for path in paths {
             let path = path.as_ref();
             match unsafe { LibLoader::load_file(path) } {
                 Ok((lib, p)) => {
                     let plugin = Self::load_plugin(name, lib, p)?;
                     self.plugins.push(PluginRecord::new(plugin));
-                    return Ok(self.plugins.last_mut().unwrap());
+                    return Ok((true, self.plugins.last_mut().unwrap()));
                 }
                 Err(e) => log::warn!("Plugin '{}' load fail at {}: {}", &name, path, e),
             }
@@ -356,9 +360,11 @@ impl<StartArgs: CompatibilityVersion, RunningPlugin: CompatibilityVersion>
     DynamicPlugin<StartArgs, RunningPlugin>
 {
     fn new(name: String, lib: Library, path: PathBuf) -> ZResult<Self> {
+        log::debug!("Loading plugin {}", &path.to_str().unwrap(),);
         let get_plugin_loader_version =
             unsafe { lib.get::<fn() -> PluginLoaderVersion>(b"get_plugin_loader_version")? };
         let plugin_loader_version = get_plugin_loader_version();
+        log::debug!("Plugin loader version: {}", &plugin_loader_version);
         if plugin_loader_version != PLUGIN_LOADER_VERSION {
             bail!(
                 "Plugin loader version mismatch: host = {}, plugin = {}",
@@ -369,15 +375,17 @@ impl<StartArgs: CompatibilityVersion, RunningPlugin: CompatibilityVersion>
         let get_compatibility = unsafe { lib.get::<fn() -> Compatibility>(b"get_compatibility")? };
         let plugin_compatibility_record = get_compatibility();
         let host_compatibility_record = Compatibility::new::<StartArgs, RunningPlugin>();
+        log::debug!(
+            "Plugin compativilty record: {:?}",
+            &plugin_compatibility_record
+        );
         if !plugin_compatibility_record.are_compatible(&host_compatibility_record) {
             bail!(
-                "Plugin compatibility mismatch:\nhost = {:?}\nplugin = {:?}\n",
+                "Plugin compatibility mismatch:\n\nHost:\n{}\nPlugin:\n{}\n",
                 host_compatibility_record,
                 plugin_compatibility_record
             );
         }
-
-        // TODO: check loader version and compatibility
         let load_plugin =
             unsafe { lib.get::<fn() -> PluginVTable<StartArgs, RunningPlugin>>(b"load_plugin")? };
         let vtable = load_plugin();

plugins/zenoh-plugin-trait/src/vtable.rs

@@ -1,3 +1,5 @@
+use std::fmt::Display;
+
 //
 // Copyright (c) 2023 ZettaScale Technology
 //
@@ -25,30 +27,79 @@ pub struct PluginVTable<StartArgs, RunningPlugin> {
     pub start: StartFn<StartArgs, RunningPlugin>,
 }
 impl<StartArgs, RunningPlugin> CompatibilityVersion for PluginVTable<StartArgs, RunningPlugin> {
-    fn version() -> &'static str{
-        "1"
+    fn version() -> u64 {
+        1
+    }
+    fn features() -> &'static str {
+        concat_enabled_features!(
+            "auth_pubkey",
+            "auth_usrpwd",
+            "complete_n",
+            "shared-memory",
+            "stats",
+            "transport_multilink",
+            "transport_quic",
+            "transport_serial",
+            "transport_unixpipe",
+            "transport_tcp",
+            "transport_tls",
+            "transport_udp",
+            "transport_unixsock-stream",
+            "transport_ws",
+            "unstable",
+            "default"
+        )
+    }
+}
+
+#[repr(C)]
+#[derive(Debug, PartialEq, Eq, Clone)]
+pub struct StructVersion {
+    pub version: u64,
+    pub name: &'static str,
+    pub features: &'static str,
+}
+
+impl StructVersion {
+    pub fn new<T: CompatibilityVersion>() -> Self {
+        Self {
+            version: T::version(),
+            name: std::any::type_name::<T>(),
+            features: T::features(),
+        }
+    }
+}
+
+impl Display for StructVersion {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(
+            f,
+            "  version: {}\n  type: {}\n  features: {}\n",
+            self.version, self.name, self.features
+        )
     }
 }
 
 #[repr(C)]
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub struct Compatibility {
     rust_version: RustVersion,
-    vtable_version: &'static str,
-    start_args_version: (&'static str, &'static str),
-    running_plugin_version: (&'static str, &'static str),
+    vtable_version: StructVersion,
+    start_args_version: StructVersion,
+    running_plugin_version: StructVersion,
 }
 
 impl Compatibility {
     pub fn new<StartArgs: CompatibilityVersion, RunningPlugin: CompatibilityVersion>() -> Self {
+        let rust_version = RustVersion::new();
+        let vtable_version = StructVersion::new::<PluginVTable<StartArgs, RunningPlugin>>();
+        let start_args_version = StructVersion::new::<StartArgs>();
+        let running_plugin_version = StructVersion::new::<RunningPlugin>();
         Self {
-            rust_version: RustVersion::new(),
-            vtable_version: PluginVTable::<StartArgs, RunningPlugin>::version(),
-            start_args_version: (std::any::type_name::<StartArgs>(), StartArgs::version()),
-            running_plugin_version: (
-                std::any::type_name::<RunningPlugin>(),
-                RunningPlugin::version(),
-            ),
+            rust_version,
+            vtable_version,
+            start_args_version,
+            running_plugin_version,
         }
     }
     pub fn are_compatible(&self, other: &Self) -> bool {
@@ -59,6 +110,19 @@ impl Compatibility {
     }
 }
 
+impl Display for Compatibility {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(
+            f,
+            "{}\nVTable:{}StartArgs:{}RunningPlugin:{}",
+            self.rust_version,
+            self.vtable_version,
+            self.start_args_version,
+            self.running_plugin_version
+        )
+    }
+}
+
 #[repr(C)]
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub struct RustVersion {
@@ -69,6 +133,20 @@ pub struct RustVersion {
     commit: &'static str,
 }
 
+impl Display for RustVersion {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(
+            f,
+            "Rust {}.{}.{}{} commit {}",
+            self.major,
+            self.minor,
+            self.patch,
+            if self.stable { "" } else { "-nightly" },
+            self.commit
+        )
+    }
+}
+
 const RELEASE_AND_COMMIT: (&str, &str) = zenoh_macros::rustc_version_release!();
 impl RustVersion {
     pub fn new() -> Self {

zenoh/src/net/runtime/adminspace.rs

@@ -200,12 +200,20 @@ impl AdminSpace {
                                     plugins_mgr.load_plugin_by_backend_name(name, &plugin.name)
                                 };
                                 match rec {
-                                    Ok(rec) => {
-                                        log::info!(
-                                            "Loaded plugin `{}` from {}",
-                                            rec.name(),
-                                            rec.path()
-                                        );
+                                    Ok((loaded, rec)) => {
+                                        if loaded {
+                                            log::info!(
+                                                "Loaded plugin `{}` from {}",
+                                                rec.name(),
+                                                rec.path()
+                                            );
+                                        } else {
+                                            log::warn!(
+                                                "Plugin `{}` was already loaded from {}",
+                                                rec.name(),
+                                                rec.path()
+                                            );
+                                        }
                                         match rec.start(&admin.context.runtime) {
                                             Ok((true, rec)) => {
                                                 active_plugins

zenoh/src/net/runtime/mod.rs

@@ -38,7 +38,7 @@ use stop_token::future::FutureExt;
 use stop_token::{StopSource, TimedOutError};
 use uhlc::{HLCBuilder, HLC};
 use zenoh_link::{EndPoint, Link};
-use zenoh_plugin_trait::{CompatibilityVersion, concat_enabled_features};
+use zenoh_plugin_trait::{concat_enabled_features, CompatibilityVersion};
 use zenoh_protocol::core::{whatami::WhatAmIMatcher, Locator, WhatAmI, ZenohId};
 use zenoh_protocol::network::{NetworkBody, NetworkMessage};
 use zenoh_result::{bail, ZResult};
@@ -66,12 +66,12 @@ pub struct Runtime {
     state: Arc<RuntimeState>,
 }
 
-const RUNTIME_VERSION: &str = "1";
-
 impl CompatibilityVersion for Runtime {
-    fn version() -> &'static str {
+    fn version() -> u64 {
+        1
+    }
+    fn features() -> &'static str {
         concat_enabled_features!(
-            RUNTIME_VERSION,
             "auth_pubkey",
             "auth_usrpwd",
             "complete_n",

zenoh/src/plugins/sealed.rs

@@ -31,12 +31,12 @@ pub type StartArgs = Runtime;
 /// A zenoh plugin, when started, must return this type.
 pub type RunningPlugin = Box<dyn RunningPluginTrait + 'static>;
 
-const RUNNING_PLUGIN_VERSION: &str = "1";
-
 impl CompatibilityVersion for RunningPlugin {
-    fn version() -> &'static str {
+    fn version() -> u64 {
+        1
+    }
+    fn features() -> &'static str {
         concat_enabled_features!(
-            RUNNING_PLUGIN_VERSION,
             "auth_pubkey",
             "auth_usrpwd",
             "complete_n",