Subject: Re: [bug#78280] [PATCH rust-team] doc: Document lockfile importer based Rust packaging workflow.

From: Hilton Chain <[email-masked]>
Date: Thu, 08 May 2025 19:16:28 +0800

"Murilo" <[email-masked]> writes:

> Hi, a few suggestions.
>
> On Tue May 6, 2025 at 9:47 AM -03, Hilton Chain wrote:
>> --- a/doc/contributing.texi
>> +++ b/doc/contributing.texi
>
>> +In the Rust community it is common for multiple incompatible versions of a
> s/community/ecosystem/
> I think ecosystem is a better word here

It's a word to avoid[1], and I think it can apply here.

[1]: https://www.gnu.org/philosophy/words-to-avoid.html#Ecosystem

>> +specifically for some Rust applications, and can't simply identity them by
> s/identity/identify/
> Typo
>
>> +version.  In this case we can use a @code{for-@var{program}} suffix, for
> s/program/application/
> Just to keep it consistent with the previous definitions (binary
> crate)

I'll use ‘package’, since there might be development snapshot
dependencies for libraries as well.

>> --- a/doc/guix-cookbook.texi
>> +++ b/doc/guix-cookbook.texi
>
>> +The following sections are real life examples on working with specific build
> s/are real life/provide real-life/
> More cohesive and real-life as an adjective for the examples noun
>
>> +Since @code{cargo-audit} is available on crates.io, we can generate a template
>> +via the crates.io importer (@pxref{Invoking guix import,,, guix, GNU Guix
> How about adding the used command to import here?
> ...
>> +Reference Manual}).  After manual editing, we'll have the following definiton:
>
>> +The identifier used to invoke @code{cargo-inputs}, @code{'cargo-audit} here,
> The identifier used to invoke @code{cargo-inputs}, in this case @code{'cargo-audit},
>
>> +# Or use short options, in this case the shell processes pathes before passing
>> +# them to Guix, allowing expansion of @code{~}, for example.
>> +$ guix import -i gnu/packages/rust-crates.scm \
> $ guix import -i ~/my-local-guix/gnu/packages/rust-crates.scm \
> Could use an example with expanding '~', for extra clarification

I don't think this is necessary.

>> +At this stage, package @code{cargo-audit} is buildable.
> s/package/the package/
> more cohesive
>
>> +Finally we'll unbundle vendored dependencies.  The lockfile importer inserts
> s/unbundle/unbundle the/
> more cohesive
>
>> +To facility various tasks in the common workflow, several scripts are provided
> s/facility/facilitate/
> typo
>
>> +@code{niri} has Cargo workspace dependencies.  When packaging a Cargo workspace,
>> +parameter @code{#:cargo-package-crates} is required.
> I think here is a good place to mention how to indentify if an application has
> dependencies that are workspaces.
>
> My suggestion is something along these lines:
>
> ```
> We can see in the @code{niri} dependencies, inside @code{Cargo.toml}, that
> @code{pipewire} is being fetched from an external source, and by looking at
> @code{pipewire-rs}' @code{Cargo.toml}, we can see that @code{pipewire-rs} is a
> workspace. Thus, @code{niri} has Cargo workspace dependencies.  When packaging a
> Cargo workspace, the parameter @code{#:cargo-package-crates} is required.
> ```

‘guix import’ inserts TODO for development snapshots, Cargo workspaces
can be identified by build error.

> Also, is it really required for all Cargo workspaces? Or only required for
> "Cargo workspace dependencies" (the ones with #:skip-build? #t)?
> I think you mean `s/Cargo workspace,/Cargo workspace dependency,/`, but I'm not
> sure.

Yes, dependencies.

>> +To use our packaged development snapshots, it's also necessary to modify
>> +@file{Cargo.toml} in a build phase, with a package-specific substitution
>> +pattern.
> Could it be in the source snippet too? or is it more desirable to have it in
> a build phase?

When possible, we should make sure the result of ‘guix build --source’
has vendored dependencies unbundled, and buildable without Guix.

> Thank you for all the work and effort!

Thanks for the review!