diff --git a/Cargo.lock b/Cargo.lock index 24a39b3d63..4eff17d654 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1179,9 +1179,9 @@ checksum = "bbfc4744c1b8f2a09adc0e55242f60b1af195d88596bd8700be74418c056c555" [[package]] name = "either" -version = "1.9.0" +version = "1.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a26ae43d7bcc3b814de94796a5e736d4029efb0ee900c12e2d54c993ad1a1e07" +checksum = "60b1af1c220855b6ceac025d3f6ecdd2b7c4894bfe9cd9bda4fbb4bc7c0d4cf0" [[package]] name = "encoding_rs" @@ -3587,6 +3587,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "763f8cd0d4c71ed8389c90cb8100cba87e763bd01a8e614d4f0af97bcd50a161" dependencies = [ "dyn-clone", + "either", "schemars_derive", "serde", "serde_json", @@ -5942,6 +5943,7 @@ dependencies = [ "urlencoding", "zenoh", "zenoh-plugin-trait", + "zenoh-util", "zenoh_backend_traits", ] @@ -6095,6 +6097,8 @@ dependencies = [ "libc", "libloading", "pnet_datalink", + "serde", + "serde_json", "shellexpand", "tokio", "tracing", @@ -6112,6 +6116,7 @@ dependencies = [ "async-trait", "const_format", "derive_more", + "either", "schemars", "serde_json", "zenoh", diff --git a/Cargo.toml b/Cargo.toml index 254cdc19b9..20df253e6a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -148,7 +148,7 @@ rustls-native-certs = "0.7.0" rustls-pemfile = "2.0.0" rustls-webpki = "0.102.0" rustls-pki-types = "1.1.0" -schemars = "0.8.12" +schemars = { version = "0.8.12", features = ["either"] } secrecy = { version = "0.8.0", features = ["serde", "alloc"] } serde = { version = "1.0.154", default-features = false, features = [ "derive", @@ -187,6 +187,7 @@ webpki-roots = "0.26.0" winapi = { version = "0.3.9", features = ["iphlpapi"] } x509-parser = "0.16.0" z-serial = "0.2.3" +either = "1.13.0" zenoh-ext = { version = "0.11.0-dev", path = "zenoh-ext" } zenoh-shm = { version = "0.11.0-dev", path = "commons/zenoh-shm" } zenoh-result = { version = "0.11.0-dev", path = "commons/zenoh-result", default-features = false } diff --git a/DEFAULT_CONFIG.json5 b/DEFAULT_CONFIG.json5 index de5baa4725..c4052313d9 100644 --- a/DEFAULT_CONFIG.json5 +++ b/DEFAULT_CONFIG.json5 @@ -199,7 +199,7 @@ // /// Id has to be unique within the rule set // "id": "rule1", // "messages": [ - // "put", "delete", "declare_subscriber", + // "put", "delete", "declare_subscriber", // "query", "reply", "declare_queryable", // ], // "flows":["egress","ingress"], @@ -211,7 +211,7 @@ // { // "id": "rule2", // "messages": [ - // "put", "delete", "declare_subscriber", + // "put", "delete", "declare_subscriber", // "query", "reply", "declare_queryable", // ], // "flows":["ingress"], @@ -462,11 +462,15 @@ /// // // plugins_loading: { - // // Enable plugins loading. + // /// Enable plugins loading. // enabled: false, // /// Directories where plugins configured by name should be looked for. Plugins configured by __path__ are not subject to lookup. - // /// If `enabled: true` and `search_dirs` is not specified then `search_dirs` falls back to the default value: ".:~/.zenoh/lib:/opt/homebrew/lib:/usr/local/lib:/usr/lib" - // search_dirs: [], + // /// Directories are specified as object with fields `kind` and `value` is accepted. + // /// 1. If `kind` is `current_exe_parent`, then the parent of the current executable's directory is searched and `value` should be `null`. + // /// In Bash notation, `{ "kind": "current_exe_parent" }` equals `$(dirname $(which zenohd))` while `"."` equals `$PWD`. + // /// 2. If `kind` is `path`, then `value` is interpreted as a filesystem path. Simply supplying a string instead of a object is equivalent to this. + // /// If `enabled: true` and `search_dirs` is not specified then `search_dirs` falls back to the default value: + // search_dirs: [{ "kind": "current_exe_parent" }, ".", "~/.zenoh/lib", "/opt/homebrew/lib", "/usr/local/lib", "/usr/lib"], // }, // /// Plugins are only loaded if `plugins_loading: { enabled: true }` and present in the configuration when starting. // /// Once loaded, they may react to changes in the configuration made through the zenoh instance's adminspace. diff --git a/commons/zenoh-config/src/lib.rs b/commons/zenoh-config/src/lib.rs index f5fc01aa63..b7b63e1602 100644 --- a/commons/zenoh-config/src/lib.rs +++ b/commons/zenoh-config/src/lib.rs @@ -45,7 +45,7 @@ use zenoh_protocol::{ transport::{BatchSize, TransportSn}, }; use zenoh_result::{bail, zerror, ZResult}; -use zenoh_util::LibLoader; +use zenoh_util::{LibLoader, LibSearchDirs}; pub mod mode_dependent; pub use mode_dependent::*; @@ -547,7 +547,7 @@ validated_struct::validator! { pub plugins_loading: #[derive(Default)] PluginsLoading { pub enabled: bool, - pub search_dirs: Option>, // TODO (low-prio): Switch this String to a PathBuf? (applies to other paths in the config as well) + pub search_dirs: LibSearchDirs, }, #[validated(recursive_accessors)] /// The configuration for plugins. @@ -573,19 +573,6 @@ fn set_false() -> bool { false } -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct PluginSearchDirs(Vec); -impl Default for PluginSearchDirs { - fn default() -> Self { - Self( - (*zenoh_util::LIB_DEFAULT_SEARCH_PATHS) - .split(':') - .map(|c| c.to_string()) - .collect(), - ) - } -} - #[test] fn config_deser() { let config = Config::from_deserializer( @@ -763,10 +750,7 @@ impl Config { pub fn libloader(&self) -> LibLoader { if self.plugins_loading.enabled { - match self.plugins_loading.search_dirs() { - Some(dirs) => LibLoader::new(dirs, true), - None => LibLoader::default(), - } + LibLoader::new(self.plugins_loading.search_dirs().clone()) } else { LibLoader::empty() } diff --git a/commons/zenoh-util/Cargo.toml b/commons/zenoh-util/Cargo.toml index e41433b85f..df99e01385 100644 --- a/commons/zenoh-util/Cargo.toml +++ b/commons/zenoh-util/Cargo.toml @@ -49,6 +49,8 @@ shellexpand = { workspace = true } zenoh-core = { workspace = true } zenoh-result = { workspace = true, features = ["default"] } const_format = { workspace = true } +serde = { workspace = true, features = ["default"] } +serde_json = { workspace = true } [target.'cfg(windows)'.dependencies] winapi = { workspace = true } diff --git a/commons/zenoh-util/src/lib.rs b/commons/zenoh-util/src/lib.rs index 745e790711..a6cf03e5fb 100644 --- a/commons/zenoh-util/src/lib.rs +++ b/commons/zenoh-util/src/lib.rs @@ -21,6 +21,7 @@ use lazy_static::lazy_static; pub mod ffi; mod lib_loader; +pub mod lib_search_dirs; pub mod net; pub mod time_range; @@ -28,6 +29,7 @@ pub use lib_loader::*; pub mod timer; pub use timer::*; pub mod log; +pub use lib_search_dirs::*; pub use log::*; /// The "ZENOH_HOME" environment variable name diff --git a/commons/zenoh-util/src/lib_loader.rs b/commons/zenoh-util/src/lib_loader.rs index 9d4a52c332..082bb04839 100644 --- a/commons/zenoh-util/src/lib_loader.rs +++ b/commons/zenoh-util/src/lib_loader.rs @@ -23,13 +23,13 @@ use tracing::{debug, warn}; use zenoh_core::{zconfigurable, zerror}; use zenoh_result::{bail, ZResult}; +use crate::LibSearchDirs; + zconfigurable! { /// The libraries prefix for the current platform (usually: `"lib"`) pub static ref LIB_PREFIX: String = DLL_PREFIX.to_string(); /// The libraries suffix for the current platform (`".dll"` or `".so"` or `".dylib"`...) pub static ref LIB_SUFFIX: String = DLL_SUFFIX.to_string(); - /// The default list of paths where to search for libraries to load - pub static ref LIB_DEFAULT_SEARCH_PATHS: String = ".:~/.zenoh/lib:/opt/homebrew/lib:/usr/local/lib:/usr/lib".to_string(); } /// LibLoader allows search for libraries and to load them. @@ -44,40 +44,16 @@ impl LibLoader { LibLoader { search_paths: None } } - /// Returns the list of search paths used by `LibLoader::default()` - pub fn default_search_paths() -> &'static str { - &LIB_DEFAULT_SEARCH_PATHS - } - /// Creates a new [LibLoader] with a set of paths where the libraries will be searched for. /// If `exe_parent_dir`is true, the parent directory of the current executable is also added /// to the set of paths for search. - pub fn new(search_dirs: &[S], exe_parent_dir: bool) -> LibLoader - where - S: AsRef, - { - let mut search_paths: Vec = vec![]; - for s in search_dirs { - match shellexpand::full(s) { - Ok(cow_str) => match PathBuf::from(&*cow_str).canonicalize() { - Ok(path) => search_paths.push(path), - Err(err) => debug!("Cannot search for libraries in {}: {}", cow_str, err), - }, - Err(err) => warn!("Cannot search for libraries in '{}': {} ", s.as_ref(), err), - } - } - Self::_new(search_paths, exe_parent_dir) - } - fn _new(mut search_paths: Vec, exe_parent_dir: bool) -> Self { - if exe_parent_dir { - match std::env::current_exe() { - Ok(path) => match path.parent() { - Some(p) => if p.is_dir() { - search_paths.push(p.canonicalize().unwrap()) - }, - None => warn!("Can't search for plugins in executable parent directory: no parent directory for {}.", path.to_string_lossy()), - }, - Err(e) => warn!("Can't search for plugins in executable parent directory: {}.", e), + pub fn new(dirs: LibSearchDirs) -> LibLoader { + let mut search_paths = Vec::new(); + + for path in dirs.into_iter() { + match path { + Ok(path) => search_paths.push(path), + Err(err) => tracing::error!("{err}"), } } @@ -237,7 +213,6 @@ impl LibLoader { impl Default for LibLoader { fn default() -> Self { - let paths: Vec<&str> = (*LIB_DEFAULT_SEARCH_PATHS).split(':').collect(); - LibLoader::new(&paths, true) + LibLoader::new(LibSearchDirs::default()) } } diff --git a/commons/zenoh-util/src/lib_search_dirs.rs b/commons/zenoh-util/src/lib_search_dirs.rs new file mode 100644 index 0000000000..58046ff3a7 --- /dev/null +++ b/commons/zenoh-util/src/lib_search_dirs.rs @@ -0,0 +1,236 @@ +// +// Copyright (c) 2024 ZettaScale Technology +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License 2.0 which is available at +// http://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// +// Contributors: +// ZettaScale Zenoh Team, +// +use std::{env, error::Error, fmt::Display, path::PathBuf, str::FromStr}; + +use serde::{ + de::{value::MapAccessDeserializer, Visitor}, + Deserialize, Serialize, +}; + +#[derive(Clone, Debug, Serialize, Deserialize, Eq, Hash, PartialEq)] +#[serde(default)] +pub struct LibSearchDirs(Vec); + +impl LibSearchDirs { + pub fn from_paths>(paths: &[T]) -> Self { + Self( + paths + .iter() + .map(|s| LibSearchDir::Path(s.as_ref().to_string())) + .collect(), + ) + } + + pub fn from_specs>(paths: &[T]) -> Result { + let dirs = paths + .iter() + .map(|s| { + let de = &mut serde_json::Deserializer::from_str(s.as_ref()); + LibSearchDir::deserialize(de) + }) + .collect::, _>>()?; + + Ok(Self(dirs)) + } +} + +#[derive(Debug)] +pub struct InvalidLibSearchDir { + found: LibSearchDir, + source: String, +} + +impl Display for InvalidLibSearchDir { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "invalid library search directory `{:?}`: {}", + self.found, self.source + ) + } +} + +impl Error for InvalidLibSearchDir {} + +pub struct IntoIter { + iter: std::vec::IntoIter, +} + +impl Iterator for IntoIter { + type Item = Result; + + fn next(&mut self) -> Option { + self.iter.next().map(LibSearchDir::into_path) + } +} + +impl IntoIterator for LibSearchDirs { + type Item = Result; + + type IntoIter = IntoIter; + + fn into_iter(self) -> Self::IntoIter { + IntoIter { + iter: self.0.into_iter(), + } + } +} + +impl Default for LibSearchDirs { + fn default() -> Self { + LibSearchDirs(vec![ + LibSearchDir::Spec(LibSearchSpec { + kind: LibSearchSpecKind::CurrentExeParent, + value: None, + }), + LibSearchDir::Path(".".to_string()), + LibSearchDir::Path("~/.zenoh/lib".to_string()), + LibSearchDir::Path("/opt/homebrew/lib".to_string()), + LibSearchDir::Path("/usr/local/lib".to_string()), + LibSearchDir::Path("/usr/lib".to_string()), + ]) + } +} + +#[derive(Clone, Debug, Eq, Hash, PartialEq)] +pub enum LibSearchDir { + Path(String), + Spec(LibSearchSpec), +} + +impl LibSearchDir { + fn into_path(self) -> Result { + match self { + LibSearchDir::Path(path) => LibSearchSpec { + kind: LibSearchSpecKind::Path, + value: Some(path), + } + .into_path(), + LibSearchDir::Spec(spec) => spec.into_path(), + } + } +} + +#[derive(Clone, Debug, Deserialize, Serialize, Eq, Hash, PartialEq)] +#[serde(rename_all = "snake_case")] +pub struct LibSearchSpec { + kind: LibSearchSpecKind, + value: Option, +} + +impl LibSearchSpec { + fn into_path(self) -> Result { + fn error_from_source(spec: &LibSearchSpec, err: T) -> InvalidLibSearchDir { + InvalidLibSearchDir { + found: LibSearchDir::Spec(spec.clone()), + source: err.to_string(), + } + } + + fn error_from_str(spec: &LibSearchSpec, err: &str) -> InvalidLibSearchDir { + InvalidLibSearchDir { + found: LibSearchDir::Spec(spec.clone()), + source: err.to_string(), + } + } + + match self.kind { + LibSearchSpecKind::Path => { + let Some(value) = &self.value else { + return Err(error_from_str( + &self, + "`path` specs should have a `value` field", + )); + }; + + let expanded = + shellexpand::full(value).map_err(|err| error_from_source(&self, err))?; + + let path = + PathBuf::from_str(&expanded).map_err(|err| error_from_source(&self, err))?; + + Ok(path) + } + LibSearchSpecKind::CurrentExeParent => { + let current_exe = + env::current_exe().map_err(|err| error_from_source(&self, err))?; + + let Some(current_exe_parent) = current_exe.parent() else { + return Err(error_from_str( + &self, + "current executable's path has no parent directory", + )); + }; + + let canonicalized = current_exe_parent + .canonicalize() + .map_err(|err| error_from_source(&self, err))?; + + Ok(canonicalized) + } + } + } +} + +#[derive(Clone, Debug, Deserialize, Serialize, Eq, Hash, PartialEq)] +#[serde(rename_all = "snake_case")] +pub enum LibSearchSpecKind { + Path, + CurrentExeParent, +} + +impl<'de> Deserialize<'de> for LibSearchDir { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + deserializer.deserialize_any(LibSearchSpecOrPathVisitor) + } +} + +impl Serialize for LibSearchDir { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + match self { + LibSearchDir::Path(path) => serializer.serialize_str(path), + LibSearchDir::Spec(spec) => spec.serialize(serializer), + } + } +} + +struct LibSearchSpecOrPathVisitor; + +impl<'de> Visitor<'de> for LibSearchSpecOrPathVisitor { + type Value = LibSearchDir; + + fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { + formatter.write_str("str or map with field `kind` and optionally field `value`") + } + + fn visit_str(self, v: &str) -> Result + where + E: serde::de::Error, + { + Ok(LibSearchDir::Path(v.to_string())) + } + + fn visit_map(self, map: A) -> Result + where + A: serde::de::MapAccess<'de>, + { + LibSearchSpec::deserialize(MapAccessDeserializer::new(map)).map(LibSearchDir::Spec) + } +} diff --git a/plugins/zenoh-backend-traits/Cargo.toml b/plugins/zenoh-backend-traits/Cargo.toml index 5997dc5c65..1a574dd118 100644 --- a/plugins/zenoh-backend-traits/Cargo.toml +++ b/plugins/zenoh-backend-traits/Cargo.toml @@ -37,6 +37,7 @@ zenoh-util = { workspace = true } schemars = { workspace = true } zenoh-plugin-trait = { workspace = true } const_format = { workspace = true } +either = { workspace = true } [features] default = [] diff --git a/plugins/zenoh-backend-traits/src/config.rs b/plugins/zenoh-backend-traits/src/config.rs index 98167680c8..e440e3014e 100644 --- a/plugins/zenoh-backend-traits/src/config.rs +++ b/plugins/zenoh-backend-traits/src/config.rs @@ -15,6 +15,7 @@ use std::{convert::TryFrom, time::Duration}; use const_format::concatcp; use derive_more::{AsMut, AsRef}; +use either::Either; use schemars::JsonSchema; use serde_json::{Map, Value}; use zenoh::{ @@ -23,6 +24,7 @@ use zenoh::{ }; use zenoh_plugin_trait::{PluginStartArgs, StructVersion}; use zenoh_result::{bail, zerror, Error}; +use zenoh_util::LibSearchDirs; #[derive(JsonSchema, Debug, Clone, AsMut, AsRef)] pub struct PluginConfig { @@ -30,7 +32,9 @@ pub struct PluginConfig { pub name: String, #[schemars(with = "Option")] pub required: bool, - pub backend_search_dirs: Option>, + // REVIEW: This is inconsistent with `plugins_loading/search_dirs` + #[schemars(with = "Option, String>>>>")] + pub backend_search_dirs: LibSearchDirs, #[schemars(with = "Map")] pub volumes: Vec, #[schemars(with = "Map")] @@ -161,16 +165,18 @@ impl + AsRef, V: AsObject> TryFrom<(S, &V)> for PluginConfi }) .unwrap_or(Ok(true))?; let backend_search_dirs = match value.get("backend_search_dirs") { - Some(serde_json::Value::String(path)) => Some(vec![path.clone()]), + Some(serde_json::Value::String(path)) => LibSearchDirs::from_paths(&[path.clone()]), Some(serde_json::Value::Array(paths)) => { - let mut result = Vec::with_capacity(paths.len()); + let mut specs = Vec::with_capacity(paths.len()); for path in paths { - let path = if let serde_json::Value::String(path) = path {path} else {bail!("`backend_search_dirs` field of {}'s configuration must be a string or array of strings", name.as_ref())}; - result.push(path.clone()); + let serde_json::Value::String(path) = path else { + bail!("`backend_search_dirs` field of {}'s configuration must be a string or array of strings", name.as_ref()); + }; + specs.push(path.clone()); } - Some(result) + LibSearchDirs::from_specs(&specs)? } - None => None, + None => LibSearchDirs::default(), _ => bail!("`backend_search_dirs` field of {}'s configuration must be a string or array of strings", name.as_ref()) }; let volumes = match value.get("volumes") { diff --git a/plugins/zenoh-plugin-storage-manager/Cargo.toml b/plugins/zenoh-plugin-storage-manager/Cargo.toml index 9ef1846d72..fa7650fcc2 100644 --- a/plugins/zenoh-plugin-storage-manager/Cargo.toml +++ b/plugins/zenoh-plugin-storage-manager/Cargo.toml @@ -54,6 +54,7 @@ zenoh = { workspace = true, features = [ ] } zenoh-plugin-trait = { workspace = true } zenoh_backend_traits = { workspace = true } +zenoh-util = { workspace = true } [build-dependencies] rustc_version = { workspace = true } diff --git a/plugins/zenoh-plugin-storage-manager/src/lib.rs b/plugins/zenoh-plugin-storage-manager/src/lib.rs index 3f98725a5e..4043665c5d 100644 --- a/plugins/zenoh-plugin-storage-manager/src/lib.rs +++ b/plugins/zenoh-plugin-storage-manager/src/lib.rs @@ -112,9 +112,7 @@ impl StorageRuntimeInner { storages, .. } = config; - let lib_loader = backend_search_dirs - .map(|search_dirs| LibLoader::new(&search_dirs, false)) - .unwrap_or_default(); + let lib_loader = LibLoader::new(backend_search_dirs); let plugins_manager = PluginsManager::dynamic(lib_loader.clone(), BACKEND_LIB_PREFIX) diff --git a/zenohd/src/main.rs b/zenohd/src/main.rs index 60d898d84f..37fbf10880 100644 --- a/zenohd/src/main.rs +++ b/zenohd/src/main.rs @@ -21,6 +21,7 @@ use zenoh::{ config::{Config, EndPoint, ModeDependentValue, PermissionsConf, ValidatedMap, WhatAmI}, Result, }; +use zenoh_util::LibSearchDirs; #[cfg(feature = "loki")] const LOKI_ENDPOINT_VAR: &str = "LOKI_ENDPOINT"; @@ -146,7 +147,11 @@ fn config_from_args(args: &Args) -> Config { if !args.plugin_search_dir.is_empty() { config .plugins_loading - .set_search_dirs(Some(args.plugin_search_dir.clone())) + // REVIEW: Should this append to search_dirs instead? As there is no way to pass the new + // `current_exe_parent` unless we change the format of the argument and this overrides + // the one set from the default config. + // Also, --cfg plugins_loading/search_dirs=[...] makes this argument superfluous. + .set_search_dirs(LibSearchDirs::from_paths(&args.plugin_search_dir)) .unwrap(); } for plugin in &args.plugin {