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

emacs: 27.2 -> 28.1 #168076

Merged
merged 2 commits into from
Apr 16, 2022
Merged

emacs: 27.2 -> 28.1 #168076

merged 2 commits into from
Apr 16, 2022

Conversation

azahi
Copy link
Member

@azahi azahi commented Apr 10, 2022

Description of changes

https://github.com/emacs-mirror/emacs/blob/emacs-28.1/etc/NEWS
https://lists.gnu.org/archive/html/emacs-devel/2022-04/msg00093.html

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.05 Release Notes (or backporting 21.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@azahi
Copy link
Member Author

azahi commented Apr 10, 2022

I took a liberty enabling nativecomp by default and deleting (possibly) an unused patch file.

I did not build and test all of the 44 packages that depend on the emacs package. I also didn't manage to make emacs-daemon test to succeed: it errors out with something like this

vm-test-run-emacs-daemon> (finished: waiting for success: systemctl --user status emacs.service | grep 'Active: active', in 3.30 seconds)
vm-test-run-emacs-daemon> machine: must succeed: emacsclient --create-frame $EDITOR >&2 &
vm-test-run-emacs-daemon> (finished: must succeed: emacsclient --create-frame $EDITOR >&2 &, in 0.02 seconds)
vm-test-run-emacs-daemon> machine: waiting for emacseditor to appear on screen
vm-test-run-emacs-daemon> machine: sending monitor command: screendump /build/tmp9p0v4guu/ppm
vm-test-run-emacs-daemon> machine: waiting for monitor prompt
vm-test-run-emacs-daemon> (finished: waiting for monitor prompt, in 0.00 seconds)
vm-test-run-emacs-daemon> (finished: sending monitor command: screendump /build/tmp9p0v4guu/ppm, in 0.00 seconds)
vm-test-run-emacs-daemon> machine # Waiting for Emacs...
vm-test-run-emacs-daemon> machine # *ERROR*: Unknown terminal type

and then just cycles infinitely. Executing the binary and doing some trivial tasks works just fine.

Any help would be appreciated.

@wahjava
Copy link
Contributor

wahjava commented Apr 11, 2022

Thanks for working on the update.

I took a liberty enabling nativecomp by default and deleting (possibly) an unused patch file.

IMHO, nativecomp should not be enabled by default. It takes a long time to build itself, and then it uses 100% CPU (in its default configuration) when you install any new packages for native compilation. I've used it for sometime, and other than increasing the temperature of the CPU, I didn't feel much difference in the performance, and so I ended up reverting back to without nativecomp.

@siraben
Copy link
Member

siraben commented Apr 11, 2022

nativecomp can be disabled by setting no-native-compile or no-byte-compile to non-nil.

I think that creating another definition for Emacs with native compilation (similar to emacs-overlay) would be a good idea, leaving native compilation disabled for the emacs derivation.

@azahi
Copy link
Member Author

azahi commented Apr 11, 2022 via email

@azahi
Copy link
Member Author

azahi commented Apr 11, 2022

@wahjava @siraben I've added an additional derivation and called it emacs28NativeComp.

@jwiegley
Copy link
Contributor

I'll give this a try today.

@azahi
Copy link
Member Author

azahi commented Apr 11, 2022 via email

@jwiegley
Copy link
Contributor

@azahi I figured out the problem. Everything seems to be looking good on my side, I'll switch to using this version for the next few days and will let you know if I run into any problems. Thank you for the work!

@wahjava
Copy link
Contributor

wahjava commented Apr 12, 2022

I switched to emacs28, and everything seems to work okay.

Thank you!

@wineee
Copy link
Member

wineee commented Apr 12, 2022

Can we add the emacsPgtk derivation, which support Wayland natively, (like emacs-overlay)

@azahi
Copy link
Member Author

azahi commented Apr 12, 2022 via email

@axelf4
Copy link
Contributor

axelf4 commented Apr 13, 2022

I also didn't manage to make emacs-daemon test to succeed: it errors out with something like this

[...]
vm-test-run-emacs-daemon> machine # Waiting for Emacs...
vm-test-run-emacs-daemon> machine # *ERROR*: Unknown terminal type

and then just cycles infinitely.

I took a look at this, and from what I gather explicitly setting the display frame parameter makes make-frame not try to make sense of the dumb TTY type when no GUI frames exist yet. That is, this patch makes the test succeed:

diff --git a/nixos/tests/emacs-daemon.nix b/nixos/tests/emacs-daemon.nix
index d53031a67f6..310e93e19b0 100644
--- a/nixos/tests/emacs-daemon.nix
+++ b/nixos/tests/emacs-daemon.nix
@@ -33,7 +33,7 @@ import ./make-test-python.nix ({ pkgs, ...} : {
       )
 
       # connects to the daemon
-      machine.succeed("emacsclient --create-frame $EDITOR >&2 &")
+      machine.succeed("emacsclient --no-wait --frame-parameters='((display . \"'\"$DISPLAY\"'\"))' --create-frame $EDITOR >&2")
 
       # checks that Emacs shows the edited filename
       machine.wait_for_text("emacseditor")

I have to say, I do not fully understand what actually caused the test to start failing, but it is safe to say that it is an upstream issue based on the workaround.

@jwiegley
Copy link
Contributor

All has been working well for me.

@azahi
Copy link
Member Author

azahi commented Apr 13, 2022

@axelf4 Thanks. I've pushed an additional commit with your patch. The test is working fine now.

@ofborg ofborg bot requested a review from jwiegley April 13, 2022 21:48
@adisbladis
Copy link
Member

adisbladis commented Apr 14, 2022

I'm actually wondering if there is much point in not enabling native compilation outright?
It has quite significant speedups and hasn't broken all that much in a very long time for us in the emacs overlay.

@siraben
Copy link
Member

siraben commented Apr 14, 2022

@adisbladis probably concerns about increased CPU usage since it compiles everything and can drain the battery on laptop devices.

@axelf4
Copy link
Contributor

axelf4 commented Apr 14, 2022

probably concerns about increased CPU usage since it compiles everything and can drain the battery on laptop devices.

Would not those users be better off disabling just-in-time native compilation in their init files, while still benefiting from the packaged Emacs distribution being ahead-of-time compiled?

@azahi
Copy link
Member Author

azahi commented Apr 14, 2022

I went on Repology and looked up how other distributions package 28.1:

I'm still a bit uncertain over that if we should enable it by default or not. I think having a separate package could be the safest bet for the time being.

@adisbladis
Copy link
Member

adisbladis commented Apr 16, 2022

Would not those users be better off disabling just-in-time native compilation in their init files, while still benefiting from the packaged Emacs distribution being ahead-of-time compiled?

@axelf4 I think that's what we'll eventually end up doing, but for now the additional attribute is fine and risk-free.
We could revisit this soon after the 22.05 branch-off or in a follow-up PR and quickly relegate the emacsNativeComp attribute to an alias.

axelf4 figured[1] out a workaround for fixing a failure due to an unset
$DISPLAY variable.

[1] NixOS#168076 (comment)
adisbladis added a commit to nix-community/emacs-overlay that referenced this pull request Apr 16, 2022
In upstream nixpkgs PR NixOS/nixpkgs#168076
native compilation support was introduced to the nixpkgs attribute.
This is a better naming more in line with upstream, so we should also
adopt it.

I have aliased emacsGcc to emacsNativeComp for backwards compatibility
with a trace notice informing users about the change.

Additionally we will now default to the upstream native comp derivation if it exists.
@jian-lin
Copy link
Contributor

jian-lin commented Jun 7, 2022

A follow-up PR #176780 to enable native-comp by default.

Reviews are welcome.

@jian-lin jian-lin mentioned this pull request Aug 17, 2022
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants