diff --git a/.github/workflows/zsh-n.yml b/.github/workflows/zsh-n.yml index 7d62b5f..3f52667 100644 --- a/.github/workflows/zsh-n.yml +++ b/.github/workflows/zsh-n.yml @@ -46,6 +46,7 @@ on: - "tests/plugin-autoload-ownership.zsh" - "tests/plugin-standard-callbacks.zsh" - "tests/release-tag-verification.zsh" + - "tests/repeated-load-ownership.zsh" - "tests/scheduler-idle.zsh" - "tests/fixtures/plugin-standard-callbacks/**" - "tests/self-update-reload.zsh" @@ -114,6 +115,17 @@ jobs: - name: Test unload ownership contracts run: zsh -f tests/unload-ownership-contracts.zsh + repeated-load-ownership: + name: Repeated Load Ownership + runs-on: ubuntu-latest + steps: + - name: Check out code + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - name: Install Zsh + run: sudo apt update && sudo apt-get install -yq zsh + - name: Test ownership across repeated loads of one plugin + run: zsh -f tests/repeated-load-ownership.zsh + unload-hook-dispatch: name: Unload Hook Dispatch runs-on: ubuntu-latest diff --git a/lib/zsh/autoload.zsh b/lib/zsh/autoload.zsh index 43623ab..ae8815d 100755 --- a/lib/zsh/autoload.zsh +++ b/lib/zsh/autoload.zsh @@ -262,6 +262,7 @@ ZI[EXTENDED_GLOB]="" ZI[FUNCTIONS__$REPLY]="" ZI[FUNCTIONS_BEFORE__$REPLY]="" ZI[FUNCTIONS_AFTER__$REPLY]="" + ZI[FUNCTIONS_OWNED__$REPLY]="" # Option diffing ZI[OPTIONS__$REPLY]="" ZI[OPTIONS_BEFORE__$REPLY]="" @@ -1137,8 +1138,9 @@ ZI[EXTENDED_GLOB]="" integer found_idx2="${entry_splitted2[(I)*\ $orig_saved1\ *]}" if (( found_idx || found_idx2 )) then - # Skip multiple loads of the same plugin - # TODO: #113 Fully handle multiple plugin loads + # Skip later loads of the same plugin. A repeated load keeps the + # first load's records unless another plugin took them over in + # between, which is still open in z-shell/zi#113. if [[ "$oth_uspl2" != "$uspl2" ]]; then to_process_plugin="$oth_uspl2" break # Only the first one is needed @@ -1229,6 +1231,24 @@ ZI[EXTENDED_GLOB]="" .zi-diff-functions-compute "$uspl2" typeset -a func func=( "${(z)ZI[FUNCTIONS__$uspl2]}" ) + # Functions earlier loads of this plugin created (z-shell/zi#113), except + # those another registered plugin created as well, on any of its loads. + () { + builtin setopt local_options extended_glob + local owned other_uspl2 + local -a others_created + [[ -n ${ZI[FUNCTIONS_OWNED__$uspl2]} ]] || return 0 + for other_uspl2 in "${ZI_REGISTERED_PLUGINS[@]}"; do + [[ $other_uspl2 == "$uspl2" ]] && continue + .zi-diff-functions-compute "$other_uspl2" 2>/dev/null + others_created+=( "${(z)ZI[FUNCTIONS__$other_uspl2]}" "${(z)ZI[FUNCTIONS_OWNED__$other_uspl2]}" ) + done + for owned in "${(z)ZI[FUNCTIONS_OWNED__$uspl2]}"; do + [[ -z $owned ]] && continue + (( ${others_created[(Ie)$owned]} )) && continue + (( ${func[(Ie)$owned]} )) || func+=( "$owned" ) + done + } local f for f in "${(on)func[@]}"; do [[ -z "$f" ]] && continue diff --git a/tests/repeated-load-ownership.zsh b/tests/repeated-load-ownership.zsh new file mode 100644 index 0000000..a4d925a --- /dev/null +++ b/tests/repeated-load-ownership.zsh @@ -0,0 +1,219 @@ +#!/usr/bin/env zsh +# -*- mode: zsh; sh-indentation: 2; indent-tabs-mode: nil; sh-basic-offset: 2; -*- +# vim: ft=zsh sw=2 ts=2 et +# +# z-shell/zi#113: a repeated load of the same plug-in, then one unload, must +# remove everything the plug-in added across its loads and restore what it +# replaced. When another plug-in took the plug-in's widget or binding over +# between its loads, the result must stay as it is on next: that case is still +# open in the issue. Each scenario runs in its own clean shell. + +builtin emulate -R zsh +setopt pipe_fail + +fail() { + builtin print -u2 -r -- "not ok - $1" + exit 1 +} + +typeset project_root="${ZI_TEST_CHECKOUT:-${0:A:h:h}}" +typeset temp_root +temp_root="$(command mktemp -d "${TMPDIR:-/tmp}/zi-repeated-load-test.XXXXXXXX")" || + fail "create temporary directory" +trap 'command rm -rf -- "$temp_root"' EXIT INT TERM + +# write_plugin DIR FUNCTION: a plug-in that defines FUNCTION, makes it the +# zi-repeat-widget widget and binds ^X^P to that widget. +write_plugin() { + command mkdir -p "${temp_root}/$1" || fail "create the $1 plug-in directory" + builtin print -rl -- \ + "$2() { builtin print -r -- $2; }" \ + "zle -N zi-repeat-widget $2" \ + "bindkey '^X^P' zi-repeat-widget" \ + > "${temp_root}/$1/$1.plugin.zsh" || fail "write the $1 plug-in" +} + +# run_case NAME: run the scenario NAME in a fresh isolated shell. +run_case() { + local case_root="${temp_root}/$1" + command mkdir -p "${case_root}"/{home,cache,config,data,zdotdir} || fail "$1: create isolated environment" + env \ + HOME="${case_root}/home" \ + XDG_CACHE_HOME="${case_root}/cache" \ + XDG_CONFIG_HOME="${case_root}/config" \ + XDG_DATA_HOME="${case_root}/data" \ + ZDOTDIR="${case_root}/zdotdir" \ + ZI_TEST_CHECKOUT="$project_root" \ + ZI_TEST_ROOT="$temp_root" \ + ZI_TEST_CASE="$1" \ + zsh -f <<'ZSH' || fail "$1" +builtin emulate -R zsh +setopt pipe_fail + +builtin source "${ZI_TEST_CHECKOUT}/zi.zsh" || return 1 +.zi-prepare-home || return 1 + +check() { + eval "$1" && return 0 + builtin print -u2 -r -- "${ZI_TEST_CASE}: $2" + return 1 +} + +typeset binding_before="$(bindkey '^X^P')" +typeset p="${ZI_TEST_ROOT}/p" q="${ZI_TEST_ROOT}/q" + +case $ZI_TEST_CASE in + twice) + # The same plug-in loaded twice, then unloaded once. + zi load "$p" >/dev/null 2>&1 + zi load "$p" >/dev/null 2>&1 + check '(( ${+functions[pa_fn]} ))' "the loads did not define the function" || return 1 + check '[[ ${widgets[zi-repeat-widget]} == user:pa_fn ]]' "the loads did not define the widget: ${widgets[zi-repeat-widget]}" || return 1 + zi unload "$p" >/dev/null 2>&1 + check '(( ! ${+functions[pa_fn]} ))' "the function survived unload" || return 1 + check '[[ -z ${widgets[zi-repeat-widget]} ]]' "the widget survived unload: ${widgets[zi-repeat-widget]}" || return 1 + check '[[ "$(bindkey "^X^P")" == "$binding_before" ]]' "the binding was not restored: $(bindkey '^X^P')" || return 1 + ;; + changed) + # The plug-in gains a function between its loads; both must go. + zi load "$p" >/dev/null 2>&1 + builtin print -r -- 'pb_fn() { :; }' >> "${p}/p.plugin.zsh" + zi load "$p" >/dev/null 2>&1 + check '(( ${+functions[pa_fn]} && ${+functions[pb_fn]} ))' "the loads did not define both functions" || return 1 + zi unload "$p" >/dev/null 2>&1 + check '(( ! ${+functions[pa_fn]} ))' "the first load's function survived unload" || return 1 + check '(( ! ${+functions[pb_fn]} ))' "the second load's function survived unload" || return 1 + check '[[ -z ${widgets[zi-repeat-widget]} ]]' "the widget survived unload: ${widgets[zi-repeat-widget]}" || return 1 + check '[[ "$(bindkey "^X^P")" == "$binding_before" ]]' "the binding was not restored: $(bindkey '^X^P')" || return 1 + ;; + interleaved) + # Another plug-in takes the widget and binding between the two loads. + # That case stays open in z-shell/zi#113; the repeated load must leave it + # exactly as next does: p's second load records afresh, its unload hands + # the widget back to what q replaced, and q's unload then restores q's. + zi load "$p" >/dev/null 2>&1 + zi load "$q" >/dev/null 2>&1 + zi load "$p" >/dev/null 2>&1 + zi unload "$p" >/dev/null 2>&1 + check '(( ${+functions[qa_fn]} ))' "q's function was removed" || return 1 + check '[[ ${widgets[zi-repeat-widget]} == user:pa_fn ]]' "the widget differs from next: ${widgets[zi-repeat-widget]}" || return 1 + check '[[ "$(bindkey "^X^P")" == "\"^X^P\" zi-repeat-widget" ]]' "the binding differs from next: $(bindkey '^X^P')" || return 1 + ;; + interleaved-other-order) + # The same, unloading q first: p keeps its live widget, as on next. + zi load "$p" >/dev/null 2>&1 + zi load "$q" >/dev/null 2>&1 + zi load "$p" >/dev/null 2>&1 + zi unload "$q" >/dev/null 2>&1 + check '(( ${+functions[pa_fn]} ))' "p's function was removed by q's unload" || return 1 + check '[[ ${widgets[zi-repeat-widget]} == user:pa_fn ]]' "q's unload changed p's live widget: ${widgets[zi-repeat-widget]}" || return 1 + check '[[ "$(bindkey "^X^P")" == "\"^X^P\" zi-repeat-widget" ]]' "q's unload changed p's binding: $(bindkey '^X^P')" || return 1 + ;; + older) + # q loaded before p's first load is not a takeover between p's loads: + # p's repeated load still keeps its records, and its unload removes it. + zi load "$q" >/dev/null 2>&1 + zi load "$p" >/dev/null 2>&1 + zi load "$p" >/dev/null 2>&1 + zi unload "$p" >/dev/null 2>&1 + check '(( ! ${+functions[pa_fn]} ))' "p's function survived unload" || return 1 + check '[[ ${widgets[zi-repeat-widget]} == user:qa_fn ]]' "q's widget was not restored: ${widgets[zi-repeat-widget]}" || return 1 + ;; + binding-taken) + # Another plug-in rebinds only the key between p's loads: like a widget + # takeover, the result stays as on next. + zi load "$p" >/dev/null 2>&1 + zi load "${ZI_TEST_ROOT}/k" >/dev/null 2>&1 + zi load "$p" >/dev/null 2>&1 + zi unload "$p" >/dev/null 2>&1 + check '[[ "$(bindkey "^X^P")" == "\"^X^P\" end-of-line" ]]' "the binding differs from next: $(bindkey '^X^P')" || return 1 + ;; + main-taken) + # The same, with the key rebound through `bindkey -M main`: main is the + # default keymap, so this is the same takeover. + zi load "$p" >/dev/null 2>&1 + zi load "${ZI_TEST_ROOT}/m" >/dev/null 2>&1 + zi load "$p" >/dev/null 2>&1 + zi unload "$p" >/dev/null 2>&1 + check '[[ "$(bindkey "^X^P")" == "\"^X^P\" end-of-line" ]]' "the binding differs from next: $(bindkey '^X^P')" || return 1 + ;; + prior) + # A function the user defined before any load is not the plug-in's. + pa_fn() { builtin print -r -- user; } + zi load "$p" >/dev/null 2>&1 + zi load "$p" >/dev/null 2>&1 + check '[[ ${widgets[zi-repeat-widget]} == user:pa_fn ]]' "the loads did not run the plug-in: ${widgets[zi-repeat-widget]}" || return 1 + zi unload "$p" >/dev/null 2>&1 + check '(( ${+functions[pa_fn]} ))' "a function defined before the first load was removed" || return 1 + ;; + wrapped) + # A plug-in that replaces an existing widget, loaded twice, restores the + # original widget on unload, not its own first replacement. + zi load "${ZI_TEST_ROOT}/w" >/dev/null 2>&1 + zi load "${ZI_TEST_ROOT}/w" >/dev/null 2>&1 + check '[[ ${widgets[forward-char]} == user:w_fn ]]' "the loads did not replace the widget: ${widgets[forward-char]}" || return 1 + zi unload "${ZI_TEST_ROOT}/w" >/dev/null 2>&1 + check '[[ ${widgets[forward-char]} == builtin ]]' "the replaced widget was not restored: ${widgets[forward-char]}" || return 1 + check '(( ! ${+functions[w_fn]} ))' "the replacement function survived unload" || return 1 + ;; + shared) + # p defines a helper on its first load; r, loaded next, defines the same + # helper again. After p's repeated load and unload, r still owns it. + builtin print -r -- 'shared_fn() { :; }' >> "${p}/p.plugin.zsh" + zi load "$p" >/dev/null 2>&1 + unfunction shared_fn + zi load "${ZI_TEST_ROOT}/r" >/dev/null 2>&1 + zi load "$p" >/dev/null 2>&1 + zi unload "$p" >/dev/null 2>&1 + check '(( ${+functions[shared_fn]} ))' "a function the still-loaded r created was removed" || return 1 + check '(( ! ${+functions[pa_fn]} ))' "p's function survived unload" || return 1 + ;; + shared-twice) + # As shared, but r is itself loaded twice, so r's claim on the helper + # comes from r's earlier load, not from its newest one. + builtin print -r -- 'shared_fn() { :; }' >> "${p}/p.plugin.zsh" + zi load "$p" >/dev/null 2>&1 + unfunction shared_fn + zi load "${ZI_TEST_ROOT}/r" >/dev/null 2>&1 + zi load "${ZI_TEST_ROOT}/r" >/dev/null 2>&1 + zi load "$p" >/dev/null 2>&1 + zi unload "$p" >/dev/null 2>&1 + check '(( ${+functions[shared_fn]} ))' "a function the still-loaded r created on its earlier load was removed" || return 1 + check '(( ! ${+functions[pa_fn]} ))' "p's function survived unload" || return 1 + ;; + reload) + # Load, unload, load again, unload: each cycle is independent. + zi load "$p" >/dev/null 2>&1 + zi unload "$p" >/dev/null 2>&1 + zi load "$p" >/dev/null 2>&1 + check '[[ ${widgets[zi-repeat-widget]} == user:pa_fn ]]' "the second cycle did not install the widget: ${widgets[zi-repeat-widget]}" || return 1 + zi unload "$p" >/dev/null 2>&1 + check '(( ! ${+functions[pa_fn]} ))' "the function survived the second unload" || return 1 + check '[[ -z ${widgets[zi-repeat-widget]} ]]' "the widget survived the second unload: ${widgets[zi-repeat-widget]}" || return 1 + check '[[ "$(bindkey "^X^P")" == "$binding_before" ]]' "the binding was not restored after the second unload: $(bindkey '^X^P')" || return 1 + ;; + *) + builtin print -u2 -r -- "unknown case: ${ZI_TEST_CASE}" + return 1 + ;; +esac +ZSH +} + +typeset -i failures=0 +typeset scenario +for scenario in twice changed interleaved interleaved-other-order older binding-taken main-taken prior wrapped shared shared-twice reload; do + write_plugin p pa_fn + write_plugin q qa_fn + command mkdir -p "${temp_root}/r" "${temp_root}/k" "${temp_root}/m" "${temp_root}/w" || fail "create the r, k, m and w plug-in directories" + builtin print -r -- 'shared_fn() { :; }' > "${temp_root}/r/r.plugin.zsh" || fail "write the r plug-in" + builtin print -r -- "bindkey '^X^P' end-of-line" > "${temp_root}/k/k.plugin.zsh" || fail "write the k plug-in" + builtin print -r -- "bindkey -M main '^X^P' end-of-line" > "${temp_root}/m/m.plugin.zsh" || fail "write the m plug-in" + builtin print -rl -- 'w_fn() { zle .forward-char; }' 'zle -N forward-char w_fn' > "${temp_root}/w/w.plugin.zsh" || fail "write the w plug-in" + if ( run_case "$scenario" ); then + builtin print -r -- "ok - ${scenario}" + else + (( failures++ )) + fi +done +(( failures == 0 )) || fail "${failures} repeated-load scenario(s) failed" diff --git a/tests/unload-ownership-contracts.zsh b/tests/unload-ownership-contracts.zsh index b44a949..6560fba 100644 --- a/tests/unload-ownership-contracts.zsh +++ b/tests/unload-ownership-contracts.zsh @@ -9,8 +9,9 @@ # per load instance rather than by plug-in id. Measuring the current behaviour # first showed the defect is narrower than that issue's prose: every # multi-plug-in path already restores the correct previous owner, and only a -# repeated load of the *same* plug-in leaks. The repeated-load case is therefore -# deliberately absent here; it belongs with its fix, not ahead of it. +# repeated load of the *same* plug-in leaked. That case is covered by +# tests/repeated-load-ownership.zsh, together with its fix; a plug-in taken +# over between two of its own loads is still open there. builtin emulate -R zsh setopt pipe_fail diff --git a/zi.zsh b/zi.zsh index d209cf5..fc880e4 100644 --- a/zi.zsh +++ b/zi.zsh @@ -1189,20 +1189,118 @@ builtin setopt no_aliases # Full or light load? [[ $mode == light ]] && ZI[STATES__$uspl2]=1 || ZI[STATES__$uspl2]=2 ZI_REPORTS[$uspl2]= ZI_CUR_BIND_MAP=( empty 1 ) + if (( ret )) && ! .zi-repeat-taken-over "$uspl2"; then + # A repeated load keeps what earlier loads of this plugin created, so one + # unload still removes it (z-shell/zi#113): the functions the previous + # load added join the owned set, and the widget and bindkey records stay. + # When another plugin took one of those widgets or bindings over in + # between, the records start again as before; that case remains open. + .zi-keep-previous-load-functions "$uspl2" + else + ZI[FUNCTIONS_OWNED__$uspl2]= + ZI[BINDKEYS__$uspl2]= + ZI[WIDGETS_SAVED__$uspl2]= ZI[WIDGETS_DELETE__$uspl2]= + fi # Functions. ZI[FUNCTIONS_BEFORE__$uspl2]= ZI[FUNCTIONS_AFTER__$uspl2]= ZI[FUNCTIONS__$uspl2]= # Objects. - ZI[ZSTYLES__$uspl2]= ZI[BINDKEYS__$uspl2]= + ZI[ZSTYLES__$uspl2]= ZI[ALIASES__$uspl2]= - # Widgets. - ZI[WIDGETS_SAVED__$uspl2]= ZI[WIDGETS_DELETE__$uspl2]= # Rest (options and (f)path). ZI[OPTIONS__$uspl2]= ZI[PATH__$uspl2]= ZI[OPTIONS_BEFORE__$uspl2]= ZI[OPTIONS_AFTER__$uspl2]= ZI[FPATH__$uspl2]= return ret } # ]]] +# FUNCTION: .zi-keep-previous-load-functions. [[[ +# Adds the functions the previous load of plugin $1 created to its owned set, +# before the next load takes a fresh function baseline. +.zi-keep-previous-load-functions() { + builtin emulate -LR zsh ${=${options[xtrace]:#off}:+-o xtrace} + local uspl2="$1" f + [[ ${ZI[FUNCTIONS_BEFORE__$uspl2]} == *[^[:space:]]* ]] || return 0 + local -A created + for f in "${(z)ZI[FUNCTIONS_AFTER__$uspl2]}"; do + created[$f]=1 + done + for f in "${(z)ZI[FUNCTIONS_BEFORE__$uspl2]}"; do + created[$f]=0 + done + for f in "${(@k)created}"; do + [[ ${created[$f]} == 1 ]] && ZI[FUNCTIONS_OWNED__$uspl2]+="$f " + done + return 0 +} # ]]] +# FUNCTION: .zi-repeat-object-key. [[[ +# Sets REPLY to a comparable identity for record $2 of kind $1 (widget-saved, +# widget-delete or bindkey): "widget NAME", or "bindkey MAP KEY" with an empty +# MAP for the default keymap, also when it is named main. Fails for a record +# without such an object. +.zi-repeat-object-key() { + local kind="$1" entry="$2" map + local -a fields + REPLY= + case $kind in + widget-saved) + fields=( "${(z)${(Q)entry}}" ) + REPLY="widget ${(Q)fields[2]}" ;; + widget-delete) + REPLY="widget ${(Q)entry}" ;; + bindkey) + fields=( "${(z)${(Q)entry}}" ) + [[ ${(Q)fields[4]} == -[AN] ]] && return 1 + # main is the default keymap: `bindkey -M main KEY` and `bindkey KEY` + # change the same binding, so they share one identity. + map= + [[ ${(Q)fields[4]} == -M ]] && map=${(Q)fields[5]} + [[ $map == main ]] && map= + REPLY="bindkey $map ${fields[1]}" ;; + *) return 1 ;; + esac + [[ -n $REPLY ]] +} # ]]] +# FUNCTION: .zi-repeat-objects. [[[ +# Sets reply to the widget and bindkey identities plugin $1 has recorded. +.zi-repeat-objects() { + local uspl2="$1" entry + reply=() + for entry in "${(z)ZI[WIDGETS_SAVED__$uspl2]}"; do + [[ -n $entry ]] && .zi-repeat-object-key widget-saved "$entry" && reply+=( "$REPLY" ) + done + for entry in "${(z)ZI[WIDGETS_DELETE__$uspl2]}"; do + [[ -n $entry ]] && .zi-repeat-object-key widget-delete "$entry" && reply+=( "$REPLY" ) + done + for entry in "${(z)ZI[BINDKEYS__$uspl2]}"; do + [[ -n $entry ]] && .zi-repeat-object-key bindkey "$entry" && reply+=( "$REPLY" ) + done +} # ]]] +# FUNCTION: .zi-repeat-taken-over. [[[ +# Succeeds when a plugin loaded after the newest load of plugin $1 recorded +# one of the widgets or bindings $1 recorded, i.e. took it over in between. +.zi-repeat-taken-over() { + builtin emulate -LR zsh ${=${options[xtrace]:#off}:+-o xtrace} + builtin setopt extended_glob + local uspl2="$1" key other REPLY + local -a reply mine shared + .zi-repeat-objects "$uspl2" + mine=( "${reply[@]}" ) + (( ${#mine} )) || return 1 + integer last=0 + for key in ${(k)ZI[(I)TIME_<->_${(b)uspl2//\//---}]}; do + [[ $key == (#b)TIME_(<->)_* ]] && (( match[1] > last )) && last=${match[1]} + done + for key in ${(k)ZI[(I)TIME_<->_*]}; do + [[ $key == (#b)TIME_(<->)_(*) ]] || continue + (( match[1] > last )) || continue + other="${match[2]//---//}" + [[ $other == "$uspl2" ]] && continue + .zi-repeat-objects "$other" + shared=( "${(@)reply:*mine}" ) + (( ${#shared} )) && return 0 + done + return 1 +} # ]]] # FUNCTION: .zi-get-object-path. [[[ .zi-get-object-path() { local type="$1" id_as="$2" local_dir dirname