Emacs macport version fix #20706
Emacs macport version fix #20706
Conversation
Previously, emacs-macport was giving the error: - AppKit/AppKit.h: present but cannot be compiled - Carbon/Carbon.h: present but cannot be compiled This add "MAC_OS_X_VERSION_MIN_REQUIRED=1090" to CFLAGS to allow them to compile with the new 10.10 headers. Fixes NixOS#20682.
|
@matthewbauer, thanks for your PR! By analyzing the history of the files in this pull request, we identified @periklis, @acowley and @jwiegley to be potential reviewers. |
LnL7
added
6.topic: darwin
|
Unless I'm misreading, That said, the change does get past the referenced error. |
|
Sorry about that. I had that commit originally but it got overwritten. |
|
To be clear, the change here seems to work for me, but I want to make sure it is the entire intended change since the diagnosed problem is that we have 10.10 headers, but the expression still includes the It looks like that flag isn't hurting anything at the moment, so I'm fine with merging, but maybe we should bump it just on principle? |
|
@acowley The reason we need it is because of some 10.9 specific things that emacs-mac is using. You can try compiling |
|
Would it be okay to get rid of the "lowPrio" for these two derivations? |
|
I hope this can be merged soon. I had a frustrating time bringing a computer up to date today because the |
|
@acowley AFAICT, emacs25Macport isn't in Hydra (lowPrio). Could you be looking at the regular "emacs" (which also provides a .app)? |
|
Hm, the path in the store definitely said mac, and it was during an update from 24 that I hit this, and the result definitely did not have an .app. |
|
looks like like emacs24 is still broken, also does not build on sierra https://gist.github.com/LnL7/dda8973b7deac7cbe0ee9d30505d54d6 |
|
let me check if it's just during the build. |
|
The resulting build seems to work. What's the reason we also have a build for emacs24, is that important? |
|
If 25 works and 24 doesn't, we should split the PR. I do think 24 matters, but some emacs is better than no emacs! |
| @@ -48,7 +48,7 @@ stdenv.mkDerivation rec { | |||
| "--enable-mac-app=$$out/Applications" | |||
| ]; | |||
|
|
|||
| CFLAGS = "-O3 -DMAC_OS_X_VERSION_MAX_ALLOWED=1090"; | |||
| CFLAGS = "-O3 -DMAC_OS_X_VERSION_MAX_ALLOWED=1090 -DMAC_OS_X_VERSION_MIN_REQUIRED=1090"; | |||
There was a problem hiding this comment.
This looks a little strange, do we need both?
There was a problem hiding this comment.
Well we can't have MAC_OS_X_VERSION_MAX_ALLOWED without MAC_OS_X_VERSION_MIN_REQUIRED because max allowed can't be less than min required (which it is with the latest apple-sdk). Why MAC_OS_X_VERSION_MAX_ALLOWED is necessary is unclear but it seems to fail without it.
There was a problem hiding this comment.
Allright, do you want to fix emacs24 or do I just merge this for now.
There was a problem hiding this comment.
I would say to just merge this now. If you can merge #20536 after this, it should fix emacs24Macport as well
Well this change is definitely needed for 24 even though other things are broken. #20536 seems to fix 24. |
Motivation for this change
This sets MAC_OS_X_VERSION_MIN_REQUIRED=1090 to get the emacs25macport working again. This should also be needed for emacs24macport.
Also: update to emacs-mac 6.1.
Things done
(nix.useSandbox on NixOS,
or option
build-use-sandboxinnix.confon non-NixOS)
nix-shell -p nox --run "nox-review wip"./result/bin/)Previously emacs-macport was giving the error:
This adds "MAC_OS_X_VERSION_MIN_REQUIRED=1090" to CFLAGS to allow them
to compile with the new 10.10 headers.
Fixes #20682.