diff mbox series

[bug#39804] gnu: add emacs-exwm-next package (i.e. exwm for emacs-next)

Message ID 87tv3bd6v1.fsf@web.de
State Superseded
Headers show
Series [bug#39804] gnu: add emacs-exwm-next package (i.e. exwm for emacs-next) | expand

Checks

Context Check Description
cbaines/applying patch fail View Laminar job

Commit Message

dario Feb. 27, 2020, 10:04 p.m. UTC
Thanks for your feedback.

>> ---
>>  gnu/packages/emacs-xyz.scm | 71 ++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 71 insertions(+)
>>
>> diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
>> index 6b9027df8a..c6df469895 100644
>> --- a/gnu/packages/emacs-xyz.scm
>> +++ b/gnu/packages/emacs-xyz.scm
>> @@ -8601,6 +8601,77 @@ It should enable you to implement low-level X11 applications.")
>>  built on top of XELB.")
>>      (license license:gpl3+)))
>>
>> +(define-public emacs-exwm-next
>
> I think you don't need to copy the whole package definition.  Instead,
> you could `inherit' from the original definition and only adjust the
> name, description, inputs and maybe arguments.
>
> For instance
>
> --8<---------------cut here---------------start------------->8---
> (define-public emacs-exwm-next
>   (package
>     (inherit emacs)
>     (name "emacs-exwm-next")
>     (inputs ...)
>     (synopsys ...)))
> --8<---------------cut here---------------end--------------->8---
>
> See `substitute-keyword-arguments' in the fftwf package for a convenient
> way to modify just one argument.
>
>> +  (package
>> +    (name "emacs-exwm-next")
>> +    (version "0.23")
>> +    (synopsis "Emacs X window manager")

Nice - I was not aware of that. The following definition seems to work
(see also the patch at the end of this mail)
> --8<---------------cut here---------------start------------->8---
(define-public emacs-exwm-next
  (package
    (inherit emacs-exwm)
    (name "emacs-exwm-next")
    (synopsis "Emacs X window manager using the next version of emacs")
    (inputs
     (cons
      `("emacs-next" ,emacs-next)
      (delete `("emacs" ,emacs)
              (package-inputs emacs-exwm))))))
> --8<---------------cut here---------------end------------->8---

>> +    (arguments
>> +     `(#:emacs ,emacs
>
> Shouldn't this be `emacs-next` as well?
I have to admit that I do not understand that part. If I change it to
emacs-next, the build fails with the error
> --8<---------------cut here---------------start------------->8---
phase `install' succeeded after 0.0 seconds
starting phase `make-autoloads'
Wrong type argument: stringp, nil
command "/gnu/store/1z520fgx6fiq426yf2174kal2q63a9q7-emacs-next-27.0.50-0.36abf68/bin/emacs" "--quick" "--batch" "--eval=(let ((backup-inhibited t) (generated-autoload-file \"/gnu/store/nnjcqc448yj79dxaj11fnq7s9a8zpc1z-emacs-exwm-next-test-0.23/share/emacs/site-lisp/exwm-next-test-autoloads.el\")) (update-directory-autoloads \"/gnu/store/nnjcqc448yj79dxaj11fnq7s9a8zpc1z-emacs-exwm-next-test-0.23/share/emacs/site-lisp\"))" failed with status 255
> --8<---------------cut here---------------end------------->8---
It is a bit difficult for me to understand what is going on here,
because, like I said, I do not really understand this part of the
package definition in the first place - sorry. However, without
modifying the arguments, everything seems to work.

The following patch seems to work (do I need to send it on its own? I am
new to this type of workflow.)

Best
Dario

---
 gnu/packages/emacs-xyz.scm | 72 ++++----------------------------------
 1 file changed, 6 insertions(+), 66 deletions(-)

--
2.25.1

Comments

Pierre Neidhardt Feb. 28, 2020, 8:32 a.m. UTC | #1
Hi Dario,

dario <dario.klingenberg@web.de> writes:

> Thanks for your feedback.
>
>>> ---
>>>  gnu/packages/emacs-xyz.scm | 71 ++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 71 insertions(+)
>>>
>>> diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
>>> index 6b9027df8a..c6df469895 100644
>>> --- a/gnu/packages/emacs-xyz.scm
>>> +++ b/gnu/packages/emacs-xyz.scm
>>> @@ -8601,6 +8601,77 @@ It should enable you to implement low-level X11 applications.")
>>>  built on top of XELB.")
>>>      (license license:gpl3+)))
>>>
>>> +(define-public emacs-exwm-next
>>
>> I think you don't need to copy the whole package definition.  Instead,
>> you could `inherit' from the original definition and only adjust the
>> name, description, inputs and maybe arguments.
>>
>> For instance
>>
>> --8<---------------cut here---------------start------------->8---
>> (define-public emacs-exwm-next
>>   (package
>>     (inherit emacs)
>>     (name "emacs-exwm-next")
>>     (inputs ...)
>>     (synopsys ...)))
>> --8<---------------cut here---------------end--------------->8---
>>
>> See `substitute-keyword-arguments' in the fftwf package for a convenient
>> way to modify just one argument.
>>
>>> +  (package
>>> +    (name "emacs-exwm-next")
>>> +    (version "0.23")
>>> +    (synopsis "Emacs X window manager")
>
> Nice - I was not aware of that. The following definition seems to work
> (see also the patch at the end of this mail)
>> --8<---------------cut here---------------start------------->8---
> (define-public emacs-exwm-next
>   (package
>     (inherit emacs-exwm)
>     (name "emacs-exwm-next")
>     (synopsis "Emacs X window manager using the next version of emacs")
>     (inputs
>      (cons
>       `("emacs-next" ,emacs-next)
>       (delete `("emacs" ,emacs)
>               (package-inputs emacs-exwm))))))

Maybe a bit better:

    (inputs
     `(("emacs" ,emacs-next)
       ,@(alist-delete "emacs" (package-inputs emacs-exwm))))))

>> --8<---------------cut here---------------end------------->8---
>
>>> +    (arguments
>>> +     `(#:emacs ,emacs
>>
>> Shouldn't this be `emacs-next` as well?
> I have to admit that I do not understand that part. If I change it to
> emacs-next, the build fails with the error
>> --8<---------------cut here---------------start------------->8---
> phase `install' succeeded after 0.0 seconds
> starting phase `make-autoloads'
> Wrong type argument: stringp, nil
> command "/gnu/store/1z520fgx6fiq426yf2174kal2q63a9q7-emacs-next-27.0.50-0.36abf68/bin/emacs" "--quick" "--batch" "--eval=(let ((backup-inhibited t) (generated-autoload-file \"/gnu/store/nnjcqc448yj79dxaj11fnq7s9a8zpc1z-emacs-exwm-next-test-0.23/share/emacs/site-lisp/exwm-next-test-autoloads.el\")) (update-directory-autoloads \"/gnu/store/nnjcqc448yj79dxaj11fnq7s9a8zpc1z-emacs-exwm-next-test-0.23/share/emacs/site-lisp\"))" failed with status 255
>> --8<---------------cut here---------------end------------->8---
> It is a bit difficult for me to understand what is going on here,
> because, like I said, I do not really understand this part of the
> package definition in the first place - sorry. However, without
> modifying the arguments, everything seems to work.

The `#:emacs` field tells the build system which Emacs package to use to
build this package.  There may be something that not compatible between
our current build system and emacs-next.

I've CC'ed Maxim and Leo, they might know more than me.

> The following patch seems to work (do I need to send it on its own? I am
> new to this type of workflow.)

Attaching to this email is fine.
Better is to use `git send-email` like so:

--8<---------------cut here---------------start------------->8---
git send-email --to=39804@debbugs.gnu.org --in-reply-to=<MESSAGE-ID> 0001-your.patch
--8<---------------cut here---------------end--------------->8---

with <MESSAGE-ID> being the message ID of the email you are replying to.
You can get this message ID in various ways: one way that always works
is to look up the "message-id" header of the raw email.

Also when you generate the patch with git, set the subject prefix to
[PATCH v2] for the second iteration, PATCH v3 for the third, etc.

Thanks!
diff mbox series

Patch

diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
index c6df469895..3d5b650df9 100644
--- a/gnu/packages/emacs-xyz.scm
+++ b/gnu/packages/emacs-xyz.scm
@@ -8603,74 +8603,14 @@  built on top of XELB.")

 (define-public emacs-exwm-next
   (package
+    (inherit emacs-exwm)
     (name "emacs-exwm-next")
-    (version "0.23")
-    (synopsis "Emacs X window manager")
-    (source (origin
-              (method url-fetch)
-              (uri (string-append "https://elpa.gnu.org/packages/exwm-"
-                                  version ".tar"))
-              (sha256
-               (base32
-                "05w1v3wrp1lzz20zd9lcvr5nhk809kgy6svvkbs15xhnr6x55ad5"))))
-    (build-system emacs-build-system)
-    (propagated-inputs
-     `(("emacs-xelb" ,emacs-xelb)))
+    (synopsis "Emacs X window manager using the next version of emacs")
     (inputs
-     `(("xhost" ,xhost)
-       ("emacs-next" ,emacs-next)
-       ("dbus" ,dbus)))
-    ;; The following functions and variables needed by emacs-exwm are
-    ;; not included in emacs-minimal:
-    ;; scroll-bar-mode, fringe-mode
-    ;; x-display-pixel-width, x-display-pixel-height
-    (arguments
-     `(#:emacs ,emacs
-       #:phases
-       (modify-phases %standard-phases
-         (add-after 'build 'install-xsession
-           (lambda* (#:key inputs outputs #:allow-other-keys)
-             (let* ((out (assoc-ref outputs "out"))
-                    (xsessions (string-append out "/share/xsessions"))
-                    (bin (string-append out "/bin"))
-                    (exwm-executable (string-append bin "/exwm")))
-               ;; Add a .desktop file to xsessions
-               (mkdir-p xsessions)
-               (mkdir-p bin)
-               (with-output-to-file
-                   (string-append xsessions "/exwm.desktop")
-                 (lambda _
-                   (format #t "[Desktop Entry]~@
-                     Name=~a~@
-                     Comment=~a~@
-                     Exec=~a~@
-                     TryExec=~:*~a~@
-                     Type=Application~%" ,name ,synopsis exwm-executable)))
-               ;; Add a shell wrapper to bin
-               (with-output-to-file exwm-executable
-                 (lambda _
-                   (format #t "#!~a ~@
-                     ~a +SI:localuser:$USER ~@
-                     exec ~a --exit-with-session ~a \"$@\" --eval '~s' ~%"
-                           (string-append (assoc-ref inputs "bash") "/bin/sh")
-                           (string-append (assoc-ref inputs "xhost") "/bin/xhost")
-                           (string-append (assoc-ref inputs "dbus") "/bin/dbus-launch")
-                           (string-append (assoc-ref inputs "emacs-next") "/bin/emacs")
-                           '(cond
-                             ((file-exists-p "~/.exwm")
-                              (load-file "~/.exwm"))
-                             ((not (featurep 'exwm))
-                              (require 'exwm)
-                              (require 'exwm-config)
-                              (exwm-config-default)
-                              (message (concat "exwm configuration not found. "
-                                               "Falling back to default configuration...")))))))
-               (chmod exwm-executable #o555)
-               #t))))))
-    (home-page "https://github.com/ch11ng/exwm")
-    (description "EXWM is a full-featured tiling X window manager for Emacs
-built on top of XELB.")
-    (license license:gpl3+)))
+     (cons
+      `("emacs-next" ,emacs-next)
+      (delete `("emacs" ,emacs)
+              (package-inputs emacs-exwm))))))

 (define-public emacs-switch-window
   (package