GNU bug report logs - #51845
[PATCH 0/2] Add librsvg-bootstrap

Previous Next

Package: guix-patches;

Reported by: Efraim Flashner <efraim <at> flashner.co.il>

Date: Sun, 14 Nov 2021 14:09:02 UTC

Severity: normal

Tags: patch

Done: Efraim Flashner <efraim <at> flashner.co.il>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 51845 in the body.
You can then email your comments to 51845 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to guix-patches <at> gnu.org:
bug#51845; Package guix-patches. (Sun, 14 Nov 2021 14:09:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Efraim Flashner <efraim <at> flashner.co.il>:
New bug report received and forwarded. Copy sent to guix-patches <at> gnu.org. (Sun, 14 Nov 2021 14:09:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Efraim Flashner <efraim <at> flashner.co.il>
To: guix-patches <at> gnu.org
Cc: Efraim Flashner <efraim <at> flashner.co.il>
Subject: [PATCH 0/2] Add librsvg-bootstrap
Date: Sun, 14 Nov 2021 16:07:47 +0200
librsvg is an input for emacs, gtk+@2 and gtk+@3. With the rust inputs
this leads to (unknown) rust libraries causing the rebuild of over 3000
packages on core-updates-frozen. Rather than hunt them down I tracked
down the packages which would have many rebuilds and added a copy of
librsvg for them to use.

Efraim Flashner (2):
  Add librsvg-bootstrap.
  gnu: Use librsvg-bootstrap.

 gnu/packages/emacs.scm |  2 +-
 gnu/packages/gnome.scm | 23 +++++++++++++++++++++++
 gnu/packages/gtk.scm   |  4 ++--
 3 files changed, 26 insertions(+), 3 deletions(-)


base-commit: 75b5ad6aa3b55b2cbd7f333411cbc9e21ab1e186
-- 
2.33.1





Information forwarded to guix-patches <at> gnu.org:
bug#51845; Package guix-patches. (Sun, 14 Nov 2021 14:16:02 GMT) Full text and rfc822 format available.

Message #8 received at 51845 <at> debbugs.gnu.org (full text, mbox):

From: Efraim Flashner <efraim <at> flashner.co.il>
To: 51845 <at> debbugs.gnu.org
Cc: Efraim Flashner <efraim <at> flashner.co.il>
Subject: [PATCH 1/2] gnu: Add librsvg-bootstrap.
Date: Sun, 14 Nov 2021 16:14:39 +0200
* gnu/packages/gnome.scm (librsvg-bootstrap): New variable.
---
 gnu/packages/gnome.scm | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/gnu/packages/gnome.scm b/gnu/packages/gnome.scm
index 924d1326cc..a0436a4edb 100644
--- a/gnu/packages/gnome.scm
+++ b/gnu/packages/gnome.scm
@@ -3580,6 +3580,29 @@ (define-public librsvg
     (home-page "https://wiki.gnome.org/LibRsvg")
     (license license:lgpl2.1+)))
 
+;; This copy of librsvg uses the bundled rust libraries. It is useful for
+;; packages which have too many dependencies to be rebuilt as frequently
+;; as the rust inputs are updated.
+(define-public librsvg-bootstrap
+  (package
+    (inherit librsvg)
+    (name "librsvg")
+    (version "2.50.7")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append "mirror://gnome/sources/librsvg/"
+                                  (version-major+minor version)  "/"
+                                  "librsvg-" version ".tar.xz"))
+              (sha256
+               (base32
+                "1g3f8byg5w08fx1bka12mmpl59v6a4q2p827w6m2la6mijq63yzz"))))
+    (arguments
+     (substitute-keyword-arguments (package-arguments librsvg)
+       ((#:vendor-dir _ "vendor") "vendor")
+       ((#:cargo-inputs _) '())
+       ((#:cargo-development-inputs _) '())))
+    (properties '((hidden? . #t)))))
+
 (define-public libidl
   (package
     (name "libidl")
-- 
2.33.1





Information forwarded to guix-patches <at> gnu.org:
bug#51845; Package guix-patches. (Sun, 14 Nov 2021 14:16:02 GMT) Full text and rfc822 format available.

Message #11 received at 51845 <at> debbugs.gnu.org (full text, mbox):

From: Efraim Flashner <efraim <at> flashner.co.il>
To: 51845 <at> debbugs.gnu.org
Cc: Efraim Flashner <efraim <at> flashner.co.il>
Subject: [PATCH 2/2] gnu: Use librsvg-bootstrap.
Date: Sun, 14 Nov 2021 16:14:40 +0200
* gnu/packages/emacs.scm (inputs): Use librsvg-bootstrap.
* gnu/pacakges/gtk.scm (gtk+-2, gtk+)[propagated-inputs]: Same.
---
 gnu/packages/emacs.scm | 2 +-
 gnu/packages/gtk.scm   | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
index 734f3dfaa3..3a5215b31a 100644
--- a/gnu/packages/emacs.scm
+++ b/gnu/packages/emacs.scm
@@ -269,7 +269,7 @@ (define* (emacs-byte-compile-directory dir)
        ;; supported well on every architecture yet.
        ,@(if (string-prefix? "x86_64" (or (%current-target-system)
                                           (%current-system)))
-             `(("librsvg" ,librsvg))
+             `(("librsvg" ,librsvg-bootstrap))
              '())
        ("libxpm" ,libxpm)
        ("libxml2" ,libxml2)
diff --git a/gnu/packages/gtk.scm b/gnu/packages/gtk.scm
index 199ca13981..4dfeba4b7f 100644
--- a/gnu/packages/gtk.scm
+++ b/gnu/packages/gtk.scm
@@ -867,7 +867,7 @@ (define-public gtk+-2
        ;; Rust is not supported well on every architecture yet.
        ("gdk-pixbuf" ,(if (string-prefix? "x86_64" (or (%current-target-system)
                                                        (%current-system)))
-                          librsvg
+                          librsvg-bootstrap
                           gdk-pixbuf))
        ("glib" ,glib)
        ("pango" ,pango)))
@@ -969,7 +969,7 @@ (define-public gtk+
        ;; SVG support is optional and requires librsvg, which pulls in rust.
        ;; Rust is not supported well on every architecture yet.
        ("gdk-pixbuf" ,(if (target-x86-64?)
-                          librsvg
+                          librsvg-bootstrap
                           gdk-pixbuf))
        ("glib" ,glib)
        ("libcloudproviders" ,libcloudproviders-minimal)
-- 
2.33.1





Information forwarded to guix-patches <at> gnu.org:
bug#51845; Package guix-patches. (Sun, 14 Nov 2021 17:28:01 GMT) Full text and rfc822 format available.

Message #14 received at 51845 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Efraim Flashner <efraim <at> flashner.co.il>, 51845 <at> debbugs.gnu.org
Subject: Re: [PATCH 0/2] Add librsvg-bootstrap
Date: Sun, 14 Nov 2021 18:27:02 +0100
Hi,

Am Sonntag, den 14.11.2021, 16:07 +0200 schrieb Efraim Flashner:
> librsvg is an input for emacs, gtk+@2 and gtk+@3. With the rust
> inputs this leads to (unknown) rust libraries causing the rebuild of
> over 3000 packages on core-updates-frozen. Rather than hunt them down
> I tracked down the packages which would have many rebuilds and added
> a copy of librsvg for them to use.
In my opinion, one of the selling points of Guix is that of
bootstrappability.  I don't think adding big blobs to Emacs of all
things is a great way of delivering on that promise.  I think we ought
to rather "invest" in alternatives to Rust and Rust-locked libraries or
make Rust packaging itself sane (if it can at all).

I think librsvg is optional already and people who want to save on
compilation time can decide to replace it with e.g. GNU hello using the
--input option.  In the similar case of mozjs, a replacement with
duktape is discussed on guix-devel, at least for polkit.

As a temporary resolution to the rebuild issue, we could pin the
dependencies of librsvg to some specific versions and only bump them
when something awful happens.  I'm not sure whether librsvg exposes any
of the Rust nastiness to its dependencies, ideally hoping that it would
not.

WDYT?





Information forwarded to guix-patches <at> gnu.org:
bug#51845; Package guix-patches. (Sun, 14 Nov 2021 18:08:02 GMT) Full text and rfc822 format available.

Message #17 received at 51845 <at> debbugs.gnu.org (full text, mbox):

From: Efraim Flashner <efraim <at> flashner.co.il>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: 51845 <at> debbugs.gnu.org
Subject: Re: [PATCH 0/2] Add librsvg-bootstrap
Date: Sun, 14 Nov 2021 20:07:01 +0200
[Message part 1 (text/plain, inline)]
On Sun, Nov 14, 2021 at 06:27:02PM +0100, Liliana Marie Prikler wrote:
> Hi,
> 
> Am Sonntag, den 14.11.2021, 16:07 +0200 schrieb Efraim Flashner:
> > librsvg is an input for emacs, gtk+@2 and gtk+@3. With the rust
> > inputs this leads to (unknown) rust libraries causing the rebuild of
> > over 3000 packages on core-updates-frozen. Rather than hunt them down
> > I tracked down the packages which would have many rebuilds and added
> > a copy of librsvg for them to use.
> In my opinion, one of the selling points of Guix is that of
> bootstrappability.  I don't think adding big blobs to Emacs of all
> things is a great way of delivering on that promise.  I think we ought
> to rather "invest" in alternatives to Rust and Rust-locked libraries or
> make Rust packaging itself sane (if it can at all).
> 
> I think librsvg is optional already and people who want to save on
> compilation time can decide to replace it with e.g. GNU hello using the
> --input option.  In the similar case of mozjs, a replacement with
> duktape is discussed on guix-devel, at least for polkit.

It seems I was wrong about emacs; both emacs-minimal and emacs-no-x are
built without librsvg.

> As a temporary resolution to the rebuild issue, we could pin the
> dependencies of librsvg to some specific versions and only bump them
> when something awful happens.  I'm not sure whether librsvg exposes any
> of the Rust nastiness to its dependencies, ideally hoping that it would
> not.

I don't believe librsvg exposes any rust-y stuff.

> WDYT?

(ins)efraim <at> 3900XT /tmp/librsvg-2.50.7$ ls vendor/ | wc -l
226

There are 226 crates that upstream bundles with their source. I suppose
we could pare it down to about 200 by careful pruning but it's part of
librsvg and not going away.

(ins)efraim <at> 3900XT ~/workspace/guix-core-updates$ git grep \,librsvg | wc -l
103

I'm suggesting that for gtk+@2 and gtk+@3 we use the bundled crates and
for the other 101 packages we continue to use our current version, where
we replace all of the bundled crates with our own copies, which get
updated more often than librsvg does.

With our current rust tooling I don't think it'd be that easy to find
the ~226 crates that librsvg depends on, and it wouldn't be great to
lock them due to librsvg being an input for gtk2/3.

-- 
Efraim Flashner   <efraim <at> flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted
[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#51845; Package guix-patches. (Sun, 14 Nov 2021 19:06:01 GMT) Full text and rfc822 format available.

Message #20 received at 51845 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Efraim Flashner <efraim <at> flashner.co.il>
Cc: 51845 <at> debbugs.gnu.org
Subject: Re: [PATCH 0/2] Add librsvg-bootstrap
Date: Sun, 14 Nov 2021 20:05:30 +0100
Hi,

Am Sonntag, den 14.11.2021, 20:07 +0200 schrieb Efraim Flashner:
> > As a temporary resolution to the rebuild issue, we could pin the
> > dependencies of librsvg to some specific versions and only bump 
> > them something awful happens.  I'm not sure whether librsvg
> > exposes any of the Rust nastiness to its dependencies, ideally
> > hoping that it would not.
> 
> I don't believe librsvg exposes any rust-y stuff.
That sounds like a good start for once.

> > WDYT?
> 
> (ins)efraim <at> 3900XT /tmp/librsvg-2.50.7$ ls vendor/ | wc -l
> 226
> 
> There are 226 crates that upstream bundles with their source. I
> suppose we could pare it down to about 200 by careful pruning but
> it's part of librsvg and not going away.
Well, I'd suggest snippeting them away, but that's a different topic.

> (ins)efraim <at> 3900XT ~/workspace/guix-core-updates$ git grep \,librsvg
> | wc -l
> 103
I'd hazard a guess that most if not all of these 103 packages are
themselves gtk-adjacent, so what really is the issue we're solving
here?  What is the point of maintaining an extra version for 101 of
them when a potentially vulnerable GTK sits right next to them?

> I'm suggesting that for gtk+@2 and gtk+@3 we use the bundled crates
> and for the other 101 packages we continue to use our current
> version, where we replace all of the bundled crates with our own
> copies, which get updated more often than librsvg does.
> 
> With our current rust tooling I don't think it'd be that easy to find
> the ~226 crates that librsvg depends on, and it wouldn't be great to
> lock them due to librsvg being an input for gtk2/3.
Said input exists due to gdk-pixbuf+svg, with the +svg part being
largely optional – the most common failure mode of it not being
included are broken button textures, which we could fix by pre-
rendering images with a suitable tool, such as inkscape.  We could
easily do a minimal gtk[+]? without it.

As for the lock, why can't we?  gtk+ is already core-updates material,
so it stands to reason that anything causing it to rebuild is too. 
Rather than push down blobs to the users because we can't deal with
Rust, we should fix Rust or make it go away from the build.

WDYT?





Information forwarded to guix-patches <at> gnu.org:
bug#51845; Package guix-patches. (Mon, 06 Dec 2021 12:18:01 GMT) Full text and rfc822 format available.

Message #23 received at 51845 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Efraim Flashner <efraim <at> flashner.co.il>
Cc: 51845 <at> debbugs.gnu.org
Subject: Re: bug#51845: [PATCH 0/2] Add librsvg-bootstrap
Date: Mon, 06 Dec 2021 13:17:47 +0100
Hi Efraim,

I had completely overlooked these patches, oops!

Efraim Flashner <efraim <at> flashner.co.il> skribis:

> librsvg is an input for emacs, gtk+@2 and gtk+@3. With the rust inputs
> this leads to (unknown) rust libraries causing the rebuild of over 3000
> packages on core-updates-frozen. Rather than hunt them down I tracked
> down the packages which would have many rebuilds and added a copy of
> librsvg for them to use.

[...]

> I'm suggesting that for gtk+@2 and gtk+@3 we use the bundled crates and
> for the other 101 packages we continue to use our current version, where
> we replace all of the bundled crates with our own copies, which get
> updated more often than librsvg does.
>
> With our current rust tooling I don't think it'd be that easy to find
> the ~226 crates that librsvg depends on, and it wouldn't be great to
> lock them due to librsvg being an input for gtk2/3.

Yes, that’s a problem, though Liliana is right that bundling isn’t great
either.

I’m annoyed by this whole librsvg situation.  On non-x86_64, we now
depend on librsvg 2.40, the old C version, and guess what, it just
works.  That has me tempted to stick with 2.40 all along because these
Rust problems don’t seem to have a pleasant, or even an easy solution.

Now, using the proposed ‘librsvg-bootstrap’ in GTK+ looks like a lesser
evil.

Thoughts?

Ludo’.




Information forwarded to guix-patches <at> gnu.org:
bug#51845; Package guix-patches. (Mon, 06 Dec 2021 13:08:02 GMT) Full text and rfc822 format available.

Message #26 received at 51845 <at> debbugs.gnu.org (full text, mbox):

From: Efraim Flashner <efraim <at> flashner.co.il>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 51845 <at> debbugs.gnu.org
Subject: Re: bug#51845: [PATCH 0/2] Add librsvg-bootstrap
Date: Mon, 6 Dec 2021 15:06:18 +0200
[Message part 1 (text/plain, inline)]
On Mon, Dec 06, 2021 at 01:17:47PM +0100, Ludovic Courtès wrote:
> Hi Efraim,
> 
> I had completely overlooked these patches, oops!
> 
> Efraim Flashner <efraim <at> flashner.co.il> skribis:
> 
> > librsvg is an input for emacs, gtk+@2 and gtk+@3. With the rust inputs
> > this leads to (unknown) rust libraries causing the rebuild of over 3000
> > packages on core-updates-frozen. Rather than hunt them down I tracked
> > down the packages which would have many rebuilds and added a copy of
> > librsvg for them to use.
> 
> [...]
> 
> > I'm suggesting that for gtk+@2 and gtk+@3 we use the bundled crates and
> > for the other 101 packages we continue to use our current version, where
> > we replace all of the bundled crates with our own copies, which get
> > updated more often than librsvg does.
> >
> > With our current rust tooling I don't think it'd be that easy to find
> > the ~226 crates that librsvg depends on, and it wouldn't be great to
> > lock them due to librsvg being an input for gtk2/3.
> 
> Yes, that’s a problem, though Liliana is right that bundling isn’t great
> either.
> 
> I’m annoyed by this whole librsvg situation.  On non-x86_64, we now
> depend on librsvg 2.40, the old C version, and guess what, it just
> works.  That has me tempted to stick with 2.40 all along because these
> Rust problems don’t seem to have a pleasant, or even an easy solution.
> 
> Now, using the proposed ‘librsvg-bootstrap’ in GTK+ looks like a lesser
> evil.
> 
> Thoughts?

Unbundling the rust crates is the right option, but not the easy option.
With the assumption that rust-libc-0.2 is in the graph for librsvg, we
add another copy named rust-libc-0.2.101 (the current version) and a
comment that it only gets adjusted on core-updates or that it causes
XXXX package rebuilds.

On a small tangent, the work I do sometimes to try to actually have a
dependency graph with the crates would only make these easier to find,
not actually address the issue here.

I'm not sure if it'd be better to mostly copy the packages with a new
name and keep the cargo-inputs or to actually adjust the
cargo-inputs->inputs and cargo-development-inputs->native-inputs  so we
get the dependency graph from rust-libc-0.2.101 to librsvg. I'd like to
make the change but if we don't get the others changed then we
effectively really have two sets of rust crates.

If we have both cargo-inputs and inputs then the cargo-build-system
doesn't have issues with using either type with later packages, so that
might be the best option for now.

-- 
Efraim Flashner   <efraim <at> flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted
[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#51845; Package guix-patches. (Mon, 06 Dec 2021 16:38:02 GMT) Full text and rfc822 format available.

Message #29 received at 51845 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Efraim Flashner <efraim <at> flashner.co.il>
Cc: 51845 <at> debbugs.gnu.org
Subject: Re: bug#51845: [PATCH 0/2] Add librsvg-bootstrap
Date: Mon, 06 Dec 2021 17:37:12 +0100
Hi Efraim,

Efraim Flashner <efraim <at> flashner.co.il> skribis:

> On a small tangent, the work I do sometimes to try to actually have a
> dependency graph with the crates would only make these easier to find,
> not actually address the issue here.
>
> I'm not sure if it'd be better to mostly copy the packages with a new
> name and keep the cargo-inputs or to actually adjust the
> cargo-inputs->inputs and cargo-development-inputs->native-inputs  so we
> get the dependency graph from rust-libc-0.2.101 to librsvg. I'd like to
> make the change but if we don't get the others changed then we
> effectively really have two sets of rust crates.
>
> If we have both cargo-inputs and inputs then the cargo-build-system
> doesn't have issues with using either type with later packages, so that
> might be the best option for now.

Thinking out loud… would it work to change:

  (arguments '(#:cargo-inputs X #:cargo-development-inputs Y))

to:

  (native-inputs (map package-source Y))
  (inputs (map package-source X))

?

Or am I just saying nonsense?

Thanks,
Ludo’.




Information forwarded to guix-patches <at> gnu.org:
bug#51845; Package guix-patches. (Mon, 06 Dec 2021 17:04:01 GMT) Full text and rfc822 format available.

Message #32 received at 51845 <at> debbugs.gnu.org (full text, mbox):

From: Efraim Flashner <efraim <at> flashner.co.il>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 51845 <at> debbugs.gnu.org
Subject: Re: bug#51845: [PATCH 0/2] Add librsvg-bootstrap
Date: Mon, 06 Dec 2021 17:02:45 +0000

On December 6, 2021 4:37:12 PM UTC, "Ludovic Courtès" <ludo <at> gnu.org> wrote:
>Hi Efraim,
>
>Efraim Flashner <efraim <at> flashner.co.il> skribis:
>
>> On a small tangent, the work I do sometimes to try to actually have a
>> dependency graph with the crates would only make these easier to find,
>> not actually address the issue here.
>>
>> I'm not sure if it'd be better to mostly copy the packages with a new
>> name and keep the cargo-inputs or to actually adjust the
>> cargo-inputs->inputs and cargo-development-inputs->native-inputs  so we
>> get the dependency graph from rust-libc-0.2.101 to librsvg. I'd like to
>> make the change but if we don't get the others changed then we
>> effectively really have two sets of rust crates.
>>
>> If we have both cargo-inputs and inputs then the cargo-build-system
>> doesn't have issues with using either type with later packages, so that
>> might be the best option for now.
>
>Thinking out loud… would it work to change:
>
>  (arguments '(#:cargo-inputs X #:cargo-development-inputs Y))
>
>to:
>
>  (native-inputs (map package-source Y))
>  (inputs (map package-source X))
>
>?
>
>Or am I just saying nonsense?
>
>Thanks,
>Ludo’.

Then we lose the transitive package sources, which is how we ended up where we are today.

I can go and change the cargo-build-system to use the skip-build flag in more phases to skip them when we aren't going to be building them anyway. No need to generate cargo checksums if we're not building I think.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.




Information forwarded to guix-patches <at> gnu.org:
bug#51845; Package guix-patches. (Mon, 06 Dec 2021 22:18:02 GMT) Full text and rfc822 format available.

Message #35 received at 51845 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Efraim Flashner <efraim <at> flashner.co.il>
Cc: 51845 <at> debbugs.gnu.org
Subject: Using ‘native-inputs’ and
 ‘inputs’ for Cargo packages?
Date: Mon, 06 Dec 2021 23:17:47 +0100
[Message part 1 (text/plain, inline)]
Efraim Flashner <efraim <at> flashner.co.il> skribis:

> On December 6, 2021 4:37:12 PM UTC, "Ludovic Courtès" <ludo <at> gnu.org> wrote:

[...]

>>Thinking out loud… would it work to change:
>>
>>  (arguments '(#:cargo-inputs X #:cargo-development-inputs Y))
>>
>>to:
>>
>>  (native-inputs (map package-source Y))
>>  (inputs (map package-source X))
>>
>>?

[...]

> Then we lose the transitive package sources, which is how we ended up where we are today.

True.

With the minimal changes to (guix build-system cargo) below, one can use
either the current style or pass “development inputs” as ‘native-inputs’
and other dependencies as ‘inputs’.  Source transitivity is preserved
but you can write packages the normal way.

I modified some of the dependencies of librsvg to use
native-inputs/inputs and you can see when applying this part of the
patch that the librsvg derivation is unchanged.  Good thing is that
‘guix graph’ and ‘guix refresh -l’ work for these packages.

Is this a direction we want to take?

If so, we can have ‘guix style’ automate transformations.

Thanks,
Ludo’.

[Message part 2 (text/x-patch, inline)]
diff --git a/gnu/packages/crates-io.scm b/gnu/packages/crates-io.scm
index b8c4c7bd39..d7214e2d4f 100644
--- a/gnu/packages/crates-io.scm
+++ b/gnu/packages/crates-io.scm
@@ -5728,18 +5728,26 @@ (define-public rust-bitflags-1.3
         (file-name (string-append name "-" version ".tar.gz"))
         (sha256
          (base32 "12ki6w8gn1ldq7yz9y680llwk5gmrhrzszaa17g1sbrw2r2qvwxy"))))
-    (arguments
-     `(#:tests? #f      ; Tests require rust-1.46 or newer.
-       #:cargo-inputs
-       (("rust-compiler-builtins" ,rust-compiler-builtins-0.1)
-        ("rust-rustc-std-workspace-core" ,rust-rustc-std-workspace-core-1))
-       #:cargo-development-inputs
-       (("rust-rustversion" ,rust-rustversion-1)
-        ("rust-serde" ,rust-serde-1)
-        ("rust-serde-derive" ,rust-serde-derive-1)
-        ("rust-serde-json" ,rust-serde-json-1)
-        ("rust-trybuild" ,rust-trybuild-1)
-        ("rust-walkdir" ,rust-walkdir-2))))))
+    (arguments `(#:tests? #f ; Tests require rust-1.46 or newer.
+                         ))
+    (native-inputs `(("rust-rustversion" ,rust-rustversion-1) (
+                                                               "rust-serde" ,
+                                                                       rust-serde-1)
+                                                               (
+                                                               "rust-serde-derive" ,
+                                                                                  rust-serde-derive-1)
+                                                               (
+                                                               "rust-serde-json" ,
+                                                                                rust-serde-json-1)
+                                                               (
+                                                               "rust-trybuild" ,
+                                                                              rust-trybuild-1)
+                                                               (
+                                                               "rust-walkdir" ,
+                                                                             rust-walkdir-2)))
+    (inputs `(("rust-compiler-builtins" ,rust-compiler-builtins-0.1) (
+                                                                      "rust-rustc-std-workspace-core" ,
+                                                                                                 rust-rustc-std-workspace-core-1)))))
 
 (define-public rust-bitflags-0.9
   (package
@@ -8391,12 +8399,9 @@ (define-public rust-cast-0.2
         (base32
          "1c5z7zryj0zwnhdgs6rw5dfvnlwc1vm19jzrlgx5055alnwk952b"))))
     (build-system cargo-build-system)
-    (arguments
-     `(#:skip-build? #t
-       #:cargo-inputs
-       (("rust-rustc-version" ,rust-rustc-version-0.2))
-       #:cargo-development-inputs
-       (("rust-quickcheck" ,rust-quickcheck-0.9))))
+    (arguments `(#:skip-build? #t))
+    (native-inputs `(("rust-quickcheck" ,rust-quickcheck-0.9)))
+    (inputs `(("rust-rustc-version" ,rust-rustc-version-0.2)))
     (home-page "https://github.com/japaric/cast.rs")
     (synopsis
      "Ergonomic, checked cast functions for primitive types")
@@ -12459,19 +12464,34 @@ (define-public rust-cssparser-0.28
        (sha256
         (base32 "1h924c5g2rwlmgk8hllciyky3ih3z9vf04xz3xsp3cv1jyd5kf0x"))))
     (build-system cargo-build-system)
-    (arguments
-     `(#:skip-build? #t
-       #:cargo-inputs
-       (("rust-cssparser-macros" ,rust-cssparser-macros-0.6)
-        ("rust-dtoa-short" ,rust-dtoa-short-0.3)
-        ("rust-itoa" ,rust-itoa-0.4)
-        ("rust-matches" ,rust-matches-0.1)
-        ("rust-phf" ,rust-phf-0.8)
-        ("rust-proc-macro2" ,rust-proc-macro2-1)
-        ("rust-quote" ,rust-quote-1)
-        ("rust-serde" ,rust-serde-1)
-        ("rust-smallvec" ,rust-smallvec-1)
-        ("rust-syn" ,rust-syn-1))))
+    (arguments `(#:skip-build? #t))
+    (inputs `(("rust-cssparser-macros" ,rust-cssparser-macros-0.6) (
+                                                                    "rust-dtoa-short" ,
+                                                                                 rust-dtoa-short-0.3)
+                                                                    (
+                                                                    "rust-itoa" ,
+                                                                               rust-itoa-0.4)
+                                                                    (
+                                                                    "rust-matches" ,
+                                                                                  rust-matches-0.1)
+                                                                    (
+                                                                    "rust-phf" ,
+                                                                              rust-phf-0.8)
+                                                                    (
+                                                                    "rust-proc-macro2" ,
+                                                                                      rust-proc-macro2-1)
+                                                                    (
+                                                                    "rust-quote" ,
+                                                                                rust-quote-1)
+                                                                    (
+                                                                    "rust-serde" ,
+                                                                                rust-serde-1)
+                                                                    (
+                                                                    "rust-smallvec" ,
+                                                                                   rust-smallvec-1)
+                                                                    (
+                                                                    "rust-syn" ,
+                                                                              rust-syn-1)))
     (home-page "https://github.com/servo/rust-cssparser")
     (synopsis "Rust implementation of CSS Syntax Level 3")
     (description
@@ -13601,13 +13621,13 @@ (define-public rust-data-url-0.1
          (base32
           "176wa1n8h71iwyaxhar4sqwrgrvb5sxk26az0fy88vnxrsffjgyk"))))
     (build-system cargo-build-system)
-    (arguments
-     `(#:cargo-inputs
-       (("rust-matches" ,rust-matches-0.1))
-       #:cargo-development-inputs
-       (("rust-rustc-test" ,rust-rustc-test-0.3)
-        ("rust-serde" ,rust-serde-1)
-        ("rust-serde-json" ,rust-serde-json-1))))
+    (native-inputs `(("rust-rustc-test" ,rust-rustc-test-0.3) (
+                                                               "rust-serde" ,
+                                                                       rust-serde-1)
+                                                               (
+                                                               "rust-serde-json" ,
+                                                                                rust-serde-json-1)))
+    (inputs `(("rust-matches" ,rust-matches-0.1)))
     (home-page "https://github.com/servo/rust-url")
     (synopsis "Processing of data: URL according to WHATWG's Fetch Standard")
     (description
@@ -16336,21 +16356,21 @@ (define-public rust-encoding-0.2
         (base32
          "1v1ndmkarh9z3n5hk53da4z56hgk9wa5kcsm7cnx345raqw983bb"))))
     (build-system cargo-build-system)
-    (arguments
-     `(#:skip-build? #t
-       #:cargo-inputs
-       (("rust-encoding-index-japanese"
-         ,rust-encoding-index-japanese-1.20141219)
-        ("rust-encoding-index-korean"
-         ,rust-encoding-index-korean-1.20141219)
-        ("rust-encoding-index-simpchinese"
-         ,rust-encoding-index-simpchinese-1.20141219)
-        ("rust-encoding-index-singlebyte"
-         ,rust-encoding-index-singlebyte-1.20141219)
-        ("rust-encoding-index-tradchinese"
-         ,rust-encoding-index-tradchinese-1.20141219))
-       #:cargo-development-inputs
-       (("rust-getopts" ,rust-getopts-0.2))))
+    (arguments `(#:skip-build? #t))
+    (native-inputs `(("rust-getopts" ,rust-getopts-0.2)))
+    (inputs `(("rust-encoding-index-japanese" ,
+                                            rust-encoding-index-japanese-1.20141219) (
+                                                                                 "rust-encoding-index-korean" ,
+                                                                                                         rust-encoding-index-korean-1.20141219)
+                                                                                 (
+                                                                                 "rust-encoding-index-simpchinese" ,
+                                                                                                                  rust-encoding-index-simpchinese-1.20141219)
+                                                                                 (
+                                                                                 "rust-encoding-index-singlebyte" ,
+                                                                                                                 rust-encoding-index-singlebyte-1.20141219)
+                                                                                 (
+                                                                                 "rust-encoding-index-tradchinese" ,
+                                                                                                                  rust-encoding-index-tradchinese-1.20141219)))
     (home-page
      "https://github.com/lifthrasiir/rust-encoding")
     (synopsis "Character encoding support for Rust")
@@ -18388,8 +18408,7 @@ (define-public rust-float-cmp-0.8
          (base32
           "1i56hnzjn5pmrcm47fwkmfxiihk7wz5vvcgpb0kpfhzkqi57y9p1"))))
     (build-system cargo-build-system)
-    (arguments
-     `(#:cargo-inputs (("rust-num-traits" ,rust-num-traits-0.2))))
+    (inputs `(("rust-num-traits" ,rust-num-traits-0.2)))
     (home-page "https://github.com/mikedilger/float-cmp")
     (synopsis "Floating point approximate comparison traits")
     (description
@@ -26818,11 +26837,8 @@ (define-public rust-libm-0.2
         (base32
          "0akh56sh51adhagmk9l84dyrlz60gv8ri05xhr13i1b18czkpmy7"))))
     (build-system cargo-build-system)
-    (arguments
-     `(#:cargo-inputs
-       (("rust-rand" ,rust-rand-0.6))
-       #:cargo-development-inputs
-       (("rust-no-panic" ,rust-no-panic-0.1))))
+    (native-inputs `(("rust-no-panic" ,rust-no-panic-0.1)))
+    (inputs `(("rust-rand" ,rust-rand-0.6)))
     (home-page "https://github.com/rust-lang/libm")
     (synopsis "Libm in pure Rust")
     (description "This package provides an implementation of libm in pure Rust.")
diff --git a/guix/build-system/cargo.scm b/guix/build-system/cargo.scm
index 60c35eed07..b2d97beb2f 100644
--- a/guix/build-system/cargo.scm
+++ b/guix/build-system/cargo.scm
@@ -125,17 +125,21 @@ (define builder
                     #:target #f
                     #:guile-for-build guile))
 
+(define (cargo-input? input)
+  (match input
+    ((label (? package? p))
+     (eq? cargo-build-system (package-build-system p)))
+    (_ #f)))
+
 (define (package-cargo-inputs p)
-  (apply
-    (lambda* (#:key (cargo-inputs '()) #:allow-other-keys)
-      cargo-inputs)
-    (package-arguments p)))
+  (match (member #:cargo-inputs (package-arguments p))
+    (#f (filter cargo-input? (package-inputs p)))
+    ((_ inputs . _) inputs)))
 
 (define (package-cargo-development-inputs p)
-  (apply
-    (lambda* (#:key (cargo-development-inputs '()) #:allow-other-keys)
-      cargo-development-inputs)
-    (package-arguments p)))
+  (match (member #:cargo-development-inputs (package-arguments p))
+    (#f (filter cargo-input? (package-native-inputs p)))
+    ((_ inputs . _) inputs)))
 
 (define (crate-closure inputs)
   "Return the closure of INPUTS when considering the 'cargo-inputs' and
@@ -235,8 +239,8 @@ (define (expand-crate-sources cargo-inputs cargo-development-inputs)
 (define* (lower name
                 #:key source inputs native-inputs outputs system target
                 (rust (default-rust))
-                (cargo-inputs '())
-                (cargo-development-inputs '())
+                (cargo-inputs (filter cargo-input? inputs))
+                (cargo-development-inputs (filter cargo-input? native-inputs))
                 #:allow-other-keys
                 #:rest arguments)
   "Return a bag for NAME."
@@ -260,7 +264,9 @@ (define private-keywords
          (build-inputs `(("cargo" ,rust "cargo")
                          ("rustc" ,rust)
                          ,@(expand-crate-sources cargo-inputs cargo-development-inputs)
-                         ,@native-inputs))
+                         ,@(if (eq? native-inputs cargo-development-inputs)
+                               '()
+                               native-inputs)))
          (outputs outputs)
          (build cargo-build)
          (arguments (strip-keyword-arguments private-keywords arguments)))))
diff --git a/guix/packages.scm b/guix/packages.scm
index b3c5a00011..275cc3675c 100644
--- a/guix/packages.scm
+++ b/guix/packages.scm
@@ -660,7 +660,8 @@ (define (deprecated-package old-name p)
     (name old-name)
     (properties `((superseded . ,p)))))
 
-(define (package-field-location package field)
+(define* (package-field-location package field
+                                 #:key (value-location? #t))
   "Return the source code location of the definition of FIELD for PACKAGE, or
 #f if it could not be determined."
   (match (package-location package)
@@ -678,7 +679,10 @@ (define (package-field-location package field)
                    (let ((field (assoc field inits)))
                      (match field
                        ((_ value)
-                        (let ((loc (and=> (source-properties value)
+                        (let ((loc (and=> (source-properties
+                                           (if value-location?
+                                               value
+                                               field))
                                           source-properties->location)))
                           (and loc
                                ;; Preserve the original file name, which may be a
diff --git a/guix/scripts/style.scm b/guix/scripts/style.scm
index 86a46f693c..dccc20d880 100644
--- a/guix/scripts/style.scm
+++ b/guix/scripts/style.scm
@@ -29,6 +29,7 @@
 
 (define-module (guix scripts style)
   #:autoload   (gnu packages) (specification->package fold-packages)
+  #:autoload   (guix build-system cargo) (cargo-build-system)
   #:use-module (guix scripts)
   #:use-module ((guix scripts build) #:select (%standard-build-options))
   #:use-module (guix combinators)
@@ -212,6 +213,21 @@ (define (object->string* obj indent)
       (pretty-print-with-comments port obj
                                   #:indent indent))))
 
+(define (object-list->string lst indent)
+  (call-with-output-string
+    (lambda (port)
+      (let loop ((lst lst))
+        (match lst
+          ((obj)
+           (pretty-print-with-comments port obj
+                                       #:indent indent))
+          ((obj rest ...)
+           (pretty-print-with-comments port obj
+                                       #:indent indent)
+           (newline port)
+           (display (make-string indent #\space) port)
+           (loop rest)))))))
+
 
 ;;;
 ;;; Simplifying input expressions.
@@ -441,6 +457,49 @@ (define matches?
             (list package-inputs package-native-inputs
                   package-propagated-inputs)))
 
+
+;;;
+;;; Crates, Cargo, Rust, and all that.
+;;;
+
+(define* (rewrite-cargo-inputs package
+                               #:key (policy 'silent)
+                               (edit-expression edit-expression))
+  (when (eq? (package-build-system package) cargo-build-system)
+    (match (package-field-location package 'arguments
+                                   #:value-location? #f)
+      (#f #f)
+      (location
+       (let* ((indent (location-column location)))
+         (edit-expression
+          (pk 'loc (location->source-properties location))
+          (lambda (str)
+            (define arguments
+              (call-with-input-string (pk 'str str) read-with-comments))
+
+            (match arguments
+              (('arguments ('quasiquote lst))
+               (let ((inputs (match (member #:cargo-inputs lst)
+                               (#f '())
+                               ((_ inputs . _) inputs)))
+                     (native (match (member #:cargo-development-inputs lst)
+                               (#f '())
+                               ((_ inputs . _) inputs)))
+                     (rest (strip-keyword-arguments
+                            '(#:cargo-inputs #:cargo-development-inputs)
+                            lst)))
+                 (object-list->string
+                  `(,@(if (null? rest)
+                          '()
+                          `((arguments ,(list 'quasiquote rest))))
+                    ,@(if (null? native)
+                          '()
+                          `((native-inputs ,(list 'quasiquote native))))
+                    ,@(if (null? inputs)
+                          '()
+                          `((inputs ,(list 'quasiquote inputs)))))
+                  indent)))))))))))
+
 (define (package-location<? p1 p2)
   "Return true if P1's location is \"before\" P2's."
   (let ((loc1 (package-location p1))
@@ -536,7 +595,7 @@ (define (parse-options)
                        edit-expression))
          (policy   (assoc-ref opts 'input-simplification-policy)))
     (for-each (lambda (package)
-                (simplify-package-inputs package #:policy policy
+                (rewrite-cargo-inputs package #:policy policy
                                          #:edit-expression edit))
               ;; Sort package by source code location so that we start editing
               ;; files from the bottom and going upward.  That way, the

Information forwarded to guix-patches <at> gnu.org:
bug#51845; Package guix-patches. (Tue, 07 Dec 2021 10:15:02 GMT) Full text and rfc822 format available.

Message #38 received at 51845 <at> debbugs.gnu.org (full text, mbox):

From: Efraim Flashner <efraim <at> flashner.co.il>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 51845 <at> debbugs.gnu.org
Subject: Re: Using ‘native-inputs’ and ‘inputs’ for Cargo packages?
Date: Tue, 7 Dec 2021 12:11:04 +0200
[Message part 1 (text/plain, inline)]
On Mon, Dec 06, 2021 at 11:17:47PM +0100, Ludovic Courtès wrote:
> Efraim Flashner <efraim <at> flashner.co.il> skribis:
> 
> > On December 6, 2021 4:37:12 PM UTC, "Ludovic Courtès" <ludo <at> gnu.org> wrote:
> 
> [...]
> 
> >>Thinking out loud… would it work to change:
> >>
> >>  (arguments '(#:cargo-inputs X #:cargo-development-inputs Y))
> >>
> >>to:
> >>
> >>  (native-inputs (map package-source Y))
> >>  (inputs (map package-source X))
> >>
> >>?
> 
> [...]
> 
> > Then we lose the transitive package sources, which is how we ended up where we are today.
> 
> True.
> 
> With the minimal changes to (guix build-system cargo) below, one can use
> either the current style or pass “development inputs” as ‘native-inputs’
> and other dependencies as ‘inputs’.  Source transitivity is preserved
> but you can write packages the normal way.
> 
> I modified some of the dependencies of librsvg to use
> native-inputs/inputs and you can see when applying this part of the
> patch that the librsvg derivation is unchanged.  Good thing is that
> ‘guix graph’ and ‘guix refresh -l’ work for these packages.
> 
> Is this a direction we want to take?

I like the way it works out, and has Guix do the magic to give us the
crates in the graph. On the other hand I tried changing the cargo-inputs
from librsvg to regular inputs, and after 2.5 minutes of trying to run
`guix show librsvg` I still wasn't seeing the dependencies and my RAM
usage was still increasing. Also gnu/packages/gnome.scm didn't fail to
compile, so there was no notice of the loop.

I changed some more packages which are transitive inputs of
rust-encoding <at> 0.2 and didn't see any slowdown. I was worried that this
would affect a future use of `guix shell -D rust-app` not pulling in any
of the crates but it still seems to work. I tried with rust-encoding <at> 0.2
and got the crates for the packages I expected (only the ones I changed).

(ins)efraim <at> 3900XT ~/workspace/guix-core-updates$ ./pre-inst-env guix shell -D rust-encoding <at> 0.2
(ins)efraim <at> 3900XT ~/workspace/guix-core-updates [env]$ ls $GUIX_ENVIRONMENT/share/cargo/registry/ | col
cc-1.0.66.crate
compiler_builtins-0.1.26.crate
encoding-index-japanese-1.20141219.5.crate
encoding-index-korean-1.20141219.5.crate
encoding-index-simpchinese-1.20141219.5.crate
encoding-index-singlebyte-1.20141219.5.crate
encoding_index_tests-0.1.4.crate
encoding-index-tradchinese-1.20141219.5.crate
getopts-0.2.21.crate
log-0.3.9.crate
rustc-std-workspace-core-1.0.0.crate
rustc-std-workspace-std-1.0.1.crate
unicode-width-0.1.9.crate

So to summarize, between your diff to treat inputs built using
cargo-build-system as cargo-inputs and my changes to save previous
crates for the next input we reach a place where we can start to change
the crates over to use inputs and native-inputs instead of cargo-inputs
and cargo-development-inputs without needing to flip everything at once.

So I'd go with it's good, but I'm not sure it directly works to fix the
problem we're having with librsvg.

> If so, we can have ‘guix style’ automate transformations.
> 
> Thanks,
> Ludo’.
> 

I've added some inline comments in the diff (and removed a bunch of
lines)

> diff --git a/guix/build-system/cargo.scm b/guix/build-system/cargo.scm
> index 60c35eed07..b2d97beb2f 100644
> --- a/guix/build-system/cargo.scm
> +++ b/guix/build-system/cargo.scm
> @@ -125,17 +125,21 @@ (define builder
>                      #:target #f
>                      #:guile-for-build guile))
>  
> +(define (cargo-input? input)
> +  (match input
> +    ((label (? package? p))
> +     (eq? cargo-build-system (package-build-system p)))
> +    (_ #f)))
> +

I would've sorted based on the name starting with 'rust-'.

I can't think of an example quickly where it would happen, but take
librsvg, which we currently build with cargo-build-system. If we need it
as an input for pango (or some other library) and we also build that
with the cargo-build-system then we'll just get the rust inputs, not the
actual library.

Can we check the arguments field for `#:install-source? #f` and use it
as a regular {propagated-,native-,}input in that case? Then we could
have cbindgen and rust-cbindgen, depending on if we needed the binary or
the sources.

>  (define (package-cargo-inputs p)
> -  (apply
> -    (lambda* (#:key (cargo-inputs '()) #:allow-other-keys)
> -      cargo-inputs)
> -    (package-arguments p)))
> +  (match (member #:cargo-inputs (package-arguments p))
> +    (#f (filter cargo-input? (package-inputs p)))
> +    ((_ inputs . _) inputs)))
>  
>  (define (package-cargo-development-inputs p)
> -  (apply
> -    (lambda* (#:key (cargo-development-inputs '()) #:allow-other-keys)
> -      cargo-development-inputs)
> -    (package-arguments p)))
> +  (match (member #:cargo-development-inputs (package-arguments p))
> +    (#f (filter cargo-input? (package-native-inputs p)))
> +    ((_ inputs . _) inputs)))

I see we don't get rid of #:cargo-inputs or #:cargo-development-inputs.
So even if applying the style change to all the crates causes circular
dependency problems we can fall back to the current method. I ran into
problems once I hit all the rust-bindgen crates.

>  
>  (define (crate-closure inputs)
>    "Return the closure of INPUTS when considering the 'cargo-inputs' and
> @@ -235,8 +239,8 @@ (define (expand-crate-sources cargo-inputs cargo-development-inputs)
>  (define* (lower name
>                  #:key source inputs native-inputs outputs system target
>                  (rust (default-rust))
> -                (cargo-inputs '())
> -                (cargo-development-inputs '())
> +                (cargo-inputs (filter cargo-input? inputs))
> +                (cargo-development-inputs (filter cargo-input? native-inputs))

I tried commenting the cargo-development-inputs out, but it only caused
problems for me when trying to compile rust-encoding <at> 0.2.

>                  #:allow-other-keys
>                  #:rest arguments)
>    "Return a bag for NAME."
> @@ -260,7 +264,9 @@ (define private-keywords
>           (build-inputs `(("cargo" ,rust "cargo")
>                           ("rustc" ,rust)
>                           ,@(expand-crate-sources cargo-inputs cargo-development-inputs)
> -                         ,@native-inputs))
> +                         ,@(if (eq? native-inputs cargo-development-inputs)
> +                               '()
> +                               native-inputs)))
>           (outputs outputs)
>           (build cargo-build)
>           (arguments (strip-keyword-arguments private-keywords arguments)))))
> diff --git a/guix/packages.scm b/guix/packages.scm
> index b3c5a00011..275cc3675c 100644
> --- a/guix/packages.scm
> +++ b/guix/packages.scm
> @@ -660,7 +660,8 @@ (define (deprecated-package old-name p)
>      (name old-name)
>      (properties `((superseded . ,p)))))
>  
> -(define (package-field-location package field)
> +(define* (package-field-location package field
> +                                 #:key (value-location? #t))
>    "Return the source code location of the definition of FIELD for PACKAGE, or
>  #f if it could not be determined."
>    (match (package-location package)
> @@ -678,7 +679,10 @@ (define (package-field-location package field)
>                     (let ((field (assoc field inits)))
>                       (match field
>                         ((_ value)
> -                        (let ((loc (and=> (source-properties value)
> +                        (let ((loc (and=> (source-properties
> +                                           (if value-location?
> +                                               value
> +                                               field))
>                                            source-properties->location)))
>                            (and loc
>                                 ;; Preserve the original file name, which may be a
> diff --git a/guix/scripts/style.scm b/guix/scripts/style.scm
> index 86a46f693c..dccc20d880 100644
> --- a/guix/scripts/style.scm
> +++ b/guix/scripts/style.scm
> @@ -29,6 +29,7 @@
>  
>  (define-module (guix scripts style)
>    #:autoload   (gnu packages) (specification->package fold-packages)
> +  #:autoload   (guix build-system cargo) (cargo-build-system)
>    #:use-module (guix scripts)
>    #:use-module ((guix scripts build) #:select (%standard-build-options))
>    #:use-module (guix combinators)
> @@ -212,6 +213,21 @@ (define (object->string* obj indent)
>        (pretty-print-with-comments port obj
>                                    #:indent indent))))
>  
> +(define (object-list->string lst indent)
> +  (call-with-output-string
> +    (lambda (port)
> +      (let loop ((lst lst))
> +        (match lst
> +          ((obj)
> +           (pretty-print-with-comments port obj
> +                                       #:indent indent))
> +          ((obj rest ...)
> +           (pretty-print-with-comments port obj
> +                                       #:indent indent)
> +           (newline port)
> +           (display (make-string indent #\space) port)
> +           (loop rest)))))))
> +
>  
>  ;;;
>  ;;; Simplifying input expressions.
> @@ -441,6 +457,49 @@ (define matches?
>              (list package-inputs package-native-inputs
>                    package-propagated-inputs)))
>  
> +
> +;;;
> +;;; Crates, Cargo, Rust, and all that.
> +;;;
> +
> +(define* (rewrite-cargo-inputs package
> +                               #:key (policy 'silent)
> +                               (edit-expression edit-expression))
> +  (when (eq? (package-build-system package) cargo-build-system)
> +    (match (package-field-location package 'arguments
> +                                   #:value-location? #f)
> +      (#f #f)
> +      (location
> +       (let* ((indent (location-column location)))
> +         (edit-expression
> +          (pk 'loc (location->source-properties location))
> +          (lambda (str)
> +            (define arguments
> +              (call-with-input-string (pk 'str str) read-with-comments))
> +
> +            (match arguments
> +              (('arguments ('quasiquote lst))
> +               (let ((inputs (match (member #:cargo-inputs lst)
> +                               (#f '())
> +                               ((_ inputs . _) inputs)))
> +                     (native (match (member #:cargo-development-inputs lst)
> +                               (#f '())
> +                               ((_ inputs . _) inputs)))
> +                     (rest (strip-keyword-arguments
> +                            '(#:cargo-inputs #:cargo-development-inputs)
> +                            lst)))
> +                 (object-list->string
> +                  `(,@(if (null? rest)
> +                          '()
> +                          `((arguments ,(list 'quasiquote rest))))
> +                    ,@(if (null? native)
> +                          '()
> +                          `((native-inputs ,(list 'quasiquote native))))
> +                    ,@(if (null? inputs)
> +                          '()
> +                          `((inputs ,(list 'quasiquote inputs)))))
> +                  indent)))))))))))
> +
>  (define (package-location<? p1 p2)
>    "Return true if P1's location is \"before\" P2's."
>    (let ((loc1 (package-location p1))
> @@ -536,7 +595,7 @@ (define (parse-options)
>                         edit-expression))
>           (policy   (assoc-ref opts 'input-simplification-policy)))
>      (for-each (lambda (package)
> -                (simplify-package-inputs package #:policy policy
> +                (rewrite-cargo-inputs package #:policy policy
>                                           #:edit-expression edit))
>                ;; Sort package by source code location so that we start editing
>                ;; files from the bottom and going upward.  That way, the


-- 
Efraim Flashner   <efraim <at> flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted
[rust-changes-c-u-f (text/plain, attachment)]
[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#51845; Package guix-patches. (Tue, 07 Dec 2021 19:49:02 GMT) Full text and rfc822 format available.

Message #41 received at 51845 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Efraim Flashner <efraim <at> flashner.co.il>
Cc: 51845 <at> debbugs.gnu.org
Subject: Re: Using ‘native-inputs’ and
 ‘inputs’ for Cargo packages?
Date: Tue, 07 Dec 2021 20:48:00 +0100
Howdy!

Efraim Flashner <efraim <at> flashner.co.il> skribis:

> On Mon, Dec 06, 2021 at 11:17:47PM +0100, Ludovic Courtès wrote:

[...]

>> With the minimal changes to (guix build-system cargo) below, one can use
>> either the current style or pass “development inputs” as ‘native-inputs’
>> and other dependencies as ‘inputs’.  Source transitivity is preserved
>> but you can write packages the normal way.
>> 
>> I modified some of the dependencies of librsvg to use
>> native-inputs/inputs and you can see when applying this part of the
>> patch that the librsvg derivation is unchanged.  Good thing is that
>> ‘guix graph’ and ‘guix refresh -l’ work for these packages.
>> 
>> Is this a direction we want to take?
>
> I like the way it works out, and has Guix do the magic to give us the
> crates in the graph. On the other hand I tried changing the cargo-inputs
> from librsvg to regular inputs, and after 2.5 minutes of trying to run
> `guix show librsvg` I still wasn't seeing the dependencies and my RAM
> usage was still increasing. Also gnu/packages/gnome.scm didn't fail to
> compile, so there was no notice of the loop.

Notice that the dependency cycle, as discussed on IRC, breaks things
like ‘guix show’ (?) and ‘guix graph’, but doesn’t break actual package
builds because it’s a <package> cycle that vanishes once packages are
lowered to bags and derivations.

Regardless, it would be nice not to have cycles in the first place.

[...]

> So to summarize, between your diff to treat inputs built using
> cargo-build-system as cargo-inputs and my changes to save previous
> crates for the next input we reach a place where we can start to change
> the crates over to use inputs and native-inputs instead of cargo-inputs
> and cargo-development-inputs without needing to flip everything at once.

What are the “changes to save previous crates for the next input”?

> So I'd go with it's good, but I'm not sure it directly works to fix the
> problem we're having with librsvg.

No no, it’s completely unrelated to the librsvg issue, which is why I
changed subject lines.  :-)  I think it’d be nice to have anyway.

>> +(define (cargo-input? input)
>> +  (match input
>> +    ((label (? package? p))
>> +     (eq? cargo-build-system (package-build-system p)))
>> +    (_ #f)))
>> +
>
> I would've sorted based on the name starting with 'rust-'.

OK.

[...]

>>  (define (package-cargo-inputs p)
>> -  (apply
>> -    (lambda* (#:key (cargo-inputs '()) #:allow-other-keys)
>> -      cargo-inputs)
>> -    (package-arguments p)))
>> +  (match (member #:cargo-inputs (package-arguments p))
>> +    (#f (filter cargo-input? (package-inputs p)))
>> +    ((_ inputs . _) inputs)))
>>  
>>  (define (package-cargo-development-inputs p)
>> -  (apply
>> -    (lambda* (#:key (cargo-development-inputs '()) #:allow-other-keys)
>> -      cargo-development-inputs)
>> -    (package-arguments p)))
>> +  (match (member #:cargo-development-inputs (package-arguments p))
>> +    (#f (filter cargo-input? (package-native-inputs p)))
>> +    ((_ inputs . _) inputs)))
>
> I see we don't get rid of #:cargo-inputs or #:cargo-development-inputs.
> So even if applying the style change to all the crates causes circular
> dependency problems we can fall back to the current method. I ran into
> problems once I hit all the rust-bindgen crates.

Right.  Support for #:cargo-development-inputs and #:cargo-inputs is
here so we could have a smooth “upgrade” without breaking compatibility.

Anyway, I think the priority is to get ‘core-updates-frozen’, which
probably involves either pinning librsvg dependencies or using the
bundled libraries as you showed at the beginning of this thread.
We can resume work on prettified Rust packages after that; it’ll be
useful to be able to use ‘guix refresh -l’.

Ludo’.




Information forwarded to guix-patches <at> gnu.org:
bug#51845; Package guix-patches. (Wed, 08 Dec 2021 14:25:02 GMT) Full text and rfc822 format available.

Message #44 received at 51845 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: guix-devel <at> gnu.org
Cc: 51845 <at> debbugs.gnu.org, Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Subject: Re: [CORE-UPDATES] librsvg and rust
Date: Wed, 08 Dec 2021 15:24:07 +0100
Hello!

For the record, this is a followup to Efraim’s proposal in
<https://issues.guix.gnu.org/51845>.

Efraim Flashner <efraim <at> flashner.co.il> skribis:

> Option 1:
> Track down the ~220 crates which form the dependency graph (of crates)
> for librsvg and pin them until the next core-updates cycle. Continue
> like with other packages and add newer versions (like cmake or meson) as
> packages need them.¹

The advantage of this approach is that we could do it incrementally: we
could merge ‘core-updates-frozen’ today and just add pinned variants of
these 200+ crates as needed as time passes.  The downside is that it’s a
lot of crates to take care of, and we might still accidentally overlook
seemingly innocuous crate upgrades that end up causing major rebuilds.

> Option 2:
> Use the bundled crates and treat it as just part of the librsvg source
> code.²
>
> Option 2b:
> Use the bundled crates for now to finish with core-updates-frozen and
> revisit this immediately on core-updates (not frozen).

This option will involved a rebuild on x86_64, but the advantage is that
we’ll be safe going forward: we won’t accidentally cause world rebuilds
just because an obscure crate somewhere has been upgraded.

[...]

> I'm currently leaning option 2b, it'll get us past this hurdle for
> core-updates-frozen and let us make changes to the crates as we work to
> integrate them more fully into Guix.

Same here; it’s not ideal, but it seems like the most reasonable
short-term option.

If there are no objections, I’d suggest that you go ahead with this
plan.

Thanks for keeping the ball rolling!

Ludo’.




Information forwarded to guix-patches <at> gnu.org:
bug#51845; Package guix-patches. (Wed, 08 Dec 2021 14:37:01 GMT) Full text and rfc822 format available.

Message #47 received at 51845 <at> debbugs.gnu.org (full text, mbox):

From: Ricardo Wurmus <rekado <at> elephly.net>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: guix-devel <at> gnu.org, 51845 <at> debbugs.gnu.org,
 Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Subject: Re: [CORE-UPDATES] librsvg and rust
Date: Wed, 08 Dec 2021 14:36:11 +0000
Ludovic Courtès <ludo <at> gnu.org> writes:

> Hello!
>
> For the record, this is a followup to Efraim’s proposal in
> <https://issues.guix.gnu.org/51845>.
>
> Efraim Flashner <efraim <at> flashner.co.il> skribis:
>
>> Option 1:
>> Track down the ~220 crates which form the dependency graph (of crates)
>> for librsvg and pin them until the next core-updates cycle. Continue
>> like with other packages and add newer versions (like cmake or meson) as
>> packages need them.¹
>
> The advantage of this approach is that we could do it incrementally: we
> could merge ‘core-updates-frozen’ today and just add pinned variants of
> these 200+ crates as needed as time passes.  The downside is that it’s a
> lot of crates to take care of, and we might still accidentally overlook
> seemingly innocuous crate upgrades that end up causing major rebuilds.
>
>> Option 2:
>> Use the bundled crates and treat it as just part of the librsvg source
>> code.²
>>
>> Option 2b:
>> Use the bundled crates for now to finish with core-updates-frozen and
>> revisit this immediately on core-updates (not frozen).
>
> This option will involved a rebuild on x86_64, but the advantage is that
> we’ll be safe going forward: we won’t accidentally cause world rebuilds
> just because an obscure crate somewhere has been upgraded.
>
> [...]
>
>> I'm currently leaning option 2b, it'll get us past this hurdle for
>> core-updates-frozen and let us make changes to the crates as we work to
>> integrate them more fully into Guix.
>
> Same here; it’s not ideal, but it seems like the most reasonable
> short-term option.
>
> If there are no objections, I’d suggest that you go ahead with this
> plan.

I agree that 2b is the most sensible option in our current situation.

-- 
Ricardo




Information forwarded to guix-patches <at> gnu.org:
bug#51845; Package guix-patches. (Wed, 08 Dec 2021 20:02:02 GMT) Full text and rfc822 format available.

Message #50 received at 51845 <at> debbugs.gnu.org (full text, mbox):

From: Kaelyn <kaelyn.alexi <at> protonmail.com>
To: guix-devel <at> gnu.org
Cc: 51845 <at> debbugs.gnu.org
Subject: Re: [CORE-UPDATES] librsvg and rust
Date: Wed, 08 Dec 2021 20:01:37 +0000
On Wednesday, December 8th, 2021 at 6:36 AM, Ricardo Wurmus <rekado <at> elephly.net> wrote:

> Ludovic Courtès ludo <at> gnu.org writes:
>
> > Hello!
> >
> > For the record, this is a followup to Efraim’s proposal in
> >
> > https://issues.guix.gnu.org/51845.
> >
> > Efraim Flashner efraim <at> flashner.co.il skribis:
> >
> > > Option 1:
> > >
> > > Track down the ~220 crates which form the dependency graph (of crates)
> > >
> > > for librsvg and pin them until the next core-updates cycle. Continue
> > >
> > > like with other packages and add newer versions (like cmake or meson) as
> > >
> > > packages need them.¹
> >
> > The advantage of this approach is that we could do it incrementally: we
> >
> > could merge ‘core-updates-frozen’ today and just add pinned variants of
> >
> > these 200+ crates as needed as time passes. The downside is that it’s a
> >
> > lot of crates to take care of, and we might still accidentally overlook
> >
> > seemingly innocuous crate upgrades that end up causing major rebuilds.
> >
> > > Option 2:
> > >
> > > Use the bundled crates and treat it as just part of the librsvg source
> > >
> > > code.²
> > >
> > > Option 2b:
> > >
> > > Use the bundled crates for now to finish with core-updates-frozen and
> > >
> > > revisit this immediately on core-updates (not frozen).
> >
> > This option will involved a rebuild on x86_64, but the advantage is that
> >
> > we’ll be safe going forward: we won’t accidentally cause world rebuilds
> >
> > just because an obscure crate somewhere has been upgraded.
> >
> > [...]
> >
> > > I'm currently leaning option 2b, it'll get us past this hurdle for
> > >
> > > core-updates-frozen and let us make changes to the crates as we work to
> > >
> > > integrate them more fully into Guix.
> >
> > Same here; it’s not ideal, but it seems like the most reasonable
> >
> > short-term option.
> >
> > If there are no objections, I’d suggest that you go ahead with this
> >
> > plan.
>
> I agree that 2b is the most sensible option in our current situation.

As a developer and new-ish Guix user (and not someone familiar with rust), I am also in favor of 2b. Dealing with 200+ dependencies takes time, and core-updates-frozen has been on the cusp of merging for some time now.

I'd like to see c-u-f merged back into master sooner, as master lacks support for newer hardware while also getting regular package updates that are only periodically merged to core-updates-frozen. Even before the c-u-f sprint last month where I switched all of my systems to c-u-f, I had one system that was first a frankensteined master before finally managing to switch it to c-u-f, to pick up a newer mesa that wasn't unusably buggy on a Radeon RX 6700 XT.

Cheers,
Kaelyn

P.S. Regarding switching my systems, the only issue I've run into that hasn't been fixed is that tigervnc only recently added support for building against xorg-server 21.1.1, and the current tigervnc release (1.12.0) was released before that support landed. (I have a standalone package definition for building a recent git commit.)
>
> ------------------------------------------------------------------------
>
> Ricardo




Reply sent to Efraim Flashner <efraim <at> flashner.co.il>:
You have taken responsibility. (Sun, 12 Dec 2021 13:22:01 GMT) Full text and rfc822 format available.

Notification sent to Efraim Flashner <efraim <at> flashner.co.il>:
bug acknowledged by developer. (Sun, 12 Dec 2021 13:22:02 GMT) Full text and rfc822 format available.

Message #55 received at 51845-done <at> debbugs.gnu.org (full text, mbox):

From: Efraim Flashner <efraim <at> flashner.co.il>
To: Ricardo Wurmus <rekado <at> elephly.net>
Cc: guix-devel <at> gnu.org, Ludovic Courtès <ludo <at> gnu.org>,
 51845-done <at> debbugs.gnu.org, Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Subject: Re: [bug#51845] [CORE-UPDATES] librsvg and rust
Date: Sun, 12 Dec 2021 15:20:25 +0200
[Message part 1 (text/plain, inline)]
On Wed, Dec 08, 2021 at 02:36:11PM +0000, Ricardo Wurmus wrote:
> 
> Ludovic Courtès <ludo <at> gnu.org> writes:
> 
> > Hello!
> >
> > For the record, this is a followup to Efraim’s proposal in
> > <https://issues.guix.gnu.org/51845>.
> >
> > Efraim Flashner <efraim <at> flashner.co.il> skribis:
> >
> >> Option 1:
> >> Track down the ~220 crates which form the dependency graph (of crates)
> >> for librsvg and pin them until the next core-updates cycle. Continue
> >> like with other packages and add newer versions (like cmake or meson) as
> >> packages need them.¹
> >
> > The advantage of this approach is that we could do it incrementally: we
> > could merge ‘core-updates-frozen’ today and just add pinned variants of
> > these 200+ crates as needed as time passes.  The downside is that it’s a
> > lot of crates to take care of, and we might still accidentally overlook
> > seemingly innocuous crate upgrades that end up causing major rebuilds.
> >
> >> Option 2:
> >> Use the bundled crates and treat it as just part of the librsvg source
> >> code.²
> >>
> >> Option 2b:
> >> Use the bundled crates for now to finish with core-updates-frozen and
> >> revisit this immediately on core-updates (not frozen).
> >
> > This option will involved a rebuild on x86_64, but the advantage is that
> > we’ll be safe going forward: we won’t accidentally cause world rebuilds
> > just because an obscure crate somewhere has been upgraded.
> >
> > [...]
> >
> >> I'm currently leaning option 2b, it'll get us past this hurdle for
> >> core-updates-frozen and let us make changes to the crates as we work to
> >> integrate them more fully into Guix.
> >
> > Same here; it’s not ideal, but it seems like the most reasonable
> > short-term option.
> >
> > If there are no objections, I’d suggest that you go ahead with this
> > plan.
> 
> I agree that 2b is the most sensible option in our current situation.

Patches pushed to core-updates-frozen. I added a TODO to fix the
situation.

Thanks for the input everyone.

-- 
Efraim Flashner   <efraim <at> flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted
[signature.asc (application/pgp-signature, inline)]

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Mon, 10 Jan 2022 12:24:05 GMT) Full text and rfc822 format available.

This bug report was last modified 2 years and 78 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.