Skip to content

Commit

Permalink
core: refresh zcompdump cache file in init script (#8878)
Browse files Browse the repository at this point in the history
  • Loading branch information
mcornella committed May 4, 2020
1 parent 173d4ca commit dd1a726
Showing 1 changed file with 19 additions and 0 deletions.
19 changes: 19 additions & 0 deletions oh-my-zsh.sh
Expand Up @@ -61,6 +61,18 @@ if [ -z "$ZSH_COMPDUMP" ]; then
ZSH_COMPDUMP="${ZDOTDIR:-${HOME}}/.zcompdump-${SHORT_HOST}-${ZSH_VERSION}"
fi

# Construct zcompdump OMZ metadata
zcompdump_metadata="\
#omz revision: $(cd -q "$ZSH"; git rev-parse HEAD 2>/dev/null)
#omz fpath: $fpath\
"

# Delete the zcompdump file if OMZ zcompdump metadata changed
if ! cmp -s <(command grep '^#omz' "$ZSH_COMPDUMP" 2>/dev/null) <<< "$zcompdump_metadata"; then
command rm -f "$ZSH_COMPDUMP"
zcompdump_refresh=1
fi

if [[ $ZSH_DISABLE_COMPFIX != true ]]; then
source $ZSH/lib/compfix.zsh
# If completion insecurities exist, warn the user
Expand All @@ -72,6 +84,13 @@ else
compinit -u -C -d "${ZSH_COMPDUMP}"
fi

# Append zcompdump metadata if missing
if (( $zcompdump_refresh )); then
echo "\n$zcompdump_metadata" >>! "$ZSH_COMPDUMP"
fi

unset zcompdump_metadata zcompdump_refresh


# Load all of the config files in ~/oh-my-zsh that end in .zsh
# TIP: Add files you don't want in git to .gitignore
Expand Down

15 comments on commit dd1a726

@ci4ic4
Copy link

@ci4ic4 ci4ic4 commented on dd1a726 May 4, 2020

Choose a reason for hiding this comment

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

Please revert. This commit stops proper work of this script on any system except Linux.

cmp on all the BSD's does not have the option of reading from stdin as GNU cmp.

Or modify the cmp invocation, avoiding <<<.

@mcornella
Copy link
Member Author

Choose a reason for hiding this comment

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

Does using <() fix it? It works on my end:

diff --git a/oh-my-zsh.sh b/oh-my-zsh.sh
index 09209932..285caf66 100644
--- a/oh-my-zsh.sh
+++ b/oh-my-zsh.sh
@@ -68,7 +68,7 @@ zcompdump_metadata="\
 "

 # Delete the zcompdump file if OMZ zcompdump metadata changed
-if ! cmp -s <(command grep '^#omz' "$ZSH_COMPDUMP" 2>/dev/null) <<< "$zcompdump_metadata"; then
+if ! cmp -s <(command grep '^#omz' "$ZSH_COMPDUMP" 2>/dev/null) <(echo "$zcompdump_metadata"); then
   command rm -f "$ZSH_COMPDUMP"
   zcompdump_refresh=1

@ci4ic4
Copy link

@ci4ic4 ci4ic4 commented on dd1a726 May 5, 2020

Choose a reason for hiding this comment

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

Yes, that's fine. Thanks.

@mcornella
Copy link
Member Author

@mcornella mcornella commented on dd1a726 May 5, 2020

Choose a reason for hiding this comment

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

Done, you can update with upgrade_oh_my_zsh. Thanks for the bug report!

@shoopdawoop
Copy link

Choose a reason for hiding this comment

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

I am still getting this on every launch on macOS 10.13.x (and I am only guessing it's related?):

/Users/user/.oh-my-zsh/oh-my-zsh.sh:89: no such file or directory: /Users/user/.zcompdump-¯\_(ツ)_/¯-5.3

Yes, that's the machines name, don't make me change it.

@mcornella
Copy link
Member Author

Choose a reason for hiding this comment

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

That didn't work before the change, did it?

@mcornella
Copy link
Member Author

Choose a reason for hiding this comment

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

What does echo $HOST ${(%):-%m} show?

@shoopdawoop
Copy link

Choose a reason for hiding this comment

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

It did, in fact I never had an issue until recently (and I did not rename the machine).
But I don't know for sure when I updated the last time, but I usually update every day.
The Hostname is different (uppercase and lowercase letters only).

@shoopdawoop
Copy link

Choose a reason for hiding this comment

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

I just gave it a try on another Mac with the same name (running macOS 10.14.x) that I had not updated and I did not have the issue there.
A cat .git/FETCH_HEAD showed it was still on 173d4ca68f1ff4b04e9f3fd783244c309d848092.

After I updated (Fast-forwarded master to 0736a3749a9c9ae4ba3096b0b6c55250f19fef17) it's got the same issue.

@mcornella
Copy link
Member Author

@mcornella mcornella commented on dd1a726 May 7, 2020

Choose a reason for hiding this comment

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

The compinit call works without problem, but the zcompdump cache file isn't generated. You should change the hostname, otherwise you won't get the cache benefits on startup, and it'll be slower.

I can change the echo call to use tee instead, that way the error can be silenced.

-echo "\n$zcompdump_metadata" >>! "$ZSH_COMPDUMP"
+echo "\n$zcompdump_metadata" | tee -a "$ZSH_COMPDUMP" &>/dev/null

I still think this is a hack and you'll have to change the hostname due to other problems.

EDIT: pushed the fix, update with upgrade_oh_my_zsh.

@shoopdawoop
Copy link

Choose a reason for hiding this comment

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

The hostname is fine, it's something like AbcDefGhijk:

scutil --get HostName returns AbcDefGhijk
scutil --get ComputerName returns ¯\_(ツ)_/¯
scutil --get LocalHostName returns tsu

So I would argue it's the use of the ComputerName instead of HostName or LocalHostName that causes the problem, regardless of how messed up my ComputerName is ;P

Would using $HOST instead of $SHORT_HOST cause other issues?

I can live with the slower auto-completion though.

Thanks!

@mcornella
Copy link
Member Author

Choose a reason for hiding this comment

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

I've been wanting to use ${HOST/.*/} or ${(%):-%m} instead of ComputerName for macOS. It seems those change with dhcp though, so I don't know how fitting would be to make that change.

@shoopdawoop
Copy link

Choose a reason for hiding this comment

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

What about hostname -s ?

@shoopdawoop
Copy link

Choose a reason for hiding this comment

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

FYI: Not that it bothers me much, but these are back

/Users/user/.oh-my-zsh/oh-my-zsh.sh:88: no such file or directory: /Users/user/.zcompdump-¯\_(ツ)_/¯-5.3

Created autostash: 447d1e8f
HEAD is now at c58572d init: oops

@mcornella
Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, I accidentally deleted the hack because I forgot. It should work again now.

Please sign in to comment.