From 64c66e00d68c70fed261f8b46720438efda746b5 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" <69878+youknowone@users.noreply.github.com> Date: Wed, 30 Aug 2023 21:45:36 +0900 Subject: [PATCH] Fix `encodings` related error messages to be less confusing (#5049) * Move cwd setup to interpreter code * rework import encodings failure message * try import_encodings only when `path_list` is set * std::mem::take instead of drain(..).collect() * Add empty path_list warnings to import_encodings * Prepend current working directory when !safe_path Co-authored-by: fanninpm --- src/interpreter.rs | 3 +-- src/lib.rs | 11 +++++++++- vm/src/vm/mod.rs | 51 +++++++++++++++++++++++++++++++------------- vm/src/vm/setting.rs | 7 +++--- 4 files changed, 50 insertions(+), 22 deletions(-) diff --git a/src/interpreter.rs b/src/interpreter.rs index d2ec119b4..b84f167ae 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -84,8 +84,7 @@ pub fn init_stdlib(vm: &mut VirtualMachine) { let state = PyRc::get_mut(&mut vm.state).unwrap(); let settings = &mut state.settings; - #[allow(clippy::needless_collect)] // false positive - let path_list: Vec<_> = settings.path_list.drain(..).collect(); + let path_list = std::mem::take(&mut settings.path_list); // BUILDTIME_RUSTPYTHONPATH should be set when distributing if let Some(paths) = option_env!("BUILDTIME_RUSTPYTHONPATH") { diff --git a/src/lib.rs b/src/lib.rs index c9ff15354..f916dd682 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -165,8 +165,17 @@ fn run_rustpython(vm: &VirtualMachine, run_mode: RunMode, quiet: bool) -> PyResu let scope = setup_main_module(vm)?; - let site_result = vm.import("site", None, 0); + if !vm.state.settings.safe_path { + // TODO: The prepending path depends on running mode + // See https://docs.python.org/3/using/cmdline.html#cmdoption-P + vm.run_code_string( + vm.new_scope_with_builtins(), + "import sys; sys.path.insert(0, '')", + "".to_owned(), + )?; + } + let site_result = vm.import("site", None, 0); if site_result.is_err() { warn!( "Failed to import site, consider adding the Lib directory to your RUSTPYTHONPATH \ diff --git a/vm/src/vm/mod.rs b/vm/src/vm/mod.rs index fa2dfb907..e943ab7fa 100644 --- a/vm/src/vm/mod.rs +++ b/vm/src/vm/mod.rs @@ -223,18 +223,30 @@ impl VirtualMachine { #[cfg(feature = "encodings")] fn import_encodings(&mut self) -> PyResult<()> { self.import("encodings", None, 0).map_err(|import_err| { - let msg = if !self.state.settings.path_list.iter().any(|s| s == "PYTHONPATH" || s == "RUSTPYTHONPATH"){ - "Could not import encodings. Is your RUSTPYTHONPATH or PYTHONPATH set? If you don't have \ - access to a consistent external environment (e.g. if you're embedding \ - rustpython in another application), try enabling the freeze-stdlib feature" - .to_owned() + let rustpythonpath_env = std::env::var("RUSTPYTHONPATH").ok(); + let pythonpath_env = std::env::var("PYTHONPATH").ok(); + let env_set = rustpythonpath_env.as_ref().is_some() || pythonpath_env.as_ref().is_some(); + let path_contains_env = self.state.settings.path_list.iter().any(|s| { + Some(s.as_str()) == rustpythonpath_env.as_deref() || Some(s.as_str()) == pythonpath_env.as_deref() + }); + + let guide_message = if !env_set { + "Neither RUSTPYTHONPATH nor PYTHONPATH is set. Try setting one of them to the stdlib directory." + } else if path_contains_env { + "RUSTPYTHONPATH or PYTHONPATH is set, but it doesn't contain the encodings library. If you are customizing the RustPython vm/interpreter, try adding the stdlib directory to the path. If you are developing the RustPython interpreter, it might be a bug during development." } else { - "Could not import encodings. Try adding your path to Setting struct's path_list field. If you don't have \ - access to a consistent external environment (e.g. if you're embedding \ - rustpython in another application), try enabling the freeze-stdlib feature" - .to_owned() + "RUSTPYTHONPATH or PYTHONPATH is set, but it wasn't loaded to `Settings::path_list`. If you are going to customize the RustPython vm/interpreter, those environment variables are not loaded in the Settings struct by default. Please try creating a customized instance of the Settings struct. If you are developing the RustPython interpreter, it might be a bug during development." }; + let msg = format!( + "RustPython could not import the encodings module. It usually means something went wrong. Please carefully read the following messages and follow the steps.\n\ + \n\ + {guide_message}\n\ + If you don't have access to a consistent external environment (e.g. targeting wasm, embedding \ + rustpython in another application), try enabling the `freeze-stdlib` feature.\n\ + If this is intended and you want to exclude the encodings module from your interpreter, please remove the `encodings` feature from `rustpython-vm` crate." + ); + let err = self.new_runtime_error(msg); err.set_cause(Some(import_err)); err @@ -267,9 +279,6 @@ impl VirtualMachine { panic!("Double Initialize Error"); } - // add the current directory to sys.path - self.state_mut().settings.path_list.insert(0, "".to_owned()); - stdlib::builtins::init_module(self, &self.builtins); stdlib::sys::init_module(self, &self.sys_module, &self.builtins); @@ -327,9 +336,21 @@ impl VirtualMachine { } #[cfg(feature = "encodings")] - if let Err(e) = self.import_encodings() { - eprintln!("encodings initialization failed. Only utf-8 encoding will be supported."); - self.print_exception(e); + if cfg!(feature = "freeze-stdlib") || !self.state.settings.path_list.is_empty() { + if let Err(e) = self.import_encodings() { + eprintln!( + "encodings initialization failed. Only utf-8 encoding will be supported." + ); + self.print_exception(e); + } + } else { + // Here may not be the best place to give general `path_list` advice, + // but bare rustpython_vm::VirtualMachine users skipped proper settings must hit here while properly setup vm never enters here. + eprintln!( + "feature `encodings` is enabled but `settings.path_list` is empty. \ + Please add the library path to `settings.path_list`. If you intended to disable the entire standard library (including the `encodings` feature), please also make sure to disable the `encodings` feature.\n\ + Tip: You may also want to add `\"\"` to `settings.path_list` in order to enable importing from the current working directory." + ); } self.initialized = true; diff --git a/vm/src/vm/setting.rs b/vm/src/vm/setting.rs index c4d1f85b2..a30c75560 100644 --- a/vm/src/vm/setting.rs +++ b/vm/src/vm/setting.rs @@ -89,10 +89,9 @@ pub struct Settings { } impl Settings { - pub fn with_path(path: String) -> Self { - let mut settings = Self::default(); - settings.path_list.push(path); - settings + pub fn with_path(mut self, path: String) -> Self { + self.path_list.push(path); + self } }