Migrate to a new PWD API (part 2) (#12749)

Refer to #12603 for part 1.

We need to be careful when migrating to the new API, because the new API
has slightly different semantics (PWD can contain symlinks). This PR
handles the "obviously safe" part of the migrations. Namely, it handles
two specific use cases:

* Passing PWD into `canonicalize_with()`
* Passing PWD into `EngineState::merge_env()`

The first case is safe because symlinks are canonicalized away. The
second case is safe because `EngineState::merge_env()` only uses PWD to
call `std::env::set_current_dir()`, which shouldn't affact Nushell. The
commit message contains detailed stats on the updated files.

Because these migrations touch a lot of files, I want to keep these PRs
small to avoid merge conflicts.
This commit is contained in:
YizhePKU 2024-05-07 23:17:49 +08:00 committed by GitHub
parent b9331d1b08
commit 7a86b98f61
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 76 additions and 71 deletions

View file

@ -23,8 +23,7 @@ fn load_bench_commands() -> EngineState {
}
fn canonicalize_path(engine_state: &EngineState, path: &Path) -> PathBuf {
#[allow(deprecated)]
let cwd = engine_state.current_work_dir();
let cwd = engine_state.cwd_as_string(None).unwrap();
if path.exists() {
match nu_path::canonicalize_with(path, cwd) {

View file

@ -177,36 +177,36 @@ pub fn add_plugin_file(
use std::path::Path;
let working_set = StateWorkingSet::new(engine_state);
#[allow(deprecated)]
let cwd = working_set.get_cwd();
if let Some(plugin_file) = plugin_file {
let path = Path::new(&plugin_file.item);
let path_dir = path.parent().unwrap_or(path);
// Just try to canonicalize the directory of the plugin file first.
if let Ok(path_dir) = canonicalize_with(path_dir, &cwd) {
// Try to canonicalize the actual filename, but it's ok if that fails. The file doesn't
// have to exist.
let path = path_dir.join(path.file_name().unwrap_or(path.as_os_str()));
let path = canonicalize_with(&path, &cwd).unwrap_or(path);
engine_state.plugin_path = Some(path)
} else {
// It's an error if the directory for the plugin file doesn't exist.
report_error(
&working_set,
&ParseError::FileNotFound(
path_dir.to_string_lossy().into_owned(),
plugin_file.span,
),
);
if let Ok(cwd) = engine_state.cwd_as_string(None) {
if let Some(plugin_file) = plugin_file {
let path = Path::new(&plugin_file.item);
let path_dir = path.parent().unwrap_or(path);
// Just try to canonicalize the directory of the plugin file first.
if let Ok(path_dir) = canonicalize_with(path_dir, &cwd) {
// Try to canonicalize the actual filename, but it's ok if that fails. The file doesn't
// have to exist.
let path = path_dir.join(path.file_name().unwrap_or(path.as_os_str()));
let path = canonicalize_with(&path, &cwd).unwrap_or(path);
engine_state.plugin_path = Some(path)
} else {
// It's an error if the directory for the plugin file doesn't exist.
report_error(
&working_set,
&ParseError::FileNotFound(
path_dir.to_string_lossy().into_owned(),
plugin_file.span,
),
);
}
} else if let Some(mut plugin_path) = nu_path::config_dir() {
// Path to store plugins signatures
plugin_path.push(storage_path);
let mut plugin_path = canonicalize_with(&plugin_path, &cwd).unwrap_or(plugin_path);
plugin_path.push(PLUGIN_FILE);
let plugin_path = canonicalize_with(&plugin_path, &cwd).unwrap_or(plugin_path);
engine_state.plugin_path = Some(plugin_path);
}
} else if let Some(mut plugin_path) = nu_path::config_dir() {
// Path to store plugins signatures
plugin_path.push(storage_path);
let mut plugin_path = canonicalize_with(&plugin_path, &cwd).unwrap_or(plugin_path);
plugin_path.push(PLUGIN_FILE);
let plugin_path = canonicalize_with(&plugin_path, &cwd).unwrap_or(plugin_path);
engine_state.plugin_path = Some(plugin_path);
}
}
@ -236,8 +236,7 @@ pub fn eval_config_contents(
engine_state.file = prev_file;
// Merge the environment in case env vars changed in the config
#[allow(deprecated)]
match nu_engine::env::current_dir(engine_state, stack) {
match engine_state.cwd(Some(stack)) {
Ok(cwd) => {
if let Err(e) = engine_state.merge_env(stack, cwd) {
let working_set = StateWorkingSet::new(engine_state);
@ -274,8 +273,9 @@ pub fn migrate_old_plugin_file(engine_state: &EngineState, storage_path: &str) -
let start_time = std::time::Instant::now();
#[allow(deprecated)]
let cwd = engine_state.current_work_dir();
let Ok(cwd) = engine_state.cwd_as_string(None) else {
return false;
};
let Some(config_dir) = nu_path::config_dir().and_then(|mut dir| {
dir.push(storage_path);

View file

@ -1,8 +1,7 @@
use crate::util::eval_source;
use log::{info, trace};
use miette::{IntoDiagnostic, Result};
#[allow(deprecated)]
use nu_engine::{convert_env_values, current_dir, eval_block};
use nu_engine::{convert_env_values, eval_block};
use nu_parser::parse;
use nu_path::canonicalize_with;
use nu_protocol::{
@ -30,8 +29,7 @@ pub fn evaluate_file(
std::process::exit(1);
}
#[allow(deprecated)]
let cwd = current_dir(engine_state, stack)?;
let cwd = engine_state.cwd_as_string(Some(stack))?;
let file_path = canonicalize_with(&path, cwd).unwrap_or_else(|e| {
let working_set = StateWorkingSet::new(engine_state);

View file

@ -13,8 +13,7 @@ pub fn get_init_cwd() -> PathBuf {
}
pub fn get_guaranteed_cwd(engine_state: &EngineState, stack: &Stack) -> PathBuf {
#[allow(deprecated)]
nu_engine::env::current_dir(engine_state, stack).unwrap_or_else(|e| {
engine_state.cwd(Some(stack)).unwrap_or_else(|e| {
let working_set = StateWorkingSet::new(engine_state);
report_error(&working_set, &e);
crate::util::get_init_cwd()

View file

@ -1,5 +1,4 @@
#[allow(deprecated)]
use nu_engine::{command_prelude::*, current_dir};
use nu_engine::command_prelude::*;
use nu_plugin_engine::{GetPlugin, PersistentPlugin};
use nu_protocol::{PluginGcConfig, PluginIdentity, PluginRegistryItem, RegisteredPlugin};
use std::sync::Arc;
@ -82,8 +81,7 @@ apparent the next time `nu` is next launched with that plugin registry file.
let filename: Spanned<String> = call.req(engine_state, stack, 0)?;
let shell: Option<Spanned<String>> = call.get_flag(engine_state, stack, "shell")?;
#[allow(deprecated)]
let cwd = current_dir(engine_state, stack)?;
let cwd = engine_state.cwd(Some(stack))?;
// Check the current directory, or fall back to NU_PLUGIN_DIRS
let filename_expanded = nu_path::locate_in_dirs(&filename.item, &cwd, || {

View file

@ -1,5 +1,4 @@
#[allow(deprecated)]
use nu_engine::{command_prelude::*, env::current_dir};
use nu_engine::command_prelude::*;
use std::sync::{atomic::AtomicBool, Arc};
use wax::{Glob as WaxGlob, WalkBehavior, WalkEntry};
@ -179,8 +178,7 @@ impl Command for Glob {
}
};
#[allow(deprecated)]
let path = current_dir(engine_state, stack)?;
let path = engine_state.cwd_as_string(Some(stack))?;
let path = match nu_path::canonicalize_with(prefix, path) {
Ok(path) => path,
Err(e) if e.to_string().contains("os error 2") =>

View file

@ -5,8 +5,7 @@ use notify_debouncer_full::{
EventKind, RecursiveMode, Watcher,
},
};
#[allow(deprecated)]
use nu_engine::{command_prelude::*, current_dir, ClosureEval};
use nu_engine::{command_prelude::*, ClosureEval};
use nu_protocol::{
engine::{Closure, StateWorkingSet},
format_error,
@ -74,8 +73,7 @@ impl Command for Watch {
_input: PipelineData,
) -> Result<PipelineData, ShellError> {
let head = call.head;
#[allow(deprecated)]
let cwd = current_dir(engine_state, stack)?;
let cwd = engine_state.cwd_as_string(Some(stack))?;
let path_arg: Spanned<String> = call.req(engine_state, stack, 0)?;
let path_no_whitespace = &path_arg

View file

@ -286,8 +286,7 @@ pub fn find_in_dirs_env(
Err(e) => return Err(e),
}
} else {
#[allow(deprecated)]
current_dir_str(engine_state, stack)?
engine_state.cwd_as_string(Some(stack))?
};
let check_dir = |lib_dirs: Option<Value>| -> Option<PathBuf> {

View file

@ -984,6 +984,20 @@ impl EngineState {
}
}
/// Like `EngineState::cwd()`, but returns a String instead of a PathBuf for convenience.
pub fn cwd_as_string(&self, stack: Option<&Stack>) -> Result<String, ShellError> {
let cwd = self.cwd(stack)?;
cwd.into_os_string()
.into_string()
.map_err(|err| ShellError::NonUtf8Custom {
msg: format!(
"The current working directory is not a valid utf-8 string: {:?}",
err
),
span: Span::unknown(),
})
}
// TODO: see if we can completely get rid of this
pub fn get_file_contents(&self) -> &[CachedFile] {
&self.files

View file

@ -1,6 +1,5 @@
use log::trace;
#[allow(deprecated)]
use nu_engine::{env::current_dir, eval_block};
use nu_engine::eval_block;
use nu_parser::parse;
use nu_protocol::{
debugger::WithoutDebug,
@ -99,8 +98,7 @@ use std pwd
eval_block::<WithoutDebug>(engine_state, &mut stack, &block, pipeline_data)?;
#[allow(deprecated)]
let cwd = current_dir(engine_state, &stack)?;
let cwd = engine_state.cwd(Some(&stack))?;
engine_state.merge_env(&mut stack, cwd)?;
Ok(())

View file

@ -31,14 +31,19 @@ pub(crate) fn read_config_file(
// Load config startup file
if let Some(file) = config_file {
let working_set = StateWorkingSet::new(engine_state);
#[allow(deprecated)]
let cwd = working_set.get_cwd();
if let Ok(path) = canonicalize_with(&file.item, cwd) {
eval_config_contents(path, engine_state, stack);
} else {
let e = ParseError::FileNotFound(file.item, file.span);
report_error(&working_set, &e);
match engine_state.cwd_as_string(Some(stack)) {
Ok(cwd) => {
if let Ok(path) = canonicalize_with(&file.item, cwd) {
eval_config_contents(path, engine_state, stack);
} else {
let e = ParseError::FileNotFound(file.item, file.span);
report_error(&working_set, &e);
}
}
Err(e) => {
report_error(&working_set, &e);
}
}
} else if let Some(mut config_path) = nu_path::config_dir() {
config_path.push(NUSHELL_FOLDER);
@ -144,8 +149,7 @@ pub(crate) fn read_default_env_file(engine_state: &mut EngineState, stack: &mut
info!("read_config_file {}:{}:{}", file!(), line!(), column!());
// Merge the environment in case env vars changed in the config
#[allow(deprecated)]
match nu_engine::env::current_dir(engine_state, stack) {
match engine_state.cwd(Some(stack)) {
Ok(cwd) => {
if let Err(e) = engine_state.merge_env(stack, cwd) {
let working_set = StateWorkingSet::new(engine_state);
@ -186,8 +190,7 @@ fn eval_default_config(
);
// Merge the environment in case env vars changed in the config
#[allow(deprecated)]
match nu_engine::env::current_dir(engine_state, stack) {
match engine_state.cwd(Some(stack)) {
Ok(cwd) => {
if let Err(e) = engine_state.merge_env(stack, cwd) {
let working_set = StateWorkingSet::new(engine_state);

View file

@ -249,8 +249,9 @@ pub fn nu_repl() {
for (i, line) in source_lines.iter().enumerate() {
let mut stack = Stack::with_parent(top_stack.clone());
#[allow(deprecated)]
let cwd = nu_engine::env::current_dir(&engine_state, &stack)
let cwd = engine_state
.cwd(Some(&stack))
.unwrap_or_else(|err| outcome_err(&engine_state, &err));
// Before doing anything, merge the environment from the previous REPL iteration into the