Fix encodings related error messages to be less confusing (RustPython#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 <fanninpm@miamioh.edu>

Author: youknowone

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") {

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, '')",
+            "<embedded>".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 \

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;

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
     }
 }