all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [bug#35318] [PATCH] Update cargo-build-system to expand package inputs
@ 2019-04-19  5:34 Ivan Petkov
  2019-05-04 16:40 ` Ivan Petkov
  2019-05-06  8:00 ` Ludovic Courtès
  0 siblings, 2 replies; 16+ messages in thread
From: Ivan Petkov @ 2019-04-19  5:34 UTC (permalink / raw)
  To: 35318; +Cc: Chris Marusich

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

When building, cargo requires the source of all transitive dependencies to be present in its index.
Rather than force package definitions to list out all transitive inputs, the build system will
automatically expand the inputs as necessary.

Because it is possible for crates to have apparent cycles in their native dependencies,
the build system must take some measures to work around potential cycles. This patch series
takes an initial naive stab at resolving the problem, by never building native-inputs.

I plan on revisiting this sometime soon and making the system a bit more intelligent
(namely building native-inputs where possible and breaking any cycles). But for now this should
unblock working on package definitions.

I’ve also included three rust crates as a proof of concept that the system works and it handles
potential native-input cycles. These crates do nothing on their own, but they’re heavily depended
upon by the rest of the crates ecosystem, so they will eventually be useful to have in guix.

—Ivan


[-- Attachment #2: 0001-packages-allow-dynamic-input-closure-computation.patch --]
[-- Type: application/octet-stream, Size: 3435 bytes --]

From ca6dfd9451f22c4d6dc02aa7eceee0c35800dd57 Mon Sep 17 00:00:00 2001
From: Ivan Petkov <ivanppetkov@gmail.com>
Date: Tue, 16 Apr 2019 03:32:44 -0700
Subject: [PATCH 1/4] packages: allow dynamic input closure computation

* guix/packages: (transitive-inputs): Rename to
package-transitive-dependencies.
(package-transitive-dependencies): Add proc parameter and use it.
(transitive-inputs): Add it.
---
 guix/packages.scm | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/guix/packages.scm b/guix/packages.scm
index c94a651f27..658b2427ca 100644
--- a/guix/packages.scm
+++ b/guix/packages.scm
@@ -4,6 +4,7 @@
 ;;; Copyright © 2015 Eric Bavier <bavier@member.fsf.org>
 ;;; Copyright © 2016 Alex Kost <alezost@gmail.com>
 ;;; Copyright © 2017 Efraim Flashner <efraim@flashner.co.il>
+;;; Copyright © 2019 Ivan Petkov <ivanppetkov@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -95,6 +96,7 @@
             package-direct-sources
             package-transitive-sources
             package-direct-inputs
+            package-transitive-dependencies
             package-transitive-inputs
             package-transitive-target-inputs
             package-transitive-native-inputs
@@ -650,13 +652,13 @@ specifies modules in scope when evaluating SNIPPET."
                         #:deprecation-warnings #t ;to avoid a rebuild
                         #:guile-for-build guile-for-build))))
 
-(define (transitive-inputs inputs)
-  "Return the closure of INPUTS when considering the 'propagated-inputs'
-edges.  Omit duplicate inputs, except for those already present in INPUTS
-itself.
+(define (package-transitive-dependencies inputs proc)
+  "Return the closure of INPUTS when considering the edges extracted
+from each package via PROC.  Omit duplicate inputs, except for those
+already present in INPUTS itself.
 
 This is implemented as a breadth-first traversal such that INPUTS is
-preserved, and only duplicate propagated inputs are removed."
+preserved, and only duplicate extracted inputs are removed."
   (define (seen? seen item outputs)
     ;; FIXME: We're using pointer identity here, which is extremely sensitive
     ;; to memoization in package-producing procedures; see
@@ -680,7 +682,7 @@ preserved, and only duplicate propagated inputs are removed."
            (loop rest result propagated first? seen)
            (loop rest
                  (cons input result)
-                 (cons (package-propagated-inputs package) propagated)
+                 (cons (proc package) propagated)
                  first?
                  (vhash-consq package outputs seen))))
       ((input rest ...)
@@ -696,6 +698,15 @@ PACKAGE's inputs."
                    (_ #f))
                   (package-direct-inputs package))))
 
+(define (transitive-inputs inputs)
+  "Return the closure of INPUTS when considering the 'propagated-inputs'
+edges.  Omit duplicate inputs, except for those already present in INPUTS
+itself.
+
+This is implemented as a breadth-first traversal such that INPUTS is
+preserved, and only duplicate propagated inputs are removed."
+  (package-transitive-dependencies inputs package-propagated-inputs))
+
 (define (package-transitive-sources package)
   "Return PACKAGE's direct sources, and their direct sources, recursively."
   (delete-duplicates
-- 
2.21.0


[-- Attachment #3: 0002-build-system-cargo-expand-transitive-package-inputs.patch --]
[-- Type: application/octet-stream, Size: 4894 bytes --]

From 7b42290f238d5ab50d83232ec6d95db665a98302 Mon Sep 17 00:00:00 2001
From: Ivan Petkov <ivanppetkov@gmail.com>
Date: Tue, 16 Apr 2019 03:37:44 -0700
Subject: [PATCH 2/4] build-system/cargo: expand transitive package inputs

* guix/build/cargo: (package-target-inputs): Add it.
(expand-inputs): Add it.
(expand-native-inputs): Add it.
(lower): Use new variables.
---
 guix/build-system/cargo.scm | 66 +++++++++++++++++++++++++++++++++++--
 1 file changed, 64 insertions(+), 2 deletions(-)

diff --git a/guix/build-system/cargo.scm b/guix/build-system/cargo.scm
index dc137421e9..65dcd5d339 100644
--- a/guix/build-system/cargo.scm
+++ b/guix/build-system/cargo.scm
@@ -121,6 +121,68 @@ to NAME and VERSION."
                                 #:outputs (cons "src" outputs)
                                 #:guile-for-build guile-for-build))
 
+(define package-target-inputs
+  (lambda (pkg)
+    (append (package-inputs pkg)
+            (package-propagated-inputs pkg))))
+
+(define* (expand-inputs inputs)
+  "Return the transitive target inputs for each package in INPUTS.
+
+Cargo requires all transitive crate dependencies to be available in its index,
+even if they are optional (this is so it can generate deterministic
+Cargo.lock files regardless of the target platform or enabled features).
+
+Rather than burden the package definition to expand these, we'll automate
+that process in the build system."
+  (package-transitive-dependencies inputs package-target-inputs))
+
+(define* (expand-native-inputs native-inputs)
+  "Return the transitive target inputs for each package in NATIVE-INPUTS.
+
+We need all transitive crate dependencies for any cargo dev-dependencies,
+but this is only needed when building/testing a crate directly
+(i.e. we will never need transitive dev-dependencies for any dependency
+crates).
+
+In addition, we need to modify any native-inputs to avoid any apparent
+cycles. Cargo does not permit cyclic dependencies between crates, however,
+it permits cycles to occur via dev-dependencies. For example, if crate X
+depends on crate Y, crate Y's tests could pull in crate X to to verify
+everything builds properly (this is a rare scenario, but it it happens for
+example with the `proc-macro2` and `quote` crates). This is allowed by cargo
+because tests are built as a pseudo-crate which happens to depend on the
+X and Y crates, forming an acyclic graph.
+
+Cargo ultimately needs only the crate sources available when doing a build.
+Ultimately, if a cycle is detected, we can break it by replacing the current
+crate we're trying to build with a no-op build which only packages the crate
+source; cargo will then find the source available in its index and build things
+properly."
+  (map
+    (lambda (dep)
+      (match dep
+       ((name input rest ...)
+        ;; TODO: Starting out real simple by ensuring no native-inputs are
+        ;; actually built so we don't accidentally hit a cycle. We can revisit
+        ;; this later and more intelligently replace any dependency on the cycle
+        ;; root with a version of the root with #:skip-build? enabled.
+        ;;
+        ;; Right now this means that if a package shows up in both input
+        ;; and native-input closures, we'll try to build it's src output twice.
+        ;;
+        ;; This also means that we're potentially setting the #:skip-build?
+        ;; flag on non cargo-build-system packages, but this should be okay
+        ;; for now.
+        (let* ((translated-input (package
+                                   (inherit input)
+                                   (inputs '())
+                                   (propagated-inputs '())
+                                   (native-inputs '())
+                                   (arguments '(#:skip-build? #t)))))
+          `(,name ,translated-input ,@rest)))))
+   (package-transitive-dependencies native-inputs package-target-inputs)))
+
 (define* (lower name
                 #:key source inputs native-inputs outputs system target
                 (rust (default-rust))
@@ -139,13 +201,13 @@ to NAME and VERSION."
          (host-inputs `(,@(if source
                               `(("source" ,source))
                               '())
-                        ,@inputs
+                        ,@(expand-inputs inputs)
 
                         ;; Keep the standard inputs of 'gnu-build-system'
                         ,@(standard-packages)))
          (build-inputs `(("cargo" ,rust "cargo")
                          ("rustc" ,rust)
-                         ,@native-inputs))
+                         ,@(expand-native-inputs native-inputs)))
          (outputs outputs)
          (build cargo-build)
          (arguments (strip-keyword-arguments private-keywords arguments)))))
-- 
2.21.0


[-- Attachment #4: 0003-gnu-crate-add-unicode-xid.patch --]
[-- Type: application/octet-stream, Size: 2957 bytes --]

From 922e985c684b4fcafe30f1c25d698396ffb58191 Mon Sep 17 00:00:00 2001
From: Ivan Petkov <ivanppetkov@gmail.com>
Date: Tue, 16 Apr 2019 03:40:14 -0700
Subject: [PATCH 3/4] gnu: crate: add unicode-xid

gnu/local.mk: (GNU_SYSTEM_MODULES): Add new file.
gnu/packages/crates-io.scm: (rust-unicode-xid): New variable.
---
 gnu/local.mk               |  1 +
 gnu/packages/crates-io.scm | 45 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)
 create mode 100644 gnu/packages/crates-io.scm

diff --git a/gnu/local.mk b/gnu/local.mk
index 41924a7de5..bf44af76b7 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -121,6 +121,7 @@ GNU_SYSTEM_MODULES =				\
   %D%/packages/cpp.scm 				\
   %D%/packages/cppi.scm				\
   %D%/packages/cran.scm				\
+  %D%/packages/crates-io.scm			\
   %D%/packages/cross-base.scm			\
   %D%/packages/crypto.scm			\
   %D%/packages/cryptsetup.scm			\
diff --git a/gnu/packages/crates-io.scm b/gnu/packages/crates-io.scm
new file mode 100644
index 0000000000..533fe0d21e
--- /dev/null
+++ b/gnu/packages/crates-io.scm
@@ -0,0 +1,45 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2019 Ivan Petkov <ivanppetkov@gmail.com>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu packages crates-io)
+  #:use-module (guix build-system cargo)
+  #:use-module (guix download)
+  #:use-module ((guix licenses) #:prefix license:)
+  #:use-module (guix packages))
+
+(define-public rust-unicode-xid
+  (package
+    (name "rust-unicode-xid")
+    (version "0.1.0")
+    (source
+      (origin
+        (method url-fetch)
+        (uri (crate-uri "unicode-xid" version))
+        (file-name
+          (string-append name "-" version ".tar.gz"))
+        (sha256
+          (base32
+            "1z57lqh4s18rr4x0j4fw4fmp9hf9346h0kmdgqsqx0fhjr3k0wpw"))))
+    (build-system cargo-build-system)
+    (home-page
+      "https://github.com/unicode-rs/unicode-xid")
+    (synopsis "Determine Unicode XID related properties")
+    (description "Determine whether characters have the XID_Start
+or XID_Continue properties according to Unicode Standard Annex #31.")
+    ;; Dual licensed.
+    (license (list license:asl2.0 license:expat))))
-- 
2.21.0


[-- Attachment #5: 0004-gnu-crate-Add-proc-macro2-and-quote.patch --]
[-- Type: application/octet-stream, Size: 2534 bytes --]

From 70f96162e55ac1d1116a5d94134d61aa769f8be7 Mon Sep 17 00:00:00 2001
From: Ivan Petkov <ivanppetkov@gmail.com>
Date: Tue, 16 Apr 2019 03:42:27 -0700
Subject: [PATCH 4/4] gnu: crate: Add proc-macro2 and quote

* gnu/packages/crates-io.scm: (rust-proc-macro2): New variable.
(rust-quote): New variable.
---
 gnu/packages/crates-io.scm | 48 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/gnu/packages/crates-io.scm b/gnu/packages/crates-io.scm
index 533fe0d21e..90e08731ea 100644
--- a/gnu/packages/crates-io.scm
+++ b/gnu/packages/crates-io.scm
@@ -43,3 +43,51 @@
 or XID_Continue properties according to Unicode Standard Annex #31.")
     ;; Dual licensed.
     (license (list license:asl2.0 license:expat))))
+
+(define-public rust-proc-macro2
+  (package
+    (name "rust-proc-macro2")
+    (version "0.4.27")
+    (source
+      (origin
+        (method url-fetch)
+        (uri (crate-uri "proc-macro2" version))
+        (file-name
+          (string-append name "-" version ".tar.gz"))
+        (sha256
+          (base32
+            "05c92v787snyaq4ss16vxc9mdv6zndfgsdq8k3hnnyffmsf7ycad"))))
+    (build-system cargo-build-system)
+    (native-inputs
+      `(("rust-quote" ,rust-quote "src")))
+    (inputs
+      `(("rust-unicode-xid" ,rust-unicode-xid "src")))
+    (home-page "https://github.com/alexcrichton/proc-macro2")
+    (synopsis "Stable implementation of the upcoming new `proc_macro` API")
+    (description "This package provides a stable implementation of the upcoming new
+`proc_macro` API.  Comes with an option, off by default, to also reimplement itself
+in terms of the upstream unstable API.")
+    ;; Dual licensed.
+    (license (list license:asl2.0 license:expat))))
+
+(define-public rust-quote
+  (package
+    (name "rust-quote")
+    (version "0.6.12")
+    (source
+      (origin
+        (method url-fetch)
+        (uri (crate-uri "quote" version))
+        (file-name
+          (string-append name "-" version ".tar.gz"))
+        (sha256
+          (base32
+            "1nw0klza45hf127kfyrpxsxd5jw2l6h21qxalil3hkr7bnf7kx7s"))))
+    (build-system cargo-build-system)
+    (inputs
+      `(("rust-proc-macro2" ,rust-proc-macro2 "src")))
+    (home-page "https://github.com/dtolnay/quote")
+    (synopsis "Quasi-quoting macro quote!(...)")
+    (description "Quasi-quoting macro quote!(...)")
+    ;; Dual licensed.
+    (license (list license:asl2.0 license:expat))))
-- 
2.21.0


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

 

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [bug#35318] [PATCH] Update cargo-build-system to expand package inputs
  2019-04-19  5:34 [bug#35318] [PATCH] Update cargo-build-system to expand package inputs Ivan Petkov
@ 2019-05-04 16:40 ` Ivan Petkov
  2019-05-04 18:31   ` Danny Milosavljevic
  2019-05-06  8:00 ` Ludovic Courtès
  1 sibling, 1 reply; 16+ messages in thread
From: Ivan Petkov @ 2019-05-04 16:40 UTC (permalink / raw)
  To: 35318; +Cc: Chris Marusich

Friendly ping!

Thanks,
—Ivan

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [bug#35318] [PATCH] Update cargo-build-system to expand package inputs
  2019-05-04 16:40 ` Ivan Petkov
@ 2019-05-04 18:31   ` Danny Milosavljevic
  2019-05-04 21:09     ` Ivan Petkov
  0 siblings, 1 reply; 16+ messages in thread
From: Danny Milosavljevic @ 2019-05-04 18:31 UTC (permalink / raw)
  To: Ivan Petkov, ludo; +Cc: 35318, Chris Marusich

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

Hi Ludo,
Hi Ivan,

@Ludo:

Could you take a look at patch 1?  It's allowing the lookup of transitive
dependencies in Guix to be more flexible.  Is it OK?

It's used in patch 2 in order to consider both inputs and propagated inputs
rather than just propagated inputs.

@Ivan:

Thanks!  I've tested it and it works.

But I don't understand yet why you change the role of "inputs" compared
to how it is in the rest of Guix.

You have this:

+(define-public rust-proc-macro2
+  (package
[...]
+    (build-system cargo-build-system)
+    (native-inputs
+      `(("rust-quote" ,rust-quote "src")))
+    (inputs
+      `(("rust-unicode-xid" ,rust-unicode-xid "src")))
[...]

Here, inputs refer to SOURCE parts of packages which are definitely not
referred to at runtime.  Does "guix gc --references ...rust-proc-macro2..."
really refer to the source of rust-unicode-xid ?  I checked, it doesn't,
neither for the "src" derivation nor for the "out" derivation.

I think the general approach is good but I'm not certain that this won't
break other parts of Guix.  If it doesn't, fine.  @Ludo: WDYT?

Details:

A Rust crate has dependencies and dev-dependencies.

The crate needs the dev-dependencies only when building, not at runtime.

Let "transitives of X" mean "X and transitives of immediate dependencies of X and
transitives of immediate dev-dependencies of X", recursively.

The crate needs the source code of all its transitives to be available when
building, but needs none of the source code at runtime.

A crate at run time only requires the immediate, if even that (probably not!),
dependencies and none of the dev-dependencies, and not as source code.

So it's really not a propagated-input, although it kinda seems like a weird
version of a propagated-input while building (something like a
native-propagated-input).

If this can't be generalized (and I'm not sure of that--Go has a similar
static library-y view), we could also do those as (arguments ...) for the
rust build system only--although not sure how to do the resolving of
transitives then.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [bug#35318] [PATCH] Update cargo-build-system to expand package inputs
  2019-05-04 18:31   ` Danny Milosavljevic
@ 2019-05-04 21:09     ` Ivan Petkov
  0 siblings, 0 replies; 16+ messages in thread
From: Ivan Petkov @ 2019-05-04 21:09 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: 35318, Chris Marusich

Hi Danny,

Thanks for the feedback!

> On May 4, 2019, at 11:31 AM, Danny Milosavljevic <dannym@scratchpost.org> wrote:
> 
> @Ivan:
> 
> Thanks!  I've tested it and it works.
> 
> But I don't understand yet why you change the role of "inputs" compared
> to how it is in the rest of Guix.
> 
> You have this:
> 
> +(define-public rust-proc-macro2
> +  (package
> [...]
> +    (build-system cargo-build-system)
> +    (native-inputs
> +      `(("rust-quote" ,rust-quote "src")))
> +    (inputs
> +      `(("rust-unicode-xid" ,rust-unicode-xid "src")))
> [...]
> 
> Here, inputs refer to SOURCE parts of packages which are definitely not
> referred to at runtime.  Does "guix gc --references ...rust-proc-macro2..."
> really refer to the source of rust-unicode-xid ?  I checked, it doesn't,
> neither for the "src" derivation nor for the "out" derivation.
> 
> I think the general approach is good but I'm not certain that this won't
> break other parts of Guix.  If it doesn't, fine.  @Ludo: WDYT?

To my understanding, Guix only needs the inputs and native-inputs to be present
in the store during build time, and only propagated-inputs need to be present
in the store during runtime. Since cargo crates don’t need the source present
at runtime, propagated-inputs seemed inappropriate to me.

Pardon my ignorance on Guix, but what do you mean by “changing the role
of inputs”? Unless by this you mean changing the semantics of “expanding”
the inputs, then yes this is a departure from the existing usage. In my mind,
I want the Guix package definition to mirror the cargo one as it would be a
nightmare to maintain a list of the expanded transitive packages in each
definition by hand.

> Details:
> 
> A Rust crate has dependencies and dev-dependencies.
> 
> The crate needs the dev-dependencies only when building, not at runtime.
> 
> Let "transitives of X" mean "X and transitives of immediate dependencies of X and
> transitives of immediate dev-dependencies of X", recursively.
> 
> The crate needs the source code of all its transitives to be available when
> building, but needs none of the source code at runtime.
> 
> A crate at run time only requires the immediate, if even that (probably not!),
> dependencies and none of the dev-dependencies, and not as source code.
> 
> So it's really not a propagated-input, although it kinda seems like a weird
> version of a propagated-input while building (something like a
> native-propagated-input).
> 
> If this can't be generalized (and I'm not sure of that--Go has a similar
> static library-y view), we could also do those as (arguments ...) for the
> rust build system only--although not sure how to do the resolving of
> transitives then.

If this needs to be it’s own Guix concept, perhaps it would be along the lines
of propagated-native-inputs?

I opted to make the cargo-build-system perform the work of the transitive
lookups since I wasn’t sure if this would truly be a generalized feature…

—Ivan

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [bug#35318] [PATCH] Update cargo-build-system to expand package inputs
  2019-04-19  5:34 [bug#35318] [PATCH] Update cargo-build-system to expand package inputs Ivan Petkov
  2019-05-04 16:40 ` Ivan Petkov
@ 2019-05-06  8:00 ` Ludovic Courtès
  2019-05-06 16:04   ` Ivan Petkov
  1 sibling, 1 reply; 16+ messages in thread
From: Ludovic Courtès @ 2019-05-06  8:00 UTC (permalink / raw)
  To: Ivan Petkov; +Cc: 35318, Chris Marusich

Hello,

Ivan Petkov <ivanppetkov@gmail.com> skribis:

> From ca6dfd9451f22c4d6dc02aa7eceee0c35800dd57 Mon Sep 17 00:00:00 2001
> From: Ivan Petkov <ivanppetkov@gmail.com>
> Date: Tue, 16 Apr 2019 03:32:44 -0700
> Subject: [PATCH 1/4] packages: allow dynamic input closure computation
>
> * guix/packages: (transitive-inputs): Rename to
> package-transitive-dependencies.
> (package-transitive-dependencies): Add proc parameter and use it.
> (transitive-inputs): Add it.

There’s nothing written in terms of “dependencies”; instead everything
is written in terms of “inputs”, so I’d like to remain consistent here.

If we need something more specific, I’d rather see it as a private
procedure in (guix build-system cargo-build-system).

Danny wrote:

> It's used in patch 2 in order to consider both inputs and propagated inputs
> rather than just propagated inputs.

I’m not sure I want to know the details :-), but it seems to be what
‘package-transitive-inputs’ does, no?

  (define (package-transitive-inputs package)
    "Return the transitive inputs of PACKAGE---i.e., its direct inputs along
  with their propagated inputs, recursively."
    (transitive-inputs (package-direct-inputs package)))

Do you have an example of a package where this is not enough?

Thanks,
Ludo’.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [bug#35318] [PATCH] Update cargo-build-system to expand package inputs
  2019-05-06  8:00 ` Ludovic Courtès
@ 2019-05-06 16:04   ` Ivan Petkov
  2019-05-09 23:17     ` Ivan Petkov
  0 siblings, 1 reply; 16+ messages in thread
From: Ivan Petkov @ 2019-05-06 16:04 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 35318, Chris Marusich

Hi Ludo,

> On May 6, 2019, at 1:00 AM, Ludovic Courtès <ludo@gnu.org> wrote:
> 
> There’s nothing written in terms of “dependencies”; instead everything
> is written in terms of “inputs”, so I’d like to remain consistent here.

I admit, I wasn’t feeling very inspired with coming up with a name, but I wanted
to avoid confusing it with the existing package-transitive-inputs procedure.

> If we need something more specific, I’d rather see it as a private
> procedure in (guix build-system cargo-build-system).

I wanted to reuse the core traversal/memoization that was defined in the 
original package-transitive-inputs procedure, but I can fork the implementation
as a private procedure in the cargo-build-system for now.

> I’m not sure I want to know the details :-), but it seems to be what
> ‘package-transitive-inputs’ does, no?

package-transitive-inputs captures all transitive propagated-inputs, but the
cargo-build-system needs all transitive propagated-inputs and regular inputs
as well (modeling cargo dependencies as propagated-inputs does not seem
appropriate).

Thanks,
—Ivan

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [bug#35318] [PATCH] Update cargo-build-system to expand package inputs
  2019-05-06 16:04   ` Ivan Petkov
@ 2019-05-09 23:17     ` Ivan Petkov
  2019-05-15  6:08       ` Ivan Petkov
  0 siblings, 1 reply; 16+ messages in thread
From: Ivan Petkov @ 2019-05-09 23:17 UTC (permalink / raw)
  To: 35318; +Cc: Chris Marusich

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

Updated the patch series as per Ludo’s feedback.

Thanks,
—Ivan


[-- Attachment #2: 0001-build-system-cargo-expand-transitive-package-inputs.patch --]
[-- Type: application/octet-stream, Size: 6864 bytes --]

From 3ac8a092fad93754e323f7d9bec2af1a3912b36f Mon Sep 17 00:00:00 2001
From: Ivan Petkov <ivanppetkov@gmail.com>
Date: Tue, 16 Apr 2019 03:37:44 -0700
Subject: [PATCH 1/3] build-system/cargo: expand transitive package inputs

* guix/build/cargo: (package-target-inputs): Add it.
(expand-inputs): Add it.
(expand-native-inputs): Add it.
(package-transitive-dependencies): Add it.
(lower): Use new variables.
---
 guix/build-system/cargo.scm | 105 +++++++++++++++++++++++++++++++++++-
 1 file changed, 103 insertions(+), 2 deletions(-)

diff --git a/guix/build-system/cargo.scm b/guix/build-system/cargo.scm
index dc137421e9..811a044a8c 100644
--- a/guix/build-system/cargo.scm
+++ b/guix/build-system/cargo.scm
@@ -29,6 +29,8 @@
   #:use-module (guix build-system)
   #:use-module (guix build-system gnu)
   #:use-module (ice-9 match)
+  #:use-module (ice-9 vlist)
+  #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
   #:export (%cargo-build-system-modules
             %cargo-utils-modules
@@ -121,6 +123,105 @@ to NAME and VERSION."
                                 #:outputs (cons "src" outputs)
                                 #:guile-for-build guile-for-build))
 
+(define (package-transitive-dependencies inputs)
+  "Return the closure of INPUTS when considering the 'inputs' and
+'propagated-inputs' edges.  Omit duplicate inputs, except for those
+already present in INPUTS itself.
+
+This is implemented as a breadth-first traversal such that INPUTS is
+preserved, and only duplicate extracted inputs are removed.
+
+Forked from ((guix packages) transitive-inputs) since this extraction
+uses slightly different rules compared to the rest of guix (i.e. we
+extract more than just propagated-inputs)."
+  (define (seen? seen item outputs)
+    ;; FIXME: We're using pointer identity here, which is extremely sensitive
+    ;; to memoization in package-producing procedures; see
+    ;; <https://bugs.gnu.org/30155>.
+    (match (vhash-assq item seen)
+      ((_ . o) (equal? o outputs))
+      (_       #f)))
+
+  (let loop ((inputs     inputs)
+             (result     '())
+             (propagated '())
+             (first?     #t)
+             (seen       vlist-null))
+    (match inputs
+      (()
+       (if (null? propagated)
+           (reverse result)
+           (loop (reverse (concatenate propagated)) result '() #f seen)))
+      (((and input (label (? package? package) outputs ...)) rest ...)
+       (if (and (not first?) (seen? seen package outputs))
+           (loop rest result propagated first? seen)
+           (loop rest
+                 (cons input result)
+                 (cons (append (package-inputs package)
+                               (package-propagated-inputs package))
+                       propagated)
+                 first?
+                 (vhash-consq package outputs seen))))
+      ((input rest ...)
+       (loop rest (cons input result) propagated first? seen)))))
+
+(define* (expand-inputs inputs)
+  "Return the transitive target inputs for each package in INPUTS.
+
+Cargo requires all transitive crate dependencies to be available in its index,
+even if they are optional (this is so it can generate deterministic
+Cargo.lock files regardless of the target platform or enabled features).
+
+Rather than burden the package definition to expand these, we'll automate
+that process in the build system."
+  (package-transitive-dependencies inputs))
+
+(define* (expand-native-inputs native-inputs)
+  "Return the transitive target inputs for each package in NATIVE-INPUTS.
+
+We need all transitive crate dependencies for any cargo dev-dependencies,
+but this is only needed when building/testing a crate directly
+(i.e. we will never need transitive dev-dependencies for any dependency
+crates).
+
+In addition, we need to modify any native-inputs to avoid any apparent
+cycles. Cargo does not permit cyclic dependencies between crates, however,
+it permits cycles to occur via dev-dependencies. For example, if crate X
+depends on crate Y, crate Y's tests could pull in crate X to to verify
+everything builds properly (this is a rare scenario, but it it happens for
+example with the `proc-macro2` and `quote` crates). This is allowed by cargo
+because tests are built as a pseudo-crate which happens to depend on the
+X and Y crates, forming an acyclic graph.
+
+Cargo ultimately needs only the crate sources available when doing a build.
+Ultimately, if a cycle is detected, we can break it by replacing the current
+crate we're trying to build with a no-op build which only packages the crate
+source; cargo will then find the source available in its index and build things
+properly."
+  (map
+    (lambda (dep)
+      (match dep
+       ((name input rest ...)
+        ;; TODO: Starting out real simple by ensuring no native-inputs are
+        ;; actually built so we don't accidentally hit a cycle. We can revisit
+        ;; this later and more intelligently replace any dependency on the cycle
+        ;; root with a version of the root with #:skip-build? enabled.
+        ;;
+        ;; Right now this means that if a package shows up in both input
+        ;; and native-input closures, we'll try to build it's src output twice.
+        ;;
+        ;; This also means that we're potentially setting the #:skip-build?
+        ;; flag on non cargo-build-system packages, but this should be okay
+        ;; for now.
+        (let* ((translated-input (package
+                                   (inherit input)
+                                   (inputs '())
+                                   (propagated-inputs '())
+                                   (native-inputs '())
+                                   (arguments '(#:skip-build? #t)))))
+          `(,name ,translated-input ,@rest)))))
+   (package-transitive-dependencies native-inputs)))
+
 (define* (lower name
                 #:key source inputs native-inputs outputs system target
                 (rust (default-rust))
@@ -139,13 +240,13 @@ to NAME and VERSION."
          (host-inputs `(,@(if source
                               `(("source" ,source))
                               '())
-                        ,@inputs
+                        ,@(expand-inputs inputs)
 
                         ;; Keep the standard inputs of 'gnu-build-system'
                         ,@(standard-packages)))
          (build-inputs `(("cargo" ,rust "cargo")
                          ("rustc" ,rust)
-                         ,@native-inputs))
+                         ,@(expand-native-inputs native-inputs)))
          (outputs outputs)
          (build cargo-build)
          (arguments (strip-keyword-arguments private-keywords arguments)))))
-- 
2.21.0


[-- Attachment #3: 0002-gnu-crate-add-unicode-xid.patch --]
[-- Type: application/octet-stream, Size: 2957 bytes --]

From ec8509623a3b59a25a4ae995dca0f18cd5b1ce48 Mon Sep 17 00:00:00 2001
From: Ivan Petkov <ivanppetkov@gmail.com>
Date: Tue, 16 Apr 2019 03:40:14 -0700
Subject: [PATCH 2/3] gnu: crate: add unicode-xid

gnu/local.mk: (GNU_SYSTEM_MODULES): Add new file.
gnu/packages/crates-io.scm: (rust-unicode-xid): New variable.
---
 gnu/local.mk               |  1 +
 gnu/packages/crates-io.scm | 45 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)
 create mode 100644 gnu/packages/crates-io.scm

diff --git a/gnu/local.mk b/gnu/local.mk
index 3f97397c4a..f92db5821c 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -121,6 +121,7 @@ GNU_SYSTEM_MODULES =				\
   %D%/packages/cpp.scm 				\
   %D%/packages/cppi.scm				\
   %D%/packages/cran.scm				\
+  %D%/packages/crates-io.scm			\
   %D%/packages/cross-base.scm			\
   %D%/packages/crypto.scm			\
   %D%/packages/cryptsetup.scm			\
diff --git a/gnu/packages/crates-io.scm b/gnu/packages/crates-io.scm
new file mode 100644
index 0000000000..533fe0d21e
--- /dev/null
+++ b/gnu/packages/crates-io.scm
@@ -0,0 +1,45 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2019 Ivan Petkov <ivanppetkov@gmail.com>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu packages crates-io)
+  #:use-module (guix build-system cargo)
+  #:use-module (guix download)
+  #:use-module ((guix licenses) #:prefix license:)
+  #:use-module (guix packages))
+
+(define-public rust-unicode-xid
+  (package
+    (name "rust-unicode-xid")
+    (version "0.1.0")
+    (source
+      (origin
+        (method url-fetch)
+        (uri (crate-uri "unicode-xid" version))
+        (file-name
+          (string-append name "-" version ".tar.gz"))
+        (sha256
+          (base32
+            "1z57lqh4s18rr4x0j4fw4fmp9hf9346h0kmdgqsqx0fhjr3k0wpw"))))
+    (build-system cargo-build-system)
+    (home-page
+      "https://github.com/unicode-rs/unicode-xid")
+    (synopsis "Determine Unicode XID related properties")
+    (description "Determine whether characters have the XID_Start
+or XID_Continue properties according to Unicode Standard Annex #31.")
+    ;; Dual licensed.
+    (license (list license:asl2.0 license:expat))))
-- 
2.21.0


[-- Attachment #4: 0003-gnu-crate-Add-proc-macro2-and-quote.patch --]
[-- Type: application/octet-stream, Size: 2534 bytes --]

From 1cff79e586664fd76bb7eb97fe5228986b32d8b4 Mon Sep 17 00:00:00 2001
From: Ivan Petkov <ivanppetkov@gmail.com>
Date: Tue, 16 Apr 2019 03:42:27 -0700
Subject: [PATCH 3/3] gnu: crate: Add proc-macro2 and quote

* gnu/packages/crates-io.scm: (rust-proc-macro2): New variable.
(rust-quote): New variable.
---
 gnu/packages/crates-io.scm | 48 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/gnu/packages/crates-io.scm b/gnu/packages/crates-io.scm
index 533fe0d21e..90e08731ea 100644
--- a/gnu/packages/crates-io.scm
+++ b/gnu/packages/crates-io.scm
@@ -43,3 +43,51 @@
 or XID_Continue properties according to Unicode Standard Annex #31.")
     ;; Dual licensed.
     (license (list license:asl2.0 license:expat))))
+
+(define-public rust-proc-macro2
+  (package
+    (name "rust-proc-macro2")
+    (version "0.4.27")
+    (source
+      (origin
+        (method url-fetch)
+        (uri (crate-uri "proc-macro2" version))
+        (file-name
+          (string-append name "-" version ".tar.gz"))
+        (sha256
+          (base32
+            "05c92v787snyaq4ss16vxc9mdv6zndfgsdq8k3hnnyffmsf7ycad"))))
+    (build-system cargo-build-system)
+    (native-inputs
+      `(("rust-quote" ,rust-quote "src")))
+    (inputs
+      `(("rust-unicode-xid" ,rust-unicode-xid "src")))
+    (home-page "https://github.com/alexcrichton/proc-macro2")
+    (synopsis "Stable implementation of the upcoming new `proc_macro` API")
+    (description "This package provides a stable implementation of the upcoming new
+`proc_macro` API.  Comes with an option, off by default, to also reimplement itself
+in terms of the upstream unstable API.")
+    ;; Dual licensed.
+    (license (list license:asl2.0 license:expat))))
+
+(define-public rust-quote
+  (package
+    (name "rust-quote")
+    (version "0.6.12")
+    (source
+      (origin
+        (method url-fetch)
+        (uri (crate-uri "quote" version))
+        (file-name
+          (string-append name "-" version ".tar.gz"))
+        (sha256
+          (base32
+            "1nw0klza45hf127kfyrpxsxd5jw2l6h21qxalil3hkr7bnf7kx7s"))))
+    (build-system cargo-build-system)
+    (inputs
+      `(("rust-proc-macro2" ,rust-proc-macro2 "src")))
+    (home-page "https://github.com/dtolnay/quote")
+    (synopsis "Quasi-quoting macro quote!(...)")
+    (description "Quasi-quoting macro quote!(...)")
+    ;; Dual licensed.
+    (license (list license:asl2.0 license:expat))))
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [bug#35318] [PATCH] Update cargo-build-system to expand package inputs
  2019-05-09 23:17     ` Ivan Petkov
@ 2019-05-15  6:08       ` Ivan Petkov
  2019-05-15 12:44         ` Ludovic Courtès
  0 siblings, 1 reply; 16+ messages in thread
From: Ivan Petkov @ 2019-05-15  6:08 UTC (permalink / raw)
  To: 35318; +Cc: Chris Marusich

Hi everyone,

Chris and I had a very productive discussion around this patch series this
evening. We discussed an alternative approach to allowing the cargo-build-system
to capture all transitive Rust crate sources without changing the established
semantics around Guix inputs and native-inputs.

The short summary is introducing crates as new arguments in the package
definition. These arguments will be expanded to include the sources of any
transitive sources when lowered to a derivation, while preserving any
other Guix inputs/native-inputs the package may wish to include.

I'll be sending an updated patch series here once I get a chance to work on
this, and I’ll elaborate on the solution with more specifics then!

Thanks,
--Ivan

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [bug#35318] [PATCH] Update cargo-build-system to expand package inputs
  2019-05-15  6:08       ` Ivan Petkov
@ 2019-05-15 12:44         ` Ludovic Courtès
  2019-05-20  1:00           ` Ivan Petkov
  0 siblings, 1 reply; 16+ messages in thread
From: Ludovic Courtès @ 2019-05-15 12:44 UTC (permalink / raw)
  To: Ivan Petkov; +Cc: 35318, Chris Marusich

Hello!

Ivan Petkov <ivanppetkov@gmail.com> skribis:

> Chris and I had a very productive discussion around this patch series this
> evening. We discussed an alternative approach to allowing the cargo-build-system
> to capture all transitive Rust crate sources without changing the established
> semantics around Guix inputs and native-inputs.
>
> The short summary is introducing crates as new arguments in the package
> definition. These arguments will be expanded to include the sources of any
> transitive sources when lowered to a derivation, while preserving any
> other Guix inputs/native-inputs the package may wish to include.
>
> I'll be sending an updated patch series here once I get a chance to work on
> this, and I’ll elaborate on the solution with more specifics then!

OK, sounds good, thanks for the update!

And sorry for not being more responsive.

Ludo’.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [bug#35318] [PATCH] Update cargo-build-system to expand package inputs
  2019-05-15 12:44         ` Ludovic Courtès
@ 2019-05-20  1:00           ` Ivan Petkov
  2019-05-20 19:38             ` Ludovic Courtès
  2019-06-08 18:44             ` Chris Marusich
  0 siblings, 2 replies; 16+ messages in thread
From: Ivan Petkov @ 2019-05-20  1:00 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 35318, Chris Marusich

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

Hi everyone!

I’ve updated this patch series. The cargo-build-system will now expect any
crate dependencies to be listed as arguments. Specifically, regular cargo
dependencies should be specified via the #:cargo-deps parameter, and any cargo
dev-dependnecies should be specified via the #:cargo-dev-deps parameter.

The cargo-build-system will traverse any inputs specified in those parameters,
and any inputs they may have in their #:cargo-deps parameter as well,
extracting their package sources and adding them as native-inputs to the
current bag being built. This avoids having to define new semantics for package
inputs/native-inputs for expanding all transitive sources.

There are several implications of this decision:
* Building a package definition does not require actually building/checking
any dependent crates. This can be a benefits:
 - For example, sometimes a crate may have an optional dependency on some OS
 specific package which cannot be built or run on the current system. This
 approach means that the build will not fail if cargo ends up internally ignoring
 the dependency.
 - It avoids waiting for quadratic builds from source: cargo always builds
 dependencies within the current workspace. This is largely due to Rust not
 having a stable ABI and other resolutions that cargo applies. This means that
 if we have a depencency chain of X -> Y -> Z and we build each definition
 independently the following will happen:
  * Cargo will build and test crate Z
  * Cargo will build crate Z in Y's workspace, then build and test Y
  * Cargo will build crates Y and Z in X's workspace, then build and test X
* But there are also some downsides with this approach:
  - If a dependent crate is subtly broken on the system (i.e. it builds but its
  tests fail) the consuming crates may build and test successfully but
  actually fail during normal usage (however, the CI will still build all
  packages which will give visibility in case packages suddenly break).
  - Because crates aren't declared as regular inputs, other Guix facilities
  such as tracking package graphs may not work by default (however, this is
  something that can always be extended or reworked in the future).

Please let me know if anything is unclear, I’m happy to elaborate if needed!

Thanks,
—Ivan


[-- Attachment #2: 0001-build-system-cargo-expand-transitive-crate-sources.patch --]
[-- Type: application/octet-stream, Size: 7547 bytes --]

From 5457f60036ce1354b4b89b9c3c423cca14e3a777 Mon Sep 17 00:00:00 2001
From: Ivan Petkov <ivanppetkov@gmail.com>
Date: Tue, 16 Apr 2019 03:37:44 -0700
Subject: [PATCH 1/8] build-system/cargo: expand transitive crate sources

* guix/build/cargo: (package-cargo-deps): Add it.
(package-cargo-dev-deps): Add it.
(cargo-transitive-deps): Add it.
(expand-crate-sources): Add it.
(lower): New cargo-deps nd cargo-dev-deps keywords.
Use expand-crate-sources.
(private-keywords): Add new keywords.
---
 guix/build-system/cargo.scm | 115 +++++++++++++++++++++++++++++++++++-
 1 file changed, 114 insertions(+), 1 deletion(-)

diff --git a/guix/build-system/cargo.scm b/guix/build-system/cargo.scm
index dc137421e9..c1bfd13b2f 100644
--- a/guix/build-system/cargo.scm
+++ b/guix/build-system/cargo.scm
@@ -29,6 +29,8 @@
   #:use-module (guix build-system)
   #:use-module (guix build-system gnu)
   #:use-module (ice-9 match)
+  #:use-module (ice-9 vlist)
+  #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
   #:export (%cargo-build-system-modules
             %cargo-utils-modules
@@ -121,15 +123,125 @@ to NAME and VERSION."
                                 #:outputs (cons "src" outputs)
                                 #:guile-for-build guile-for-build))
 
+(define (package-cargo-deps p)
+  (apply
+    (lambda* (#:key (cargo-deps '()) #:allow-other-keys)
+      cargo-deps)
+    (package-arguments p)))
+
+(define (package-cargo-dev-deps p)
+  (apply
+    (lambda* (#:key (cargo-dev-deps '()) #:allow-other-keys)
+      cargo-dev-deps)
+    (package-arguments p)))
+
+(define (crate-transitive-deps inputs)
+  "Return the closure of INPUTS when considering the 'cargo-deps' and
+'cargod-dev-deps' edges.  Omit duplicate inputs, except for those
+already present in INPUTS itself.
+
+This is implemented as a breadth-first traversal such that INPUTS is
+preserved, and only duplicate extracted inputs are removed.
+
+Forked from ((guix packages) transitive-inputs) since this extraction
+uses slightly different rules compared to the rest of Guix (i.e. we
+do not extract the conventional inputs)."
+  (define (seen? seen item)
+    ;; FIXME: We're using pointer identity here, which is extremely sensitive
+    ;; to memoization in package-producing procedures; see
+    ;; <https://bugs.gnu.org/30155>.
+    (vhash-assq item seen))
+
+  (let loop ((inputs     inputs)
+             (result     '())
+             (propagated '())
+             (first?     #t)
+             (seen       vlist-null))
+    (match inputs
+      (()
+       (if (null? propagated)
+           (reverse result)
+           (loop (reverse (concatenate propagated)) result '() #f seen)))
+      (((and input (label (? package? package))) rest ...)
+       (if (and (not first?) (seen? seen package))
+           (loop rest result propagated first? seen)
+           (loop rest
+                 (cons input result)
+                 (cons (package-cargo-deps package)
+                       propagated)
+                 first?
+                 (vhash-consq package package seen))))
+      ((input rest ...)
+       (loop rest (cons input result) propagated first? seen)))))
+
+(define (expand-crate-sources cargo-deps cargo-dev-deps)
+  "Extract all transitive sources for CARGO-DEPS and CARGO-DEV-DEPS along their
+'cargo-deps' edges.
+
+Cargo requires all transitive crate dependencies' sources to be available
+in its index, even if they are optional (this is so it can generate
+deterministic Cargo.lock files regardless of the target platform or enabled
+features). Thus we need all transitive crate dependencies for any cargo
+dev-dependencies, but this is only needed when building/testing a crate directly
+(i.e. we will never need transitive dev-dependencies for any dependency crates).
+
+Another complication arises due potential dependency cycles from Guix's
+perspective: Although cargo does not permit cyclic dependencies between crates,
+however, it permits cycles to occur via dev-dependencies. For example, if crate
+X depends on crate Y, crate Y's tests could pull in crate X to to verify
+everything builds properly (this is a rare scenario, but it it happens for
+example with the `proc-macro2` and `quote` crates). This is allowed by cargo
+because tests are built as a pseudo-crate which happens to depend on the
+X and Y crates, forming an acyclic graph.
+
+We can side step this problem by only considering regular cargo dependencies
+since they are guaranteed to not have cycles. We can further resolve any
+potential dev-dependency cycles by extracting package sources (which never have
+any dependencies and thus no cycles can exist).
+
+There are several implications of this decision:
+* Building a package definition does not require actually building/checking
+any dependent crates. This can be a benefits:
+ - For example, sometimes a crate may have an optional dependency on some OS
+ specific package which cannot be built or run on the current system. This
+ approach means that the build will not fail if cargo ends up internally ignoring
+ the dependency.
+ - It avoids waiting for quadratic builds from source: cargo always builds
+ dependencies within the current workspace. This is largely due to Rust not
+ having a stable ABI and other resolutions that cargo applies. This means that
+ if we have a depencency chain of X -> Y -> Z and we build each definition
+ independently the following will happen:
+  * Cargo will build and test crate Z
+  * Cargo will build crate Z in Y's workspace, then build and test Y
+  * Cargo will build crates Y and Z in X's workspace, then build and test X
+* But there are also some downsides with this approach:
+  - If a dependent crate is subtly broken on the system (i.e. it builds but its
+  tests fail) the consuming crates may build and test successfully but
+  actually fail during normal usage (however, the CI will still build all
+  packages which will give visibility in case packages suddenly break).
+  - Because crates aren't declared as regular inputs, other Guix facilities
+  such as tracking package graphs may not work by default (however, this is
+  something that can always be extended or reworked in the future)."
+  (filter-map
+    (match-lambda
+      ((label (? package? p))
+       (list label (package-source p)))
+      ((label input)
+       (list label input)))
+    (crate-transitive-deps (append cargo-deps cargo-dev-deps))))
+
 (define* (lower name
                 #:key source inputs native-inputs outputs system target
                 (rust (default-rust))
+                (cargo-deps '())
+                (cargo-dev-deps '())
                 #:allow-other-keys
                 #:rest arguments)
   "Return a bag for NAME."
 
   (define private-keywords
-    '(#:source #:target #:rust #:inputs #:native-inputs #:outputs))
+    '(#:source #:target #:rust #:inputs #:native-inputs #:outputs
+      #:cargo-deps #:cargo-dev-deps))
 
   (and (not target) ;; TODO: support cross-compilation
        (bag
@@ -145,6 +257,7 @@ to NAME and VERSION."
                         ,@(standard-packages)))
          (build-inputs `(("cargo" ,rust "cargo")
                          ("rustc" ,rust)
+                         ,@(expand-crate-sources cargo-deps cargo-dev-deps)
                          ,@native-inputs))
          (outputs outputs)
          (build cargo-build)
-- 
2.21.0


[-- Attachment #3: 0002-build-system-cargo-use-sources-from-package-sources.patch --]
[-- Type: application/octet-stream, Size: 2029 bytes --]

From 3aa329c44b7ebff26dd98276ab268ee120cf9bba Mon Sep 17 00:00:00 2001
From: Ivan Petkov <ivanppetkov@gmail.com>
Date: Thu, 16 May 2019 23:02:12 -0700
Subject: [PATCH 2/8] build-system/cargo: use sources from package sources

* guix/build/cargo-build-system: (configure): Expand crate tarballs in
vendor directory.
---
 guix/build/cargo-build-system.scm | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/guix/build/cargo-build-system.scm b/guix/build/cargo-build-system.scm
index 9f44bd6ee9..44ad9744ce 100644
--- a/guix/build/cargo-build-system.scm
+++ b/guix/build/cargo-build-system.scm
@@ -67,14 +67,21 @@
   (for-each
     (match-lambda
       ((name . path)
-       (let* ((rust-share (string-append path "/share/rust-source"))
-              (basepath (basename path))
-              (link-dir (string-append vendor-dir "/" basepath)))
-         (and (file-exists? rust-share)
+       (let* ((basepath (basename path))
+              (crate-dir (string-append vendor-dir "/" basepath)))
+         (and (string-suffix? ".crate" path)
               ;; Gracefully handle duplicate inputs
-              (not (file-exists? link-dir))
-              (symlink rust-share link-dir)))))
+              (not (file-exists? crate-dir))
+              (mkdir-p crate-dir)
+              ;; Cargo crates are simply gzipped tarballs but with a .crate
+              ;; extension. We expand the source to a directory name we control
+              ;; so that we can generate any cargo checksums.
+              ;; The --strip-components argument is needed to prevent creating
+              ;; an extra directory within `crate-dir`.
+              (invoke "tar" "xvf" path "-C" crate-dir "--strip-components" "1")
+              (generate-checksums crate-dir "/dev/null")))))
     inputs)
+
   ;; Configure cargo to actually use this new directory.
   (mkdir-p ".cargo")
   (let ((port (open-file ".cargo/config" "w" #:encoding "utf-8")))
-- 
2.21.0


[-- Attachment #4: 0003-build-system-cargo-don-t-copy-source-as-an-output.patch --]
[-- Type: application/octet-stream, Size: 3196 bytes --]

From 5615665eafdc3a543e0eb4ec1ed84f7c38475446 Mon Sep 17 00:00:00 2001
From: Ivan Petkov <ivanppetkov@gmail.com>
Date: Thu, 16 May 2019 23:05:50 -0700
Subject: [PATCH 3/8] build-system/cargo: don't copy source as an output

* guix/build-system/cargo: (cargo-build)[build-expression->derivation]:
Don't add "src" output.
* guix/build/cargo-build-system: (install-source): Delete it.
(%standard-phases): Delete 'install-source.
---
 guix/build-system/cargo.scm       |  2 +-
 guix/build/cargo-build-system.scm | 21 +--------------------
 2 files changed, 2 insertions(+), 21 deletions(-)

diff --git a/guix/build-system/cargo.scm b/guix/build-system/cargo.scm
index c1bfd13b2f..fb24ea9892 100644
--- a/guix/build-system/cargo.scm
+++ b/guix/build-system/cargo.scm
@@ -120,7 +120,7 @@ to NAME and VERSION."
                                 #:inputs inputs
                                 #:system system
                                 #:modules imported-modules
-                                #:outputs (cons "src" outputs)
+                                #:outputs outputs
                                 #:guile-for-build guile-for-build))
 
 (define (package-cargo-deps p)
diff --git a/guix/build/cargo-build-system.scm b/guix/build/cargo-build-system.scm
index 44ad9744ce..26d474c9b5 100644
--- a/guix/build/cargo-build-system.scm
+++ b/guix/build/cargo-build-system.scm
@@ -79,7 +79,7 @@
               ;; The --strip-components argument is needed to prevent creating
               ;; an extra directory within `crate-dir`.
               (invoke "tar" "xvf" path "-C" crate-dir "--strip-components" "1")
-              (generate-checksums crate-dir "/dev/null")))))
+              (generate-checksums crate-dir)))))
     inputs)
 
   ;; Configure cargo to actually use this new directory.
@@ -124,24 +124,6 @@ directory = '" port)
 (define (touch file-name)
   (call-with-output-file file-name (const #t)))
 
-(define* (install-source #:key inputs outputs #:allow-other-keys)
-  "Install the source for a given Cargo package."
-  (let* ((out (assoc-ref outputs "out"))
-         (src (assoc-ref inputs "source"))
-         (rsrc (string-append (assoc-ref outputs "src")
-                              "/share/rust-source")))
-    (mkdir-p rsrc)
-    ;; Rust doesn't have a stable ABI yet. Because of this
-    ;; Cargo doesn't have a search path for binaries yet.
-    ;; Until this changes we are working around this by
-    ;; vendoring the crates' sources by symlinking them
-    ;; to store paths.
-    (copy-recursively "." rsrc)
-    (touch (string-append rsrc "/.cargo-ok"))
-    (generate-checksums rsrc)
-    (install-file "Cargo.toml" rsrc)
-    #t))
-
 (define* (install #:key inputs outputs skip-build? #:allow-other-keys)
   "Install a given Cargo package."
   (let* ((out (assoc-ref outputs "out")))
@@ -163,7 +145,6 @@ directory = '" port)
 (define %standard-phases
   (modify-phases gnu:%standard-phases
     (delete 'bootstrap)
-    (add-before 'configure 'install-source install-source)
     (replace 'configure configure)
     (replace 'build build)
     (replace 'check check)
-- 
2.21.0


[-- Attachment #5: 0004-doc-Update-cargo-build-system-parameter-docs.patch --]
[-- Type: application/octet-stream, Size: 2052 bytes --]

From e79254d9ca054dcc5c03d5e54a5206e0793ab3e7 Mon Sep 17 00:00:00 2001
From: Ivan Petkov <ivanppetkov@gmail.com>
Date: Fri, 17 May 2019 09:07:54 -0700
Subject: [PATCH 4/8] doc: Update cargo-build-system parameter docs

* doc/guix.texi: (Build Systems)[cargo-build-system]: Add references to
the #:rust, #:cargo-deps, and #:cargo-dev-deps parameters.
Remove reference to installing crate sources.
---
 doc/guix.texi | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index ae9ad0739e..8683a5f4ed 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -5780,10 +5780,20 @@ This variable is exported by @code{(guix build-system cargo)}.  It
 supports builds of packages using Cargo, the build tool of the
 @uref{https://www.rust-lang.org, Rust programming language}.
 
-In its @code{configure} phase, this build system replaces dependencies
-specified in the @file{Cargo.toml} file with inputs to the Guix package.
-The @code{install} phase installs the binaries, and it also installs the
-source code and @file{Cargo.toml} file.
+It adds @code{rustc} and @code{cargo} to the set of inputs.
+A different Rust package can be specified with the @code{#:rust} parameter.
+
+Regular cargo dependencies should be added to the package definition via
+the @code{#:cargo-deps} parameter as a list of name and spec pairs, where the
+spec can be a package or a source definition. Note that the spec must evaluate
+to a path to a gzipped tarball with the @code{.crate} extension, or it will be
+ignored. Similarly, cargo dev-dependencies should be added to the package
+definition via the @code{#:cargo-dev-deps} parameter.
+
+In its @code{configure} phase, this build system will make any source inputs
+specified in the @code{#:cargo-deps} and @code{#:cargo-dev-deps} parameters
+available to cargo. The @code{install} phase installs any crate the binaries
+if they are defined by the crate.
 @end defvr
 
 @cindex Clojure (programming language)
-- 
2.21.0


[-- Attachment #6: 0005-import-crate-import-sources-with-.crate-extension.patch --]
[-- Type: application/octet-stream, Size: 1152 bytes --]

From d802ae3164c6724fd831b847c468046bc32d332d Mon Sep 17 00:00:00 2001
From: Ivan Petkov <ivanppetkov@gmail.com>
Date: Fri, 17 May 2019 00:25:06 -0700
Subject: [PATCH 5/8] import: crate: import sources with .crate extension

* guix/import/crate.scm: (make-crate-sexp)[file-name]: Use .crate extension.
---
 guix/import/crate.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/guix/import/crate.scm b/guix/import/crate.scm
index e0b400d054..24d3a7d961 100644
--- a/guix/import/crate.scm
+++ b/guix/import/crate.scm
@@ -94,7 +94,7 @@ VERSION, INPUTS, NATIVE-INPUTS, HOME-PAGE, SYNOPSIS, DESCRIPTION, and LICENSE."
                    (source (origin
                              (method url-fetch)
                              (uri (crate-uri ,name version))
-                             (file-name (string-append name "-" version ".tar.gz"))
+                             (file-name (string-append name "-" version ".crate"))
                              (sha256
                               (base32
                                ,(bytevector->nix-base32-string (port-sha256 port))))))
-- 
2.21.0


[-- Attachment #7: 0006-import-crate-define-dependencies-as-arguments.patch --]
[-- Type: application/octet-stream, Size: 5464 bytes --]

From c81535f09c78c8008f285ab05b127a7e68b6f00b Mon Sep 17 00:00:00 2001
From: Ivan Petkov <ivanppetkov@gmail.com>
Date: Fri, 17 May 2019 00:26:07 -0700
Subject: [PATCH 6/8] import: crate: define dependencies as arguments

* guix/import/crate.scm:
(crate-fetch)[input-crates]: Rename to dev-crates.
[native-input-crates]: Rename to dev-dep-crates.
[inputs]: Rename to cargo-deps.
[native-inputs]: Rename to cargo-dev-deps.
(maybe-cargo-deps): Add it.
(maybe-cargo-dev-deps): Add it.
(maybe-arguments): Add it.
(make-crate-sexp): Use new procedures.
[inputs]: Rename to cargo-deps.
[native-inputs]: Rename to cargo-dev-deps.
* guix/import/utils.scm: (package-names->package-inputs): Make public.
Add doc string.
---
 guix/import/crate.scm | 43 ++++++++++++++++++++++++++++++++-----------
 guix/import/utils.scm |  4 ++++
 2 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/guix/import/crate.scm b/guix/import/crate.scm
index 24d3a7d961..0ed683298b 100644
--- a/guix/import/crate.scm
+++ b/guix/import/crate.scm
@@ -65,29 +65,50 @@
              (path (string-append "/" version "/dependencies"))
              (deps-json (json-fetch-alist (string-append crate-url name path)))
              (deps (assoc-ref deps-json "dependencies"))
-             (input-crates (filter (crate-kind-predicate "normal") deps))
-             (native-input-crates
+             (dep-crates (filter (crate-kind-predicate "normal") deps))
+             (dev-dep-crates
               (filter (lambda (dep)
                         (not ((crate-kind-predicate "normal") dep))) deps))
-             (inputs (crates->inputs input-crates))
-             (native-inputs (crates->inputs native-input-crates))
+             (cargo-deps (crates->inputs dep-crates))
+             (cargo-dev-deps (crates->inputs dev-dep-crates))
              (home-page (match homepage
                           (() repository)
                           (_ homepage))))
     (callback #:name name #:version version
-              #:inputs inputs #:native-inputs native-inputs
+              #:cargo-deps cargo-deps #:cargo-dev-deps cargo-dev-deps
               #:home-page home-page #:synopsis synopsis
               #:description description #:license license)))
 
-(define* (make-crate-sexp #:key name version inputs native-inputs
+(define (maybe-cargo-deps package-names)
+  (match (package-names->package-inputs package-names)
+    (()
+     '())
+    ((package-inputs ...)
+     `((#:cargo-deps ,package-inputs)))))
+
+(define (maybe-cargo-dev-deps package-names)
+  (match (package-names->package-inputs package-names)
+    (()
+     '())
+    ((package-inputs ...)
+     `((#:cargo-dev-deps ,package-inputs)))))
+
+(define (maybe-arguments arguments)
+  (match arguments
+    (()
+     '())
+    ((args ...)
+     `((arguments (,'quasiquote ,args))))))
+
+(define* (make-crate-sexp #:key name version cargo-deps cargo-dev-deps
                           home-page synopsis description license
                           #:allow-other-keys)
   "Return the `package' s-expression for a rust package with the given NAME,
-VERSION, INPUTS, NATIVE-INPUTS, HOME-PAGE, SYNOPSIS, DESCRIPTION, and LICENSE."
+VERSION, CARGO-DEPS, CARGO-DEV-DEPS, HOME-PAGE, SYNOPSIS, DESCRIPTION, and LICENSE."
   (let* ((port (http-fetch (crate-uri name version)))
          (guix-name (crate-name->package-name name))
-         (inputs (map crate-name->package-name inputs))
-         (native-inputs (map crate-name->package-name native-inputs))
+         (cargo-deps (map crate-name->package-name cargo-deps))
+         (cargo-dev-deps (map crate-name->package-name cargo-dev-deps))
          (pkg `(package
                    (name ,guix-name)
                    (version ,version)
@@ -99,8 +120,8 @@ VERSION, INPUTS, NATIVE-INPUTS, HOME-PAGE, SYNOPSIS, DESCRIPTION, and LICENSE."
                               (base32
                                ,(bytevector->nix-base32-string (port-sha256 port))))))
                    (build-system cargo-build-system)
-                   ,@(maybe-native-inputs native-inputs "src")
-                   ,@(maybe-inputs inputs "src")
+                   ,@(maybe-arguments (append (maybe-cargo-deps cargo-deps)
+                                              (maybe-cargo-dev-deps cargo-dev-deps)))
                    (home-page ,(match home-page
                                  (() "")
                                  (_ home-page)))
diff --git a/guix/import/utils.scm b/guix/import/utils.scm
index 516c0cfaa2..33cb9bbc36 100644
--- a/guix/import/utils.scm
+++ b/guix/import/utils.scm
@@ -51,6 +51,7 @@
             url-fetch
             guix-hash-url
 
+            package-names->package-inputs
             maybe-inputs
             maybe-native-inputs
             package->definition
@@ -235,6 +236,9 @@ into a proper sentence and by using two spaces between sentences."
                               cleaned 'pre ".  " 'post)))
 
 (define* (package-names->package-inputs names #:optional (output #f))
+  "Given a list of PACKAGE-NAMES, and an optional OUTPUT, tries to generate a
+quoted list of inputs, as suitable to use in an 'inputs' field of a package
+definition."
   (map (lambda (input)
          (cons* input (list 'unquote (string->symbol input))
                             (or (and output (list output))
-- 
2.21.0


[-- Attachment #8: 0007-gnu-crate-add-unicode-xid.patch --]
[-- Type: application/octet-stream, Size: 2956 bytes --]

From 990ec0caa1d8f86750fc183e85e4b7bae9dc5106 Mon Sep 17 00:00:00 2001
From: Ivan Petkov <ivanppetkov@gmail.com>
Date: Tue, 16 Apr 2019 03:40:14 -0700
Subject: [PATCH 7/8] gnu: crate: add unicode-xid

gnu/local.mk: (GNU_SYSTEM_MODULES): Add new file.
gnu/packages/crates-io.scm: (rust-unicode-xid): New variable.
---
 gnu/local.mk               |  1 +
 gnu/packages/crates-io.scm | 45 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)
 create mode 100644 gnu/packages/crates-io.scm

diff --git a/gnu/local.mk b/gnu/local.mk
index 694bbfd367..bfa66e0840 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -121,6 +121,7 @@ GNU_SYSTEM_MODULES =				\
   %D%/packages/cpp.scm 				\
   %D%/packages/cppi.scm				\
   %D%/packages/cran.scm				\
+  %D%/packages/crates-io.scm			\
   %D%/packages/cross-base.scm			\
   %D%/packages/crypto.scm			\
   %D%/packages/cryptsetup.scm			\
diff --git a/gnu/packages/crates-io.scm b/gnu/packages/crates-io.scm
new file mode 100644
index 0000000000..1440edef9b
--- /dev/null
+++ b/gnu/packages/crates-io.scm
@@ -0,0 +1,45 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2019 Ivan Petkov <ivanppetkov@gmail.com>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu packages crates-io)
+  #:use-module (guix build-system cargo)
+  #:use-module (guix download)
+  #:use-module ((guix licenses) #:prefix license:)
+  #:use-module (guix packages))
+
+(define-public rust-unicode-xid
+  (package
+    (name "rust-unicode-xid")
+    (version "0.1.0")
+    (source
+      (origin
+        (method url-fetch)
+        (uri (crate-uri "unicode-xid" version))
+        (file-name
+          (string-append name "-" version ".crate"))
+        (sha256
+          (base32
+            "1z57lqh4s18rr4x0j4fw4fmp9hf9346h0kmdgqsqx0fhjr3k0wpw"))))
+    (build-system cargo-build-system)
+    (home-page
+      "https://github.com/unicode-rs/unicode-xid")
+    (synopsis "Determine Unicode XID related properties")
+    (description "Determine whether characters have the XID_Start
+or XID_Continue properties according to Unicode Standard Annex #31.")
+    ;; Dual licensed.
+    (license (list license:asl2.0 license:expat))))
-- 
2.21.0


[-- Attachment #9: 0008-gnu-crate-Add-proc-macro2-and-quote.patch --]
[-- Type: application/octet-stream, Size: 2546 bytes --]

From cc4008769de81015749e2af46612540dca05cea7 Mon Sep 17 00:00:00 2001
From: Ivan Petkov <ivanppetkov@gmail.com>
Date: Tue, 16 Apr 2019 03:42:27 -0700
Subject: [PATCH 8/8] gnu: crate: Add proc-macro2 and quote

* gnu/packages/crates-io.scm: (rust-proc-macro2): New variable.
(rust-quote): New variable.
---
 gnu/packages/crates-io.scm | 47 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/gnu/packages/crates-io.scm b/gnu/packages/crates-io.scm
index 1440edef9b..a10128413a 100644
--- a/gnu/packages/crates-io.scm
+++ b/gnu/packages/crates-io.scm
@@ -43,3 +43,50 @@
 or XID_Continue properties according to Unicode Standard Annex #31.")
     ;; Dual licensed.
     (license (list license:asl2.0 license:expat))))
+
+(define-public rust-proc-macro2
+  (package
+    (name "rust-proc-macro2")
+    (version "0.4.27")
+    (source
+      (origin
+        (method url-fetch)
+        (uri (crate-uri "proc-macro2" version))
+        (file-name
+          (string-append name "-" version ".crate"))
+        (sha256
+          (base32
+            "05c92v787snyaq4ss16vxc9mdv6zndfgsdq8k3hnnyffmsf7ycad"))))
+    (build-system cargo-build-system)
+    (arguments
+      `(#:cargo-deps (("rust-unicode-xid" ,rust-unicode-xid))
+        #:cargo-dev-deps (("rust-quote" ,rust-quote))))
+    (home-page "https://github.com/alexcrichton/proc-macro2")
+    (synopsis "Stable implementation of the upcoming new `proc_macro` API")
+    (description "This package provides a stable implementation of the upcoming new
+`proc_macro` API.  Comes with an option, off by default, to also reimplement itself
+in terms of the upstream unstable API.")
+    ;; Dual licensed.
+    (license (list license:asl2.0 license:expat))))
+
+(define-public rust-quote
+  (package
+    (name "rust-quote")
+    (version "0.6.12")
+    (source
+      (origin
+        (method url-fetch)
+        (uri (crate-uri "quote" version))
+        (file-name
+          (string-append name "-" version ".crate"))
+        (sha256
+          (base32
+            "1nw0klza45hf127kfyrpxsxd5jw2l6h21qxalil3hkr7bnf7kx7s"))))
+    (build-system cargo-build-system)
+    (arguments
+      `(#:cargo-deps (("rust-proc-macro2" ,rust-proc-macro2))))
+    (home-page "https://github.com/dtolnay/quote")
+    (synopsis "Quasi-quoting macro quote!(...)")
+    (description "Quasi-quoting macro quote!(...)")
+    ;; Dual licensed.
+    (license (list license:asl2.0 license:expat))))
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [bug#35318] [PATCH] Update cargo-build-system to expand package inputs
  2019-05-20  1:00           ` Ivan Petkov
@ 2019-05-20 19:38             ` Ludovic Courtès
  2019-05-22  2:48               ` Ivan Petkov
  2019-06-08 18:44             ` Chris Marusich
  1 sibling, 1 reply; 16+ messages in thread
From: Ludovic Courtès @ 2019-05-20 19:38 UTC (permalink / raw)
  To: Ivan Petkov; +Cc: 35318, Chris Marusich

Hi Ivan!

Ivan Petkov <ivanppetkov@gmail.com> skribis:

> Please let me know if anything is unclear, I’m happy to elaborate if needed!

I’ll only comment superficially because I haven’t followed the rest of
the discussion and I know too little about Rust; hopefully Danny and
Chris can provide feedback.

> From 5457f60036ce1354b4b89b9c3c423cca14e3a777 Mon Sep 17 00:00:00 2001
> From: Ivan Petkov <ivanppetkov@gmail.com>
> Date: Tue, 16 Apr 2019 03:37:44 -0700
> Subject: [PATCH 1/8] build-system/cargo: expand transitive crate sources
>
> * guix/build/cargo: (package-cargo-deps): Add it.
> (package-cargo-dev-deps): Add it.
> (cargo-transitive-deps): Add it.
> (expand-crate-sources): Add it.
> (lower): New cargo-deps nd cargo-dev-deps keywords.
> Use expand-crate-sources.
> (private-keywords): Add new keywords.

[...]

> +(define (package-cargo-deps p)
> +  (apply
> +    (lambda* (#:key (cargo-deps '()) #:allow-other-keys)
> +      cargo-deps)
> +    (package-arguments p)))

It’s surprising style.  It seems redundant with the ‘inputs’ field, but
IIUC, the main difference here is that you can simply name dependencies,
even if there’s no Guix package for it, right?

> +(define (package-cargo-dev-deps p)
> +  (apply
> +    (lambda* (#:key (cargo-dev-deps '()) #:allow-other-keys)
> +      cargo-dev-deps)
> +    (package-arguments p)))

As a rule of thumb, please avoid abbreviations in identifiers (info
"(guix) Coding Style").  So that would be
‘package-development-dependencies’ or something like that.

> +(define (crate-transitive-deps inputs)
> +  "Return the closure of INPUTS when considering the 'cargo-deps' and
> +'cargod-dev-deps' edges.  Omit duplicate inputs, except for those
> +already present in INPUTS itself.
> +
> +This is implemented as a breadth-first traversal such that INPUTS is
> +preserved, and only duplicate extracted inputs are removed.
> +
> +Forked from ((guix packages) transitive-inputs) since this extraction
> +uses slightly different rules compared to the rest of Guix (i.e. we
> +do not extract the conventional inputs)."

Perhaps call it ‘crate-closure’?

> +(define (expand-crate-sources cargo-deps cargo-dev-deps)
> +  "Extract all transitive sources for CARGO-DEPS and CARGO-DEV-DEPS along their
> +'cargo-deps' edges.

Maybe s/cargo-deps/inputs/ and s/cargo-dev-deps/development-inputs/?

I’d prefer to stick to the same terminology as in the rest of the code
if we’re talking about the same sort of input lists.

That’s it.  :-)

Thank you for improving Rust support!

Ludo’.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [bug#35318] [PATCH] Update cargo-build-system to expand package inputs
  2019-05-20 19:38             ` Ludovic Courtès
@ 2019-05-22  2:48               ` Ivan Petkov
  0 siblings, 0 replies; 16+ messages in thread
From: Ivan Petkov @ 2019-05-22  2:48 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 35318, Chris Marusich

Hi Ludo!

> On May 20, 2019, at 12:38 PM, Ludovic Courtès <ludo@gnu.org> wrote:
> 
>> +(define (package-cargo-deps p)
>> +  (apply
>> +    (lambda* (#:key (cargo-deps '()) #:allow-other-keys)
>> +      cargo-deps)
>> +    (package-arguments p)))
> 
> It’s surprising style.  It seems redundant with the ‘inputs’ field, but
> IIUC, the main difference here is that you can simply name dependencies,
> even if there’s no Guix package for it, right?

That’s one benefit, the other is that we’re defining our own new semantics
on the cargo-specific inputs here to be treated like propagated-inputs, but
without actually making the store install them when a Rust binary is substituted.

>> +(define (package-cargo-dev-deps p)
>> +  (apply
>> +    (lambda* (#:key (cargo-dev-deps '()) #:allow-other-keys)
>> +      cargo-dev-deps)
>> +    (package-arguments p)))
> 
> As a rule of thumb, please avoid abbreviations in identifiers (info
> "(guix) Coding Style").  So that would be
> ‘package-development-dependencies’ or something like that.

Thanks for the tip, I’ll update these names. 

Since the actual cargo documentation actually refers to “dev-dependencies”
do you think it’s better to use “cargo-dev-dependencies” (for consistency that
Rust programmers might be used to), or stick with “cargo-development-dependencies”
(for Guix consistencies)?

>> +(define (crate-transitive-deps inputs)
>> +  "Return the closure of INPUTS when considering the 'cargo-deps' and
>> +'cargod-dev-deps' edges.  Omit duplicate inputs, except for those
>> +already present in INPUTS itself.
>> +
>> +This is implemented as a breadth-first traversal such that INPUTS is
>> +preserved, and only duplicate extracted inputs are removed.
>> +
>> +Forked from ((guix packages) transitive-inputs) since this extraction
>> +uses slightly different rules compared to the rest of Guix (i.e. we
>> +do not extract the conventional inputs)."
> 
> Perhaps call it ‘crate-closure’?

Sure that works, I’ll rename this!

>> +(define (expand-crate-sources cargo-deps cargo-dev-deps)
>> +  "Extract all transitive sources for CARGO-DEPS and CARGO-DEV-DEPS along their
>> +'cargo-deps' edges.
> 
> Maybe s/cargo-deps/inputs/ and s/cargo-dev-deps/development-inputs/?
> 
> I’d prefer to stick to the same terminology as in the rest of the code
> if we’re talking about the same sort of input lists.

I can rename this as well.

> 
> That’s it.  :-)
> 
> Thank you for improving Rust support!

Happy to help :)

—Ivan

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [bug#35318] [PATCH] Update cargo-build-system to expand package inputs
  2019-05-20  1:00           ` Ivan Petkov
  2019-05-20 19:38             ` Ludovic Courtès
@ 2019-06-08 18:44             ` Chris Marusich
  2019-06-08 23:33               ` Ivan Petkov
  1 sibling, 1 reply; 16+ messages in thread
From: Chris Marusich @ 2019-06-08 18:44 UTC (permalink / raw)
  To: Ivan Petkov; +Cc: 35318

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

Hi Ivan,

Sorry for taking so long to review this.  In short, I think these
changes are good, and if nobody has more feedback in the next few days,
I will merge it and we can see how it works in practice!

At a high level, I think these changes have the following pros and cons:

Pros:

    - We can now build Rust crates.  Fantastic!
    - The "dependencies" and "dev-dependencies" terminology is the same
      as Cargo uses, so it will be familiar to Rust programmers.
    - We avoid O(N^2) build time, where N is the number of dependencies.
    - We can use the importer to quickly import new crates and iterate.

Cons:

    - Because the "dependencies" and "dev-dependencies" are specified as
      package arguments instead of any kind of "input", they won't show
      up in some of the graphs produced by "guix package".  However, in
      theory "guix graph" could be taught to display nodes for crate
      "dependencies" and "dev-dependencies", too.
      
    - Everyone who defines a Guix package for a crate must make sure the
      origin's file name ends in ".cargo", since the cargo-build-system
      now assumes that any input ending in ".cargo" is a crate that
      should be extracted into the build's crate-dir.  This is a little
      brittle, and I wish we had a better way to check this, but I can't
      think of a better way at the moment.  Since you've updated the
      importer to always add this, it probably won't be much of an issue
      in practice, since that's the default way new crates will be added
      to Guix.  Going forward, maybe we can avoid this by just checking
      the inputs to see which ones are gzipped tarballs containing
      Cargo.toml files.

Limitations imposed by Rust/Cargo itself:

    - I'm not a Rust or Cargo expert, but my current understanding is
      that it isn't feasible to save the artifacts produced when
      building a crate for re-use when building another crate.  In the
      world of C, it is common to produce a library, and then link
      against that library when building other software later.  In Guix,
      when building a C library, we install the built artifacts (e.g.,
      .so files) into the output, so that those artifacts can be used as
      the input for another package's build.  It seems that, by Cargo's
      design, it isn't currently feasible to do the same sort of thing
      with Cargo: that is, it isn't feasible to build artifacts, install
      them somewhere for later use, and then later re-use them in
      another Cargo build.  I'd be glad to learn that I'm mistaken, but
      currently that is my understanding.

    - Related to this, I doubt that a Rust programmer will be able to
      invoke a command like "guix environment my-crate" (even if we
      teach it to understand crate "dependencies" and
      "dev-dependencies") to make all the dependencies required to build
      my-crate available.  If a Rust programmer wants to hack on
      my-crate, they'll probably still just use "cargo" to do it without
      using Guix at all.  Is there any way to avoid this and make it
      possible to get the dependencies used by Guix in the build, so
      that a Rust programmer can hack around using precisely those
      dependencies?  If this were C or Python, you could do that using
      "guix environment," but I'm not sure how this could work with Rust
      crates.

Thanks again for working on this!

-- 
Chris

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [bug#35318] [PATCH] Update cargo-build-system to expand package inputs
  2019-06-08 18:44             ` Chris Marusich
@ 2019-06-08 23:33               ` Ivan Petkov
  2019-06-09 23:53                 ` Ivan Petkov
  0 siblings, 1 reply; 16+ messages in thread
From: Ivan Petkov @ 2019-06-08 23:33 UTC (permalink / raw)
  To: Chris Marusich; +Cc: 35318

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

Hi Chris,

> On Jun 8, 2019, at 11:44 AM, Chris Marusich <cmmarusich@gmail.com> wrote:
> 
> Sorry for taking so long to review this.  In short, I think these
> changes are good, and if nobody has more feedback in the next few days,
> I will merge it and we can see how it works in practice!

Thanks for the feedback, and thanks for the wonderful pros and cons summary!

> Cons:
> 
>    - Because the "dependencies" and "dev-dependencies" are specified as
>      package arguments instead of any kind of "input", they won't show
>      up in some of the graphs produced by "guix package".  However, in
>      theory "guix graph" could be taught to display nodes for crate
>      "dependencies" and "dev-dependencies", too.

Totally correct, we can iterate on this in the future.

>    - Everyone who defines a Guix package for a crate must make sure the
>      origin's file name ends in ".cargo", since the cargo-build-system
>      now assumes that any input ending in ".cargo" is a crate that
>      should be extracted into the build's crate-dir.  This is a little
>      brittle, and I wish we had a better way to check this, but I can't
>      think of a better way at the moment.  Since you've updated the
>      importer to always add this, it probably won't be much of an issue
>      in practice, since that's the default way new crates will be added
>      to Guix.  Going forward, maybe we can avoid this by just checking
>      the inputs to see which ones are gzipped tarballs containing
>      Cargo.toml files.

We discussed an alternative solution to this offline, namely we can ask
tar to check if a Cargo.toml file exists at the top level without fully unpacking
the archive. If it exists, we can assume the tarball is a crate source and only
then unpack it in the vendor directory.

This will make things less brittle as the `.crate` convention won’t be necessary
(a potential downside would be that if there happens to be some arbitrary input
which happens to be a tarball which happens to have a Cargo.toml, it will get
included in the vendor directory without us knowing about it. I think the chances
of this happening in practice are virtually zero, so we can iterate on this if it ever
becomes a problem).

I’m going to try to update the patch series to include this change over the next
few days (along with Ludo’s naming-related feedback). If I don’t get a chance to
finish this, we can always merge it in later!

> Limitations imposed by Rust/Cargo itself:
> 
>    - I'm not a Rust or Cargo expert, but my current understanding is
>      that it isn't feasible to save the artifacts produced when
>      building a crate for re-use when building another crate.  In the
>      world of C, it is common to produce a library, and then link
>      against that library when building other software later.  In Guix,
>      when building a C library, we install the built artifacts (e.g.,
>      .so files) into the output, so that those artifacts can be used as
>      the input for another package's build.  It seems that, by Cargo's
>      design, it isn't currently feasible to do the same sort of thing
>      with Cargo: that is, it isn't feasible to build artifacts, install
>      them somewhere for later use, and then later re-use them in
>      another Cargo build.  I'd be glad to learn that I'm mistaken, but
>      currently that is my understanding.

Your understanding here is correct.

>    - Related to this, I doubt that a Rust programmer will be able to
>      invoke a command like "guix environment my-crate" (even if we
>      teach it to understand crate "dependencies" and
>      "dev-dependencies") to make all the dependencies required to build
>      my-crate available.  If a Rust programmer wants to hack on
>      my-crate, they'll probably still just use "cargo" to do it without
>      using Guix at all.  Is there any way to avoid this and make it
>      possible to get the dependencies used by Guix in the build, so
>      that a Rust programmer can hack around using precisely those
>      dependencies?  If this were C or Python, you could do that using
>      "guix environment," but I'm not sure how this could work with Rust
>      crates.

Correct again, a programmer will not be able to run `guix environment my-crate`,
however, I don’t think anyone will do this in practice, since you can’t “point” cargo
at that source closure/outputs anyway.

For hacking on Rust crates, I’d imagine a Rust programmer would simply stick with
cargo and the crates.io <http://crates.io/> ecosystem proper. I don’t ever see guix as being a replacement
for crates.io <http://crates.io/> (it would be impossible to keep up with the package changes, even if we
automate things, crates aren’t guaranteed to even build, etc.).

I’d imagine that crates should only be imported into guix if they’re necessary for supporting
an actual application or service written in Rust, but not used for day-to-day development.

—Ivan



[-- Attachment #2: Type: text/html, Size: 7946 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [bug#35318] [PATCH] Update cargo-build-system to expand package inputs
  2019-06-08 23:33               ` Ivan Petkov
@ 2019-06-09 23:53                 ` Ivan Petkov
  2019-06-12  1:14                   ` bug#35318: " Chris Marusich
  0 siblings, 1 reply; 16+ messages in thread
From: Ivan Petkov @ 2019-06-09 23:53 UTC (permalink / raw)
  To: Chris Marusich; +Cc: 35318

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

I’ve updated the patch series with the following improvements:

* I've applied any naming feedback which came from Ludo
* I've changed the cargo-build-system to only unpack inputs into the cargo
vendor directory if the following applies:
  - The input is a path to a gzip tarball
  - The archive contains a file called Cargo.toml at its root
  - This means that we no longer require crate sources to include a
  ".crate" extension

Whew, given that this has gotten pretty long, I'd be happy to land this as it
is for now (barring any blocking issues!), and iterating further going forward!

Thanks again to everyone for their feedback!
—Ivan


[-- Attachment #2: 0001-build-system-cargo-expand-transitive-crate-sources.patch --]
[-- Type: application/octet-stream, Size: 7662 bytes --]

From c063c354c02df84ae2a3aaad81175725622cc9d2 Mon Sep 17 00:00:00 2001
From: Ivan Petkov <ivanppetkov@gmail.com>
Date: Tue, 16 Apr 2019 03:37:44 -0700
Subject: [PATCH 1/7] build-system/cargo: expand transitive crate sources

* guix/build/cargo: (package-cargo-inputs): Add it.
(package-cargo-development-inputs): Add it.
(crate-clsoure): Add it.
(expand-crate-sources): Add it.
(lower): New cargo-inputs and cargo-development-inputs keywords.
Use expand-crate-sources.
(private-keywords): Add new keywords.
---
 guix/build-system/cargo.scm | 115 +++++++++++++++++++++++++++++++++++-
 1 file changed, 114 insertions(+), 1 deletion(-)

diff --git a/guix/build-system/cargo.scm b/guix/build-system/cargo.scm
index dc137421e9..828382678d 100644
--- a/guix/build-system/cargo.scm
+++ b/guix/build-system/cargo.scm
@@ -29,6 +29,8 @@
   #:use-module (guix build-system)
   #:use-module (guix build-system gnu)
   #:use-module (ice-9 match)
+  #:use-module (ice-9 vlist)
+  #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
   #:export (%cargo-build-system-modules
             %cargo-utils-modules
@@ -121,15 +123,125 @@ to NAME and VERSION."
                                 #:outputs (cons "src" outputs)
                                 #:guile-for-build guile-for-build))
 
+(define (package-cargo-inputs p)
+  (apply
+    (lambda* (#:key (cargo-inputs '()) #:allow-other-keys)
+      cargo-inputs)
+    (package-arguments p)))
+
+(define (package-cargo-development-inputs p)
+  (apply
+    (lambda* (#:key (cargo-development-inputs '()) #:allow-other-keys)
+      cargo-development-inputs)
+    (package-arguments p)))
+
+(define (crate-closure inputs)
+  "Return the closure of INPUTS when considering the 'cargo-inputs' and
+'cargod-dev-deps' edges.  Omit duplicate inputs, except for those
+already present in INPUTS itself.
+
+This is implemented as a breadth-first traversal such that INPUTS is
+preserved, and only duplicate extracted inputs are removed.
+
+Forked from ((guix packages) transitive-inputs) since this extraction
+uses slightly different rules compared to the rest of Guix (i.e. we
+do not extract the conventional inputs)."
+  (define (seen? seen item)
+    ;; FIXME: We're using pointer identity here, which is extremely sensitive
+    ;; to memoization in package-producing procedures; see
+    ;; <https://bugs.gnu.org/30155>.
+    (vhash-assq item seen))
+
+  (let loop ((inputs     inputs)
+             (result     '())
+             (propagated '())
+             (first?     #t)
+             (seen       vlist-null))
+    (match inputs
+      (()
+       (if (null? propagated)
+           (reverse result)
+           (loop (reverse (concatenate propagated)) result '() #f seen)))
+      (((and input (label (? package? package))) rest ...)
+       (if (and (not first?) (seen? seen package))
+           (loop rest result propagated first? seen)
+           (loop rest
+                 (cons input result)
+                 (cons (package-cargo-inputs package)
+                       propagated)
+                 first?
+                 (vhash-consq package package seen))))
+      ((input rest ...)
+       (loop rest (cons input result) propagated first? seen)))))
+
+(define (expand-crate-sources cargo-inputs cargo-development-inputs)
+  "Extract all transitive sources for CARGO-INPUTS and CARGO-DEVELOPMENT-INPUTS
+along their 'cargo-inputs' edges.
+
+Cargo requires all transitive crate dependencies' sources to be available
+in its index, even if they are optional (this is so it can generate
+deterministic Cargo.lock files regardless of the target platform or enabled
+features). Thus we need all transitive crate dependencies for any cargo
+dev-dependencies, but this is only needed when building/testing a crate directly
+(i.e. we will never need transitive dev-dependencies for any dependency crates).
+
+Another complication arises due potential dependency cycles from Guix's
+perspective: Although cargo does not permit cyclic dependencies between crates,
+however, it permits cycles to occur via dev-dependencies. For example, if crate
+X depends on crate Y, crate Y's tests could pull in crate X to to verify
+everything builds properly (this is a rare scenario, but it it happens for
+example with the `proc-macro2` and `quote` crates). This is allowed by cargo
+because tests are built as a pseudo-crate which happens to depend on the
+X and Y crates, forming an acyclic graph.
+
+We can side step this problem by only considering regular cargo dependencies
+since they are guaranteed to not have cycles. We can further resolve any
+potential dev-dependency cycles by extracting package sources (which never have
+any dependencies and thus no cycles can exist).
+
+There are several implications of this decision:
+* Building a package definition does not require actually building/checking
+any dependent crates. This can be a benefits:
+ - For example, sometimes a crate may have an optional dependency on some OS
+ specific package which cannot be built or run on the current system. This
+ approach means that the build will not fail if cargo ends up internally ignoring
+ the dependency.
+ - It avoids waiting for quadratic builds from source: cargo always builds
+ dependencies within the current workspace. This is largely due to Rust not
+ having a stable ABI and other resolutions that cargo applies. This means that
+ if we have a depencency chain of X -> Y -> Z and we build each definition
+ independently the following will happen:
+  * Cargo will build and test crate Z
+  * Cargo will build crate Z in Y's workspace, then build and test Y
+  * Cargo will build crates Y and Z in X's workspace, then build and test X
+* But there are also some downsides with this approach:
+  - If a dependent crate is subtly broken on the system (i.e. it builds but its
+  tests fail) the consuming crates may build and test successfully but
+  actually fail during normal usage (however, the CI will still build all
+  packages which will give visibility in case packages suddenly break).
+  - Because crates aren't declared as regular inputs, other Guix facilities
+  such as tracking package graphs may not work by default (however, this is
+  something that can always be extended or reworked in the future)."
+  (filter-map
+    (match-lambda
+      ((label (? package? p))
+       (list label (package-source p)))
+      ((label input)
+       (list label input)))
+    (crate-closure (append cargo-inputs cargo-development-inputs))))
+
 (define* (lower name
                 #:key source inputs native-inputs outputs system target
                 (rust (default-rust))
+                (cargo-inputs '())
+                (cargo-development-inputs '())
                 #:allow-other-keys
                 #:rest arguments)
   "Return a bag for NAME."
 
   (define private-keywords
-    '(#:source #:target #:rust #:inputs #:native-inputs #:outputs))
+    '(#:source #:target #:rust #:inputs #:native-inputs #:outputs
+      #:cargo-inputs #:cargo-development-inputs))
 
   (and (not target) ;; TODO: support cross-compilation
        (bag
@@ -145,6 +257,7 @@ to NAME and VERSION."
                         ,@(standard-packages)))
          (build-inputs `(("cargo" ,rust "cargo")
                          ("rustc" ,rust)
+                         ,@(expand-crate-sources cargo-inputs cargo-development-inputs)
                          ,@native-inputs))
          (outputs outputs)
          (build cargo-build)
-- 
2.21.0


[-- Attachment #3: 0002-build-system-cargo-use-sources-from-package-sources.patch --]
[-- Type: application/octet-stream, Size: 3294 bytes --]

From 93d8e04dc6d863213ad240ab11147a140bd665a9 Mon Sep 17 00:00:00 2001
From: Ivan Petkov <ivanppetkov@gmail.com>
Date: Thu, 16 May 2019 23:02:12 -0700
Subject: [PATCH 2/7] build-system/cargo: use sources from package sources

* guix/build/cargo-build-system: (configure): Expand crate tarballs in
vendor directory.
---
 guix/build/cargo-build-system.scm | 35 +++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/guix/build/cargo-build-system.scm b/guix/build/cargo-build-system.scm
index 9f44bd6ee9..b368074e8a 100644
--- a/guix/build/cargo-build-system.scm
+++ b/guix/build/cargo-build-system.scm
@@ -54,6 +54,22 @@
          (bin-dep? (lambda (dep) (find bin? (get-kinds dep)))))
     (find bin-dep? (manifest-targets))))
 
+(define (crate-src? path)
+  "Check if PATH refers to a crate source, namely a gzipped tarball with a
+Cargo.toml file present at its root."
+    (and (gzip-file? path)
+         ;; First we print out all file names within the tarball to see if it
+         ;; looks like the source of a crate. However, the tarball will include
+         ;; an extra path component which we would like to ignore (since we're
+         ;; interested in checking if a Cargo.toml exists at the root of the
+         ;; archive, but not nested anywhere else). We do this by cutting up
+         ;; each output line and only looking at the second component. We then
+         ;; check if it matches Cargo.toml exactly and short circuit if it does.
+         (zero? (apply system* (list "sh" "-c"
+                                     (string-append "tar -tf " path
+                                                    " | cut -d/ -f2"
+                                                    " | grep -q '^Cargo.toml$'"))))))
+
 (define* (configure #:key inputs
                     (vendor-dir "guix-vendor")
                     #:allow-other-keys)
@@ -67,14 +83,21 @@
   (for-each
     (match-lambda
       ((name . path)
-       (let* ((rust-share (string-append path "/share/rust-source"))
-              (basepath (basename path))
-              (link-dir (string-append vendor-dir "/" basepath)))
-         (and (file-exists? rust-share)
+       (let* ((basepath (basename path))
+              (crate-dir (string-append vendor-dir "/" basepath)))
+         (and (crate-src? path)
               ;; Gracefully handle duplicate inputs
-              (not (file-exists? link-dir))
-              (symlink rust-share link-dir)))))
+              (not (file-exists? crate-dir))
+              (mkdir-p crate-dir)
+              ;; Cargo crates are simply gzipped tarballs but with a .crate
+              ;; extension. We expand the source to a directory name we control
+              ;; so that we can generate any cargo checksums.
+              ;; The --strip-components argument is needed to prevent creating
+              ;; an extra directory within `crate-dir`.
+              (invoke "tar" "xvf" path "-C" crate-dir "--strip-components" "1")
+              (generate-checksums crate-dir)))))
     inputs)
+
   ;; Configure cargo to actually use this new directory.
   (mkdir-p ".cargo")
   (let ((port (open-file ".cargo/config" "w" #:encoding "utf-8")))
-- 
2.21.0


[-- Attachment #4: 0003-build-system-cargo-don-t-copy-source-as-an-output.patch --]
[-- Type: application/octet-stream, Size: 2765 bytes --]

From fec67a58e05588813bfc4808bd0792dd8f7564c6 Mon Sep 17 00:00:00 2001
From: Ivan Petkov <ivanppetkov@gmail.com>
Date: Thu, 16 May 2019 23:05:50 -0700
Subject: [PATCH 3/7] build-system/cargo: don't copy source as an output

* guix/build-system/cargo: (cargo-build)[build-expression->derivation]:
Don't add "src" output.
* guix/build/cargo-build-system: (install-source): Delete it.
(%standard-phases): Delete 'install-source.
---
 guix/build-system/cargo.scm       |  2 +-
 guix/build/cargo-build-system.scm | 19 -------------------
 2 files changed, 1 insertion(+), 20 deletions(-)

diff --git a/guix/build-system/cargo.scm b/guix/build-system/cargo.scm
index 828382678d..fa211d456d 100644
--- a/guix/build-system/cargo.scm
+++ b/guix/build-system/cargo.scm
@@ -120,7 +120,7 @@ to NAME and VERSION."
                                 #:inputs inputs
                                 #:system system
                                 #:modules imported-modules
-                                #:outputs (cons "src" outputs)
+                                #:outputs outputs
                                 #:guile-for-build guile-for-build))
 
 (define (package-cargo-inputs p)
diff --git a/guix/build/cargo-build-system.scm b/guix/build/cargo-build-system.scm
index b368074e8a..1f36304b15 100644
--- a/guix/build/cargo-build-system.scm
+++ b/guix/build/cargo-build-system.scm
@@ -140,24 +140,6 @@ directory = '" port)
 (define (touch file-name)
   (call-with-output-file file-name (const #t)))
 
-(define* (install-source #:key inputs outputs #:allow-other-keys)
-  "Install the source for a given Cargo package."
-  (let* ((out (assoc-ref outputs "out"))
-         (src (assoc-ref inputs "source"))
-         (rsrc (string-append (assoc-ref outputs "src")
-                              "/share/rust-source")))
-    (mkdir-p rsrc)
-    ;; Rust doesn't have a stable ABI yet. Because of this
-    ;; Cargo doesn't have a search path for binaries yet.
-    ;; Until this changes we are working around this by
-    ;; vendoring the crates' sources by symlinking them
-    ;; to store paths.
-    (copy-recursively "." rsrc)
-    (touch (string-append rsrc "/.cargo-ok"))
-    (generate-checksums rsrc)
-    (install-file "Cargo.toml" rsrc)
-    #t))
-
 (define* (install #:key inputs outputs skip-build? #:allow-other-keys)
   "Install a given Cargo package."
   (let* ((out (assoc-ref outputs "out")))
@@ -179,7 +161,6 @@ directory = '" port)
 (define %standard-phases
   (modify-phases gnu:%standard-phases
     (delete 'bootstrap)
-    (add-before 'configure 'install-source install-source)
     (replace 'configure configure)
     (replace 'build build)
     (replace 'check check)
-- 
2.21.0


[-- Attachment #5: 0004-doc-Update-cargo-build-system-parameter-docs.patch --]
[-- Type: application/octet-stream, Size: 2502 bytes --]

From 7d75538f8e24bb0dcbdd252a19ec68dd42947831 Mon Sep 17 00:00:00 2001
From: Ivan Petkov <ivanppetkov@gmail.com>
Date: Fri, 17 May 2019 09:07:54 -0700
Subject: [PATCH 4/7] doc: Update cargo-build-system parameter docs

* doc/guix.texi: (Build Systems)[cargo-build-system]: Add references to
the #:rust, #:cargo-inputs, and #:cargo-development-inputs parameters.
Remove reference to installing crate sources.
---
 doc/guix.texi | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index a9cd66ce87..61f5de9e5d 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -64,6 +64,7 @@ Copyright @copyright{} 2018 Laura Lazzati@*
 Copyright @copyright{} 2018 Alex Vong@*
 Copyright @copyright{} 2019 Josh Holland@*
 Copyright @copyright{} 2019 Diego Nicola Barbato@*
+Copyright @copyright{} 2019 Ivan Petkov@*
 
 Permission is granted to copy, distribute and/or modify this document
 under the terms of the GNU Free Documentation License, Version 1.3 or
@@ -5814,10 +5815,20 @@ This variable is exported by @code{(guix build-system cargo)}.  It
 supports builds of packages using Cargo, the build tool of the
 @uref{https://www.rust-lang.org, Rust programming language}.
 
-In its @code{configure} phase, this build system replaces dependencies
-specified in the @file{Cargo.toml} file with inputs to the Guix package.
-The @code{install} phase installs the binaries, and it also installs the
-source code and @file{Cargo.toml} file.
+It adds @code{rustc} and @code{cargo} to the set of inputs.
+A different Rust package can be specified with the @code{#:rust} parameter.
+
+Regular cargo dependencies should be added to the package definition via
+the @code{#:cargo-inputs} parameter as a list of name and spec pairs, where the
+spec can be a package or a source definition. Note that the spec must evaluate
+to a path to a gzipped tarball which includes a @code{Cargo.toml} file at its
+root, or it will be ignored. Similarly, cargo dev-dependencies should be added
+to the package definition via the @code{#:cargo-development-inputs} parameter.
+
+In its @code{configure} phase, this build system will make any source inputs
+specified in the @code{#:cargo-inputs} and @code{#:cargo-development-inputs}
+parameters available to cargo. The @code{install} phase installs any crate the
+binaries if they are defined by the crate.
 @end defvr
 
 @cindex Clojure (programming language)
-- 
2.21.0


[-- Attachment #6: 0005-import-crate-define-dependencies-as-arguments.patch --]
[-- Type: application/octet-stream, Size: 5742 bytes --]

From ee47fe46390f7996e77a8af7974899be68000dda Mon Sep 17 00:00:00 2001
From: Ivan Petkov <ivanppetkov@gmail.com>
Date: Fri, 17 May 2019 00:26:07 -0700
Subject: [PATCH 5/7] import: crate: define dependencies as arguments

* guix/import/crate.scm:
(crate-fetch)[input-crates]: Rename to dev-crates.
[native-input-crates]: Rename to dev-dep-crates.
[inputs]: Rename to cargo-inputs.
[native-inputs]: Rename to cargo-development-inputs.
(maybe-cargo-inputs): Add it.
(maybe-cargo-development-inputs): Add it.
(maybe-arguments): Add it.
(make-crate-sexp): Use new procedures.
[inputs]: Rename to cargo-inputs.
[native-inputs]: Rename to cargo-development-inputs.
* guix/import/utils.scm: (package-names->package-inputs): Make public.
Add doc string.
---
 guix/import/crate.scm | 47 +++++++++++++++++++++++++++++++++----------
 guix/import/utils.scm |  4 ++++
 2 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/guix/import/crate.scm b/guix/import/crate.scm
index e0b400d054..9a73d9fe16 100644
--- a/guix/import/crate.scm
+++ b/guix/import/crate.scm
@@ -65,29 +65,53 @@
              (path (string-append "/" version "/dependencies"))
              (deps-json (json-fetch-alist (string-append crate-url name path)))
              (deps (assoc-ref deps-json "dependencies"))
-             (input-crates (filter (crate-kind-predicate "normal") deps))
-             (native-input-crates
+             (dep-crates (filter (crate-kind-predicate "normal") deps))
+             (dev-dep-crates
               (filter (lambda (dep)
                         (not ((crate-kind-predicate "normal") dep))) deps))
-             (inputs (crates->inputs input-crates))
-             (native-inputs (crates->inputs native-input-crates))
+             (cargo-inputs (crates->inputs dep-crates))
+             (cargo-development-inputs (crates->inputs dev-dep-crates))
              (home-page (match homepage
                           (() repository)
                           (_ homepage))))
     (callback #:name name #:version version
-              #:inputs inputs #:native-inputs native-inputs
+              #:cargo-inputs cargo-inputs
+              #:cargo-development-inputs cargo-development-inputs
               #:home-page home-page #:synopsis synopsis
               #:description description #:license license)))
 
-(define* (make-crate-sexp #:key name version inputs native-inputs
+(define (maybe-cargo-inputs package-names)
+  (match (package-names->package-inputs package-names)
+    (()
+     '())
+    ((package-inputs ...)
+     `((#:cargo-inputs ,package-inputs)))))
+
+(define (maybe-cargo-development-inputs package-names)
+  (match (package-names->package-inputs package-names)
+    (()
+     '())
+    ((package-inputs ...)
+     `((#:cargo-development-inputs ,package-inputs)))))
+
+(define (maybe-arguments arguments)
+  (match arguments
+    (()
+     '())
+    ((args ...)
+     `((arguments (,'quasiquote ,args))))))
+
+(define* (make-crate-sexp #:key name version cargo-inputs cargo-development-inputs
                           home-page synopsis description license
                           #:allow-other-keys)
   "Return the `package' s-expression for a rust package with the given NAME,
-VERSION, INPUTS, NATIVE-INPUTS, HOME-PAGE, SYNOPSIS, DESCRIPTION, and LICENSE."
+VERSION, CARGO-INPUTS, CARGO-DEVELOPMENT-INPUTS, HOME-PAGE, SYNOPSIS, DESCRIPTION,
+and LICENSE."
   (let* ((port (http-fetch (crate-uri name version)))
          (guix-name (crate-name->package-name name))
-         (inputs (map crate-name->package-name inputs))
-         (native-inputs (map crate-name->package-name native-inputs))
+         (cargo-inputs (map crate-name->package-name cargo-inputs))
+         (cargo-development-inputs (map crate-name->package-name
+                                        cargo-development-inputs))
          (pkg `(package
                    (name ,guix-name)
                    (version ,version)
@@ -99,8 +123,9 @@ VERSION, INPUTS, NATIVE-INPUTS, HOME-PAGE, SYNOPSIS, DESCRIPTION, and LICENSE."
                               (base32
                                ,(bytevector->nix-base32-string (port-sha256 port))))))
                    (build-system cargo-build-system)
-                   ,@(maybe-native-inputs native-inputs "src")
-                   ,@(maybe-inputs inputs "src")
+                   ,@(maybe-arguments (append (maybe-cargo-inputs cargo-inputs)
+                                              (maybe-cargo-development-inputs
+                                                cargo-development-inputs)))
                    (home-page ,(match home-page
                                  (() "")
                                  (_ home-page)))
diff --git a/guix/import/utils.scm b/guix/import/utils.scm
index 63fc9bbb27..84503ab907 100644
--- a/guix/import/utils.scm
+++ b/guix/import/utils.scm
@@ -52,6 +52,7 @@
             url-fetch
             guix-hash-url
 
+            package-names->package-inputs
             maybe-inputs
             maybe-native-inputs
             package->definition
@@ -236,6 +237,9 @@ into a proper sentence and by using two spaces between sentences."
                               cleaned 'pre ".  " 'post)))
 
 (define* (package-names->package-inputs names #:optional (output #f))
+  "Given a list of PACKAGE-NAMES, and an optional OUTPUT, tries to generate a
+quoted list of inputs, as suitable to use in an 'inputs' field of a package
+definition."
   (map (lambda (input)
          (cons* input (list 'unquote (string->symbol input))
                             (or (and output (list output))
-- 
2.21.0


[-- Attachment #7: 0006-gnu-crate-add-unicode-xid.patch --]
[-- Type: application/octet-stream, Size: 2957 bytes --]

From d6ee18d7df3af4b2a6ca445328c3c88cf145ffd5 Mon Sep 17 00:00:00 2001
From: Ivan Petkov <ivanppetkov@gmail.com>
Date: Tue, 16 Apr 2019 03:40:14 -0700
Subject: [PATCH 6/7] gnu: crate: add unicode-xid

gnu/local.mk: (GNU_SYSTEM_MODULES): Add new file.
gnu/packages/crates-io.scm: (rust-unicode-xid): New variable.
---
 gnu/local.mk               |  1 +
 gnu/packages/crates-io.scm | 45 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)
 create mode 100644 gnu/packages/crates-io.scm

diff --git a/gnu/local.mk b/gnu/local.mk
index 907641ff3e..048359ce4e 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -121,6 +121,7 @@ GNU_SYSTEM_MODULES =				\
   %D%/packages/cpp.scm 				\
   %D%/packages/cppi.scm				\
   %D%/packages/cran.scm				\
+  %D%/packages/crates-io.scm			\
   %D%/packages/cross-base.scm			\
   %D%/packages/crypto.scm			\
   %D%/packages/cryptsetup.scm			\
diff --git a/gnu/packages/crates-io.scm b/gnu/packages/crates-io.scm
new file mode 100644
index 0000000000..533fe0d21e
--- /dev/null
+++ b/gnu/packages/crates-io.scm
@@ -0,0 +1,45 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2019 Ivan Petkov <ivanppetkov@gmail.com>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu packages crates-io)
+  #:use-module (guix build-system cargo)
+  #:use-module (guix download)
+  #:use-module ((guix licenses) #:prefix license:)
+  #:use-module (guix packages))
+
+(define-public rust-unicode-xid
+  (package
+    (name "rust-unicode-xid")
+    (version "0.1.0")
+    (source
+      (origin
+        (method url-fetch)
+        (uri (crate-uri "unicode-xid" version))
+        (file-name
+          (string-append name "-" version ".tar.gz"))
+        (sha256
+          (base32
+            "1z57lqh4s18rr4x0j4fw4fmp9hf9346h0kmdgqsqx0fhjr3k0wpw"))))
+    (build-system cargo-build-system)
+    (home-page
+      "https://github.com/unicode-rs/unicode-xid")
+    (synopsis "Determine Unicode XID related properties")
+    (description "Determine whether characters have the XID_Start
+or XID_Continue properties according to Unicode Standard Annex #31.")
+    ;; Dual licensed.
+    (license (list license:asl2.0 license:expat))))
-- 
2.21.0


[-- Attachment #8: 0007-gnu-crate-Add-proc-macro2-and-quote.patch --]
[-- Type: application/octet-stream, Size: 2562 bytes --]

From 4caa240fd6a245b89e640f6ef1fcaaf9a2b06c3e Mon Sep 17 00:00:00 2001
From: Ivan Petkov <ivanppetkov@gmail.com>
Date: Tue, 16 Apr 2019 03:42:27 -0700
Subject: [PATCH 7/7] gnu: crate: Add proc-macro2 and quote

* gnu/packages/crates-io.scm: (rust-proc-macro2): New variable.
(rust-quote): New variable.
---
 gnu/packages/crates-io.scm | 47 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/gnu/packages/crates-io.scm b/gnu/packages/crates-io.scm
index 533fe0d21e..b480b6fe56 100644
--- a/gnu/packages/crates-io.scm
+++ b/gnu/packages/crates-io.scm
@@ -43,3 +43,50 @@
 or XID_Continue properties according to Unicode Standard Annex #31.")
     ;; Dual licensed.
     (license (list license:asl2.0 license:expat))))
+
+(define-public rust-proc-macro2
+  (package
+    (name "rust-proc-macro2")
+    (version "0.4.27")
+    (source
+      (origin
+        (method url-fetch)
+        (uri (crate-uri "proc-macro2" version))
+        (file-name
+          (string-append name "-" version ".tar.gz"))
+        (sha256
+          (base32
+            "05c92v787snyaq4ss16vxc9mdv6zndfgsdq8k3hnnyffmsf7ycad"))))
+    (build-system cargo-build-system)
+    (arguments
+      `(#:cargo-inputs (("rust-unicode-xid" ,rust-unicode-xid))
+        #:cargo-development-inputs (("rust-quote" ,rust-quote))))
+    (home-page "https://github.com/alexcrichton/proc-macro2")
+    (synopsis "Stable implementation of the upcoming new `proc_macro` API")
+    (description "This package provides a stable implementation of the upcoming new
+`proc_macro` API.  Comes with an option, off by default, to also reimplement itself
+in terms of the upstream unstable API.")
+    ;; Dual licensed.
+    (license (list license:asl2.0 license:expat))))
+
+(define-public rust-quote
+  (package
+    (name "rust-quote")
+    (version "0.6.12")
+    (source
+      (origin
+        (method url-fetch)
+        (uri (crate-uri "quote" version))
+        (file-name
+          (string-append name "-" version ".tar.gz"))
+        (sha256
+          (base32
+            "1nw0klza45hf127kfyrpxsxd5jw2l6h21qxalil3hkr7bnf7kx7s"))))
+    (build-system cargo-build-system)
+    (arguments
+      `(#:cargo-inputs (("rust-proc-macro2" ,rust-proc-macro2))))
+    (home-page "https://github.com/dtolnay/quote")
+    (synopsis "Quasi-quoting macro quote!(...)")
+    (description "Quasi-quoting macro quote!(...)")
+    ;; Dual licensed.
+    (license (list license:asl2.0 license:expat))))
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* bug#35318: [PATCH] Update cargo-build-system to expand package inputs
  2019-06-09 23:53                 ` Ivan Petkov
@ 2019-06-12  1:14                   ` Chris Marusich
  0 siblings, 0 replies; 16+ messages in thread
From: Chris Marusich @ 2019-06-12  1:14 UTC (permalink / raw)
  To: Ivan Petkov; +Cc: Chris Marusich, 35318-done

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

Hi Ivan,

I've merged your changes as 2444abd9c124cc55f8f19a0462e06a2094f25a9d.
Thank you for taking the time to write this patch series!  Now let's
start importing some crates!  :-)

Some minor feedback for next time (I've made these minor changes on your
behalf already):

- In the ChangeLog, we generally capitalize the heading and add a
  period.
- In the manual, we put two spaces after periods, not one.
- The tests/crate.scm began failing when you changed the importer logic;
  I have taken the liberty of fixing it.
- When listing many parts that changed, you can write it like this,
  instead of listing it on 3 separate lines:

    (maybe-cargo-inputs, maybe-cargo-development-inputs)
    (maybe-arguments): Add them.

That's all.  Thanks again!  I'm closing this now.

-- 
Chris

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-06-12  1:15 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-19  5:34 [bug#35318] [PATCH] Update cargo-build-system to expand package inputs Ivan Petkov
2019-05-04 16:40 ` Ivan Petkov
2019-05-04 18:31   ` Danny Milosavljevic
2019-05-04 21:09     ` Ivan Petkov
2019-05-06  8:00 ` Ludovic Courtès
2019-05-06 16:04   ` Ivan Petkov
2019-05-09 23:17     ` Ivan Petkov
2019-05-15  6:08       ` Ivan Petkov
2019-05-15 12:44         ` Ludovic Courtès
2019-05-20  1:00           ` Ivan Petkov
2019-05-20 19:38             ` Ludovic Courtès
2019-05-22  2:48               ` Ivan Petkov
2019-06-08 18:44             ` Chris Marusich
2019-06-08 23:33               ` Ivan Petkov
2019-06-09 23:53                 ` Ivan Petkov
2019-06-12  1:14                   ` bug#35318: " Chris Marusich

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.