refactor(shared): centralize directory overrides

Author: fengmk2

crates/vp_installer/src/main.rs

@@ -616,43 +616,12 @@ fn prepare_dirs(opts: &cli::Options) -> Result<VpDirs, Box<dyn std::error::Error
             .ok_or("The installation directory must be an absolute path")?;
         // Safety: called in main() before any threads are spawned (or under
         // EnvConfig::with_vars in tests, which serializes env mutation).
-        unsafe { std::env::set_var("VP_HOME", abs.as_path()) };
+        unsafe { std::env::set_var(vp_shared::env_vars::VP_HOME, abs.as_path()) };
     }
-    validate_dir_env()?;
+    vp_shared::validate_vp_dir_env()?;
     Ok(vp_shared::EnvConfig::get().dirs.clone())
 }
 
-fn validate_dir_env() -> Result<(), Box<dyn std::error::Error>> {
-    use vp_shared::env_vars;
-
-    let env_path = |name| std::env::var_os(name).filter(|value| !value.is_empty());
-    if let Some(home) = env_path(env_vars::VP_HOME)
-        && !std::path::Path::new(&home).is_absolute()
-    {
-        return Err(format!("{} must be an absolute path.", env_vars::VP_HOME).into());
-    }
-
-    let split_dirs = [
-        (env_vars::VP_BIN_DIR, env_path(env_vars::VP_BIN_DIR)),
-        (env_vars::VP_DATA_DIR, env_path(env_vars::VP_DATA_DIR)),
-        (env_vars::VP_CACHE_DIR, env_path(env_vars::VP_CACHE_DIR)),
-    ];
-    let configured_count = split_dirs.iter().filter(|(_, value)| value.is_some()).count();
-    if configured_count != 0 && configured_count != split_dirs.len() {
-        return Err(
-            "Set VP_BIN_DIR, VP_DATA_DIR, and VP_CACHE_DIR together, or leave all three unset."
-                .into(),
-        );
-    }
-    for (name, value) in split_dirs {
-        if value.is_some_and(|path| !std::path::Path::new(&path).is_absolute()) {
-            return Err(format!("{name} must be an absolute path.").into());
-        }
-    }
-
-    Ok(())
-}
-
 #[allow(clippy::print_stdout)]
 fn modify_path(bin_dir: &str, quiet: bool) -> Result<(), Box<dyn std::error::Error>> {
     #[cfg(windows)]
@@ -961,11 +930,9 @@ mod tests {
     }
 
     #[test]
-    fn incomplete_vp_dir_groups_are_rejected_without_creating_roots() {
+    fn invalid_vp_dir_env_is_rejected_without_creating_roots() {
         let tmp = tempfile::tempdir().unwrap();
-        let bin = tmp.path().join("requested-bin");
         let data = tmp.path().join("requested-data");
-        let cache = tmp.path().join("requested-cache");
 
         with_clean_home(tmp.path(), || {
             let default_dirs = EnvConfig::get().dirs.clone();
@@ -977,91 +944,28 @@ mod tests {
                 default_dirs.state.as_path().to_path_buf(),
             ];
             let default_existed = default_roots.each_ref().map(|root| root.exists());
-            let paths = [bin.as_os_str(), data.as_os_str(), cache.as_os_str()];
-            let cases = [
-                [Some(paths[0]), None, None],
-                [None, Some(paths[1]), None],
-                [None, None, Some(paths[2])],
-                [Some(paths[0]), Some(paths[1]), None],
-                [Some(paths[0]), None, Some(paths[2])],
-                [None, Some(paths[1]), Some(paths[2])],
-            ];
-
-            for values in cases {
-                EnvConfig::with_vars(
-                    [
-                        (env_vars::VP_HOME, None),
-                        (env_vars::VP_BIN_DIR, values[0]),
-                        (env_vars::VP_DATA_DIR, values[1]),
-                        (env_vars::VP_CACHE_DIR, values[2]),
-                    ],
-                    |_| {
-                        let error = prepare_dirs(&opts(None)).unwrap_err().to_string();
-                        assert_eq!(
-                            error,
-                            "Set VP_BIN_DIR, VP_DATA_DIR, and VP_CACHE_DIR together, or leave all three unset."
-                        );
-                    },
-                );
-            }
-
-            for requested in [&bin, &data, &cache] {
-                assert!(!requested.exists(), "validation created {}", requested.display());
-            }
-            for (root, existed) in default_roots.iter().zip(default_existed) {
-                if !existed {
-                    assert!(!root.exists(), "validation created default root {}", root.display());
-                }
-            }
-        });
-    }
-
-    #[test]
-    fn relative_directory_overrides_are_rejected() {
-        let tmp = tempfile::tempdir().unwrap();
-        with_clean_home(tmp.path(), || {
             EnvConfig::with_vars(
                 [
-                    (env_vars::VP_HOME, Some(std::ffi::OsStr::new("relative-home"))),
+                    (env_vars::VP_HOME, None),
                     (env_vars::VP_BIN_DIR, None),
-                    (env_vars::VP_DATA_DIR, None),
+                    (env_vars::VP_DATA_DIR, Some(data.as_os_str())),
                     (env_vars::VP_CACHE_DIR, None),
                 ],
                 |_| {
                     let error = prepare_dirs(&opts(None)).unwrap_err().to_string();
-                    assert_eq!(error, "VP_HOME must be an absolute path.");
+                    assert_eq!(
+                        error,
+                        "Set VP_BIN_DIR, VP_DATA_DIR, and VP_CACHE_DIR together, or leave all three unset."
+                    );
                 },
             );
 
-            let bin = tmp.path().join("bin");
-            let data = tmp.path().join("data");
-            let cache = tmp.path().join("cache");
-            let relative = std::ffi::OsStr::new("relative-dir");
-            let cases = [
-                (env_vars::VP_BIN_DIR, [relative, data.as_os_str(), cache.as_os_str()]),
-                (env_vars::VP_DATA_DIR, [bin.as_os_str(), relative, cache.as_os_str()]),
-                (env_vars::VP_CACHE_DIR, [bin.as_os_str(), data.as_os_str(), relative]),
-            ];
-            for (name, values) in cases {
-                EnvConfig::with_vars(
-                    [
-                        (env_vars::VP_HOME, None),
-                        (env_vars::VP_BIN_DIR, Some(values[0])),
-                        (env_vars::VP_DATA_DIR, Some(values[1])),
-                        (env_vars::VP_CACHE_DIR, Some(values[2])),
-                    ],
-                    |_| {
-                        let error = prepare_dirs(&opts(None)).unwrap_err().to_string();
-                        assert_eq!(error, format!("{name} must be an absolute path."));
-                    },
-                );
-            }
-
-            for root in [&bin, &data, &cache] {
-                assert!(!root.exists(), "validation created {}", root.display());
+            assert!(!data.exists(), "validation created {}", data.display());
+            for (root, existed) in default_roots.iter().zip(default_existed) {
+                if !existed {
+                    assert!(!root.exists(), "validation created default root {}", root.display());
+                }
             }
-            assert!(!tmp.path().join("relative-home").exists());
-            assert!(!tmp.path().join("relative-dir").exists());
         });
     }
 

crates/vp_shared/src/dirs.rs

@@ -10,10 +10,17 @@
 //! `<CONFIG>/`, and `<STATE>/` for category roots. They do not use a concrete
 //! path for each layout. See `rfcs/directory-layout.md`.
 
+mod env_overrides;
 mod resolution;
 
+pub use env_overrides::{VpDirEnvError, validate_vp_dir_env};
 use vt_path::{AbsolutePath, AbsolutePathBuf};
 
+/// Absolute `VP_HOME` override from the process environment, if set.
+pub(crate) fn vp_home_override() -> Option<AbsolutePathBuf> {
+    env_overrides::DirEnvOverrides::from_env().home()
+}
+
 /// Platform-specific binary name for the `vp` CLI.
 pub const VP_BINARY_NAME: &str = if cfg!(windows) { "vp.exe" } else { "vp" };
 
@@ -146,8 +153,7 @@ impl VpDirs {
     /// payload cannot report category roots through `VP_DUMP_DIRS`.
     #[must_use]
     pub fn legacy_single_root(home: &AbsolutePath) -> Self {
-        let root = resolution::vp_home_override()
-            .unwrap_or_else(|| home.join(resolution::VP_HOME_DIR_NAME));
+        let root = vp_home_override().unwrap_or_else(|| home.join(resolution::VP_HOME_DIR_NAME));
         resolution::single_root_dirs(root)
     }
 

crates/vp_shared/src/dirs/env_overrides.rs

@@ -0,0 +1,197 @@
+use thiserror::Error;
+use vt_path::AbsolutePathBuf;
+
+use crate::env_vars;
+
+enum PathOverride {
+    Unset,
+    Absolute(AbsolutePathBuf),
+    Relative,
+}
+
+impl PathOverride {
+    fn from_env(name: &str) -> Self {
+        let Some(path) = std::env::var_os(name).filter(|value| !value.is_empty()) else {
+            return Self::Unset;
+        };
+        AbsolutePathBuf::new(path.into()).map_or(Self::Relative, Self::Absolute)
+    }
+
+    fn absolute(&self) -> Option<&AbsolutePathBuf> {
+        match self {
+            Self::Absolute(path) => Some(path),
+            Self::Unset | Self::Relative => None,
+        }
+    }
+
+    fn is_set(&self) -> bool {
+        !matches!(self, Self::Unset)
+    }
+
+    fn is_relative(&self) -> bool {
+        matches!(self, Self::Relative)
+    }
+}
+
+/// One snapshot of the Vite+ directory override variables.
+pub(super) struct DirEnvOverrides {
+    home: PathOverride,
+    bin: PathOverride,
+    data: PathOverride,
+    cache: PathOverride,
+}
+
+impl DirEnvOverrides {
+    pub(super) fn from_env() -> Self {
+        Self {
+            home: PathOverride::from_env(env_vars::VP_HOME),
+            bin: PathOverride::from_env(env_vars::VP_BIN_DIR),
+            data: PathOverride::from_env(env_vars::VP_DATA_DIR),
+            cache: PathOverride::from_env(env_vars::VP_CACHE_DIR),
+        }
+    }
+
+    pub(super) fn home(&self) -> Option<AbsolutePathBuf> {
+        self.home.absolute().cloned()
+    }
+
+    pub(super) fn split_dirs(&self) -> Option<(AbsolutePathBuf, AbsolutePathBuf, AbsolutePathBuf)> {
+        Some((
+            self.bin.absolute()?.clone(),
+            self.data.absolute()?.clone(),
+            self.cache.absolute()?.clone(),
+        ))
+    }
+
+    fn validate(&self) -> Result<(), VpDirEnvError> {
+        if self.home.is_relative() {
+            return Err(VpDirEnvError::RelativePath { name: env_vars::VP_HOME });
+        }
+
+        let split_dirs = [
+            (env_vars::VP_BIN_DIR, &self.bin),
+            (env_vars::VP_DATA_DIR, &self.data),
+            (env_vars::VP_CACHE_DIR, &self.cache),
+        ];
+        let configured_count = split_dirs.iter().filter(|(_, value)| value.is_set()).count();
+        if configured_count != 0 && configured_count != split_dirs.len() {
+            return Err(VpDirEnvError::IncompleteSplitGroup);
+        }
+        for (name, value) in split_dirs {
+            if value.is_relative() {
+                return Err(VpDirEnvError::RelativePath { name });
+            }
+        }
+
+        Ok(())
+    }
+}
+
+/// An invalid Vite+ directory override configuration.
+#[derive(Debug, Error, PartialEq, Eq)]
+pub enum VpDirEnvError {
+    #[error("Set VP_BIN_DIR, VP_DATA_DIR, and VP_CACHE_DIR together, or leave all three unset.")]
+    IncompleteSplitGroup,
+    #[error("{name} must be an absolute path.")]
+    RelativePath { name: &'static str },
+}
+
+/// Validate Vite+ directory overrides for an installer.
+///
+/// Runtime resolution ignores relative or incomplete overrides and continues
+/// to the next source. Installers use this stricter check before they resolve
+/// or create installation roots.
+pub fn validate_vp_dir_env() -> Result<(), VpDirEnvError> {
+    DirEnvOverrides::from_env().validate()
+}
+
+#[cfg(test)]
+mod tests {
+    use std::ffi::OsStr;
+
+    use super::*;
+
+    fn validate_with(
+        home: Option<&OsStr>,
+        [bin, data, cache]: [Option<&OsStr>; 3],
+    ) -> Result<(), VpDirEnvError> {
+        temp_env::with_vars(
+            [
+                (env_vars::VP_HOME, home),
+                (env_vars::VP_BIN_DIR, bin),
+                (env_vars::VP_DATA_DIR, data),
+                (env_vars::VP_CACHE_DIR, cache),
+            ],
+            validate_vp_dir_env,
+        )
+    }
+
+    #[test]
+    fn installer_validation_accepts_valid_overrides() {
+        let root = tempfile::tempdir().unwrap();
+        let bin = root.path().join("bin");
+        let data = root.path().join("data");
+        let cache = root.path().join("cache");
+
+        assert_eq!(validate_with(None, [None, None, None]), Ok(()));
+        assert_eq!(validate_with(Some(root.path().as_os_str()), [None, None, None]), Ok(()));
+        assert_eq!(
+            validate_with(
+                None,
+                [Some(bin.as_os_str()), Some(data.as_os_str()), Some(cache.as_os_str())],
+            ),
+            Ok(())
+        );
+    }
+
+    #[test]
+    fn installer_validation_rejects_incomplete_split_groups() {
+        let root = tempfile::tempdir().unwrap();
+        let paths = [root.path().join("bin"), root.path().join("data"), root.path().join("cache")];
+        let paths = paths.each_ref().map(|path| path.as_os_str());
+        let cases = [
+            [Some(paths[0]), None, None],
+            [None, Some(paths[1]), None],
+            [None, None, Some(paths[2])],
+            [Some(paths[0]), Some(paths[1]), None],
+            [Some(paths[0]), None, Some(paths[2])],
+            [None, Some(paths[1]), Some(paths[2])],
+        ];
+
+        for dirs in cases {
+            assert_eq!(validate_with(None, dirs), Err(VpDirEnvError::IncompleteSplitGroup));
+        }
+    }
+
+    #[test]
+    fn installer_validation_rejects_relative_overrides() {
+        let root = tempfile::tempdir().unwrap();
+        let bin = root.path().join("bin");
+        let data = root.path().join("data");
+        let cache = root.path().join("cache");
+        let relative = OsStr::new("relative-dir");
+
+        assert_eq!(
+            validate_with(Some(relative), [None, None, None]),
+            Err(VpDirEnvError::RelativePath { name: env_vars::VP_HOME })
+        );
+
+        let cases = [
+            (
+                env_vars::VP_BIN_DIR,
+                [Some(relative), Some(data.as_os_str()), Some(cache.as_os_str())],
+            ),
+            (
+                env_vars::VP_DATA_DIR,
+                [Some(bin.as_os_str()), Some(relative), Some(cache.as_os_str())],
+            ),
+            (
+                env_vars::VP_CACHE_DIR,
+                [Some(bin.as_os_str()), Some(data.as_os_str()), Some(relative)],
+            ),
+        ];
+        for (name, dirs) in cases {
+            assert_eq!(validate_with(None, dirs), Err(VpDirEnvError::RelativePath { name }));
+        }
+    }
+}