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: allow zinit to be run from non-interactive scripts #227

Conversation

jankatins
Copy link
Contributor

@jankatins jankatins commented Apr 25, 2022

Exclamation marks (per default) get escaped in interactive mode, but not in non-interactive modes. In this case it meant that zinit would not find any hooks which implements the ices when running in non-interactive mode (e.g. in a script). This in turn would NOT result in errors but would just silently not execute any of the ices which of course would be a bad thing(tm).

This actually happened in the zunit tests as they run non-interactively and therefore any ices based on exclamation mark hooks were not ran. I fixed the few places where (after the fix in this PR) the ice were actually ran and errored due to mis-specifications. I also added explicit checks for a few ices. -> before this PR, ices are basically not tested (and the exclamation mark hook based ones not testable at all!) :-(

Description

This a) disables history mark expansion during hook register and changes any search pattern to not include the (double) escaped exclamation mark

Motivation and Context

It broke using zinit calls with ices in scripts (e.g. from a $HOME bootstrap script or an update script)

#199 has the details...

Closes: #199

How Has This Been Tested?

See the new zunit tests and a few fixes in old tests which were needed because the actual ices were not run beforehand and therefore didn't error when they were not specified correctly.

Running the same ice (mv in this case) interactively and in a script. Both calls contain the direnv.darwin-amd64 -> direnv log line.

[16:30:23] λ  zinit delete direnv/direnv -y ; zsh -i -c -f --no-rcs 'source ~/.zinit/bin/zinit.zsh ; source ~/.zinit/bin/zinit-install.zsh; zinit from"gh-r" mv"direnv* -> direnv" for direnv/direnv'
bin-gem-node annex: The direnv shim didn't exist in $ZPFX/bin (or isn't a regular file)

Done (action executed, exit code: 0)

Downloading direnv/direnv…
(Requesting `direnv.darwin-amd64'…)
#################################################################################################################################################################################################################### 100.0%
ziextract: Successfully extracted and assigned +x chmod to the file: `direnv.darwin-amd64'.
direnv.darwin-amd64 -> direnv
No files for compilation found.
Warning: ∞zinit-compile-plugin-hook hook returned with 1


[16:30:29] [1] ✗  ls -la ~/.zinit/plugins/direnv---direnv/direnv*
-rwxr-xr-x  1 jankatins  staff  8811104 Apr 25 16:30 /Users/jankatins/.zinit/plugins/direnv---direnv/direnv


[16:30:37] λ  zinit delete direnv/direnv -y ; source ~/.zinit/bin/zinit.zsh ; source ~/.zinit/bin/zinit-install.zsh; zinit from"gh-r" mv"direnv* -> direnv" for direnv/direnv
bin-gem-node annex: The direnv shim didn't exist in $ZPFX/bin (or isn't a regular file)

Done (action executed, exit code: 0)

Downloading direnv/direnv…
(Requesting `direnv.darwin-amd64'…)
#################################################################################################################################################################################################################### 100.0%
ziextract: Successfully extracted and assigned +x chmod to the file: `direnv.darwin-amd64'.
direnv.darwin-amd64 -> direnv
No files for compilation found.
Warning: ∞zinit-compile-plugin-hook hook returned with 1

[16:31:02] [1] ✗  ls -la ~/.zinit/plugins/direnv---direnv/direnv*
-rwxr-xr-x  1 jankatins  staff  8811104 Apr 25 16:31 /Users/jankatins/.zinit/plugins/direnv---direnv/direnv

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.

@jankatins
Copy link
Contributor Author

There are still two places where \\\! exists: in test-excl-id-as and in zinit-autoload.zsh: https://github.com/zdharma-continuum/zinit/blob/main/zinit-autoload.zsh#L3324:

    [[ -e $local_dir ]] && {
        for el ( ${ice_order[@]} ) {
            val="${ice[$el]}"
            cand1="${(qqq)val}"
            cand2="${(qq)val}"
            if [[ -n "$val" ]] {
                [[ "${cand1/\\\$/}" != "$cand1" || "${cand1/\\\!/}" != "$cand1" ]] && output+=( "$el$cand2" ) || output+=( "$el$cand1" )
            } elif [[ ${+ice[$el]} = 1 && -n "${nval_ices[(r)$el]}" ]] {
                output+=( "$el" )
            }
        }

In both cases, I do not know what to do with it :-(

Also: not sure if this should get a test (and how: given that tests would run in scripts and so would always be non-interactive?).

@vladdoster
Copy link
Member

For the failing documentation test, you should be able to run:

zinit make"PREFIX=$ZPFX install" for @zdharma-continuum/zshelldoc
make doc

@vladdoster
Copy link
Member

And if the unit test tests are failing after #226 is merged, I'll look and make sure it isn't related to my work over the weekend.

@vladdoster vladdoster changed the title Do not escape exclamation marks in hook keys fix: don't escape exclamation marks in hook keys Apr 25, 2022
@vladdoster
Copy link
Member

vladdoster commented Apr 25, 2022

this XKCD seems warranted ;)

image

@jankatins jankatins force-pushed the dont_escape_exclamation_mark_in_hook_names branch from a0b0f94 to b439f0d Compare April 25, 2022 19:24
@jankatins
Copy link
Contributor Author

Manually added the doc change, as I got a real bunch of changes here (mac? Different version?). Also fixed the commit message to be the same as the PR title

@jankatins
Copy link
Contributor Author

The unittest failure looks more like a problem with downloading the wrong file from gh (386 instead of amd64)? #226 ?

✘ gh-hub
  default-ice: stored the ices: as null nocompletions '' nocompile '' from gh-r.

Downloading github/hub…
(Requesting `hub-linux-386-2.14.2.tgz'…)
...
ziextract: Unpacking the files from: `hub-linux-386-2.14.2.tgz'…

ziextract: Successfully extracted and marked executable the appropriate files (hub, install) contained in `hub-linux-386-2.14.2.tgz'.

'hub-linux-386-2.14.2/etc/hub.zsh_completion' -> '_hub'
Warning: ∞zinit-cp-hook hook returned with 1
bin-gem-node annex: Created the hub shim and set +x on the hub binary
'1' is not equal to '0'

@jankatins jankatins force-pushed the dont_escape_exclamation_mark_in_hook_names branch from b439f0d to 2b38534 Compare April 27, 2022 11:15
@jankatins
Copy link
Contributor Author

@vladdoster Rebased and force pushed on top of the 386 fixes in #226

@vladdoster
Copy link
Member

@jankatins
Copy link
Contributor Author

@vladdoster the failure on github/hub looks a bit strange:

[10:27:11] λ  zinit cp"hub-*/etc/hub.zsh_completion -> _hub" sbin"hub-*/bin/hub" from"gh-r" as'null' for @github/hub

Downloading github/hub…
(Requesting `hub-darwin-amd64-2.14.2.tgz'…)
[...]
ziextract: Unpacking the files from: `hub-darwin-amd64-2.14.2.tgz'…
ziextract: Successfully extracted and marked executable the appropriate files (hub, install) contained in `hub-darwin-amd64-2.14.2.tgz'.
hub-darwin-amd64-2.14.2/etc/hub.zsh_completion -> _hub
Warning: ∞zinit-cp-hook hook returned with 1
bin-gem-node annex: Created the hub shim and set +x on the hub binary


~/.zinit/bin on  dont_escape_exclamation_mark_in_hook_names (2b385344) [?] [🐳: local]
took 2s
[10:27:24] [1] ✗  ls -la ~/.zinit/plugins/github---hub/_hub
-rw-r--r--  1 jankatins  staff  5603 Apr 28 10:27 /Users/jankatins/.zinit/plugins/github---hub/_hub

So the file is there, but the cp hook returns 1 and therefore the zinit call fails. I wonder if this is actually triggering the codepath which this PR should fix and the cp is simply not executed currently. The current output, e.g. in https://github.com/zdharma-continuum/zinit/runs/6202981953?check_suite_focus=true#step:8:5380 (run of current master)) supports that hypothesis:

Downloading github/hub…

(Requesting `hub-darwin-amd64-2.14.2.tgz'…)
[...]
ziextract: Unpacking the files from: `hub-darwin-amd64-2.14.2.tgz'…

ziextract: Successfully extracted and marked executable the appropriate files (hub, install) contained in `hub-darwin-amd64-2.14.2.tgz'.

bin-gem-node annex: Created the hub shim and set +x on the hub binary
git version 2.35.1
hub version 2.14.2
✔ gh-hub

-> the hub-darwin-amd64-2.14.2/etc/hub.zsh_completion -> _hub line is missing and therefore the "hook returned 1" not triggered.

@jankatins jankatins force-pushed the dont_escape_exclamation_mark_in_hook_names branch from 2b38534 to 45e569e Compare May 1, 2022 20:11
@jankatins
Copy link
Contributor Author

jankatins commented May 1, 2022

A bit more tracing and this turned out to be these two lines:

             command cp -vf "${afr[1]}" "$to"
             command cp -vf "${afr[1]}".zwc "$to".zwc 2>/dev/null

It seems the return value of the second copy (which didn't find files to copy) was returned from the hook and therefore the hook run marked as failed.

Verified with

λ  zsh -c 'source ~/.zshrc ; zinit delete github/hub -y; zinit cp"hub-*/etc/hub.zsh_completion -> _hub" sbin"hub-*/bin/hub" from"gh-r" as"null" for @github/hub '

bin-gem-node annex: Correctly removed the hub shim from $ZPFX/bin

Done (action executed, exit code: 0)

Downloading github/hub…
(Requesting `hub-darwin-amd64-2.14.2.tgz'…)
ziextract: Unpacking the files from: `hub-darwin-amd64-2.14.2.tgz'…
ziextract: Successfully extracted and marked executable the appropriate files (hub, install) contained in `hub-darwin-amd64-2.14.2.tgz'.
hub-darwin-amd64-2.14.2/etc/hub.zsh_completion -> _hub
bin-gem-node annex: Created the hub shim and set +x on the hub binary

@vladdoster See the second commit here for a possible fix (modeled after the pattern found in the mv hook)

@jankatins
Copy link
Contributor Author

BTW: is there any better way to enable tracing everywhere than s/setopt / setopt xtrace/ in all files? Seems every time emulate -LR zsh is called in a function, xtrace is off again :-( E.g. I started testing with zsh -c 'source ~/.zshrc ; export PS4="%? // %x.%I: "; exec 3>&2 2>/tmp/zinit.log ; setopt xtrace ; zinit delete github/hub -y; zinit cp"hub-*/etc/hub.zsh_completion -> _hub" sbin"hub-*/bin/hub" from"gh-r" as"null" for @github/hub ; unsetopt xtrace ; exec 2>&3 3>&-' and missed most of the interesting trace until I ran the above search&replace...

@jankatins jankatins force-pushed the dont_escape_exclamation_mark_in_hook_names branch 3 times, most recently from bf71515 to b371adf Compare May 3, 2022 10:29
@github-actions github-actions bot added the tests label May 3, 2022
@jankatins
Copy link
Contributor Author

Current failure:

✘ tldr-completion::gh-r
  �
Downloading dbrgn/tealdeer… (at label: tldr-completion/gh-r…)

(Requesting `completions_zsh'…)



#=#=#                                                                         



######################################################################## 100.0%
ziextract: WARNING: didn't recognize the archive type of `completions_zsh'  }(no extraction has been done).%f%b

No files for compilation found.

Warning: ∞zinit-compile-plugin-hook hook returned with 1
'1' is not equal to '0'

I can reproduce locally and the fix is to add a pick"completions_zsh" to the test...

λ  zinit delete "tldr-completion/gh-r" -y ; zinit for as"completion" from"gh-r" id-as"tldr-completion/gh-r" bpick"completions_zsh" pick"completions_zsh"  @dbrgn/tealdeer

Done (action executed, exit code: 0)

Downloading dbrgn/tealdeer… (at label: tldr-completion/gh-r…)
(Requesting `completions_zsh'…)
############################################################################################################################################################################################################ 100.0%
ziextract: WARNING: didn't recognize the archive type of `completions_zsh'  }(no extraction has been done).%f%b
Note: Compiling: completions_zsh… OK.

-> Pushed in a separate commit as I'm not completely sure if this need is expected and why this is now failing (no exclamation mark in the hook name) after the fix.

@jankatins
Copy link
Contributor Author

@vladdoster It's green :-)

@vladdoster
Copy link
Member

Even though its green, the workaround seems like there is a regression somewhere, no?

@jankatins
Copy link
Contributor Author

jankatins commented May 3, 2022

It feels more like the unit tests were having the same problem (I guess they are running non-interactive) and therefore config errors were not noticed. Eg nothing in the tests check for compiling in the tldr gh-r case or for the copied _hub completion file and so the missing hooks didn't lead to errors.

And running it interactively will get you a error message but the main functionality is there (hub command and completion for tldr).

@@ -52,7 +52,7 @@
}
@test 'tldr-completion::gh-r' {
artifact="$ZINIT[PLUGINS_DIR]/tldr-completion---gh-r"
run zinit for as"completion" from"gh-r" id-as'tldr-completion/gh-r' bpick"completions_zsh" @dbrgn/tealdeer
run zinit for as"completion" from"gh-r" id-as'tldr-completion/gh-r' bpick"completions_zsh" pick"completions_zsh" @dbrgn/tealdeer
Copy link
Contributor Author

@jankatins jankatins May 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding of the need for this is that the compile now fails without this because the compile step wasn't run before in the unit test. After all, it was running non-interactive and triggered the bug, which got fixed in this PR.

Copy link
Member

@vladdoster vladdoster May 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, okay. I'm going to (but feel to if you want) add a check using zi clist or something to ensure that the completion is registered.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason, both versions neither work on main nor on this branch, at least I didn't find any tldr/tealdeer entries in zinit clist when running it in my console.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On main and my branch, this one works:

[14:31:37] λ  zinit for as"completion" from"gh-r" id-as"tldr-completion/gh-r" bpick"completions_zsh" mv"completions_zsh -> _tldr" pick"_tldr" @dbrgn/tealdeer || echo bad

Downloading dbrgn/tealdeer… (at label: tldr-completion/gh-r…)
(Requesting `completions_zsh'…)
#################################################################################################################################################################################################################### 100.0%
ziextract: WARNING: didn't recognize the archive type of `completions_zsh'  }(no extraction has been done).%f%b
completions_zsh -> _tldr
Note: Compiling: _tldr… OK.
Installed 0 completions. They are stored in the $INSTALLED_COMPS array.
Skipped installing 1 completions. They are stored in the $SKIPPED_COMPS array.

The SKIPPED is because the symlink in ~/.zinit/completion/ seems to not get removed on delete:

[14:32:48] λ  zinit delete tldr-completion/gh-r -y

[14:33:56] λ  ls -la ../completions/_tldr
lrwxr-xr-x  1 jankatins  staff  60 May  6 14:30 ../completions/_tldr -> /Users/jankatins/.zinit/plugins/tldr-completion---gh-r/_tldr

[14:34:22] λ  ls -la /Users/jankatins/.zinit/plugins/tldr-completion---gh-r/_tldr
ls: /Users/jankatins/.zinit/plugins/tldr-completion---gh-r/_tldr: No such file or directory

[14:34:37] [1] ✗  zinit delete tldr-completion/gh-r -y
No such (plugin or snippet): tldr-completion/gh-r.

-> Will adjust the test but I can also imagine that there is some better way to handle a as"completion" which does not install a completion.

( () { setopt localoptions noautopushd; builtin cd -q "$dir"; } || return 1
afr=( ${~from}(DN) )
if (( ${#afr} )) {
if (( !OPTS[opt_-q,--quiet] )) {
command cp -vf "${afr[1]}" "$to"
retval=$?
# ignore errors if no compiled file is found
command cp -vf "${afr[1]}".zwc "$to".zwc 2>/dev/null
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here: the cp hook wasn't run and therefore the second cp which tries to copy the compiled files fails now. Pattern with retval is the same as a few lines above in mv hook.

@jankatins jankatins force-pushed the dont_escape_exclamation_mark_in_hook_names branch from 05f7f9d to cb35b75 Compare May 6, 2022 11:21
@vladdoster
Copy link
Member

I'm also really wanting @psprint to look over this. This has been why I've held off on merging this. No personal qualms with changeset -- this fixes a huge bug in Zinit functionality (and makes the behavior consistent) :^).

@jankatins
Copy link
Contributor Author

Then run (this does not work interactively):
So AFAIK, these changes should make the aforementioned command work now.

My understanding of this bug+fix is that it only changes behavior for non-interactive usages. So i think it will still not work.

Will test tomorrow/day after when I'm back at my laptop.

jankatins and others added 5 commits July 26, 2022 16:15
Co-authored-by: Philipp Schmitt <pschmitt@users.noreply.github.com>
Exclamation marks (per default) get escaped in interactive mode,
but not in non-interactive modes. In this case it meant that zinit
would not find any hooks which implements the ices when running in
non-interactive mode (e.g. in a script). This in turn would NOT result
in errors but would just silently not execute any of the ices which
of course would be a bad thing(tm).

zdharma-continuum#199 has the details...

Closes: zdharma-continuum#199
This would otherwise bubble up as a hook failure and further up as a zinit call failure
After the fix, the compile hook is now run and would fail without this information. IN addition, only files starting with _ are linked into the completion folder.
@jankatins jankatins force-pushed the dont_escape_exclamation_mark_in_hook_names branch from 7461754 to fc77c1c Compare July 26, 2022 14:16
@jankatins
Copy link
Contributor Author

Results of the above commands:

[16:18:39] λ  zi rustup cargo'!exa -> ls' load for zdharma-continuum/null

Downloading zdharma-continuum/null…
Cloning into '/home/jan/.local/share/zinit/plugins/zdharma-continuum---null'...
⠙ ███████████ OBJ: 100, PACK: 8/8, COMPR: 100%, REC: 100%
No files for compilation found.
Warning: ∞zinit-compile-plugin-hook hook returned with 1
rust annex: Running the rustup installer...
warning: rustup should not be installed alongside Rust. Please uninstall your existing Rust first.
warning: If you are sure that you want both rustup and your already installed Rust
error: cannot install while Rust is installed
info: installing component 'cargo'
info: installing component 'clippy'
info: installing component 'rust-std'
info: installing component 'rustc'
  nightly-x86_64-unknown-linux-gnu installed - rustc 1.64.0-nightly (6dbae3ad1 2022-07-25)
Rust is installed now. Great!
rust annex: Installing the requested crates...
    Updating crates.io index
  Downloaded exa v0.10.1
  Downloaded 1 crate (136.6 KB) in 0.76s
  Installing exa v0.10.1
  Downloaded ansi_term v0.12.1
  Downloaded glob v0.3.0
  Downloaded idna v0.2.3
  Downloaded jobserver v0.1.24
  Downloaded lazy_static v1.4.0
  Downloaded matches v0.1.9
  Downloaded log v0.4.17
  Downloaded locale v0.2.2
  Downloaded percent-encoding v2.1.0
  Downloaded pkg-config v0.3.25
  Downloaded natord v1.0.9
  Downloaded number_prefix v0.4.0
  Downloaded scoped_threadpool v0.1.9
  Downloaded pad v0.1.6
  Downloaded term_grid v0.1.7
  Downloaded tinyvec_macros v0.1.0
  Downloaded tinyvec v1.6.0
  Downloaded unicode-bidi v0.3.8
  Downloaded unicode-normalization v0.1.21
  Downloaded term_size v0.3.2
  Downloaded unicode-width v0.1.9
  Downloaded url v2.2.2
  Downloaded users v0.11.0
  Downloaded zoneinfo_compiled v0.5.1
  Downloaded form_urlencoded v1.0.1
  Downloaded cfg-if v1.0.0
  Downloaded cc v1.0.73
  Downloaded byteorder v1.4.3
  Downloaded bitflags v1.3.2
  Downloaded num_cpus v1.13.1
  Downloaded datetime v0.5.2
  Downloaded git2 v0.13.25
  Downloaded libc v0.2.126
  Downloaded libz-sys v1.1.8
  Downloaded libgit2-sys v0.12.26+1.3.0
  Downloaded 35 crates (5.8 MB) in 4.08s (largest was `libz-sys` at 2.5 MB)
   Compiling libc v0.2.126
   Compiling pkg-config v0.3.25
   Compiling tinyvec_macros v0.1.0
   Compiling log v0.4.17
   Compiling matches v0.1.9
   Compiling unicode-width v0.1.9
   Compiling percent-encoding v2.1.0
   Compiling unicode-bidi v0.3.8
   Compiling cfg-if v1.0.0
   Compiling bitflags v1.3.2
   Compiling byteorder v1.4.3
   Compiling ansi_term v0.12.1
   Compiling scoped_threadpool v0.1.9
   Compiling natord v1.0.9
   Compiling lazy_static v1.4.0
   Compiling number_prefix v0.4.0
   Compiling glob v0.3.0
   Compiling tinyvec v1.6.0
   Compiling pad v0.1.6
   Compiling term_grid v0.1.7
   Compiling form_urlencoded v1.0.1
   Compiling unicode-normalization v0.1.21
   Compiling jobserver v0.1.24
   Compiling locale v0.2.2
   Compiling users v0.11.0
   Compiling term_size v0.3.2
   Compiling num_cpus v1.13.1
   Compiling cc v1.0.73
   Compiling datetime v0.5.2
   Compiling idna v0.2.3
   Compiling zoneinfo_compiled v0.5.1
   Compiling exa v0.10.1
   Compiling url v2.2.2
   Compiling libz-sys v1.1.8
   Compiling libgit2-sys v0.12.26+1.3.0
   Compiling git2 v0.13.25
    Finished release [optimized] target(s) in 2m 16s
  Installing /home/jan/.local/share/zinit/plugins/zdharma-continuum---null/bin/exa
   Installed package `exa v0.10.1` (executable `exa`)
rust annex: Created the ls shim.

[16:38:26] λ  which ls
/usr/bin/ls

[16:39:37] λ  ls -la .local/share/zinit/plugins/zdharma-continuum---null/bin/
total 204152
drwxr-xr-x  2 jan jan     4096 Jul 26 16:22 .
drwxr-xr-x  7 jan jan     4096 Jul 26 16:39 ..
-rwxr-xr-x 13 jan jan 15825920 Jul 26 16:19 cargo
-rwxr-xr-x 13 jan jan 15825920 Jul 26 16:19 cargo-clippy
-rwxr-xr-x 13 jan jan 15825920 Jul 26 16:19 cargo-fmt
-rwxr-xr-x 13 jan jan 15825920 Jul 26 16:19 cargo-miri
-rwxr-xr-x 13 jan jan 15825920 Jul 26 16:19 clippy-driver
-rwxr-xr-x  1 jan jan  3290968 Jul 26 16:22 exa
-rwxr-xr-x 13 jan jan 15825920 Jul 26 16:19 rls
-rwxr-xr-x 13 jan jan 15825920 Jul 26 16:19 rustc
-rwxr-xr-x 13 jan jan 15825920 Jul 26 16:19 rustdoc
-rwxr-xr-x 13 jan jan 15825920 Jul 26 16:19 rustfmt
-rwxr-xr-x 13 jan jan 15825920 Jul 26 16:19 rust-gdb
-rwxr-xr-x 13 jan jan 15825920 Jul 26 16:19 rust-gdbgui
-rwxr-xr-x 13 jan jan 15825920 Jul 26 16:19 rust-lldb
-rwxr-xr-x 13 jan jan 15825920 Jul 26 16:19 rustup


[16:40:37] λ  ls -la .local/share/zinit/polaris/bin/ls
-rwxr-xr-x 1 jan jan 436 Jul 26 16:22 .local/share/zinit/polaris/bin/ls

[16:40:54] λ  cat .local/share/zinit/polaris/bin/ls
#!/usr/bin/env zsh

function ls {
    local bindir="/home/jan/.local/share/zinit/plugins/zdharma-continuum---null/bin"
    local -x PATH="/home/jan/.local/share/zinit/plugins/zdharma-continuum---null"/bin:"$PATH"
    local -x RUSTUP_HOME="/home/jan/.local/share/zinit/plugins/zdharma-continuum---null"/rustup CARGO_HOME="/home/jan/.local/share/zinit/plugins/zdharma-continuum---null"



    "$bindir"/"exa" "$@"

}

ls "$@"

But after opening a new shell:

[16:43:00] λ  which ls
/home/jan/.local/share/zinit/polaris/bin/ls

@jankatins jankatins force-pushed the dont_escape_exclamation_mark_in_hook_names branch from fc77c1c to a115aa3 Compare July 29, 2022 10:35
@psprint
Copy link
Contributor

psprint commented Jul 30, 2022 via email

@vladdoster
Copy link
Member

@psprint Thank you for a stamp of approval.

@jankatins Zunit tests failed due to network (API rate limiting). Kicked off failed zunit tests. Will merge this PR when they pass.

@jankatins Thanks again for the work on this fix. :^)

zunit runs tests non-interactively and these tests fail without the fix in this branch.
@jankatins jankatins force-pushed the dont_escape_exclamation_mark_in_hook_names branch from a115aa3 to 711916b Compare July 30, 2022 21:11
@jankatins
Copy link
Contributor Author

jankatins commented Jul 30, 2022

@vladdoster / @psprint Woohoo!

Restarted the CI by doing a force push

@vladdoster vladdoster merged commit c3d1bb5 into zdharma-continuum:main Aug 6, 2022
@jankatins
Copy link
Contributor Author

Woohoo, thank you!

psprint pushed a commit to psprint/.priv-zinit-fork that referenced this pull request Sep 20, 2022
…inuum#227)

* maint: Ignore .idea folder

Co-authored-by: Philipp Schmitt <pschmitt@users.noreply.github.com>

* fix: Don't escape exclamation marks in hook keys

Exclamation marks (per default) get escaped in interactive mode,
but not in non-interactive modes. In this case it meant that zinit
would not find any hooks which implements the ices when running in
non-interactive mode (e.g. in a script). This in turn would NOT result
in errors but would just silently not execute any of the ices which
of course would be a bad thing(tm).

zdharma-continuum#199 has the details...

Closes: zdharma-continuum#199

* fix: Ignore errors if cp cannot copy .zwc files

This would otherwise bubble up as a hook failure and further up as a zinit call failure

* fix: Add pick ice now that the compile hook is run

After the fix, the compile hook is now run and would fail without this information. IN addition, only files starting with _ are linked into the completion folder.

* fix: Fix docker-buildx gh-r test now that atclone ice is run

* maint: Add basic tests for cp/mv/atclone/make ices

zunit runs tests non-interactively and these tests fail without the fix in this branch.

Co-authored-by: Philipp Schmitt <pschmitt@users.noreply.github.com>
@psprint
Copy link
Contributor

psprint commented Oct 11, 2022 via email

@jankatins
Copy link
Contributor Author

jankatins commented Oct 11, 2022

@psprint Summary is in the description, copied below to get it into the mail. The original issue is fixed via c3d1bb5

Given that issue was undiscovered for ages, it seems a lot of ices are not covered by actual tests (e.g. test just adds an ice, but does not assure that the ice had an effect).

Exclamation marks (per default) get escaped in interactive mode, but not in non-interactive modes. In this case it meant that zinit would not find any hooks which implements the ices when running in non-interactive mode (e.g. in a script). This in turn would NOT result in errors but would just silently not execute any of the ices which of course would be a bad thing(tm).

This actually happened in the zunit tests as they run non-interactively and therefore any ices based on exclamation mark hooks were not ran. I fixed the few places where (after the fix in this PR) the ice were actually ran and errored due to mis-specifications. I also added explicit checks for a few ices. -> before this PR, ices are basically not tested (and the exclamation mark hook based ones not testable at all!) :-(

@vladdoster
Copy link
Member

@jankatins This broke patch-dl ice

@jankatins
Copy link
Contributor Author

It probably broke the annexes because I removed all \\\\ thingies, but forgot the place where it was still escaped (in the annex hook registering)

@@ -451,9 +451,9 @@ builtin source "${ZINIT[BIN_DIR]}/zinit-side.zsh" || {
# Store ices at clone of a plugin
.zinit-store-ices "$local_path/._zinit" ICE "" "" "" ""
reply=(
${(on)ZINIT_EXTS2[(I)zinit hook:\\\!atclone-pre <->]}
${(on)ZINIT_EXTS[(I)z-annex hook:\\\!atclone-<-> <->]}
Copy link
Contributor Author

@jankatins jankatins Oct 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vladdoster Probably this changed line broke zdharma-continuum/zinit-annex-patch-dl#6

... and the similar changes to ZINIT_EXTS lines below...

github-actions bot pushed a commit that referenced this pull request Nov 7, 2022
# [3.8.0](v3.7.0...v3.8.0) (2022-11-07)

### Bug Fixes

* account for systems where musl is present ([#269](#269)) ([8620574](8620574))
* alist repository for gh-r test ([#305](#305)) ([fb3c082](fb3c082))
* allow zinit to be run from non-interactive scripts ([#227](#227)) ([c3d1bb5](c3d1bb5)), closes [#199](#199)
* broken yaml syntax in issue template ([#355](#355)) ([f729e06](f729e06))
* calico gh-r zunit test ([#356](#356)) ([56fb9e0](56fb9e0))
* change ctags symbols browser key  binding from `ctrl-k` to `alt-Q` ([#387](#387)) ([7f6dc7d](7f6dc7d)), closes [#386](#386)
* Do not try to escape exclamation marks ([#399](#399)) ([0e55b2e](0e55b2e))
* docs workflow should fail if out-of-date ([#278](#278)) ([07cde66](07cde66))
* Don't error if $OPTS is not yet defined in .zinit-compinit call ([44765e0](44765e0))
* filter by runtime detected CPU before compiled CPU ([#304](#304)) ([a4dc13f](a4dc13f)), closes [#287](#287)
* gh-r & plugin zunit tests ([dd12fce](dd12fce))
* gh-r filters i686 (32 bit) for x86_64 ([#226](#226)) ([57f0d82](57f0d82)), closes [#225](#225)
* gh-r logic ignores [36]86 assets ([#235](#235)) ([d60638f](d60638f)), closes [#225](#225) [#246](#246) [#247](#247)
* gh-r removes linux32 assets on 64 bit OS ([1864c0b](1864c0b))
* gh-r retrieves release data GH REST API  ([#373](#373)) ([4a2a120](4a2a120)), closes [#374](#374)
* modify regex in gh-r for assets to not consider for selection ([#244](#244)) ([6ef8439](6ef8439))
* more cleaning up urls ([672ae51](672ae51)), closes [#47](#47)
* names of ctag Make target deps ([#407](#407)) ([9987d5c](9987d5c))
* package are broken again ([24f10f6](24f10f6))
* permissions for PR labeler GH action workflow ([#236](#236)) ([8a0d567](8a0d567))
* read without -r is generally bad. ([00c70a4](00c70a4))
* remove curl option "--tcp-fastopen" which is not always available ([#299](#299)) ([308c9d4](308c9d4))
* remove macOS 10.5 & 11 from test matrix ([c613193](c613193))
* remove use less line ([4f87076](4f87076))
* rename `docs` to `doc` to match doc dir ([#212](#212)) ([3a7dc95](3a7dc95))
* rm linux32 assets in aarch64/arm64 gh-r regex ([#414](#414)) ([529aa20](529aa20))
* syntax error when checking for `realpath` command  ([#259](#259)) ([05559eb](05559eb)), closes [#257](#257)
* trigger for PR labeler GH action workflow ([#237](#237)) ([49af866](49af866))
* typo & triggers in documentation workflow ([#308](#308)) ([161d7c1](161d7c1))
* unmatched "(" in windows gh-r patterns ([#280](#280)) ([1f4ba5a](1f4ba5a))
* update `zdharma` to `zdharma-continuum` ([66b1700](66b1700))
* update docs for new jq-check ([6207427](6207427))
* use [*] inside arbitrary strings. ([73a8c92](73a8c92))
* workflow pkg mgmt due to base OS changes ([195f72d](195f72d))
* ziextract execs discovery regex ([#410](#410)) ([105b38a](105b38a))
* zunit install in GH workflow ([#412](#412)) ([f4787dc](f4787dc))

### Features

* ability to set program for `zinit ls` to use ([#221](#221)) ([bad7af3](bad7af3)), closes [#170](#170)
* add `-a` (actual time) to `zinit times` cmd ([#223](#223)) ([450d3c1](450d3c1))
* add `krew` and `prebuilt-ripgrep` gh-r zunit tests ([#267](#267)) ([f25b4ae](f25b4ae))
* add compile vim from source zunit test ([#232](#232)) ([126528c](126528c))
* add configure"" ice ([#334](#334)) ([40a46c6](40a46c6))
* add GH action to remove old workflow logs ([#248](#248)) ([6647bdc](6647bdc))
* add PR labeler to show what parts of Zinit are changed ([#211](#211)) ([42e83d7](42e83d7))
* add releases via semantic-release ([73542b4](73542b4))
* add releases via semantic-release ([#415](#415)) ([cfa2f0e](cfa2f0e))
* expand linted file types to markdown and shell ([96fe03f](96fe03f))
* **git-process-output:** simplify progress-bar ([#204](#204)) ([c888917](c888917))
* update output messaging to be more informative ([047320a](047320a))
@github-actions
Copy link

github-actions bot commented Nov 7, 2022

🎉 This PR is included in version 3.8.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

psprint pushed a commit to psprint/.priv-zinit-fork that referenced this pull request Dec 30, 2022
…inuum#227)

* maint: Ignore .idea folder

Co-authored-by: Philipp Schmitt <pschmitt@users.noreply.github.com>

* fix: Don't escape exclamation marks in hook keys

Exclamation marks (per default) get escaped in interactive mode,
but not in non-interactive modes. In this case it meant that zinit
would not find any hooks which implements the ices when running in
non-interactive mode (e.g. in a script). This in turn would NOT result
in errors but would just silently not execute any of the ices which
of course would be a bad thing(tm).

zdharma-continuum#199 has the details...

Closes: zdharma-continuum#199

* fix: Ignore errors if cp cannot copy .zwc files

This would otherwise bubble up as a hook failure and further up as a zinit call failure

* fix: Add pick ice now that the compile hook is run

After the fix, the compile hook is now run and would fail without this information. IN addition, only files starting with _ are linked into the completion folder.

* fix: Fix docker-buildx gh-r test now that atclone ice is run

* maint: Add basic tests for cp/mv/atclone/make ices

zunit runs tests non-interactively and these tests fail without the fix in this branch.

Co-authored-by: Philipp Schmitt <pschmitt@users.noreply.github.com>
psprint pushed a commit to psprint/.priv-zinit-fork that referenced this pull request Dec 30, 2022
# [3.8.0](zdharma-continuum/zinit@v3.7.0...v3.8.0) (2022-11-07)

### Bug Fixes

* account for systems where musl is present ([zdharma-continuum#269](zdharma-continuum#269)) ([8620574](zdharma-continuum@8620574))
* alist repository for gh-r test ([zdharma-continuum#305](zdharma-continuum#305)) ([fb3c082](zdharma-continuum@fb3c082))
* allow zinit to be run from non-interactive scripts ([zdharma-continuum#227](zdharma-continuum#227)) ([c3d1bb5](zdharma-continuum@c3d1bb5)), closes [zdharma-continuum#199](zdharma-continuum#199)
* broken yaml syntax in issue template ([zdharma-continuum#355](zdharma-continuum#355)) ([f729e06](zdharma-continuum@f729e06))
* calico gh-r zunit test ([zdharma-continuum#356](zdharma-continuum#356)) ([56fb9e0](zdharma-continuum@56fb9e0))
* change ctags symbols browser key  binding from `ctrl-k` to `alt-Q` ([zdharma-continuum#387](zdharma-continuum#387)) ([7f6dc7d](zdharma-continuum@7f6dc7d)), closes [zdharma-continuum#386](zdharma-continuum#386)
* Do not try to escape exclamation marks ([zdharma-continuum#399](zdharma-continuum#399)) ([0e55b2e](zdharma-continuum@0e55b2e))
* docs workflow should fail if out-of-date ([zdharma-continuum#278](zdharma-continuum#278)) ([07cde66](zdharma-continuum@07cde66))
* Don't error if $OPTS is not yet defined in .zinit-compinit call ([44765e0](zdharma-continuum@44765e0))
* filter by runtime detected CPU before compiled CPU ([zdharma-continuum#304](zdharma-continuum#304)) ([a4dc13f](zdharma-continuum@a4dc13f)), closes [zdharma-continuum#287](zdharma-continuum#287)
* gh-r & plugin zunit tests ([dd12fce](zdharma-continuum@dd12fce))
* gh-r filters i686 (32 bit) for x86_64 ([zdharma-continuum#226](zdharma-continuum#226)) ([57f0d82](zdharma-continuum@57f0d82)), closes [zdharma-continuum#225](zdharma-continuum#225)
* gh-r logic ignores [36]86 assets ([zdharma-continuum#235](zdharma-continuum#235)) ([d60638f](zdharma-continuum@d60638f)), closes [zdharma-continuum#225](zdharma-continuum#225) [zdharma-continuum#246](zdharma-continuum#246) [zdharma-continuum#247](zdharma-continuum#247)
* gh-r removes linux32 assets on 64 bit OS ([1864c0b](zdharma-continuum@1864c0b))
* gh-r retrieves release data GH REST API  ([zdharma-continuum#373](zdharma-continuum#373)) ([4a2a120](zdharma-continuum@4a2a120)), closes [zdharma-continuum#374](zdharma-continuum#374)
* modify regex in gh-r for assets to not consider for selection ([zdharma-continuum#244](zdharma-continuum#244)) ([6ef8439](zdharma-continuum@6ef8439))
* more cleaning up urls ([672ae51](zdharma-continuum@672ae51)), closes [zdharma-continuum#47](zdharma-continuum#47)
* names of ctag Make target deps ([zdharma-continuum#407](zdharma-continuum#407)) ([9987d5c](zdharma-continuum@9987d5c))
* package are broken again ([24f10f6](zdharma-continuum@24f10f6))
* permissions for PR labeler GH action workflow ([zdharma-continuum#236](zdharma-continuum#236)) ([8a0d567](zdharma-continuum@8a0d567))
* read without -r is generally bad. ([00c70a4](zdharma-continuum@00c70a4))
* remove curl option "--tcp-fastopen" which is not always available ([zdharma-continuum#299](zdharma-continuum#299)) ([308c9d4](zdharma-continuum@308c9d4))
* remove macOS 10.5 & 11 from test matrix ([c613193](zdharma-continuum@c613193))
* remove use less line ([4f87076](zdharma-continuum@4f87076))
* rename `docs` to `doc` to match doc dir ([zdharma-continuum#212](zdharma-continuum#212)) ([3a7dc95](zdharma-continuum@3a7dc95))
* rm linux32 assets in aarch64/arm64 gh-r regex ([zdharma-continuum#414](zdharma-continuum#414)) ([529aa20](zdharma-continuum@529aa20))
* syntax error when checking for `realpath` command  ([zdharma-continuum#259](zdharma-continuum#259)) ([05559eb](zdharma-continuum@05559eb)), closes [zdharma-continuum#257](zdharma-continuum#257)
* trigger for PR labeler GH action workflow ([zdharma-continuum#237](zdharma-continuum#237)) ([49af866](zdharma-continuum@49af866))
* typo & triggers in documentation workflow ([zdharma-continuum#308](zdharma-continuum#308)) ([161d7c1](zdharma-continuum@161d7c1))
* unmatched "(" in windows gh-r patterns ([zdharma-continuum#280](zdharma-continuum#280)) ([1f4ba5a](zdharma-continuum@1f4ba5a))
* update `zdharma` to `zdharma-continuum` ([66b1700](zdharma-continuum@66b1700))
* update docs for new jq-check ([6207427](zdharma-continuum@6207427))
* use [*] inside arbitrary strings. ([73a8c92](zdharma-continuum@73a8c92))
* workflow pkg mgmt due to base OS changes ([195f72d](zdharma-continuum@195f72d))
* ziextract execs discovery regex ([zdharma-continuum#410](zdharma-continuum#410)) ([105b38a](zdharma-continuum@105b38a))
* zunit install in GH workflow ([zdharma-continuum#412](zdharma-continuum#412)) ([f4787dc](zdharma-continuum@f4787dc))

### Features

* ability to set program for `zinit ls` to use ([zdharma-continuum#221](zdharma-continuum#221)) ([bad7af3](zdharma-continuum@bad7af3)), closes [zdharma-continuum#170](zdharma-continuum#170)
* add `-a` (actual time) to `zinit times` cmd ([zdharma-continuum#223](zdharma-continuum#223)) ([450d3c1](zdharma-continuum@450d3c1))
* add `krew` and `prebuilt-ripgrep` gh-r zunit tests ([zdharma-continuum#267](zdharma-continuum#267)) ([f25b4ae](zdharma-continuum@f25b4ae))
* add compile vim from source zunit test ([zdharma-continuum#232](zdharma-continuum#232)) ([126528c](zdharma-continuum@126528c))
* add configure"" ice ([zdharma-continuum#334](zdharma-continuum#334)) ([40a46c6](zdharma-continuum@40a46c6))
* add GH action to remove old workflow logs ([zdharma-continuum#248](zdharma-continuum#248)) ([6647bdc](zdharma-continuum@6647bdc))
* add PR labeler to show what parts of Zinit are changed ([zdharma-continuum#211](zdharma-continuum#211)) ([42e83d7](zdharma-continuum@42e83d7))
* add releases via semantic-release ([73542b4](zdharma-continuum@73542b4))
* add releases via semantic-release ([zdharma-continuum#415](zdharma-continuum#415)) ([cfa2f0e](zdharma-continuum@cfa2f0e))
* expand linted file types to markdown and shell ([96fe03f](zdharma-continuum@96fe03f))
* **git-process-output:** simplify progress-bar ([zdharma-continuum#204](zdharma-continuum#204)) ([c888917](zdharma-continuum@c888917))
* update output messaging to be more informative ([047320a](zdharma-continuum@047320a))
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.

[bug]: atclone/mv/... are not run when installing plugins non-interactively
4 participants