Re: [PATCH v4] gnu: emacs: Update to 27.1.

Re: [PATCH v4] gnu: emacs: Update to 27.1.

[Ludovic Courtès]

Hi Morgan, Mark, and all,

Morgan.J.Smith@outlook.com skribis:

> From: Morgan Smith <Morgan.J.Smith@outlook.com>
>
> * gnu/packages/emacs.scm (emacs): Update to 27.1.
> [arguments]: Add --with-cairo and --with-modules to #:configure-flags. Add
> restore-emacs-pdump phase.
> [inputs]: Add cairo, libxaw, jansson, gmp, and harfbuzz. Remove imagemagick
> and libxft.
> [native-inputs]: Add texlive.
> (emacs-wide-int): Mark as deprecated package.
> (emacs-no-x):
> [arguments]: Add --with-jpeg=no --with-gif=no --with-tiff=no
> to #:configure-flags.

I see that Mark committed a similar patch just yesterday:

  https://git.savannah.gnu.org/cgit/guix.git/commit/?id=36a09d185343375a5cba370431870f9c4435d623

I suppose Mark hadn’t seen the ongoing discussion.

Mark, Morgan: could you see if there’s anything we’re missing from the
patch Morgan submitted?

At any rate, thanks a lot for the work everyone put in!

Ludo’, a happy Emacs user.

Re: [PATCH v4] gnu: emacs: Update to 27.1.

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 153 bytes --]

I confirm that with Mark's commit 
emacs-clojure-mode and emacs-elisp-refs are also broken.

Cheers!

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: [bug#42738] [PATCH v4] gnu: emacs: Update to 27.1.

[Brett Gilio]

Ludovic Courtès <ludo@gnu.org> writes:

> Hi Morgan, Mark, and all,
>
> Morgan.J.Smith@outlook.com skribis:
>
>> From: Morgan Smith <Morgan.J.Smith@outlook.com>
>>
>> * gnu/packages/emacs.scm (emacs): Update to 27.1.
>> [arguments]: Add --with-cairo and --with-modules to #:configure-flags. Add
>> restore-emacs-pdump phase.
>> [inputs]: Add cairo, libxaw, jansson, gmp, and harfbuzz. Remove imagemagick
>> and libxft.
>> [native-inputs]: Add texlive.
>> (emacs-wide-int): Mark as deprecated package.
>> (emacs-no-x):
>> [arguments]: Add --with-jpeg=no --with-gif=no --with-tiff=no
>> to #:configure-flags.
>
> I see that Mark committed a similar patch just yesterday:
>
>   https://git.savannah.gnu.org/cgit/guix.git/commit/?id=36a09d185343375a5cba370431870f9c4435d623
>
> I suppose Mark hadn’t seen the ongoing discussion.
>
> Mark, Morgan: could you see if there’s anything we’re missing from the
> patch Morgan submitted?
>
> At any rate, thanks a lot for the work everyone put in!
>
> Ludo’, a happy Emacs user.

Also, are we planning to keep emacs-next and have it track 28.x or
remove it?

Brett Gilio

Re: [bug#42738] [PATCH v4] gnu: emacs: Update to 27.1.

[Jack Hill]

On Fri, 28 Aug 2020, Brett Gilio wrote:

> Also, are we planning to keep emacs-next and have it track 28.x or
> remove it?

I believe bandali was planning to track 28.x.

Best,
Jack

Re: [bug#42738] [PATCH v4] gnu: emacs: Update to 27.1.

[Amin Bandali]

[-- Attachment #1: Type: text/plain, Size: 696 bytes --]

Brett Gilio writes:

[...]
>
> Also, are we planning to keep emacs-next and have it track 28.x or
> remove it?
>
> Brett Gilio

I would like it if it's kept.  Though, if it is truly meant to track the
"next" release of Emacs, it should be pointed at the `emacs-27' branch,
as there will likely be one or more point releases in the 27 series.

On the other hand, as an Emacs developer, I would appreciate having a
package that tracks `master' that I could use.  Perhaps emacs-trunk
would be a more appropriate name for that package.  I know it's not
typical for guix.git to contain package definitions that point to
non-release commits; but perhaps Emacs could be an exception to that.

Thoughts?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]

Re: [bug#42738] [PATCH v4] gnu: emacs: Update to 27.1.

[Mark H Weaver]

Hi Amin,

Amin Bandali <bandali@gnu.org> wrote:

> Brett Gilio writes:
> 
> [...]
>>
>> Also, are we planning to keep emacs-next and have it track 28.x or
>> remove it?
>>
>> Brett Gilio
>
> I would like it if it's kept.  Though, if it is truly meant to track the
> "next" release of Emacs, it should be pointed at the `emacs-27' branch,
> as there will likely be one or more point releases in the 27 series.

Feel free to re-add it.  I was torn about whether to remove it.  Anyway,
most of its code is now integrated into the main 'emacs' package, so it
should be a lot simpler now.  Here's a first draft (untested), but of
course it should be updated to a later commit:

--8<---------------cut here---------------start------------->8---
(define-public emacs-next
  (let ((commit "c36c5a3dedbb2e0349be1b6c3b7567ea7b594f1c")
        (revision "0")
        (emacs-version "27.0.91"))
    (package
      (inherit emacs)
      (name "emacs-next")
      (version (git-version emacs-version revision commit))
      (source
       (origin
         (inherit (package-source emacs))
         (method git-fetch)
         (uri (git-reference
               (url "https://git.savannah.gnu.org/git/emacs.git")
               (commit commit)))
         (sha256
          (base32 "0mlrg2npy1r79laahkgzhxd1qassfcdz8qk1cpw7mqgf6y5x505h"))
         (file-name (git-file-name name version))))
      (native-inputs
       `(("autoconf" ,autoconf)      ; needed when building from trunk
         ,@(package-native-inputs emacs)))

      ;; TODO: consider changing `emacs' to use a more robust way of
      ;; specifying version for "EMACSLOADPATH", so as to avoid having to
      ;; duplicate native-search-paths here.
      (native-search-paths
       (list (search-path-specification
              (variable "EMACSLOADPATH")
              ;; The versioned entry is for the Emacs' builtin libraries.
              (files
               (list "share/emacs/site-lisp"
                     (string-append "share/emacs/" emacs-version "/lisp"))))
             (search-path-specification
              (variable "INFOPATH")
              (files '("share/info"))))))))
--8<---------------cut here---------------end--------------->8---

     Regards,
       Mark

Re: [bug#42738] [PATCH v4] gnu: emacs: Update to 27.1.

[Ludovic Courtès]

Hi,

Mark H Weaver <mhw@netris.org> skribis:

> (define-public emacs-next
>   (let ((commit "c36c5a3dedbb2e0349be1b6c3b7567ea7b594f1c")
>         (revision "0")
>         (emacs-version "27.0.91"))
>     (package
>       (inherit emacs)
>       (name "emacs-next")
>       (version (git-version emacs-version revision commit))
>       (source
>        (origin
>          (inherit (package-source emacs))
>          (method git-fetch)
>          (uri (git-reference
>                (url "https://git.savannah.gnu.org/git/emacs.git")
>                (commit commit)))

This can be handled with ‘--with-git-url’.

>          (sha256
>           (base32 "0mlrg2npy1r79laahkgzhxd1qassfcdz8qk1cpw7mqgf6y5x505h"))
>          (file-name (git-file-name name version))))
>       (native-inputs
>        `(("autoconf" ,autoconf)      ; needed when building from trunk
>          ,@(package-native-inputs emacs)))

For this, we’d need a new ‘--with-extra-input’ package transformation
option or similar.  That way, we wouldn’t even need an ‘emacs-next’
package: people would just run

  guix install emacs --with-git-url=… --with-extra-input=autoconf

OTOH it’s more typing and not necessarily obvious…

Ludo’.

Re: [bug#42738] [PATCH v4] gnu: emacs: Update to 27.1.

[Mark H Weaver]

Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> wrote:
> Mark H Weaver <mhw@netris.org> skribis:
>
>> (define-public emacs-next
>>   (let ((commit "c36c5a3dedbb2e0349be1b6c3b7567ea7b594f1c")
>>         (revision "0")
>>         (emacs-version "27.0.91"))
>>     (package
>>       (inherit emacs)
>>       (name "emacs-next")
>>       (version (git-version emacs-version revision commit))
>>       (source
>>        (origin
>>          (inherit (package-source emacs))
>>          (method git-fetch)
>>          (uri (git-reference
>>                (url "https://git.savannah.gnu.org/git/emacs.git")
>>                (commit commit)))
>
> This can be handled with ‘--with-git-url’.

I think that wouldn't work in this case, because we also need to
preserve the existing 'patches' and 'snippet' fields, which I arranged
to inherit above via (inherit (package-source emacs)).  That probably
deserves a comment, since it's easily overlooked.

>>          (sha256
>>           (base32 "0mlrg2npy1r79laahkgzhxd1qassfcdz8qk1cpw7mqgf6y5x505h"))
>>          (file-name (git-file-name name version))))
>>       (native-inputs
>>        `(("autoconf" ,autoconf)      ; needed when building from trunk
>>          ,@(package-native-inputs emacs)))
>
> For this, we’d need a new ‘--with-extra-input’ package transformation
> option or similar.  That way, we wouldn’t even need an ‘emacs-next’
> package: people would just run
>
>   guix install emacs --with-git-url=… --with-extra-input=autoconf

There's also the 'native-search-paths' field, which cannot simply be
inherited because of the version number embedded within EMACSLOADPATH.
This particular issue could be avoided if the 'native-search-paths'
field were a function of the version number, but that raises migration
issues and I'm not sure it's worth it.

What do you think?

    Best regards,
        Mark

Re: [bug#42738] [PATCH v4] gnu: emacs: Update to 27.1.

[Ludovic Courtès]

Hi Mark,

Mark H Weaver <mhw@netris.org> skribis:

> Ludovic Courtès <ludo@gnu.org> wrote:
>> Mark H Weaver <mhw@netris.org> skribis:
>>
>>> (define-public emacs-next
>>>   (let ((commit "c36c5a3dedbb2e0349be1b6c3b7567ea7b594f1c")
>>>         (revision "0")
>>>         (emacs-version "27.0.91"))
>>>     (package
>>>       (inherit emacs)
>>>       (name "emacs-next")
>>>       (version (git-version emacs-version revision commit))
>>>       (source
>>>        (origin
>>>          (inherit (package-source emacs))
>>>          (method git-fetch)
>>>          (uri (git-reference
>>>                (url "https://git.savannah.gnu.org/git/emacs.git")
>>>                (commit commit)))
>>
>> This can be handled with ‘--with-git-url’.
>
> I think that wouldn't work in this case, because we also need to
> preserve the existing 'patches' and 'snippet' fields, which I arranged
> to inherit above via (inherit (package-source emacs)).  That probably
> deserves a comment, since it's easily overlooked.
>
>>>          (sha256
>>>           (base32 "0mlrg2npy1r79laahkgzhxd1qassfcdz8qk1cpw7mqgf6y5x505h"))
>>>          (file-name (git-file-name name version))))
>>>       (native-inputs
>>>        `(("autoconf" ,autoconf)      ; needed when building from trunk
>>>          ,@(package-native-inputs emacs)))
>>
>> For this, we’d need a new ‘--with-extra-input’ package transformation
>> option or similar.  That way, we wouldn’t even need an ‘emacs-next’
>> package: people would just run
>>
>>   guix install emacs --with-git-url=… --with-extra-input=autoconf
>
> There's also the 'native-search-paths' field, which cannot simply be
> inherited because of the version number embedded within EMACSLOADPATH.
> This particular issue could be avoided if the 'native-search-paths'
> field were a function of the version number, but that raises migration
> issues and I'm not sure it's worth it.
>
> What do you think?

Ah yes, both good points that I had overlooked.

Thanks,
Ludo’.

Re: [PATCH v4] gnu: emacs: Update to 27.1.

[Mark H Weaver]

Hello Guix,

Ludovic Courtès <ludo@gnu.org> wrote:

> Morgan.J.Smith@outlook.com skribis:
>
>> From: Morgan Smith <Morgan.J.Smith@outlook.com>
>>
>> * gnu/packages/emacs.scm (emacs): Update to 27.1.
>> [arguments]: Add --with-cairo and --with-modules to #:configure-flags. Add
>> restore-emacs-pdump phase.
>> [inputs]: Add cairo, libxaw, jansson, gmp, and harfbuzz. Remove imagemagick
>> and libxft.
>> [native-inputs]: Add texlive.
>> (emacs-wide-int): Mark as deprecated package.
>> (emacs-no-x):
>> [arguments]: Add --with-jpeg=no --with-gif=no --with-tiff=no
>> to #:configure-flags.
>
> I see that Mark committed a similar patch just yesterday:
>
>   https://git.savannah.gnu.org/cgit/guix.git/commit/?id=36a09d185343375a5cba370431870f9c4435d623
>
> I suppose Mark hadn’t seen the ongoing discussion.

Indeed, I hadn't.  Gah, I'm terribly sorry about this.  I had done some
quick web searches for preexisting work on this, but clearly they were
insufficient, and I've never subscribed to the patches list.  In the
future, I'll know not to rely on web search engines for this.

> Mark, Morgan: could you see if there’s anything we’re missing from the
> patch Morgan submitted?

Looking now, here are the main differences I see between our patches:

* I found that I had to remove the 'restore-emacs-pdump' phase from most
  of the other emacs variants, namely the ones that use
  'gnu-build-system', because otherwise the inherited
  'restore-emacs-pdump' phase would fail.  Morgan's patch seems not to
  consider most of the other emacs variants, and I'm not sure if they
  were tested.  I briefly tested all of them except for 'guile-emacs'.

* Morgan removed the snippet code that deletes "eshell/esh-groups.el",
  whereas I replaced it with a call to 'find-files' to remove it only if
  present, as the previous comment suggested.  I'm not sure if this is
  still needed, though.

* Morgan's patch adds "libxaw" to inputs and "texinfo" to native-inputs.
  What's the rationale for these?

* I added 'pango' to the inputs, because the Emacs NEWS mentioned that
  Pango was used for font rendering when "--with-cairo" is used.
  However, it may be that "pango" finds its way into the build inputs
  without being explicitly mentioned.

* Morgan removed 'libxft' from the inputs of 'emacs', whereas I didn't.
  This was an oversight on my part.  However, if we remove it, it's
  possible that we might need to add it back to 'emacs-no-x-toolkit'.
  The references that I see to Pango in the Emacs code are within
  "#ifdef HAVE_GTK3".

* In 'emacs-no-x', my patch removes the new graphical library inputs
  (cairo, pango, and harfbuzz) and the "--with-cairo" flag, whereas
  Morgan's patch leaves "cairo" and "harfbuzz" as inputs, and overwrites
  the inherited configure-flags to be precisely ("--with-jpeg=no"
  "--with-gif=no" "--with-tiff=no"), apparently discarding the inherited
  "--with-modules" and "--disable-build-details" flags.

* Morgan made 'emacs-wide-int' into a deprecated package, whereas I
  thought that it might still be useful.  My rationale was this: from a
  brief skim, it looks like '--with-wide-int' might make *immediate*
  integers wider, which for some applications might perform much better
  than the heap-allocated arbitrary-size integers supported by Emacs 27.
  However, I didn't look carefully at this.

* I updated "emacs-exec-path.patch" and removed
  "emacs27-exec-path.patch", whereas Morgan's patch keeps both files and
  possibly leaves "emacs-exec-path.patch" orphaned.

* I updated the patches to apply cleanly to Emacs 27, although this was
  not strictly needed.

* I updated 'notmuch' in the previous commit to a version that builds
  successfully with Emacs 27.

Pierre Neidhardt <mail@ambrevar.xyz> wrote:
> I confirm that with Mark's commit
> emacs-clojure-mode and emacs-elisp-refs are also broken.

Sorry about that.  If the Emacs 27 update breaks important packages, it
might be that reverting it is the proper action.  If the maintainers
decide to do this, I would not object.

   Best regards,
       Mark

Re: [PATCH v4] gnu: emacs: Update to 27.1.

[Morgan Smith]

Hello!

It seems I am taking some credit for Jack Hill's patch. I simply took
Jack's patch (labeled as patch v3 in the debbugs thread) and attempted
to build it with my personal config. I noticed that it wouldn't build
properly since emacs wanted libxaw and emacs-no-x wanted some image
librarys (libtiff, libjpeg, etc). The only contributions I made, where
to add libxaw to emacs and to add the configure flags to emacs-no-x. I
made these changes and posted them as patch v4. However, the patch Mark
committed does build for me so you can ignore my contributions (which
weren't well thought out to begin with).

The rest of your questions should be directed to Jack.

(Also just my 2 cents, we should have upgraded emacs-next to 27.1 and
only upgraded emacs a week later. Emacs deserves more care than most
packages due to its dependence on so many other packages. Not trying to
shame anyone, just something to think of for next time.)

Thanks,
Morgan

On 8/28/20 2:57 PM, Mark H Weaver wrote:
> Looking now, here are the main differences I see between our patches:
> 
> * I found that I had to remove the 'restore-emacs-pdump' phase from most
>   of the other emacs variants, namely the ones that use
>   'gnu-build-system', because otherwise the inherited
>   'restore-emacs-pdump' phase would fail.  Morgan's patch seems not to
>   consider most of the other emacs variants, and I'm not sure if they
>   were tested.  I briefly tested all of them except for 'guile-emacs'.
> 
> * Morgan removed the snippet code that deletes "eshell/esh-groups.el",
>   whereas I replaced it with a call to 'find-files' to remove it only if
>   present, as the previous comment suggested.  I'm not sure if this is
>   still needed, though.
> 
> * Morgan's patch adds "libxaw" to inputs and "texinfo" to native-inputs.
>   What's the rationale for these?
> 
> * I added 'pango' to the inputs, because the Emacs NEWS mentioned that
>   Pango was used for font rendering when "--with-cairo" is used.
>   However, it may be that "pango" finds its way into the build inputs
>   without being explicitly mentioned.
> 
> * Morgan removed 'libxft' from the inputs of 'emacs', whereas I didn't.
>   This was an oversight on my part.  However, if we remove it, it's
>   possible that we might need to add it back to 'emacs-no-x-toolkit'.
>   The references that I see to Pango in the Emacs code are within
>   "#ifdef HAVE_GTK3".
> 
> * In 'emacs-no-x', my patch removes the new graphical library inputs
>   (cairo, pango, and harfbuzz) and the "--with-cairo" flag, whereas
>   Morgan's patch leaves "cairo" and "harfbuzz" as inputs, and overwrites
>   the inherited configure-flags to be precisely ("--with-jpeg=no"
>   "--with-gif=no" "--with-tiff=no"), apparently discarding the inherited
>   "--with-modules" and "--disable-build-details" flags.
> 
> * Morgan made 'emacs-wide-int' into a deprecated package, whereas I
>   thought that it might still be useful.  My rationale was this: from a
>   brief skim, it looks like '--with-wide-int' might make *immediate*
>   integers wider, which for some applications might perform much better
>   than the heap-allocated arbitrary-size integers supported by Emacs 27.
>   However, I didn't look carefully at this.
> 
> * I updated "emacs-exec-path.patch" and removed
>   "emacs27-exec-path.patch", whereas Morgan's patch keeps both files and
>   possibly leaves "emacs-exec-path.patch" orphaned.
> 
> * I updated the patches to apply cleanly to Emacs 27, although this was
>   not strictly needed.
> 
> * I updated 'notmuch' in the previous commit to a version that builds
>   successfully with Emacs 27.
> 
> Pierre Neidhardt <mail@ambrevar.xyz> wrote:
>> I confirm that with Mark's commit
>> emacs-clojure-mode and emacs-elisp-refs are also broken.
> 
> Sorry about that.  If the Emacs 27 update breaks important packages, it
> might be that reverting it is the proper action.  If the maintainers
> decide to do this, I would not object.
> 
>    Best regards,
>        Mark
> 

Re: [PATCH v4] gnu: emacs: Update to 27.1.

[Mark H Weaver]

Hi Morgan,

Morgan Smith <Morgan.J.Smith@outlook.com> wrote:
> It seems I am taking some credit for Jack Hill's patch. I simply took
> Jack's patch (labeled as patch v3 in the debbugs thread) and attempted
> to build it with my personal config.

Indeed, I see that now.  Sorry for the mistake.  I suppose it happened
because it was presented to me as "the patch Morgan submitted", but it
would have been better if I had taken the time to read the bug report
more carefully.

Also, I regret if my comments on the preliminary patch made anyone feel
badly.  To be clear, there's no shame in submitting an unfinished patch
for review.  On the contrary, it is a *very* useful contribution.  In
this case, it makes sense to start by updating the main 'emacs' package,
and to worry about the other variants later.  There's no fault in that.

The only fault here lies with me.  I should have looked more carefully
for prior work, and I probably also pushed it prematurely.

> (Also just my 2 cents, we should have upgraded emacs-next to 27.1 and
> only upgraded emacs a week later. Emacs deserves more care than most
> packages due to its dependence on so many other packages. Not trying to
> shame anyone, just something to think of for next time.)

Agreed, or perhaps 'emacs' itself should have been updated on a separate
branch.  In the past, when this was a smaller community and there were
far fewer emacs packages in Guix, I did major updates of Emacs a few
times without causing any obvious problems, but that was a different
time.  It's also possible that Emacs 27 was a more disruptive update
than usual.

Thanks very much for your comments and contributions.

   Best regards,
       Mark

Re: [bug#42738] [PATCH v4] gnu: emacs: Update to 27.1.

[Brett Gilio]

Mark H Weaver <mhw@netris.org> writes:

> Agreed, or perhaps 'emacs' itself should have been updated on a separate
> branch.

+1 for WIP-emacs branch in future.

Brett Gilio

Re: [PATCH v4] gnu: emacs: Update to 27.1.

[Giovanni Biscuolo]

[-- Attachment #1: Type: text/plain, Size: 823 bytes --]

Mark H Weaver <mhw@netris.org> writes:

[...]

> Pierre Neidhardt <mail@ambrevar.xyz> wrote:
>> I confirm that with Mark's commit
>> emacs-clojure-mode

I've filed bug#43090 with a trivial patch for emacs-clojure-mode, no
need to revert Emacs 27.1 for this bug IMHO.

> and emacs-elisp-refs are also broken.

is there a specific bug report for this plz?

there is also bug#43093 for emacs-ess

can we coordinate our efforts in order to avoid to revert emacs 27.1 by
tagging relevant [1] open emacs packages bugs as critical? [2]

[...]


Thanks! Gio'


[1] can we define "relevant" as: all bugs that breaks a previously
compiling emacs package?

[2]
https://issues.guix.gnu.org/search?query=subject%3Aemacs+is%3Aopen+severity%3Acritical

-- 
Giovanni Biscuolo

Xelera IT Infrastructures

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH v4] gnu: emacs: Update to 27.1.

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 143 bytes --]

emacs-helpful still seems to be broken on master.
Any idea?
I'll try to fix it later today.

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: [PATCH v4] gnu: emacs: Update to 27.1.

[Giovanni Biscuolo]

[-- Attachment #1: Type: text/plain, Size: 331 bytes --]

Hi Pierre,

thank you for working on this!

Pierre Neidhardt <mail@ambrevar.xyz> writes:

> Looks like an upstream issue:
>
> https://github.com/Wilfred/helpful/issues/248

This thread is getting hard to follow, should we open a specific bug on
bug-guix?

WDYT?

-- 
Giovanni Biscuolo

Xelera IT Infrastructures

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH v4] gnu: emacs: Update to 27.1.

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 80 bytes --]

Done in 43254@debbugs.gnu.org.

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]