[bug#38643] [PATCH] Add spacemacs package

["Ludovic Courtès" <ludo@gnu.org>]

Hello!

saffronsnail <saffronsnail@protonmail.com> skribis:

> I have been using a local package for installing Spacemacs (https://github.com/syl20bnr/spacemacs) and would like to contribute it. I ran through the packaging guidelines and believe that everything should be in order - though note that the way I personally format lisp code is not standard, so while I tried to match the style I saw in the repository there may be some feedback there. There are 2 new packages added, spacemacs-rolling-release (to add the spacemacs code to the store) and emacs-spacemacs (to install the spacemacs command which launches vanilla emacs with command-line options to load emacs, which allows for side-by-side installations of vanilla emacs and spacemacs). There are 3 patch files which have been included to address bugs that arise when upstream spacemacs is installed to a read-only filesystem.

Awesome!

Do I get it right that Spacemacs will try to fetch the packages it needs
via pkg.el (ELPA)?

Longer-term, it would be nice if it could fetch packages through Guix,
using Emacs-Guix.  Are you familiar with this part of Spacemacs?

Some (minor) comments follow:

> From 67e44b3cdec7248d00ca2b10b5617738ab3f0d45 Mon Sep 17 00:00:00 2001
> From: Bryan Ferris <saffronsnail@protonmail.com>
> Date: Wed, 11 Dec 2019 23:57:26 -0800
> Subject: [PATCH] gnu: Add emacs-spacemacs
>
> * gnu/packages/patches/spacemacs-rolling-release-add-data-dir.patch: New file.
> * gnu/packages/patches/spacemacs-rolling-release-inhibit-read-only.patch: New file.
> * gnu/packages/patches/spacemacs-rolling-release-quelpa-permissions.patch: New file.
> * gnu/packages/spacemacs.scm: New file.
> * guix/build/spacemacs-utils.scm: New file.

Please add the new files to gnu/local.mk (and mention that as well in
the commit log.)

> --- /dev/null
> +++ b/gnu/packages/patches/spacemacs-rolling-release-add-data-dir.patch
> @@ -0,0 +1,255 @@
> +Spacemacs uses ~/.emacs.d in 2 ways: to store the code implementing the spacemacs framework and to track state. When we tell spacemacs that it lives in the store it tries to use the store location to track state as well. This patch adds a new variable, spacemacs-data-directory, for keeping track of state. This defaults to spacemacs-start-directory for upstream compatibility.

Please wrap lines to ~80 characters.  :-)

> +++ b/gnu/packages/spacemacs.scm
> @@ -0,0 +1,117 @@
> +;;; GNU Guix --- Functional package management for GNU
> +;;; Copyright © 2016 David Craven <david@craven.ch>

I believe this is incorrect ;-), could you adjust it?

> +(define-public spacemacs-rolling-release
> +  (let ((commit "1e278a3cb9cd4730ee17416b55fb778b62da2fd0"))
> +    (package
> +      (name "spacemacs-rolling-release")
> +      (version (git-version "0.3.0" "0" commit))
> +      (source (origin
> +                (method git-fetch)
> +                (uri (git-reference
> +                      (url "https://github.com/syl20bnr/spacemacs")
> +                      (commit commit)))
> +                (sha256 (base32 "17yxgchj7qilgljpjai3ad0pzj7k6sq6gqbnxrvcizvkvcnv10z5"))
> +                (file-name (string-append name "-" version))
> +                (patches (search-patches
> +                          "spacemacs-rolling-release-add-data-dir.patch"
> +                          "spacemacs-rolling-release-inhibit-read-only.patch"
> +                          "spacemacs-rolling-release-quelpa-permissions.patch"))))
> +      (build-system trivial-build-system)
> +      (native-inputs `(("tar" ,tar) ("xz" ,xz)))
> +      (arguments (list
> +                  #:modules '((guix build utils))
> +                  #:builder '(begin (use-modules (guix build utils))
> +                                    (setenv "PATH" (string-append (getenv "PATH") ":"
> +                                                                  (assoc-ref %build-inputs
> +                                                                             "xz")
> +                                                                  "/bin"))
> +                                    (mkdir-p (assoc-ref %outputs "out"))
> +                                    (system* (string-append
> +                                               (assoc-ref %build-inputs "tar") "/bin/tar")
> +                                             "xf" (assoc-ref %build-inputs "source")
> +                                             "-C" (assoc-ref %outputs "out")
> +                                             "--strip-components" "1"))))
> +      (synopsis "Automatically configured emacs for both emacs and vim users")
> +      (description "Spacemacs is a configuration framework for emacs designed to work well for people with experience using either emacs or vim.  It has 4 driving principles: mnemonics, discoverability, consistency, and crowd configuration.  Mnemonics mean that key bindings use letters that stand for the action they take, making the easier to remember.  Discoverability means that help is displayed when partial keybindings are entered, and prepared configuration units are searchable.  Consistency means that bindings for different use-cases (eg, different programming languages) use the same keybindings for similar actions.  And crowd-configuration means that the spacemacs community collaborates to provide the best default experience for new and expert users alike.")

Please wrap lines to 80 characters as well (you can look at the other
files for style hints.)

> +      (home-page "https://spacemacs.org")
> +      (license license:gpl3))))

Should be ‘gpl3+’ (meaning “version 3 or any later version”), if I’m not
mistaken.

> +(define* (generate-wrapped-emacs-spacemacs emacs spacemacs #:optional (name "emacs-spacemacs"))
> +  "Given an emacs package and a spacemacs package, create wrappers that allow the use of spacemacs without conflicting with the base emacs."
> +  (package
> +    (name name)
> +    (version (string-append (package-version emacs) "_" (package-version spacemacs)))

We normally use a hyphen instead of underscore in the version string.

> +    (source #f)
> +    (build-system trivial-build-system)
> +    (inputs `(("sh" ,bash)
> +              ("emacs" ,emacs)
> +              ("spacemacs" ,spacemacs)))
> +    (arguments `(#:modules ((guix build spacemacs-utils) (guix build utils) (srfi srfi-1))
> +                 #:builder (begin (use-modules (guix build spacemacs-utils) (guix build utils) (srfi srfi-1))
> +                                  (let* ((shell (string-append
> +                                                 (assoc-ref %build-inputs "sh")
> +                                                 "/bin/sh"))
> +                                         (emacs (string-append
> +                                                 (assoc-ref %build-inputs "emacs")
> +                                                 "/bin/emacs"))
> +                                         (spacemacs
> +                                          (assoc-ref %build-inputs "spacemacs"))
> +                                         (out (string-append (assoc-ref %outputs "out")
> +                                                             "/bin"))
> +
> +                                         (init-code (create-init-string spacemacs)))
> +                                    (mkdir-p out)
> +
> +                                    (generate-wrapper shell
> +                                                      (string-append out "/spacemacs")
> +                                                      emacs " --no-init-file" "--eval"
> +                                                      init-code)
> +
> +                                    (generate-wrapper shell
> +                                                      (string-append out
> +                                                                     "/spacemacsdaemon")
> +                                                      (string-append out "/spacemacs")
> +                                                      "--daemon=spacemacs")
> +
> +                                    (generate-wrapper shell
> +                                                      (string-append out
> +                                                                     "/spacemacsclient")
> +                                                      (string-append emacs "client")
> +                                                      "--socket-name" "spacemacs")))))

Please wrap lines as well.

> +(define-public emacs-spacemacs
> +  (generate-wrapped-emacs-spacemacs (@ (gnu packages emacs) emacs) spacemacs-rolling-release))

Please add “#:use-module (gnu packages emacs)” add the top and refer to
‘emacs’ directly, without the ‘@’ trick.

> +++ b/guix/build/spacemacs-utils.scm
> @@ -0,0 +1,47 @@
> +;;; GNU Guix --- Functional package management for GNU
> +;;; Copyright © 2016 David Craven <david@craven.ch>

That one also.  :-)

> +(define (create-init-string spacemacs)

As per <https://guix.gnu.org/manual/en/html_node/Coding-Style.html>,
please avoid abbreviations and add a docstring.  How about calling it
‘spacemacs-initialization-string’ or similar?

> +  (string-append "\"(progn (setq spacemacs-start-directory \\\"" spacemacs "/\\\") (setq "
> +                 "spacemacs-data-directory (concat (or (getenv \\\"XDG_DATA_DIR\\\") "
> +                 "(concat (getenv \\\"HOME\\\") \\\"/.local/share\\\")) "
> +                 "\\\"/spacemacs/\\\")) (setq package-user-dir (concat"
> +                 "spacemacs-data-directory \\\"elpa/\\\"))(load-file (concat "
> +                 " spacemacs-start-directory \\\"init.el\\\")))\""))

For clarity, I’d recommend writing it as:

  (object->string
   '(progn
      (setq spacemacs-start-directory "\" spacemacs "/"")
      …))

> +(define (string-append-with-space arg . arglist)
> +  (if (nil? arglist)
> +      arg
> +      (apply string-append-with-space
> +             (string-append arg " " (first arglist)) (drop arglist 1))))

You can use ‘string-join’ and remove this procedure.  It works like
this:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> (string-join '("a" "b"))
$2 = "a b"
--8<---------------cut here---------------end--------------->8---

> +(define (generate-wrapper shell output executable . args)

Please add a docstring.

> +  (with-output-to-file
> +      output (lambda ()
> +               (format #t (string-append "#!" shell "~%"
> +                                         (apply string-append-with-space
> +                                                "exec" "-a" shell executable args)
> +                                         " \"$@\""))))
> +  (chmod output #o555))

Nitpick: I’d suggest writing it like this:

  (call-with-output-file output
    (lambda (port)
      (format port …)))

Could you send an updated patch?  Thank you!

Ludo’.