Re: [PATCH 1/7] build-system: Add cargo build system.

[Ricardo Wurmus]

David Craven <address@hidden> writes:

> * guix/build-system/cargo.scm (default-cargo, default-rustc,
>   %cargo-build-system-modules, cargo-build, lower, cargo-build-system):
>   New variables.
> * guix/build/cargo-build-system.scm (configure, build, check, install,
>   %standard-phases, cargo-build): New variables.

Since these are new files you can just write

* guix/build-system/cargo.scm: New file.
* guix/build/cargo-build-system.scm: New file.

But you should also add them to “gnu/local.mk”.

> +(define (system->rust-platform system)
> +  (cond
> +   ((string-prefix? "x86_64" system) "x86_64-unknown-linux-gnu")
> +   ((string-prefix? "i686" system) "i686-unknown-linux-gnu")))

Is there no rust for other systems?  How about adding a default case, or
are you ensuring that system can only have one of these two values?

> +(define* (cargo-build store name inputs
> +                      #:key
> +                      (tests? #t)
> +                      (test-target #f)

Is this correct?  What happens when test-target is not specified?  I see
below that the target is hard-coded to “test”.  Maybe set it to “test”
here and use it on the build side?

> diff --git a/guix/build/cargo-build-system.scm 
> b/guix/build/cargo-build-system.scm
> new file mode 100644
> index 0000000..abe7e05
> --- /dev/null
> +++ b/guix/build/cargo-build-system.scm
> @@ -0,0 +1,81 @@
> +;;; GNU Guix --- Functional package management for GNU
> +;;; Copyright © 2016 David Craven <address@hidden>
> +;;;
> +;;; 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 (guix build cargo-build-system)
> +  #:use-module ((guix build gnu-build-system) #:prefix gnu:)
> +  #:use-module (guix build utils)
> +  #:use-module (ice-9 match)
> +  #:use-module (ice-9 ftw)
> +  #:use-module (srfi srfi-1)
> +  #:use-module (srfi srfi-26)
> +  #:export (%standard-phases
> +            cargo-build))
> +
> +;; Commentary:
> +;;
> +;; Builder-side code of the standard Rust package build procedure.
> +;;
> +;; Code:
> +
> +(define* (configure #:rest _)
> +  "Replace Cargo.toml [dependencies] section with guix inputs."
> +  ;; TODO
> +  #t)

I don’t understand this.  Does this mean that currently dependencies are
always bundled?

> +(define* (build #:key (cargo-build-flags '("--release")) #:allow-other-keys)
> +  "Build a given Cargo package."
> +  (zero? (apply system* `("cargo" "build" ,@cargo-build-flags))))
> +
> +(define* (check #:rest _)
> +  "Run tests for a given Cargo package."
> +  (zero? (system* "cargo" "test")))

Shouldn’t this respect “test-target”?

> +
> +(define* (install #:key inputs outputs #:allow-other-keys)
> +  "Install a given Cargo package."
> +  (let* ((out (assoc-ref outputs "out"))
> +         (src (assoc-ref inputs "source"))
> +         (bin (string-append out "/bin"))
> +         (rsrc (string-append out "/rustsrc")))
> +    (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
> +    ;; distributing crates as source and replacing
> +    ;; references in Cargo.toml with store paths.
> +    (copy-recursively "src" (string-append rsrc "/src"))
> +    (install-file "Cargo.toml" rsrc)

I’m not familiar with Rust so I don’t know what crates are.  Are they
actually source files?  Are they archives?

You write that we are replacing references in Cargo.toml with store
paths but I see no evidence of this.  Could you please clarify?

> +    ;; When the package includes executables we install
> +    ;; it using cargo install. This fails when the crate
> +    ;; doesn't contain an executable.
> +    (system* "cargo" "install" "--root" bin)
> +    #t))

Why only use “cargo install” in case there are executables?  Can we
detect this by looking at some description file of the package?  It
doesn’t seem right to unconditionally end on #t.

~~ Ricardo