zdharma-continuum / zinit

🌻 Flexible and fast ZSH plugin manager
MIT License
2.89k stars 124 forks source link

fix: broken symbolic link after `creinstall .` #453

Closed miles170 closed 1 year ago

miles170 commented 1 year ago

Description

Run creinstall . in the zsh-users/zsh-completions plugin folder manually, then the symbolic links in the completions folder are broken.

Motivation and Context

I found this issue when running zinit update with the following minimal setup.

zinit wait lucid light-mode for
  blockf atpull'zinit creinstall -q .' \
    zsh-users/zsh-completions

Related Issue(s)

Usage examples

$ find ~/.local/share/zinit/completions -xtype l | wc -l # no broken symbolic links before creinstall
0
$ pwd
~/.local/share/zinit/plugins/zsh-users---zsh-completions
$ zinit creinstall -q .
Installed 139 completions. They are stored in the $INSTALLED_COMPS array.
$ find ~/.local/share/zinit/completions -xtype l | wc -l # count broken symbolic links
139
$ ls -l ~/.local/share/zinit/completions
total 560
lrwxrwxrwx 1 miles miles 11 Jan  7 11:14 _afew -> ./src/_afew # broken symbolic link due to relative path
...

How Has This Been Tested?

Types of changes

Checklist:

miles170 commented 1 year ago

Force-pushed to merge the latest changes from the main branch.

akliuxingyuan commented 1 year ago

better remove ./ in c? [[ "${c:0:1}" != "/" ]] && c="${PWD}/${c:2}"

psprint commented 1 year ago

The bash style indexing should be changed to Zsh one: $c[1,2] or just to $c[1].

On Tue, 10 Jan 2023 at 13:24, akliuxingyuan @.***> wrote:

better remove ./ in c? [[ "${c:0:1}" != "/" ]] && c="${PWD}/${c:2}"

— Reply to this email directly, view it on GitHub https://github.com/zdharma-continuum/zinit/pull/453#issuecomment-1377263637, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABOE4CC7W3KDDK6TFHQ4UGDWRVPJJANCNFSM6AAAAAATTVY34M . You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- Best regards, Sebastian Gniazdowski

miles170 commented 1 year ago

better remove ./ in c? [[ "${c:0:1}" != "/" ]] && c="${PWD}/${c:2}"

My initial thought was to use realpath to normalize paths, but I'm not sure realpath will exist on every platform.

vladdoster commented 1 year ago

better remove ./ in c? [[ "${c:0:1}" != "/" ]] && c="${PWD}/${c:2}"

My initial thought was to use realpath to normalize paths, but I'm not sure realpath will exist on every platform.

@miles170,

You can use readlink which is the successor of realpath.

More specifically, you'll want readlink -f.

miles170 commented 1 year ago

You can use readlink which is the successor of realpath.

More specifically, you'll want readlink -f.

Do I have to test readlink before using it?

diff --git a/zinit-install.zsh b/zinit-install.zsh
index 09dd726..db0a1fa 100644
--- a/zinit-install.zsh
+++ b/zinit-install.zsh
@@ -565,7 +565,11 @@ builtin source "${ZINIT[BIN_DIR]}/zinit-side.zsh" || {
     # OR - if its a reinstall
     for c in "${completions[@]}"; do
         # If filepath is relative, prepend the current directory
-        [[ "${c[1]}" != "/" ]] && { [[ "${c[1,2]}" == "./" ]] && c="${PWD}/${c[3,-1]}" || c="${PWD}/${c}" }
+        if type readlink 2>/dev/null 1>&2; then
+            c=$(command readlink -f "$c")
+        else
+            [[ "${c[1]}" != "/" ]] && { [[ "${c[1,2]}" == "./" ]] && c="${PWD}/${c[3,-1]}" || c="${PWD}/${c}" }
+        fi
         cfile="${c:t}"
         bkpfile="${cfile#_}"
         if [[ ( -z ${already_symlinked[(r)*/$cfile]} || $reinstall = 1 ) &&
vladdoster commented 1 year ago

You can use readlink which is the successor of realpath. More specifically, you'll want readlink -f.

Do I have to test readlink before using it?

@miles170,

One step ahead of you :^)

There is a .zinit-prepare-readlink function you could use.

Screenshot 2023-01-15 at 01 35 13
miles170 commented 1 year ago

Furthermore, readlink -f does not work on FreeBSD or macOS, which are used to display information in the specified format.

I found a portable way to get an absolute path by using the A modifier. (https://zsh.sourceforge.io/Doc/Release/Expansion.html#Modifiers)

miles170 commented 1 year ago

This PR should be ready to be merged.

miles170 commented 1 year ago

Thanks ♥