`antigen init` fails generating cache when path includes spaces

davidstosik commented 2 years ago

Description

antigen init fails with the following error when the path to antigenrc includes spaces:

-antigen-cache-generate:zcompile:66: can't write zwc file: {broken path}/.antigenrc.zwc

Steps to reproduce

Here's how to reproduce on Docker, with a minimal setup:

# Dockerfile
FROM ubuntu:latest

RUN apt-get update && apt-get install -y \
  git \
  zsh

COPY .zshrc /root
COPY [".antigenrc", "/antigen space/"]
RUN ["git", "clone", "https://github.com/zsh-users/antigen.git", "/antigen space/git"]

WORKDIR /root
ENTRYPOINT zsh

# .zshrc
source "/antigen space/git/antigen.zsh"
antigen init "/antigen space/.antigenrc"

# .antigenrc
antigen use oh-my-zsh
antigen theme robbyrussell
antigen apply

Having created the three files above in the same directory, use the following commands to build and run the docker image:

docker build . -t antigen-test
docker run -it --rm antigen-test

The result I get is:

Installing robbyrussell/oh-my-zsh!...
-antigen-cache-generate:zcompile:66: can't write zwc file: space/.antigenrc.zwc

Note how the zwc file's path is broken: space/.antigenrc.zwc.

Software version

antigen version develop (d1dd78b)
zsh --version zsh 5.8 (aarch64-unknown-linux-gnu)
uname -a Linux f3c4b1e2289b 5.10.76-linuxkit #1 SMP PREEMPT Mon Nov 8 11:22:26 UTC 2021 aarch64 aarch64 aarch64 GNU/Linux

Configuration

See steps to reproduce above.

More information

I understand antigen is pretty much unmaintained at this point, but I thought I'd share this problem in case someone else tries to look it up...

davidstosik commented 2 years ago

This for loop here breaks on white spaces and ends up calling zcompile space/.antigenrc (whereas the complete path was /antigen space/.antigenrc).

https://github.com/zsh-users/antigen/blob/64de2dcd95d6a8e879cd2244c763d99f0144e78e/bin/antigen.zsh#L1866-L1867

(Update: actually the loop works because $ANTIGEN_CHECK_FILES is an array? 🤔 )

Here's another thing that shows the problem:

# cat /root/.antigen/.resources 
/root/.zshrc
space/.antigenrc

The /antigen part in /antigen space/.antigenrc's path got mangled.

davidstosik commented 2 years ago

Here's the part to blame (interestingly includes a TODO 😅 ):

https://github.com/zsh-users/antigen/blob/64de2dcd95d6a8e879cd2244c763d99f0144e78e/bin/antigen.zsh#L1918-L1921

> trace="/antigen space/.antigenrc:2"
> echo ${${trace%:*}##* }
space/.antigenrc

(Honestly no idea what to do with this. 😅 )

Alright, I had to read some doc, but here I am. (It's super hard to find the right piece of doc by the way!)

${trace%:*} looks like the ${name%pattern} expansion syntax:

If the pattern matches the end of the value of name, then substitute the value of name with the matched portion deleted; otherwise, just substitute the value of name. In the first form, the smallest matching pattern is preferred; in the second form, the largest matching pattern is preferred.

This is meant to delete the :2 portion of the trace line.
${path##* } looks like the ${name##pattern} expansion syntax:

If the pattern matches the beginning of the value of name, then substitute the value of name with the matched portion deleted; otherwise, just substitute the value of name. In the first form, the smallest matching pattern is preferred; in the second form, the largest matching pattern is preferred.

This seems to delete "everything until the last space at the beginning of the string".

It's hard to understand whether the expansion that deletes until the last space is required or not, or why it's there. It was original introduced in this commit, and both the commit and associated PR lack details that could explain why it's necessary... 🤔

For what it's worth, this diff seemed to fix it for my very simplistic setup (I can't imagine it won't break anything though 😰):

diff --git i/bin/antigen.zsh w/bin/antigen.zsh
index aeba2a7..f0d83d9 100644
--- i/bin/antigen.zsh
+++ w/bin/antigen.zsh
@@ -1917,7 +1917,7 @@ EOC
       fi
       # TODO Fix: Fuzzy match shoud be replaced by a sane way to determine it.
       if [[ $#funcfiletrace -ge 6 ]]; then
-        ANTIGEN_CHECK_FILES+=("${${funcfiletrace[6]%:*}##* }")
+        ANTIGEN_CHECK_FILES+=("${funcfiletrace[6]%:*}")
       fi
     fi

diff --git i/src/ext/cache.zsh w/src/ext/cache.zsh
index 248638d..85f1d04 100644
--- i/src/ext/cache.zsh
+++ w/src/ext/cache.zsh
@@ -133,7 +133,7 @@ EOC
       fi
       # TODO Fix: Fuzzy match shoud be replaced by a sane way to determine it.
       if [[ $#funcfiletrace -ge 6 ]]; then
-        ANTIGEN_CHECK_FILES+=("${${funcfiletrace[6]%:*}##* }")
+        ANTIGEN_CHECK_FILES+=("${funcfiletrace[6]%:*}")
       fi
     fi

zsh-users / antigen