reubeno / brush

bash/POSIX-compatible shell implemented in Rust
MIT License
26 stars 4 forks source link

feat: implement physical and logical cwd #219

Open 39555 opened 1 month ago

39555 commented 1 month ago

After diving into the system's paths rabbit hole for a while, I've implemented cross-platform cwd handling for the shell, along with support for cd -L -P and pwd -L -P.

Closes: #202

With this PR, the shell now maintains the logical current working directory alongside with the physical one. To correctly create an absolute path and expand all .. and . from the logical path, the functions have been added to the brush_core::sys::fs crate:

Posix notes:

There is a special handling for paths with double slashes, such as //users/home

If a pathname begins with two successive characters, the first component following the leading characters may be interpreted in an implementation-defined manner, although more than two leading characters shall be treated as a single character.

See: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_13

Windows notes

There is a lot of special handling for Windows weird path formats:

See: https://learn.microsoft.com/en-us/dotnet/standard/io/file-path-formats See: The book of windows file path: https://chrisdenton.github.io/omnipath/

I've written a lot of tests, trying to cover all the special cases.

github-actions[bot] commented 1 month ago

Performance Benchmark Report

Benchmark name Baseline (μs) Test/PR (μs) Delta (μs) Delta %
expand_one_string 3.42 μs 3.57 μs 0.15 μs 🟠 +4.27%
instantiate_shell 59.72 μs 70.61 μs 10.89 μs 🟠 +18.24%
instantiate_shell_with_init_scripts 31137.02 μs 30989.98 μs -147.04 μs ⚪ Unchanged
parse_bash_completion 2829.91 μs 2702.85 μs -127.05 μs ⚪ Unchanged
parse_sample_script 4.31 μs 4.16 μs -0.15 μs 🟢 -3.48%
run_echo_builtin_command 90.77 μs 102.49 μs 11.72 μs 🟠 +12.91%
run_one_builtin_command 109.48 μs 122.58 μs 13.10 μs 🟠 +11.97%
run_one_external_command 1991.67 μs 1956.48 μs -35.19 μs 🟢 -1.77%
run_one_external_command_directly 1017.00 μs 1017.80 μs 0.80 μs ⚪ Unchanged

Code Coverage Report: Only Changed Files listed

Package Base Coverage New Coverage Difference
brush-core/src/builtins/cd.rs 🟢 81.82% 🟢 82.61% 🟢 0.79%
brush-core/src/builtins/dirs.rs 🟢 85.71% 🟢 88.1% 🟢 2.39%
brush-core/src/builtins/pwd.rs 🟢 80.95% 🟢 83.33% 🟢 2.38%
brush-core/src/interp.rs 🟢 90.02% 🟢 90.01% 🔴 -0.01%
brush-core/src/patterns.rs 🟢 97.77% 🟢 97.78% 🟢 0.01%
brush-core/src/shell.rs 🟢 77.97% 🟢 80.03% 🟢 2.06%
brush-core/src/sys/fs.rs 🔴 0% 🟢 97.04% 🟢 97.04%
brush-shell/src/main.rs 🟢 90.2% 🟢 90.85% 🟢 0.65%
Overall Coverage 🟢 73.41% 🟢 74.13% 🟢 0.72%

Minimum allowed coverage is 70%, this run produced 74.13%

reubeno commented 1 month ago

Thanks for diving deep into this; I'll need a bit of time to review the changes here. Is there a reasonable way to break this up into multiple, smaller changes?

39555 commented 1 month ago

I don't think so, maybe cd and pwd, but they just call the new shell api Shell.set_current_working_dir_from_logical.

All changes are based on the new Cwd type instead of PathBuf for Shell.working_dir. Cwd contains both a physical PathBuf and a logical PathBuf. When we set a new cwd with Shell.set_current_working_dir_from_logical, the path should be normalized but without using syscalls (for preserving symlinks):

  1. expand tilde
  2. make it absolute based on our current physical or logical cwd
  3. expand dots

For startup cwd Inside Shell::new, we try to get the logical path from the PWD environment variable if it is present.

In other changed files, such as dirs or interp, I fixed the usage of working_dir to use the physical path for now. This should be addressed in other PRs if completion needs the logical path instead.