Skip to content

Commit

Permalink
Allow plugins to report their own version and store it in the registry (
Browse files Browse the repository at this point in the history
#12883)

# 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
  • Loading branch information
devyn committed Jun 21, 2024
1 parent dd8f886 commit 91d44f1
Show file tree
Hide file tree
Showing 38 changed files with 360 additions and 46 deletions.
5 changes: 4 additions & 1 deletion crates/nu-cli/src/config_files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
});
}

Expand Down
11 changes: 9 additions & 2 deletions crates/nu-cmd-lang/src/core_commands/version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,18 @@ pub fn version(engine_state: &EngineState, span: Span) -> Result<PipelineData, S
Value::string(features_enabled().join(", "), span),
);

// Get a list of plugin names
// Get a list of plugin names and versions if present
let installed_plugins = engine_state
.plugins()
.iter()
.map(|x| x.identity().name())
.map(|x| {
let name = x.identity().name();
if let Some(version) = x.metadata().and_then(|m| m.version) {
format!("{name} {version}")
} else {
name.into()
}
})
.collect::<Vec<_>>();

record.push(
Expand Down
5 changes: 3 additions & 2 deletions crates/nu-cmd-plugin/src/commands/plugin/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
})?;
Expand Down
9 changes: 9 additions & 0 deletions crates/nu-cmd-plugin/src/commands/plugin/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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),
Expand Down
33 changes: 21 additions & 12 deletions crates/nu-parser/src/parse_keywords.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]),
)?;
Expand Down
5 changes: 4 additions & 1 deletion crates/nu-plugin-engine/src/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,14 +252,17 @@ 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
// make sure the running plugin reflects those new signatures, and it's possible that it
// 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());
Expand Down
17 changes: 15 additions & 2 deletions crates/nu-plugin-engine/src/interface/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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<PluginMetadata, ShellError> {
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<Vec<PluginSignature>, ShellError> {
match self.plugin_call(PluginCall::Signature, None)? {
Expand Down Expand Up @@ -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) => {
Expand Down
21 changes: 20 additions & 1 deletion crates/nu-plugin-engine/src/interface/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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();
Expand Down
15 changes: 14 additions & 1 deletion crates/nu-plugin-engine/src/persistent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -31,6 +31,8 @@ pub struct PersistentPlugin {
struct MutableState {
/// Reference to the plugin if running
running: Option<RunningPlugin>,
/// Metadata for the plugin, e.g. version.
metadata: Option<PluginMetadata>,
/// Plugin's preferred communication mode (if known)
preferred_mode: Option<PreferredCommunicationMode>,
/// Garbage collector config
Expand Down Expand Up @@ -59,6 +61,7 @@ impl PersistentPlugin {
identity,
mutable: Mutex::new(MutableState {
running: None,
metadata: None,
preferred_mode: None,
gc_config,
}),
Expand Down Expand Up @@ -268,6 +271,16 @@ impl RegisteredPlugin for PersistentPlugin {
self.stop_internal(true)
}

fn metadata(&self) -> Option<PluginMetadata> {
self.mutable.lock().ok().and_then(|m| m.metadata.clone())
}

fn set_metadata(&self, metadata: Option<PluginMetadata>) {
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
Expand Down
7 changes: 6 additions & 1 deletion crates/nu-plugin-protocol/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<D> {
Metadata,
Signature,
Run(CallInfo<D>),
CustomValueOp(Spanned<PluginCustomValue>, CustomValueOp),
Expand All @@ -132,6 +133,7 @@ impl<D> PluginCall<D> {
f: impl FnOnce(D) -> Result<T, ShellError>,
) -> Result<PluginCall<T>, 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) => {
Expand All @@ -143,6 +145,7 @@ impl<D> PluginCall<D> {
/// The span associated with the call.
pub fn span(&self) -> Option<Span> {
match self {
PluginCall::Metadata => None,
PluginCall::Signature => None,
PluginCall::Run(CallInfo { call, .. }) => Some(call.head),
PluginCall::CustomValueOp(val, _) => Some(val.span),
Expand Down Expand Up @@ -309,6 +312,7 @@ pub enum StreamMessage {
#[derive(Serialize, Deserialize, Debug, Clone)]
pub enum PluginCallResponse<D> {
Error(LabeledError),
Metadata(PluginMetadata),
Signature(Vec<PluginSignature>),
Ordering(Option<Ordering>),
PipelineData(D),
Expand All @@ -323,6 +327,7 @@ impl<D> PluginCallResponse<D> {
) -> Result<PluginCallResponse<T>, 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)?),
Expand Down
8 changes: 7 additions & 1 deletion crates/nu-plugin-test-support/src/fake_persistent_plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -42,6 +42,12 @@ impl RegisteredPlugin for FakePersistentPlugin {
None
}

fn metadata(&self) -> Option<PluginMetadata> {
None
}

fn set_metadata(&self, _metadata: Option<PluginMetadata>) {}

fn set_gc_config(&self, _gc_config: &PluginGcConfig) {
// We don't have a GC
}
Expand Down
4 changes: 4 additions & 0 deletions crates/nu-plugin-test-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@
//! }
//!
//! impl Plugin for LowercasePlugin {
//! fn version(&self) -> String {
//! env!("CARGO_PKG_VERSION").into()
//! }
//!
//! fn commands(&self) -> Vec<Box<dyn PluginCommand<Plugin=Self>>> {
//! vec![Box::new(Lowercase)]
//! }
Expand Down
4 changes: 4 additions & 0 deletions crates/nu-plugin-test-support/tests/custom_value/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ struct IntoU32;
struct IntoIntFromU32;

impl Plugin for CustomU32Plugin {
fn version(&self) -> String {
"0.0.0".into()
}

fn commands(&self) -> Vec<Box<dyn nu_plugin::PluginCommand<Plugin = Self>>> {
vec![Box::new(IntoU32), Box::new(IntoIntFromU32)]
}
Expand Down
4 changes: 4 additions & 0 deletions crates/nu-plugin-test-support/tests/hello/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ struct HelloPlugin;
struct Hello;

impl Plugin for HelloPlugin {
fn version(&self) -> String {
"0.0.0".into()
}

fn commands(&self) -> Vec<Box<dyn PluginCommand<Plugin = Self>>> {
vec![Box::new(Hello)]
}
Expand Down
4 changes: 4 additions & 0 deletions crates/nu-plugin-test-support/tests/lowercase/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ impl PluginCommand for Lowercase {
}

impl Plugin for LowercasePlugin {
fn version(&self) -> String {
"0.0.0".into()
}

fn commands(&self) -> Vec<Box<dyn PluginCommand<Plugin = Self>>> {
vec![Box::new(Lowercase)]
}
Expand Down
4 changes: 4 additions & 0 deletions crates/nu-plugin/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@
//! struct MyCommand;
//!
//! impl Plugin for MyPlugin {
//! fn version(&self) -> String {
//! env!("CARGO_PKG_VERSION").into()
//! }
//!
//! fn commands(&self) -> Vec<Box<dyn PluginCommand<Plugin = Self>>> {
//! vec![Box::new(MyCommand)]
//! }
Expand Down
Loading

0 comments on commit 91d44f1

Please sign in to comment.