From 91d44f15c12005744c94bb883fe4092d6acea1d2 Mon Sep 17 00:00:00 2001 From: Devyn Cairns Date: Fri, 21 Jun 2024 04:27:09 -0700 Subject: [PATCH] Allow plugins to report their own version and store it in the registry (#12883) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description This allows plugins to report their version (and potentially other metadata in the future). The version is shown in `plugin list` and in `version`. The metadata is stored in the registry file, and reflects whatever was retrieved on `plugin add`, not necessarily the running binary. This can help you to diagnose if there's some kind of mismatch with what you expect. We could potentially use this functionality to show a warning or error if a plugin being run does not have the same version as what was in the cache file, suggesting `plugin add` be run again, but I haven't done that at this point. It is optional, and it requires the plugin author to make some code changes if they want to provide it, since I can't automatically determine the version of the calling crate or anything tricky like that to do it. Example: ``` > plugin list | select name version is_running pid ╭───┬────────────────┬─────────┬────────────┬─────╮ │ # │ name │ version │ is_running │ pid │ ├───┼────────────────┼─────────┼────────────┼─────┤ │ 0 │ example │ 0.93.1 │ false │ │ │ 1 │ gstat │ 0.93.1 │ false │ │ │ 2 │ inc │ 0.93.1 │ false │ │ │ 3 │ python_example │ 0.1.0 │ false │ │ ╰───┴────────────────┴─────────┴────────────┴─────╯ ``` cc @maxim-uvarov (he asked for it) # User-Facing Changes - `plugin list` gets a `version` column - `version` shows plugin versions when available - plugin authors *should* add `fn metadata()` to their `impl Plugin`, but don't have to # Tests + Formatting Tested the low level stuff and also the `plugin list` column. # After Submitting - [ ] update plugin guide docs - [ ] update plugin protocol docs (`Metadata` call & response) - [ ] update plugin template (`fn metadata()` should be easy) - [ ] release notes --- crates/nu-cli/src/config_files.rs | 5 ++- .../nu-cmd-lang/src/core_commands/version.rs | 11 +++++- .../nu-cmd-plugin/src/commands/plugin/add.rs | 5 ++- .../nu-cmd-plugin/src/commands/plugin/list.rs | 9 +++++ crates/nu-parser/src/parse_keywords.rs | 33 ++++++++++------ crates/nu-plugin-engine/src/init.rs | 5 ++- crates/nu-plugin-engine/src/interface/mod.rs | 17 ++++++++- .../nu-plugin-engine/src/interface/tests.rs | 21 +++++++++- crates/nu-plugin-engine/src/persistent.rs | 15 +++++++- crates/nu-plugin-protocol/src/lib.rs | 7 +++- .../src/fake_persistent_plugin.rs | 8 +++- crates/nu-plugin-test-support/src/lib.rs | 4 ++ .../tests/custom_value/mod.rs | 4 ++ .../nu-plugin-test-support/tests/hello/mod.rs | 4 ++ .../tests/lowercase/mod.rs | 4 ++ crates/nu-plugin/src/lib.rs | 4 ++ crates/nu-plugin/src/plugin/command.rs | 6 +++ crates/nu-plugin/src/plugin/interface/mod.rs | 21 ++++++++-- .../nu-plugin/src/plugin/interface/tests.rs | 20 ++++++++++ crates/nu-plugin/src/plugin/mod.rs | 31 ++++++++++++++- crates/nu-protocol/src/plugin/metadata.rs | 38 +++++++++++++++++++ crates/nu-protocol/src/plugin/mod.rs | 2 + crates/nu-protocol/src/plugin/registered.rs | 8 +++- .../src/plugin/registry_file/mod.rs | 10 +++-- .../src/plugin/registry_file/tests.rs | 9 ++++- crates/nu_plugin_custom_values/src/main.rs | 4 ++ crates/nu_plugin_example/src/lib.rs | 4 ++ crates/nu_plugin_formats/src/lib.rs | 4 ++ crates/nu_plugin_gstat/src/nu/mod.rs | 4 ++ crates/nu_plugin_inc/src/nu/mod.rs | 4 ++ .../nu_plugin_nu_example.nu | 8 ++++ crates/nu_plugin_polars/src/lib.rs | 4 ++ .../nu_plugin_python_example.py | 9 ++++- crates/nu_plugin_query/src/query.rs | 4 ++ crates/nu_plugin_stress_internals/src/main.rs | 16 +++++++- src/main.rs | 12 ++++-- tests/plugin_persistence/mod.rs | 11 ++++++ tests/plugins/registry_file.rs | 21 ++++++---- 38 files changed, 360 insertions(+), 46 deletions(-) create mode 100644 crates/nu-protocol/src/plugin/metadata.rs diff --git a/crates/nu-cli/src/config_files.rs b/crates/nu-cli/src/config_files.rs index ec7ad2f412..6cf4735c46 100644 --- a/crates/nu-cli/src/config_files.rs +++ b/crates/nu-cli/src/config_files.rs @@ -344,7 +344,10 @@ pub fn migrate_old_plugin_file(engine_state: &EngineState, storage_path: &str) - name: identity.name().to_owned(), filename: identity.filename().to_owned(), shell: identity.shell().map(|p| p.to_owned()), - data: PluginRegistryItemData::Valid { commands }, + data: PluginRegistryItemData::Valid { + metadata: Default::default(), + commands, + }, }); } diff --git a/crates/nu-cmd-lang/src/core_commands/version.rs b/crates/nu-cmd-lang/src/core_commands/version.rs index 77283105a9..5491db65fc 100644 --- a/crates/nu-cmd-lang/src/core_commands/version.rs +++ b/crates/nu-cmd-lang/src/core_commands/version.rs @@ -116,11 +116,18 @@ pub fn version(engine_state: &EngineState, span: Span) -> Result>(); record.push( diff --git a/crates/nu-cmd-plugin/src/commands/plugin/add.rs b/crates/nu-cmd-plugin/src/commands/plugin/add.rs index 225941db01..14f3541168 100644 --- a/crates/nu-cmd-plugin/src/commands/plugin/add.rs +++ b/crates/nu-cmd-plugin/src/commands/plugin/add.rs @@ -118,11 +118,12 @@ apparent the next time `nu` is next launched with that plugin registry file. }, )); let interface = plugin.clone().get_plugin(Some((engine_state, stack)))?; + let metadata = interface.get_metadata()?; let commands = interface.get_signature()?; modify_plugin_file(engine_state, stack, call.head, custom_path, |contents| { - // Update the file with the received signatures - let item = PluginRegistryItem::new(plugin.identity(), commands); + // Update the file with the received metadata and signatures + let item = PluginRegistryItem::new(plugin.identity(), metadata, commands); contents.upsert_plugin(item); Ok(()) })?; diff --git a/crates/nu-cmd-plugin/src/commands/plugin/list.rs b/crates/nu-cmd-plugin/src/commands/plugin/list.rs index 6b715a0001..030a3341d6 100644 --- a/crates/nu-cmd-plugin/src/commands/plugin/list.rs +++ b/crates/nu-cmd-plugin/src/commands/plugin/list.rs @@ -16,6 +16,7 @@ impl Command for PluginList { Type::Table( [ ("name".into(), Type::String), + ("version".into(), Type::String), ("is_running".into(), Type::Bool), ("pid".into(), Type::Int), ("filename".into(), Type::String), @@ -43,6 +44,7 @@ impl Command for PluginList { description: "List installed plugins.", result: Some(Value::test_list(vec![Value::test_record(record! { "name" => Value::test_string("inc"), + "version" => Value::test_string(env!("CARGO_PKG_VERSION")), "is_running" => Value::test_bool(true), "pid" => Value::test_int(106480), "filename" => if cfg!(windows) { @@ -98,8 +100,15 @@ impl Command for PluginList { .map(|s| Value::string(s.to_string_lossy(), head)) .unwrap_or(Value::nothing(head)); + let metadata = plugin.metadata(); + let version = metadata + .and_then(|m| m.version) + .map(|s| Value::string(s, head)) + .unwrap_or(Value::nothing(head)); + let record = record! { "name" => Value::string(plugin.identity().name(), head), + "version" => version, "is_running" => Value::bool(plugin.is_running(), head), "pid" => pid, "filename" => Value::string(plugin.identity().filename().to_string_lossy(), head), diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 2849b4e39c..673a59fb05 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -3740,28 +3740,37 @@ pub fn parse_register(working_set: &mut StateWorkingSet, lite_command: &LiteComm ) })?; - let signatures = plugin + let metadata_and_signatures = plugin .clone() .get(get_envs) - .and_then(|p| p.get_signature()) + .and_then(|p| { + let meta = p.get_metadata()?; + let sigs = p.get_signature()?; + Ok((meta, sigs)) + }) .map_err(|err| { - log::warn!("Error getting signatures: {err:?}"); + log::warn!("Error getting metadata and signatures: {err:?}"); ParseError::LabeledError( - "Error getting signatures".into(), + "Error getting metadata and signatures".into(), err.to_string(), spans[0], ) }); - if let Ok(ref signatures) = signatures { - // Add the loaded plugin to the delta - working_set.update_plugin_registry(PluginRegistryItem::new( - &identity, - signatures.clone(), - )); + match metadata_and_signatures { + Ok((meta, sigs)) => { + // Set the metadata on the plugin + plugin.set_metadata(Some(meta.clone())); + // Add the loaded plugin to the delta + working_set.update_plugin_registry(PluginRegistryItem::new( + &identity, + meta, + sigs.clone(), + )); + Ok(sigs) + } + Err(err) => Err(err), } - - signatures }, |sig| sig.map(|sig| vec![sig]), )?; diff --git a/crates/nu-plugin-engine/src/init.rs b/crates/nu-plugin-engine/src/init.rs index 198a01cd1c..44fa4f031f 100644 --- a/crates/nu-plugin-engine/src/init.rs +++ b/crates/nu-plugin-engine/src/init.rs @@ -252,7 +252,7 @@ pub fn load_plugin_registry_item( })?; match &plugin.data { - PluginRegistryItemData::Valid { commands } => { + PluginRegistryItemData::Valid { metadata, commands } => { let plugin = add_plugin_to_working_set(working_set, &identity)?; // Ensure that the plugin is reset. We're going to load new signatures, so we want to @@ -260,6 +260,9 @@ pub fn load_plugin_registry_item( // doesn't. plugin.reset()?; + // Set the plugin metadata from the file + plugin.set_metadata(Some(metadata.clone())); + // Create the declarations from the commands for signature in commands { let decl = PluginDeclaration::new(plugin.clone(), signature.clone()); diff --git a/crates/nu-plugin-engine/src/interface/mod.rs b/crates/nu-plugin-engine/src/interface/mod.rs index adab9dc68d..9a4c72d2c1 100644 --- a/crates/nu-plugin-engine/src/interface/mod.rs +++ b/crates/nu-plugin-engine/src/interface/mod.rs @@ -11,8 +11,8 @@ use nu_plugin_protocol::{ PluginOutput, ProtocolInfo, StreamId, StreamMessage, }; use nu_protocol::{ - ast::Operator, CustomValue, IntoSpanned, PipelineData, PluginSignature, ShellError, Span, - Spanned, Value, + ast::Operator, CustomValue, IntoSpanned, PipelineData, PluginMetadata, PluginSignature, + ShellError, Span, Spanned, Value, }; use std::{ collections::{btree_map, BTreeMap}, @@ -716,6 +716,7 @@ impl PluginInterface { // Convert the call into one with a header and handle the stream, if necessary let (call, writer) = match call { + PluginCall::Metadata => (PluginCall::Metadata, Default::default()), PluginCall::Signature => (PluginCall::Signature, Default::default()), PluginCall::CustomValueOp(value, op) => { (PluginCall::CustomValueOp(value, op), Default::default()) @@ -913,6 +914,17 @@ impl PluginInterface { self.receive_plugin_call_response(result.receiver, context, result.state) } + /// Get the metadata from the plugin. + pub fn get_metadata(&self) -> Result { + match self.plugin_call(PluginCall::Metadata, None)? { + PluginCallResponse::Metadata(meta) => Ok(meta), + PluginCallResponse::Error(err) => Err(err.into()), + _ => Err(ShellError::PluginFailedToDecode { + msg: "Received unexpected response to plugin Metadata call".into(), + }), + } + } + /// Get the command signatures from the plugin. pub fn get_signature(&self) -> Result, ShellError> { match self.plugin_call(PluginCall::Signature, None)? { @@ -1206,6 +1218,7 @@ impl CurrentCallState { source: &PluginSource, ) -> Result<(), ShellError> { match call { + PluginCall::Metadata => Ok(()), PluginCall::Signature => Ok(()), PluginCall::Run(CallInfo { call, .. }) => self.prepare_call_args(call, source), PluginCall::CustomValueOp(_, op) => { diff --git a/crates/nu-plugin-engine/src/interface/tests.rs b/crates/nu-plugin-engine/src/interface/tests.rs index e718886b3b..5665beb92b 100644 --- a/crates/nu-plugin-engine/src/interface/tests.rs +++ b/crates/nu-plugin-engine/src/interface/tests.rs @@ -18,7 +18,7 @@ use nu_protocol::{ ast::{Math, Operator}, engine::Closure, ByteStreamType, CustomValue, IntoInterruptiblePipelineData, IntoSpanned, PipelineData, - PluginSignature, ShellError, Span, Spanned, Value, + PluginMetadata, PluginSignature, ShellError, Span, Spanned, Value, }; use serde::{Deserialize, Serialize}; use std::{ @@ -1019,6 +1019,25 @@ fn start_fake_plugin_call_responder( .expect("failed to spawn thread"); } +#[test] +fn interface_get_metadata() -> Result<(), ShellError> { + let test = TestCase::new(); + let manager = test.plugin("test"); + let interface = manager.get_interface(); + + start_fake_plugin_call_responder(manager, 1, |_| { + vec![ReceivedPluginCallMessage::Response( + PluginCallResponse::Metadata(PluginMetadata::new().with_version("test")), + )] + }); + + let metadata = interface.get_metadata()?; + + assert_eq!(Some("test"), metadata.version.as_deref()); + assert!(test.has_unconsumed_write()); + Ok(()) +} + #[test] fn interface_get_signature() -> Result<(), ShellError> { let test = TestCase::new(); diff --git a/crates/nu-plugin-engine/src/persistent.rs b/crates/nu-plugin-engine/src/persistent.rs index 5f01c70ca7..6a87aa1e6b 100644 --- a/crates/nu-plugin-engine/src/persistent.rs +++ b/crates/nu-plugin-engine/src/persistent.rs @@ -7,7 +7,7 @@ use super::{PluginInterface, PluginSource}; use nu_plugin_core::CommunicationMode; use nu_protocol::{ engine::{EngineState, Stack}, - PluginGcConfig, PluginIdentity, RegisteredPlugin, ShellError, + PluginGcConfig, PluginIdentity, PluginMetadata, RegisteredPlugin, ShellError, }; use std::{ collections::HashMap, @@ -31,6 +31,8 @@ pub struct PersistentPlugin { struct MutableState { /// Reference to the plugin if running running: Option, + /// Metadata for the plugin, e.g. version. + metadata: Option, /// Plugin's preferred communication mode (if known) preferred_mode: Option, /// Garbage collector config @@ -59,6 +61,7 @@ impl PersistentPlugin { identity, mutable: Mutex::new(MutableState { running: None, + metadata: None, preferred_mode: None, gc_config, }), @@ -268,6 +271,16 @@ impl RegisteredPlugin for PersistentPlugin { self.stop_internal(true) } + fn metadata(&self) -> Option { + self.mutable.lock().ok().and_then(|m| m.metadata.clone()) + } + + fn set_metadata(&self, metadata: Option) { + if let Ok(mut mutable) = self.mutable.lock() { + mutable.metadata = metadata; + } + } + fn set_gc_config(&self, gc_config: &PluginGcConfig) { if let Ok(mut mutable) = self.mutable.lock() { // Save the new config for future calls diff --git a/crates/nu-plugin-protocol/src/lib.rs b/crates/nu-plugin-protocol/src/lib.rs index db19ee02f6..2f582c3009 100644 --- a/crates/nu-plugin-protocol/src/lib.rs +++ b/crates/nu-plugin-protocol/src/lib.rs @@ -23,7 +23,7 @@ pub mod test_util; use nu_protocol::{ ast::Operator, engine::Closure, ByteStreamType, Config, LabeledError, PipelineData, - PluginSignature, ShellError, Span, Spanned, Value, + PluginMetadata, PluginSignature, ShellError, Span, Spanned, Value, }; use serde::{Deserialize, Serialize}; use std::collections::HashMap; @@ -119,6 +119,7 @@ pub struct ByteStreamInfo { /// Calls that a plugin can execute. The type parameter determines the input type. #[derive(Serialize, Deserialize, Debug, Clone)] pub enum PluginCall { + Metadata, Signature, Run(CallInfo), CustomValueOp(Spanned, CustomValueOp), @@ -132,6 +133,7 @@ impl PluginCall { f: impl FnOnce(D) -> Result, ) -> Result, ShellError> { Ok(match self { + PluginCall::Metadata => PluginCall::Metadata, PluginCall::Signature => PluginCall::Signature, PluginCall::Run(call) => PluginCall::Run(call.map_data(f)?), PluginCall::CustomValueOp(custom_value, op) => { @@ -143,6 +145,7 @@ impl PluginCall { /// The span associated with the call. pub fn span(&self) -> Option { match self { + PluginCall::Metadata => None, PluginCall::Signature => None, PluginCall::Run(CallInfo { call, .. }) => Some(call.head), PluginCall::CustomValueOp(val, _) => Some(val.span), @@ -309,6 +312,7 @@ pub enum StreamMessage { #[derive(Serialize, Deserialize, Debug, Clone)] pub enum PluginCallResponse { Error(LabeledError), + Metadata(PluginMetadata), Signature(Vec), Ordering(Option), PipelineData(D), @@ -323,6 +327,7 @@ impl PluginCallResponse { ) -> Result, ShellError> { Ok(match self { PluginCallResponse::Error(err) => PluginCallResponse::Error(err), + PluginCallResponse::Metadata(meta) => PluginCallResponse::Metadata(meta), PluginCallResponse::Signature(sigs) => PluginCallResponse::Signature(sigs), PluginCallResponse::Ordering(ordering) => PluginCallResponse::Ordering(ordering), PluginCallResponse::PipelineData(input) => PluginCallResponse::PipelineData(f(input)?), diff --git a/crates/nu-plugin-test-support/src/fake_persistent_plugin.rs b/crates/nu-plugin-test-support/src/fake_persistent_plugin.rs index 617e55c3d1..e316e85147 100644 --- a/crates/nu-plugin-test-support/src/fake_persistent_plugin.rs +++ b/crates/nu-plugin-test-support/src/fake_persistent_plugin.rs @@ -6,7 +6,7 @@ use std::{ use nu_plugin_engine::{GetPlugin, PluginInterface}; use nu_protocol::{ engine::{EngineState, Stack}, - PluginGcConfig, PluginIdentity, RegisteredPlugin, ShellError, + PluginGcConfig, PluginIdentity, PluginMetadata, RegisteredPlugin, ShellError, }; pub struct FakePersistentPlugin { @@ -42,6 +42,12 @@ impl RegisteredPlugin for FakePersistentPlugin { None } + fn metadata(&self) -> Option { + None + } + + fn set_metadata(&self, _metadata: Option) {} + fn set_gc_config(&self, _gc_config: &PluginGcConfig) { // We don't have a GC } diff --git a/crates/nu-plugin-test-support/src/lib.rs b/crates/nu-plugin-test-support/src/lib.rs index caa7cbac1a..a53b353981 100644 --- a/crates/nu-plugin-test-support/src/lib.rs +++ b/crates/nu-plugin-test-support/src/lib.rs @@ -66,6 +66,10 @@ //! } //! //! impl Plugin for LowercasePlugin { +//! fn version(&self) -> String { +//! env!("CARGO_PKG_VERSION").into() +//! } +//! //! fn commands(&self) -> Vec>> { //! vec![Box::new(Lowercase)] //! } diff --git a/crates/nu-plugin-test-support/tests/custom_value/mod.rs b/crates/nu-plugin-test-support/tests/custom_value/mod.rs index f703a92e33..888f05c1cb 100644 --- a/crates/nu-plugin-test-support/tests/custom_value/mod.rs +++ b/crates/nu-plugin-test-support/tests/custom_value/mod.rs @@ -53,6 +53,10 @@ struct IntoU32; struct IntoIntFromU32; impl Plugin for CustomU32Plugin { + fn version(&self) -> String { + "0.0.0".into() + } + fn commands(&self) -> Vec>> { vec![Box::new(IntoU32), Box::new(IntoIntFromU32)] } diff --git a/crates/nu-plugin-test-support/tests/hello/mod.rs b/crates/nu-plugin-test-support/tests/hello/mod.rs index 424940f156..a222637a76 100644 --- a/crates/nu-plugin-test-support/tests/hello/mod.rs +++ b/crates/nu-plugin-test-support/tests/hello/mod.rs @@ -8,6 +8,10 @@ struct HelloPlugin; struct Hello; impl Plugin for HelloPlugin { + fn version(&self) -> String { + "0.0.0".into() + } + fn commands(&self) -> Vec>> { vec![Box::new(Hello)] } diff --git a/crates/nu-plugin-test-support/tests/lowercase/mod.rs b/crates/nu-plugin-test-support/tests/lowercase/mod.rs index 0072a08aa2..50271a8cc2 100644 --- a/crates/nu-plugin-test-support/tests/lowercase/mod.rs +++ b/crates/nu-plugin-test-support/tests/lowercase/mod.rs @@ -59,6 +59,10 @@ impl PluginCommand for Lowercase { } impl Plugin for LowercasePlugin { + fn version(&self) -> String { + "0.0.0".into() + } + fn commands(&self) -> Vec>> { vec![Box::new(Lowercase)] } diff --git a/crates/nu-plugin/src/lib.rs b/crates/nu-plugin/src/lib.rs index 915c8d36fe..4c1a033a02 100644 --- a/crates/nu-plugin/src/lib.rs +++ b/crates/nu-plugin/src/lib.rs @@ -24,6 +24,10 @@ //! struct MyCommand; //! //! impl Plugin for MyPlugin { +//! fn version(&self) -> String { +//! env!("CARGO_PKG_VERSION").into() +//! } +//! //! fn commands(&self) -> Vec>> { //! vec![Box::new(MyCommand)] //! } diff --git a/crates/nu-plugin/src/plugin/command.rs b/crates/nu-plugin/src/plugin/command.rs index 5def950b0b..7ccedfd4e5 100644 --- a/crates/nu-plugin/src/plugin/command.rs +++ b/crates/nu-plugin/src/plugin/command.rs @@ -60,6 +60,9 @@ use crate::{EngineInterface, EvaluatedCall, Plugin}; /// } /// /// # impl Plugin for LowercasePlugin { +/// # fn version(&self) -> String { +/// # "0.0.0".into() +/// # } /// # fn commands(&self) -> Vec>> { /// # vec![Box::new(Lowercase)] /// # } @@ -195,6 +198,9 @@ pub trait PluginCommand: Sync { /// } /// /// # impl Plugin for HelloPlugin { +/// # fn version(&self) -> String { +/// # "0.0.0".into() +/// # } /// # fn commands(&self) -> Vec>> { /// # vec![Box::new(Hello)] /// # } diff --git a/crates/nu-plugin/src/plugin/interface/mod.rs b/crates/nu-plugin/src/plugin/interface/mod.rs index e3e9679471..fc9c9e85b3 100644 --- a/crates/nu-plugin/src/plugin/interface/mod.rs +++ b/crates/nu-plugin/src/plugin/interface/mod.rs @@ -11,8 +11,8 @@ use nu_plugin_protocol::{ ProtocolInfo, }; use nu_protocol::{ - engine::Closure, Config, LabeledError, PipelineData, PluginSignature, ShellError, Span, - Spanned, Value, + engine::Closure, Config, LabeledError, PipelineData, PluginMetadata, PluginSignature, + ShellError, Span, Spanned, Value, }; use std::{ collections::{btree_map, BTreeMap, HashMap}, @@ -29,6 +29,9 @@ use std::{ #[derive(Debug)] #[doc(hidden)] pub enum ReceivedPluginCall { + Metadata { + engine: EngineInterface, + }, Signature { engine: EngineInterface, }, @@ -280,8 +283,11 @@ impl InterfaceManager for EngineInterfaceManager { } }; match call { - // We just let the receiver handle it rather than trying to store signature here - // or something + // Ask the plugin for metadata + PluginCall::Metadata => { + self.send_plugin_call(ReceivedPluginCall::Metadata { engine: interface }) + } + // Ask the plugin for signatures PluginCall::Signature => { self.send_plugin_call(ReceivedPluginCall::Signature { engine: interface }) } @@ -416,6 +422,13 @@ impl EngineInterface { } } + /// Write a call response of plugin metadata. + pub(crate) fn write_metadata(&self, metadata: PluginMetadata) -> Result<(), ShellError> { + let response = PluginCallResponse::Metadata(metadata); + self.write(PluginOutput::CallResponse(self.context()?, response))?; + self.flush() + } + /// Write a call response of plugin signatures. /// /// Any custom values in the examples will be rendered using `to_base_value()`. diff --git a/crates/nu-plugin/src/plugin/interface/tests.rs b/crates/nu-plugin/src/plugin/interface/tests.rs index 6c3dfdf6c9..b195b43197 100644 --- a/crates/nu-plugin/src/plugin/interface/tests.rs +++ b/crates/nu-plugin/src/plugin/interface/tests.rs @@ -322,6 +322,26 @@ fn manager_consume_goodbye_closes_plugin_call_channel() -> Result<(), ShellError Ok(()) } +#[test] +fn manager_consume_call_metadata_forwards_to_receiver_with_context() -> Result<(), ShellError> { + let mut manager = TestCase::new().engine(); + set_default_protocol_info(&mut manager)?; + + let rx = manager + .take_plugin_call_receiver() + .expect("couldn't take receiver"); + + manager.consume(PluginInput::Call(0, PluginCall::Metadata))?; + + match rx.try_recv().expect("call was not forwarded to receiver") { + ReceivedPluginCall::Metadata { engine } => { + assert_eq!(Some(0), engine.context); + Ok(()) + } + call => panic!("wrong call type: {call:?}"), + } +} + #[test] fn manager_consume_call_signature_forwards_to_receiver_with_context() -> Result<(), ShellError> { let mut manager = TestCase::new().engine(); diff --git a/crates/nu-plugin/src/plugin/mod.rs b/crates/nu-plugin/src/plugin/mod.rs index 85283aadd0..997381f97b 100644 --- a/crates/nu-plugin/src/plugin/mod.rs +++ b/crates/nu-plugin/src/plugin/mod.rs @@ -16,7 +16,8 @@ use nu_plugin_core::{ }; use nu_plugin_protocol::{CallInfo, CustomValueOp, PluginCustomValue, PluginInput, PluginOutput}; use nu_protocol::{ - ast::Operator, CustomValue, IntoSpanned, LabeledError, PipelineData, ShellError, Spanned, Value, + ast::Operator, CustomValue, IntoSpanned, LabeledError, PipelineData, PluginMetadata, + ShellError, Spanned, Value, }; use thiserror::Error; @@ -52,6 +53,10 @@ pub(crate) const OUTPUT_BUFFER_SIZE: usize = 16384; /// struct Hello; /// /// impl Plugin for HelloPlugin { +/// fn version(&self) -> String { +/// env!("CARGO_PKG_VERSION").into() +/// } +/// /// fn commands(&self) -> Vec>> { /// vec![Box::new(Hello)] /// } @@ -89,6 +94,23 @@ pub(crate) const OUTPUT_BUFFER_SIZE: usize = 16384; /// # } /// ``` pub trait Plugin: Sync { + /// The version of the plugin. + /// + /// The recommended implementation, which will use the version from your crate's `Cargo.toml` + /// file: + /// + /// ```no_run + /// # use nu_plugin::{Plugin, PluginCommand}; + /// # struct MyPlugin; + /// # impl Plugin for MyPlugin { + /// fn version(&self) -> String { + /// env!("CARGO_PKG_VERSION").into() + /// } + /// # fn commands(&self) -> Vec>> { vec![] } + /// # } + /// ``` + fn version(&self) -> String; + /// The commands supported by the plugin /// /// Each [`PluginCommand`] contains both the signature of the command and the functionality it @@ -216,6 +238,7 @@ pub trait Plugin: Sync { /// # struct MyPlugin; /// # impl MyPlugin { fn new() -> Self { Self }} /// # impl Plugin for MyPlugin { +/// # fn version(&self) -> String { "0.0.0".into() } /// # fn commands(&self) -> Vec>> {todo!();} /// # } /// fn main() { @@ -504,6 +527,12 @@ where } match plugin_call { + // Send metadata back to nushell so it can be stored with the plugin signatures + ReceivedPluginCall::Metadata { engine } => { + engine + .write_metadata(PluginMetadata::new().with_version(plugin.version())) + .try_to_report(&engine)?; + } // Sending the signature back to nushell to create the declaration definition ReceivedPluginCall::Signature { engine } => { let sigs = commands diff --git a/crates/nu-protocol/src/plugin/metadata.rs b/crates/nu-protocol/src/plugin/metadata.rs new file mode 100644 index 0000000000..d2fab7a89b --- /dev/null +++ b/crates/nu-protocol/src/plugin/metadata.rs @@ -0,0 +1,38 @@ +use serde::{Deserialize, Serialize}; + +/// Metadata about the installed plugin. This is cached in the registry file along with the +/// signatures. None of the metadata fields are required, and more may be added in the future. +#[derive(Serialize, Deserialize, PartialEq, Debug, Clone)] +#[non_exhaustive] +pub struct PluginMetadata { + /// The version of the plugin itself, as self-reported. + pub version: Option, +} + +impl PluginMetadata { + /// Create empty metadata. + pub const fn new() -> PluginMetadata { + PluginMetadata { version: None } + } + + /// Set the version of the plugin on the metadata. A suggested way to construct this is: + /// + /// ```no_run + /// # use nu_protocol::PluginMetadata; + /// # fn example() -> PluginMetadata { + /// PluginMetadata::new().with_version(env!("CARGO_PKG_VERSION")) + /// # } + /// ``` + /// + /// which will use the version of your plugin's crate from its `Cargo.toml` file. + pub fn with_version(mut self, version: impl Into) -> Self { + self.version = Some(version.into()); + self + } +} + +impl Default for PluginMetadata { + fn default() -> Self { + Self::new() + } +} diff --git a/crates/nu-protocol/src/plugin/mod.rs b/crates/nu-protocol/src/plugin/mod.rs index b266f8ebac..1678c9c40a 100644 --- a/crates/nu-protocol/src/plugin/mod.rs +++ b/crates/nu-protocol/src/plugin/mod.rs @@ -1,9 +1,11 @@ mod identity; +mod metadata; mod registered; mod registry_file; mod signature; pub use identity::*; +pub use metadata::*; pub use registered::*; pub use registry_file::*; pub use signature::*; diff --git a/crates/nu-protocol/src/plugin/registered.rs b/crates/nu-protocol/src/plugin/registered.rs index 46d65b41d1..abd75b4dc6 100644 --- a/crates/nu-protocol/src/plugin/registered.rs +++ b/crates/nu-protocol/src/plugin/registered.rs @@ -1,6 +1,6 @@ use std::{any::Any, sync::Arc}; -use crate::{PluginGcConfig, PluginIdentity, ShellError}; +use crate::{PluginGcConfig, PluginIdentity, PluginMetadata, ShellError}; /// Trait for plugins registered in the [`EngineState`](crate::engine::EngineState). pub trait RegisteredPlugin: Send + Sync { @@ -13,6 +13,12 @@ pub trait RegisteredPlugin: Send + Sync { /// Process ID of the plugin executable, if running. fn pid(&self) -> Option; + /// Get metadata for the plugin, if set. + fn metadata(&self) -> Option; + + /// Set metadata for the plugin. + fn set_metadata(&self, metadata: Option); + /// Set garbage collection config for the plugin. fn set_gc_config(&self, gc_config: &PluginGcConfig); diff --git a/crates/nu-protocol/src/plugin/registry_file/mod.rs b/crates/nu-protocol/src/plugin/registry_file/mod.rs index dcaba26b90..17adec1f4e 100644 --- a/crates/nu-protocol/src/plugin/registry_file/mod.rs +++ b/crates/nu-protocol/src/plugin/registry_file/mod.rs @@ -5,7 +5,7 @@ use std::{ use serde::{Deserialize, Serialize}; -use crate::{PluginIdentity, PluginSignature, ShellError, Span}; +use crate::{PluginIdentity, PluginMetadata, PluginSignature, ShellError, Span}; // This has a big impact on performance const BUFFER_SIZE: usize = 65536; @@ -121,9 +121,10 @@ pub struct PluginRegistryItem { } impl PluginRegistryItem { - /// Create a [`PluginRegistryItem`] from an identity and signatures. + /// Create a [`PluginRegistryItem`] from an identity, metadata, and signatures. pub fn new( identity: &PluginIdentity, + metadata: PluginMetadata, mut commands: Vec, ) -> PluginRegistryItem { // Sort the commands for consistency @@ -133,7 +134,7 @@ impl PluginRegistryItem { name: identity.name().to_owned(), filename: identity.filename().to_owned(), shell: identity.shell().map(|p| p.to_owned()), - data: PluginRegistryItemData::Valid { commands }, + data: PluginRegistryItemData::Valid { metadata, commands }, } } } @@ -144,6 +145,9 @@ impl PluginRegistryItem { #[serde(untagged)] pub enum PluginRegistryItemData { Valid { + /// Metadata for the plugin, including its version. + #[serde(default)] + metadata: PluginMetadata, /// Signatures and examples for each command provided by the plugin. commands: Vec, }, diff --git a/crates/nu-protocol/src/plugin/registry_file/tests.rs b/crates/nu-protocol/src/plugin/registry_file/tests.rs index 0d34ecca1c..e264e4463d 100644 --- a/crates/nu-protocol/src/plugin/registry_file/tests.rs +++ b/crates/nu-protocol/src/plugin/registry_file/tests.rs @@ -1,6 +1,7 @@ use super::{PluginRegistryFile, PluginRegistryItem, PluginRegistryItemData}; use crate::{ - Category, PluginExample, PluginSignature, ShellError, Signature, SyntaxShape, Type, Value, + Category, PluginExample, PluginMetadata, PluginSignature, ShellError, Signature, SyntaxShape, + Type, Value, }; use pretty_assertions::assert_eq; use std::io::Cursor; @@ -11,6 +12,9 @@ fn foo_plugin() -> PluginRegistryItem { filename: "/path/to/nu_plugin_foo".into(), shell: None, data: PluginRegistryItemData::Valid { + metadata: PluginMetadata { + version: Some("0.1.0".into()), + }, commands: vec![PluginSignature { sig: Signature::new("foo") .input_output_type(Type::Int, Type::List(Box::new(Type::Int))) @@ -36,6 +40,9 @@ fn bar_plugin() -> PluginRegistryItem { filename: "/path/to/nu_plugin_bar".into(), shell: None, data: PluginRegistryItemData::Valid { + metadata: PluginMetadata { + version: Some("0.2.0".into()), + }, commands: vec![PluginSignature { sig: Signature::new("bar") .usage("overwrites files with random data") diff --git a/crates/nu_plugin_custom_values/src/main.rs b/crates/nu_plugin_custom_values/src/main.rs index 9ab69135fb..65cc0b500f 100644 --- a/crates/nu_plugin_custom_values/src/main.rs +++ b/crates/nu_plugin_custom_values/src/main.rs @@ -42,6 +42,10 @@ impl CustomValuePlugin { } impl Plugin for CustomValuePlugin { + fn version(&self) -> String { + env!("CARGO_PKG_VERSION").into() + } + fn commands(&self) -> Vec>> { vec![ Box::new(Generate), diff --git a/crates/nu_plugin_example/src/lib.rs b/crates/nu_plugin_example/src/lib.rs index 182bc85121..acbebf9b39 100644 --- a/crates/nu_plugin_example/src/lib.rs +++ b/crates/nu_plugin_example/src/lib.rs @@ -7,6 +7,10 @@ pub use commands::*; pub use example::ExamplePlugin; impl Plugin for ExamplePlugin { + fn version(&self) -> String { + env!("CARGO_PKG_VERSION").into() + } + fn commands(&self) -> Vec>> { // This is a list of all of the commands you would like Nu to register when your plugin is // loaded. diff --git a/crates/nu_plugin_formats/src/lib.rs b/crates/nu_plugin_formats/src/lib.rs index 748d29cd21..2ae24a4971 100644 --- a/crates/nu_plugin_formats/src/lib.rs +++ b/crates/nu_plugin_formats/src/lib.rs @@ -10,6 +10,10 @@ pub use from::vcf::FromVcf; pub struct FromCmds; impl Plugin for FromCmds { + fn version(&self) -> String { + env!("CARGO_PKG_VERSION").into() + } + fn commands(&self) -> Vec>> { vec![ Box::new(FromEml), diff --git a/crates/nu_plugin_gstat/src/nu/mod.rs b/crates/nu_plugin_gstat/src/nu/mod.rs index 223e5a49b5..f44894085d 100644 --- a/crates/nu_plugin_gstat/src/nu/mod.rs +++ b/crates/nu_plugin_gstat/src/nu/mod.rs @@ -5,6 +5,10 @@ use nu_protocol::{Category, LabeledError, Signature, Spanned, SyntaxShape, Value pub struct GStatPlugin; impl Plugin for GStatPlugin { + fn version(&self) -> String { + env!("CARGO_PKG_VERSION").into() + } + fn commands(&self) -> Vec>> { vec![Box::new(GStat)] } diff --git a/crates/nu_plugin_inc/src/nu/mod.rs b/crates/nu_plugin_inc/src/nu/mod.rs index 148f1a7002..b7d9c8960f 100644 --- a/crates/nu_plugin_inc/src/nu/mod.rs +++ b/crates/nu_plugin_inc/src/nu/mod.rs @@ -5,6 +5,10 @@ use nu_protocol::{ast::CellPath, LabeledError, Signature, SyntaxShape, Value}; pub struct IncPlugin; impl Plugin for IncPlugin { + fn version(&self) -> String { + env!("CARGO_PKG_VERSION").into() + } + fn commands(&self) -> Vec>> { vec![Box::new(Inc::new())] } diff --git a/crates/nu_plugin_nu_example/nu_plugin_nu_example.nu b/crates/nu_plugin_nu_example/nu_plugin_nu_example.nu index 028e9735fd..4bfc066815 100755 --- a/crates/nu_plugin_nu_example/nu_plugin_nu_example.nu +++ b/crates/nu_plugin_nu_example/nu_plugin_nu_example.nu @@ -7,6 +7,7 @@ # language without adding any extra dependencies to our tests. const NUSHELL_VERSION = "0.94.3" +const PLUGIN_VERSION = "0.1.0" # bump if you change commands! def main [--stdio] { if ($stdio) { @@ -229,6 +230,13 @@ def handle_input []: any -> nothing { } { Call: [$id, $plugin_call] } => { match $plugin_call { + "Metadata" => { + write_response $id { + Metadata: { + version: $PLUGIN_VERSION + } + } + } "Signature" => { write_response $id { Signature: $SIGNATURES } } diff --git a/crates/nu_plugin_polars/src/lib.rs b/crates/nu_plugin_polars/src/lib.rs index 76da2eb039..9fc9a7ab2d 100644 --- a/crates/nu_plugin_polars/src/lib.rs +++ b/crates/nu_plugin_polars/src/lib.rs @@ -99,6 +99,10 @@ pub struct PolarsPlugin { } impl Plugin for PolarsPlugin { + fn version(&self) -> String { + env!("CARGO_PKG_VERSION").into() + } + fn commands(&self) -> Vec>> { let mut commands: Vec>> = vec![Box::new(PolarsCmd)]; commands.append(&mut eager_commands()); diff --git a/crates/nu_plugin_python/nu_plugin_python_example.py b/crates/nu_plugin_python/nu_plugin_python_example.py index 3db89a0afc..80715abf11 100755 --- a/crates/nu_plugin_python/nu_plugin_python_example.py +++ b/crates/nu_plugin_python/nu_plugin_python_example.py @@ -28,6 +28,7 @@ import json NUSHELL_VERSION = "0.94.3" +PLUGIN_VERSION = "0.1.0" # bump if you change commands! def signatures(): @@ -228,7 +229,13 @@ def handle_input(input): exit(0) elif "Call" in input: [id, plugin_call] = input["Call"] - if plugin_call == "Signature": + if plugin_call == "Metadata": + write_response(id, { + "Metadata": { + "version": PLUGIN_VERSION, + } + }) + elif plugin_call == "Signature": write_response(id, signatures()) elif "Run" in plugin_call: process_call(id, plugin_call["Run"]) diff --git a/crates/nu_plugin_query/src/query.rs b/crates/nu_plugin_query/src/query.rs index 1e143068c4..c22339ab4a 100644 --- a/crates/nu_plugin_query/src/query.rs +++ b/crates/nu_plugin_query/src/query.rs @@ -16,6 +16,10 @@ impl Query { } impl Plugin for Query { + fn version(&self) -> String { + env!("CARGO_PKG_VERSION").into() + } + fn commands(&self) -> Vec>> { vec![ Box::new(QueryCommand), diff --git a/crates/nu_plugin_stress_internals/src/main.rs b/crates/nu_plugin_stress_internals/src/main.rs index bdbe5c8943..96023b44b7 100644 --- a/crates/nu_plugin_stress_internals/src/main.rs +++ b/crates/nu_plugin_stress_internals/src/main.rs @@ -136,7 +136,21 @@ fn handle_message( ) -> Result<(), Box> { if let Some(plugin_call) = message.get("Call") { let (id, plugin_call) = (&plugin_call[0], &plugin_call[1]); - if plugin_call.as_str() == Some("Signature") { + if plugin_call.as_str() == Some("Metadata") { + write( + output, + &json!({ + "CallResponse": [ + id, + { + "Metadata": { + "version": env!("CARGO_PKG_VERSION"), + } + } + ] + }), + ) + } else if plugin_call.as_str() == Some("Signature") { write( output, &json!({ diff --git a/src/main.rs b/src/main.rs index 41d5534bb0..c74fd7641d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -400,7 +400,7 @@ fn main() -> Result<()> { #[cfg(feature = "plugin")] if let Some(plugins) = &parsed_nu_cli_args.plugins { use nu_plugin_engine::{GetPlugin, PluginDeclaration}; - use nu_protocol::{engine::StateWorkingSet, ErrSpan, PluginIdentity}; + use nu_protocol::{engine::StateWorkingSet, ErrSpan, PluginIdentity, RegisteredPlugin}; // Load any plugins specified with --plugins start_time = std::time::Instant::now(); @@ -419,8 +419,14 @@ fn main() -> Result<()> { // Create the plugin and add it to the working set let plugin = nu_plugin_engine::add_plugin_to_working_set(&mut working_set, &identity)?; - // Spawn the plugin to get its signatures, and then add the commands to the working set - for signature in plugin.clone().get_plugin(None)?.get_signature()? { + // Spawn the plugin to get the metadata and signatures + let interface = plugin.clone().get_plugin(None)?; + + // Set its metadata + plugin.set_metadata(Some(interface.get_metadata()?)); + + // Add the commands from the signature to the working set + for signature in interface.get_signature()? { let decl = PluginDeclaration::new(plugin.clone(), signature); working_set.add_decl(Box::new(decl)); } diff --git a/tests/plugin_persistence/mod.rs b/tests/plugin_persistence/mod.rs index d9925f6bbf..6729c657c7 100644 --- a/tests/plugin_persistence/mod.rs +++ b/tests/plugin_persistence/mod.rs @@ -16,6 +16,17 @@ fn plugin_list_shows_installed_plugins() { assert!(out.status.success()); } +#[test] +fn plugin_list_shows_installed_plugin_version() { + let out = nu_with_plugins!( + cwd: ".", + plugin: ("nu_plugin_inc"), + r#"(plugin list).version.0"# + ); + assert_eq!(env!("CARGO_PKG_VERSION"), out.out); + assert!(out.status.success()); +} + #[test] fn plugin_keeps_running_after_calling_it() { let out = nu_with_plugins!( diff --git a/tests/plugins/registry_file.rs b/tests/plugins/registry_file.rs index 3f7da6dd03..c0f0fa0724 100644 --- a/tests/plugins/registry_file.rs +++ b/tests/plugins/registry_file.rs @@ -18,6 +18,13 @@ fn example_plugin_path() -> PathBuf { .expect("nu_plugin_example not found") } +fn valid_plugin_item_data() -> PluginRegistryItemData { + PluginRegistryItemData::Valid { + metadata: Default::default(), + commands: vec![], + } +} + #[test] fn plugin_add_then_restart_nu() { let result = nu_with_plugins!( @@ -149,7 +156,7 @@ fn plugin_rm_then_restart_nu() { name: "example".into(), filename: example_plugin_path, shell: None, - data: PluginRegistryItemData::Valid { commands: vec![] }, + data: valid_plugin_item_data(), }); contents.upsert_plugin(PluginRegistryItem { @@ -157,7 +164,7 @@ fn plugin_rm_then_restart_nu() { // this doesn't exist, but it should be ok filename: dirs.test().join("nu_plugin_foo"), shell: None, - data: PluginRegistryItemData::Valid { commands: vec![] }, + data: valid_plugin_item_data(), }); contents @@ -225,7 +232,7 @@ fn plugin_rm_from_custom_path() { name: "example".into(), filename: example_plugin_path, shell: None, - data: PluginRegistryItemData::Valid { commands: vec![] }, + data: valid_plugin_item_data(), }); contents.upsert_plugin(PluginRegistryItem { @@ -233,7 +240,7 @@ fn plugin_rm_from_custom_path() { // this doesn't exist, but it should be ok filename: dirs.test().join("nu_plugin_foo"), shell: None, - data: PluginRegistryItemData::Valid { commands: vec![] }, + data: valid_plugin_item_data(), }); contents @@ -273,7 +280,7 @@ fn plugin_rm_using_filename() { name: "example".into(), filename: example_plugin_path.clone(), shell: None, - data: PluginRegistryItemData::Valid { commands: vec![] }, + data: valid_plugin_item_data(), }); contents.upsert_plugin(PluginRegistryItem { @@ -281,7 +288,7 @@ fn plugin_rm_using_filename() { // this doesn't exist, but it should be ok filename: dirs.test().join("nu_plugin_foo"), shell: None, - data: PluginRegistryItemData::Valid { commands: vec![] }, + data: valid_plugin_item_data(), }); contents @@ -331,7 +338,7 @@ fn warning_on_invalid_plugin_item() { name: "example".into(), filename: example_plugin_path, shell: None, - data: PluginRegistryItemData::Valid { commands: vec![] }, + data: valid_plugin_item_data(), }); contents.upsert_plugin(PluginRegistryItem {