diff --git a/CHANGELOG.md b/CHANGELOG.md index b2f21ae2e..ead29b546 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ fish 3.0 is a major release which brings with it both improvements in functional - Range expansion (`$foo[1..5]`) will now always go forward if only the end is negative, and in reverse if just the start is. This is to enable clamping to the last valid index without changing direction if the list has fewer elements than expected. - Background jobs not first `disown`'d will be reaped upon `exec`, bringing the behavior in line with that of `exit`. - `read` now uses `-s` as short for `--silent` (à la `bash`); `--shell`'s abbreviation (formerly `-s`) is now `-S` instead (#4490). +- `cd` no longer resolves symlinks. fish now maintains a virtual path, matching other shells. (#3350). ## Notable fixes and improvements ### Syntax/semantic changes and new builtins diff --git a/doc_src/faq.hdr b/doc_src/faq.hdr index 133ac438c..96477099d 100644 --- a/doc_src/faq.hdr +++ b/doc_src/faq.hdr @@ -17,7 +17,6 @@ - Why doesn't `set -Ux` (exported universal variables) seem to work? - How do I customize my syntax highlighting colors? - How do I update man page completions? -- Why does cd, pwd and other fish commands always resolve symlinked directories to their canonical path? - I accidentally entered a directory path and fish changed directory. What happened? - The open command doesn't work. - How do I make fish my default shell? @@ -154,17 +153,6 @@ Use the web configuration tool, `fish_config Use the `fish_update_completions` command. -
-\section faq-cwd-symlink Why does cd, $PWD and and various fish commands always resolve symlinked directories to their canonical path? - -For example if `~/images` is a symlink to `~/Documents/Images`, if I write '`cd images`', my prompt will say `~/Documents/Images`, not `~/images`. - -Because it is impossible to consistently keep symlinked directories unresolved. It is indeed possible to do this partially, and many other shells do so. But it was felt there are enough serious corner cases that this is a bad idea. Most such issues have to do with how '..' is handled, and are variations of the following example: - -Writing `cd images; ls ..` given the above directory structure would list the contents of `~/Documents`, not of `~`, even though using `cd ..` changes the current directory to `~`, and the prompt, the `pwd` builtin and many other directory information sources suggest that the current directory is `~/images` and its parent is `~`. This issue is not possible to fix without either making every single command into a builtin, breaking Unix semantics or implementing kludges in every single command. This issue can also be seen when doing IO redirection. - -Another related issue is that many programs that operate on recursive directory trees, like the find command, silently ignore symlinked directories. For example, ```find $PWD -name '*.txt'``` silently fails in shells that don't resolve symlinked paths. -
\section faq-cd-implicit I accidentally entered a directory path and fish changed directory. What happened? diff --git a/doc_src/pwd.txt b/doc_src/pwd.txt index 46b6419f9..b3860498e 100644 --- a/doc_src/pwd.txt +++ b/doc_src/pwd.txt @@ -9,4 +9,8 @@ pwd `pwd` outputs (prints) the current working directory. -Note that `fish` always resolves symbolic links in the current directory path. +The following options are available: + +- `-L`, Output the logical working directory, without resolving symlinks (default behavior). + +- `-P`, Output the physical working directory, with symlinks resolved. diff --git a/share/functions/__fish_hg_prompt.fish b/share/functions/__fish_hg_prompt.fish index 21f6eaf7a..ee71d305f 100644 --- a/share/functions/__fish_hg_prompt.fish +++ b/share/functions/__fish_hg_prompt.fish @@ -30,7 +30,7 @@ function __fish_hg_prompt --description 'Write out the hg prompt' # Find an hg directory above $PWD # without calling `hg root` because that's too slow set -l root - set -l dir $PWD + set -l dir (pwd -P) while test $dir != "/" if test -f $dir'/.hg/dirstate' set root $dir"/.hg" diff --git a/src/builtin_cd.cpp b/src/builtin_cd.cpp index 98a4da3fe..b79350408 100644 --- a/src/builtin_cd.cpp +++ b/src/builtin_cd.cpp @@ -65,7 +65,11 @@ int builtin_cd(parser_t &parser, io_streams_t &streams, wchar_t **argv) { return STATUS_CMD_ERROR; } - if (wchdir(dir) != 0) { + // Prepend the PWD if we don't start with a slash, and then normalize the directory. + wcstring norm_dir = + normalize_path(string_prefixes_string(L"/", dir) ? dir : env_get_pwd_slash() + dir); + + if (wchdir(norm_dir) != 0) { struct stat buffer; int status; @@ -84,10 +88,6 @@ int builtin_cd(parser_t &parser, io_streams_t &streams, wchar_t **argv) { return STATUS_CMD_ERROR; } - if (!env_set_pwd()) { - streams.err.append_format(_(L"%ls: Could not set PWD variable\n"), cmd); - return STATUS_CMD_ERROR; - } - + env_set_one(L"PWD", ENV_EXPORT | ENV_GLOBAL, std::move(norm_dir)); return STATUS_CMD_OK; } diff --git a/src/builtin_pwd.cpp b/src/builtin_pwd.cpp index 79cea74e5..f32a783c7 100644 --- a/src/builtin_pwd.cpp +++ b/src/builtin_pwd.cpp @@ -6,35 +6,64 @@ #include "common.h" #include "fallback.h" // IWYU pragma: keep #include "io.h" +#include "wgetopt.h" #include "wutil.h" // IWYU pragma: keep -/// The pwd builtin. We don't respect -P to resolve symbolic links because we -/// try to always resolve them. +/// The pwd builtin. Respect -P to resolve symbolic links. Respect -L to not do that (the default). +static const wchar_t *short_options = L"LPh"; +static const struct woption long_options[] = {{L"help", no_argument, NULL, 'h'}, + {NULL, 0, NULL, 0}}; int builtin_pwd(parser_t &parser, io_streams_t &streams, wchar_t **argv) { UNUSED(parser); const wchar_t *cmd = argv[0]; int argc = builtin_count_args(argv); - help_only_cmd_opts_t opts; - - int optind; - int retval = parse_help_only_cmd_opts(opts, &optind, argc, argv, parser, streams); - if (retval != STATUS_CMD_OK) return retval; - - if (opts.print_help) { - builtin_print_help(parser, streams, cmd, streams.out); - return STATUS_CMD_OK; + bool resolve_symlinks = false; + wgetopter_t w; + int opt; + while ((opt = w.wgetopt_long(argc, argv, short_options, long_options, NULL)) != -1) { + switch (opt) { + case 'L': + resolve_symlinks = false; + break; + case 'P': + resolve_symlinks = true; + break; + case 'h': + builtin_print_help(parser, streams, cmd, streams.out); + return STATUS_CMD_OK; + case '?': { + builtin_unknown_option(parser, streams, cmd, argv[w.woptind - 1]); + return STATUS_INVALID_ARGS; + } + default: { + DIE("unexpected retval from wgetopt_long"); + break; + } + } } - if (optind != argc) { + if (w.woptind != argc) { streams.err.append_format(BUILTIN_ERR_ARG_COUNT1, cmd, 0, argc - 1); return STATUS_INVALID_ARGS; } - wcstring res = wgetcwd(); - if (res.empty()) { + wcstring pwd; + if (auto tmp = env_get(L"PWD")) { + pwd = tmp->as_string(); + } + if (resolve_symlinks) { + if (auto real_pwd = wrealpath(pwd)) { + pwd = std::move(*real_pwd); + } else { + const char *error = strerror(errno); + streams.err.append_format(L"%ls: realpath failed:", cmd, error); + return STATUS_CMD_ERROR; + } + } + if (pwd.empty()) { return STATUS_CMD_ERROR; } - streams.out.append(res); + streams.out.append(pwd); streams.out.push_back(L'\n'); return STATUS_CMD_OK; } diff --git a/src/env.cpp b/src/env.cpp index 635dffcb7..69e9d8a03 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -663,15 +663,15 @@ static void env_set_termsize() { if (rows.missing_or_empty()) env_set_one(L"LINES", ENV_GLOBAL, DFLT_TERM_ROW_STR); } -bool env_set_pwd() { +/// Update the PWD variable directory from the result of getcwd(). +void env_set_pwd_from_getcwd() { wcstring cwd = wgetcwd(); if (cwd.empty()) { debug(0, _(L"Could not determine current working directory. Is your locale set correctly?")); - return false; + return; } - env_set_one(L"PWD", ENV_EXPORT | ENV_GLOBAL, cwd); - return true; + env_set_one(L"PWD", ENV_EXPORT | ENV_GLOBAL, std::move(cwd)); } /// Allow the user to override the limit on how much data the `read` command will process. @@ -982,7 +982,11 @@ void env_init(const struct config_paths_t *paths /* or NULL */) { } } - env_set_pwd(); // initialize the PWD variable + // initialize the PWD variable if necessary + // Note we may inherit a virtual PWD that doesn't match what getcwd would return; respect that. + if (env_get(L"PWD").missing_or_empty()) { + env_set_pwd_from_getcwd(); + } env_set_termsize(); // initialize the terminal size variables env_set_read_limit(); // initialize the read_byte_limit diff --git a/src/env.h b/src/env.h index 6a7ddb15f..3f6091f1a 100644 --- a/src/env.h +++ b/src/env.h @@ -156,8 +156,8 @@ void env_set_argv(const wchar_t *const *argv); /// Returns all variable names. wcstring_list_t env_get_names(int flags); -/// Update the PWD variable directory. -bool env_set_pwd(); +/// Update the PWD variable based on the result of getcwd. +void env_set_pwd_from_getcwd(); /// Returns the PWD with a terminating slash. wcstring env_get_pwd_slash(); diff --git a/src/expand.cpp b/src/expand.cpp index d5c30fdfc..805a94cc2 100644 --- a/src/expand.cpp +++ b/src/expand.cpp @@ -772,7 +772,9 @@ static void expand_home_directory(wcstring &input) { } } - maybe_t realhome = (home ? wrealpath(*home) : none()); + maybe_t realhome; + if (home) realhome = normalize_path(*home); + if (realhome) { input.replace(input.begin(), input.begin() + tail_idx, *realhome); } else { diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 4f9b9fddb..bda7b9896 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -172,7 +172,7 @@ static bool pushd(const char *path) { return false; } - env_set_pwd(); + env_set_pwd_from_getcwd(); return true; } @@ -183,7 +183,7 @@ static void popd() { } free((void *)old_cwd); pushed_dirs.pop_back(); - env_set_pwd(); + env_set_pwd_from_getcwd(); } // The odd formulation of these macros is to avoid "multiple unary operator" warnings from oclint @@ -4664,6 +4664,22 @@ void test_layout_cache() { do_test(seqs.find_prompt_layout(L"whatever")->line_count == 100); } +void test_normalize_path() { + say(L"Testing path normalization"); + do_test(normalize_path(L"") == L"."); + do_test(normalize_path(L"..") == L".."); + do_test(normalize_path(L"./") == L"."); + do_test(normalize_path(L"////abc") == L"//abc"); + do_test(normalize_path(L"/abc") == L"/abc"); + do_test(normalize_path(L"/abc/") == L"/abc"); + do_test(normalize_path(L"/abc/..def/") == L"/abc/..def"); + do_test(normalize_path(L"//abc/../def/") == L"//def"); + do_test(normalize_path(L"abc/../abc/../abc/../abc") == L"abc"); + do_test(normalize_path(L"../../") == L"../.."); + do_test(normalize_path(L"foo/./bar") == L"foo/bar"); + do_test(normalize_path(L"foo/././bar/.././baz") == L"foo/baz"); +} + /// Main test. int main(int argc, char **argv) { UNUSED(argc); @@ -4762,6 +4778,7 @@ int main(int argc, char **argv) { if (should_test_function("illegal_command_exit_code")) test_illegal_command_exit_code(); if (should_test_function("maybe")) test_maybe(); if (should_test_function("layout_cache")) test_layout_cache(); + if (should_test_function("normalize")) test_normalize_path(); // history_tests_t::test_history_speed(); say(L"Encountered %d errors in low-level tests", err_count); diff --git a/src/wutil.cpp b/src/wutil.cpp index 51eaa44ca..807a82d50 100644 --- a/src/wutil.cpp +++ b/src/wutil.cpp @@ -433,6 +433,40 @@ maybe_t wrealpath(const wcstring &pathname) { return str2wcstring(real_path); } +wcstring normalize_path(const wcstring &path) { + // Count the leading slashes. + // Preserve up to 2. + const wchar_t sep = L'/'; + size_t leading_slashes = 0; + for (wchar_t c : path) { + if (c != sep) break; + leading_slashes++; + } + + wcstring_list_t comps = split_string(path, sep); + wcstring_list_t new_comps; + for (wcstring &comp : comps) { + if (comp.empty() || comp == L".") { + continue; + } else if (comp == L"..") { + if (new_comps.empty() || new_comps.back() == L"..") { + // We underflowed the ..s, retain this component. + new_comps.push_back(L".."); + } else { + new_comps.pop_back(); + } + } else { + new_comps.push_back(std::move(comp)); + } + } + + // Prepend up to two leading slashes (as empty components). + new_comps.insert(new_comps.begin(), leading_slashes > 2 ? 2 : leading_slashes, wcstring()); + // Ensure e.g. './' normalizes to '.' and not empty. + if (new_comps.empty()) new_comps.push_back(L"."); + return join_strings(new_comps, sep); +} + wcstring wdirname(const wcstring &path) { char *tmp = wcs2str(path); char *narrow_res = dirname(tmp); diff --git a/src/wutil.h b/src/wutil.h index 055493432..971247283 100644 --- a/src/wutil.h +++ b/src/wutil.h @@ -68,6 +68,12 @@ int wchdir(const wcstring &dir); /// \returns the canonicalized path, or none if the path is invalid. maybe_t wrealpath(const wcstring &pathname); +/// Given an input path, "normalize" it: +/// 1. Collapse multiple /s into a single /, except maybe at the beginning. +/// 2. .. goes up a level. +/// 3. Remove /./ in the middle. +wcstring normalize_path(const wcstring &path); + /// Wide character version of readdir(). bool wreaddir(DIR *dir, wcstring &out_name); bool wreaddir_resolving(DIR *dir, const std::wstring &dir_path, wcstring &out_name, diff --git a/tests/cd.err b/tests/cd.err new file mode 100644 index 000000000..3dbd252c2 --- /dev/null +++ b/tests/cd.err @@ -0,0 +1,3 @@ + +#################### +# cd symlink non-resolution diff --git a/tests/cd.in b/tests/cd.in new file mode 100644 index 000000000..281ff94ec --- /dev/null +++ b/tests/cd.in @@ -0,0 +1,9 @@ +logmsg cd symlink non-resolution +set real (mktemp -d) +set link (mktemp -u) +ln -s $real $link +cd $link +test "$PWD" = "$link" || echo "\$PWD != \$link:"\n "\$PWD: $PWD"\n "\$link: $link"\n +test (pwd) = "$link" || echo "(pwd) != \$link:"\n "\$PWD: "(pwd)\n "\$link: $link"\n +test (pwd -P) = "$real" || echo "(pwd -P) != \$real:"\n "\$PWD: $PWD"\n "\$real: $real"\n +test (pwd -P -L) = "$link" || echo "(pwd -P -L) != \$link:"\n "\$PWD: $PWD"\n "\$link: $link"\n diff --git a/tests/cd.out b/tests/cd.out new file mode 100644 index 000000000..3dbd252c2 --- /dev/null +++ b/tests/cd.out @@ -0,0 +1,3 @@ + +#################### +# cd symlink non-resolution diff --git a/tests/expansion.in b/tests/expansion.in index f175ff7f7..f0f594bbd 100644 --- a/tests/expansion.in +++ b/tests/expansion.in @@ -115,8 +115,8 @@ cd $saved mkdir $tmpdir/realhome ln -s $tmpdir/realhome $tmpdir/linkhome set expandedtilde (env HOME=$tmpdir/linkhome ../test/root/bin/fish -c 'echo ~') -if test $expandedtilde != $tmpdir/realhome - echo '~ expands to' $expandedtilde ' - expected ' $tmpdir/realhome +if test $expandedtilde != $tmpdir/linkhome + echo '~ expands to' $expandedtilde ' - expected ' $tmpdir/linkhome end unlink $tmpdir/linkhome rmdir $tmpdir/realhome diff --git a/tests/test_functions/mktemp.fish b/tests/test_functions/mktemp.fish index e32da29fc..bb4cf41e4 100644 --- a/tests/test_functions/mktemp.fish +++ b/tests/test_functions/mktemp.fish @@ -18,6 +18,8 @@ function mktemp set opts $opts d case -t set opts $opts t + case -u + set opts $opts u case -- set -e argv[1] break @@ -69,6 +71,9 @@ function mktemp end set -l args + if contains u $opts + set args $args -u + end if contains d $opts set args $args -d end @@ -88,7 +93,7 @@ function mktemp end set args $args $template - command mktemp $args + realpath (command mktemp $args) end function _mktemp_help