Make plugin list read state from plugin registry file as well (#14085)

# Description

[Context on
Discord](https://discord.com/channels/601130461678272522/855947301380947968/1292279795035668583)

**This is a breaking change, due to the removal of `is_running`.**

Some users find the `plugin list` command confusing, because it doesn't
show anything different after running `plugin add` or `plugin rm`. This
modifies the `plugin list` command to also look at the plugin registry
file to give some idea of how the plugins in engine state differ from
those in the plugin registry file.

The following values of `status` are now produced instead of
`is_running`:

- `added`: The plugin is present in the plugin registry file, but not in
the engine.
- `loaded`: The plugin is present both in the plugin registry file and
in the engine, but is not running.
- `running`: The plugin is currently running, and the `pid` column
should contain its process ID.
- `modified`: The plugin state present in the plugin registry file is
different from the state in the engine.
- `removed`: The plugin is still loaded in the engine, but is not
present in the plugin registry file.
- `invalid`: The data in the plugin registry file couldn't be
deserialized, and the plugin most likely needs to be added again.

Example (`commands` omitted):

```
╭──────┬─────────────────────┬────────────┬───────────┬──────────┬─────────────────────────────────────────────────────┬─────────╮
│    # │        name         │  version   │  status   │   pid    │                      filename                       │  shell  │
├──────┼─────────────────────┼────────────┼───────────┼──────────┼─────────────────────────────────────────────────────┼─────────┤
│    0 │ custom_values       │ 0.1.0      │ loaded    │          │ /home/devyn/.cargo/bin/nu_plugin_custom_values      │         │
│    1 │ dbus                │ 0.11.0     │ loaded    │          │ /home/devyn/.cargo/bin/nu_plugin_dbus               │         │
│    2 │ example             │ 0.98.1     │ loaded    │          │ /home/devyn/.cargo/bin/nu_plugin_example            │         │
│    3 │ explore_ir          │ 0.3.0      │ loaded    │          │ /home/devyn/.cargo/bin/nu_plugin_explore_ir         │         │
│    4 │ formats             │ 0.98.1     │ loaded    │          │ /home/devyn/.cargo/bin/nu_plugin_formats            │         │
│    5 │ gstat               │ 0.98.1     │ running   │   236662 │ /home/devyn/.cargo/bin/nu_plugin_gstat              │         │
│    6 │ inc                 │ 0.98.1     │ loaded    │          │ /home/devyn/.cargo/bin/nu_plugin_inc                │         │
│    7 │ polars              │ 0.98.1     │ added     │          │ /home/devyn/.cargo/bin/nu_plugin_polars             │         │
│    8 │ query               │ 0.98.1     │ removed   │          │ /home/devyn/.cargo/bin/nu_plugin_query              │         │
│    9 │ stress_internals    │ 0.98.1     │ loaded    │          │ /home/devyn/.cargo/bin/nu_plugin_stress_internals   │         │
╰──────┴─────────────────────┴────────────┴───────────┴──────────┴─────────────────────────────────────────────────────┴─────────╯

```

# User-Facing Changes

To `plugin list`:

* **Breaking:** The `is_running` column is removed and replaced with
`status`. Use `status == running` to filter equivalently.
* The `--plugin-config` from other plugin management commands is now
supported.
* Added an `--engine` flag which behaves more or less like before, and
doesn't load the plugin registry file at all.
* Added a `--registry` flag which only checks the plugin registry file.
All plugins appear as `added` since there is no state to compare with.

Because the default is to check both, the `plugin list` command might be
a little bit slower. If you don't need to check the plugin registry
file, the `--engine` flag does not load the plugin registry file at all,
so it should be just as fast as before.

# Tests + Formatting

Added tests for `added` and `removed` statuses. `modified` and `invalid`
are a bit more tricky so I didn't try.

# After Submitting

- [ ] update documentation that references the `plugin list` command
- [ ] release notes
This commit is contained in:
Devyn Cairns 2024-10-16 19:24:45 -07:00 committed by sholderbach
parent f787d272e6
commit 3af575cce7
6 changed files with 339 additions and 81 deletions

View File

@ -119,7 +119,7 @@ apparent the next time `nu` is next launched with that plugin registry file.
let metadata = interface.get_metadata()?;
let commands = interface.get_signature()?;
modify_plugin_file(engine_state, stack, call.head, custom_path, |contents| {
modify_plugin_file(engine_state, stack, call.head, &custom_path, |contents| {
// Update the file with the received metadata and signatures
let item = PluginRegistryItem::new(plugin.identity(), metadata, commands);
contents.upsert_plugin(item);

View File

@ -1,5 +1,8 @@
use itertools::Itertools;
use itertools::{EitherOrBoth, Itertools};
use nu_engine::command_prelude::*;
use nu_protocol::{IntoValue, PluginRegistryItemData};
use crate::util::read_plugin_file;
#[derive(Clone)]
pub struct PluginList;
@ -17,7 +20,7 @@ impl Command for PluginList {
[
("name".into(), Type::String),
("version".into(), Type::String),
("is_running".into(), Type::Bool),
("status".into(), Type::String),
("pid".into(), Type::Int),
("filename".into(), Type::String),
("shell".into(), Type::String),
@ -26,11 +29,54 @@ impl Command for PluginList {
.into(),
),
)
.named(
"plugin-config",
SyntaxShape::Filepath,
"Use a plugin registry file other than the one set in `$nu.plugin-path`",
None,
)
.switch(
"engine",
"Show info for plugins that are loaded into the engine only.",
Some('e'),
)
.switch(
"registry",
"Show info for plugins from the registry file only.",
Some('r'),
)
.category(Category::Plugin)
}
fn description(&self) -> &str {
"List installed plugins."
"List loaded and installed plugins."
}
fn extra_description(&self) -> &str {
r#"
The `status` column will contain one of the following values:
- `added`: The plugin is present in the plugin registry file, but not in
the engine.
- `loaded`: The plugin is present both in the plugin registry file and in
the engine, but is not running.
- `running`: The plugin is currently running, and the `pid` column should
contain its process ID.
- `modified`: The plugin state present in the plugin registry file is different
from the state in the engine.
- `removed`: The plugin is still loaded in the engine, but is not present in
the plugin registry file.
- `invalid`: The data in the plugin registry file couldn't be deserialized,
and the plugin most likely needs to be added again.
`running` takes priority over any other status. Unless `--registry` is used
or the plugin has not been loaded yet, the values of `version`, `filename`,
`shell`, and `commands` reflect the values in the engine and not the ones in
the plugin registry file.
See also: `plugin use`
"#
.trim()
}
fn search_terms(&self) -> Vec<&str> {
@ -45,7 +91,7 @@ impl Command for PluginList {
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),
"status" => Value::test_string("running"),
"pid" => Value::test_int(106480),
"filename" => if cfg!(windows) {
Value::test_string(r"C:\nu\plugins\nu_plugin_inc.exe")
@ -67,58 +113,189 @@ impl Command for PluginList {
fn run(
&self,
engine_state: &EngineState,
_stack: &mut Stack,
stack: &mut Stack,
call: &Call,
_input: PipelineData,
) -> Result<PipelineData, ShellError> {
let head = call.head;
let custom_path = call.get_flag(engine_state, stack, "plugin-config")?;
let engine_mode = call.has_flag(engine_state, stack, "engine")?;
let registry_mode = call.has_flag(engine_state, stack, "registry")?;
// Group plugin decls by plugin identity
let decls = engine_state.plugin_decls().into_group_map_by(|decl| {
decl.plugin_identity()
.expect("plugin decl should have identity")
});
let plugins_info = match (engine_mode, registry_mode) {
// --engine and --registry together is equivalent to the default.
(false, false) | (true, true) => {
if engine_state.plugin_path.is_some() || custom_path.is_some() {
let plugins_in_engine = get_plugins_in_engine(engine_state);
let plugins_in_registry =
get_plugins_in_registry(engine_state, stack, call.head, &custom_path)?;
merge_plugin_info(plugins_in_engine, plugins_in_registry)
} else {
// Don't produce error when running nu --no-config-file
get_plugins_in_engine(engine_state)
}
}
(true, false) => get_plugins_in_engine(engine_state),
(false, true) => get_plugins_in_registry(engine_state, stack, call.head, &custom_path)?,
};
// Build plugins list
let list = engine_state.plugins().iter().map(|plugin| {
// Find commands that belong to the plugin
let commands = decls.get(plugin.identity())
.into_iter()
.flat_map(|decls| {
decls.iter().map(|decl| Value::string(decl.name(), head))
})
.collect();
let pid = plugin
.pid()
.map(|p| Value::int(p as i64, head))
.unwrap_or(Value::nothing(head));
let shell = plugin
.identity()
.shell()
.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),
"shell" => shell,
"commands" => Value::list(commands, head),
};
Value::record(record, head)
}).collect();
Ok(Value::list(list, head).into_pipeline_data())
Ok(plugins_info.into_value(call.head).into_pipeline_data())
}
}
#[derive(Debug, Clone, IntoValue, PartialOrd, Ord, PartialEq, Eq)]
struct PluginInfo {
name: String,
version: Option<String>,
status: PluginStatus,
pid: Option<u32>,
filename: String,
shell: Option<String>,
commands: Vec<String>,
}
#[derive(Debug, Clone, Copy, IntoValue, PartialOrd, Ord, PartialEq, Eq)]
#[nu_value(rename_all = "snake_case")]
enum PluginStatus {
Added,
Loaded,
Running,
Modified,
Removed,
Invalid,
}
fn get_plugins_in_engine(engine_state: &EngineState) -> Vec<PluginInfo> {
// Group plugin decls by plugin identity
let decls = engine_state.plugin_decls().into_group_map_by(|decl| {
decl.plugin_identity()
.expect("plugin decl should have identity")
});
// Build plugins list
engine_state
.plugins()
.iter()
.map(|plugin| {
// Find commands that belong to the plugin
let commands = decls
.get(plugin.identity())
.into_iter()
.flat_map(|decls| decls.iter().map(|decl| decl.name().to_owned()))
.sorted()
.collect();
PluginInfo {
name: plugin.identity().name().into(),
version: plugin.metadata().and_then(|m| m.version),
status: if plugin.pid().is_some() {
PluginStatus::Running
} else {
PluginStatus::Loaded
},
pid: plugin.pid(),
filename: plugin.identity().filename().to_string_lossy().into_owned(),
shell: plugin
.identity()
.shell()
.map(|path| path.to_string_lossy().into_owned()),
commands,
}
})
.sorted()
.collect()
}
fn get_plugins_in_registry(
engine_state: &EngineState,
stack: &mut Stack,
span: Span,
custom_path: &Option<Spanned<String>>,
) -> Result<Vec<PluginInfo>, ShellError> {
let plugin_file_contents = read_plugin_file(engine_state, stack, span, custom_path)?;
let plugins_info = plugin_file_contents
.plugins
.into_iter()
.map(|plugin| {
let mut info = PluginInfo {
name: plugin.name,
version: None,
status: PluginStatus::Added,
pid: None,
filename: plugin.filename.to_string_lossy().into_owned(),
shell: plugin.shell.map(|path| path.to_string_lossy().into_owned()),
commands: vec![],
};
if let PluginRegistryItemData::Valid { metadata, commands } = plugin.data {
info.version = metadata.version;
info.commands = commands
.into_iter()
.map(|command| command.sig.name)
.sorted()
.collect();
} else {
info.status = PluginStatus::Invalid;
}
info
})
.sorted()
.collect();
Ok(plugins_info)
}
/// If no options are provided, the command loads from both the plugin list in the engine and what's
/// in the registry file. We need to reconcile the two to set the proper states and make sure that
/// new plugins that were added to the plugin registry file show up.
fn merge_plugin_info(
from_engine: Vec<PluginInfo>,
from_registry: Vec<PluginInfo>,
) -> Vec<PluginInfo> {
from_engine
.into_iter()
.merge_join_by(from_registry, |info_a, info_b| {
info_a.name.cmp(&info_b.name)
})
.map(|either_or_both| match either_or_both {
// Exists in the engine, but not in the registry file
EitherOrBoth::Left(info) => PluginInfo {
status: match info.status {
PluginStatus::Running => info.status,
// The plugin is not in the registry file, so it should be marked as `removed`
_ => PluginStatus::Removed,
},
..info
},
// Exists in the registry file, but not in the engine
EitherOrBoth::Right(info) => info,
// Exists in both
EitherOrBoth::Both(info_engine, info_registry) => PluginInfo {
status: match (info_engine.status, info_registry.status) {
// Above all, `running` should be displayed if the plugin is running
(PluginStatus::Running, _) => PluginStatus::Running,
// `invalid` takes precedence over other states because the user probably wants
// to fix it
(_, PluginStatus::Invalid) => PluginStatus::Invalid,
// Display `modified` if the state in the registry is different somehow
_ if info_engine.is_modified(&info_registry) => PluginStatus::Modified,
// Otherwise, `loaded` (it's not running)
_ => PluginStatus::Loaded,
},
..info_engine
},
})
.sorted()
.collect()
}
impl PluginInfo {
/// True if the plugin info shows some kind of change (other than status/pid) relative to the
/// other
fn is_modified(&self, other: &PluginInfo) -> bool {
self.name != other.name
|| self.filename != other.filename
|| self.shell != other.shell
|| self.commands != other.commands
}
}

View File

@ -87,7 +87,7 @@ fixed with `plugin add`.
let filename = canonicalize_possible_filename_arg(engine_state, stack, &name.item);
modify_plugin_file(engine_state, stack, call.head, custom_path, |contents| {
modify_plugin_file(engine_state, stack, call.head, &custom_path, |contents| {
if let Some(index) = contents
.plugins
.iter()

View File

@ -6,18 +6,17 @@ use std::{
path::PathBuf,
};
pub(crate) fn modify_plugin_file(
fn get_plugin_registry_file_path(
engine_state: &EngineState,
stack: &mut Stack,
span: Span,
custom_path: Option<Spanned<String>>,
operate: impl FnOnce(&mut PluginRegistryFile) -> Result<(), ShellError>,
) -> Result<(), ShellError> {
custom_path: &Option<Spanned<String>>,
) -> Result<PathBuf, ShellError> {
#[allow(deprecated)]
let cwd = current_dir(engine_state, stack)?;
let plugin_registry_file_path = if let Some(ref custom_path) = custom_path {
nu_path::expand_path_with(&custom_path.item, cwd, true)
if let Some(ref custom_path) = custom_path {
Ok(nu_path::expand_path_with(&custom_path.item, cwd, true))
} else {
engine_state
.plugin_path
@ -28,8 +27,53 @@ pub(crate) fn modify_plugin_file(
span: Some(span),
help: Some("you may be running `nu` with --no-config-file".into()),
inner: vec![],
})?
};
})
}
}
pub(crate) fn read_plugin_file(
engine_state: &EngineState,
stack: &mut Stack,
span: Span,
custom_path: &Option<Spanned<String>>,
) -> Result<PluginRegistryFile, ShellError> {
let plugin_registry_file_path =
get_plugin_registry_file_path(engine_state, stack, span, custom_path)?;
let file_span = custom_path.as_ref().map(|p| p.span).unwrap_or(span);
// Try to read the plugin file if it exists
if fs::metadata(&plugin_registry_file_path).is_ok_and(|m| m.len() > 0) {
PluginRegistryFile::read_from(
File::open(&plugin_registry_file_path).map_err(|err| ShellError::IOErrorSpanned {
msg: format!(
"failed to read `{}`: {}",
plugin_registry_file_path.display(),
err
),
span: file_span,
})?,
Some(file_span),
)
} else if let Some(path) = custom_path {
Err(ShellError::FileNotFound {
file: path.item.clone(),
span: path.span,
})
} else {
Ok(PluginRegistryFile::default())
}
}
pub(crate) fn modify_plugin_file(
engine_state: &EngineState,
stack: &mut Stack,
span: Span,
custom_path: &Option<Spanned<String>>,
operate: impl FnOnce(&mut PluginRegistryFile) -> Result<(), ShellError>,
) -> Result<(), ShellError> {
let plugin_registry_file_path =
get_plugin_registry_file_path(engine_state, stack, span, custom_path)?;
let file_span = custom_path.as_ref().map(|p| p.span).unwrap_or(span);

View File

@ -12,7 +12,7 @@ fn plugin_list_shows_installed_plugins() {
plugins: [("nu_plugin_inc"), ("nu_plugin_custom_values")],
r#"(plugin list).name | str join ','"#
);
assert_eq!("inc,custom_values", out.out);
assert_eq!("custom_values,inc", out.out);
assert!(out.status.success());
}
@ -34,15 +34,15 @@ fn plugin_keeps_running_after_calling_it() {
plugin: ("nu_plugin_inc"),
r#"
plugin stop inc
(plugin list).0.is_running | print
(plugin list).0.status == running | print
print ";"
"2.0.0" | inc -m | ignore
(plugin list).0.is_running | print
(plugin list).0.status == running | print
"#
);
assert_eq!(
"false;true", out.out,
"plugin list didn't show is_running = true"
"plugin list didn't show status = running"
);
assert!(out.status.success());
}
@ -244,7 +244,7 @@ fn plugin_gc_can_be_configured_to_stop_plugins_immediately() {
$env.config.plugin_gc = { default: { stop_after: 0sec } }
"2.3.0" | inc -M
sleep 100ms
(plugin list | where name == inc).0.is_running
(plugin list | where name == inc).0.status == running
"#
);
assert!(out.status.success());
@ -261,7 +261,7 @@ fn plugin_gc_can_be_configured_to_stop_plugins_immediately() {
}
"2.3.0" | inc -M
sleep 100ms
(plugin list | where name == inc).0.is_running
(plugin list | where name == inc).0.status == running
"#
);
assert!(out.status.success());
@ -281,7 +281,7 @@ fn plugin_gc_can_be_configured_to_stop_plugins_after_delay() {
while $cond {
sleep 100ms
$cond = (
(plugin list | where name == inc).0.is_running and
(plugin list | where name == inc).0.status == running and
((date now) - $start) < 5sec
)
}
@ -310,7 +310,7 @@ fn plugin_gc_can_be_configured_to_stop_plugins_after_delay() {
while $cond {
sleep 100ms
$cond = (
(plugin list | where name == inc).0.is_running and
(plugin list | where name == inc).0.status == running and
((date now) - $start) < 5sec
)
}
@ -333,7 +333,7 @@ fn plugin_gc_can_be_configured_as_disabled() {
r#"
$env.config.plugin_gc = { default: { enabled: false, stop_after: 0sec } }
"2.3.0" | inc -M
(plugin list | where name == inc).0.is_running
(plugin list | where name == inc).0.status == running
"#
);
assert!(out.status.success());
@ -350,7 +350,7 @@ fn plugin_gc_can_be_configured_as_disabled() {
}
}
"2.3.0" | inc -M
(plugin list | where name == inc).0.is_running
(plugin list | where name == inc).0.status == running
"#
);
assert!(out.status.success());
@ -367,7 +367,7 @@ fn plugin_gc_can_be_disabled_by_plugin() {
$env.config.plugin_gc = { default: { stop_after: 0sec } }
example one 1 foo | ignore # ensure we've run the plugin with the new config
sleep 100ms
(plugin list | where name == example).0.is_running
(plugin list | where name == example).0.status == running
"#
);
assert!(out.status.success());

View File

@ -37,7 +37,7 @@ fn plugin_add_then_restart_nu() {
--config $nu.config-path
--env-config $nu.env-path
--plugin-config $nu.plugin-path
--commands 'plugin list | get name | to json --raw'
--commands 'plugin list --engine | get name | to json --raw'
)
", example_plugin_path().display())
);
@ -69,7 +69,7 @@ fn plugin_add_in_nu_plugin_dirs_const() {
--config $nu.config-path
--env-config $nu.env-path
--plugin-config $nu.plugin-path
--commands 'plugin list | get name | to json --raw'
--commands 'plugin list --engine | get name | to json --raw'
)
"#,
dirname.display(),
@ -103,7 +103,7 @@ fn plugin_add_in_nu_plugin_dirs_env() {
--config $nu.config-path
--env-config $nu.env-path
--plugin-config $nu.plugin-path
--commands 'plugin list | get name | to json --raw'
--commands 'plugin list --engine | get name | to json --raw'
)
"#,
dirname.display(),
@ -199,7 +199,7 @@ fn plugin_rm_then_restart_nu() {
"--plugin-config",
"test-plugin-file.msgpackz",
"--commands",
"plugin list | get name | to json --raw",
"plugin list --engine | get name | to json --raw",
])
.assert()
.success()
@ -364,7 +364,7 @@ fn warning_on_invalid_plugin_item() {
"--plugin-config",
"test-plugin-file.msgpackz",
"--commands",
"plugin list | get name | to json --raw",
"plugin list --engine | get name | to json --raw",
])
.output()
.expect("failed to run nu");
@ -412,6 +412,43 @@ fn plugin_use_error_not_found() {
})
}
#[test]
fn plugin_shows_up_in_default_plugin_list_after_add() {
let example_plugin_path = example_plugin_path();
let result = nu_with_plugins!(
cwd: ".",
plugins: [],
&format!(r#"
plugin add '{}'
plugin list | get status | to json --raw
"#, example_plugin_path.display())
);
assert!(result.status.success());
assert_eq!(r#"["added"]"#, result.out);
}
#[test]
fn plugin_shows_removed_after_removing() {
let example_plugin_path = example_plugin_path();
let result = nu_with_plugins!(
cwd: ".",
plugins: [],
&format!(r#"
plugin add '{}'
plugin list | get status | to json --raw
(
^$nu.current-exe
--config $nu.config-path
--env-config $nu.env-path
--plugin-config $nu.plugin-path
--commands 'plugin rm example; plugin list | get status | to json --raw'
)
"#, example_plugin_path.display())
);
assert!(result.status.success());
assert_eq!(r#"["removed"]"#, result.out);
}
#[test]
fn plugin_add_and_then_use() {
let example_plugin_path = example_plugin_path();
@ -425,7 +462,7 @@ fn plugin_add_and_then_use() {
--config $nu.config-path
--env-config $nu.env-path
--plugin-config $nu.plugin-path
--commands 'plugin use example; plugin list | get name | to json --raw'
--commands 'plugin use example; plugin list --engine | get name | to json --raw'
)
"#, example_plugin_path.display())
);
@ -446,7 +483,7 @@ fn plugin_add_and_then_use_by_filename() {
--config $nu.config-path
--env-config $nu.env-path
--plugin-config $nu.plugin-path
--commands 'plugin use '{0}'; plugin list | get name | to json --raw'
--commands 'plugin use '{0}'; plugin list --engine | get name | to json --raw'
)
"#, example_plugin_path.display())
);
@ -471,7 +508,7 @@ fn plugin_add_then_use_with_custom_path() {
cwd: dirs.test(),
r#"
plugin use --plugin-config test-plugin-file.msgpackz example
plugin list | get name | to json --raw
plugin list --engine | get name | to json --raw
"#
);