Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: broken symbolic link after creinstall . #453

Merged
merged 1 commit into from
Feb 13, 2023

Conversation

miles170
Copy link
Contributor

@miles170 miles170 commented Jan 7, 2023

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

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@miles170
Copy link
Contributor Author

miles170 commented Jan 9, 2023

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

@akliuxingyuan
Copy link
Contributor

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

@psprint
Copy link
Contributor

psprint commented Jan 10, 2023 via email

@miles170
Copy link
Contributor Author

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
Copy link
Member

vladdoster commented Jan 15, 2023

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
Copy link
Contributor Author

miles170 commented Jan 15, 2023

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
Copy link
Member

vladdoster commented Jan 15, 2023

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
Copy link
Contributor Author

miles170 commented Jan 15, 2023

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)

Signed-off-by: Miles Liu <miles@bung.cc>
@miles170
Copy link
Contributor Author

miles170 commented Feb 1, 2023

This PR should be ready to be merged.

@vladdoster vladdoster merged commit c9c8a10 into zdharma-continuum:main Feb 13, 2023
@miles170 miles170 deleted the fix-creinstall branch February 13, 2023 02:40
@miles170
Copy link
Contributor Author

Thanks ♥

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants