Deprecate register and add plugin use (#12607)

# Description

Adds a new keyword, `plugin use`. Unlike `register`, this merely loads
the signatures from the plugin cache file. The file is configurable with
the `--plugin-config` option either to `nu` or to `plugin use` itself,
just like the other `plugin` family of commands. At the REPL, one might
do this to replace `register`:

```nushell
> plugin add ~/.cargo/bin/nu_plugin_foo
> plugin use foo
```

This will not work in a script, because `plugin use` is a keyword and
`plugin add` does not evaluate at parse time (intentionally). This means
we no longer run random binaries during parse.

The `--plugins` option has been added to allow running `nu` with certain
plugins in one step. This is used especially for the `nu_with_plugins!`
test macro, but I'd imagine is generally useful. The only weird quirk is
that it has to be a list, and we don't really do this for any of our
other CLI args at the moment.

`register` now prints a deprecation parse warning.

This should fix #11923, as we now have a complete alternative to
`register`.

# User-Facing Changes

- Add `plugin use` command
- Deprecate `register`
- Add `--plugins` option to `nu` to replace a common use of `register`

# Tests + Formatting

I think I've tested it thoroughly enough and every existing test passes.
Testing nu CLI options and alternate config files is a little hairy and
I wish there were some more generic helpers for this, so this will go on
my TODO list for refactoring.

- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`

# After Submitting

- [ ] Update plugins sections of book
- [ ] Release notes
This commit is contained in:
Devyn Cairns
2024-04-23 04:37:50 -07:00
committed by GitHub
parent 5c7f7883c8
commit 1f4131532d
27 changed files with 759 additions and 172 deletions

View File

@ -71,6 +71,7 @@ pub const UNALIASABLE_PARSER_KEYWORDS: &[&[u8]] = &[
b"source",
b"where",
b"register",
b"plugin use",
];
/// Check whether spans start with a parser keyword that can be aliased
@ -93,11 +94,14 @@ pub fn is_unaliasable_parser_keyword(working_set: &StateWorkingSet, spans: &[Spa
/// This is a new more compact method of calling parse_xxx() functions without repeating the
/// parse_call() in each function. Remaining keywords can be moved here.
pub fn parse_keyword(working_set: &mut StateWorkingSet, lite_command: &LiteCommand) -> Pipeline {
let orig_parse_errors_len = working_set.parse_errors.len();
let call_expr = parse_call(working_set, &lite_command.parts, lite_command.parts[0]);
// if err.is_some() {
// return (Pipeline::from_vec(vec![call_expr]), err);
// }
// If an error occurred, don't invoke the keyword-specific functionality
if working_set.parse_errors.len() > orig_parse_errors_len {
return Pipeline::from_vec(vec![call_expr]);
}
if let Expression {
expr: Expr::Call(call),
@ -121,6 +125,8 @@ pub fn parse_keyword(working_set: &mut StateWorkingSet, lite_command: &LiteComma
"overlay hide" => parse_overlay_hide(working_set, call),
"overlay new" => parse_overlay_new(working_set, call),
"overlay use" => parse_overlay_use(working_set, call),
#[cfg(feature = "plugin")]
"plugin use" => parse_plugin_use(working_set, call),
_ => Pipeline::from_vec(vec![call_expr]),
}
} else {
@ -1946,7 +1952,7 @@ pub fn parse_module_file_or_dir(
let cwd = working_set.get_cwd();
let module_path =
if let Some(path) = find_in_dirs(&module_path_str, working_set, &cwd, LIB_DIRS_VAR) {
if let Some(path) = find_in_dirs(&module_path_str, working_set, &cwd, Some(LIB_DIRS_VAR)) {
path
} else {
working_set.error(ParseError::ModuleNotFound(path_span, module_path_str));
@ -3402,7 +3408,7 @@ pub fn parse_source(working_set: &mut StateWorkingSet, lite_command: &LiteComman
}
};
if let Some(path) = find_in_dirs(&filename, working_set, &cwd, LIB_DIRS_VAR) {
if let Some(path) = find_in_dirs(&filename, working_set, &cwd, Some(LIB_DIRS_VAR)) {
if let Some(contents) = path.read(working_set) {
// Add the file to the stack of files being processed.
if let Err(e) = working_set.files.push(path.clone().path_buf(), spans[1]) {
@ -3546,11 +3552,13 @@ pub fn parse_where(working_set: &mut StateWorkingSet, lite_command: &LiteCommand
}
}
/// `register` is deprecated and will be removed in 0.94. Use `plugin add` and `plugin use` instead.
#[cfg(feature = "plugin")]
pub fn parse_register(working_set: &mut StateWorkingSet, lite_command: &LiteCommand) -> Pipeline {
use nu_plugin::{get_signature, PersistentPlugin, PluginDeclaration};
use nu_plugin::{get_signature, PluginDeclaration};
use nu_protocol::{
engine::Stack, ErrSpan, PluginCacheItem, PluginIdentity, PluginSignature, RegisteredPlugin,
engine::Stack, ErrSpan, ParseWarning, PluginCacheItem, PluginIdentity, PluginSignature,
RegisteredPlugin,
};
let spans = &lite_command.parts;
@ -3561,7 +3569,7 @@ pub fn parse_register(working_set: &mut StateWorkingSet, lite_command: &LiteComm
// Maybe this is not necessary but it is a sanity check
if working_set.get_span_contents(spans[0]) != b"register" {
working_set.error(ParseError::UnknownState(
"internal error: Wrong call name for parse plugin function".into(),
"internal error: Wrong call name for 'register' function".into(),
span(spans),
));
return garbage_pipeline(spans);
@ -3609,6 +3617,16 @@ pub fn parse_register(working_set: &mut StateWorkingSet, lite_command: &LiteComm
}
};
// Now that the call is parsed, add the deprecation warning
working_set
.parse_warnings
.push(ParseWarning::DeprecatedWarning {
old_command: "register".into(),
new_suggestion: "use `plugin add` and `plugin use`".into(),
span: call.head,
url: "https://www.nushell.sh/book/plugins.html".into(),
});
// Extracting the required arguments from the call and keeping them together in a tuple
let arguments = call
.positional_nth(0)
@ -3619,7 +3637,8 @@ pub fn parse_register(working_set: &mut StateWorkingSet, lite_command: &LiteComm
.coerce_into_string()
.map_err(|err| err.wrap(working_set, call.head))?;
let Some(path) = find_in_dirs(&filename, working_set, &cwd, PLUGIN_DIRS_VAR) else {
let Some(path) = find_in_dirs(&filename, working_set, &cwd, Some(PLUGIN_DIRS_VAR))
else {
return Err(ParseError::RegisteredFileNotFound(filename, expr.span));
};
@ -3695,27 +3714,8 @@ pub fn parse_register(working_set: &mut StateWorkingSet, lite_command: &LiteComm
// Create the plugin identity. This validates that the plugin name starts with `nu_plugin_`
let identity = PluginIdentity::new(path, shell).err_span(path_span)?;
// Find garbage collection config
let gc_config = working_set
.get_config()
.plugin_gc
.get(identity.name())
.clone();
// Add it to the working set
let plugin = working_set.find_or_create_plugin(&identity, || {
Arc::new(PersistentPlugin::new(identity.clone(), gc_config.clone()))
});
// Downcast the plugin to `PersistentPlugin` - we generally expect this to succeed. The
// trait object only exists so that nu-protocol can contain plugins without knowing anything
// about their implementation, but we only use `PersistentPlugin` in practice.
let plugin: Arc<PersistentPlugin> = plugin.as_any().downcast().map_err(|_| {
ParseError::InternalError(
"encountered unexpected RegisteredPlugin type".into(),
spans[0],
)
})?;
let plugin = nu_plugin::add_plugin_to_working_set(working_set, &identity)
.map_err(|err| err.wrap(working_set, call.head))?;
let signatures = signature.map_or_else(
|| {
@ -3731,8 +3731,6 @@ pub fn parse_register(working_set: &mut StateWorkingSet, lite_command: &LiteComm
)
})?;
plugin.set_gc_config(&gc_config);
let signatures = get_signature(plugin.clone(), get_envs).map_err(|err| {
log::warn!("Error getting signatures: {err:?}");
ParseError::LabeledError(
@ -3776,6 +3774,100 @@ pub fn parse_register(working_set: &mut StateWorkingSet, lite_command: &LiteComm
}])
}
#[cfg(feature = "plugin")]
pub fn parse_plugin_use(working_set: &mut StateWorkingSet, call: Box<Call>) -> Pipeline {
use nu_protocol::{FromValue, PluginCacheFile};
let cwd = working_set.get_cwd();
if let Err(err) = (|| {
let name = call
.positional_nth(0)
.map(|expr| {
eval_constant(working_set, expr)
.and_then(Spanned::<String>::from_value)
.map_err(|err| err.wrap(working_set, call.head))
})
.expect("required positional should have been checked")?;
let plugin_config = call
.named_iter()
.find(|(arg_name, _, _)| arg_name.item == "plugin-config")
.map(|(_, _, expr)| {
let expr = expr
.as_ref()
.expect("--plugin-config arg should have been checked already");
eval_constant(working_set, expr)
.and_then(Spanned::<String>::from_value)
.map_err(|err| err.wrap(working_set, call.head))
})
.transpose()?;
// Find the actual plugin config path location. We don't have a const/env variable for this,
// it either lives in the current working directory or in the script's directory
let plugin_config_path = if let Some(custom_path) = &plugin_config {
find_in_dirs(&custom_path.item, working_set, &cwd, None).ok_or_else(|| {
ParseError::FileNotFound(custom_path.item.clone(), custom_path.span)
})?
} else {
ParserPath::RealPath(
working_set
.permanent_state
.plugin_path
.as_ref()
.ok_or_else(|| ParseError::LabeledErrorWithHelp {
error: "Plugin cache file not set".into(),
label: "can't load plugin without cache file".into(),
span: call.head,
help:
"pass --plugin-config to `plugin use` when $nu.plugin-path is not set"
.into(),
})?
.to_owned(),
)
};
let file = plugin_config_path.open(working_set).map_err(|err| {
ParseError::LabeledError(
"Plugin cache file can't be opened".into(),
err.to_string(),
plugin_config.as_ref().map(|p| p.span).unwrap_or(call.head),
)
})?;
// The file is now open, so we just have to parse the contents and find the plugin
let contents = PluginCacheFile::read_from(file, Some(call.head))
.map_err(|err| err.wrap(working_set, call.head))?;
let plugin_item = contents
.plugins
.iter()
.find(|plugin| plugin.name == name.item)
.ok_or_else(|| ParseError::PluginNotFound {
name: name.item.clone(),
name_span: name.span,
plugin_config_span: plugin_config.as_ref().map(|p| p.span),
})?;
// Now add the signatures to the working set
nu_plugin::load_plugin_cache_item(working_set, plugin_item, Some(call.head))
.map_err(|err| err.wrap(working_set, call.head))?;
Ok(())
})() {
working_set.error(err);
}
let call_span = call.span();
Pipeline::from_vec(vec![Expression {
expr: Expr::Call(call),
span: call_span,
ty: Type::Nothing,
custom_completion: None,
}])
}
pub fn find_dirs_var(working_set: &StateWorkingSet, var_name: &str) -> Option<VarId> {
working_set
.find_variable(format!("${}", var_name).as_bytes())
@ -3799,13 +3891,13 @@ pub fn find_in_dirs(
filename: &str,
working_set: &StateWorkingSet,
cwd: &str,
dirs_var_name: &str,
dirs_var_name: Option<&str>,
) -> Option<ParserPath> {
pub fn find_in_dirs_with_id(
filename: &str,
working_set: &StateWorkingSet,
cwd: &str,
dirs_var_name: &str,
dirs_var_name: Option<&str>,
) -> Option<ParserPath> {
// Choose whether to use file-relative or PWD-relative path
let actual_cwd = working_set
@ -3845,8 +3937,10 @@ pub fn find_in_dirs(
}
// Look up relative path from NU_LIB_DIRS
working_set
.get_variable(find_dirs_var(working_set, dirs_var_name)?)
dirs_var_name
.as_ref()
.and_then(|dirs_var_name| find_dirs_var(working_set, dirs_var_name))
.map(|var_id| working_set.get_variable(var_id))?
.const_val
.as_ref()?
.as_list()
@ -3868,7 +3962,7 @@ pub fn find_in_dirs(
filename: &str,
working_set: &StateWorkingSet,
cwd: &str,
dirs_env: &str,
dirs_env: Option<&str>,
) -> Option<PathBuf> {
// Choose whether to use file-relative or PWD-relative path
let actual_cwd = working_set
@ -3882,7 +3976,9 @@ pub fn find_in_dirs(
let path = Path::new(filename);
if path.is_relative() {
if let Some(lib_dirs) = working_set.get_env_var(dirs_env) {
if let Some(lib_dirs) =
dirs_env.and_then(|dirs_env| working_set.get_env_var(dirs_env))
{
if let Ok(dirs) = lib_dirs.as_list() {
for lib_dir in dirs {
if let Ok(dir) = lib_dir.to_path() {

View File

@ -5149,12 +5149,24 @@ pub fn parse_expression(working_set: &mut StateWorkingSet, spans: &[Span]) -> Ex
#[cfg(feature = "plugin")]
b"register" => {
working_set.error(ParseError::BuiltinCommandInPipeline(
"plugin".into(),
"register".into(),
spans[0],
));
parse_call(working_set, &spans[pos..], spans[0])
}
#[cfg(feature = "plugin")]
b"plugin" => {
if spans.len() > 1 && working_set.get_span_contents(spans[1]) == b"use" {
// only 'plugin use' is banned
working_set.error(ParseError::BuiltinCommandInPipeline(
"plugin use".into(),
spans[0],
));
}
parse_call(working_set, &spans[pos..], spans[0])
}
_ => parse_call(working_set, &spans[pos..], spans[0]),
}
@ -5286,6 +5298,20 @@ pub fn parse_builtin_commands(
b"where" => parse_where(working_set, lite_command),
#[cfg(feature = "plugin")]
b"register" => parse_register(working_set, lite_command),
// Only "plugin use" is a keyword
#[cfg(feature = "plugin")]
b"plugin"
if lite_command
.parts
.get(1)
.is_some_and(|span| working_set.get_span_contents(*span) == b"use") =>
{
if let Some(redirection) = lite_command.redirection.as_ref() {
working_set.error(redirecting_builtin_error("plugin use", redirection));
return garbage_pipeline(&lite_command.parts);
}
parse_keyword(working_set, lite_command)
}
_ => {
let element = parse_pipeline_element(working_set, lite_command);

View File

@ -103,17 +103,33 @@ impl ParserPath {
}
}
pub fn read<'a>(&'a self, working_set: &'a StateWorkingSet) -> Option<Vec<u8>> {
pub fn open<'a>(
&'a self,
working_set: &'a StateWorkingSet,
) -> std::io::Result<Box<dyn std::io::Read + 'a>> {
match self {
ParserPath::RealPath(p) => std::fs::read(p).ok(),
ParserPath::RealPath(p) => {
std::fs::File::open(p).map(|f| Box::new(f) as Box<dyn std::io::Read>)
}
ParserPath::VirtualFile(_, file_id) => working_set
.get_contents_of_file(*file_id)
.map(|bytes| bytes.to_vec()),
.map(|bytes| Box::new(bytes) as Box<dyn std::io::Read>)
.ok_or(std::io::ErrorKind::NotFound.into()),
ParserPath::VirtualDir(..) => None,
ParserPath::VirtualDir(..) => Err(std::io::ErrorKind::NotFound.into()),
}
}
pub fn read<'a>(&'a self, working_set: &'a StateWorkingSet) -> Option<Vec<u8>> {
self.open(working_set)
.and_then(|mut reader| {
let mut vec = vec![];
reader.read_to_end(&mut vec)?;
Ok(vec)
})
.ok()
}
pub fn from_virtual_path(
working_set: &StateWorkingSet,
name: &str,