[bug#38643] [PATCH] Add spacemacs package

[Ludovic Courtès]

Hello!

saffronsnail <address@hidden> 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 <address@hidden>
> 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 <address@hidden>

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 <address@hidden>

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’.