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

Zsh completion path check broke upgrade #9455

Closed
3 tasks done
maciejp-ro opened this issue Dec 7, 2020 · 7 comments · Fixed by #9480
Closed
3 tasks done

Zsh completion path check broke upgrade #9455

maciejp-ro opened this issue Dec 7, 2020 · 7 comments · Fixed by #9480
Assignees
Labels
outdated PR was locked due to age

Comments

@maciejp-ro
Copy link

Bug report

Please note we will close your issue without comment if you delete, do not read or do not fill out the issue checklist below and provide ALL the requested information. If you repeatedly fail to use the issue template, we will block you from ever submitting issues to Homebrew again.

  • ran brew update and can still reproduce the problem?
  • ran brew doctor, fixed all issues and can still reproduce the problem?
  • ran brew config and brew doctor and included their output with your issue?

What you were trying to do (and why)

I was trying to run brew upgrade, as i do daily, to upgrade installed software

What happened (include command output)

It hanged. Multiple times.

Command output
[maciejp:~] 2s % brew upgrade
Updating Homebrew...
==> Auto-updated Homebrew!
Updated Homebrew from de1afcbfc to 20af7ee11.
Updated 6 taps (puppetlabs/puppet, homebrew/cask-versions, homebrew/core, homebrew/cask, homebrew/cask-fonts and homebrew/cask-drivers).
==> New Formulae
docui
==> Updated Formulae
Updated 322 formulae.
==> New Casks
berrycast                                      font-castoro                                   pushplaylabs-sidekick
blackhole-16ch                                 font-langar                                    raze
blackhole-2ch                                  font-libre-barcode-ean13-text                  snipy
clocker                                        font-nerko-one                                 stems
coscreen                                       font-recursive-code                            unity-appletv-support-for-editor
font-andika-new-basic                          font-syne-mono                                 unity-linux-il2cpp-support-for-editor
font-ballet                                    font-syne-tactile                              unity-linux-support-for-editor
font-big-shoulders-inline-display              font-texturina                                 unity-macos-il2cpp-support-for-editor
font-big-shoulders-inline-text                 font-trispace                                  utm
font-big-shoulders-stencil-display             font-xanh-mono
font-big-shoulders-stencil-text                memory-clean-3
==> Updated Casks
caffeine ✔                         font-glow-sans-j-normal            liya                               sauerbraten
activedock                         font-glow-sans-j-wide              loading                            scapple
airtool                            font-glow-sans-sc-compressed       logitech-options                   screaming-frog-seo-spider
amadeus-pro                        font-glow-sans-sc-condensed        logmein-client                     screen
amorphousdiskmark                  font-glow-sans-sc-extended         loom                               scroll-reverser
android-studio-preview-canary      font-glow-sans-sc-normal           ltspice                            scrutiny
anydo                              font-glow-sans-sc-wide             lyx                                segger-embedded-studio-for-arm
anytrans                           font-glow-sans-tc-compressed       macbreakz                          segger-ozone
anzeigenchef                       font-glow-sans-tc-condensed        macclean                           sessionrestore
apptivate                          font-glow-sans-tc-extended         macgamestore                       sherlock
aqua-data-studio                   font-glow-sans-tc-normal           mactracker                         sigil
aria2d                             font-glow-sans-tc-wide             macx-video-converter-pro           sipgate-softphone
arq                                font-iosevka                       macx-youtube-downloader            sirimote
atlauncher                         font-iosevka-aile                  magicprefs                         sky-ticket
axe-edit-iii                       font-iosevka-curly                 mailmaster                         smooze
back-in-time                       font-iosevka-curly-slab            manictime                          snagit
balenaetcher                       font-iosevka-etoile                mediahuman-audio-converter         softorino-youtube-converter
bigsur-cache-cleaner               font-iosevka-slab                  medibangpaintpro                   sonarr
binary-ninja                       font-iosevka-sparkle               meshlab                            spideroak-share
birdfont                           font-iosevka-ss01                  meta                               srware-iron
blocs                              font-iosevka-ss02                  mic-drop                           stand
blu-ray-player-pro                 font-iosevka-ss03                  microsoft-edge-beta                stats
blueharvest                        font-iosevka-ss04                  middle                             subsurface
bluejeans                          font-iosevka-ss05                  miro                               superproductivity
boost-note                         font-iosevka-ss06                  mobirise                           surfeasy-vpn
box-sync                           font-iosevka-ss07                  modern-csv                         surfshark
brave-browser-dev                  font-iosevka-ss08                  monal                              swift-publisher
canon-eos-utility                  font-iosevka-ss09                  moneymoney                         syncovery
captin                             font-iosevka-ss10                  multitouch                         syntax-highlight
chatwork                           font-iosevka-ss11                  mweb                               tableplus
chromium                           font-iosevka-ss12                  nagbar                             teambition
chronicle                          font-iosevka-ss13                  name-mangler                       teamdrive
clash-for-windows                  font-iosevka-ss14                  netbeans                           techsmith-capture
clashx                             font-juliamono                     netron                             teensy
classicftp                         font-sarasa-gothic                 netxms-console                     tencent-meeting
cleanmymac                         font-source-han-sans               nifty                              texpad
cncnet                             font-tt2020                        nodeclipse                         texshop
coccoc                             fontlab                            noisy                              thonny-xxl
coin-wallet                        fork                               nordic-nrf-connect                 tiger-trade
command-tab-plus                   freeter                            nsregextester                      tigervpn
connectiq                          fsnotes                            odrive                             timer
continuity-activation-tool         fstream                            ondesoft-audiobook-converter       tofu
corsair-icue                       gameranger                         one-switch                         transcribe
daedalus-flight                    geomap                             openbazaar                         treesheets
daedalus-mainnet                   get-iplayer-automator              openphone                          tribler
dashlane                           get-lyrical                        opensesame                         trilium-notes
datagraph                          gitfox                             openxcom                           tutanota
datagrip                           githubpulse                        opera-developer                    twitch
dbeaver-enterprise                 globalmeet                         opera-neon                         tyme
dbkoda                             glyphs                             pagico                             ubersicht
dcv-viewer                         gns3                               pdf-expert                         ubiquiti-unifi-controller
deathtodsstore                     gog-galaxy                         pdf-expert-beta                    uc-one
deepl                              gopanda                            pdf-squeezer                       unified-remote
default-folder-x                   gotiengviet                        pdfelement                         unity
discord                            gqrx                               perimeter81                        unity-android-support-for-editor
disk-drill                         happymac                           phonebrowse                        unity-ios-support-for-editor
displaperture                      hazeover                           phoneclean                         unity-lumin-support-for-editor
djview                             hbuilderx                          phonetrans                         unity-webgl-support-for-editor
dmm-player                         hessenbox-da                       pitchperfect                       unity-windows-support-for-editor
docker-edge                        home-inventory                     popo                               vesta
dosbox                             hookshot                           porting-kit                        veusz
doubletwist                        houseparty                         praat                              videofusion
dropbox-beta                       hue-topia                          privatevpn                         vienna
edex-ui                            hype                               profind                            vipriser
elecom-mouse-assistant             icollections                       proxifier                          virtualhere
electrocrud                        iina-plus                          psychopy                           virtualhereserver
electrum                           inav-configurator                  puppet-bolt                        waves-central
element                            infoflow                           purevpn                            whatsapp
elgato-video-capture               insomnia                           qnap-external-raid-manager         whatsize
elgiganten-jotta                   insomnia-designer                  qobuz                              window-switch
elmedia-player                     install-disk-creator               qownnotes                          winx-hd-video-converter
eloston-chromium                   instasizer                         qqbrowser                          wowmatrix
elpass                             inxmail-professional               quickhue                           x-swiftformat
emailchemy                         iriunwebcam                        qutebrowser                        xnconvert
epichrome                          ishowu                             raiderio                           yakyak
eqmac                              iswiff                             raindropio                         yinxiangbiji
exodus                             jami                               rapidminer-studio                  yojimbo
expressvpn                         jandi                              recents                            youdaonote
feishu                             jedit-omega                        recordit                           youtube-to-mp3
figma                              jqbx                               rectangle                          zeitgeist
flickr-uploadr                     keeper-password-manager            refined-github-safari              zeplin
flipper                            klokki                             remember-the-milk                  zerotier-one
flow                               knock                              remote-buddy                       zettlr
folx                               latexdraw                          resxtreme                          zoc
font-anonymous-pro                 launchpad-manager                  rightfont                          zoho-docs
font-glow-sans-j-compressed        league-of-legends                  rocks-n-diamonds                   zoho-workdrive
font-glow-sans-j-condensed         lens                               rss
font-glow-sans-j-extended          liclipse                           rsyncosx
==> Deleted Casks
blackhole                   canary                      kiwi-for-g-suite            manuscripts                 noteplan

==> Upgrading 11 outdated packages:
nghttp2 1.42.0 -> 1.42.0_1
gtk+3 3.24.23 -> 3.24.24
awscli 2.1.5 -> 2.1.7
sqlite 3.33.0 -> 3.34.0
pcre2 10.35 -> 10.36
youtube-dl 2020.12.2 -> 2020.12.7
eslint 7.14.0 -> 7.15.0
pugixml 1.11 -> 1.11.1
pulumi 2.15.0 -> 2.15.1
poppler 20.11.0 -> 20.12.0
imagemagick 7.0.10-45 -> 7.0.10-46
==> Upgrading sqlite 3.33.0 -> 3.34.0
==> Downloading https://homebrew.bintray.com/bottles/sqlite-3.34.0.mojave.bottle.tar.gz
==> Downloading from https://d29vzk4ow07wi7.cloudfront.net/64729f1390a8379a9c7e6c8579dda0a0c450328868ebeb7e7e632aa448bda2d1?response-content
######################################################################## 100.0%
==> Pouring sqlite-3.34.0.mojave.bottle.tar.gz
==> Caveats
sqlite is keg-only, which means it was not symlinked into /usr/local,
because macOS already provides this software and installing another version in
parallel can cause all kinds of trouble.

If you need to have sqlite first in your PATH run:
echo 'export PATH="/usr/local/opt/sqlite/bin:$PATH"' >> ~/.zshrc

For compilers to find sqlite you may need to set:
export LDFLAGS="-L/usr/local/opt/sqlite/lib"
export CPPFLAGS="-I/usr/local/opt/sqlite/include"

For pkg-config to find sqlite you may need to set:
export PKG_CONFIG_PATH="/usr/local/opt/sqlite/lib/pkgconfig"

==> Summary
🍺 /usr/local/Cellar/sqlite/3.34.0: 11 files, 4MB
Removing: /usr/local/Cellar/sqlite/3.33.0... (11 files, 4MB)
Removing: /Users/maciejp/Library/Caches/Homebrew/sqlite--3.33.0.mojave.bottle.tar.gz... (2.0MB)
==> Upgrading gtk+3 3.24.23 -> 3.24.24
==> Downloading https://homebrew.bintray.com/bottles/gtk%2B3-3.24.24.mojave.bottle.tar.gz
==> Downloading from https://d29vzk4ow07wi7.cloudfront.net/798c425db2e840d23c95298fb44cadbd5b1d944240a3ccd0ae369386b3120ca9?response-content
######################################################################## 100.0%
==> Pouring gtk+3-3.24.24.mojave.bottle.tar.gz
==> /usr/local/opt/glib/bin/glib-compile-schemas /usr/local/share/glib-2.0/schemas
==> /usr/local/Cellar/gtk+3/3.24.24/bin/gtk3-update-icon-cache -f -t /usr/local/share/icons/hicolor
==> /usr/local/Cellar/gtk+3/3.24.24/bin/gtk-query-immodules-3.0 > /usr/local/lib/gtk-3.0/3.0.0/immodules.cache
🍺 /usr/local/Cellar/gtk+3/3.24.24: 716 files, 51MB
Removing: /usr/local/Cellar/gtk+3/3.24.23... (714 files, 51.0MB)
Removing: /Users/maciejp/Library/Caches/Homebrew/gtk+3--3.24.23.mojave.bottle.tar.gz... (14.5MB)
==> Upgrading awscli 2.1.5 -> 2.1.7
==> Downloading https://homebrew.bintray.com/bottles/awscli-2.1.7.mojave.bottle.tar.gz
==> Downloading from https://d29vzk4ow07wi7.cloudfront.net/1253c0cbd62bf978ad0a0f104b34a1d8a968053c14a8c19e5fbbf4b6e1fc5719?response-content
######################################################################## 100.0%
==> Pouring awscli-2.1.7.mojave.bottle.tar.gz

This is the point when it hanged. I looked at pstree what it was doing:

 |-+- 56107 maciejp alacritty
 | \-+= 56108 maciejp /usr/local/bin/zsh --login
 |   \-+= 56605 maciejp /usr/local/Homebrew/Library/Homebrew/vendor/portable-ruby/current/bin/ruby -W0 --disable=gems,did_you_mean,rubyopt /usr/local/Homebrew/Library/Homebrew/brew.rb upgrade
 |     \--- 60956 maciejp /bin/zsh -ic echo $FPATH

Killing /bin/zsh made it continue and emit a caveat:

==> Caveats
The "examples" directory has been installed to:
  /usr/local/share/awscli/examples

zsh completions and functions have been installed to:
  /usr/local/share/zsh/site-functions

/usr/local/share/zsh/site-functions is not in your zsh FPATH!
Add it by following these steps:
  https://docs.brew.sh/Shell-Completion#configuring-completions-in-zsh
==> Summary
🍺  /usr/local/Cellar/awscli/2.1.7: 11,783 files, 87.4MB
Removing: /usr/local/Cellar/awscli/2.1.5... (12,340 files, 90.5MB)
Removing: /Users/maciejp/Library/Caches/Homebrew/awscli--2.1.5.mojave.bottle.tar.gz... (19.9MB)
==> Upgrading pcre2 10.35 -> 10.36
==> Downloading https://homebrew.bintray.com/bottles/pcre2-10.36.mojave.bottle.tar.gz
[…]

This repeated for every package that was installing ZSH completion functions.

What you expected to happen

I expected brew to run and upgrade my packages. I would be happy with an unnecessary notice about site-functions, or with no notice at all (even if it would be necessary), or with being able to turn the extra helpfulness off.

I expected brew not to run an "interactive" shell when it's not interactive, in a wrong version. Homebrew was started from a zsh session, in zsh that brew itself installed, first on $PATH, and with $SHELL set to /usr/local/bin/zsh. Homebrew really went out of its way to pick the wrong zsh.

I don't know if finding correct zsh would help. My .zshrc is complex, and interactive shells are supposed to be interactive. Maybe there would be a workaround, but it won't ever be universal, because I am allowed to put sleep 3600 in my .zshrc for no reason and it shouldn't break Homebrew.

Step-by-step reproduction instructions (by running brew commands)

  • have homebrew with Fix ZSH FPATH handling #9404 applied
  • have something interesting in ~/.zshrc
  • run brew upgrade or brew install of a package that has ZSH completion
  • watch brew run zsh -i non-interactively using wrong zsh and get confused if zsh tries to be interactive

Output of brew config and brew doctor commands

[maciejp:~] 11m2s % brew doctor
Please note that these warnings are just used to help the Homebrew maintainers
with debugging if you file an issue. If everything you use Homebrew for is
working fine: please don't worry or file an issue; just ignore this. Thanks!

Warning: A newer Command Line Tools release is available.
Update them from Software Update in System Preferences or run:
  softwareupdate --all --install --force

If that doesn't show you an update run:
  sudo rm -rf /Library/Developer/CommandLineTools
  sudo xcode-select --install

Alternatively, manually download them from:
  https://developer.apple.com/download/more/.


Warning: You have unlinked kegs in your Cellar.
Leaving kegs unlinked can lead to build-trouble and cause brews that depend on
those kegs to fail to run properly once built. Run `brew link` on these:
  terraform
  dash

Warning: Homebrew's sbin was not found in your PATH but you have installed
formulae that put executables in /usr/local/sbin.
Consider setting the PATH for example like so:
  echo 'export PATH="/usr/local/sbin:$PATH"' >> ~/.zshrc
[maciejp:~] 33s 1 % brew config
HOMEBREW_VERSION: 2.6.0-133-g20af7ee
ORIGIN: https://github.com/Homebrew/brew
HEAD: 20af7ee11fcf1b186eb5978ba4871a2dba984a3c
Last commit: 4 hours ago
Core tap ORIGIN: https://github.com/Homebrew/homebrew-core
Core tap HEAD: 1a60e2e6309cd922b8dc50cd514472daa71b5fb2
Core tap last commit: 62 minutes ago
Core tap branch: master
HOMEBREW_PREFIX: /usr/local
HOMEBREW_CASK_OPTS: []
HOMEBREW_EDITOR: vim
HOMEBREW_MAKE_JOBS: 12
Homebrew Ruby: 2.6.3 => /usr/local/Homebrew/Library/Homebrew/vendor/portable-ruby/2.6.3_2/bin/ruby
CPU: dodeca-core 64-bit kabylake
Clang: 11.0 build 1100
Git: 2.29.2 => /usr/local/bin/git
Curl: 7.54.0 => /usr/bin/curl
Java: 1.8.0_275
macOS: 10.14.6-x86_64
CLT: 10.3.0.0.1.1562985497
Xcode: 11.3.1
[maciejp:~] 2s %
@maciejp-ro
Copy link
Author

maciejp-ro commented Dec 8, 2020

For the record, the following snippet on top of ~/.zshrc helps (by checking executable path and exiting early if it's /bin/zsh):

if [[ "$(lsof -p $$ | awk '$4 == "txt" && /\/zsh$/ { print $9 }')" == '/bin/zsh' ]]; then
    return
fi

If the fix involves using Homebrew's zsh, please indicate a way to detect in ~/.zshrc that the shell is executed from homebrew and/or implement a way to prevent Homebrew from trying to run my shell.

@MikeMcQuaid
Copy link
Member

How do you suggest that we query FPATH?

@maciejp-ro
Copy link
Author

maciejp-ro commented Dec 8, 2020

My approach would be not to query it at all because we can't predict what user has in their config (including a completely broken config because they don't run zsh and don't care). Just display a message that they might need to add site-functions directory to fpath. Then I'd make it possible to turn this message off when user already read and applied it (or knows they won't need it because they don't use zsh), something like "to never see this message again, run brew dismiss zsh-completion-caveat", which would be universally helpful for all kinds of messages.

If we try to check, it should be safeguarded with timeouts, exit status checks (which also don't seem to be there because homebrew didn't mind when I just killed the hanging zsh), stdin redirected from /dev/null to break most attempts at reading input from user (they still might hang in a retry loop if badly written, but it should catch most interaction attempts) and written with defensive programming matching the awareness that we literally execute arbitrary shell that might go wrong in all sorts of ways. Writing it defensively enough to be robust feels beyond my skill level – which is why my first instinct would be to find a way to avoid writing it at all.

@jottr
Copy link

jottr commented Dec 8, 2020

I second this.
Experienced this very issue today.
This broke my upgrade path out of the blue. I'm glad that @maciejp-ro caught this so early.

I don't see an obvious reason why CI tests were skipped for #9404

@MikeMcQuaid
Copy link
Member

I don't see an obvious reason why CI tests were skipped for #9404

@jottr They weren't.

@jottr
Copy link

jottr commented Dec 9, 2020

My bad!
Thanks for the quick fix.

@MikeMcQuaid
Copy link
Member

@jottr No worries, thanks for the kind words!

@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Jan 9, 2021
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Jan 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants