Subject: Re: [bug#35318] [PATCH] Update cargo-build-system to expand package inputs

From: Ludovic Courtès <[email-masked]>
Date: Mon, 20 May 2019 21:38:36 +0200

Hi Ivan!

Ivan Petkov <[email-masked]> 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 <[email-masked]>
> 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’.