jankatins commented 1 year ago

Status: The "problem" was worked around by changing the patch annex to hook into a different place, but the underlying problem is still there (from https://github.com/zdharma-continuum/zinit/issues/467#issuecomment-1464989941):

basically no hook for hook:!atpull- works (update: unless the zinit call has a atpull ice? I still don't really understand that line...), that problem is at the zinit side and needs a fix. The easiest would be to use the same pattern as for hook:atpull- (without exclamation mark), but i have no idea why that was actually used/built in. Does anyone still know?

Old issue

What happened?

I'Ve the following recipe for fzf:

zinit light-mode depth"1" for \
  @zdharma-continuum/zinit-annex-binary-symlink \
  @zdharma-continuum/zinit-annex-bin-gem-node \
  @zdharma-continuum/zinit-annex-default-ice \
  @zdharma-continuum/zinit-annex-patch-dl \
  @zdharma-continuum/zinit-annex-readurl

zinit default-ice --quiet as'null' from"gh-r" lbin'!' lucid nocompile completions

zinit lbin'!' completions \
    dl'https://raw.githubusercontent.com/junegunn/fzf/master/bin/fzf-tmux;
       https://raw.githubusercontent.com/junegunn/fzf/master/shell/completion.zsh -> _fzf_completion;
       https://raw.githubusercontent.com/junegunn/fzf/master/shell/key-bindings.zsh;
       https://raw.githubusercontent.com/junegunn/fzf/master/man/man1/fzf-tmux.1 -> $ZPFX/man/man1/fzf-tmux.1;
       https://raw.githubusercontent.com/junegunn/fzf/master/man/man1/fzf.1 -> $ZPFX/man/man1/fzf.1' \
    src"key-bindings.zsh" \
    for @junegunn/fzf

Recently, it seems fzf got an update and it now does not have the files which should be downloaded via the patch-dl annex. They are there after a fresh install.

Steps to reproduce

Update zinit: zinit update
Use the above recipe
Change the versions in the files in the <plugin dir>/._zinit/ dir so zinit thinks it has an old version
run zinit update junegunn/fzf -> no patch-dl annex lines are shown
ls -la ~/.local/share/zinit/plugins/junegunn---fzf/ -> observe that the dl files are missing
zinit delete -y junegunn/fzf && exec zsh -> observe that the patch-dl annex lines
ls -la ~/.local/share/zinit/plugins/junegunn---fzf/ -> the dl ice files are there

Relevant output

# to change the version numbers!
[11:19:20] λ  subl ~/.local/share/zinit/plugins/junegunn---fzf/ 

[11:19:43] λ  zinit update junegunn/fzf
(Requesting `fzf-0.37.0-darwin_amd64.zip', version 0.37.0… Current version: 0.36.0.)
... 100.0%
[ziextract] Unpacking the files from: `fzf-0.37.0-darwin_amd64.zip'…
Archive:  fzf-0.37.0-darwin_amd64.zip
  inflating: fzf
[ziextract] Successfully extracted and assigned +x chmod to the file: fzf.

[linkbin annex] Re-created the fzf soft link and set +x on the fzf binary

[11:20:31] λ  ls -la ~/.local/share/zinit/plugins/junegunn---fzf/
total 6840
drwxr-xr-x   5 jankatins  staff      160 Jan 27 11:20 .
drwxr-xr-x  67 jankatins  staff     2144 Jan 20 13:11 ..
drwxr-xr-x   3 jankatins  staff       96 Jan 27 11:20 ._backup
drwxr-xr-x  14 jankatins  staff      448 Jan 20 13:11 ._zinit
-rwxr-xr-x   1 jankatins  staff  3498016 Jan 24 14:16 fzf

[11:20:41] λ  zinit delete -y junegunn/fzf
linkbin annex: Removed fzf soft link

Done (action executed, exit code: 0)

[11:20:51] λ  exec zsh

Downloading junegunn/fzf…
(Requesting `fzf-0.37.0-darwin_amd64.zip'…)
... 100.0%
[ziextract] Unpacking the files from: `fzf-0.37.0-darwin_amd64.zip'…
Archive:  fzf-0.37.0-darwin_amd64.zip
  inflating: fzf
[ziextract] Successfully extracted and assigned +x chmod to the file: fzf.
[patch-dl annex] fzf-tmux downloaded successfully
[patch-dl annex] _fzf_completion downloaded successfully
[patch-dl annex] key-bindings.zsh downloaded successfully
[patch-dl annex] fzf-tmux.1 downloaded successfully
[patch-dl annex] fzf.1 downloaded successfully
[linkbin annex] Created the fzf soft link and set +x on the fzf binary
Installed 0 completions. They are stored in the $INSTALLED_COMPS array.
Skipped installing 1 completions. They are stored in the $SKIPPED_COMPS array.
[WARN] - (starship::config): Error in 'Kubernetes' at 'contexts': Unknown key (Did you mean 'context_aliases'?)

[11:21:00] λ  ls -la ~/.local/share/zinit/plugins/junegunn---fzf/
total 6888
drwxr-xr-x   8 jankatins  staff      256 Jan 27 11:20 .
drwxr-xr-x  67 jankatins  staff     2144 Jan 27 11:20 ..
drwxr-xr-x   2 jankatins  staff       64 Jan 27 11:20 ._backup
drwxr-xr-x  14 jankatins  staff      448 Jan 27 11:20 ._zinit
-rw-r--r--   1 jankatins  staff    10650 Jan 27 11:20 _fzf_completion
-rwxr-xr-x   1 jankatins  staff  3498016 Jan 24 14:16 fzf
-rw-r--r--   1 jankatins  staff     6253 Jan 27 11:20 fzf-tmux
-rw-r--r--   1 jankatins  staff     3934 Jan 27 11:20 key-bindings.zsh

Screenshots and recordings

No response

Operating System & Version

Zsh version

zsh 5.9 (x86_64-apple-darwin21.3.0)

Terminal emulator

xterm-256color (wezterm)

If using WSL on Windows, which version of WSL

None

Additional context

No response

Code of Conduct

[X] I agree to follow this project's Code of Conduct

jankatins commented 1 year ago

This seems to be already a bug since November 2022: https://github.com/zdharma-continuum/zinit-annex-patch-dl/issues/6#issuecomment-1321357390

jankatins commented 1 year ago

I debugged this a bit (BTW: a wiki doc on how to debug stuff would be much appreciated, I basically add prints until I find something interesting...):

Right before update, we see this hooks from dl annex:

print -l $ZINIT_EXTS
11 z-annex-data: zinit-annex-patch-dl hook:!atclone-20 za-patch-dl-handler za-patch-dl-help-null-handler dl''|patch''
12 z-annex-data: zinit-annex-patch-dl hook:!atpull-20 za-patch-dl-handler za-patch-dl-help-null-handler ''

There is a difference in the lines and funnily, if I change my zinit call to include a atpull"%atclone" (despite no atclone ice!), an update does the right thing:

λ zinit light-mode depth"1" for \
  @zdharma-continuum/zinit-annex-binary-symlink \
  @zdharma-continuum/zinit-annex-bin-gem-node \
  @zdharma-continuum/zinit-annex-default-ice \
  @zdharma-continuum/zinit-annex-patch-dl \
  @zdharma-continuum/zinit-annex-readurl

λ zinit default-ice --quiet as'null' from"gh-r" lbin'!' lucid nocompile completions

λ zinit lbin'!' atpull"%atclone" completions \
    dl'https://raw.githubusercontent.com/junegunn/fzf/master/bin/fzf-tmux;
       https://raw.githubusercontent.com/junegunn/fzf/master/shell/completion.zsh -> _fzf_completion;
       https://raw.githubusercontent.com/junegunn/fzf/master/shell/key-bindings.zsh;
       https://raw.githubusercontent.com/junegunn/fzf/master/man/man1/fzf-tmux.1 -> $ZPFX/man/man1/fzf-tmux.1;
       https://raw.githubusercontent.com/junegunn/fzf/master/man/man1/fzf.1 -> $ZPFX/man/man1/fzf.1' \
    src"key-bindings.zsh" \
    for @junegunn/fzf

λ  rm ~/.local/share/zinit/plugins/junegunn---fzf/._zinit/is_release

λ  zinit update junegunn/fzf
(Requesting `fzf-0.38.0-darwin_amd64.zip', version 0.38.0…)
########################################################################################################################################################################### 100.0%
[ziextract] Unpacking the files from: `fzf-0.38.0-darwin_amd64.zip'…
Archive:  fzf-0.38.0-darwin_amd64.zip
  inflating: fzf
[ziextract] Successfully extracted and assigned +x chmod to the file: fzf.
Installed 0 completions. They are stored in the $INSTALLED_COMPS array.
Skipped installing 1 completions. They are stored in the $SKIPPED_COMPS array.

[patch-dl annex] fzf-tmux downloaded successfully
[patch-dl annex] _fzf_completion downloaded successfully
[patch-dl annex] key-bindings.zsh downloaded successfully
[patch-dl annex] fzf-tmux.1 downloaded successfully
[patch-dl annex] fzf.1 downloaded successfully
[linkbin annex] Re-created the fzf soft link and set +x on the fzf binary

-> So this seems to be a bug in the annex not registering the right command for the hook?

jankatins commented 1 year ago

Ok, seems there is multiple things and some of them are on the zinit side:

The registering in the patch-dl repo:

@zinit-register-annex "zinit-annex-patch-dl" \
  hook:\!atpull-20 \
  za-patch-dl-handler \
  za-patch-dl-help-null-handler

As the above, that works, it's in $ZINIT_EXTS

My understanding is that the following line(s) should return the hook which got registered in for the dl annex (from the middle part of that "thing"):

                reply=(
                    ${(on)ZINIT_EXTS2[(I)zinit hook:e-\!atpull-pre <->]}
                    ${${(M)ICE[atpull]#\!}:+${(on)ZINIT_EXTS[(I)z-annex hook:\!atpull-<-> <->]}}
                    ${(on)ZINIT_EXTS2[(I)zinit hook:e-\!atpull-post <->]}
                )

There are multiple things here:

a) the line ${${(M)ICE[atpull]#\!}:+${(on)ZINIT_EXTS[(I)z-annex hook:\!atpull-<-> <->]}} returns nothing, but the second part should (and does!) return the registered hook:

λ  echo ${${(M)ICE[atpull]#\!}:+${(on)ZINIT_EXTS[(I)z-annex hook:\!atpull-<-> <->]}}

λ  echo ${(on)ZINIT_EXTS[(I)z-annex hook:\!atpull-<-> <->]}
z-annex hook:!atpull-20 12

λ  echo ${(M)ICE[atpull]#\!}

the "12" entry is 12 z-annex-data: zinit-annex-patch-dl hook:!atpull-20 za-patch-dl-handler za-patch-dl-help-null-handler '' -> the dl-patch function.

b) Even if that line would return the hook, it would run before the actual gh-r download which is registered as @zinit-register-hook "atpull'!'" hook:e-\!atpull-post ∞zinit-atpull-e-hook, so gets returned by ${(on)ZINIT_EXTS2[(I)zinit hook:e-\!atpull-post <->]}. For install it's the reverse: the gh-r download is first, the dl lines second.

So and here I'm stuck: I've no clue what the intend of the ${${(M)ICE[atpull]#\!}:+${(on)ZINIT_EXTS[(I)z-annex hook:\!atpull-<-> <->]}} line is: why should there an hook in the ICE variable: it should contain a "atpull" ice (not a hook) from the saved ices. But I anyway have no atpull. And why isn't it overwritten by the ZINIT_EXTS stuff?

-> I'm out for now, I will just add the atpull'%atclone' and call it a day for now...

jankatins commented 1 year ago

Actually, one fix would be to change the hook registering to

@zinit-register-annex "zinit-annex-patch-dl" \
  hook:atpull-20 \
  za-patch-dl-handler \
  za-patch-dl-help-null-handler

-> without exclamation mark, so runs after the pull!

It works, because the post pull hooks are looked up without looking at the ICE one:

            reply=(
                ${(on)ZINIT_EXTS2[(I)zinit hook:atpull-pre <->]}
                ${(on)ZINIT_EXTS[(I)z-annex hook:atpull-<-> <->]}
                ${(on)ZINIT_EXTS2[(I)zinit hook:atpull-post <->]}
            )

Nevertheless:

it seems there is no way for a annex to contribute hook which is run before the ghr download.

jankatins commented 1 year ago

There is a PR which fixes the original problem, but there is still a bug here if you want to register a hook from an annex to be run before gtit pull/gh-r download happens.

vladdoster commented 1 year ago

@jankatins,

Circling back on this issue to see if this issue can be closed?

Additionally, +1 for thoroughly documenting the what & why of the issue & fix.

jankatins commented 1 year ago

No, basically no hook for hook:\!atpull-* works (update: unless the zinit call has a atpull ice? I still don't really understand that line...), that problem is at the zinit side and needs a fix. The easiest would be to use the same pattern as for hook:atpull-* (without exclamation mark), but i have no idea why that was actually used/built in. Does anyone still know?

vladdoster commented 1 year ago

No, basically no hook for hook:\!atpull-* works, that problem is at the zinit side and needs a fix. The easiest would be to use the same pattern as for hook:atpull-* (without exclamation mark), but i have no idea why that was actually used/built in. Did anyone still know?

@psprint,

Could you shed some light on the aforementioned design choice for @jankatins and I?

jankatins commented 1 year ago

I guess this is the original commit which introduced this: https://github.com/zdharma-continuum/zinit/commit/dd51c6e960ef9d0a66792245cfe936931d4cc51f and this is what converted it to the current form: https://github.com/zdharma-continuum/zinit/commit/4236d9769ec87edf58bd86dd9b79a951cd8e3e09

Both of these are above my level of understanding of zsh code to understand the intend of what these lines should do... :-/

Interestingly, all atclone hook calls are done without looking at the ice content -> another argument to simply remove that part?

psprint commented 1 year ago

Afair the !hook is run on atclone'!...' ice, without it's run on atclone'...'. if the problem is with exclamation mark then you can switch to some other char, like x (xatclone).

zdharma-continuum / zinit

[bug]: no hook for hook:!atpull-* works #467

Old issue

What happened?

Steps to reproduce

Relevant output

Screenshots and recordings

Operating System & Version

Zsh version

Terminal emulator

If using WSL on Windows, which version of WSL

Additional context

Code of Conduct