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

[Ludovic Courtès]

David Craven <address@hidden> 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’.