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

Missing tap argument in initialize in upstream #567

Merged
merged 1 commit into from
Apr 28, 2023

Conversation

danielfleischer
Copy link
Contributor

@danielfleischer danielfleischer commented Apr 28, 2023

See commit change in initialize function. Homebrew 4.0.15-97-ge191b82 or newer.

Related to #566.

@danielfleischer
Copy link
Contributor Author

danielfleischer commented Apr 28, 2023

It works locally but fails your CI; the affected homebrew version is from 3 hours ago. How do we put the argument for newer homebrew but remove it for older versions?

Update: the macos image is 2023-04-25 so it's not updated yet. Also, if someone is calling brew update then it updates both brew and the emacs-plus formula with the new argument so it's fine.

@d12frosted
Copy link
Owner

@danielfleischer thank you for such a quick fix ❤️

How do we put the argument for newer homebrew but remove it for older versions?

Not sure it's possible. IMO let's merge it. As you say, brew update will bring latest version anyways.

Regarding CI: we can live with broken CI until new brew version arrives. IMO fixing the issue for users is more important.

@d12frosted d12frosted merged commit 3af308d into d12frosted:master Apr 28, 2023
0 of 13 checks passed
@UnderGreen
Copy link

But I feel that PR makes incompatible changes for anyone with an older brew version. Or something is wrong with the fix.

brew --version
Homebrew 4.0.15
Homebrew/homebrew-core (git revision 7e9c774bbae; last commit 2023-02-16)
Homebrew/homebrew-cask (git revision ce33115a81; last commit 2023-02-16)

brew install emacs-plus@28 --with-elrumo2-icon --with-native-comp
Error: unknown keyword: tap
Please report this issue:
  https://docs.brew.sh/Troubleshooting
/usr/local/Homebrew/Library/Homebrew/formula.rb:186:in `initialize'
/usr/local/Homebrew/Library/Taps/d12frosted/homebrew-emacs-plus/Formula/[email protected]:103:in `initialize'
/usr/local/Homebrew/Library/Homebrew/formulary.rb:401:in `new'
/usr/local/Homebrew/Library/Homebrew/formulary.rb:401:in `get_formula'
/usr/local/Homebrew/Library/Homebrew/formulary.rb:671:in `factory'
/usr/local/Homebrew/Library/Homebrew/cli/parser.rb:645:in `block in formulae'
/usr/local/Homebrew/Library/Homebrew/cli/parser.rb:641:in `map'
/usr/local/Homebrew/Library/Homebrew/cli/parser.rb:641:in `formulae'
/usr/local/Homebrew/Library/Homebrew/cli/parser.rb:317:in `parse'
/usr/local/Homebrew/Library/Homebrew/cmd/install.rb:169:in `install'
/usr/local/Homebrew/Library/Homebrew/brew.rb:94:in `<main>'

Error is clearly stating about unknown keyword. Not like you need to add it into initialize with nil.

@risset
Copy link

risset commented Apr 28, 2023

Yeah this is broken with brew stable sadly. I had to checkout master in my brew directory to get it working

@d12frosted
Copy link
Owner

Actually, not sure why it doesn't work on old version. Will be able to check tomorrow.
Meanwhile, you can simply brew edit to fix it locally.

@danielfleischer
Copy link
Contributor Author

But I feel that PR makes incompatible changes for anyone with an older brew version. Or something is wrong with the fix.

I was worried about new formula syntax with old brew installation, but how could you have a the new emacs-plus recipe with and old brew installtion though? they update together via brew update.

@UnderGreen
Copy link

@danielfleischer I am not aware how homebrew and taps are in sync, but from that comment looks like it is a problem with homebrew itself.

@nidan841g
Copy link

nidan841g commented Apr 28, 2023

I can confirm a clean build with this versioning on MacOS Ventura, M1 Pro CPU. I git pull'd manually for these

Homebrew 4.0.15-97-ge191b82
Homebrew/homebrew-core (git revision a284f020aa1; last commit 2023-04-28)
Homebrew/homebrew-cask (git revision aa39184af4; last commit 2023-04-28)

Addendum: unfortunately, this error has reared its ugly head once again.

test> open /Applications/Emacs.app
The application cannot be opened for an unexpected reason, error=Error Domain=NSOSStatusErrorDomain Code=-600 "procNotFound: no eligible process with specified descriptor" UserInfo={_LSLine=388, _LSFunction=_LSAnnotateAndSendAppleEventWithOptions}

@risset
Copy link

risset commented Apr 28, 2023

how could you have a the new emacs-plus recipe with and old brew installtion though?

I installed homebrew via the install script (/bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh), and it installs the latest tag (4.0.15) rather than the head commit in master. brew update in this case doesn't look for any commits newer than that tag.

I've switched to the master branch and all is working happily now, just something to keep in mind that this tap will be broken for anyone using stable homebrew, until brew's stable tag gets updated

@danielfleischer
Copy link
Contributor Author

danielfleischer commented Apr 28, 2023

I've switched to the master branch and all is working happily now, just something to keep in mind that this tap will be broken for anyone using stable homebrew, until brew's stable tag gets updated

Thanks, I wasn't aware there's stable and master for homebrew and that I probably live on master without thinking about it. To be honest this issue is due to a breaking change (API change) in homebrew so stuff is going to be either compatible or not. Not sure we want to have logic in the formulas doing different things depending on homebrew's version. Let's see how long it takes today's homebrew commit to reach stable.

@d12frosted
Copy link
Owner

Seems like there is a way to make it {future,past}-proof. See #569. So it would work with new version and old version of brew. Tested with new, waiting for CI to make sure that it builds with old version of brew.

@d12frosted
Copy link
Owner

d12frosted commented Apr 29, 2023

Update disregard - it was an issue related to my local hacks.

Ah, I was too fast to judge the local build. Actually it failed eventually. The difference with previous state is that the error happens after the build instead of before.

$ ./build 30
Uninstalling /opt/homebrew/Cellar/emacs-plus-local/30.0.50... (4,258 files, 207.7MB)
==> Fetching emacs-plus-local
==> Cloning https://github.com/emacs-mirror/emacs.git
Updating /Users/d12frosted/Library/Caches/Homebrew/emacs-plus-local--git
==> Checking out branch master
Already on 'master'
Your branch is up to date with 'origin/master'.
HEAD is now at e03cfec0a45 Improve call indentation for elixir-ts-mode
Warning: Your Xcode (14.2) is outdated.
Please update to Xcode 14.3 (or delete it).
Xcode can be updated from the App Store.

==> Patching
==> Applying fix-window-role.patch
patching file 'src/nsterm.m'
==> Applying system-appearance.patch
patching file 'src/frame.h'
patching file 'src/nsfns.m'
patching file 'src/nsterm.m'
==> Applying round-undecorated-frame.patch
patching file 'src/frame.c'
patching file 'src/frame.h'
patching file 'src/nsfns.m'
patching file 'src/nsterm.h'
patching file 'src/nsterm.m'
==> ./autogen.sh
==> ./configure --enable-locallisppath=/opt/homebrew/share/emacs/site-lisp --infodir=/opt/homebrew/Cellar/emacs-plus-local/30.0.50/share
==> gmake
==> gmake install
==> Injecting PATH value to Emacs.app/Contents/Info.plist
Patching plist at /opt/homebrew/Cellar/emacs-plus-local/30.0.50/Emacs.app/Contents/Info.plist with following PATH value:
/Users/d12frosted/.amplify/bin
/Users/d12frosted/.config/bin
/Users/d12frosted/.local/bin
/Users/d12frosted/.nix-profile/bin
/opt/homebrew/opt/llvm/bin
/opt/homebrew/bin
/opt/homebrew/sbin
/run/current-system/sw/bin
/nix/var/nix/profiles/default/bin
/usr/local/bin
/System/Cryptexes/App/usr/bin
/opt/X11/bin
/usr/bin
/bin
/usr/sbin
/sbin
/Applications/Xcode.app/Contents/Developer/usr/bin
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
==> /usr/libexec/PlistBuddy -c 'Add :LSEnvironment dict' '/opt/homebrew/Cellar/emacs-plus-local/30.0.50/Emacs.app/Contents/Info.plist'
==> /usr/libexec/PlistBuddy -c 'Add :LSEnvironment:PATH string' '/opt/homebrew/Cellar/emacs-plus-local/30.0.50/Emacs.app/Contents/Info.p
==> /usr/libexec/PlistBuddy -c 'Set :LSEnvironment:PATH /Users/d12frosted/.amplify/bin:/Users/d12frosted/.config/bin:/Users/d12frosted/.
==> /usr/libexec/PlistBuddy -c 'Print :LSEnvironment' '/opt/homebrew/Cellar/emacs-plus-local/30.0.50/Emacs.app/Contents/Info.plist'
==> touch '/opt/homebrew/Cellar/emacs-plus-local/30.0.50/Emacs.app'
==> Injecting description for protected resources usage
==> /usr/libexec/PlistBuddy -c 'Add NSCameraUsageDescription string' '/opt/homebrew/Cellar/emacs-plus-local/30.0.50/Emacs.app/Contents/I
==> /usr/libexec/PlistBuddy -c 'Set NSCameraUsageDescription Emacs requires permission to access the Camera.' '/opt/homebrew/Cellar/emac
==> touch '/opt/homebrew/Cellar/emacs-plus-local/30.0.50/Emacs.app'
/opt/homebrew/Library/Taps/d12frosted/homebrew-emacs-plus/Library/UrlResolver.rb:1: warning: already initialized constant TAP_OWNER
/Users/d12frosted/Developer/homebrew-emacs-plus/Library/UrlResolver.rb:1: warning: previous definition of TAP_OWNER was here
/opt/homebrew/Library/Taps/d12frosted/homebrew-emacs-plus/Library/UrlResolver.rb:2: warning: already initialized constant TAP_REPO
/Users/d12frosted/Developer/homebrew-emacs-plus/Library/UrlResolver.rb:2: warning: previous definition of TAP_REPO was here
/opt/homebrew/Library/Taps/d12frosted/homebrew-emacs-plus/Library/Icons.rb:1: warning: already initialized constant ICONS_CONFIG
/Users/d12frosted/Developer/homebrew-emacs-plus/Library/Icons.rb:1: warning: previous definition of ICONS_CONFIG was here
Error: unknown keyword: tap
Do not report this issue until you've run `brew update` and tried again.
/opt/homebrew/Library/Taps/d12frosted/homebrew-emacs-plus/Formula/[email protected]:97:in `initialize'
/opt/homebrew/Library/Homebrew/formulary.rb:404:in `new'
/opt/homebrew/Library/Homebrew/formulary.rb:404:in `get_formula'
/opt/homebrew/Library/Homebrew/formulary.rb:663:in `factory'
/opt/homebrew/Library/Homebrew/formula.rb:1243:in `link_overwrite?'
/opt/homebrew/Library/Homebrew/formula_installer.rb:989:in `rescue in link'
/opt/homebrew/Library/Homebrew/formula_installer.rb:985:in `link'
/opt/homebrew/Library/Homebrew/formula_installer.rb:782:in `finish'
/opt/homebrew/Library/Homebrew/upgrade.rb:204:in `install_formula'
/opt/homebrew/Library/Homebrew/install.rb:358:in `install_formula'
/opt/homebrew/Library/Homebrew/install.rb:303:in `block in install_formulae'
/opt/homebrew/Library/Homebrew/install.rb:302:in `each'
/opt/homebrew/Library/Homebrew/install.rb:302:in `install_formulae'
/opt/homebrew/Library/Homebrew/cmd/install.rb:274:in `install'
/opt/homebrew/Library/Homebrew/brew.rb:94:in `<main>'

@d12frosted
Copy link
Owner

but from #566 (comment) looks like it is a problem with homebrew itself.

@UnderGreen to be fair, it's a problem of Emacs+ because I am trying to hook into private method.

@d12frosted
Copy link
Owner

On the good part, I see green tests when building on CI. For example emacs-plus@29.

I also switched to my branch and built emacs-plus@30 as usual (e.g. without my local hacks) and it worked. 🎉 So waiting for #569 to become green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants