Subject: Re: core-updates: Emacs is only supported on x86_64-linux?

From: Chris Marusich <[email-masked]>
Date: Sun, 07 Mar 2021 18:53:48 -0800

Hi all,

FYI, I'm sending this message to [email-masked] via Bcc, also.
I'm not sure if that works, but we'll see...

Mark H Weaver <[email-masked]> writes:

> It's not intended.  Emacs should certainly be supported on every system
> that Guix supports.

Thank you for confirming.  That is what I expected.

> For now, I suggest that Emacs should have input 'librsvg' only on
> 'x86_64-linux' systems.  Something like this (untested), for
> core-updates:
>
> diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
> index 98061c93ae..de6101cf17 100644
> --- a/gnu/packages/emacs.scm
> +++ b/gnu/packages/emacs.scm
> @@ -71,6 +71,7 @@
>    #:use-module (gnu packages xml)
>    #:use-module (gnu packages xorg)
>    #:use-module (guix utils)
> +  #:use-module (ice-9 match)
>    #:use-module (srfi srfi-1))
>  
>  (define-public emacs
> @@ -236,7 +237,10 @@
>         ("libpng" ,libpng)
>         ("zlib" ,zlib)
>  
> -       ("librsvg" ,librsvg)
> +       ,@(match (or (%current-target-system)
> +                    (%current-system))
> +           ("x86_64-linux" `(("librsvg" ,librsvg)))
> +           (_ `()))
>         ("libxpm" ,libxpm)
>         ("libxml2" ,libxml2)
>         ("libice" ,libice)
>
> Ditto for all other packages with 'librsvg' as an optional dependency.

I've confirmed that works for emacs, except that you actually have to
also do it for gtk+, too, since rust gets pulled in via gtk+ also.  So,
something like this:

From 649c89e5862e1ed887f5fd863ef7bb32f97bbe74 Mon Sep 17 00:00:00 2001
From: Chris Marusich <[email-masked]>
Date: Sun, 7 Mar 2021 17:42:37 -0800
Subject: [PATCH] gnu: emacs: Use librsvg and gtk+ on x86_64-linux only.

* gnu/packages/emacs.scm (emacs)[inputs]: Only add librsvg when the
%current-target-system or %current-system is "x86_64-linux".  This avoids
pulling rust into the transitive closure of inputs on systems where Rust
support is currently lacking.
---
 gnu/packages/emacs.scm | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
index 98061c93ae0..f0797ae2347 100644
--- a/gnu/packages/emacs.scm
+++ b/gnu/packages/emacs.scm
@@ -71,6 +71,7 @@
   #:use-module (gnu packages xml)
   #:use-module (gnu packages xorg)
   #:use-module (guix utils)
+  #:use-module (ice-9 match)
   #:use-module (srfi srfi-1))
 
 (define-public emacs
@@ -219,7 +220,6 @@
 
        ;; TODO: Add the optional dependencies.
        ("libx11" ,libx11)
-       ("gtk+" ,gtk+)
        ("cairo" ,cairo)
        ("pango" ,pango)
        ("harfbuzz" ,harfbuzz)
@@ -236,7 +236,6 @@
        ("libpng" ,libpng)
        ("zlib" ,zlib)
 
-       ("librsvg" ,librsvg)
        ("libxpm" ,libxpm)
        ("libxml2" ,libxml2)
        ("libice" ,libice)
@@ -246,7 +245,15 @@
 
        ;; multilingualization support
        ("libotf" ,libotf)
-       ("m17n-lib" ,m17n-lib)))
+       ("m17n-lib" ,m17n-lib)
+
+       ;; These are optional dependencies that pull in rust.  Rust is not
+       ;; supported well on every architecture yet.
+       ,@(match (or (%current-target-system)
+                    (%current-system))
+           ("x86_64-linux" `(("gtk+" ,gtk+)
+                             ("librsvg" ,librsvg)))
+           (_ '()))))
     (native-inputs
      `(("guix-emacs.el" ,(search-auxiliary-file "emacs/guix-emacs.el"))
        ("pkg-config" ,pkg-config)
-- 
2.26.2

What do you think about that?  If there are no problems, I'll go ahead
and commit it to core-updates.

Note that because that patch re-orders the inputs, it causes emacs to be
rebuilt, which should be fine because it's core-updates.  It seemed
better to group these two inputs in a single match clause, rather than
scattering them in two different but basically identical match clauses
just to keep their original ordering intact.

Christopher Baines <[email-masked]> writes:

> Chris Marusich <[email-masked]> writes:
>
>> I've noticed that the emacs package only supports x86_64-linux, at least
>> on core-updates.  Is that intended?
>
> The relevant commit [1] does mention the intent "Remove "i686-linux"
> from supported systems.", but that does differ from my interpretation of
> the change, which is only support x86_64-linux.
>
> 1: https://git.savannah.gnu.org/cgit/guix.git/commit/?id=0ed631866cc0b7cece2b0a0b50e39b37ae91bb67
>
> I've sent a patch that should add back "support" for other systems:
>
>   https://issues.guix.gnu.org/46987
>
> That's for master at least, I haven't looked in to what the situation is
> on core-updates. It's worth there checking if the build still fails.

That patch doesn't apply on core-updates because that rust version
doesn't seem to exist any more on core-updates.  However, the same
change applied to the right rust on core-updates does the trick, like
this:

From e36c4cab40c5b97ffedc72acc586c0b560e7868e Mon Sep 17 00:00:00 2001
From: Chris Marusich <[email-masked]>
Date: Sun, 7 Mar 2021 15:58:19 -0800
Subject: [PATCH] gnu: rust: Make it "supported" on all systems but i686-linux.

* gnu/packages/rust.scm (rust-1.30)[supported-systems]: Instead of hard-coding
this to just "x86_64-linux", calculate the supported systems by deleting
"i686-linux" from %supported-systems.
---
 gnu/packages/rust.scm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/rust.scm b/gnu/packages/rust.scm
index 70d19e089ab..98c553cb913 100644
--- a/gnu/packages/rust.scm
+++ b/gnu/packages/rust.scm
@@ -448,7 +448,9 @@ jemalloc = \"" jemalloc "/lib/libjemalloc_pic.a" "\"
            (search-path-specification
             (variable "LIBRARY_PATH")
             (files '("lib" "lib64")))))
-    (supported-systems '("x86_64-linux"))
+    (supported-systems
+     (delete "i686-linux"  ; fails to build, see bug #35519
+             %supported-systems))
     (synopsis "Compiler for the Rust progamming language")
     (description "Rust is a systems programming language that provides memory
 safety and thread safety guarantees.")
-- 
2.26.2

Both of these patches fix the issue for me.  After applying either one
in isolation (on the wip-ppc64le branch, which was recently rebased onto
core-updates), the list of supported systems for the emacs package
correctly includes other systems, including powerpc64le-linux, and thus
the tests/guix-package.sh test passes on my powerpc64le-linux system.

I think both of these patches are important and needed.  The patch to
restore supported systems to the rust package is important because we
will want rust to build successfully on many systems.  The patch to only
add gtk+ and librsvg inputs to emacs when the system is x86_64-linux is
important because it would be advantageous to be able to use emacs
without depending on rust on systems like powerpc64le-linux, where Rust
support may not be great yet.

It's also convenient for me, personally, because I don't really want to
deal with Rust right now just to get Emacs working on powerpc64le-linux.
Once I can build a Guix release binary for powerpc64le-linux, then I
think I can start worrying more about Rust.  I have taken the liberty of
applying these patches to the wip-ppc64le branch as-is, since they work
for me.  I can remove them later if we don't like it.

We will undoubtedly run into a similar situation with other packages
going forward, like Mark mentioned.  Debian ran into this very issue
some time ago, and apparently it caused a bit of consternation:

https://lwn.net/Articles/771355/

Apparently, powerpc64le-unknown-linux-gnu is a "Tier 2" Rust platform,
which I guess means it's pretty well supported, but not as well as "Tier
1".  I hope that we can get it all working, since librsvg is depended
upon in some way or another by about 2700 packages (about 17% of the
total Guix package collection).  In the meantime, limiting the blast
radius as needed, like Mark suggested, seems like the right thing to do
until Rust support improves on other architectures.

-- 
Chris