Subject: [bug#35155] [PATCH] build-system/cargo: refactor phases to successfully build

From: Ivan Petkov <[email-masked]>
Date: Sun, 7 Apr 2019 08:49:09 -0700

Hi Chris,

> On Apr 7, 2019, at 1:40 AM, Chris Marusich <[email-masked]> wrote:
> 
> Do you mean that if a crate X has an optional feature that requires
> crate Y, then Cargo requires Y to be present when building X even if X
> is being built with that feature disabled?

Correct, the source to crate Y must be present when crate X is being built,
transitively, or otherwise.

>> * If we have a “circular” dependency as part of dev-dependencies
>> (e.g. one crate pulls in an upstream crate during testing to ensure it
>> doesn’t break anything) we’ll need to detangle the dependency graph by
>> rewriting duplicate packages to include the #:skip-build? flag. I can
>> elaborate more on this in a separate email.
> 
> I think here, you're talking about the situation in which crate A
> depends on crate B, which in turn depends on crate A's source, and to
> break the cycle we will replace B's dependency on A with a dependency on
> A', where A' is effectively just a source build of A (via #:skip-build?
> #t).  Is that basically right?
> 
> I agree it would be good to discuss the resolution of circular
> dependencies in another email thread, but I just wanted to double check
> with you that my basic understanding of your intent is correct.

Yep, your understanding of the situation and potential solution is correct.
I’ll send out a different email thread around this since we might need some
small tweaks to the overall build system to support this.

>>> What about Cargo packages that are
>>> both libraries and applications?  Do those even exist?
>> 
>> Yes these are a bit rare but they do exist. I don’t have any examples on
>> hand, but you can have something akin to curl which can be used
>> as a binary, as well as imported as a library to other projects.
> 
> In those rare cases, your changes are still good to go, right?  We don't
> actually interact with the Cargo.lock file, and if there are any
> executables, your code will install them.

Yep, there’s no harm in building the “src” output of an application crate
(outside of using up some store space) in case something else wants to
depend on it.

> Something else occurred to me.  In packages of C libraries, such as the
> glibc package, we install libraries (e.g., ".so" files go into the "out"
> output, and ".a" files go into "static" output).  However, here we are
> not installing any libraries like that.  Do all Rust developers just use
> Cargo.toml files to declare their dependencies, and then let Cargo
> manage all the actual compiling and linking?  Do they ever manually
> install a Rust library (without using Cargo) when hacking on a project?

In general, cargo is used to perform all building an linking of the final rust
outputs (binaries, shared libraries, and other rust artifacts). Cargo also supports
defining arbitrary build scripts such as building some other non-rust dependency
which is to be linked in the final outputs.

https://doc.rust-lang.org/cargo/reference/build-scripts.html <https://doc.rust-lang.org/cargo/reference/build-scripts.html>

However, cargo does not perform the actual distribution of these “external”
dependencies. Crates may vendor their own external sources, or they may
expect them to be present in the usual places, or they may support an environment
variable which points them in the right direction.

One example is the `jemalloc-sys` crate. If built with an environment variable
pointing to a compiled version of jemalloc, it will build rust bindings which link
to that binary. Otherwise it will build its own vendored version of the jemalloc source.

Handling these packages will need to be done on a case-by-case basis
with some additional setup glue in the package definitions, as necessary.

>>>> -    (generate-checksums rsrc src)
>>>> +    (generate-checksums rsrc "/dev/null")
>>> 
>>> This probably deserves a short comment to clarify the intent.
>> 
>> Do you mean commenting on the intent of `generate-checksums`
>> or the intent of the /dev/null parameter?
> 
> I mean the "/dev/null" argument, mainly.  As far as I can tell, it looks
> like the generate-checksums procedure builds a file with checksums to
> satisfy some requirement of the cargo tool.  That seems reasonable, but
> I'm not sure why we use "/dev/null" here.

Cargo expects the checksum package to include a checksum of each individual
file as well as a checksum for the entire directory. The `generate-checksums`
procedure doesn’t correctly handle the second parameter being a directory
and raises an error. Luckily, cargo doesn’t seem to care about the contents
of this checksum, as long as the declaration is there.

I didn’t want to change the `generate-checksums` procedure just yet since
it’s used during building of rust-1.20, and doing so will require a full bootstrap
of the compiler chain, which would have blocked me for a while.

> Finally, two more minor comments about code style which I don't think
> you need to change but are good to know going forward:
> 
> - Instead of "system*", we prefer to use "invoke" (defined in (guix
>  build utils)) whenever possible.  It throws an exception when an error
>  occurs, so it's harder to accidentally ignore errors.
> 
> - Using "and" and "or" statements for control flow is OK, especially
>  since you're using "system*".  In fact, we do this in other parts of
>  Guix, too.  However, I personally feel that forms like "if", "when",
>  and "unless" are clearer in some cases and should probably be
>  preferred, especially when using "invoke" instead of "system*”.

Thank you for the tips, I’ll keep these in mind going forward.
Still pretty new to guile, its APIs, and guix’s utilities on top of that! :)

—Ivan