extern/nushell
1
0
Fork 1
mirror of https://github.com/nushell/nushell.git synced 2024-11-22 00:13:21 +01:00
Code Issues Packages Projects Releases Wiki Activity

Remove --encoding argument during register plugin (#6486)

Browse Source 
* first implement new plugin protocol core logic

* fix debug body construct

* fix output message from plugin

* finish plugin commands calling

* fix tests and adjust plugin_custom_value call

* fmt code

* fmt code, fix clippy

* add FIXME comment

* change from FIXME to TODO
This commit is contained in:
WindSoilder 2022-09-07 22:07:42 +08:00 committed by GitHub
parent 80624267fd
commit aa92141ad7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 142 additions and 125 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
crates/nu-command/src/core_commands/register.rs
View File

@ -21,12 +21,6 @@ impl Command for Register {
SyntaxShape::Filepath,
"path of executable for plugin",
)
.required_named(
"encoding",
SyntaxShape::String,
"Encoding used to communicate with plugin. Options: [json, msgpack]",
Some('e'),
)
.optional(
"signature",
SyntaxShape::Any,
@ -64,12 +58,12 @@ impl Command for Register {
vec![
Example {
description: "Register `nu_plugin_query` plugin from ~/.cargo/bin/ dir",
example: r#"register -e json ~/.cargo/bin/nu_plugin_query"#,
example: r#"register ~/.cargo/bin/nu_plugin_query"#,
result: None,
},
Example {
description: "Register `nu_plugin_query` plugin from `nu -c`(plugin will be available in that nu session only)",
example: r#"let plugin = ((which nu).path.0 | path dirname | path join 'nu_plugin_query'); nu -c $'register -e json ($plugin); version'"#,
example: r#"let plugin = ((which nu).path.0 | path dirname | path join 'nu_plugin_query'); nu -c $'register ($plugin); version'"#,
result: None,
},
]

35
crates/nu-parser/src/parse_keywords.rs
View File

@ -3027,7 +3027,7 @@ pub fn parse_register(
spans: &[Span],
expand_aliases_denylist: &[usize],
) -> (Pipeline, Option<ParseError>) {
use nu_plugin::{get_signature, EncodingType, PluginDeclaration};
use nu_plugin::{get_signature, PluginDeclaration};
use nu_protocol::{engine::Stack, Signature};
let cwd = working_set.get_cwd();
@ -3120,22 +3120,7 @@ pub fn parse_register(
}
}
})
.expect("required positional has being checked")
.and_then(|path| {
call.get_flag_expr("encoding")
.map(|expr| {
EncodingType::try_from_bytes(working_set.get_span_contents(expr.span))
.ok_or_else(|| {
ParseError::IncorrectValue(
"wrong encoding".into(),
expr.span,
"Encodings available: json, and msgpack".into(),
)
})
})
.expect("required named has being checked")
.map(|encoding| (path, encoding))
});
.expect("required positional has being checked");
// Signature is an optional value from the call and will be used to decide if
// the plugin is called to get the signatures or to use the given signature
@ -3196,7 +3181,7 @@ pub fn parse_register(
let current_envs =
nu_engine::env::env_to_strings(working_set.permanent_state, &stack).unwrap_or_default();
let error = match signature {
Some(signature) => arguments.and_then(|(path, encoding)| {
Some(signature) => arguments.and_then(|path| {
// restrict plugin file name starts with `nu_plugin_`
let f_name = path
.file_name()
@ -3204,7 +3189,7 @@ pub fn parse_register(
if let Some(true) = f_name {
signature.map(|signature| {
let plugin_decl = PluginDeclaration::new(path, signature, encoding, shell);
let plugin_decl = PluginDeclaration::new(path, signature, shell);
working_set.add_decl(Box::new(plugin_decl));
working_set.mark_plugins_file_dirty();
})
@ -3212,14 +3197,14 @@ pub fn parse_register(
Ok(())
}
}),
None => arguments.and_then(|(path, encoding)| {
None => arguments.and_then(|path| {
// restrict plugin file name starts with `nu_plugin_`
let f_name = path
.file_name()
.map(|s| s.to_string_lossy().starts_with("nu_plugin_"));
if let Some(true) = f_name {
get_signature(path.as_path(), &encoding, &shell, &current_envs)
get_signature(path.as_path(), &shell, &current_envs)
.map_err(|err| {
ParseError::LabeledError(
"Error getting signatures".into(),
@ -3231,12 +3216,8 @@ pub fn parse_register(
for signature in signatures {
// create plugin command declaration (need struct impl Command)
// store declaration in working set
let plugin_decl = PluginDeclaration::new(
path.clone(),
signature,
encoding.clone(),
shell.clone(),
);
let plugin_decl =
PluginDeclaration::new(path.clone(), signature, shell.clone());
working_set.add_decl(Box::new(plugin_decl));
}

50
crates/nu-plugin/src/plugin/declaration.rs
View File

@ -1,6 +1,6 @@
use crate::{EncodingType, EvaluatedCall};
use crate::EvaluatedCall;
use super::{call_plugin, create_command};
use super::{call_plugin, create_command, get_plugin_encoding};
use crate::protocol::{
CallInfo, CallInput, PluginCall, PluginCustomValue, PluginData, PluginResponse,
};
@ -16,21 +16,14 @@ pub struct PluginDeclaration {
signature: Signature,
filename: PathBuf,
shell: Option<PathBuf>,
encoding: EncodingType,
}
impl PluginDeclaration {
pub fn new(
filename: PathBuf,
signature: Signature,
encoding: EncodingType,
shell: Option<PathBuf>,
) -> Self {
pub fn new(filename: PathBuf, signature: Signature, shell: Option<PathBuf>) -> Self {
Self {
name: signature.name.clone(),
signature,
filename,
encoding,
shell,
}
}
@ -111,17 +104,27 @@ impl Command for PluginDeclaration {
input,
});
let response =
call_plugin(&mut child, plugin_call, &self.encoding, call.head).map_err(|err| {
let decl = engine_state.get_decl(call.decl_id);
ShellError::GenericError(
format!("Unable to decode call for {}", decl.name()),
err.to_string(),
Some(call.head),
None,
Vec::new(),
)
});
let encoding = {
let stdout_reader = match &mut child.stdout {
Some(out) => out,
None => {
return Err(ShellError::PluginFailedToLoad(
"Plugin missing stdout reader".into(),
))
}
};
get_plugin_encoding(stdout_reader)?
};
let response = call_plugin(&mut child, plugin_call, &encoding, call.head).map_err(|err| {
let decl = engine_state.get_decl(call.decl_id);
ShellError::GenericError(
format!("Unable to decode call for {}", decl.name()),
err.to_string(),
Some(call.head),
None,
Vec::new(),
)
});
let pipeline_data = match response {
Ok(PluginResponse::Value(value)) => {
@ -134,7 +137,6 @@ impl Command for PluginDeclaration {
data: plugin_data.data,
filename: self.filename.clone(),
shell: self.shell.clone(),
encoding: self.encoding.clone(),
source: engine_state.get_decl(call.decl_id).name().to_owned(),
}),
span: plugin_data.span,
@ -158,7 +160,7 @@ impl Command for PluginDeclaration {
pipeline_data
}
fn is_plugin(&self) -> Option<(&PathBuf, &str, &Option<PathBuf>)> {
Some((&self.filename, self.encoding.to_str(), &self.shell))
fn is_plugin(&self) -> Option<(&PathBuf, &Option<PathBuf>)> {
Some((&self.filename, &self.shell))
}
}

86
crates/nu-plugin/src/plugin/mod.rs
View File

@ -7,9 +7,9 @@ use crate::protocol::{CallInput, LabeledError, PluginCall, PluginData, PluginRes
use crate::EncodingType;
use std::env;
use std::fmt::Write;
use std::io::BufReader;
use std::io::{BufReader, Read, Write as WriteTrait};
use std::path::{Path, PathBuf};
use std::process::{Child, Command as CommandSys, Stdio};
use std::process::{Child, ChildStdout, Command as CommandSys, Stdio};
use nu_protocol::{CustomValue, ShellError, Span};
use nu_protocol::{Signature, Value};
@ -116,7 +116,6 @@ pub(crate) fn call_plugin(
pub fn get_signature(
path: &Path,
encoding: &EncodingType,
shell: &Option<PathBuf>,
current_envs: &HashMap<String, String>,
) -> Result<Vec<Signature>, ShellError> {
@ -127,32 +126,34 @@ pub fn get_signature(
ShellError::PluginFailedToLoad(format!("Error spawning child process: {}", err))
})?;
let mut stdin_writer = child
.stdin
.take()
.ok_or_else(|| ShellError::PluginFailedToLoad("plugin missing stdin writer".into()))?;
let mut stdout_reader = child
.stdout
.take()
.ok_or_else(|| ShellError::PluginFailedToLoad("Plugin missing stdout reader".into()))?;
let encoding = get_plugin_encoding(&mut stdout_reader)?;
// Create message to plugin to indicate that signature is required and
// send call to plugin asking for signature
if let Some(mut stdin_writer) = child.stdin.take() {
let encoding_clone = encoding.clone();
std::thread::spawn(move || {
encoding_clone.encode_call(&PluginCall::Signature, &mut stdin_writer)
});
}
let encoding_clone = encoding.clone();
std::thread::spawn(move || {
encoding_clone.encode_call(&PluginCall::Signature, &mut stdin_writer)
});
// deserialize response from plugin to extract the signature
let signatures = if let Some(stdout_reader) = &mut child.stdout {
let reader = stdout_reader;
let mut buf_read = BufReader::with_capacity(OUTPUT_BUFFER_SIZE, reader);
let response = encoding.decode_response(&mut buf_read)?;
let reader = stdout_reader;
let mut buf_read = BufReader::with_capacity(OUTPUT_BUFFER_SIZE, reader);
let response = encoding.decode_response(&mut buf_read)?;
match response {
PluginResponse::Signature(sign) => Ok(sign),
PluginResponse::Error(err) => Err(err.into()),
_ => Err(ShellError::PluginFailedToLoad(
"Plugin missing signature".into(),
)),
}
} else {
Err(ShellError::PluginFailedToLoad(
"Plugin missing stdout reader".into(),
))
let signatures = match response {
PluginResponse::Signature(sign) => Ok(sign),
PluginResponse::Error(err) => Err(err.into()),
_ => Err(ShellError::PluginFailedToLoad(
"Plugin missing signature".into(),
)),
}?;
match child.wait() {
@ -196,6 +197,24 @@ pub fn serve_plugin(plugin: &mut impl Plugin, encoder: impl PluginEncoder) {
std::process::exit(0)
}
// tell nushell encoding.
//
// 1 byte
// encoding format: | content-length | content |
{
let mut stdout = std::io::stdout();
let encoding = encoder.name();
let length = encoding.len() as u8;
let mut encoding_content: Vec<u8> = encoding.as_bytes().to_vec();
encoding_content.insert(0, length);
stdout
.write_all(&encoding_content)
.expect("Failed to tell nushell my encoding");
stdout
.flush()
.expect("Failed to tell nushell my encoding when flushing stdout");
}
let mut stdin_buf = BufReader::with_capacity(OUTPUT_BUFFER_SIZE, std::io::stdin());
let plugin_call = encoder.decode_call(&mut stdin_buf);
@ -332,3 +351,22 @@ fn print_help(plugin: &mut impl Plugin, encoder: impl PluginEncoder) {
println!("{}", help)
}
pub fn get_plugin_encoding(child_stdout: &mut ChildStdout) -> Result<EncodingType, ShellError> {
let mut length_buf = [0u8; 1];
child_stdout.read_exact(&mut length_buf).map_err(|e| {
ShellError::PluginFailedToLoad(format!("unable to get encoding from plugin: {e}"))
})?;
let mut buf = vec![0u8; length_buf[0] as usize];
child_stdout.read_exact(&mut buf).map_err(|e| {
ShellError::PluginFailedToLoad(format!("unable to get encoding from plugin: {e}"))
})?;
EncodingType::try_from_bytes(&buf).ok_or_else(|| {
let encoding_for_debug = String::from_utf8_lossy(&buf);
ShellError::PluginFailedToLoad(format!(
"get unsupported plugin encoding: {encoding_for_debug}"
))
})
}

20
crates/nu-plugin/src/protocol/plugin_custom_value.rs
View File

@ -3,10 +3,7 @@ use std::path::PathBuf;
use nu_protocol::{CustomValue, ShellError, Value};
use serde::Serialize;
use crate::{
plugin::{call_plugin, create_command},
EncodingType,
};
use crate::plugin::{call_plugin, create_command, get_plugin_encoding};
use super::{PluginCall, PluginData, PluginResponse};
@ -32,8 +29,6 @@ pub struct PluginCustomValue {
#[serde(skip)]
pub shell: Option<PathBuf>,
#[serde(skip)]
pub encoding: EncodingType,
#[serde(skip)]
pub source: String,
}
@ -72,8 +67,19 @@ impl CustomValue for PluginCustomValue {
data: self.data.clone(),
span,
});
let encoding = {
let stdout_reader = match &mut child.stdout {
Some(out) => out,
None => {
return Err(ShellError::PluginFailedToLoad(
"Plugin missing stdout reader".into(),
))
}
};
get_plugin_encoding(stdout_reader)?
};
let response = call_plugin(&mut child, plugin_call, &self.encoding, span).map_err(|err| {
let response = call_plugin(&mut child, plugin_call, &encoding, span).map_err(|err| {
ShellError::GenericError(
format!(
"Unable to decode call for {} to get base value",

2
crates/nu-plugin/src/serializers/json.rs
View File

@ -7,7 +7,7 @@ pub struct JsonSerializer;
impl PluginEncoder for JsonSerializer {
fn name(&self) -> &str {
"Json Serializer"
"json"
}
fn encode_call(

6
crates/nu-plugin/src/serializers/msgpack.rs
View File

@ -6,7 +6,7 @@ pub struct MsgPackSerializer;
impl PluginEncoder for MsgPackSerializer {
fn name(&self) -> &str {
"MsgPack Serializer"
"msgpack"
}
fn encode_call(
@ -23,7 +23,7 @@ impl PluginEncoder for MsgPackSerializer {
reader: &mut impl std::io::BufRead,
) -> Result<crate::protocol::PluginCall, nu_protocol::ShellError> {
rmp_serde::from_read(reader)
.map_err(|err| ShellError::PluginFailedToEncode(err.to_string()))
.map_err(|err| ShellError::PluginFailedToDecode(err.to_string()))
}
fn encode_response(
@ -40,7 +40,7 @@ impl PluginEncoder for MsgPackSerializer {
reader: &mut impl std::io::BufRead,
) -> Result<PluginResponse, ShellError> {
rmp_serde::from_read(reader)
.map_err(|err| ShellError::PluginFailedToEncode(err.to_string()))
.map_err(|err| ShellError::PluginFailedToDecode(err.to_string()))
}
}

5
crates/nu-protocol/src/engine/command.rs
View File

@ -57,9 +57,8 @@ pub trait Command: Send + Sync + CommandClone {
false
}
// Is a plugin command (returns plugin's path, encoding and type of shell
// if the declaration is a plugin)
fn is_plugin(&self) -> Option<(&PathBuf, &str, &Option<PathBuf>)> {
// Is a plugin command (returns plugin's path, type of shell if the declaration is a plugin)
fn is_plugin(&self) -> Option<(&PathBuf, &Option<PathBuf>)> {
None
}

11
crates/nu-protocol/src/engine/engine_state.rs
View File

@ -366,8 +366,7 @@ impl EngineState {
self.plugin_decls().try_for_each(|decl| {
// A successful plugin registration already includes the plugin filename
// No need to check the None option
let (path, encoding, shell) =
decl.is_plugin().expect("plugin should have file name");
let (path, shell) = decl.is_plugin().expect("plugin should have file name");
let mut file_name = path
.to_str()
.expect("path was checked during registration as a str")
@ -394,14 +393,10 @@ impl EngineState {
None => "".into(),
};
// Each signature is stored in the plugin file with the required
// encoding, shell and signature
// Each signature is stored in the plugin file with the shell and signature
// This information will be used when loading the plugin
// information when nushell starts
format!(
"register {} -e {} {} {}\n\n",
file_name, encoding, shell_str, signature
)
format!("register {} {} {}\n\n", file_name, shell_str, signature)
})
.map_err(|err| ShellError::PluginFailedToLoad(err.to_string()))
.and_then(|line| {

12
crates/nu-test-support/src/macros.rs
View File

@ -225,11 +225,11 @@ macro_rules! with_exe {
#[macro_export]
macro_rules! nu_with_plugins {
(cwd: $cwd:expr, plugins: [$(($format:expr, $plugin_name:expr)),+$(,)?], $command:expr) => {{
nu_with_plugins!($cwd, [$(($format, $plugin_name)),+], $command)
(cwd: $cwd:expr, plugins: [$(($plugin_name:expr)),+$(,)?], $command:expr) => {{
nu_with_plugins!($cwd, [$(("", $plugin_name)),+], $command)
}};
(cwd: $cwd:expr, plugin: ($format:expr, $plugin_name:expr), $command:expr) => {{
nu_with_plugins!($cwd, [($format, $plugin_name)], $command)
(cwd: $cwd:expr, plugin: ($plugin_name:expr), $command:expr) => {{
nu_with_plugins!($cwd, [("", $plugin_name)], $command)
}};
($cwd:expr, [$(($format:expr, $plugin_name:expr)),+$(,)?], $command:expr) => {{
@ -254,8 +254,10 @@ macro_rules! nu_with_plugins {
$($crate::commands::ensure_binary_present($plugin_name);)+
// TODO: the `$format` is a dummy empty string, but `plugin_name` is repeatable
// just keep it here for now. Need to find a way to remove it.
let registrations = format!(
concat!($(concat!("register -e ", $format, " {};")),+),
concat!($(concat!("register ", $format, " {};")),+),
$(
nu_path::canonicalize_with(with_exe!($plugin_name), &test_bins)
.unwrap_or_else(|e| {

16
tests/plugins/core_inc.rs
View File

@ -6,7 +6,7 @@ use nu_test_support::playground::Playground;
fn chooses_highest_increment_if_given_more_than_one() {
let actual = nu_with_plugins!(
cwd: "tests/fixtures/formats",
plugin: ("json", "nu_plugin_inc"),
plugin: ("nu_plugin_inc"),
"open cargo_sample.toml | first 1 | inc package.version --major --minor | get package.version"
);
@ -14,7 +14,7 @@ fn chooses_highest_increment_if_given_more_than_one() {
let actual = nu_with_plugins!(
cwd: "tests/fixtures/formats",
plugin: ("json", "nu_plugin_inc"),
plugin: ("nu_plugin_inc"),
// Regardless of order of arguments
"open cargo_sample.toml | first 1 | inc package.version --minor --major | get package.version"
);
@ -35,7 +35,7 @@ fn by_one_with_field_passed() {
let actual = nu_with_plugins!(
cwd: dirs.test(),
plugin: ("json", "nu_plugin_inc"),
plugin: ("nu_plugin_inc"),
"open sample.toml | inc package.edition | get package.edition"
);
@ -56,7 +56,7 @@ fn by_one_with_no_field_passed() {
let actual = nu_with_plugins!(
cwd: dirs.test(),
plugin: ("json", "nu_plugin_inc"),
plugin: ("nu_plugin_inc"),
"open sample.toml | get package.contributors | inc"
);
@ -77,7 +77,7 @@ fn semversion_major_inc() {
let actual = nu_with_plugins!(
cwd: dirs.test(),
plugin: ("json", "nu_plugin_inc"),
plugin: ("nu_plugin_inc"),
"open sample.toml | inc package.version -M | get package.version"
);
@ -98,7 +98,7 @@ fn semversion_minor_inc() {
let actual = nu_with_plugins!(
cwd: dirs.test(),
plugin: ("json", "nu_plugin_inc"),
plugin: ("nu_plugin_inc"),
"open sample.toml | inc package.version --minor | get package.version"
);
@ -119,7 +119,7 @@ fn semversion_patch_inc() {
let actual = nu_with_plugins!(
cwd: dirs.test(),
plugin: ("json", "nu_plugin_inc"),
plugin: ("nu_plugin_inc"),
"open sample.toml | inc package.version --patch | get package.version"
);
@ -140,7 +140,7 @@ fn semversion_without_passing_field() {
let actual = nu_with_plugins!(
cwd: dirs.test(),
plugin: ("json", "nu_plugin_inc"),
plugin: ("nu_plugin_inc"),
"open sample.toml | get package.version | inc --patch"
);

14
tests/plugins/custom_values.rs
View File

@ -4,7 +4,7 @@ use nu_test_support::nu_with_plugins;
fn can_get_custom_value_from_plugin_and_instantly_collapse_it() {
let actual = nu_with_plugins!(
cwd: "tests",
plugin: ("msgpack", "nu_plugin_custom_values"),
plugin: ("nu_plugin_custom_values"),
"custom-value generate"
);
@ -15,7 +15,7 @@ fn can_get_custom_value_from_plugin_and_instantly_collapse_it() {
fn can_get_custom_value_from_plugin_and_pass_it_over() {
let actual = nu_with_plugins!(
cwd: "tests",
plugin: ("msgpack", "nu_plugin_custom_values"),
plugin: ("nu_plugin_custom_values"),
"custom-value generate | custom-value update"
);
@ -29,7 +29,7 @@ fn can_get_custom_value_from_plugin_and_pass_it_over() {
fn can_generate_and_updated_multiple_types_of_custom_values() {
let actual = nu_with_plugins!(
cwd: "tests",
plugin: ("msgpack", "nu_plugin_custom_values"),
plugin: ("nu_plugin_custom_values"),
"custom-value generate2 | custom-value update"
);
@ -43,7 +43,7 @@ fn can_generate_and_updated_multiple_types_of_custom_values() {
fn can_get_describe_plugin_custom_values() {
let actual = nu_with_plugins!(
cwd: "tests",
plugin: ("msgpack", "nu_plugin_custom_values"),
plugin: ("nu_plugin_custom_values"),
"custom-value generate | describe"
);
@ -58,7 +58,7 @@ fn can_get_describe_plugin_custom_values() {
fn fails_if_passing_engine_custom_values_to_plugins() {
let actual = nu_with_plugins!(
cwd: "tests/fixtures/formats",
plugin: ("msgpack", "nu_plugin_custom_values"),
plugin: ("nu_plugin_custom_values"),
"open-db sample.db | custom-value update"
);
@ -72,8 +72,8 @@ fn fails_if_passing_custom_values_across_plugins() {
let actual = nu_with_plugins!(
cwd: "tests",
plugins: [
("msgpack", "nu_plugin_custom_values"),
("json", "nu_plugin_inc")
("nu_plugin_custom_values"),
("nu_plugin_inc")
],
"custom-value generate | inc --major"
);