From b3d202bb6f9fb29a4f13eae54a4fd31e1291e172 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 21 Sep 2026 15:25:53 -0700 Subject: [PATCH 1/9] Require unicode paths --- Cargo.lock | 3 + Cargo.toml | 2 +- src/analyzer.rs | 16 ++-- src/arguments.rs | 15 ++- src/ast.rs | 2 +- src/cache.rs | 10 +- src/cache_key.rs | 2 +- src/cache_lock.rs | 2 +- src/clean.rs | 10 +- src/command_ext.rs | 14 +-- src/compilation.rs | 4 +- src/compiler.rs | 34 ++++--- src/config.rs | 36 ++------ src/config_error.rs | 4 +- src/dir.rs | 38 ++++++++ src/error.rs | 126 ++++++++++++------------- src/execution_context.rs | 4 +- src/executor.rs | 4 +- src/filesystem.rs | 4 +- src/function.rs | 188 ++++++++++++++------------------------ src/item.rs | 4 +- src/justfile.rs | 6 +- src/lexer.rs | 6 +- src/lib.rs | 22 ++++- src/load_dotenv.rs | 6 +- src/loader.rs | 11 +-- src/parser.rs | 6 +- src/path_error.rs | 20 ++++ src/platform/unix.rs | 17 ++-- src/platform/windows.rs | 12 ++- src/platform_interface.rs | 12 ++- src/recipe.rs | 8 +- src/scope.rs | 2 +- src/search.rs | 79 ++++++++-------- src/search_config.rs | 12 ++- src/search_error.rs | 26 +++--- src/settings.rs | 2 +- src/shell_kind.rs | 2 +- src/source.rs | 12 +-- src/subcommand.rs | 42 +++++---- src/token.rs | 4 +- src/which.rs | 19 ++-- 42 files changed, 431 insertions(+), 417 deletions(-) create mode 100644 src/dir.rs create mode 100644 src/path_error.rs diff --git a/Cargo.lock b/Cargo.lock index 8ce292f408..14af529a16 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -168,6 +168,9 @@ name = "camino" version = "1.2.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bb1307f12aa967b5a58416e87b3653360e0fd614a016b6e970db08fecbb1b80d" +dependencies = [ + "serde_core", +] [[package]] name = "cc" diff --git a/Cargo.toml b/Cargo.toml index f32bab92a4..194fd2b95b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,7 +19,7 @@ members = [".", "crates/*"] [dependencies] blake3 = { version = "1.5.0", features = ["mmap", "rayon", "serde"] } -camino = "1.0.4" +camino = { version = "1.0.4", features = ["serde1"] } chrono = "0.4.38" clap = { version = "4.0.0", features = ["derive", "env", "string", "wrap_help"] } clap_complete = { version = "=4.6.8", features = ["unstable-dynamic"] } diff --git a/src/analyzer.rs b/src/analyzer.rs index 05ee83cea1..acd8de99d3 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -14,17 +14,17 @@ pub(crate) struct Analyzer<'run, 'src> { impl<'run, 'src> Analyzer<'run, 'src> { pub(crate) fn analyze( - asts: &'run HashMap<(Modulepath, PathBuf), Ast<'src>>, + asts: &'run HashMap<(Modulepath, Utf8PathBuf), Ast<'src>>, config: &Config, doc: Option, groups: &[StringLiteral<'src>], - loaded: &[PathBuf], + loaded: &[Utf8PathBuf], module_path: &Modulepath, name: Option>, overrides: &mut HashMap, - paths: &HashMap, + paths: &HashMap, private: bool, - root: &Path, + root: &Utf8Path, ) -> CompileResult<'src, Justfile<'src>> { Self::default().justfile( asts, @@ -43,17 +43,17 @@ impl<'run, 'src> Analyzer<'run, 'src> { fn justfile( mut self, - asts: &'run HashMap<(Modulepath, PathBuf), Ast<'src>>, + asts: &'run HashMap<(Modulepath, Utf8PathBuf), Ast<'src>>, config: &Config, doc: Option, groups: &[StringLiteral<'src>], - loaded: &[PathBuf], + loaded: &[Utf8PathBuf], module_path: &Modulepath, name: Option>, overrides: &mut HashMap, - paths: &HashMap, + paths: &HashMap, private: bool, - root: &Path, + root: &Utf8Path, ) -> CompileResult<'src, Justfile<'src>> { let mut absent_modules = BTreeSet::new(); let mut definitions = HashMap::new(); diff --git a/src/arguments.rs b/src/arguments.rs index ecb44ed996..ff88261a1c 100644 --- a/src/arguments.rs +++ b/src/arguments.rs @@ -54,7 +54,7 @@ pub struct Arguments { help = "Do not ascend above directory when searching for a justfile.", long, )] - pub(crate) ceiling: Option, + pub(crate) ceiling: Option, #[arg( help = "Run `--fmt` in 'check' mode. Exits with 0 if justfile is formatted correctly. \ Exits with 1 and prints a diff if formatting is required.", @@ -68,7 +68,7 @@ pub struct Arguments { help = "Override binary invoked by `--choose`", long )] - pub(crate) chooser: Option, + pub(crate) chooser: Option, #[arg(help = "Clear shell arguments", long, overrides_with = "shell_arg")] pub(crate) clear_shell_args: bool, #[arg( @@ -99,7 +99,7 @@ pub struct Arguments { help = "Use binary at to convert between unix and Windows paths", long, )] - pub(crate) cygpath: PathBuf, + pub(crate) cygpath: Utf8PathBuf, #[arg( env = "JUST_DEFAULT_LIST", help = "List recipes when no arguments are provided", @@ -205,7 +205,7 @@ pub struct Arguments { long, short = 'f', )] - pub(crate) justfile: Option, + pub(crate) justfile: Option, #[arg( env = "JUST_JUSTFILE_NAME", help = "Search for justfile named , accepts multiple `,`-separated values and may be \ @@ -303,7 +303,7 @@ pub struct Arguments { help = "Save temporary files to .", long, )] - pub(crate) tempdir: Option, + pub(crate) tempdir: Option, #[arg(env = "JUST_TIME", help = "Print recipe execution time", long)] pub(crate) time: bool, #[arg(env = "JUST_TIMESTAMP", help = "Print recipe command timestamps", long)] @@ -345,7 +345,7 @@ pub struct Arguments { requires = "justfile", short = 'd', )] - pub(crate) working_directory: Option, + pub(crate) working_directory: Option, #[arg(env = "JUST_YES", help = "Automatically confirm all recipes.", long)] pub(crate) yes: bool, } @@ -383,9 +383,8 @@ pub(crate) struct Subcommand { long, num_args = 1.., short = 'c', - value_parser = clap::value_parser!(OsString), )] - pub(crate) command: Option>, + pub(crate) command: Option>, #[arg( help = "Print shell completion script for ", help_heading = Self::HEADING, diff --git a/src/ast.rs b/src/ast.rs index 865ff59cf4..7b01b11f97 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -10,7 +10,7 @@ pub(crate) struct Ast<'src> { pub(crate) module_path: Modulepath, pub(crate) unstable_features: BTreeSet, pub(crate) warnings: Vec, - pub(crate) working_directory: PathBuf, + pub(crate) working_directory: Utf8PathBuf, } impl Ast<'_> { diff --git a/src/cache.rs b/src/cache.rs index 6dbdc14bea..25dc1611c5 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -4,7 +4,7 @@ const DIR: &str = ".justcache"; pub(crate) struct Cache { initialized: Mutex, - path: PathBuf, + path: Utf8PathBuf, } impl Cache { @@ -12,7 +12,7 @@ impl Cache { &self, config: &Config, key: CacheKey, - outputs: &BTreeMap, + outputs: &BTreeMap, ) -> RunResult<'static, CacheStatus> { let mut hasher = blake3::Hasher::new(); @@ -77,7 +77,7 @@ impl Cache { } } - fn entry(&self, key: blake3::Hash) -> RunResult<'static, PathBuf> { + fn entry(&self, key: blake3::Hash) -> RunResult<'static, Utf8PathBuf> { let mut initialized = self.initialized.lock().unwrap(); if !*initialized { @@ -93,7 +93,7 @@ impl Cache { pub(crate) fn inputs( value: Value, - working_directory: &Path, + working_directory: &Utf8Path, ) -> RunResult<'static, BTreeMap> { let mut inputs = BTreeMap::new(); @@ -127,7 +127,7 @@ impl Cache { Ok(inputs) } - pub(crate) fn dir(search: &Search) -> PathBuf { + pub(crate) fn dir(search: &Search) -> Utf8PathBuf { search.justfile_parent().join(DIR) } } diff --git a/src/cache_key.rs b/src/cache_key.rs index c5e4f1932e..d2297d1f50 100644 --- a/src/cache_key.rs +++ b/src/cache_key.rs @@ -10,5 +10,5 @@ pub(crate) struct CacheKey<'a> { pub(crate) inputs: Option>, pub(crate) positional: Option<&'a [String]>, pub(crate) recipe: &'a Modulepath, - pub(crate) working_directory: Option<&'a Path>, + pub(crate) working_directory: Option<&'a Utf8Path>, } diff --git a/src/cache_lock.rs b/src/cache_lock.rs index 41b03a5749..3e442ee99a 100644 --- a/src/cache_lock.rs +++ b/src/cache_lock.rs @@ -2,7 +2,7 @@ use super::*; pub(crate) struct CacheLock { pub(crate) file: File, - pub(crate) path: PathBuf, + pub(crate) path: Utf8PathBuf, pub(crate) recipe: Modulepath, } diff --git a/src/clean.rs b/src/clean.rs index 659a816a46..b3a8b9e756 100644 --- a/src/clean.rs +++ b/src/clean.rs @@ -1,12 +1,12 @@ use super::*; pub(crate) trait Clean { - fn clean(self) -> PathBuf; + fn clean(self) -> Utf8PathBuf; } -impl Clean for &Path { - fn clean(self) -> PathBuf { - use Component::*; +impl Clean for &Utf8Path { + fn clean(self) -> Utf8PathBuf { + use Utf8Component::*; let mut components = Vec::new(); @@ -39,7 +39,7 @@ mod tests { #[track_caller] fn case(path: &str, expected: &str) { - assert_eq!(Path::new(path).clean(), Path::new(expected)); + assert_eq!(Utf8Path::new(path).clean(), Utf8Path::new(expected)); } #[test] diff --git a/src/command_ext.rs b/src/command_ext.rs index 7e9ac7f1f1..40d5bc4171 100644 --- a/src/command_ext.rs +++ b/src/command_ext.rs @@ -5,9 +5,9 @@ pub(crate) trait CommandExt { fn output_guard_stdout(self) -> Result; - fn resolve(program: impl AsRef) -> Command; + fn resolve(program: impl AsRef) -> Command; - fn shell_arg(&mut self, arg: impl AsRef) -> &mut Command; + fn shell_arg(&mut self, arg: impl AsRef) -> &mut Command; fn status_guard(self) -> (io::Result, Option); } @@ -39,8 +39,8 @@ impl CommandExt for Command { ) } - fn resolve(program: impl AsRef) -> Self { - let program = Path::new(program.as_ref()); + fn resolve(program: impl AsRef) -> Self { + let program = Utf8Path::new(program.as_ref()); if !cfg!(windows) { return Self::new(program); @@ -49,7 +49,7 @@ impl CommandExt for Command { let mut candidates = vec![program.into()]; let mut components = program.components(); - if matches!(components.next(), Some(Component::Normal(_))) + if matches!(components.next(), Some(Utf8Component::Normal(_))) && components.next().is_none() && let Some(path) = env::var_os("PATH") { @@ -90,14 +90,14 @@ impl CommandExt for Command { Self::new(program) } - fn shell_arg(&mut self, arg: impl AsRef) -> &mut Command { + fn shell_arg(&mut self, arg: impl AsRef) -> &mut Command { #[cfg(windows)] if ShellKind::from(&*self) == ShellKind::Cmd { use std::os::windows::process::CommandExt; return self.raw_arg(arg); } - self.arg(arg) + self.arg(arg.as_ref()) } fn status_guard(self) -> (io::Result, Option) { diff --git a/src/compilation.rs b/src/compilation.rs index d30999c058..7f6d001b32 100644 --- a/src/compilation.rs +++ b/src/compilation.rs @@ -2,10 +2,10 @@ use super::*; #[derive(Debug)] pub(crate) struct Compilation<'src> { - pub(crate) asts: HashMap<(Modulepath, PathBuf), Ast<'src>>, + pub(crate) asts: HashMap<(Modulepath, Utf8PathBuf), Ast<'src>>, pub(crate) justfile: Justfile<'src>, pub(crate) overrides: HashMap, - pub(crate) root: PathBuf, + pub(crate) root: Utf8PathBuf, } impl<'src> Compilation<'src> { diff --git a/src/compiler.rs b/src/compiler.rs index 37a6752f87..4b3b7bf267 100644 --- a/src/compiler.rs +++ b/src/compiler.rs @@ -6,12 +6,12 @@ impl Compiler { pub(crate) fn compile<'src>( config: &Config, loader: &'src Loader, - root: &Path, + root: &Utf8Path, ) -> RunResult<'src, Compilation<'src>> { - let mut asts = HashMap::<(Modulepath, PathBuf), Ast>::new(); + let mut asts = HashMap::<(Modulepath, Utf8PathBuf), Ast>::new(); let mut loaded = Vec::new(); let mut numerator = Numerator::new(); - let mut paths = HashMap::::new(); + let mut paths = HashMap::::new(); let mut stack = Vec::new(); stack.push(Source::root(root)); @@ -29,7 +29,7 @@ impl Compiler { continue; } - let (relative, src) = loader.load(config, root, ¤t.path)?; + let (relative, src) = loader.load(root, ¤t.path)?; if paths .insert(current.path.clone(), relative.into()) @@ -154,10 +154,10 @@ impl Compiler { } fn find_module_file<'src>( - parent: &Path, + parent: &Utf8Path, module: Name<'src>, - path: Option<&Path>, - ) -> RunResult<'src, Option> { + path: Option<&Utf8Path>, + ) -> RunResult<'src, Option> { let mut candidates = Vec::new(); if let Some(path) = path { @@ -181,7 +181,7 @@ impl Compiler { } } - let mut grouped = BTreeMap::>::new(); + let mut grouped = BTreeMap::>::new(); for (candidate, case_sensitive) in candidates { let candidate = parent.join(candidate).clean(); @@ -221,7 +221,7 @@ impl Compiler { if let Some(name) = entry.file_name().to_str() { for (candidate, case_sensitive) in &candidates { - let candidate_name = candidate.file_name().unwrap().to_str().unwrap(); + let candidate_name = candidate.file_name().unwrap(); let eq = if *case_sensitive { name == candidate_name @@ -245,7 +245,7 @@ impl Compiler { .map(|found| { found .strip_prefix(parent) - .map(PathBuf::from) + .map(Utf8PathBuf::from) .unwrap_or(found) }) .collect(), @@ -256,13 +256,11 @@ impl Compiler { } } - fn expand_tilde(path: &str) -> RunResult<'static, PathBuf> { + fn expand_tilde(path: &str) -> RunResult<'static, Utf8PathBuf> { Ok(if let Some(path) = path.strip_prefix("~/") { - dirs::home_dir() - .ok_or(Error::Homedir)? - .join(path.trim_start_matches('/')) + dir::home_directory_required()?.join(path.trim_start_matches('/')) } else { - PathBuf::from(path) + Utf8PathBuf::from(path) }) } @@ -270,10 +268,10 @@ impl Compiler { pub(crate) fn test_compile(src: &str) -> CompileResult { let tokens = Lexer::test_lex(src)?; let ast = Parser::parse_tokens(&mut Numerator::new(), &tokens)?; - let root = PathBuf::from("justfile"); - let mut asts: HashMap<(Modulepath, PathBuf), Ast> = HashMap::new(); + let root = Utf8PathBuf::from("justfile"); + let mut asts: HashMap<(Modulepath, Utf8PathBuf), Ast> = HashMap::new(); asts.insert((Modulepath::default(), root.clone()), ast); - let mut paths: HashMap = HashMap::new(); + let mut paths: HashMap = HashMap::new(); paths.insert(root.clone(), root.clone()); Analyzer::analyze( &asts, diff --git a/src/config.rs b/src/config.rs index 1983e477e3..f8f5e55dd2 100644 --- a/src/config.rs +++ b/src/config.rs @@ -4,12 +4,12 @@ use super::*; pub(crate) struct Config { pub(crate) alias_style: AliasStyle, pub(crate) allow_missing: bool, - pub(crate) ceiling: Option, + pub(crate) ceiling: Option, pub(crate) check: bool, pub(crate) color: Color, pub(crate) command_color: Option, pub(crate) complete_aliases: bool, - pub(crate) cygpath: PathBuf, + pub(crate) cygpath: Utf8PathBuf, pub(crate) default_list: bool, pub(crate) dotenv_command: Vec, pub(crate) dotenv_filename: Vec, @@ -19,7 +19,7 @@ pub(crate) struct Config { pub(crate) groups: Vec, pub(crate) highlight: bool, pub(crate) indentation: Option, - pub(crate) invocation_directory: PathBuf, + pub(crate) invocation_directory: Utf8PathBuf, pub(crate) jobs: Option, pub(crate) justfile_names: Option>, pub(crate) list_heading: String, @@ -36,7 +36,7 @@ pub(crate) struct Config { pub(crate) shell_args: Option>, pub(crate) shell_command: bool, pub(crate) subcommand: Subcommand, - pub(crate) tempdir: Option, + pub(crate) tempdir: Option, pub(crate) time: bool, pub(crate) timestamp: bool, pub(crate) timestamp_format: String, @@ -66,7 +66,7 @@ impl Config { groups: Vec::new(), highlight: true, indentation: None, - invocation_directory: env::current_dir().context(config_error::CurrentDir)?, + invocation_directory: dir::current_directory()?, jobs: None, justfile_names: None, list_heading: Arguments::DEFAULT_LIST_HEADING.into(), @@ -120,7 +120,7 @@ impl Config { let working_directory = arguments.working_directory.clone(); - if let Some(search_directory) = positional.search_directory.as_ref().map(PathBuf::from) { + if let Some(search_directory) = positional.search_directory.as_ref().map(Utf8PathBuf::from) { if arguments.global_justfile || justfile.is_some() || working_directory.is_some() { return Err(ConfigError::SearchDirConflict); } @@ -131,7 +131,7 @@ impl Config { match (justfile, working_directory) { (None, None) => Ok(SearchConfig::FromInvocationDirectory), (Some(justfile), working_directory) => { - if justfile == Path::new(STANDARD_INPUT_ARGUMENT) { + if justfile == Utf8Path::new(STANDARD_INPUT_ARGUMENT) { Ok(SearchConfig::FromStandardInput { working_directory }) } else if let Some(working_directory) = working_directory { Ok(SearchConfig::WithJustfileAndWorkingDirectory { @@ -309,18 +309,13 @@ impl Config { } let unstable = arguments.unstable || subcommand == Subcommand::Summary; - let color = Color::new(arguments.indentation.unwrap_or_default(), arguments.color); - - let invocation_directory = env::current_dir().context(config_error::CurrentDir)?; - - Self::warn_non_unicode_path(color, "invocation directory", &invocation_directory); Ok(Self { alias_style: arguments.alias_style, allow_missing: arguments.allow_missing, ceiling: arguments.ceiling, check: arguments.check, - color, + color: Color::new(arguments.indentation.unwrap_or_default(), arguments.color), command_color: arguments.command_color.map(CommandColor::into), complete_aliases: arguments.complete_aliases, cygpath: arguments.cygpath, @@ -333,7 +328,7 @@ impl Config { groups: arguments.group, highlight: !arguments.no_highlight, indentation: arguments.indentation, - invocation_directory, + invocation_directory: dir::current_directory()?, jobs: arguments.jobs, justfile_names: arguments.justfile_names, list_heading: arguments.list_heading, @@ -382,19 +377,6 @@ impl Config { Err(Error::UnstableFeature { unstable_feature }) } } - - pub(crate) fn warn_non_unicode_path(color: Color, name: &str, path: &Path) { - if path.to_str().is_none() { - eprintln!( - "{}The {name} path `{}` is not Unicode. Just is considering phasing-out support for \ - non-Unicode paths. If you see this warning, please leave a comment on \ - https://github.com/casey/just/issues/3229. Thank you!{}", - color.warning().prefix(), - path.display(), - color.warning().suffix(), - ); - } - } } #[cfg(test)] diff --git a/src/config_error.rs b/src/config_error.rs index f1c739838f..06a37762ac 100644 --- a/src/config_error.rs +++ b/src/config_error.rs @@ -3,8 +3,6 @@ use super::*; #[derive(Debug, Snafu)] #[snafu(visibility(pub(crate)), context(suffix(false)))] pub(crate) enum ConfigError { - #[snafu(display("failed to get current directory: {}", source))] - CurrentDir { source: io::Error }, #[snafu(display( "internal config error, this may indicate a bug in just: {message} \ consider filing an issue: https://github.com/casey/just/issues/new", @@ -14,6 +12,8 @@ pub(crate) enum ConfigError { ModulePath { path: Vec }, #[snafu(display("invalid override path `{path}`"))] OverridePath { path: String }, + #[snafu(transparent)] + Path { source: PathError }, #[snafu(display("failed to parse request: {source}"))] RequestParse { source: serde_json::Error }, #[snafu(display( diff --git a/src/dir.rs b/src/dir.rs new file mode 100644 index 0000000000..f566f4d564 --- /dev/null +++ b/src/dir.rs @@ -0,0 +1,38 @@ +use super::*; + +pub(crate) fn config_directory() -> PathResult> { + dirs::config_dir() + .map(|path| Utf8PathBuf::try_from(path).context(path_error::ConfigDirectoryUnicode)) + .transpose() +} + +pub(crate) fn current_directory() -> PathResult { + Ok( + env::current_dir() + .context(path_error::CurrentDirectoryIo)? + .try_into() + .context(path_error::CurrentDirectoryUnicode)?, + ) +} + +pub(crate) fn home_directory() -> PathResult> { + dirs::home_dir() + .map(|path| Utf8PathBuf::try_from(path).context(path_error::HomeDirectoryUnicode)) + .transpose() +} + +pub(crate) fn home_directory_required() -> PathResult { + home_directory()?.context(path_error::HomeDirectoryMissing) +} + +pub(crate) fn runtime_directory() -> PathResult> { + dirs::runtime_dir() + .map(|path| Utf8PathBuf::try_from(path).context(path_error::RuntimeDirectoryUnicode)) + .transpose() +} + +pub(crate) fn temporary_directory(tempdir: &TempDir) -> PathResult<&Utf8Path> { + Utf8Path::from_path(tempdir.path()).context(path_error::TemporaryDirectoryUnicode { + path: tempdir.path(), + }) +} diff --git a/src/error.rs b/src/error.rs index f65195f60f..f8ced9d2d7 100644 --- a/src/error.rs +++ b/src/error.rs @@ -8,7 +8,7 @@ pub(crate) enum Error<'src> { }, AmbiguousModuleFile { module: Name<'src>, - found: Vec, + found: Vec, }, ArgumentPatternMismatch { argument: String, @@ -37,18 +37,21 @@ pub(crate) enum Error<'src> { output_error: OutputError, }, CacheEntryRead { - path: PathBuf, + path: Utf8PathBuf, source: serde_json::Error, }, + CacheEntryUnicode { + source: FromPathBufError, + }, CacheEntryWrite { - path: PathBuf, + path: Utf8PathBuf, source: serde_json::Error, }, CacheInputDirectory { - path: PathBuf, + path: Utf8PathBuf, }, CacheInputMissing { - path: PathBuf, + path: Utf8PathBuf, }, CacheKeySerialize { source: serde_json::Error, @@ -60,24 +63,24 @@ pub(crate) enum Error<'src> { ChooserInvoke { shell_binary: String, shell_arguments: String, - chooser: OsString, + chooser: String, io_error: io::Error, }, ChooserRead { - chooser: OsString, + chooser: String, io_error: io::Error, }, ChooserStatus { - chooser: OsString, + chooser: String, status: ExitStatus, }, ChooserWrite { - chooser: OsString, + chooser: String, io_error: io::Error, }, CircularImport { - current: PathBuf, - import: PathBuf, + current: Utf8PathBuf, + import: Utf8PathBuf, }, Code { recipe: &'src str, @@ -86,13 +89,13 @@ pub(crate) enum Error<'src> { print_message: bool, }, CommandInvoke { - binary: OsString, - arguments: Vec, + binary: String, + arguments: Vec, io_error: io::Error, }, CommandStatus { - binary: OsString, - arguments: Vec, + binary: String, + arguments: Vec, status: ExitStatus, }, Compile { @@ -104,9 +107,6 @@ pub(crate) enum Error<'src> { Const { const_error: ConstError<'src>, }, - CurrentDirectory { - source: io::Error, - }, Cygpath { recipe: &'src str, output_error: OutputError, @@ -118,7 +118,7 @@ pub(crate) enum Error<'src> { }, Dotenv { dotenv_error: dotenvy::Error, - path: PathBuf, + path: Utf8PathBuf, }, DotenvArgumentsRequireLists, DotenvCommand { @@ -134,11 +134,11 @@ pub(crate) enum Error<'src> { switch: Switch, }, EditorInvoke { - editor: OsString, + editor: String, io_error: io::Error, }, EditorStatus { - editor: OsString, + editor: String, status: ExitStatus, }, EmptyListArgument { @@ -165,7 +165,7 @@ pub(crate) enum Error<'src> { }, FilesystemIo { source: io::Error, - path: PathBuf, + path: Utf8PathBuf, }, FlagWithValue { recipe: &'src str, @@ -184,9 +184,8 @@ pub(crate) enum Error<'src> { line_number: usize, code: i32, }, - Homedir, InitExists { - justfile: PathBuf, + justfile: Utf8PathBuf, }, Internal { message: String, @@ -212,7 +211,7 @@ pub(crate) enum Error<'src> { token: Box>, }, Load { - path: PathBuf, + path: Utf8PathBuf, io_error: io::Error, }, MissingImportFile { @@ -242,6 +241,9 @@ pub(crate) enum Error<'src> { recipe: &'src str, switch: Switch, }, + Path { + source: PathError, + }, PositionalArgumentCountMismatch { recipe: Box>, found: usize, @@ -264,7 +266,7 @@ pub(crate) enum Error<'src> { }, RuntimeDirIo { io_error: io::Error, - path: PathBuf, + path: Utf8PathBuf, }, Script { command: String, @@ -346,7 +348,7 @@ pub(crate) enum Error<'src> { unstable_feature: UnstableFeature, }, WriteJustfile { - justfile: PathBuf, + justfile: Utf8PathBuf, io_error: io::Error, }, } @@ -530,7 +532,7 @@ impl ColorDisplay for Error<'_> { AmbiguousModuleFile { module, found } => write!( f, "found multiple source files for module `{module}`: {}", - List::and_ticked(found.iter().map(|path| path.display())), + List::and_ticked(found.iter().map(|path| path)), )?, ArgumentPatternMismatch { argument, @@ -598,21 +600,20 @@ impl ColorDisplay for Error<'_> { "backtick succeeded but stdout was not utf8: {utf8_error}", )?, }, - CacheEntryRead { path, source } => write!( - f, - "failed to read cache entry at `{}`: {source}", - path.display(), - )?, - CacheEntryWrite { path, source } => write!( - f, - "failed to write cache entry at `{}`: {source}", - path.display(), - )?, + CacheEntryRead { path, source } => { + write!(f, "failed to read cache entry at `{path}`: {source}",)? + } + CacheEntryUnicode { source } => { + write!(f, "cache entry path is not valid unicode: {source}",)? + } + CacheEntryWrite { path, source } => { + write!(f, "failed to write cache entry at `{path}`: {source}",)? + } CacheInputDirectory { path } => { - write!(f, "cache input is directory: `{}`", path.display())?; + write!(f, "cache input is directory: `{path}`")?; } CacheInputMissing { path } => { - write!(f, "cache input does not exist: `{}`", path.display())?; + write!(f, "cache input does not exist: `{path}`")?; } CacheKeySerialize { source } => write!(f, "failed to serialize cache key: {source}")?, CacheOutputMissing { recipe, output } => { @@ -627,30 +628,24 @@ impl ColorDisplay for Error<'_> { chooser, io_error, } => { - let chooser = chooser.to_string_lossy(); write!( f, "chooser `{shell_binary} {shell_arguments} {chooser}` invocation failed: {io_error}", )?; } ChooserRead { chooser, io_error } => { - let chooser = chooser.to_string_lossy(); write!( f, "failed to read output from chooser `{chooser}`: {io_error}", )?; } ChooserStatus { chooser, status } => { - let chooser = chooser.to_string_lossy(); write!(f, "chooser `{chooser}` failed: {status}")?; } ChooserWrite { chooser, io_error } => { - let chooser = chooser.to_string_lossy(); write!(f, "failed to write to chooser `{chooser}`: {io_error}")?; } CircularImport { current, import } => { - let import = import.display(); - let current = current.display(); write!(f, "import `{import}` in `{current}` is circular")?; } Code { @@ -687,7 +682,6 @@ impl ColorDisplay for Error<'_> { Compile { compile_error } => Display::fmt(compile_error, f)?, Config { config_error } => Display::fmt(config_error, f)?, Const { const_error } => write!(f, "{const_error}")?, - CurrentDirectory { source } => write!(f, "failed to get current directory: {source}")?, Cygpath { recipe, output_error, @@ -740,8 +734,7 @@ impl ColorDisplay for Error<'_> { Dotenv { dotenv_error, path } => { write!( f, - "failed to load environment file from `{}`: {dotenv_error}", - path.display(), + "failed to load environment file from `{path}`: {dotenv_error}", )?; } DotenvArgumentsRequireLists => { @@ -769,11 +762,9 @@ impl ColorDisplay for Error<'_> { )?; } EditorInvoke { editor, io_error } => { - let editor = editor.to_string_lossy(); write!(f, "editor `{editor}` invocation failed: {io_error}")?; } EditorStatus { editor, status } => { - let editor = editor.to_string_lossy(); write!(f, "editor `{editor}` failed: {status}")?; } EnvVarUnicode { name, value } => { @@ -808,7 +799,7 @@ impl ColorDisplay for Error<'_> { write!(f, "expected submodule at `{path}` but found recipe")?; } FilesystemIo { source, path } => { - write!(f, "I/O error at `{}`: {source}", path.display())?; + write!(f, "I/O error at `{path}`: {source}")?; } FlagWithValue { recipe, switch } => { write!(f, "recipe `{recipe}` flag `{switch}` does not take value")?; @@ -832,11 +823,8 @@ impl ColorDisplay for Error<'_> { "guard line in recipe `{recipe}` on line {line_number} returned reserved exit code {code}", )?; } - Homedir => { - write!(f, "failed to get homedir")?; - } InitExists { justfile } => { - write!(f, "justfile `{}` already exists", justfile.display())?; + write!(f, "justfile `{justfile}` already exists")?; } Internal { message } => { write!( @@ -903,11 +891,7 @@ impl ColorDisplay for Error<'_> { }?; } Load { io_error, path } => { - write!( - f, - "failed to read justfile at `{}`: {io_error}", - path.display() - )?; + write!(f, "failed to read justfile at `{path}`: {io_error}",)?; } NonFinalOptionWithValue { recipe, switch } => { write!( @@ -934,6 +918,9 @@ impl ColorDisplay for Error<'_> { OptionMissingValue { recipe, switch } => { write!(f, "recipe `{recipe}` option `{switch}` missing value")?; } + Path { source } => { + write!(f, "{source}")?; + } PositionalArgumentCountMismatch { recipe, found, @@ -981,11 +968,7 @@ impl ColorDisplay for Error<'_> { )?, RegexCompile { source, .. } => write!(f, "{source}")?, RuntimeDirIo { io_error, path } => { - write!( - f, - "I/O error in runtime dir `{}`: {io_error}", - path.display(), - )?; + write!(f, "I/O error in runtime dir `{path}`: {io_error}",)?; } Script { command, @@ -1108,7 +1091,6 @@ impl ColorDisplay for Error<'_> { )?; } WriteJustfile { justfile, io_error } => { - let justfile = justfile.display(); write!(f, "failed to write justfile to `{justfile}`: {io_error}")?; } } @@ -1147,10 +1129,16 @@ impl ColorDisplay for Error<'_> { } } -fn format_cmd(binary: &OsString, arguments: &Vec) -> String { +fn format_cmd(binary: &String, arguments: &Vec) -> String { iter::once(binary) .chain(arguments) - .map(|value| Enclosure::tick(value.to_string_lossy()).to_string()) + .map(|value| Enclosure::tick(value).to_string()) .collect::>() .join(" ") } + +impl From for Error<'_> { + fn from(source: PathError) -> Self { + Self::Path { source } + } +} diff --git a/src/execution_context.rs b/src/execution_context.rs index 335e59c46b..e1df9241c2 100644 --- a/src/execution_context.rs +++ b/src/execution_context.rs @@ -22,7 +22,7 @@ impl<'src: 'run, 'run> ExecutionContext<'src, 'run> { match &self.module.settings.tempdir { Some(tempdir) => builder.tempdir_in(self.search.working_directory.join(tempdir)), None => { - if let Some(runtime_dir) = dirs::runtime_dir() { + if let Some(runtime_dir) = dir::runtime_directory()? { let path = runtime_dir.join(JUST_DIRECTORY); fs::create_dir_all(&path).map_err(|io_error| Error::RuntimeDirIo { io_error, @@ -41,7 +41,7 @@ impl<'src: 'run, 'run> ExecutionContext<'src, 'run> { }) } - pub(crate) fn working_directory(&self) -> PathBuf { + pub(crate) fn working_directory(&self) -> Utf8PathBuf { let base = if self.module.is_submodule() { &self.module.working_directory } else { diff --git a/src/executor.rs b/src/executor.rs index 4e7b4abfa7..e87b6ec8dc 100644 --- a/src/executor.rs +++ b/src/executor.rs @@ -11,9 +11,9 @@ impl Executor<'_> { pub(crate) fn command<'src>( &self, config: &Config, - path: &Path, + path: &Utf8Path, recipe: &'src str, - working_directory: Option<&Path>, + working_directory: Option<&Utf8Path>, ) -> RunResult<'src, Command> { match self { Self::Command(interpreter) => { diff --git a/src/filesystem.rs b/src/filesystem.rs index b39ad106cc..8d9fa0a1f6 100644 --- a/src/filesystem.rs +++ b/src/filesystem.rs @@ -1,6 +1,6 @@ use super::*; -pub(crate) fn exists(path: &Path) -> RunResult<'static, bool> { +pub(crate) fn exists(path: &Utf8Path) -> RunResult<'static, bool> { match path.metadata() { Ok(_) => Ok(true), Err(source) => { @@ -16,7 +16,7 @@ pub(crate) fn exists(path: &Path) -> RunResult<'static, bool> { } } -pub(crate) fn is_file(path: &Path) -> RunResult<'static, bool> { +pub(crate) fn is_file(path: &Utf8Path) -> RunResult<'static, bool> { match path.metadata() { Ok(metadata) => Ok(metadata.is_file()), Err(source) => { diff --git a/src/function.rs b/src/function.rs index 2c21797078..5ef89dfda3 100644 --- a/src/function.rs +++ b/src/function.rs @@ -178,18 +178,14 @@ fn bool(context: Context, value: &Value) -> ValueResult { } fn absolute_path(context: Context, path: &str) -> StringResult { - let abs_path_unchecked = context - .execution_context - .working_directory() - .join(path) - .clean(); - match abs_path_unchecked.to_str() { - Some(absolute_path) => Ok(absolute_path.to_owned()), - None => Err(format!( - "working directory is not valid Unicode: {}", - context.execution_context.search.working_directory.display() - )), - } + Ok( + context + .execution_context + .working_directory() + .join(path) + .clean() + .into(), + ) } fn append(context: Context, suffix: &str, s: &Value) -> ValueResult { @@ -221,7 +217,7 @@ fn blake3_file(context: Context, path: &str) -> StringResult { let mut hasher = blake3::Hasher::new(); hasher .update_mmap_rayon(&path) - .map_err(|err| format!("failed to hash `{}`: {err}", path.display()))?; + .map_err(|err| format!("failed to hash `{path}`: {err}"))?; Ok(hasher.finalize().to_string()) } @@ -274,10 +270,10 @@ fn choose(_context: Context, n: &str, alphabet: &str) -> StringResult { } fn clean(_context: Context, path: &str) -> StringResult { - Ok(Path::new(path).clean().to_str().unwrap().to_owned()) + Ok(Utf8Path::new(path).clean().into()) } -fn dir(name: &'static str, f: fn() -> Option) -> StringResult { +fn dir(name: &'static str, f: fn() -> Option) -> StringResult { match f() { Some(path) => path .as_os_str() @@ -391,22 +387,13 @@ fn invocation_directory(context: Context) -> StringResult { } fn invocation_directory_native(context: Context) -> StringResult { - context - .execution_context - .config - .invocation_directory - .to_str() - .map(str::to_owned) - .ok_or_else(|| { - format!( - "invocation directory is not valid Unicode: {}", - context - .execution_context - .config - .invocation_directory - .display() - ) - }) + Ok( + context + .execution_context + .config + .invocation_directory + .to_string(), + ) } fn is_dependency(context: Context) -> ValueResult { @@ -462,42 +449,24 @@ fn just_version(_context: Context) -> StringResult { } fn justfile(context: Context) -> StringResult { - context - .execution_context - .search - .justfile - .to_str() - .map(str::to_owned) - .ok_or_else(|| { - format!( - "justfile path is not valid Unicode: {}", - context.execution_context.search.justfile.display() - ) - }) + Ok(context.execution_context.search.justfile.to_string()) } fn justfile_directory(context: Context) -> StringResult { - let justfile_directory = context - .execution_context - .search - .justfile - .parent() - .ok_or_else(|| { - format!( - "could not resolve justfile directory, justfile `{}` had no parent", - context.execution_context.search.justfile.display() - ) - })?; - - justfile_directory - .to_str() - .map(str::to_owned) - .ok_or_else(|| { - format!( - "justfile directory is not valid Unicode: {}", - justfile_directory.display() - ) - }) + Ok( + context + .execution_context + .search + .justfile + .parent() + .ok_or_else(|| { + format!( + "could not resolve justfile directory, justfile `{}` had no parent", + context.execution_context.search.justfile, + ) + })? + .to_string(), + ) } fn kebabcase(_context: Context, s: &str) -> StringResult { @@ -517,23 +486,20 @@ fn lowercase(_context: Context, s: &str) -> StringResult { } fn module_directory(context: Context) -> StringResult { - let module_directory = context.execution_context.module.source.parent().unwrap(); - module_directory.to_str().map(str::to_owned).ok_or_else(|| { - format!( - "module directory is not valid Unicode: {}", - module_directory.display(), - ) - }) + Ok( + context + .execution_context + .module + .source + .parent() + .unwrap() + .as_str() + .into(), + ) } fn module_file(context: Context) -> StringResult { - let module_file = &context.execution_context.module.source; - module_file.to_str().map(str::to_owned).ok_or_else(|| { - format!( - "module file path is not valid Unicode: {}", - module_file.display(), - ) - }) + Ok(context.execution_context.module.source.to_string()) } fn module_path(context: Context) -> StringResult { @@ -630,11 +596,9 @@ fn sha256(_context: Context, s: &str) -> StringResult { fn sha256_file(context: Context, path: &str) -> StringResult { let path = context.execution_context.working_directory().join(path); - let mut file = - File::open(&path).map_err(|err| format!("failed to open `{}`: {err}", path.display()))?; + let mut file = File::open(&path).map_err(|err| format!("failed to open `{path}`: {err}"))?; let mut writer = HashWriter::::new(io::sink()); - io::copy(&mut file, &mut writer) - .map_err(|err| format!("failed to read `{}`: {err}", path.display()))?; + io::copy(&mut file, &mut writer).map_err(|err| format!("failed to read `{path}`: {err}"))?; Ok(hex::encode(writer.finalize())) } @@ -682,41 +646,32 @@ fn snakecase(_context: Context, s: &str) -> StringResult { } fn source_directory(context: Context) -> StringResult { - context - .execution_context - .search - .justfile - .parent() - .unwrap() - .join(context.name.token.path) - .parent() - .unwrap() - .to_str() - .map(str::to_owned) - .ok_or_else(|| { - format!( - "source file path is not valid Unicode: {}", - context.name.token.path.display(), - ) - }) + Ok( + context + .execution_context + .search + .justfile + .parent() + .unwrap() + .join(context.name.token.path) + .parent() + .unwrap() + .as_str() + .into(), + ) } fn source_file(context: Context) -> StringResult { - context - .execution_context - .search - .justfile - .parent() - .unwrap() - .join(context.name.token.path) - .to_str() - .map(str::to_owned) - .ok_or_else(|| { - format!( - "source file path is not valid Unicode: {}", - context.name.token.path.display(), - ) - }) + Ok( + context + .execution_context + .search + .justfile + .parent() + .unwrap() + .join(context.name.token.path) + .into_string(), + ) } fn split(_context: Context, s: &str, separator: Option<&str>) -> ValueResult { @@ -927,10 +882,7 @@ mod tests { fn dir_not_unicode() { use std::os::unix::ffi::OsStrExt; assert_eq!( - dir("foo", || Some( - std::ffi::OsStr::from_bytes(b"\xe0\x80\x80").into() - )) - .unwrap_err(), + dir("foo", || Some(OsStr::from_bytes(b"\xe0\x80\x80").into())).unwrap_err(), "unable to convert foo directory path to string: ���", ); } diff --git a/src/item.rs b/src/item.rs index 710e8ec87c..b47124ce0f 100644 --- a/src/item.rs +++ b/src/item.rs @@ -11,13 +11,13 @@ pub(crate) enum Item<'src> { Comment(&'src str), Function(FunctionDefinition<'src>), Import { - absolute: Option, + absolute: Option, attributes: AttributeSet<'src>, optional: bool, relative: StringLiteral<'src>, }, Module { - absolute: Option, + absolute: Option, attributes: AttributeSet<'src>, doc: Option, name: Name<'src>, diff --git a/src/justfile.rs b/src/justfile.rs index a199b75ad2..25fb26c121 100644 --- a/src/justfile.rs +++ b/src/justfile.rs @@ -29,7 +29,7 @@ pub(crate) struct Justfile<'src> { pub(crate) functions: Table<'src, FunctionDefinition<'src>>, pub(crate) groups: Vec>, #[serde(skip)] - pub(crate) loaded: Vec, + pub(crate) loaded: Vec, #[serde(skip)] pub(crate) module_aliases: Table<'src, ModuleAlias<'src>>, pub(crate) module_path: Modulepath, @@ -42,13 +42,13 @@ pub(crate) struct Justfile<'src> { pub(crate) recipe_aliases: Table<'src, RecipeAlias<'src>>, pub(crate) recipes: Table<'src, Arc>>, pub(crate) settings: Settings, - pub(crate) source: PathBuf, + pub(crate) source: Utf8PathBuf, pub(crate) unexports: BTreeSet, #[serde(skip)] pub(crate) unstable_features: BTreeSet, pub(crate) warnings: Vec, #[serde(skip)] - pub(crate) working_directory: PathBuf, + pub(crate) working_directory: Utf8PathBuf, } impl<'src> Justfile<'src> { diff --git a/src/lexer.rs b/src/lexer.rs index 443a5a97a4..6f8b182ddd 100644 --- a/src/lexer.rs +++ b/src/lexer.rs @@ -25,7 +25,7 @@ pub(crate) struct Lexer<'src> { /// Current open delimiters open_delimiters: Vec<(Delimiter, usize)>, /// Path to source file - path: &'src Path, + path: &'src Utf8Path, /// Inside recipe body recipe_body: bool, /// Next indent will start a recipe body @@ -46,7 +46,7 @@ impl<'src> Lexer<'src> { pub(crate) const INTERPOLATION_START: &'static str = "{{"; /// Lex `src` - pub(crate) fn lex(path: &'src Path, src: &'src str) -> CompileResult<'src, Vec>> { + pub(crate) fn lex(path: &'src Utf8Path, src: &'src str) -> CompileResult<'src, Vec>> { Self::new(path, src).tokenize() } @@ -56,7 +56,7 @@ impl<'src> Lexer<'src> { } /// Create a new Lexer to lex `src` - fn new(path: &'src Path, src: &'src str) -> Self { + fn new(path: &'src Utf8Path, src: &'src str) -> Self { let mut chars = src.chars(); let next = chars.next(); diff --git a/src/lib.rs b/src/lib.rs index 8623938bb1..c1ee266201 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -88,6 +88,7 @@ pub(crate) use { parameter::Parameter, parameter_kind::ParameterKind, parser::Parser, + path_error::PathError, pattern::Pattern, platform::Platform, platform_interface::PlatformInterface, @@ -143,7 +144,7 @@ pub(crate) use { warning::Warning, which::which, }, - camino::Utf8Path, + camino::{FromPathBufError, Utf8Component, Utf8Path, Utf8PathBuf}, chrono::{DateTime, Local, TimeZone, Utc, format::StrftimeItems}, clap::{CommandFactory, FromArgMatches, Parser as _, ValueEnum}, clap_complete::{ArgValueCompleter, CompletionCandidate, PathCompleter, engine::ValueCompleter}, @@ -156,7 +157,7 @@ pub(crate) use { ser::{SerializeMap, SerializeSeq, SerializeStruct}, }, sha2::{Digest, Sha256}, - snafu::{ResultExt, Snafu}, + snafu::{OptionExt, ResultExt, Snafu}, std::{ borrow::Borrow, cmp::Ordering, @@ -171,7 +172,6 @@ pub(crate) use { num::{NonZeroU64, ParseIntError}, ops::Deref, ops::{Index, RangeInclusive}, - path::{self, Component, Path, PathBuf}, process::{self, Command, ExitStatus, Stdio}, slice, str::{self, Chars, FromStr}, @@ -199,6 +199,7 @@ pub use {arguments::Arguments, request::Response, subcommand::INIT_JUSTFILE, uni type CompileResult<'a, T = ()> = Result>; type ConfigResult = Result; +type PathResult = Result; type RunResult<'a, T = ()> = Result>; type SearchResult = Result; type StringResult = Result; @@ -212,6 +213,19 @@ const RECURSION_LIMIT: usize = if cfg!(windows) { 48 } else { 256 }; const TEMPDIR_PREFIX: &str = "just-"; const VERSION: &str = env!("CARGO_PKG_VERSION"); +fn env_var(name: &str) -> RunResult<'static, Option> { + match env::var(name) { + Err(env::VarError::NotPresent) => Ok(None), + Err(env::VarError::NotUnicode(value)) => { + return Err(Error::EnvVarUnicode { + name: name.into(), + value, + }); + } + Ok(value) => Ok(Some(value)), + } +} + fn signal_exit_code(number: i32) -> Option { number.checked_add(128) } @@ -268,6 +282,7 @@ mod datetime_format_error; mod delimiter; mod dependency; mod dependency_argument; +mod dir; mod disabled; mod dump_format; mod element; @@ -312,6 +327,7 @@ mod output_error; mod parameter; mod parameter_kind; mod parser; +mod path_error; mod pattern; mod platform; mod platform_interface; diff --git a/src/load_dotenv.rs b/src/load_dotenv.rs index 1f4ac5a3cf..ccacd59a23 100644 --- a/src/load_dotenv.rs +++ b/src/load_dotenv.rs @@ -3,7 +3,7 @@ use super::*; pub(crate) fn load_dotenv( config: &Config, justfile: &Justfile, - working_directory: &Path, + working_directory: &Utf8Path, ) -> RunResult<'static, BTreeMap> { let settings = &justfile.settings; @@ -100,7 +100,7 @@ fn load_from_command( command: &str, config: &Config, settings: &Settings, - working_directory: &Path, + working_directory: &Utf8Path, ) -> RunResult<'static, BTreeMap> { let mut cmd = settings.shell_command(config); @@ -139,7 +139,7 @@ fn load_from_command( } fn load_from_file( - path: &Path, + path: &Utf8Path, settings: &Settings, ) -> RunResult<'static, Option>> { if path.is_dir() { diff --git a/src/loader.rs b/src/loader.rs index a641b2b69b..43d227f1f7 100644 --- a/src/loader.rs +++ b/src/loader.rs @@ -1,7 +1,7 @@ use super::*; pub(crate) struct Loader { - paths: Arena, + paths: Arena, srcs: Arena, } @@ -15,12 +15,9 @@ impl Loader { pub(crate) fn load<'src>( &'src self, - config: &Config, - root: &Path, - path: &Path, - ) -> RunResult<'src, (&'src Path, &'src str)> { - Config::warn_non_unicode_path(config.color, "justfile", path); - + root: &Utf8Path, + path: &Utf8Path, + ) -> RunResult<'src, (&'src Utf8Path, &'src str)> { let src = fs::read_to_string(path).map_err(|io_error| Error::Load { path: path.into(), io_error, diff --git a/src/parser.rs b/src/parser.rs index a185e3ce8a..8de643be5f 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -35,7 +35,7 @@ pub(crate) struct Parser<'run, 'src> { recursion_depth: usize, tokens: &'run [Token<'src>], unstable_features: BTreeSet, - working_directory: &'run Path, + working_directory: &'run Utf8Path, } impl<'run, 'src> Parser<'run, 'src> { @@ -46,7 +46,7 @@ impl<'run, 'src> Parser<'run, 'src> { module_namepath: Option<&'run Namepath<'src>>, numerator: &'run mut Numerator, tokens: &'run [Token<'src>], - working_directory: &'run Path, + working_directory: &'run Utf8Path, ) -> CompileResult<'src, Ast<'src>> { Self { expected_tokens: BTreeSet::new(), @@ -67,7 +67,7 @@ impl<'run, 'src> Parser<'run, 'src> { pub(crate) fn parse_source( numerator: &mut Numerator, - path: &'src Path, + path: &'src Utf8Path, source: &Source<'src>, src: &'src str, ) -> CompileResult<'src, Ast<'src>> { diff --git a/src/path_error.rs b/src/path_error.rs new file mode 100644 index 0000000000..09ecfb62bb --- /dev/null +++ b/src/path_error.rs @@ -0,0 +1,20 @@ +use super::*; + +#[derive(Debug, Snafu)] +#[snafu(visibility(pub(crate)), context(suffix(false)))] +pub(crate) enum PathError { + #[snafu(display("config directory is not valid unicode: {source}"))] + ConfigDirectoryUnicode { source: FromPathBufError }, + #[snafu(display("I/O error retrieving current directory: {source}"))] + CurrentDirectoryIo { source: io::Error }, + #[snafu(display("current directory is not valid unicode: {source}"))] + CurrentDirectoryUnicode { source: FromPathBufError }, + #[snafu(display("failed to get home directory"))] + HomeDirectoryMissing, + #[snafu(display("home directory is not valid unicode: {source}"))] + HomeDirectoryUnicode { source: FromPathBufError }, + #[snafu(display("runtime directory is not valid unicode: {source}"))] + RuntimeDirectoryUnicode { source: FromPathBufError }, + #[snafu(display("temporary directory is not valid unicode: `{}`", path.display()))] + TemporaryDirectoryUnicode { path: std::path::PathBuf }, +} diff --git a/src/platform/unix.rs b/src/platform/unix.rs index d1857545d2..7f0fc7e029 100644 --- a/src/platform/unix.rs +++ b/src/platform/unix.rs @@ -3,9 +3,9 @@ use super::*; impl PlatformInterface for Platform { fn make_shebang_command( _config: &Config, - path: &Path, + path: &Utf8Path, _shebang: Shebang, - working_directory: Option<&Path>, + working_directory: Option<&Utf8Path>, ) -> Result { // shebang scripts can be executed directly on unix let mut command = Command::resolve(path); @@ -17,7 +17,7 @@ impl PlatformInterface for Platform { Ok(command) } - fn set_execute_permission(path: &Path) -> io::Result<()> { + fn set_execute_permission(path: &Utf8Path) -> io::Result<()> { use std::os::unix::fs::PermissionsExt; // get current permissions @@ -36,11 +36,12 @@ impl PlatformInterface for Platform { exit_status.signal() } - fn convert_native_path(_config: &Config, _working_directory: &Path, path: &Path) -> StringResult { - path - .to_str() - .map(str::to_string) - .ok_or_else(|| String::from("Error getting current directory: unicode decode error")) + fn convert_native_path( + _config: &Config, + _working_directory: &Utf8Path, + path: &Utf8Path, + ) -> StringResult { + Ok(path.as_str().into()) } fn install_signal_handler(handler: T) -> RunResult<'static> { diff --git a/src/platform/windows.rs b/src/platform/windows.rs index 8a6c8f3161..c06249a98f 100644 --- a/src/platform/windows.rs +++ b/src/platform/windows.rs @@ -3,9 +3,9 @@ use super::*; impl PlatformInterface for Platform { fn make_shebang_command( config: &Config, - path: &Path, + path: &Utf8Path, shebang: Shebang, - working_directory: Option<&Path>, + working_directory: Option<&Utf8Path>, ) -> Result { use std::borrow::Cow; @@ -45,7 +45,7 @@ impl PlatformInterface for Platform { Ok(cmd) } - fn set_execute_permission(_path: &Path) -> io::Result<()> { + fn set_execute_permission(_path: &Utf8Path) -> io::Result<()> { // it is not necessary to set an execute permission on a script on windows, so // this is a nop Ok(()) @@ -57,7 +57,11 @@ impl PlatformInterface for Platform { None } - fn convert_native_path(config: &Config, working_directory: &Path, path: &Path) -> StringResult { + fn convert_native_path( + config: &Config, + working_directory: &Utf8Path, + path: &Utf8Path, + ) -> StringResult { // Translate path from windows style to unix style let mut cygpath = Command::resolve(&config.cygpath); diff --git a/src/platform_interface.rs b/src/platform_interface.rs index df3b8da66a..0334b5b80e 100644 --- a/src/platform_interface.rs +++ b/src/platform_interface.rs @@ -2,7 +2,11 @@ use super::*; pub(crate) trait PlatformInterface { /// translate path from "native" path to path interpreter expects - fn convert_native_path(config: &Config, working_directory: &Path, path: &Path) -> StringResult; + fn convert_native_path( + config: &Config, + working_directory: &Utf8Path, + path: &Utf8Path, + ) -> StringResult; /// install handler, may only be called once fn install_signal_handler(handler: T) -> RunResult<'static>; @@ -11,13 +15,13 @@ pub(crate) trait PlatformInterface { /// line `shebang` fn make_shebang_command( config: &Config, - path: &Path, + path: &Utf8Path, shebang: Shebang, - working_directory: Option<&Path>, + working_directory: Option<&Utf8Path>, ) -> Result; /// set the execute permission on file pointed to by `path` - fn set_execute_permission(path: &Path) -> io::Result<()>; + fn set_execute_permission(path: &Utf8Path) -> io::Result<()>; /// extract signal from process exit status fn signal_from_exit_status(exit_status: ExitStatus) -> Option; diff --git a/src/recipe.rs b/src/recipe.rs index 345b4acad5..0db77610c3 100644 --- a/src/recipe.rs +++ b/src/recipe.rs @@ -178,7 +178,7 @@ impl<'src> Recipe<'src> { &'a self, context: &'a ExecutionContext, evaluator: &mut Evaluator<'src, 'run>, - ) -> RunResult<'src, Option> { + ) -> RunResult<'src, Option> { if !self.change_directory(&context.module.settings) { return Ok(None); } @@ -576,7 +576,7 @@ impl<'src> Recipe<'src> { { let working_directory = match &working_directory { Some(working_directory) => working_directory.to_owned(), - None => env::current_dir().map_err(|source| Error::CurrentDirectory { source })?, + None => dir::current_directory()?, }; let environment_attribute = environment_attribute @@ -623,7 +623,7 @@ impl<'src> Recipe<'src> { let outputs = outputs .as_ref() - .map(|outputs| -> RunResult> { + .map(|outputs| -> RunResult> { let outputs = evaluator.evaluate_value(outputs)?; Ok( outputs @@ -675,7 +675,7 @@ impl<'src> Recipe<'src> { let tempdir = context.tempdir(self)?; - let mut path = tempdir.path().to_path_buf(); + let mut path = dir::temporary_directory(&tempdir)?.to_owned(); path.push(executor.script_filename(self.name(), extension)); diff --git a/src/scope.rs b/src/scope.rs index 8e02fb0c91..36b0ff5c0b 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -33,7 +33,7 @@ impl<'src, 'run> Scope<'src, 'run> { length: key.len(), line: 0, offset: 0, - path: Path::new("PRELUDE"), + path: Utf8Path::new("PRELUDE"), src: key, }, }, diff --git a/src/search.rs b/src/search.rs index 0ec36eff79..40f4af2ec6 100644 --- a/src/search.rs +++ b/src/search.rs @@ -6,24 +6,24 @@ const PROJECT_ROOT_CHILDREN: &[&str] = &[".bzr", ".git", ".hg", ".svn", "_darcs" #[derive(Debug)] pub(crate) struct Search { - pub(crate) justfile: PathBuf, + pub(crate) justfile: Utf8PathBuf, pub(crate) tempdir: Option, - pub(crate) working_directory: PathBuf, + pub(crate) working_directory: Utf8PathBuf, } impl Search { - pub(crate) fn justfile_parent(&self) -> &Path { + pub(crate) fn justfile_parent(&self) -> &Utf8Path { self.justfile.parent().unwrap() } - fn global_justfile_paths() -> Vec<(PathBuf, &'static str)> { + fn global_justfile_paths() -> SearchResult> { let mut paths = Vec::new(); - if let Some(config_dir) = dirs::config_dir() { + if let Some(config_dir) = dir::config_directory()? { paths.push((config_dir.join(JUST_DIRECTORY), DEFAULT_JUSTFILE_NAME)); } - if let Some(home_dir) = dirs::home_dir() { + if let Some(home_dir) = dir::home_directory()? { paths.push(( home_dir.join(".config").join(JUST_DIRECTORY), DEFAULT_JUSTFILE_NAME, @@ -34,7 +34,7 @@ impl Search { } } - paths + Ok(paths) } /// Find justfile given search configuration and invocation directory @@ -97,8 +97,8 @@ impl Search { fn with_justfile( config: &Config, - justfile: PathBuf, - working_directory: PathBuf, + justfile: Utf8PathBuf, + working_directory: Utf8PathBuf, ) -> SearchResult { if justfile .extension() @@ -128,7 +128,7 @@ impl Search { } } - fn tempdir_justfile(config: &Config, source: &str) -> SearchResult<(PathBuf, TempDir)> { + fn tempdir_justfile(config: &Config, source: &str) -> SearchResult<(Utf8PathBuf, TempDir)> { let mut builder = tempfile::Builder::new(); builder.prefix(TEMPDIR_PREFIX); @@ -140,7 +140,7 @@ impl Search { } .map_err(|io_error| SearchError::TempdirIo { io_error })?; - let justfile = tempdir.path().join("justfile"); + let justfile = dir::temporary_directory(&tempdir)?.join("justfile"); fs::write(&justfile, source).map_err(|io_error| SearchError::FilesystemIo { io_error, @@ -150,18 +150,24 @@ impl Search { Ok((justfile, tempdir)) } - fn find_global_justfile() -> SearchResult { - for (directory, filename) in Self::global_justfile_paths() { + fn find_global_justfile() -> SearchResult { + for (directory, filename) in Self::global_justfile_paths()? { if let Ok(read_dir) = fs::read_dir(&directory) { for entry in read_dir { let entry = entry.map_err(|io_error| SearchError::FilesystemIo { io_error, path: directory.clone(), })?; - if let Some(candidate) = entry.file_name().to_str() - && candidate.eq_ignore_ascii_case(filename) + + let candidate = + Utf8PathBuf::try_from(entry.path()).context(search_error::CandidateUnicode)?; + + if candidate + .file_name() + .unwrap() + .eq_ignore_ascii_case(filename) { - return Ok(entry.path()); + return Ok(candidate); } } } @@ -183,7 +189,7 @@ impl Search { } /// Find justfile starting in given directory searching upwards in directory tree - fn find_in_directory(config: &Config, starting_dir: &Path) -> SearchResult { + fn find_in_directory(config: &Config, starting_dir: &Utf8Path) -> SearchResult { let justfile = Self::justfile(config, starting_dir)?; let working_directory = Self::working_directory_from_justfile(&justfile)?; Self::with_justfile(config, justfile, working_directory) @@ -243,7 +249,7 @@ impl Search { /// Search upwards from `directory` for a file whose name matches one of /// `JUSTFILE_NAMES` - fn justfile(config: &Config, directory: &Path) -> SearchResult { + fn justfile(config: &Config, directory: &Utf8Path) -> SearchResult { for directory in directory.ancestors() { let mut candidates = BTreeSet::new(); @@ -257,19 +263,20 @@ impl Search { io_error, path: directory.to_owned(), })?; - if let Some(name) = entry.file_name().to_str() { - let justfile_names: Box> = - if let Some(justfile_names) = &config.justfile_names { - Box::new(justfile_names.iter().map(String::as_str)) - } else { - Box::new(JUSTFILE_NAMES.into_iter()) - }; - - for justfile_name in justfile_names { - if name.eq_ignore_ascii_case(justfile_name) { - candidates.insert(entry.path()); - } - } + + let entry = Utf8PathBuf::try_from(entry.path()).context(search_error::CandidateUnicode)?; + + let name = entry.file_name().unwrap(); + + let mut justfile_names: Box> = + if let Some(justfile_names) = &config.justfile_names { + Box::new(justfile_names.iter().map(String::as_str)) + } else { + Box::new(JUSTFILE_NAMES.into_iter()) + }; + + if justfile_names.any(|justfile_name| name.eq_ignore_ascii_case(justfile_name)) { + candidates.insert(entry); } } @@ -289,14 +296,14 @@ impl Search { Err(SearchError::NotFound) } - fn clean(config: &Config, path: &Path) -> PathBuf { + fn clean(config: &Config, path: &Utf8Path) -> Utf8PathBuf { config.invocation_directory.join(path).clean() } /// Search upwards from `directory` for the root directory of a software /// project, as determined by the presence of one of the version control /// system directories given in `PROJECT_ROOT_CHILDREN` - fn project_root(config: &Config, directory: &Path) -> SearchResult { + fn project_root(config: &Config, directory: &Utf8Path) -> SearchResult { for directory in directory.ancestors() { let entries = fs::read_dir(directory).map_err(|io_error| SearchError::FilesystemIo { io_error, @@ -325,7 +332,7 @@ impl Search { Ok(directory.to_owned()) } - fn working_directory_from_justfile(justfile: &Path) -> SearchResult { + fn working_directory_from_justfile(justfile: &Utf8Path) -> SearchResult { Ok( justfile .parent() @@ -365,8 +372,8 @@ mod tests { invocation_directory: prefix.into(), ..Config::new().unwrap() }; - let have = Search::clean(&config, Path::new(suffix)); - assert_eq!(have, Path::new(want)); + let have = Search::clean(&config, Utf8Path::new(suffix)); + assert_eq!(have, Utf8Path::new(want)); } } } diff --git a/src/search_config.rs b/src/search_config.rs index eec0fb02e1..458ec75d47 100644 --- a/src/search_config.rs +++ b/src/search_config.rs @@ -9,17 +9,19 @@ pub(crate) enum SearchConfig { #[default] FromInvocationDirectory, /// As in `Invocation`, but start from `search_directory`. - FromSearchDirectory { search_directory: PathBuf }, + FromSearchDirectory { search_directory: Utf8PathBuf }, /// Read justfile from standard input - FromStandardInput { working_directory: Option }, + FromStandardInput { + working_directory: Option, + }, /// Search for global justfile GlobalJustfile, /// Use user-specified justfile, with the working directory set to the /// directory that contains it. - WithJustfile { justfile: PathBuf }, + WithJustfile { justfile: Utf8PathBuf }, /// Use user-specified justfile and working directory. WithJustfileAndWorkingDirectory { - justfile: PathBuf, - working_directory: PathBuf, + justfile: Utf8PathBuf, + working_directory: Utf8PathBuf, }, } diff --git a/src/search_error.rs b/src/search_error.rs index 0c6eadd938..1313fa6af5 100644 --- a/src/search_error.rs +++ b/src/search_error.rs @@ -1,33 +1,37 @@ use super::*; #[derive(Debug, Snafu)] -#[snafu(visibility(pub(crate)))] +#[snafu(visibility(pub(crate)), context(suffix(false)))] pub(crate) enum SearchError { - #[snafu(display( - "I/O error at `{}`: {io_error}", - path.display(), - ))] - FilesystemIo { io_error: io::Error, path: PathBuf }, + #[snafu(display("justfile candidate path is not valid unicode: {source}",))] + CandidateUnicode { source: FromPathBufError }, + #[snafu(display("I/O error at `{path}`: {io_error}",))] + FilesystemIo { + io_error: io::Error, + path: Utf8PathBuf, + }, #[snafu(display("cannot initialize global justfile"))] GlobalJustfileInit, #[snafu(display("global justfile not found"))] GlobalJustfileNotFound, #[snafu(display("cannot use justfile from standard input with `--init`"))] InitWithJustfileFromStandardInput, - #[snafu(display("justfile path had no parent: {}", path.display()))] - JustfileHadNoParent { path: PathBuf }, + #[snafu(display("justfile path had no parent: {path}"))] + JustfileHadNoParent { path: Utf8PathBuf }, #[snafu(display( "multiple candidate justfiles found in `{}`: {}", - candidates.first().unwrap().parent().unwrap().display(), + candidates.first().unwrap().parent().unwrap(), List::and_ticked( candidates .iter() - .map(|candidate| candidate.file_name().unwrap().to_string_lossy()) + .map(|candidate| candidate.file_name().unwrap()) ), ))] - MultipleCandidates { candidates: BTreeSet }, + MultipleCandidates { candidates: BTreeSet }, #[snafu(display("no justfile found"))] NotFound, + #[snafu(transparent)] + Path { source: PathError }, #[snafu(display("error reading from standard input: {io_error}"))] StdinIo { io_error: io::Error }, #[snafu(display("I/O error creating temporary directory: {io_error}"))] diff --git a/src/settings.rs b/src/settings.rs index 51dff9bf96..8c4aed305d 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -48,7 +48,7 @@ pub(crate) struct Settings { pub(crate) unstable: bool, pub(crate) windows_powershell: bool, pub(crate) windows_shell: Option>, - pub(crate) working_directory: Option, + pub(crate) working_directory: Option, } impl Settings { diff --git a/src/shell_kind.rs b/src/shell_kind.rs index 2020fa8bdb..89375a4007 100644 --- a/src/shell_kind.rs +++ b/src/shell_kind.rs @@ -36,7 +36,7 @@ impl From<&str> for ShellKind { impl From<&Command> for ShellKind { fn from(command: &Command) -> Self { - let Some(command) = Path::new(command.get_program()) + let Some(command) = std::path::Path::new(command.get_program()) .file_name() .and_then(OsStr::to_str) else { diff --git a/src/source.rs b/src/source.rs index 76192aa31e..eb179615b6 100644 --- a/src/source.rs +++ b/src/source.rs @@ -3,15 +3,15 @@ use super::*; #[derive(Debug)] pub(crate) struct Source<'src> { pub(crate) file_depth: u32, - pub(crate) file_path: Vec, + pub(crate) file_path: Vec, pub(crate) import_offsets: Vec, pub(crate) namepath: Option>, - pub(crate) path: PathBuf, - pub(crate) working_directory: PathBuf, + pub(crate) path: Utf8PathBuf, + pub(crate) working_directory: Utf8PathBuf, } impl<'src> Source<'src> { - pub(crate) fn root(path: &Path) -> Self { + pub(crate) fn root(path: &Utf8Path) -> Self { Self { file_depth: 0, file_path: vec![path.into()], @@ -22,7 +22,7 @@ impl<'src> Source<'src> { } } - pub(crate) fn import(&self, path: PathBuf, import_offset: usize) -> Self { + pub(crate) fn import(&self, path: Utf8PathBuf, import_offset: usize) -> Self { Self { file_depth: self.file_depth + 1, file_path: self @@ -43,7 +43,7 @@ impl<'src> Source<'src> { } } - pub(crate) fn module(&self, name: Name<'src>, path: PathBuf) -> Self { + pub(crate) fn module(&self, name: Name<'src>, path: Utf8PathBuf) -> Self { Self { file_depth: self.file_depth + 1, file_path: self diff --git a/src/subcommand.rs b/src/subcommand.rs index b4615a87d2..9192e415be 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -16,14 +16,14 @@ const CHOOSER_CANCELLED_EXIT_STATUS: i32 = 130; pub(crate) enum Subcommand { Changelog, Choose { - chooser: Option, + chooser: Option, }, Clean { path: Option, }, Command { - arguments: Vec, - binary: OsString, + arguments: Vec, + binary: String, }, Completions { shell: Shell, @@ -208,10 +208,9 @@ impl Subcommand { .strip_prefix(search.justfile_parent()) .unwrap() .components() - .map(|_| path::Component::ParentDir) - .collect::() + .map(|_| Utf8Component::ParentDir) + .collect::() .join(search.justfile.file_name().unwrap()) - .display() ); } @@ -260,7 +259,7 @@ impl Subcommand { } fn choose<'src>( - chooser: Option<&Path>, + chooser: Option<&Utf8Path>, config: &Config, justfile: &Justfile<'src>, overrides: &HashMap, @@ -282,12 +281,12 @@ impl Subcommand { } let chooser = if let Some(chooser) = chooser { - OsString::from(chooser) + chooser.as_str().into() } else { - let mut chooser = OsString::new(); - chooser.push("fzf --multi --preview 'just --unstable --color always --justfile \""); - chooser.push(&search.justfile); - chooser.push("\" --show {}'"); + let mut chooser = String::new(); + chooser.push_str("fzf --multi --preview 'just --unstable --color always --justfile \""); + chooser.push_str(search.justfile.as_str()); + chooser.push_str("\" --show {}'"); chooser }; @@ -383,7 +382,8 @@ impl Subcommand { continue; } - let path = entry.path(); + let path = Utf8PathBuf::try_from(entry.path()) + .map_err(|source| Error::CacheEntryUnicode { source })?; if let Some(prefix) = prefix { let json = fs::read_to_string(&path).map_err(|source| Error::FilesystemIo { @@ -460,9 +460,13 @@ impl Subcommand { } fn edit(search: &Search) -> RunResult<'static> { - let editor = env::var_os("VISUAL") - .or_else(|| env::var_os("EDITOR")) - .unwrap_or_else(|| "vim".into()); + let editor = if let Some(visual) = env_var("VISUAL")? { + visual + } else if let Some(editor) = env_var("EDITOR")? { + editor + } else { + "vim".into() + }; let error = Command::resolve(&editor) .current_dir(&search.working_directory) @@ -484,7 +488,7 @@ impl Subcommand { fn format<'src>(config: &Config, loader: &'src Loader, search: &Search) -> RunResult<'src> { let root = search.justfile_parent(); - let (path, src) = loader.load(config, root, &search.justfile)?; + let (path, src) = loader.load(root, &search.justfile)?; let ast = Parser::parse_source( &mut Numerator::new(), @@ -539,7 +543,7 @@ impl Subcommand { })?; if config.verbosity.loud() { - eprintln!("wrote justfile to `{}`", search.justfile.display()); + eprintln!("wrote justfile to `{}`", search.justfile); } } @@ -564,7 +568,7 @@ impl Subcommand { } if config.verbosity.loud() { - eprintln!("wrote justfile to `{}`", search.justfile.display()); + eprintln!("wrote justfile to `{}`", search.justfile); } Ok(()) diff --git a/src/token.rs b/src/token.rs index 2f3db89f3f..dbd534d00c 100644 --- a/src/token.rs +++ b/src/token.rs @@ -7,7 +7,7 @@ pub(crate) struct Token<'src> { pub(crate) length: usize, pub(crate) line: usize, pub(crate) offset: usize, - pub(crate) path: &'src Path, + pub(crate) path: &'src Utf8Path, pub(crate) src: &'src str, } @@ -57,7 +57,7 @@ impl ColorDisplay for Token<'_> { "{:width$}{} {}:{}:{}", "", color.context().paint("——▶"), - self.path.display(), + self.path, line_number, self.column.ordinal(), width = line_number_width diff --git a/src/which.rs b/src/which.rs index 14a8b0e98b..78a5a9ad54 100644 --- a/src/which.rs +++ b/src/which.rs @@ -1,14 +1,17 @@ use super::*; pub(crate) fn which(context: &function::Context, name: &str) -> Result, String> { - let name = Path::new(name); + let name = Utf8Path::new(name); let paths = match name.components().count() { 0 => return Err("empty command".into()), 1 => { + let path = env::var("PATH") + .map_err(|source| format!("failed to retrieve `PATH` environment variable: {source}"))?; + // cmd is a regular command - env::split_paths(&env::var_os("PATH").ok_or("`PATH` environment variable not set")?) - .map(|path| path.join(name)) + env::split_paths(&path) + .map(|path| Utf8PathBuf::try_from(path).unwrap().join(name)) .collect() } _ => { @@ -50,15 +53,7 @@ pub(crate) fn which(context: &function::Context, name: &str) -> Result Date: Mon, 21 Sep 2026 15:31:34 -0700 Subject: [PATCH 2/9] Add PathBufExt --- src/dir.rs | 10 +++++++--- src/lib.rs | 2 ++ src/path_buf_ext.rs | 11 +++++++++++ src/search.rs | 11 ++++++++--- src/subcommand.rs | 4 +++- src/which.rs | 2 +- 6 files changed, 32 insertions(+), 8 deletions(-) create mode 100644 src/path_buf_ext.rs diff --git a/src/dir.rs b/src/dir.rs index f566f4d564..d851d18278 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -2,7 +2,7 @@ use super::*; pub(crate) fn config_directory() -> PathResult> { dirs::config_dir() - .map(|path| Utf8PathBuf::try_from(path).context(path_error::ConfigDirectoryUnicode)) + .map(|path| path.into_utf8().context(path_error::ConfigDirectoryUnicode)) .transpose() } @@ -17,7 +17,7 @@ pub(crate) fn current_directory() -> PathResult { pub(crate) fn home_directory() -> PathResult> { dirs::home_dir() - .map(|path| Utf8PathBuf::try_from(path).context(path_error::HomeDirectoryUnicode)) + .map(|path| path.into_utf8().context(path_error::HomeDirectoryUnicode)) .transpose() } @@ -27,7 +27,11 @@ pub(crate) fn home_directory_required() -> PathResult { pub(crate) fn runtime_directory() -> PathResult> { dirs::runtime_dir() - .map(|path| Utf8PathBuf::try_from(path).context(path_error::RuntimeDirectoryUnicode)) + .map(|path| { + path + .into_utf8() + .context(path_error::RuntimeDirectoryUnicode) + }) .transpose() } diff --git a/src/lib.rs b/src/lib.rs index c1ee266201..5e768e8539 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -88,6 +88,7 @@ pub(crate) use { parameter::Parameter, parameter_kind::ParameterKind, parser::Parser, + path_buf_ext::PathBufExt, path_error::PathError, pattern::Pattern, platform::Platform, @@ -327,6 +328,7 @@ mod output_error; mod parameter; mod parameter_kind; mod parser; +mod path_buf_ext; mod path_error; mod pattern; mod platform; diff --git a/src/path_buf_ext.rs b/src/path_buf_ext.rs new file mode 100644 index 0000000000..668d6b6e54 --- /dev/null +++ b/src/path_buf_ext.rs @@ -0,0 +1,11 @@ +use super::*; + +pub(crate) trait PathBufExt { + fn into_utf8(self) -> Result; +} + +impl PathBufExt for std::path::PathBuf { + fn into_utf8(self) -> Result { + Utf8PathBuf::try_from(self) + } +} diff --git a/src/search.rs b/src/search.rs index 40f4af2ec6..715bcae14b 100644 --- a/src/search.rs +++ b/src/search.rs @@ -159,8 +159,10 @@ impl Search { path: directory.clone(), })?; - let candidate = - Utf8PathBuf::try_from(entry.path()).context(search_error::CandidateUnicode)?; + let candidate = entry + .path() + .into_utf8() + .context(search_error::CandidateUnicode)?; if candidate .file_name() @@ -264,7 +266,10 @@ impl Search { path: directory.to_owned(), })?; - let entry = Utf8PathBuf::try_from(entry.path()).context(search_error::CandidateUnicode)?; + let entry = entry + .path() + .into_utf8() + .context(search_error::CandidateUnicode)?; let name = entry.file_name().unwrap(); diff --git a/src/subcommand.rs b/src/subcommand.rs index 9192e415be..8d24150a08 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -382,7 +382,9 @@ impl Subcommand { continue; } - let path = Utf8PathBuf::try_from(entry.path()) + let path = entry + .path() + .into_utf8() .map_err(|source| Error::CacheEntryUnicode { source })?; if let Some(prefix) = prefix { diff --git a/src/which.rs b/src/which.rs index 78a5a9ad54..734c003e07 100644 --- a/src/which.rs +++ b/src/which.rs @@ -11,7 +11,7 @@ pub(crate) fn which(context: &function::Context, name: &str) -> Result { From 68d3f883da2ad31d314cbf5912c306263bc60049 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 21 Sep 2026 16:05:13 -0700 Subject: [PATCH 3/9] Fix tests --- src/analyzer.rs | 2 +- src/command_ext.rs | 2 +- src/compiler.rs | 30 +++++++++++++++--------------- src/config.rs | 26 +++++++++++++------------- src/invocation_parser.rs | 36 +++++++++++++++++++++--------------- src/platform/windows.rs | 5 +---- src/recipe_resolver.rs | 2 +- src/search_error.rs | 6 +++--- src/testing.rs | 6 +++--- src/value.rs | 2 +- tests/non_unicode.rs | 37 +++++++------------------------------ 11 files changed, 67 insertions(+), 87 deletions(-) diff --git a/src/analyzer.rs b/src/analyzer.rs index acd8de99d3..f07af74850 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -612,7 +612,7 @@ mod tests { length: 3, line: 0, offset: 13, - path: Path::new("justfile"), + path: Utf8Path::new("justfile"), src: "alias foo := bar\n", } )) diff --git a/src/command_ext.rs b/src/command_ext.rs index 40d5bc4171..3317f65b23 100644 --- a/src/command_ext.rs +++ b/src/command_ext.rs @@ -94,7 +94,7 @@ impl CommandExt for Command { #[cfg(windows)] if ShellKind::from(&*self) == ShellKind::Cmd { use std::os::windows::process::CommandExt; - return self.raw_arg(arg); + return self.raw_arg(arg.as_ref()); } self.arg(arg.as_ref()) diff --git a/src/compiler.rs b/src/compiler.rs index 4b3b7bf267..bdbcd7a025 100644 --- a/src/compiler.rs +++ b/src/compiler.rs @@ -296,19 +296,20 @@ mod tests { #[test] fn recursive_includes_fail() { let tmp = tempfile::tempdir().unwrap(); - fs::write(tmp.path().join("justfile"), "import './subdir/b'\na: b").unwrap(); - fs::create_dir_all(tmp.path().join("subdir")).unwrap(); - fs::write(tmp.path().join("subdir/b"), "import '../justfile'\nb:").unwrap(); + let root = dir::temporary_directory(&tmp).unwrap(); + fs::write(root.join("justfile"), "import './subdir/b'\na: b").unwrap(); + fs::create_dir_all(root.join("subdir")).unwrap(); + fs::write(root.join("subdir/b"), "import '../justfile'\nb:").unwrap(); let loader = Loader::new(); - let justfile_a_path = tmp.path().join("justfile"); + let justfile_a_path = root.join("justfile"); let loader_output = Compiler::compile(&Config::new().unwrap(), &loader, &justfile_a_path).unwrap_err(); assert_matches!(loader_output, Error::CircularImport { current, import } - if current == tmp.path().join("subdir").join("b").clean() && - import == tmp.path().join("justfile").clean() + if current == root.join("subdir").join("b").clean() && + import == root.join("justfile").clean() ); } @@ -323,22 +324,23 @@ mod tests { length: 3, line: 0, offset: 0, - path: Path::new(""), + path: Utf8Path::new(""), src: "foo", }, }; let tempdir = tempfile::tempdir().unwrap(); + let root = dir::temporary_directory(&tempdir).unwrap(); for file in files { - if let Some(parent) = Path::new(file).parent() { - fs::create_dir_all(tempdir.path().join(parent)).unwrap(); + if let Some(parent) = Utf8Path::new(file).parent() { + fs::create_dir_all(root.join(parent)).unwrap(); } - fs::write(tempdir.path().join(file), "").unwrap(); + fs::write(root.join(file), "").unwrap(); } - let actual = Compiler::find_module_file(tempdir.path(), module, path.map(Path::new)); + let actual = Compiler::find_module_file(root, module, path.map(Utf8Path::new)); match expected { Err(expected) => match actual.unwrap_err() { @@ -348,16 +350,14 @@ mod tests { expected .iter() .map(|expected| expected.replace('/', std::path::MAIN_SEPARATOR_STR).into()) - .collect::>() + .collect::>() ); } _ => panic!("unexpected error"), }, Ok(Some(expected)) => assert_eq!( actual.unwrap().unwrap(), - tempdir - .path() - .join(expected.replace('/', std::path::MAIN_SEPARATOR_STR)) + root.join(expected.replace('/', std::path::MAIN_SEPARATOR_STR)) ), Ok(None) => assert_eq!(actual.unwrap(), None), } diff --git a/src/config.rs b/src/config.rs index f8f5e55dd2..607b91a1a0 100644 --- a/src/config.rs +++ b/src/config.rs @@ -954,7 +954,7 @@ mod tests { name: subcommand_list_search_directory, args: ["--list", ".."], search_config: SearchConfig::FromSearchDirectory { - search_directory: PathBuf::from(".."), + search_directory: Utf8PathBuf::from(".."), }, subcommand: Subcommand::List { path: Modulepath::default() }, } @@ -963,7 +963,7 @@ mod tests { name: subcommand_show_search_directory, args: ["--show", "../foo"], search_config: SearchConfig::FromSearchDirectory { - search_directory: PathBuf::from("../"), + search_directory: Utf8PathBuf::from("../"), }, subcommand: Subcommand::Show { path: Modulepath::try_from(["foo"].as_slice()).unwrap() }, } @@ -972,7 +972,7 @@ mod tests { name: subcommand_usage_search_directory, args: ["--usage", "foo/bar"], search_config: SearchConfig::FromSearchDirectory { - search_directory: PathBuf::from("foo/"), + search_directory: Utf8PathBuf::from("foo/"), }, subcommand: Subcommand::Usage { path: Modulepath::try_from(["bar"].as_slice()).unwrap() }, } @@ -1082,8 +1082,8 @@ mod tests { name: search_config_from_working_directory_and_justfile, args: ["--working-directory", "foo", "--justfile", "bar"], search_config: SearchConfig::WithJustfileAndWorkingDirectory { - justfile: PathBuf::from("bar"), - working_directory: PathBuf::from("foo"), + justfile: Utf8PathBuf::from("bar"), + working_directory: Utf8PathBuf::from("foo"), }, } @@ -1091,7 +1091,7 @@ mod tests { name: search_config_justfile_long, args: ["--justfile", "foo"], search_config: SearchConfig::WithJustfile { - justfile: PathBuf::from("foo"), + justfile: Utf8PathBuf::from("foo"), }, } @@ -1099,7 +1099,7 @@ mod tests { name: search_config_justfile_short, args: ["-f", "foo"], search_config: SearchConfig::WithJustfile { - justfile: PathBuf::from("foo"), + justfile: Utf8PathBuf::from("foo"), }, } @@ -1119,7 +1119,7 @@ mod tests { name: search_config_justfile_stdin_with_working_directory, args: ["--justfile", "-", "--working-directory", "foo"], search_config: SearchConfig::FromStandardInput { - working_directory: Some(PathBuf::from("foo")), + working_directory: Some(Utf8PathBuf::from("foo")), }, } @@ -1127,7 +1127,7 @@ mod tests { name: search_directory_parent, args: ["../"], search_config: SearchConfig::FromSearchDirectory { - search_directory: PathBuf::from(".."), + search_directory: Utf8PathBuf::from(".."), }, } @@ -1135,7 +1135,7 @@ mod tests { name: search_directory_parent_with_recipe, args: ["../build"], search_config: SearchConfig::FromSearchDirectory { - search_directory: PathBuf::from(".."), + search_directory: Utf8PathBuf::from(".."), }, subcommand: Subcommand::Run { arguments: vec!["build".to_owned()] }, } @@ -1144,7 +1144,7 @@ mod tests { name: search_directory_child, args: ["foo/"], search_config: SearchConfig::FromSearchDirectory { - search_directory: PathBuf::from("foo"), + search_directory: Utf8PathBuf::from("foo"), }, } @@ -1152,7 +1152,7 @@ mod tests { name: search_directory_deep, args: ["foo/bar/"], search_config: SearchConfig::FromSearchDirectory { - search_directory: PathBuf::from("foo/bar"), + search_directory: Utf8PathBuf::from("foo/bar"), }, } @@ -1160,7 +1160,7 @@ mod tests { name: search_directory_child_with_recipe, args: ["foo/build"], search_config: SearchConfig::FromSearchDirectory { - search_directory: PathBuf::from("foo"), + search_directory: Utf8PathBuf::from("foo"), }, subcommand: Subcommand::Run { arguments: vec!["build".to_owned()] }, } diff --git a/src/invocation_parser.rs b/src/invocation_parser.rs index 37da8a0278..aa450653ca 100644 --- a/src/invocation_parser.rs +++ b/src/invocation_parser.rs @@ -317,10 +317,16 @@ mod tests { use {super::*, tempfile::TempDir}; trait TempDirExt { + fn utf8_path(&self) -> &Utf8Path; + fn write(&self, path: &str, content: &str); } impl TempDirExt for TempDir { + fn utf8_path(&self) -> &Utf8Path { + Utf8Path::from_path(self.path()).unwrap() + } + fn write(&self, path: &str, content: &str) { let path = self.path().join(path); fs::create_dir_all(path.parent().unwrap()).unwrap(); @@ -396,10 +402,10 @@ mod tests { fn recipe_in_submodule() { let loader = Loader::new(); let tempdir = tempfile::tempdir().unwrap(); - let path = tempdir.path().join("justfile"); + let path = tempdir.utf8_path().join("justfile"); fs::write(&path, "mod foo").unwrap(); - fs::create_dir(tempdir.path().join("foo")).unwrap(); - fs::write(tempdir.path().join("foo/mod.just"), "bar:").unwrap(); + fs::create_dir(tempdir.utf8_path().join("foo")).unwrap(); + fs::write(tempdir.utf8_path().join("foo/mod.just"), "bar:").unwrap(); let compilation = Compiler::compile(&Config::new().unwrap(), &loader, &path).unwrap(); let invocations = @@ -420,7 +426,7 @@ mod tests { let compilation = Compiler::compile( &Config::new().unwrap(), &loader, - &tempdir.path().join("justfile"), + &tempdir.utf8_path().join("justfile"), ) .unwrap(); @@ -436,10 +442,10 @@ mod tests { fn recipe_in_submodule_unknown() { let loader = Loader::new(); let tempdir = tempfile::tempdir().unwrap(); - let path = tempdir.path().join("justfile"); + let path = tempdir.utf8_path().join("justfile"); fs::write(&path, "mod foo").unwrap(); - fs::create_dir(tempdir.path().join("foo")).unwrap(); - fs::write(tempdir.path().join("foo/mod.just"), "bar:").unwrap(); + fs::create_dir(tempdir.utf8_path().join("foo")).unwrap(); + fs::write(tempdir.utf8_path().join("foo/mod.just"), "bar:").unwrap(); let compilation = Compiler::compile(&Config::new().unwrap(), &loader, &path).unwrap(); assert_matches!( @@ -461,7 +467,7 @@ mod tests { let compilation = Compiler::compile( &Config::new().unwrap(), &loader, - &tempdir.path().join("justfile"), + &tempdir.utf8_path().join("justfile"), ) .unwrap(); @@ -483,7 +489,7 @@ mod tests { let compilation = Compiler::compile( &Config::new().unwrap(), &loader, - &tempdir.path().join("justfile"), + &tempdir.utf8_path().join("justfile"), ) .unwrap(); @@ -503,7 +509,7 @@ mod tests { let compilation = Compiler::compile( &Config::new().unwrap(), &loader, - &tempdir.path().join("justfile"), + &tempdir.utf8_path().join("justfile"), ) .unwrap(); @@ -524,7 +530,7 @@ mod tests { let compilation = Compiler::compile( &Config::new().unwrap(), &loader, - &tempdir.path().join("justfile"), + &tempdir.utf8_path().join("justfile"), ) .unwrap(); @@ -543,7 +549,7 @@ mod tests { let compilation = Compiler::compile( &Config::new().unwrap(), &loader, - &tempdir.path().join("justfile"), + &tempdir.utf8_path().join("justfile"), ) .unwrap(); @@ -566,7 +572,7 @@ mod tests { let compilation = Compiler::compile( &Config::new().unwrap(), &loader, - &tempdir.path().join("justfile"), + &tempdir.utf8_path().join("justfile"), ) .unwrap(); @@ -699,7 +705,7 @@ foo baz qux='qux' bar='bar': let compilation = Compiler::compile( &Config::new().unwrap(), &loader, - &tempdir.path().join("justfile"), + &tempdir.utf8_path().join("justfile"), ) .unwrap(); @@ -721,7 +727,7 @@ foo baz qux='qux' bar='bar': let compilation = Compiler::compile( &Config::new().unwrap(), &loader, - &tempdir.path().join("justfile"), + &tempdir.utf8_path().join("justfile"), ) .unwrap(); diff --git a/src/platform/windows.rs b/src/platform/windows.rs index c06249a98f..b0683abcf7 100644 --- a/src/platform/windows.rs +++ b/src/platform/windows.rs @@ -75,10 +75,7 @@ impl PlatformInterface for Platform { match cygpath.output_guard_stdout() { Ok(shell_path) => Ok(shell_path), - Err(_) => path - .to_str() - .map(str::to_string) - .ok_or_else(|| String::from("Error getting current directory: unicode decode error")), + Err(_) => Ok(path.as_str().into()), } } diff --git a/src/recipe_resolver.rs b/src/recipe_resolver.rs index 2af6ff96d2..a7d139d439 100644 --- a/src/recipe_resolver.rs +++ b/src/recipe_resolver.rs @@ -180,7 +180,7 @@ mod tests { length: 1, line: 0, offset: 3, - path: Path::new("justfile"), + path: Utf8Path::new("justfile"), src: "a: b" })) }, } diff --git a/src/search_error.rs b/src/search_error.rs index 1313fa6af5..e06b8d5a25 100644 --- a/src/search_error.rs +++ b/src/search_error.rs @@ -45,9 +45,9 @@ mod tests { #[test] fn multiple_candidates_formatting() { let error = SearchError::MultipleCandidates { - candidates: [Path::new("/foo/justfile"), Path::new("/foo/JUSTFILE")] - .iter() - .map(|path| path.to_path_buf()) + candidates: ["/foo/justfile", "/foo/JUSTFILE"] + .into_iter() + .map(Utf8PathBuf::from) .collect(), }; diff --git a/src/testing.rs b/src/testing.rs index 3ed1677bf9..bb633f4218 100644 --- a/src/testing.rs +++ b/src/testing.rs @@ -61,11 +61,11 @@ pub(crate) fn analysis_error( let ast = Parser::parse_tokens(&mut Numerator::new(), &tokens) .expect("Parsing failed in analysis test..."); - let root = PathBuf::from("justfile"); - let mut asts: HashMap<(Modulepath, PathBuf), Ast> = HashMap::new(); + let root = Utf8PathBuf::from("justfile"); + let mut asts: HashMap<(Modulepath, Utf8PathBuf), Ast> = HashMap::new(); asts.insert((Modulepath::default(), root.clone()), ast); - let mut paths: HashMap = HashMap::new(); + let mut paths: HashMap = HashMap::new(); paths.insert("justfile".into(), "justfile".into()); match Analyzer::analyze( diff --git a/src/value.rs b/src/value.rs index 8eb6ff429a..ee62b3eb38 100644 --- a/src/value.rs +++ b/src/value.rs @@ -248,7 +248,7 @@ mod tests { length: 0, line: 0, offset: 0, - path: Path::new(""), + path: Utf8Path::new(""), src: "", } ) diff --git a/tests/non_unicode.rs b/tests/non_unicode.rs index e03427ac05..7ffdd8f095 100644 --- a/tests/non_unicode.rs +++ b/tests/non_unicode.rs @@ -1,39 +1,16 @@ use {super::*, std::os::unix::ffi::OsStrExt}; -fn non_unicode_dir_name() -> &'static std::ffi::OsStr { - std::ffi::OsStr::from_bytes(b"foo\xff") -} - -#[test] -fn warn_for_non_unicode_invocation_directory() { - let tempdir = tempdir(); - let dir = tempdir.path().join(non_unicode_dir_name()); - fs::create_dir(&dir).unwrap(); - fs::write(dir.join("justfile"), "default:\n\ttrue\n").unwrap(); - - Test::with_tempdir(tempdir) - .current_dir(non_unicode_dir_name()) - .stderr_regex( - ".*The invocation directory path `[^`]+` is not Unicode\\. Just is considering phasing-out \ - support for non-Unicode paths\\. If you see this warning, please leave a comment on \ - https://github\\.com/casey/just/issues/3229\\. Thank you!.*", - ) - .success(); -} - #[test] -fn warn_for_non_unicode_justfile_path() { +fn non_unicode_invocation_directory_is_an_error() { + let dir = std::ffi::OsStr::from_bytes(b"foo\xff"); let tempdir = tempdir(); - let dir = tempdir.path().join(non_unicode_dir_name()); - fs::create_dir(&dir).unwrap(); - fs::write(dir.join("justfile"), "default:\n\ttrue\n").unwrap(); + fs::create_dir(tempdir.path().join(dir)).unwrap(); Test::with_tempdir(tempdir) - .current_dir(non_unicode_dir_name()) + .current_dir(dir) .stderr_regex( - ".*The justfile path `[^`]+` is not Unicode\\. Just is considering phasing-out support for \ - non-Unicode paths\\. If you see this warning, please leave a comment on \ - https://github\\.com/casey/just/issues/3229\\. Thank you!.*", + "^error: current directory is not valid unicode: PathBuf contains invalid UTF-8: \ + .*/foo\u{FFFD}\n$", ) - .success(); + .failure(); } From ebaa03e0b9e33fd1e501fdcf3bd67b8aefdb5a6b Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 21 Sep 2026 16:06:36 -0700 Subject: [PATCH 4/9] Fix clippy lints --- src/dir.rs | 10 ++++------ src/error.rs | 12 ++++++------ src/lib.rs | 10 ++++------ 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/src/dir.rs b/src/dir.rs index d851d18278..0346c8e0c0 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -7,12 +7,10 @@ pub(crate) fn config_directory() -> PathResult> { } pub(crate) fn current_directory() -> PathResult { - Ok( - env::current_dir() - .context(path_error::CurrentDirectoryIo)? - .try_into() - .context(path_error::CurrentDirectoryUnicode)?, - ) + env::current_dir() + .context(path_error::CurrentDirectoryIo)? + .try_into() + .context(path_error::CurrentDirectoryUnicode) } pub(crate) fn home_directory() -> PathResult> { diff --git a/src/error.rs b/src/error.rs index f8ced9d2d7..68bcc6d0dc 100644 --- a/src/error.rs +++ b/src/error.rs @@ -532,7 +532,7 @@ impl ColorDisplay for Error<'_> { AmbiguousModuleFile { module, found } => write!( f, "found multiple source files for module `{module}`: {}", - List::and_ticked(found.iter().map(|path| path)), + List::and_ticked(found.iter()), )?, ArgumentPatternMismatch { argument, @@ -601,13 +601,13 @@ impl ColorDisplay for Error<'_> { )?, }, CacheEntryRead { path, source } => { - write!(f, "failed to read cache entry at `{path}`: {source}",)? + write!(f, "failed to read cache entry at `{path}`: {source}")?; } CacheEntryUnicode { source } => { - write!(f, "cache entry path is not valid unicode: {source}",)? + write!(f, "cache entry path is not valid unicode: {source}")?; } CacheEntryWrite { path, source } => { - write!(f, "failed to write cache entry at `{path}`: {source}",)? + write!(f, "failed to write cache entry at `{path}`: {source}")?; } CacheInputDirectory { path } => { write!(f, "cache input is directory: `{path}`")?; @@ -891,7 +891,7 @@ impl ColorDisplay for Error<'_> { }?; } Load { io_error, path } => { - write!(f, "failed to read justfile at `{path}`: {io_error}",)?; + write!(f, "failed to read justfile at `{path}`: {io_error}")?; } NonFinalOptionWithValue { recipe, switch } => { write!( @@ -968,7 +968,7 @@ impl ColorDisplay for Error<'_> { )?, RegexCompile { source, .. } => write!(f, "{source}")?, RuntimeDirIo { io_error, path } => { - write!(f, "I/O error in runtime dir `{path}`: {io_error}",)?; + write!(f, "I/O error in runtime dir `{path}`: {io_error}")?; } Script { command, diff --git a/src/lib.rs b/src/lib.rs index 5e768e8539..2b8e86b0ff 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -217,12 +217,10 @@ const VERSION: &str = env!("CARGO_PKG_VERSION"); fn env_var(name: &str) -> RunResult<'static, Option> { match env::var(name) { Err(env::VarError::NotPresent) => Ok(None), - Err(env::VarError::NotUnicode(value)) => { - return Err(Error::EnvVarUnicode { - name: name.into(), - value, - }); - } + Err(env::VarError::NotUnicode(value)) => Err(Error::EnvVarUnicode { + name: name.into(), + value, + }), Ok(value) => Ok(Some(value)), } } From a79bdfa22d294f0171db56b6a67923c26abcdc28 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 21 Sep 2026 16:16:27 -0700 Subject: [PATCH 5/9] Address review --- src/search.rs | 29 +++++++++++------------------ src/search_error.rs | 2 -- 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/src/search.rs b/src/search.rs index 715bcae14b..ff428dadc5 100644 --- a/src/search.rs +++ b/src/search.rs @@ -159,17 +159,12 @@ impl Search { path: directory.clone(), })?; - let candidate = entry - .path() - .into_utf8() - .context(search_error::CandidateUnicode)?; - - if candidate - .file_name() - .unwrap() - .eq_ignore_ascii_case(filename) - { - return Ok(candidate); + let Ok(path) = entry.path().into_utf8() else { + continue; + }; + + if path.file_name().unwrap().eq_ignore_ascii_case(filename) { + return Ok(path); } } } @@ -266,12 +261,9 @@ impl Search { path: directory.to_owned(), })?; - let entry = entry - .path() - .into_utf8() - .context(search_error::CandidateUnicode)?; - - let name = entry.file_name().unwrap(); + let Ok(path) = entry.path().into_utf8() else { + continue; + }; let mut justfile_names: Box> = if let Some(justfile_names) = &config.justfile_names { @@ -280,8 +272,9 @@ impl Search { Box::new(JUSTFILE_NAMES.into_iter()) }; + let name = path.file_name().unwrap(); if justfile_names.any(|justfile_name| name.eq_ignore_ascii_case(justfile_name)) { - candidates.insert(entry); + candidates.insert(path); } } diff --git a/src/search_error.rs b/src/search_error.rs index e06b8d5a25..f26a82c0df 100644 --- a/src/search_error.rs +++ b/src/search_error.rs @@ -3,8 +3,6 @@ use super::*; #[derive(Debug, Snafu)] #[snafu(visibility(pub(crate)), context(suffix(false)))] pub(crate) enum SearchError { - #[snafu(display("justfile candidate path is not valid unicode: {source}",))] - CandidateUnicode { source: FromPathBufError }, #[snafu(display("I/O error at `{path}`: {io_error}",))] FilesystemIo { io_error: io::Error, From b9293712602f728331cf14691ce99bbdb20a7726 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 21 Sep 2026 16:19:59 -0700 Subject: [PATCH 6/9] Test that non-unicode siblings are ignored --- src/function.rs | 12 +++--------- tests/non_unicode.rs | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/function.rs b/src/function.rs index 5ef89dfda3..9461d3bd4e 100644 --- a/src/function.rs +++ b/src/function.rs @@ -276,15 +276,9 @@ fn clean(_context: Context, path: &str) -> StringResult { fn dir(name: &'static str, f: fn() -> Option) -> StringResult { match f() { Some(path) => path - .as_os_str() - .to_str() - .map(str::to_string) - .ok_or_else(|| { - format!( - "unable to convert {name} directory path to string: {}", - path.display(), - ) - }), + .into_utf8() + .map(|path| path.into_string()) + .map_err(|source| format!("unable to convert {name} directory path to string: `{source}`")), None => Err(format!("{name} directory not found")), } } diff --git a/tests/non_unicode.rs b/tests/non_unicode.rs index 7ffdd8f095..9e65a6ecfe 100644 --- a/tests/non_unicode.rs +++ b/tests/non_unicode.rs @@ -14,3 +14,19 @@ fn non_unicode_invocation_directory_is_an_error() { ) .failure(); } + +#[test] +fn non_unicode_sibling_files_are_ignored() { + let tempdir = tempdir(); + fs::write( + tempdir.path().join(std::ffi::OsStr::from_bytes(b"foo\xff")), + "", + ) + .unwrap(); + + Test::with_tempdir(tempdir) + .justfile("foo:\n echo bar") + .stdout("bar\n") + .stderr("echo bar\n") + .success(); +} From 109646941e48e2cc8219fe792a93291ad96083f0 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 21 Sep 2026 16:23:22 -0700 Subject: [PATCH 7/9] Placate clippy --- src/function.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/function.rs b/src/function.rs index 9461d3bd4e..c9ca5c1ea6 100644 --- a/src/function.rs +++ b/src/function.rs @@ -277,7 +277,7 @@ fn dir(name: &'static str, f: fn() -> Option) -> StringResul match f() { Some(path) => path .into_utf8() - .map(|path| path.into_string()) + .map(Utf8PathBuf::into_string) .map_err(|source| format!("unable to convert {name} directory path to string: `{source}`")), None => Err(format!("{name} directory not found")), } From 8f9edf5adda444e623129ed5a66252b1270d3cd9 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 21 Sep 2026 16:25:05 -0700 Subject: [PATCH 8/9] Fix test --- src/function.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/function.rs b/src/function.rs index c9ca5c1ea6..750ae3f811 100644 --- a/src/function.rs +++ b/src/function.rs @@ -278,7 +278,7 @@ fn dir(name: &'static str, f: fn() -> Option) -> StringResul Some(path) => path .into_utf8() .map(Utf8PathBuf::into_string) - .map_err(|source| format!("unable to convert {name} directory path to string: `{source}`")), + .map_err(|source| format!("unable to convert {name} directory path to string: {source}")), None => Err(format!("{name} directory not found")), } } @@ -877,7 +877,7 @@ mod tests { use std::os::unix::ffi::OsStrExt; assert_eq!( dir("foo", || Some(OsStr::from_bytes(b"\xe0\x80\x80").into())).unwrap_err(), - "unable to convert foo directory path to string: ���", + "unable to convert foo directory path to string: PathBuf contains invalid UTF-8: ���", ); } } From 4276d783cf91fc663f7f98b8c9f3566478a36061 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 21 Sep 2026 16:42:02 -0700 Subject: [PATCH 9/9] Review --- src/error.rs | 6 ------ src/function.rs | 43 +++++++++++++++++++++++---------------- src/path_error.rs | 8 ++++---- src/platform/unix.rs | 4 ++-- src/platform/windows.rs | 10 +++------ src/platform_interface.rs | 6 +----- src/search_error.rs | 2 +- src/subcommand.rs | 11 +++++----- tests/non_unicode.rs | 5 +---- 9 files changed, 42 insertions(+), 53 deletions(-) diff --git a/src/error.rs b/src/error.rs index 68bcc6d0dc..859b9d62eb 100644 --- a/src/error.rs +++ b/src/error.rs @@ -40,9 +40,6 @@ pub(crate) enum Error<'src> { path: Utf8PathBuf, source: serde_json::Error, }, - CacheEntryUnicode { - source: FromPathBufError, - }, CacheEntryWrite { path: Utf8PathBuf, source: serde_json::Error, @@ -603,9 +600,6 @@ impl ColorDisplay for Error<'_> { CacheEntryRead { path, source } => { write!(f, "failed to read cache entry at `{path}`: {source}")?; } - CacheEntryUnicode { source } => { - write!(f, "cache entry path is not valid unicode: {source}")?; - } CacheEntryWrite { path, source } => { write!(f, "failed to write cache entry at `{path}`: {source}")?; } diff --git a/src/function.rs b/src/function.rs index 750ae3f811..2c489abe9f 100644 --- a/src/function.rs +++ b/src/function.rs @@ -72,15 +72,15 @@ pub(crate) fn get(name: &str) -> Option { "blake3" => Unary(blake3), "blake3_file" => Unary(blake3_file), "bool" => ValueUnary(bool), - "cache_directory" => Nullary(|_| dir("cache", dirs::cache_dir)), + "cache_directory" => Nullary(|_| dir_function("cache", dirs::cache_dir)), "canonicalize" => Unary(canonicalize), "capitalize" => Unary(capitalize), "choose" => Binary(choose), "clean" => Unary(clean), - "config_directory" => Nullary(|_| dir("config", dirs::config_dir)), - "config_local_directory" => Nullary(|_| dir("local config", dirs::config_local_dir)), - "data_directory" => Nullary(|_| dir("data", dirs::data_dir)), - "data_local_directory" => Nullary(|_| dir("local data", dirs::data_local_dir)), + "config_directory" => Nullary(|_| dir_function("config", dirs::config_dir)), + "config_local_directory" => Nullary(|_| dir_function("local config", dirs::config_local_dir)), + "data_directory" => Nullary(|_| dir_function("data", dirs::data_dir)), + "data_local_directory" => Nullary(|_| dir_function("local data", dirs::data_local_dir)), "datetime" => Unary(datetime), "datetime_utc" => Unary(datetime_utc), "encode_uri_component" => Unary(encode_uri_component), @@ -88,11 +88,11 @@ pub(crate) fn get(name: &str) -> Option { "env_var" => ValueUnary(env_var), "env_var_or_default" => ValueBinary(env_var_or_default), "error" => Unary(error), - "executable_directory" => Nullary(|_| dir("executable", dirs::executable_dir)), + "executable_directory" => Nullary(|_| dir_function("executable", dirs::executable_dir)), "extension" => Unary(extension), "file_name" => Unary(file_name), "file_stem" => Unary(file_stem), - "home_directory" => Nullary(|_| dir("home", dirs::home_dir)), + "home_directory" => Nullary(|_| dir_function("home", dirs::home_dir)), "invocation_directory" => Nullary(invocation_directory), "invocation_directory_native" => Nullary(invocation_directory_native), "is_dependency" => ValueNullary(is_dependency), @@ -123,7 +123,7 @@ pub(crate) fn get(name: &str) -> Option { "replace" => Ternary(replace), "replace_regex" => Ternary(replace_regex), "require" => Unary(require), - "runtime_directory" => Nullary(|_| dir("runtime", dirs::runtime_dir)), + "runtime_directory" => Nullary(|_| dir_function("runtime", dirs::runtime_dir)), "semver_matches" => BinaryToValue(semver_matches), "sha256" => Unary(sha256), "sha256_file" => Unary(sha256_file), @@ -227,7 +227,7 @@ fn canonicalize(context: Context, path: &str) -> StringResult { canonical.to_str().map(str::to_string).ok_or_else(|| { format!( - "canonical path is not valid Unicode: {}", + "canonical path is not valid Unicode: `{}`", canonical.display(), ) }) @@ -273,12 +273,17 @@ fn clean(_context: Context, path: &str) -> StringResult { Ok(Utf8Path::new(path).clean().into()) } -fn dir(name: &'static str, f: fn() -> Option) -> StringResult { +fn dir_function(name: &'static str, f: fn() -> Option) -> StringResult { match f() { Some(path) => path .into_utf8() .map(Utf8PathBuf::into_string) - .map_err(|source| format!("unable to convert {name} directory path to string: {source}")), + .map_err(|source| { + format!( + "{name} directory is not valid unicode: `{}`", + source.as_path().display(), + ) + }), None => Err(format!("{name} directory not found")), } } @@ -372,12 +377,11 @@ fn file_stem(_context: Context, path: &str) -> StringResult { } fn invocation_directory(context: Context) -> StringResult { - Platform::convert_native_path( + Ok(Platform::convert_native_path( context.execution_context.config, &context.execution_context.search.working_directory, &context.execution_context.config.invocation_directory, - ) - .map_err(|e| format!("could not convert invocation directory to shell path: {e}")) + )) } fn invocation_directory_native(context: Context) -> StringResult { @@ -428,7 +432,7 @@ fn just_executable(_context: Context) -> StringResult { exe_path.to_str().map(str::to_owned).ok_or_else(|| { format!( - "executable path is not valid Unicode: {}", + "executable path is not valid Unicode: `{}`", exe_path.display() ) }) @@ -868,7 +872,10 @@ mod tests { #[test] fn dir_not_found() { - assert_eq!(dir("foo", || None).unwrap_err(), "foo directory not found"); + assert_eq!( + dir_function("foo", || None).unwrap_err(), + "foo directory not found" + ); } #[cfg(unix)] @@ -876,8 +883,8 @@ mod tests { fn dir_not_unicode() { use std::os::unix::ffi::OsStrExt; assert_eq!( - dir("foo", || Some(OsStr::from_bytes(b"\xe0\x80\x80").into())).unwrap_err(), - "unable to convert foo directory path to string: PathBuf contains invalid UTF-8: ���", + dir_function("foo", || Some(OsStr::from_bytes(b"\xe0\x80\x80").into())).unwrap_err(), + "foo directory is not valid unicode: `���`", ); } } diff --git a/src/path_error.rs b/src/path_error.rs index 09ecfb62bb..11aa5fedc5 100644 --- a/src/path_error.rs +++ b/src/path_error.rs @@ -3,17 +3,17 @@ use super::*; #[derive(Debug, Snafu)] #[snafu(visibility(pub(crate)), context(suffix(false)))] pub(crate) enum PathError { - #[snafu(display("config directory is not valid unicode: {source}"))] + #[snafu(display("config directory is not valid unicode: `{}`", source.as_path().display()))] ConfigDirectoryUnicode { source: FromPathBufError }, #[snafu(display("I/O error retrieving current directory: {source}"))] CurrentDirectoryIo { source: io::Error }, - #[snafu(display("current directory is not valid unicode: {source}"))] + #[snafu(display("current directory is not valid unicode: `{}`", source.as_path().display()))] CurrentDirectoryUnicode { source: FromPathBufError }, #[snafu(display("failed to get home directory"))] HomeDirectoryMissing, - #[snafu(display("home directory is not valid unicode: {source}"))] + #[snafu(display("home directory is not valid unicode: `{}`", source.as_path().display()))] HomeDirectoryUnicode { source: FromPathBufError }, - #[snafu(display("runtime directory is not valid unicode: {source}"))] + #[snafu(display("runtime directory is not valid unicode: `{}`", source.as_path().display()))] RuntimeDirectoryUnicode { source: FromPathBufError }, #[snafu(display("temporary directory is not valid unicode: `{}`", path.display()))] TemporaryDirectoryUnicode { path: std::path::PathBuf }, diff --git a/src/platform/unix.rs b/src/platform/unix.rs index 7f0fc7e029..ecd7546bd0 100644 --- a/src/platform/unix.rs +++ b/src/platform/unix.rs @@ -40,8 +40,8 @@ impl PlatformInterface for Platform { _config: &Config, _working_directory: &Utf8Path, path: &Utf8Path, - ) -> StringResult { - Ok(path.as_str().into()) + ) -> String { + path.as_str().into() } fn install_signal_handler(handler: T) -> RunResult<'static> { diff --git a/src/platform/windows.rs b/src/platform/windows.rs index b0683abcf7..90f6725517 100644 --- a/src/platform/windows.rs +++ b/src/platform/windows.rs @@ -57,11 +57,7 @@ impl PlatformInterface for Platform { None } - fn convert_native_path( - config: &Config, - working_directory: &Utf8Path, - path: &Utf8Path, - ) -> StringResult { + fn convert_native_path(config: &Config, working_directory: &Utf8Path, path: &Utf8Path) -> String { // Translate path from windows style to unix style let mut cygpath = Command::resolve(&config.cygpath); @@ -74,8 +70,8 @@ impl PlatformInterface for Platform { .stderr(Stdio::piped()); match cygpath.output_guard_stdout() { - Ok(shell_path) => Ok(shell_path), - Err(_) => Ok(path.as_str().into()), + Ok(shell_path) => shell_path, + Err(_) => path.as_str().into(), } } diff --git a/src/platform_interface.rs b/src/platform_interface.rs index 0334b5b80e..6c4486ebd7 100644 --- a/src/platform_interface.rs +++ b/src/platform_interface.rs @@ -2,11 +2,7 @@ use super::*; pub(crate) trait PlatformInterface { /// translate path from "native" path to path interpreter expects - fn convert_native_path( - config: &Config, - working_directory: &Utf8Path, - path: &Utf8Path, - ) -> StringResult; + fn convert_native_path(config: &Config, working_directory: &Utf8Path, path: &Utf8Path) -> String; /// install handler, may only be called once fn install_signal_handler(handler: T) -> RunResult<'static>; diff --git a/src/search_error.rs b/src/search_error.rs index f26a82c0df..925b7ebed4 100644 --- a/src/search_error.rs +++ b/src/search_error.rs @@ -3,7 +3,7 @@ use super::*; #[derive(Debug, Snafu)] #[snafu(visibility(pub(crate)), context(suffix(false)))] pub(crate) enum SearchError { - #[snafu(display("I/O error at `{path}`: {io_error}",))] + #[snafu(display("I/O error at `{path}`: {io_error}"))] FilesystemIo { io_error: io::Error, path: Utf8PathBuf, diff --git a/src/subcommand.rs b/src/subcommand.rs index 8d24150a08..0e963f5ca1 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -378,14 +378,13 @@ impl Subcommand { for entry in dir { let entry = entry.map_err(context)?; - if !entry_re.is_match(&entry.file_name().to_string_lossy()) { + let Ok(path) = entry.path().into_utf8() else { continue; - } + }; - let path = entry - .path() - .into_utf8() - .map_err(|source| Error::CacheEntryUnicode { source })?; + if !entry_re.is_match(path.file_name().unwrap()) { + continue; + } if let Some(prefix) = prefix { let json = fs::read_to_string(&path).map_err(|source| Error::FilesystemIo { diff --git a/tests/non_unicode.rs b/tests/non_unicode.rs index 9e65a6ecfe..423b7f9f14 100644 --- a/tests/non_unicode.rs +++ b/tests/non_unicode.rs @@ -8,10 +8,7 @@ fn non_unicode_invocation_directory_is_an_error() { Test::with_tempdir(tempdir) .current_dir(dir) - .stderr_regex( - "^error: current directory is not valid unicode: PathBuf contains invalid UTF-8: \ - .*/foo\u{FFFD}\n$", - ) + .stderr_regex("^error: current directory is not valid unicode: `.*/foo\u{FFFD}`\n$") .failure(); }