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

