From 0072367512f64e5b3ed152dc34f06b2bd9d7f152 Mon Sep 17 00:00:00 2001 From: Fabian Homborg Date: Sat, 12 Sep 2020 19:28:01 +0200 Subject: [PATCH] fish_add_path: Don't resolve symlinks The case for symlinked directories being duplicated a lot isn't there, but there *is* a usecase for adding the symlink rather than the target, and that's homebrew. E.g. homebrew installs ruby into /usr/local/Cellar/ruby/2.7.1_2/bin, and links to it from /usr/local/opt/ruby/bin. If we add the target, we would miss updates. Having path entries that point to the same location isn't a big problem - it's a path lookup, so it takes a teensy bit longer. The canonicalization is mainly so paths don't end up duplicated via weird spelling and so relative paths can be used. --- doc_src/cmds/fish_add_path.rst | 5 ++++- doc_src/cmds/realpath.rst | 2 +- share/functions/fish_add_path.fish | 2 +- tests/checks/fish_add_path.fish | 9 +++++---- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/doc_src/cmds/fish_add_path.rst b/doc_src/cmds/fish_add_path.rst index 292f319e7..c520f2106 100644 --- a/doc_src/cmds/fish_add_path.rst +++ b/doc_src/cmds/fish_add_path.rst @@ -20,7 +20,7 @@ Description It is (by default) safe to use ``fish_add_path`` in config.fish, or it can be used once, interactively, and the paths will stay in future because of :ref:`universal variables `. This is a "do what I mean" style command, if you need more control, consider modifying the variable yourself. -Components are normalized by :ref:`realpath `. This means that trailing slashes are ignored and symlinks are resolved, and relative paths are made absolute. If a component already exists, it is not added again and stays in the same place unless the ``--move`` switch is given. +Components are normalized by :ref:`realpath `. This means that trailing slashes are ignored and relative paths are made absolute (but symlinks are not resolved). If a component already exists, it is not added again and stays in the same place unless the ``--move`` switch is given. Components are added in the order they are given, and they are prepended to the path unless ``--append`` is given (if $fish_user_paths is used, that means they are last in $fish_user_paths, which is itself prepended to $PATH, so they still stay ahead of the system paths). @@ -64,3 +64,6 @@ Example # I want to add the bin/ directory of my current $PWD (say /home/nemo/) > fish_add_path -v bin/ set fish_user_paths /home/nemo/bin /usr/bin /home/nemo/.local/bin + + # I have installed ruby via homebrew + fish_add_path /usr/local/opt/ruby/bin diff --git a/doc_src/cmds/realpath.rst b/doc_src/cmds/realpath.rst index b78aeb821..e4b858a43 100644 --- a/doc_src/cmds/realpath.rst +++ b/doc_src/cmds/realpath.rst @@ -21,4 +21,4 @@ If a ``realpath`` command exists, it will be preferred, so if you want to use th The following options are available: -- ``-s`` or ``--no-symlink``: Don't resolve symlinks, only make paths absolute, squash multiple slashes and remove trailing slashes. +- ``-s`` or ``--no-symlinks``: Don't resolve symlinks, only make paths absolute, squash multiple slashes and remove trailing slashes. diff --git a/share/functions/fish_add_path.fish b/share/functions/fish_add_path.fish index dd30ee617..cac0d9d4f 100644 --- a/share/functions/fish_add_path.fish +++ b/share/functions/fish_add_path.fish @@ -41,7 +41,7 @@ function fish_add_path --description "Add paths to the PATH" # We could add a non-canonical version of the given path if no duplicate exists, but tbh that's a recipe for disaster. # realpath complains if a parent directory does not exist, so we silence stderr. - set -l p (builtin realpath -- $path 2>/dev/null) + set -l p (builtin realpath -s -- $path 2>/dev/null) # Ignore non-existing paths test -d "$p"; or continue diff --git a/tests/checks/fish_add_path.fish b/tests/checks/fish_add_path.fish index 683d7d57e..661cf2556 100644 --- a/tests/checks/fish_add_path.fish +++ b/tests/checks/fish_add_path.fish @@ -30,19 +30,20 @@ echo $status # CHECK: 1 functions --erase checkpath -# Not adding a link either +# Add a link to the same path. fish_add_path -v $tmpdir/link +# CHECK: set fish_user_paths {{.*}}/link {{.*}}/bin echo $status -# CHECK: 1 +# CHECK: 0 fish_add_path -a $tmpdir/sbin # Not printing anything because it's not verbose, the /sbin should be added at the end. string replace -- $tmpdir '' $fish_user_paths | string join ' ' -# CHECK: /bin /sbin +# CHECK: /link /bin /sbin fish_add_path -m $tmpdir/sbin string replace -- $tmpdir '' $fish_user_paths | string join ' ' -# CHECK: /sbin /bin +# CHECK: /sbin /link /bin set -l oldpath "$PATH" fish_add_path -nP $tmpdir/etc | string replace -- $tmpdir ''