NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
18.39k stars 14.34k forks source link

emacsWithPackages runs emacs with invocation-directory of nested package #145302

Open sheijk opened 3 years ago

sheijk commented 3 years ago

Describe the bug

When wrapping Emacs using emacsPackagesFor the invocation-directory will point to the wrong Emacs package. This means restarting Emacs from inside Emacs will result in a binary which doesn't see the additional packages to be started and initialization can fail.

Steps To Reproduce

nix-build the following derivation (ran out of disk space trying to build master, but looking at the source the issue seems to be the same, see additional info section). The issue happens both with and without emacs-overlay.

{
  # nixpkgs ? ../nixpkgs,
  nixpkgs ? (builtins.fetchGit {
    name = "nixos-stable-21.05";
    url = "https://github.com/nixos/nixpkgs";
    ref = "refs/heads/nixos-21.05";
  }),
  # emacsOverlay ? (import (builtins.fetchGit {
  #   url = "https://github.com/nix-community/emacs-overlay/";
  #   ref = "master";
  #   rev = "64580e3ac034e2704895a272f341a0729d165b93";
  # })),
}:

with import nixpkgs { overlays = [ # emacsOverlay
                                 ]; };

let useUnstableForAllEmacsPkgs = false;
    myEmacsPkg = emacs;
    myPackages = epkgs:
      [ epkgs.melpaStablePackages.which-key ];
    myEmacs = (emacsPackagesFor myEmacsPkg).emacsWithPackages myPackages;
in
buildEnv {
  name = "my-emacs";

  paths = [ myEmacs ];
}

This will build a package my-emacs pulling in which-key and a package emacs-27.2 which is Emacs without the extra packages. The bin/emacs executable in my-emacs is a script setting up the environment so the wrapped executable will find which-key.

Run ./result/bin/emacs and press =C-h v invocation-directory= and check the resulting path. It should point inside the store path in target, but will be the path of the emacs-27.2 package.

Start a second Emacs instance from inside this one (variables need to be inserted by hand): M-x, shell-command, {invocation-directory}{invocation-name}

in the second Emacs instance try to load the which-key package: M-x, load-library, which-key

load-library: Cannot open load file: No such file or directory, which-key

Expected behavior

The invocation-directory should point to the $my-emacs/bin/ instead of $emacs-27.2/bin in both Emacs instances.

Additional context

In nixpkgs I see wrapper.sh which does an exec without -a which seems to be where the original invocation name gets lost. This is the same on master. This will end up being the bin/emacs binary in the built derivation for my-emacs. bin/emacs in emacs-27.2 will be another wrapper doing another exec -a (I could not figure out where this was being generated, though).

I tried changing wrapper.sh using this patch but this was not successful, yet.

From 8e7802bc9ffec82080a4916169a30305ec44e891 Mon Sep 17 00:00:00 2001
From: Jan Rehders <nospam@sheijk.net>
Date: Wed, 10 Nov 2021 04:12:15 +0100
Subject: [PATCH] Pass command invocation to wrapper script

Probably the $0 is wrong
---
 pkgs/build-support/emacs/wrapper.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
 mode change 100644 => 100755 pkgs/build-support/emacs/wrapper.sh

diff --git a/pkgs/build-support/emacs/wrapper.sh b/pkgs/build-support/emacs/wrapper.sh
old mode 100644
new mode 100755
index e8eecb8c869..56079801c07
--- a/pkgs/build-support/emacs/wrapper.sh
+++ b/pkgs/build-support/emacs/wrapper.sh
@@ -44,4 +44,4 @@ export emacsWithPackages_siteLisp=@wrapperSiteLisp@
 export EMACSNATIVELOADPATH="${newNativeLoadPath[*]}"
 export emacsWithPackages_siteLispNative=@wrapperSiteLispNative@

-exec @prog@ "$@"
+exec -a "$0" @prog@ "$@"
-- 
2.31.1

I think the second wrapper script in emacs-27.2 is overriding the invocation name, again.

Notify maintainers

@Stunkymonkey @alyssais @tadfisher (based on blame, not sure where the meta info is or if this is part of some specific package?)

Metadata

Please run nix-shell -p nix-info --run "nix-info -m" and paste the result.

[user@system:~]$ nix-shell -p nix-info --run "nix-info -m"
 - system: `"x86_64-linux"`
 - host os: `Linux 5.14.15, NixOS, 21.05.3980.f0869b1a2c0 (Okapi)`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.3.16`
 - channels(root): `"nixos-21.05.3980.f0869b1a2c0"`
 - channels(sheijk): `""`
 - nixpkgs: `/nix/var/nix/profiles/per-user/root/channels/nixos`

Maintainer information:

# a list of nixpkgs attributes affected by the problem attribute:
- nixpkgs.emacs.emacsPackagesFor
# a list of nixos modules affected by the problem module:
- nixos.emacs
alyssais commented 3 years ago

cc @adisbladis

jcf commented 9 months ago

I think I've tracked down the source of the problem, which affects versions of Emacs built with Home Manager.

The trivial runCommand builder has its name overridden to append with-packages, but the same modification is not made to emacs, which substitute depends on via prog. With such a change, the call to substitute should behave as expected.

substitute ${./wrapper.sh} $out/Applications/Emacs.app/Contents/MacOS/Emacs \
  --subst-var-by bash ${emacs.stdenv.shell} \
  --subst-var-by wrapperSiteLisp "$deps/share/emacs/site-lisp" \
  --subst-var-by wrapperSiteLispNative "$deps/share/emacs/native-lisp" \
  --subst-var-by prog "$emacs/Applications/Emacs.app/Contents/MacOS/Emacs"
chmod +x $out/Applications/Emacs.app/Contents/MacOS/Emacs
wrapProgramBinary $out/Applications/Emacs.app/Contents/MacOS/Emacs

If you have Home Manager setup, you can reproduce the issue with the following configuration:

{
  programs.emacs = {
    enable = true;

    package = pkgs.emacs-unstable-pgtk;

    extraPackages = epkgs: [epkgs.vterm];

    overrides = final: prev: {
      # `emacs-28` patches are compatible with `emacs-29`.
      #
      # Where a compatible path exists, there is a symlink upstream to keep
      # things clean, but GitHub doesn't follow symlinks to generate the
      # responses we need (instead GitHub returns the target of the symlink).
      patches =
        (prev.patches or [])
        ++ [
          # Fix OS window role (needed for window managers like yabai)
          (pkgs.fetchpatch {
            url = "https://raw.githubusercontent.com/d12frosted/homebrew-emacs-plus/master/patches/emacs-28/fix-window-role.patch";
            sha256 = "0c41rgpi19vr9ai740g09lka3nkjk48ppqyqdnncjrkfgvm2710z";
          })
          # Use poll instead of select to get file descriptors
          (pkgs.fetchpatch {
            url = "https://raw.githubusercontent.com/d12frosted/homebrew-emacs-plus/master/patches/emacs-29/poll.patch";
            sha256 = "0j26n6yma4n5wh4klikza6bjnzrmz6zihgcsdx36pn3vbfnaqbh5";
          })
          # Enable rounded window with no decoration
          (pkgs.fetchpatch {
            url = "https://raw.githubusercontent.com/d12frosted/homebrew-emacs-plus/master/patches/emacs-29/round-undecorated-frame.patch";
            sha256 = "0x187xvjakm2730d1wcqbz2sny07238mabh5d97fah4qal7zhlbl";
          })
          # Make Emacs aware of OS-level light/dark mode
          (pkgs.fetchpatch {
            url = "https://raw.githubusercontent.com/d12frosted/homebrew-emacs-plus/master/patches/emacs-28/system-appearance.patch";
            sha256 = "14ndp2fqqc95s70fwhpxq58y8qqj4gzvvffp77snm2xk76c1bvnn";
          })
        ];
    };
  };
}
jcf commented 9 months ago

Mentioning Emacs maintainers for input before I open a pull request to fix this: @AndersonTorres @adisbladis @atemu @jwiegley @lovek323 @matthewbauer

I'm working on a reproducible example of this issue to ease testing, and I think I know what's causing the problem — while the runCommand call has -with-packages added, the prog substitution doesn't.

Would a pull request that applies the with-packages rename in both places solve this problem? If so, I'll work on something along those lines this weekend.


Update: I've tested my hypothesis locally, and it doesn't fix the problem. Executing the original Emacs derivation (sans with-packages suffix) is the desired behaviour. If, instead, you execute the with-packages version, Emacs never finishes launching.

Back to the drawing board!

binarin commented 3 days ago

The other thing appears to be affected by this - native compilation, which also uses invocation-name and invocation-directory - https://github.com/emacs-mirror/emacs/blob/8903106bb783c2825233c149b6799960aacdea57/lisp/emacs-lisp/comp-run.el#L302 And when emacs e.g. tries to automatically native-compile user's init.el, it can cause really funny side-effects during compilation, like (use-package :ensure t) installing package to user's .emacs.d directory (despite them already being installed emacsWithPackages)

The change introduced in https://github.com/NixOS/nixpkgs/pull/106486 (commit https://github.com/NixOS/nixpkgs/pull/106486/commits/23d4bfb6661ca57a9e331a2cf4184232d38ac38b) says that load-path sanitization is only for convenience when hacking on emacs itself or it's nixpkgs packaging.

Now that it's affecting 2 core features of emacs itself - emacs-restart and native compilation, maybe the convenience of emacs hacker/packager should be given a lower priority, with load-path sanitization removed or hidden behind an option. CC @alyssais

The last test case of the following shell script shows that without sanitization (as it's inhibited by -Q) nested emacs processes would be working in the way emacs itself expects them to work.

#!/usr/bin/env bash
nix build --impure --expr '
{ pkgs ? import <nixpkgs> {} }:
let
  myEmacs = pkgs.emacs-nox;
  emacsWithPackages = (pkgs.emacsPackagesFor myEmacs).emacsWithPackages;
in
  emacsWithPackages (epkgs:
    (with epkgs.elpaPackages; [ undo-tree ]))
'

E=./result/bin/emacs

cat <<'EOF' > test.el
(message "load-path has %s items" (length load-path))
(message "UNDO_TREE is installed?: %s" (package-installed-p (quote undo-tree)))
EOF

cat <<'EOF' > nested.el
(with-temp-buffer
  (call-process
   (expand-file-name invocation-name invocation-directory)
   nil
   (current-buffer)
   nil
   "-Q" ;; doesn't matter whether it's "-Q" or "-q"
   "--batch"
   "--load"
   "test.el")
  (message "NESTED:\n%s" (buffer-string)))
EOF

echo "======== Running directly: -q"
$E -q --batch --load test.el

echo -e "\n======== Running directly: -Q"
$E -Q --batch --load test.el

echo -e "\n======== Running as native compilation does it"
$E -q --batch --load nested.el

echo -e "\n======== Running as native compilation does it, but outer emacs with -Q - doesn't sanitize load-path"
$E -Q --batch --load nested.el