diff mbox series

[bug#50077,1/3] gnu: notmuch: Add separate 'emacs' output.

Message ID dd24f426b72a1c590cbd71d2eaa66a75b559a9e7.1629122681.git.public@yoctocell.xyz
State Accepted
Headers show
Series Various improvements to Notmuch | expand

Checks

Context Check Description
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

Xinglu Chen Aug. 16, 2021, 2:12 p.m. UTC
* gnu/packages/mail.scm (notmuch)[outputs]: Add ‘emacs’ output
[arguments]<#:modules>: Add (guix build emacs-utils).
<#:phases>{configure}: Use the ‘emacs’ output.
{make-autoloads}: Modify the default ‘make-autoloads’ phase to use the ‘emacs’
output.
---
 gnu/packages/mail.scm | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

Comments

Xinglu Chen Aug. 28, 2021, 10:34 a.m. UTC | #1
[ Debbugs wasn’t Cc’d in the previous message ]

On Mon, Aug 23 2021, Andrew Tropin wrote:

> On 2021-08-16 16:12, Xinglu Chen wrote:
>
>> * gnu/packages/mail.scm (notmuch)[outputs]: Add ‘emacs’ output
>> [arguments]<#:modules>: Add (guix build emacs-utils).
>> <#:phases>{configure}: Use the ‘emacs’ output.
>> {make-autoloads}: Modify the default ‘make-autoloads’ phase to use the ‘emacs’
>> output.
>> ---
>>  gnu/packages/mail.scm | 16 +++++++++++++---
>>  1 file changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/gnu/packages/mail.scm b/gnu/packages/mail.scm
>> index ccf44b7cf3..bbc616f6f5 100644
>> --- a/gnu/packages/mail.scm
>> +++ b/gnu/packages/mail.scm
>> @@ -1337,7 +1337,8 @@ invoking @command{notifymuch} from the post-new hook.")
>>      (arguments
>>       `(#:modules ((guix build gnu-build-system)
>>                    ((guix build emacs-build-system) #:prefix emacs:)
>> -                  (guix build utils))
>> +                  (guix build utils)
>> +                  (guix build emacs-utils))
>>         #:imported-modules (,@%gnu-build-system-modules
>>                             (guix build emacs-build-system)
>>                             (guix build emacs-utils))
>> @@ -1355,7 +1356,8 @@ invoking @command{notifymuch} from the post-new hook.")
>>                        (setenv "CONFIG_SHELL" (which "sh"))
>>  
>>                        (let* ((out (assoc-ref outputs "out"))
>> -                             (elisp (emacs:elpa-directory out)))
>> +                             (emacs (assoc-ref outputs "emacs"))
>> +                             (elisp (emacs:elpa-directory emacs)))
>>                          (invoke "./configure"
>>                                  (string-append "--prefix=" out)
>>                                  (string-append "--emacslispdir=" elisp)
>> @@ -1373,7 +1375,14 @@ invoking @command{notifymuch} from the post-new hook.")
>>                        (substitute* (find-files "test" "\\.sh$")
>>                          (("/bin/sh") (which "sh")))))
>>                    (add-after 'install 'make-autoloads
>> -                    (assoc-ref emacs:%standard-phases 'make-autoloads)))))
>> +                    (lambda* (#:key outputs inputs #:allow-other-keys)
>> +                      (let* ((emacs (string-append (assoc-ref inputs "emacs") "/bin/emacs"))
>> +                             (out (assoc-ref outputs "emacs"))
>> +                             (elpa-name-ver ((@@ (guix build emacs-build-system)
>> +                                                 store-directory->elpa-name-version) out))
>> +                             (elpa-name (package-name->name+version elpa-name-ver))
>> +                             (el-dir (emacs:elpa-directory out)))
>> +                        (emacs-generate-autoloads elpa-name el-dir)))))))
>>      (native-inputs
>>       `(("bash-completion" ,bash-completion)
>>         ("emacs" ,emacs-no-x)    ; -minimal lacks libxml, needed for some tests
>> @@ -1394,6 +1403,7 @@ invoking @command{notifymuch} from the post-new hook.")
>>         ("talloc" ,talloc)
>>         ("xapian" ,xapian)
>>         ("zlib" ,zlib)))
>> +    (outputs '("out" "emacs"))
>>      (home-page "https://notmuchmail.org/")
>>      (synopsis "Thread-based email index, search, and tagging")
>>      (description
>
> The separate output is ok, but I think making a separate emacs-notmuch
> package looks more consistent.  We can inherit it from notmuch package
> to be sure that the packages are built from the same source code.

Why would it be more consistent to make a separate package?  Making a
separate package is usually used for packaging a slightly different
version of the “regular” package, e.g., ‘emacs-next-pgtk’ adds native
compilation and pure GTK support for Emacs., ‘emacs-no-x’ removes X
suport for ‘emacs’.  ‘emacs-notmuch’ isn’t really a different version of
‘notmuch’, it’s just ‘notmuch’ but with all the non-Elisp stuff
removed.  This is usually what using different outputs tries to achieve,
e.g., separate documentation from the main package, or in this case,
separate Elisp stuff from the main package.

The ‘notmuch’ package in Nixpkgs also uses multiple outputs[1], which I
think makes sense.

<https://github.com/NixOS/nixpkgs/blob/master/pkgs/applications/networking/mailreaders/notmuch/default.nix>
>
> Thank you for working on this, the changes are good and useful!)

You are welcome!  :-)
Andrew Tropin Aug. 30, 2021, 7:14 a.m. UTC | #2
On 2021-08-28 12:34, Xinglu Chen wrote:

> [ Debbugs wasn’t Cc’d in the previous message ]
>

Oops)

>
> On Mon, Aug 23 2021, Andrew Tropin wrote:
>
>> On 2021-08-16 16:12, Xinglu Chen wrote:
>>
>>> * gnu/packages/mail.scm (notmuch)[outputs]: Add ‘emacs’ output
>>> [arguments]<#:modules>: Add (guix build emacs-utils).
>>> <#:phases>{configure}: Use the ‘emacs’ output.
>>> {make-autoloads}: Modify the default ‘make-autoloads’ phase to use the ‘emacs’
>>> output.
>>> ---
>>>  gnu/packages/mail.scm | 16 +++++++++++++---
>>>  1 file changed, 13 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/gnu/packages/mail.scm b/gnu/packages/mail.scm
>>> index ccf44b7cf3..bbc616f6f5 100644
>>> --- a/gnu/packages/mail.scm
>>> +++ b/gnu/packages/mail.scm
>>> @@ -1337,7 +1337,8 @@ invoking @command{notifymuch} from the post-new hook.")
>>>      (arguments
>>>       `(#:modules ((guix build gnu-build-system)
>>>                    ((guix build emacs-build-system) #:prefix emacs:)
>>> -                  (guix build utils))
>>> +                  (guix build utils)
>>> +                  (guix build emacs-utils))
>>>         #:imported-modules (,@%gnu-build-system-modules
>>>                             (guix build emacs-build-system)
>>>                             (guix build emacs-utils))
>>> @@ -1355,7 +1356,8 @@ invoking @command{notifymuch} from the post-new hook.")
>>>                        (setenv "CONFIG_SHELL" (which "sh"))
>>>  
>>>                        (let* ((out (assoc-ref outputs "out"))
>>> -                             (elisp (emacs:elpa-directory out)))
>>> +                             (emacs (assoc-ref outputs "emacs"))
>>> +                             (elisp (emacs:elpa-directory emacs)))
>>>                          (invoke "./configure"
>>>                                  (string-append "--prefix=" out)
>>>                                  (string-append "--emacslispdir=" elisp)
>>> @@ -1373,7 +1375,14 @@ invoking @command{notifymuch} from the post-new hook.")
>>>                        (substitute* (find-files "test" "\\.sh$")
>>>                          (("/bin/sh") (which "sh")))))
>>>                    (add-after 'install 'make-autoloads
>>> -                    (assoc-ref emacs:%standard-phases 'make-autoloads)))))
>>> +                    (lambda* (#:key outputs inputs #:allow-other-keys)
>>> +                      (let* ((emacs (string-append (assoc-ref inputs "emacs") "/bin/emacs"))
>>> +                             (out (assoc-ref outputs "emacs"))
>>> +                             (elpa-name-ver ((@@ (guix build emacs-build-system)
>>> +                                                 store-directory->elpa-name-version) out))
>>> +                             (elpa-name (package-name->name+version elpa-name-ver))
>>> +                             (el-dir (emacs:elpa-directory out)))
>>> +                        (emacs-generate-autoloads elpa-name el-dir)))))))
>>>      (native-inputs
>>>       `(("bash-completion" ,bash-completion)
>>>         ("emacs" ,emacs-no-x)    ; -minimal lacks libxml, needed for some tests
>>> @@ -1394,6 +1403,7 @@ invoking @command{notifymuch} from the post-new hook.")
>>>         ("talloc" ,talloc)
>>>         ("xapian" ,xapian)
>>>         ("zlib" ,zlib)))
>>> +    (outputs '("out" "emacs"))
>>>      (home-page "https://notmuchmail.org/")
>>>      (synopsis "Thread-based email index, search, and tagging")
>>>      (description
>>
>> The separate output is ok, but I think making a separate emacs-notmuch
>> package looks more consistent.  We can inherit it from notmuch package
>> to be sure that the packages are built from the same source code.
>
> Why would it be more consistent to make a separate package?  Making a
> separate package is usually used for packaging a slightly different
> version of the “regular” package, e.g., ‘emacs-next-pgtk’ adds native
> compilation and pure GTK support for Emacs., ‘emacs-no-x’ removes X
> suport for ‘emacs’.  ‘emacs-notmuch’ isn’t really a different version of
> ‘notmuch’, it’s just ‘notmuch’ but with all the non-Elisp stuff
> removed.  This is usually what using different outputs tries to achieve,
> e.g., separate documentation from the main package, or in this case,
> separate Elisp stuff from the main package.
>

Almost all elisp packages in Guix have a emacs- prefix, so as a user I
expect to find notmuch*.el in emacs-notmuch package and notmuch binary
in notmuch package, despite the fact that upstream distributes the
source code for both of them in one tarball.  Moreover, with
emacs-notmuch and notmuch packages it's possible to use different build
systems, which helps in automated package modification, for example I
want to native compile all emacs packages or generate some additional
metainformation, I just find all packages in my profile with
emacs-build-system and modify phases accordingly.  Yes, it's possible to
do such modification for other packages manually, but it feels hacky and
inconvinient.

The separate output can work, but a separate package looks better to me.

P.S. I know that there are some emacs packages in Guix already, which
doesn't use emacs-build-system, but I think we should keep that number
as low as possible and ideally to make it equal to 0 =)

> 
> The ‘notmuch’ package in Nixpkgs also uses multiple outputs[1], which
> I think makes sense.
>
> <https://github.com/NixOS/nixpkgs/blob/master/pkgs/applications/networking/mailreaders/notmuch/default.nix>
>>
>> Thank you for working on this, the changes are good and useful!)
>
> You are welcome!  :-)
Xinglu Chen Aug. 30, 2021, 1:33 p.m. UTC | #3
On Mon, Aug 30 2021, Andrew Tropin wrote:

>> Why would it be more consistent to make a separate package?  Making a
>> separate package is usually used for packaging a slightly different
>> version of the “regular” package, e.g., ‘emacs-next-pgtk’ adds native
>> compilation and pure GTK support for Emacs., ‘emacs-no-x’ removes X
>> suport for ‘emacs’.  ‘emacs-notmuch’ isn’t really a different version of
>> ‘notmuch’, it’s just ‘notmuch’ but with all the non-Elisp stuff
>> removed.  This is usually what using different outputs tries to achieve,
>> e.g., separate documentation from the main package, or in this case,
>> separate Elisp stuff from the main package.
>>
>
> Almost all elisp packages in Guix have a emacs- prefix, so as a user I
> expect to find notmuch*.el in emacs-notmuch package and notmuch binary
> in notmuch package, despite the fact that upstream distributes the
> source code for both of them in one tarball.

Good point, however, If we were to have separate ‘emacs-’ packages for
the packages that also contain Elisp stuff, should those packages still
include the Emacs package in their output, i.e., should the ‘notmuch’
package still include notmuch.el, or should the Elisp stuff only be in
‘emacs-notmuch’?

> Moreover, with emacs-notmuch and notmuch packages it's possible to use
> different build systems, which helps in automated package
> modification, for example I want to native compile all emacs packages
> or generate some additional metainformation, I just find all packages
> in my profile with emacs-build-system and modify phases accordingly.
> Yes, it's possible to do such modification for other packages
> manually, but it feels hacky and inconvinient.

This sounds like an interesting use-case, but I don’t know how useful
this will be for most people.
Andrew Tropin Sept. 1, 2021, 9:50 a.m. UTC | #4
On 2021-08-30 15:33, Xinglu Chen wrote:

> On Mon, Aug 30 2021, Andrew Tropin wrote:
>
>>> Why would it be more consistent to make a separate package?  Making a
>>> separate package is usually used for packaging a slightly different
>>> version of the “regular” package, e.g., ‘emacs-next-pgtk’ adds native
>>> compilation and pure GTK support for Emacs., ‘emacs-no-x’ removes X
>>> suport for ‘emacs’.  ‘emacs-notmuch’ isn’t really a different version of
>>> ‘notmuch’, it’s just ‘notmuch’ but with all the non-Elisp stuff
>>> removed.  This is usually what using different outputs tries to achieve,
>>> e.g., separate documentation from the main package, or in this case,
>>> separate Elisp stuff from the main package.
>>>
>>
>> Almost all elisp packages in Guix have a emacs- prefix, so as a user I
>> expect to find notmuch*.el in emacs-notmuch package and notmuch binary
>> in notmuch package, despite the fact that upstream distributes the
>> source code for both of them in one tarball.
>
> Good point, however, If we were to have separate ‘emacs-’ packages for
> the packages that also contain Elisp stuff, should those packages still
> include the Emacs package in their output, i.e., should the ‘notmuch’
> package still include notmuch.el, or should the Elisp stuff only be in
> ‘emacs-notmuch’?
>

IMO, notmuch package should not include Elisp stuff, at least I don't
see use cases, where it can be useful, but see where it can be harmful.

>
>> Moreover, with emacs-notmuch and notmuch packages it's possible to use
>> different build systems, which helps in automated package
>> modification, for example I want to native compile all emacs packages
>> or generate some additional metainformation, I just find all packages
>> in my profile with emacs-build-system and modify phases accordingly.
>> Yes, it's possible to do such modification for other packages
>> manually, but it feels hacky and inconvinient.
>
> This sounds like an interesting use-case, but I don’t know how useful
> this will be for most people.

Even if there are not so many people, who do it right now or even in
some future, it's very likely that some derivative distribution with
considerable amount of users will appear, which will be utilizing such
capabilities.

Actually, the exact use case doesn't matter so much, but the whole
picture, where you need to track every package, which picks some phases
of one build system to make sure that your transformation of package
list works correctly is.  More regular and consistent package
definitions are, the easier and simplier to transform them
programmatically.
Xinglu Chen Sept. 1, 2021, 12:05 p.m. UTC | #5
On Wed, Sep 01 2021, Andrew Tropin wrote:

> On 2021-08-30 15:33, Xinglu Chen wrote:
>
>> On Mon, Aug 30 2021, Andrew Tropin wrote:
>>
>>>> Why would it be more consistent to make a separate package?  Making a
>>>> separate package is usually used for packaging a slightly different
>>>> version of the “regular” package, e.g., ‘emacs-next-pgtk’ adds native
>>>> compilation and pure GTK support for Emacs., ‘emacs-no-x’ removes X
>>>> suport for ‘emacs’.  ‘emacs-notmuch’ isn’t really a different version of
>>>> ‘notmuch’, it’s just ‘notmuch’ but with all the non-Elisp stuff
>>>> removed.  This is usually what using different outputs tries to achieve,
>>>> e.g., separate documentation from the main package, or in this case,
>>>> separate Elisp stuff from the main package.
>>>>
>>>
>>> Almost all elisp packages in Guix have a emacs- prefix, so as a user I
>>> expect to find notmuch*.el in emacs-notmuch package and notmuch binary
>>> in notmuch package, despite the fact that upstream distributes the
>>> source code for both of them in one tarball.
>>
>> Good point, however, If we were to have separate ‘emacs-’ packages for
>> the packages that also contain Elisp stuff, should those packages still
>> include the Emacs package in their output, i.e., should the ‘notmuch’
>> package still include notmuch.el, or should the Elisp stuff only be in
>> ‘emacs-notmuch’?
>>
>
> IMO, notmuch package should not include Elisp stuff, at least I don't
> see use cases, where it can be useful, but see where it can be
> harmful.

Should this apply to other packages that contains Elisp stuff too, or is
it specific to ‘notmuch’?

Cc’ing guix-devel to see what other people think before we start
breaking people’s setups.  :-)
Leo Prikler Sept. 1, 2021, 12:48 p.m. UTC | #6
Am Mittwoch, den 01.09.2021, 14:05 +0200 schrieb Xinglu Chen:
> > IMO, notmuch package should not include Elisp stuff, at least I
> > don't see use cases, where it can be useful, but see where it can
> > be harmful.
> 
> Should this apply to other packages that contains Elisp stuff too, or
> is it specific to ‘notmuch’?
> 
> Cc’ing guix-devel to see what other people think before we start
> breaking people’s setups.  :-)
In my personal opinion providing a separate package (perhaps one using
emacs-build-system) is to be preferred as per the principle of least
surprise.  However, in some situations we might want to hold back on
that, e.g. if providing an extra emacs package would entail propagating
the original package just because.

On current master, there's quite a number of packages that require
mixing emacs-build-system into something else.  Reducing this number
would make changes to emacs-build-system cause less breakages, some of
which we've seen in the past and some of which could possibly happen in
the future, if e.g. post native-compilation we realize that we need an
extra phase to deal with <insert stuff here>.

TL;DR: I'm generally in favor of branching emacs support packages off,
even if origins are to be inherited.

Regards
Simon Tournier Sept. 1, 2021, 1:52 p.m. UTC | #7
Hi,

On Wed, 01 Sep 2021 at 14:05, Xinglu Chen <public@yoctocell.xyz> wrote:

> Cc’ing guix-devel to see what other people think before we start
> breaking people’s setups.  :-)

I agree with this Andrew’s comment:

        P.S. I know that there are some emacs packages in Guix already, which
        doesn't use emacs-build-system, but I think we should keep that number
        as low as possible and ideally to make it equal to 0 =)
        
<http://issues.guix.gnu.org/issue/50077#5>

If I do:

  guix install emacs-next notmuch

then there is no guarantee that “M-x notmuch” will work.  Because
’notmuch.el’ is byte-compiled using ’emacs-no-x’.  The issue is that
some Emacs packages rely on ’emacs-minimal’, others on ’emacs-no-x’ as
input, others on other Emacs VM variant, therefore the transformation

  guix build -m manifest.scm --with-input=emacs-minimal=emacs-next

will not work, as pointed by Nicolas here [1].  Well, you will tell me
that ’outputs’ does not change much the issue. :-) For sure, but IMHO
having Emacs packages using ’emacs-build-system’ eases the write of
generic transformation.  Well, there is enough corner cases with Emacs
packages using ’emacs-build-system’ which rewriting their ’#:emacs’
argument.  Other speaking about Emacs packages using other build
systems.

Another point is, if I want to build ’notmuch’ but I am not an Emacs
user, then:

  guix environment notmuch

will download ’emacs-no-x’ for nothing.  When my network is poor, I am
unhappy.  Although, it is already the case. :-) Well, this is something
known, see:

<https://git.savannah.gnu.org/cgit/guix.git/tree/TODO#n37>

I am not convinced that several outputs help.  And generally speaking,
personally, I tend to prefer package inherit over several outputs.
Matter of taste I guess. :-)

Without speaking about cross-compilation. ;-)

From my point of view, I would split the package ’notmuch’ and propagate
this new ’notmuch’ package with a new ’emacs-notmuch’ (if ’notmuch.el’
requires it).  Well, from my point of view, it would be how to improve
the situation. :-)


All the best,
simon

1: <http://issues.guix.gnu.org/41732#14>
Carlo Zancanaro Sept. 1, 2021, 11:25 p.m. UTC | #8
On Wed, Sep 01 2021, Liliana Marie Prikler wrote:
> TL;DR: I'm generally in favor of branching emacs support 
> packages off, even if origins are to be inherited.

This is my preference, and there is precedent for this in Guix 
already. I know of emacs-protobuf-mode and emacs-erlang which are 
separate packages, but which reference the source of an existing 
package (with (package-source protobuf) and (package-source 
erlang), respectively).

I like how easy it is to discover Emacs packages by looking for 
the emacs- prefix. Mu and notmuch already violate that prefix 
expectation, moving their elisp into a separate output would be 
further hiding the Emacs modes.

Carlo
Xinglu Chen Sept. 3, 2021, 4:14 p.m. UTC | #9
On Thu, Sep 02 2021, Carlo Zancanaro wrote:

> On Wed, Sep 01 2021, Liliana Marie Prikler wrote:
>> TL;DR: I'm generally in favor of branching emacs support 
>> packages off, even if origins are to be inherited.
>
> This is my preference, and there is precedent for this in Guix 
> already. I know of emacs-protobuf-mode and emacs-erlang which are 
> separate packages, but which reference the source of an existing 
> package (with (package-source protobuf) and (package-source 
> erlang), respectively).
>
> I like how easy it is to discover Emacs packages by looking for 
> the emacs- prefix. Mu and notmuch already violate that prefix 
> expectation, moving their elisp into a separate output would be 
> further hiding the Emacs modes.
>
> Carlo

Looks like there is consensus on the matter, unless someone objects, I
will send an updated series that adds ‘emacs-notmuch’ instead of adding
an extra output to ‘notmuch’.  :-)
diff mbox series

Patch

diff --git a/gnu/packages/mail.scm b/gnu/packages/mail.scm
index ccf44b7cf3..bbc616f6f5 100644
--- a/gnu/packages/mail.scm
+++ b/gnu/packages/mail.scm
@@ -1337,7 +1337,8 @@  invoking @command{notifymuch} from the post-new hook.")
     (arguments
      `(#:modules ((guix build gnu-build-system)
                   ((guix build emacs-build-system) #:prefix emacs:)
-                  (guix build utils))
+                  (guix build utils)
+                  (guix build emacs-utils))
        #:imported-modules (,@%gnu-build-system-modules
                            (guix build emacs-build-system)
                            (guix build emacs-utils))
@@ -1355,7 +1356,8 @@  invoking @command{notifymuch} from the post-new hook.")
                       (setenv "CONFIG_SHELL" (which "sh"))
 
                       (let* ((out (assoc-ref outputs "out"))
-                             (elisp (emacs:elpa-directory out)))
+                             (emacs (assoc-ref outputs "emacs"))
+                             (elisp (emacs:elpa-directory emacs)))
                         (invoke "./configure"
                                 (string-append "--prefix=" out)
                                 (string-append "--emacslispdir=" elisp)
@@ -1373,7 +1375,14 @@  invoking @command{notifymuch} from the post-new hook.")
                       (substitute* (find-files "test" "\\.sh$")
                         (("/bin/sh") (which "sh")))))
                   (add-after 'install 'make-autoloads
-                    (assoc-ref emacs:%standard-phases 'make-autoloads)))))
+                    (lambda* (#:key outputs inputs #:allow-other-keys)
+                      (let* ((emacs (string-append (assoc-ref inputs "emacs") "/bin/emacs"))
+                             (out (assoc-ref outputs "emacs"))
+                             (elpa-name-ver ((@@ (guix build emacs-build-system)
+                                                 store-directory->elpa-name-version) out))
+                             (elpa-name (package-name->name+version elpa-name-ver))
+                             (el-dir (emacs:elpa-directory out)))
+                        (emacs-generate-autoloads elpa-name el-dir)))))))
     (native-inputs
      `(("bash-completion" ,bash-completion)
        ("emacs" ,emacs-no-x)    ; -minimal lacks libxml, needed for some tests
@@ -1394,6 +1403,7 @@  invoking @command{notifymuch} from the post-new hook.")
        ("talloc" ,talloc)
        ("xapian" ,xapian)
        ("zlib" ,zlib)))
+    (outputs '("out" "emacs"))
     (home-page "https://notmuchmail.org/")
     (synopsis "Thread-based email index, search, and tagging")
     (description