Add environment engine calls for plugins (#12166)

# Description

This adds three engine calls: `GetEnvVar`, `GetEnvVars`, for getting
environment variables from the plugin command context, and
`GetCurrentDir` for getting the current working directory.

Plugins are now launched in the directory of their executable to try to
make improper use of the current directory without first setting it more
obvious. Plugins previously launched in whatever the current directory
of the engine was at the time the plugin command was run, but switching
to persistent plugins broke this, because they stay in whatever
directory they launched in initially.

This also fixes the `gstat` plugin to use `get_current_dir()` to
determine its repo location, which was directly affected by this
problem.

# User-Facing Changes
- Adds new engine calls (`GetEnvVar`, `GetEnvVars`, `GetCurrentDir`)
- Runs plugins in a different directory from before, in order to catch
bugs
- Plugins will have to use the new engine calls if they do filesystem
stuff to work properly

# Tests + Formatting
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`

# After Submitting
- [ ] Document the working directory behavior on plugin launch
- [ ] Document the new engine calls + response type (`ValueMap`)
This commit is contained in:
Devyn Cairns 2024-03-12 04:34:32 -07:00 committed by GitHub
parent 37a9f21b2a
commit 390a7e3f0b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 349 additions and 73 deletions

View File

@ -1,10 +1,13 @@
use std::sync::{atomic::AtomicBool, Arc};
use std::{
collections::HashMap,
sync::{atomic::AtomicBool, Arc},
};
use nu_engine::get_eval_block_with_early_return;
use nu_protocol::{
ast::Call,
engine::{Closure, EngineState, Stack},
Config, PipelineData, PluginIdentity, ShellError, Span, Spanned, Value,
Config, IntoSpanned, PipelineData, PluginIdentity, ShellError, Span, Spanned, Value,
};
/// Object safe trait for abstracting operations required of the plugin context.
@ -19,6 +22,12 @@ pub(crate) trait PluginExecutionContext: Send + Sync {
fn get_config(&self) -> Result<Config, ShellError>;
/// Get plugin configuration
fn get_plugin_config(&self) -> Result<Option<Value>, ShellError>;
/// Get an environment variable from `$env`
fn get_env_var(&self, name: &str) -> Result<Option<Value>, ShellError>;
/// Get all environment variables
fn get_env_vars(&self) -> Result<HashMap<String, Value>, ShellError>;
// Get current working directory
fn get_current_dir(&self) -> Result<Spanned<String>, ShellError>;
/// Evaluate a closure passed to the plugin
fn eval_closure(
&self,
@ -110,6 +119,20 @@ impl PluginExecutionContext for PluginExecutionCommandContext {
}))
}
fn get_env_var(&self, name: &str) -> Result<Option<Value>, ShellError> {
Ok(self.stack.get_env_var(&self.engine_state, name))
}
fn get_env_vars(&self) -> Result<HashMap<String, Value>, ShellError> {
Ok(self.stack.get_env_vars(&self.engine_state))
}
fn get_current_dir(&self) -> Result<Spanned<String>, ShellError> {
let cwd = nu_engine::env::current_dir_str(&self.engine_state, &self.stack)?;
// The span is not really used, so just give it call.head
Ok(cwd.into_spanned(self.call.head))
}
fn eval_closure(
&self,
closure: Spanned<Closure>,
@ -190,6 +213,24 @@ impl PluginExecutionContext for PluginExecutionBogusContext {
Ok(None)
}
fn get_env_var(&self, _name: &str) -> Result<Option<Value>, ShellError> {
Err(ShellError::NushellFailed {
msg: "get_env_var not implemented on bogus".into(),
})
}
fn get_env_vars(&self) -> Result<HashMap<String, Value>, ShellError> {
Err(ShellError::NushellFailed {
msg: "get_env_vars not implemented on bogus".into(),
})
}
fn get_current_dir(&self) -> Result<Spanned<String>, ShellError> {
Err(ShellError::NushellFailed {
msg: "get_current_dir not implemented on bogus".into(),
})
}
fn eval_closure(
&self,
_closure: Spanned<Closure>,

View File

@ -1,7 +1,7 @@
//! Interface used by the plugin to communicate with the engine.
use std::{
collections::{btree_map, BTreeMap},
collections::{btree_map, BTreeMap, HashMap},
sync::{mpsc, Arc},
};
@ -300,6 +300,7 @@ impl InterfaceManager for EngineInterfaceManager {
let response = match response {
EngineCallResponse::Error(err) => EngineCallResponse::Error(err),
EngineCallResponse::Config(config) => EngineCallResponse::Config(config),
EngineCallResponse::ValueMap(map) => EngineCallResponse::ValueMap(map),
EngineCallResponse::PipelineData(header) => {
// If there's an error with initializing this stream, change it to an engine
// call error response, but send it anyway
@ -454,6 +455,9 @@ impl EngineInterface {
// These calls have no pipeline data, so they're just the same on both sides
EngineCall::GetConfig => (EngineCall::GetConfig, Default::default()),
EngineCall::GetPluginConfig => (EngineCall::GetPluginConfig, Default::default()),
EngineCall::GetEnvVar(name) => (EngineCall::GetEnvVar(name), Default::default()),
EngineCall::GetEnvVars => (EngineCall::GetEnvVars, Default::default()),
EngineCall::GetCurrentDir => (EngineCall::GetCurrentDir, Default::default()),
};
// Register the channel
@ -495,7 +499,7 @@ impl EngineInterface {
///
/// Format a value in the user's preferred way:
///
/// ```
/// ```rust,no_run
/// # use nu_protocol::{Value, ShellError};
/// # use nu_plugin::EngineInterface;
/// # fn example(engine: &EngineInterface, value: &Value) -> Result<(), ShellError> {
@ -514,6 +518,23 @@ impl EngineInterface {
}
}
/// Do an engine call returning an `Option<Value>` as either `PipelineData::Empty` or
/// `PipelineData::Value`
fn engine_call_option_value(
&self,
engine_call: EngineCall<PipelineData>,
) -> Result<Option<Value>, ShellError> {
let name = engine_call.name();
match self.engine_call(engine_call)? {
EngineCallResponse::PipelineData(PipelineData::Empty) => Ok(None),
EngineCallResponse::PipelineData(PipelineData::Value(value, _)) => Ok(Some(value)),
EngineCallResponse::Error(err) => Err(err),
_ => Err(ShellError::PluginFailedToDecode {
msg: format!("Received unexpected response for EngineCall::{name}"),
}),
}
}
/// Get the plugin-specific configuration from the engine. This lives in
/// `$env.config.plugins.NAME` for a plugin named `NAME`. If the config is set to a closure,
/// it is automatically evaluated each time.
@ -522,7 +543,7 @@ impl EngineInterface {
///
/// Print this plugin's config:
///
/// ```
/// ```rust,no_run
/// # use nu_protocol::{Value, ShellError};
/// # use nu_plugin::EngineInterface;
/// # fn example(engine: &EngineInterface, value: &Value) -> Result<(), ShellError> {
@ -532,12 +553,71 @@ impl EngineInterface {
/// # }
/// ```
pub fn get_plugin_config(&self) -> Result<Option<Value>, ShellError> {
match self.engine_call(EngineCall::GetPluginConfig)? {
EngineCallResponse::PipelineData(PipelineData::Empty) => Ok(None),
EngineCallResponse::PipelineData(PipelineData::Value(value, _)) => Ok(Some(value)),
self.engine_call_option_value(EngineCall::GetPluginConfig)
}
/// Get an environment variable from the engine.
///
/// Returns `Some(value)` if present, and `None` if not found.
///
/// # Example
///
/// Get `$env.PATH`:
///
/// ```rust,no_run
/// # use nu_protocol::{Value, ShellError};
/// # use nu_plugin::EngineInterface;
/// # fn example(engine: &EngineInterface) -> Result<Option<Value>, ShellError> {
/// engine.get_env_var("PATH") // => Ok(Some(Value::List([...])))
/// # }
/// ```
pub fn get_env_var(&self, name: impl Into<String>) -> Result<Option<Value>, ShellError> {
self.engine_call_option_value(EngineCall::GetEnvVar(name.into()))
}
/// Get the current working directory from the engine. The result is always an absolute path.
///
/// # Example
/// ```rust,no_run
/// # use nu_protocol::{Value, ShellError};
/// # use nu_plugin::EngineInterface;
/// # fn example(engine: &EngineInterface) -> Result<String, ShellError> {
/// engine.get_current_dir() // => "/home/user"
/// # }
/// ```
pub fn get_current_dir(&self) -> Result<String, ShellError> {
match self.engine_call(EngineCall::GetCurrentDir)? {
// Always a string, and the span doesn't matter.
EngineCallResponse::PipelineData(PipelineData::Value(Value::String { val, .. }, _)) => {
Ok(val)
}
EngineCallResponse::Error(err) => Err(err),
_ => Err(ShellError::PluginFailedToDecode {
msg: "Received unexpected response for EngineCall::GetConfig".into(),
msg: "Received unexpected response for EngineCall::GetCurrentDir".into(),
}),
}
}
/// Get all environment variables from the engine.
///
/// Since this is quite a large map that has to be sent, prefer to use [`.get_env_var()`] if
/// the variables needed are known ahead of time and there are only a small number needed.
///
/// # Example
/// ```rust,no_run
/// # use nu_protocol::{Value, ShellError};
/// # use nu_plugin::EngineInterface;
/// # use std::collections::HashMap;
/// # fn example(engine: &EngineInterface) -> Result<HashMap<String, Value>, ShellError> {
/// engine.get_env_vars() // => Ok({"PATH": Value::List([...]), ...})
/// # }
/// ```
pub fn get_env_vars(&self) -> Result<HashMap<String, Value>, ShellError> {
match self.engine_call(EngineCall::GetEnvVars)? {
EngineCallResponse::ValueMap(map) => Ok(map),
EngineCallResponse::Error(err) => Err(err),
_ => Err(ShellError::PluginFailedToDecode {
msg: "Received unexpected response type for EngineCall::GetEnvVars".into(),
}),
}
}
@ -559,7 +639,7 @@ impl EngineInterface {
/// my_command { seq 1 $in | each { |n| $"Hello, ($n)" } }
/// ```
///
/// ```
/// ```rust,no_run
/// # use nu_protocol::{Value, ShellError, PipelineData};
/// # use nu_plugin::{EngineInterface, EvaluatedCall};
/// # fn example(engine: &EngineInterface, call: &EvaluatedCall) -> Result<(), ShellError> {
@ -636,7 +716,7 @@ impl EngineInterface {
/// my_command { |number| $number + 1}
/// ```
///
/// ```
/// ```rust,no_run
/// # use nu_protocol::{Value, ShellError};
/// # use nu_plugin::{EngineInterface, EvaluatedCall};
/// # fn example(engine: &EngineInterface, call: &EvaluatedCall) -> Result<(), ShellError> {

View File

@ -1,4 +1,7 @@
use std::sync::mpsc::{self, TryRecvError};
use std::{
collections::HashMap,
sync::mpsc::{self, TryRecvError},
};
use nu_protocol::{
engine::Closure, Config, CustomValue, IntoInterruptiblePipelineData, PipelineData,
@ -886,6 +889,70 @@ fn interface_get_plugin_config() -> Result<(), ShellError> {
Ok(())
}
#[test]
fn interface_get_env_var() -> Result<(), ShellError> {
let test = TestCase::new();
let manager = test.engine();
let interface = manager.interface_for_context(0);
start_fake_plugin_call_responder(manager, 2, |id| {
if id == 0 {
EngineCallResponse::empty()
} else {
EngineCallResponse::value(Value::test_string("/foo"))
}
});
let first_val = interface.get_env_var("FOO")?;
assert!(first_val.is_none(), "should be None: {first_val:?}");
let second_val = interface.get_env_var("FOO")?;
assert_eq!(Some(Value::test_string("/foo")), second_val);
assert!(test.has_unconsumed_write());
Ok(())
}
#[test]
fn interface_get_current_dir() -> Result<(), ShellError> {
let test = TestCase::new();
let manager = test.engine();
let interface = manager.interface_for_context(0);
start_fake_plugin_call_responder(manager, 1, |_| {
EngineCallResponse::value(Value::test_string("/current/directory"))
});
let val = interface.get_env_var("FOO")?;
assert_eq!(Some(Value::test_string("/current/directory")), val);
assert!(test.has_unconsumed_write());
Ok(())
}
#[test]
fn interface_get_env_vars() -> Result<(), ShellError> {
let test = TestCase::new();
let manager = test.engine();
let interface = manager.interface_for_context(0);
let envs: HashMap<String, Value> = [("FOO".to_owned(), Value::test_string("foo"))]
.into_iter()
.collect();
let envs_clone = envs.clone();
start_fake_plugin_call_responder(manager, 1, move |_| {
EngineCallResponse::ValueMap(envs_clone.clone())
});
let received_envs = interface.get_env_vars()?;
assert_eq!(envs, received_envs);
assert!(test.has_unconsumed_write());
Ok(())
}
#[test]
fn interface_eval_closure_with_stream() -> Result<(), ShellError> {
let test = TestCase::new();

View File

@ -490,6 +490,9 @@ impl InterfaceManager for PluginInterfaceManager {
let call = match call {
EngineCall::GetConfig => Ok(EngineCall::GetConfig),
EngineCall::GetPluginConfig => Ok(EngineCall::GetPluginConfig),
EngineCall::GetEnvVar(name) => Ok(EngineCall::GetEnvVar(name)),
EngineCall::GetEnvVars => Ok(EngineCall::GetEnvVars),
EngineCall::GetCurrentDir => Ok(EngineCall::GetCurrentDir),
EngineCall::EvalClosure {
closure,
mut positional,
@ -599,6 +602,7 @@ impl PluginInterface {
// No pipeline data:
EngineCallResponse::Error(err) => (EngineCallResponse::Error(err), None),
EngineCallResponse::Config(config) => (EngineCallResponse::Config(config), None),
EngineCallResponse::ValueMap(map) => (EngineCallResponse::ValueMap(map), None),
};
// Write the response, including the pipeline data header if present
@ -734,6 +738,7 @@ impl PluginInterface {
let (resp, writer) = match resp {
EngineCallResponse::Error(error) => (EngineCallResponse::Error(error), None),
EngineCallResponse::Config(config) => (EngineCallResponse::Config(config), None),
EngineCallResponse::ValueMap(map) => (EngineCallResponse::ValueMap(map), None),
EngineCallResponse::PipelineData(data) => {
match self.init_write_pipeline_data(data) {
Ok((header, writer)) => {
@ -998,6 +1003,23 @@ pub(crate) fn handle_engine_call(
let plugin_config = context.get_plugin_config()?;
Ok(plugin_config.map_or_else(EngineCallResponse::empty, EngineCallResponse::value))
}
EngineCall::GetEnvVar(name) => {
let context = require_context()?;
let value = context.get_env_var(&name)?;
Ok(value.map_or_else(EngineCallResponse::empty, EngineCallResponse::value))
}
EngineCall::GetEnvVars => {
let context = require_context()?;
context.get_env_vars().map(EngineCallResponse::ValueMap)
}
EngineCall::GetCurrentDir => {
let context = require_context()?;
let current_dir = context.get_current_dir()?;
Ok(EngineCallResponse::value(Value::string(
current_dir.item,
current_dir.span,
)))
}
EngineCall::EvalClosure {
closure,
positional,

View File

@ -140,6 +140,12 @@ fn create_command(path: &Path, shell: Option<&Path>) -> CommandSys {
#[cfg(windows)]
process.creation_flags(windows::Win32::System::Threading::CREATE_NEW_PROCESS_GROUP.0);
// In order to make bugs with improper use of filesystem without getting the engine current
// directory more obvious, the plugin always starts in the directory of its executable
if let Some(dirname) = path.parent() {
process.current_dir(dirname);
}
process
}

View File

@ -8,6 +8,8 @@ mod tests;
#[cfg(test)]
pub(crate) mod test_util;
use std::collections::HashMap;
pub use evaluated_call::EvaluatedCall;
use nu_protocol::{
ast::Operator, engine::Closure, Config, PipelineData, PluginSignature, RawStream, ShellError,
@ -439,6 +441,12 @@ pub enum EngineCall<D> {
GetConfig,
/// Get the plugin-specific configuration (`$env.config.plugins.NAME`)
GetPluginConfig,
/// Get an environment variable
GetEnvVar(String),
/// Get all environment variables
GetEnvVars,
/// Get current working directory
GetCurrentDir,
/// Evaluate a closure with stream input/output
EvalClosure {
/// The closure to call.
@ -462,6 +470,9 @@ impl<D> EngineCall<D> {
match self {
EngineCall::GetConfig => "GetConfig",
EngineCall::GetPluginConfig => "GetPluginConfig",
EngineCall::GetEnvVar(_) => "GetEnv",
EngineCall::GetEnvVars => "GetEnvs",
EngineCall::GetCurrentDir => "GetCurrentDir",
EngineCall::EvalClosure { .. } => "EvalClosure",
}
}
@ -474,6 +485,7 @@ pub enum EngineCallResponse<D> {
Error(ShellError),
PipelineData(D),
Config(Box<Config>),
ValueMap(HashMap<String, Value>),
}
impl EngineCallResponse<PipelineData> {

View File

@ -100,6 +100,28 @@ impl Example {
})
}
pub fn env(
&self,
engine: &EngineInterface,
call: &EvaluatedCall,
) -> Result<Value, LabeledError> {
if call.has_flag("cwd")? {
// Get working directory
Ok(Value::string(engine.get_current_dir()?, call.head))
} else if let Some(name) = call.opt::<String>(0)? {
// Get single env var
Ok(engine
.get_env_var(name)?
.unwrap_or(Value::nothing(call.head)))
} else {
// Get all env vars, converting the map to a record
Ok(Value::record(
engine.get_env_vars()?.into_iter().collect(),
call.head,
))
}
}
pub fn disable_gc(
&self,
engine: &EngineInterface,

View File

@ -48,6 +48,18 @@ impl Plugin for Example {
.category(Category::Experimental)
.search_terms(vec!["example".into(), "configuration".into()])
.input_output_type(Type::Nothing, Type::Table(vec![])),
PluginSignature::build("nu-example-env")
.usage("Get environment variable(s)")
.extra_usage("Returns all environment variables if no name provided")
.category(Category::Experimental)
.optional(
"name",
SyntaxShape::String,
"The name of the environment variable to get",
)
.switch("cwd", "Get current working directory instead", None)
.search_terms(vec!["example".into(), "env".into()])
.input_output_type(Type::Nothing, Type::Any),
PluginSignature::build("nu-example-disable-gc")
.usage("Disable the plugin garbage collector for `example`")
.extra_usage(
@ -85,6 +97,7 @@ using `plugin stop example`.",
"nu-example-2" => self.test2(call, input),
"nu-example-3" => self.test3(call, input),
"nu-example-config" => self.config(engine, call),
"nu-example-env" => self.env(engine, call),
"nu-example-disable-gc" => self.disable_gc(engine, call),
_ => Err(LabeledError {
label: "Plugin call with wrong name signature".into(),

View File

@ -1,9 +1,9 @@
use git2::{Branch, BranchType, DescribeOptions, Repository};
use nu_plugin::LabeledError;
use nu_protocol::{record, Span, Spanned, Value};
use nu_protocol::{record, IntoSpanned, Span, Spanned, Value};
use std::fmt::Write;
use std::ops::BitAnd;
use std::path::PathBuf;
use std::path::Path;
// git status
// https://github.com/git/git/blob/9875c515535860450bafd1a177f64f0a478900fa/Documentation/git-status.txt
@ -26,6 +26,7 @@ impl GStat {
pub fn gstat(
&self,
value: &Value,
current_dir: &str,
path: Option<Spanned<String>>,
span: Span,
) -> Result<Value, LabeledError> {
@ -33,89 +34,55 @@ impl GStat {
// eprintln!("input type: {:?} value: {:#?}", &value.type_id(), &value);
// eprintln!("path type: {:?} value: {:#?}", &path.type_id(), &path);
// This is a flag to let us know if we're using the input value (value)
// or using the path specified (path)
let mut using_input_value = false;
// let's get the input value as a string
let piped_value = match value.coerce_string() {
Ok(s) => {
using_input_value = true;
s
// If the path isn't set, get it from input, and failing that, set to "."
let path = match path {
Some(path) => path,
None => {
if !value.is_nothing() {
value.coerce_string()?.into_spanned(value.span())
} else {
String::from(".").into_spanned(span)
}
}
_ => String::new(),
};
// now let's get the path string
let mut a_path = match path {
Some(p) => {
// should we check for input and path? nah.
using_input_value = false;
p
}
None => Spanned {
item: ".".to_string(),
span,
},
};
// If there was no path specified and there is a piped in value, let's use the piped in value
if a_path.item == "." && piped_value.chars().count() > 0 {
a_path.item = piped_value;
}
// Make the path absolute based on the current_dir
let absolute_path = Path::new(current_dir).join(&path.item);
// This path has to exist
// TODO: If the path is relative, it will be expanded using `std::env::current_dir` and not
// the "PWD" environment variable. We would need a way to read the engine's environment
// variables here.
if !std::path::Path::new(&a_path.item).exists() {
if !absolute_path.exists() {
return Err(LabeledError {
label: "error with path".to_string(),
msg: format!("path does not exist [{}]", &a_path.item),
span: if using_input_value {
Some(value.span())
} else {
Some(a_path.span)
},
msg: format!("path does not exist [{}]", absolute_path.display()),
span: Some(path.span),
});
}
let metadata = std::fs::metadata(&a_path.item).map_err(|e| LabeledError {
let metadata = std::fs::metadata(&absolute_path).map_err(|e| LabeledError {
label: "error with metadata".to_string(),
msg: format!(
"unable to get metadata for [{}], error: {}",
&a_path.item, e
absolute_path.display(),
e
),
span: if using_input_value {
Some(value.span())
} else {
Some(a_path.span)
},
span: Some(path.span),
})?;
// This path has to be a directory
if !metadata.is_dir() {
return Err(LabeledError {
label: "error with directory".to_string(),
msg: format!("path is not a directory [{}]", &a_path.item),
span: if using_input_value {
Some(value.span())
} else {
Some(a_path.span)
},
msg: format!("path is not a directory [{}]", absolute_path.display()),
span: Some(path.span),
});
}
let repo_path = match PathBuf::from(&a_path.item).canonicalize() {
let repo_path = match absolute_path.canonicalize() {
Ok(p) => p,
Err(e) => {
return Err(LabeledError {
label: format!("error canonicalizing [{}]", a_path.item),
label: format!("error canonicalizing [{}]", absolute_path.display()),
msg: e.to_string(),
span: if using_input_value {
Some(value.span())
} else {
Some(a_path.span)
},
span: Some(path.span),
});
}
};

View File

@ -13,7 +13,7 @@ impl Plugin for GStat {
fn run(
&self,
name: &str,
_engine: &EngineInterface,
engine: &EngineInterface,
call: &EvaluatedCall,
input: &Value,
) -> Result<Value, LabeledError> {
@ -23,6 +23,7 @@ impl Plugin for GStat {
let repo_path: Option<Spanned<String>> = call.opt(0)?;
// eprintln!("input value: {:#?}", &input);
self.gstat(input, repo_path, call.head)
let current_dir = engine.get_current_dir()?;
self.gstat(input, &current_dir, repo_path, call.head)
}
}

44
tests/plugins/env.rs Normal file
View File

@ -0,0 +1,44 @@
use nu_test_support::nu_with_plugins;
#[test]
fn get_env_by_name() {
let result = nu_with_plugins!(
cwd: ".",
plugin: ("nu_plugin_example"),
r#"
$env.FOO = bar
nu-example-env FOO | print
$env.FOO = baz
nu-example-env FOO | print
"#
);
assert!(result.status.success());
assert_eq!("barbaz", result.out);
}
#[test]
fn get_envs() {
let result = nu_with_plugins!(
cwd: ".",
plugin: ("nu_plugin_example"),
"$env.BAZ = foo; nu-example-env | get BAZ"
);
assert!(result.status.success());
assert_eq!("foo", result.out);
}
#[test]
fn get_current_dir() {
let cwd = std::env::current_dir()
.expect("failed to get current dir")
.join("tests")
.to_string_lossy()
.into_owned();
let result = nu_with_plugins!(
cwd: ".",
plugin: ("nu_plugin_example"),
"cd tests; nu-example-env --cwd"
);
assert!(result.status.success());
assert_eq!(cwd, result.out);
}

View File

@ -1,6 +1,7 @@
mod config;
mod core_inc;
mod custom_values;
mod env;
mod formats;
mod register;
mod stream;