search

d12frosted / homebrew-emacs-plus

Emacs Plus formulae for the Homebrew package manager
MIT License
2.34k stars 181 forks source link

Could not install emacs@30 #643

Closed PabloReszczynski closed 9 months ago

PabloReszczynski commented 9 months ago

Please make sure to follow these steps (and mark the checkboxes):

What you were trying to do

Install emacs 30

What happened (include command output)

$ brew install emacs-plus@30 --with-native-comp
Command output 
❯ brew install emacs-plus@30 --with-native-comp
==> Fetching d12frosted/emacs-plus/emacs-plus@30
==> Cloning https://github.com/emacs-mirror/emacs.git
Updating /Users/p/Library/Caches/Homebrew/emacs-plus@30--git
From https://github.com/emacs-mirror/emacs
   1c8b6a00cff..9c86dd52475  master     -> origin/master
==> Checking out branch master
Already on 'master'
Your branch is behind 'origin/master' by 408 commits, and can be fast-forwarded.
  (use "git pull" to update your local branch)
HEAD is now at 9c86dd52475 Tramp's direct asynchronous processes use 'tramp-remote-path'
==> Reinstalling d12frosted/emacs-plus/emacs-plus@30 --with-native-comp
==> Patching
==> Applying fix-window-role.patch
patching file src/nsterm.m
Hunk #1 succeeded at 9456 (offset 688 lines).
==> Applying system-appearance.patch
patching file src/frame.h
patching file src/nsfns.m
Hunk #1 succeeded at 1428 (offset 172 lines).
patching file src/nsterm.m
Hunk #1 succeeded at 1935 (offset 46 lines).
Hunk #2 succeeded at 5780 (offset 385 lines).
Hunk #3 succeeded at 6068 (offset 427 lines).
Hunk #4 succeeded at 6118 (offset 427 lines).
Hunk #5 succeeded at 6352 (offset 435 lines).
Hunk #6 succeeded at 9789 (offset 887 lines).
Hunk #7 succeeded at 11122 (offset 1064 lines).
==> Applying round-undecorated-frame.patch
patching file src/frame.c
Hunk #1 succeeded at 972 (offset 2 lines).
Hunk #2 succeeded at 4043 (offset 133 lines).
Hunk #3 succeeded at 6275 (offset 221 lines).
patching file src/frame.h
Hunk #1 succeeded at 459 (offset 63 lines).
Hunk #2 succeeded at 1239 (offset 157 lines).
Hunk #3 succeeded at 1266 (offset 157 lines).
patching file src/nsfns.m
Hunk #1 succeeded at 1100 with fuzz 2 (offset 51 lines).
Hunk #2 succeeded at 1428 with fuzz 2 (offset 51 lines).
patching file src/nsterm.h
Hunk #1 succeeded at 1238 (offset 11 lines).
patching file src/nsterm.m
Hunk #1 succeeded at 1801 (offset 3 lines).
Hunk #2 succeeded at 9226 (offset 149 lines).
Hunk #3 succeeded at 9314 (offset 149 lines).
==> ./autogen.sh
==> ./configure --disable-silent-rules --enable-locallisppath=/usr/local/share/emacs/site-lisp
==> gmake
Last 15 lines from /Users/p/Library/Logs/Homebrew/emacs-plus@30/03.gmake:
gmake[2]: Leaving directory '/private/tmp/emacs-plusA30-20231222-21283-onsvu4/lisp'
gmake[1]: *** [Makefile:537: lisp] Error 2
gmake[1]: Leaving directory '/private/tmp/emacs-plusA30-20231222-21283-onsvu4'
gmake[1]: Entering directory '/private/tmp/emacs-plusA30-20231222-21283-onsvu4'
***
*** "make all" failed with exit status 2.
***
*** You could try to:
*** - run "make bootstrap", which might fix the problem
*** - run "make V=1", which displays the full commands invoked by make,
***   to further investigate the problem
***
gmake[1]: *** [Makefile:418: advice-on-failure] Error 2
gmake[1]: Leaving directory '/private/tmp/emacs-plusA30-20231222-21283-onsvu4'
gmake: *** [Makefile:374: all] Error 2

Output of brew config

$ brew config
❯ brew config
HOMEBREW_VERSION: 4.2.0
ORIGIN: https://github.com/Homebrew/brew
HEAD: ff899e5b3ff6731208564ef502db237e2627e8ba
Last commit: 4 days ago
Core tap JSON: 21 Dec 17:36 UTC
HOMEBREW_PREFIX: /usr/local
HOMEBREW_BAT_THEME: gruvbox-light
HOMEBREW_CASK_OPTS: []
HOMEBREW_EDITOR: nvim
HOMEBREW_MAKE_JOBS: 16
Homebrew Ruby: 3.1.4 => /usr/local/Homebrew/Library/Homebrew/vendor/portable-ruby/3.1.4/bin/ruby
CPU: 16-core 64-bit kabylake
Clang: 15.0.0 build 1500
Git: 2.43.0 => /usr/local/bin/git
Curl: 8.4.0 => /usr/bin/curl
macOS: 14.2.1-x86_64
CLT: 15.1.0.0.1.1700200546
Xcode: 15.1

Output of brew doctor

$ brew doctor
❯ 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: Some installed casks are deprecated or disabled.
You should find replacements for the following casks:
  ksdiff

Warning: Some installed kegs have no formulae!
This means they were either deleted or installed manually.
You should find replacements for the following formulae:
  libav

Warning: Some installed formulae are deprecated or disabled.
You should find replacements for the following formulae:
  dog
  exa
  gcal
  libav
  mcrypt
  openssl@1.1
  youtube-dl

Warning: Unbrewed dylibs were found in /usr/local/lib.
If you didn't put them there on purpose they could cause problems when
building Homebrew formulae, and may need to be deleted.

Unexpected dylibs:
  /usr/local/lib/libbasekit.dylib
  /usr/local/lib/libcoroutine.dylib
  /usr/local/lib/libgarbagecollector.dylib
  /usr/local/lib/libiovmall.dylib
  /usr/local/lib/libsample.dylib

Warning: Unbrewed header files were found in /usr/local/include.
If you didn't put them there on purpose they could cause problems when
building Homebrew formulae, and may need to be deleted.

Unexpected header files:
  /usr/local/include/io/386-ucontext.h
  /usr/local/include/io/BStream.h
  /usr/local/include/io/BStreamTag.h
  /usr/local/include/io/Base.h
  /usr/local/include/io/CHash.h
  /usr/local/include/io/CHash_inline.h
  /usr/local/include/io/Collector.h
  /usr/local/include/io/CollectorMarker.h
  /usr/local/include/io/CollectorMarker_inline.h
  /usr/local/include/io/Collector_inline.h
  /usr/local/include/io/Common.h
  /usr/local/include/io/Common_inline.h
  /usr/local/include/io/Coro.h
  /usr/local/include/io/Date.h
  /usr/local/include/io/Duration.h
  /usr/local/include/io/DynLib.h
  /usr/local/include/io/Hash_fnv.h
  /usr/local/include/io/Hash_murmur.h
  /usr/local/include/io/Hash_superfast.h
  /usr/local/include/io/IoBlock.h
  /usr/local/include/io/IoCFunction.h
  /usr/local/include/io/IoCall.h
  /usr/local/include/io/IoCollector.h
  /usr/local/include/io/IoCompiler.h
  /usr/local/include/io/IoConfig.h
  /usr/local/include/io/IoContext.h
  /usr/local/include/io/IoCoroutine.h
  /usr/local/include/io/IoDate.h
  /usr/local/include/io/IoDebugger.h
  /usr/local/include/io/IoDirectory.h
  /usr/local/include/io/IoDuration.h
  /usr/local/include/io/IoDynLib.h
  /usr/local/include/io/IoError.h
  /usr/local/include/io/IoFile.h
  /usr/local/include/io/IoFile_stat.h
  /usr/local/include/io/IoInstallPrefix.h
  /usr/local/include/io/IoLexer.h
  /usr/local/include/io/IoList.h
  /usr/local/include/io/IoMap.h
  /usr/local/include/io/IoMessage.h
  /usr/local/include/io/IoMessage_inline.h
  /usr/local/include/io/IoMessage_opShuffle.h
  /usr/local/include/io/IoMessage_parser.h
  /usr/local/include/io/IoNumber.h
  /usr/local/include/io/IoObject.h
  /usr/local/include/io/IoObject_flow.h
  /usr/local/include/io/IoObject_inline.h
  /usr/local/include/io/IoObject_struct.h
  /usr/local/include/io/IoProfiler.h
  /usr/local/include/io/IoSandbox.h
  /usr/local/include/io/IoSeq.h
  /usr/local/include/io/IoSeq_immutable.h
  /usr/local/include/io/IoSeq_inline.h
  /usr/local/include/io/IoSeq_mutable.h
  /usr/local/include/io/IoSeq_vector.h
  /usr/local/include/io/IoState.h
  /usr/local/include/io/IoState_callbacks.h
  /usr/local/include/io/IoState_coros.h
  /usr/local/include/io/IoState_debug.h
  /usr/local/include/io/IoState_eval.h
  /usr/local/include/io/IoState_exceptions.h
  /usr/local/include/io/IoState_inline.h
  /usr/local/include/io/IoState_symbols.h
  /usr/local/include/io/IoSystem.h
  /usr/local/include/io/IoTag.h
  /usr/local/include/io/IoTag_inline.h
  /usr/local/include/io/IoToken.h
  /usr/local/include/io/IoVM.h
  /usr/local/include/io/IoVMApi.h
  /usr/local/include/io/IoVersion.h
  /usr/local/include/io/IoWeakLink.h
  /usr/local/include/io/List.h
  /usr/local/include/io/List_inline.h
  /usr/local/include/io/MainArgs.h
  /usr/local/include/io/PHash.h
  /usr/local/include/io/PHash_inline.h
  /usr/local/include/io/PHash_struct.h
  /usr/local/include/io/PointerHash.h
  /usr/local/include/io/PointerHash_inline.h
  /usr/local/include/io/PointerHash_struct.h
  /usr/local/include/io/PortableGettimeofday.h
  /usr/local/include/io/PortableSorting.h
  /usr/local/include/io/PortableStdint.h
  /usr/local/include/io/PortableStrlcpy.h
  /usr/local/include/io/PortableStrptime.h
  /usr/local/include/io/PortableTruncate.h
  /usr/local/include/io/PortableUsleep.h
  /usr/local/include/io/RandomGen.h
  /usr/local/include/io/Stack.h
  /usr/local/include/io/Stack_inline.h
  /usr/local/include/io/UArray.h
  /usr/local/include/io/UArray_character.h
  /usr/local/include/io/UArray_format.h
  /usr/local/include/io/UArray_math.h
  /usr/local/include/io/UArray_path.h
  /usr/local/include/io/UArray_stream.h
  /usr/local/include/io/UArray_string.h
  /usr/local/include/io/UArray_utf.h
  /usr/local/include/io/amd64-ucontext.h
  /usr/local/include/io/cdecode.h
  /usr/local/include/io/cencode.h
  /usr/local/include/io/power-ucontext.h
  /usr/local/include/io/taskimpl.h
  /usr/local/include/io/utf8.h
  /usr/local/include/io/utf8internal.h
  /usr/local/include/io/utf_convert.h
  /usr/local/include/python3.9/greenlet/greenlet.h

Warning: You have unlinked kegs in your Cellar.
Leaving kegs unlinked can lead to build-trouble and cause formulae that depend on
those kegs to fail to run properly once built. Run `brew link` on these:
  python-certifi
  python-packaging
  python-click
  pygments
  pycparser
  cffi
  python-platformdirs
  python-filelock
  pyyaml
  python-typing-extensions
  python-cryptography
  six

Warning: Some installed formulae are not readable:
  clojure-lsp-native: /usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:4: syntax error, unexpected <<, expecting `end'
<<<<<<< HEAD
^~
/usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:8: syntax error, unexpected '|', expecting `end'
||||||| parent of 420687c (Bum...
^
/usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:8: syntax error, unexpected local variable or method, expecting `end'
||||||| parent of 420687c (Bump to version: 2022.01.20...
                        ^
/usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:8: unexpected fraction part after numeric literal
...7c (Bump to version: 2022.01.20-14.12.43)
...                            ^~~
/usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:8: unexpected fraction part after numeric literal
...to version: 2022.01.20-14.12.43)
...                            ^~~
/usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:10: syntax error, unexpected ===, expecting `end'
=======
^~~
/usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:12: syntax error, unexpected >>, expecting `end'
>>>>>>> 420687c (Bump to versio...
^~
/usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:12: unexpected fraction part after numeric literal
...7c (Bump to version: 2022.01.20-14.12.43)
...                            ^~~
/usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:12: unexpected fraction part after numeric literal
...to version: 2022.01.20-14.12.43)
...                            ^~~
/usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:15: syntax error, unexpected <<
<<<<<<< HEAD
^~
/usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:23: syntax error, unexpected '|', expecting `end'
||||||| parent of 420687c (Bum...
^
/usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:23: syntax error, unexpected local variable or method, expecting `end'
||||||| parent of 420687c (Bump to version: 2022.01.20...
                        ^
/usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:23: unexpected fraction part after numeric literal
...7c (Bump to version: 2022.01.20-14.12.43)
...                            ^~~
/usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:23: unexpected fraction part after numeric literal
...to version: 2022.01.20-14.12.43)
...                            ^~~
/usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:26: syntax error, unexpected ===, expecting `end'
=======
^~~
/usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:29: syntax error, unexpected >>, expecting `end'
>>>>>>> 420687c (Bump to versio...
^~
/usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:29: unexpected fraction part after numeric literal
...7c (Bump to version: 2022.01.20-14.12.43)
...                            ^~~
/usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:29: unexpected fraction part after numeric literal
...to version: 2022.01.20-14.12.43)
...                            ^~~
/usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:31: syntax error, unexpected <<
<<<<<<< HEAD
^~
/usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:39: syntax error, unexpected '|', expecting `end'
||||||| parent of 420687c (Bum...
^
/usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:39: syntax error, unexpected local variable or method, expecting `end'
||||||| parent of 420687c (Bump to version: 2022.01.20...
                        ^
/usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:39: unexpected fraction part after numeric literal
...7c (Bump to version: 2022.01.20-14.12.43)
...                            ^~~
/usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:39: unexpected fraction part after numeric literal
...to version: 2022.01.20-14.12.43)
...                            ^~~
/usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:42: syntax error, unexpected ===, expecting `end'
=======
^~~
/usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:45: syntax error, unexpected >>, expecting `end'
>>>>>>> 420687c (Bump to versio...
^~
/usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:45: unexpected fraction part after numeric literal
...7c (Bump to version: 2022.01.20-14.12.43)
...                            ^~~
/usr/local/Homebrew/Library/Taps/clojure-lsp/homebrew-brew/clojure-lsp-native.rb:45: unexpected fraction part after numeric literal
...to version: 2022.01.20-14.12.43)
...                            ^~~

Warning: Some taps are not on the default git origin branch and may not receive
updates. If this is a surprise to you, check out the default branch with:
  git -C $(brew --repo clojure-lsp/brew) checkout main

Warning: Homebrew's "sbin" was not found in your PATH but you have installed
formulae that put executables in /usr/local/sbin.
Consider setting your PATH for example like so:
  fish_add_path /usr/local/sbin

Warning: Some installed formulae are missing dependencies.
You should `brew install` the missing dependencies:
  brew install sdl12-compat

Run `brew missing` for more details.
d12frosted commented 9 months ago

Yup, can reproduce. Not sure what causes this though.

d12frosted commented 9 months ago

Here is the error:

nsterm.m:6211:3: error: call to undeclared function 'safe_call2'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
  safe_call2 (Qrun_hook_with_args,
  ^
1 error generated.

It's upstream issue. I am building right now from 6abea4d98d1d964c68a78cb9b5321071da851654:

  url "https://github.com/emacs-mirror/emacs.git", :revision => "6abea4d98d1d964c68a78cb9b5321071da851654"

(you can change that by brew edit emacs-plus@30)

d12frosted commented 9 months ago

Bisected a bit and found at the last working commit - https://github.com/emacs-mirror/emacs/commit/1e5357d3d1f5ecf68f1f34d017954d591eaaed14. The breaking commit is https://github.com/emacs-mirror/emacs/commit/e69fafdbc8893a0456535605082c7d7c469fdabd.

sudo-human commented 9 months ago

@d12frosted How can I install from the last known commit?

d12frosted commented 9 months ago

@sudo-human See https://github.com/d12frosted/homebrew-emacs-plus/issues/643#issuecomment-1868496062, where I explained that you can modify the formula by using brew edit emacs-plus@30.

aaronjensen commented 9 months ago

For some reason I can build from master manually, but I run into this same issue w/ this formula. Not sure why yet.

jduhamel commented 9 months ago

Looks like e8df6c311fcf59bf23d31b9db2bb8fec9d78fbe7 was reverted in 8044140b54bfe7e88c28a49cc0dc4ae129029e4f but build still fails.

aaronjensen commented 9 months ago

The problem looks to be with the system-appearance patch:

nsterm.m:6211:3: error: call to undeclared function 'safe_call2'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
  safe_call2 (Qrun_hook_with_args,

https://github.com/d12frosted/homebrew-emacs-plus/blob/master/patches/emacs-28/system-appearance.patch#L221

aaronjensen commented 9 months ago

It was removed in https://github.com/emacs-mirror/emacs/commit/0fde935b66e

d12frosted commented 8 months ago

@sudo-human just for the future, I've documented the process of pining master to specific revision. See https://github.com/d12frosted/homebrew-emacs-plus/tree/master#how-to-changepin-commit-emacs-plus30-is-built-from

aaronjensen commented 8 months ago

Is it possible/allowed to have the recipe check an environment variable and set the revision if it is set? Something like REVISION=abcabc brew install emacs-plus@30

d12frosted commented 8 months ago

Seems like it's possible. I remember trying this a few years ago and it didn't work. I was about to comment you this, but then I decided to check again... and it's possible.

  if ENV['HOMEBREW_EMACS_PLUS_30_REVISION']
    url "https://github.com/emacs-mirror/emacs.git", :revision => ENV['HOMEBREW_EMACS_PLUS_30_REVISION']
  else
    url "https://github.com/emacs-mirror/emacs.git", :branch => "master"
  end

This is rarely needed. But maybe worth adding.

d12frosted commented 8 months ago

See relevant PR - https://github.com/d12frosted/homebrew-emacs-plus/pull/653

aaronjensen commented 8 months ago

Great, thank you! It may be worth using an environment variable name that doesn't have the version number in it. That way, it's not coupled to that. I don't know if it's common to build 29 when pinned to a revision, or even possible, but at the very least when we move to 31, we won't have to use a new variable name. Not a hugely significant thing, so just mentioning it.

d12frosted commented 8 months ago

I thought about it, but it makes little sense. If I pinned some revision from Emacs 30, and I want to switch to Emacs 31, I don't want to use pin from 30, otherwise I'll get 30 (with mixed results). There are cons and pros, and I think what is really missing - being LOUD about override. Maybe it worth adding a message that warns about override. It's implicit, so easy to forget.

aaronjensen commented 8 months ago

I'm not entirely sure I follow. There's nothing that would require someone to use a revision from 30 if we had a consistent variable name, unless, for some reason, that person were setting the environment variable in their shell rc file for some reason. I would imagine it only ever being used in the way you showed in in the README: from the shell prompt. If it's used in that way, there's no way for them to conflict. It's the same reason that I can have 20 scripts that all use the CONFIRM environment variable. The script doesn't need to name the variable after itself, just after what it does, since it's highly unlikely that anyone would have an environment variable called CONFIRM in their environment for anything more than a single command. I had even considered naming the environment variable in emacs-plus REVISION for the same reason, but I can understand wanting to add the extra safety and name it more explicitly (e.g., EMACS_REVISION, or even EMACS_PLUS_REVISION, but it's worth noting that it's not the Emacs plus revision... it is the Emacs git revision... EMACS_GIT_REVISION?). That all may seem pedantic, but we get to good design through a series of carefully considered decisions that can often look like pedantry (but aren't, in my opinion).

Regarding output, I would agree that the script should print that a revision was specified and what it was. I don't know what the rules are about printing from formula, but that makes sense to me.

d12frosted commented 8 months ago

@aaronjensen I think that comparing the CONFIRM variable with REVISION isn't a straightforward or equitable comparison.

The range of possible values for REVISION is not only extensive but also continues to expand over time. More significantly, REVISION has specific semantics that influence the outcome of the configured action. It’s entirely feasible to use REVISION from Emacs 29 when building Emacs 30. Though this isn't a major issue in itself.

However, this becomes relevant for those who configure their environments using scripting tools, ranging from simple bash scripts to more sophisticated systems like dotbot or nix home-manager. In these scenarios, the variable might not be set in the same location as the Emacs installation declaration (home-manager being a prime example). This can lead to confusion, such as installing emacs-plus@X but ending up with Emacs Y.

When transitioning from version X to Y, it's common to search the configs for references to both versions to determine necessary changes. Hence, having a distinct variable for each formula is beneficial.

It would also enable the introduction of revisions for different versions as Emacs evolves, allowing users to maintain multiple versions simultaneously without complications. Admittedly, some of these reasons may seem a bit far-fetched. Nevertheless, I believe the benefits of using a specific variable like HOMEBREW_EMACS_PLUS_30_REVISION, as opposed to a generic REVISION, are substantial and come at no extra cost.

However, we must also consider the constraints imposed by Homebrew. Homebrew cleanses the environment and restricts access to certain environmental variables. Although I'm not fully certain of the specific rules, it generally requires a HOMEBREW prefix with some exceptions. For instance, REVISION is inaccessible, but HOMEBREW_REVISION is permitted. Given this, a more appropriate naming might be HOMEBREW_EMACS_PLUS_REVISION to align with Homebrew's guidelines.

Hope that makes sense :)

aaronjensen commented 8 months ago

Thanks for the additional clarification. I wasn't aware of the constraint with things like dotbot/nix. I am not familiar enough to say this definitively, but it somewhat sounds like a problem with those tools. Not being able to co-locate configuration via environment variables with the thing that needs it seems like a cohesion issue. In any case, it sounds like the reasons were well considered. What I would typically do in this situation if I had to support the *_30_* version would be to support both via something like:

revision = ENV["HOMEBREW_EMACS_PLUS_30_REVISION"] || ENV["HOMEBREW_EMACS_PLUS_REVISION"]

But I'm not calling for that, just continuing the conversation :) I'm fine with whatever you decide. Thanks!

d12frosted commented 8 months ago

@aaronjensen Indeed, it might sound like this is a limitation of these tools. However, it's important to acknowledge that using environment variables to configure brew packages is not the standard approach. These tools are designed to support the more conventional method of configuration through arguments.

While the Emacs+ formula may deviate from the norms established by brew, a significant portion of this debate (in a positive sense) could be circumvented if brew accommodated non-boolean arguments, such as --argument value. This would enable the specification of hash-like values when building from the master branch and simplify the Emacs+ formula by allowing for a single argument for icon names, rather than numerous icon-specific arguments.

I also agree with your observations on common and specialized variables.

aaronjensen commented 8 months ago

Yes, supporting arguments with values would certainly be a solid solution for this. I thought at one point they did (or they supported something more with arguments) but they trimmed that support way back. Thanks for the discussion.