Re: [PATCH 08/12] build-system: Add cargo build system.

[Ludovic Courtès]

David Craven  skribis:

>>> +(define* (configure #:rest empty)
>>> +  "Replace Cargo.toml [dependencies] section with guix inputs."
>>> +  ;;TODO
>>
>> So what would this do?  Do we end up using bundled dependencies if we
>> don’t do that?
>
> If we don't do that cargo tries to download them from crates.io, and
> since it's running in a container, would fail.

OK, fine.

>>> +(define* (build #:rest empty)
>>> +  "Build a given Cargo package."
>>> +  (zero? (system* "cargo" "build" "--release")))
>>
>> It may be useful to make "--release" configurable, like the #:build-type
>> of ‘cmake-build-system’.
>
>> Why do we need to install the source code?
>>
>> If it’s really needed for the functioning of the package, I’d suggest
>> moving it to OUT/share/rust-source/PACKAGE-VERSION or something like
>> this, no?
>
> Rust doesn't have a fixed ABI yet, so cargo builds everything from
> source. There is precedent in other distros (I think debian) and
> languages (go).

So Cargo *rebuilds* dependencies, even if they were built with the exact
same compiler?

In our case it would be great if we could disable that because we can
ensure that we are indeed building using a single compiler.

Thoughts?

>>> +(install-file "Cargo.toml" rsrc)
>>> +;; Will fail if crate doesn't contain an executable
>>> +(system* "cargo" "install" "--root" bin)
>>
>> I suppose many crates provides a library and no executable, so it’d be
>> nice to find what needs to be done here.  Thoughts?
>
> if we replace all dependencies with local ones, this isn't an issue.

I was referring to the comment above; what happens if we run “cargo
install --root BIN” on a package that does not contains executables?

Thanks,
Ludo’.

Re: [PATCH 08/12] build-system: Add cargo build system.

[David Craven]

>> +(define* (configure #:rest empty)
>> +  "Replace Cargo.toml [dependencies] section with guix inputs."
>> +  ;;TODO
>
> So what would this do?  Do we end up using bundled dependencies if we
> don’t do that?

If we don't do that cargo tries to download them from crates.io, and
since it's running in a container, would fail.

>> +(define* (build #:rest empty)
>> +  "Build a given Cargo package."
>> +  (zero? (system* "cargo" "build" "--release")))
>
> It may be useful to make "--release" configurable, like the #:build-type
> of ‘cmake-build-system’.

> Why do we need to install the source code?
>
> If it’s really needed for the functioning of the package, I’d suggest
> moving it to OUT/share/rust-source/PACKAGE-VERSION or something like
> this, no?

Rust doesn't have a fixed ABI yet, so cargo builds everything from
source. There is precedent in other distros (I think debian) and
languages (go).

>> +(install-file "Cargo.toml" rsrc)
>> +;; Will fail if crate doesn't contain an executable
>> +(system* "cargo" "install" "--root" bin)
>
> I suppose many crates provides a library and no executable, so it’d be
> nice to find what needs to be done here.  Thoughts?

if we replace all dependencies with local ones, this isn't an issue.

Re: [PATCH 08/12] build-system: Add cargo build system.

[Ludovic Courtès]

David Craven  skribis:

> * 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.

[...]

> +;; Commentary:
> +;;
> +;; Builder-side code of the standard Python package build procedure.

s/Python/Rust/  :-)

> +(define* (configure #:rest empty)
> +  "Replace Cargo.toml [dependencies] section with guix inputs."
> +  ;;TODO

So what would this do?  Do we end up using bundled dependencies if we
don’t do that?

> +(define* (build #:rest empty)
> +  "Build a given Cargo package."
> +  (zero? (system* "cargo" "build" "--release")))

It may be useful to make "--release" configurable, like the #:build-type
of ‘cmake-build-system’.

Also, ‘empty’ is a confusing variable name here, because it’s definitely
a non-empty list; maybe simply ‘rest’ or ‘_’?

> +(define* (check #:rest empty)

Ditto.

> +(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)
> +(copy-recursively "src" (string-append rsrc "/src"))

Why do we need to install the source code?

If it’s really needed for the functioning of the package, I’d suggest
moving it to OUT/share/rust-source/PACKAGE-VERSION or something like
this, no?

> +(install-file "Cargo.toml" rsrc)
> +;; Will fail if crate doesn't contain an executable
> +(system* "cargo" "install" "--root" bin)

I suppose many crates provides a library and no executable, so it’d be
nice to find what needs to be done here.  Thoughts?

The rest LGTM.

Thank you!

Ludo’.

[PATCH 08/12] build-system: Add cargo build system.

[David Craven]

* 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.
---
 guix/build-system/cargo.scm   | 135 ++
 guix/build/cargo-build-system.scm |  74 +
 2 files changed, 209 insertions(+)
 create mode 100644 guix/build-system/cargo.scm
 create mode 100644 guix/build/cargo-build-system.scm

diff --git a/guix/build-system/cargo.scm b/guix/build-system/cargo.scm
new file mode 100644
index 000..795d3b2
--- /dev/null
+++ b/guix/build-system/cargo.scm
@@ -0,0 +1,135 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2016 David Craven 
+;;;
+;;; 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 .
+
+(define-module (guix build-system cargo)
+  #:use-module (guix search-paths)
+  #:use-module (guix store)
+  #:use-module (guix utils)
+  #:use-module (guix derivations)
+  #:use-module (guix packages)
+  #:use-module (guix build-system)
+  #:use-module (guix build-system gnu)
+  #:use-module (ice-9 match)
+  #:export (cargo-build-system))
+
+(define (default-cargo)
+  "Return the default Cargo package."
+  ;; Lazily resolve the binding to avoid a circular dependency.
+  (let ((rust (resolve-interface '(gnu packages rust
+(module-ref rust 'cargo-bootstrap)))
+
+(define (default-rustc)
+  "Return the default Rustc package."
+  ;; Lazily resolve the binding to avoid a circular dependency.
+  (let ((rust (resolve-interface '(gnu packages rust
+(module-ref rust 'rustc-bootstrap)))
+
+(define %cargo-build-system-modules
+  ;; Build-side modules imported by default.
+  `((guix build cargo-build-system)
+,@%gnu-build-system-modules))
+
+(define* (cargo-build store name inputs
+  #:key
+  (tests? #t)
+  (test-target "test")
+  (configure-flags ''())
+  (phases '(@ (guix build cargo-build-system)
+  %standard-phases))
+  (outputs '("out"))
+  (search-paths '())
+  (system (%current-system))
+  (guile #f)
+  (imported-modules %cargo-build-system-modules)
+  (modules '((guix build cargo-build-system)
+ (guix build utils
+  "Build SOURCE using CARGO, and with INPUTS."
+
+  (define builder
+`(begin
+   (use-modules ,@modules)
+   (cargo-build #:name ,name
+#:source ,(match (assoc-ref inputs "source")
+(((? derivation? source))
+ (derivation->output-path source))
+((source)
+ source)
+(source
+ source))
+#:configure-flags ,configure-flags
+#:system ,system
+#:test-target ,test-target
+#:tests? ,tests?
+#:phases ,phases
+#:outputs %outputs
+#:search-paths ',(map search-path-specification->sexp
+  search-paths)
+#:inputs %build-inputs)))
+
+  (define guile-for-build
+(match guile
+  ((? package?)
+   (package-derivation store guile system #:graft? #f))
+  (#f ; the default
+   (let* ((distro (resolve-interface '(gnu packages commencement)))
+  (guile  (module-ref distro 'guile-final)))
+ (package-derivation store guile system #:graft? #f)
+
+  (build-expression->derivation store name builder
+#:inputs inputs
+#:system system
+#:modules imported-modules
+#:outputs outputs
+#:guile-for-build guile-for-build))
+
+(define* (lower name
+#:key source inputs native-inputs outputs system target
+(cargo