From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([209.51.188.92]:49641) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hDA3Y-0001tS-TP for guix-patches@gnu.org; Sun, 07 Apr 2019 11:50:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hDA3W-0003fK-Qc for guix-patches@gnu.org; Sun, 07 Apr 2019 11:50:04 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:35067) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hDA3W-0003fC-FO for guix-patches@gnu.org; Sun, 07 Apr 2019 11:50:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1hDA3W-0006ie-Br for guix-patches@gnu.org; Sun, 07 Apr 2019 11:50:02 -0400 Subject: [bug#35155] [PATCH] build-system/cargo: refactor phases to successfully build Resent-Message-ID: Content-Type: multipart/alternative; boundary="Apple-Mail=_27ECDF33-0D30-49F1-AD48-71AC0A46AEA0" Mime-Version: 1.0 (Mac OS X Mail 11.5 \(3445.9.1\)) From: Ivan Petkov In-Reply-To: <877ec6jjic.fsf@gmail.com> Date: Sun, 7 Apr 2019 08:49:09 -0700 Message-Id: References: <87k1g6zpd9.fsf@gmail.com> <51B2A728-850F-4A13-8A6B-D12FD0B0C119@gmail.com> <877ec6jjic.fsf@gmail.com> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: Chris Marusich Cc: 35155@debbugs.gnu.org --Apple-Mail=_27ECDF33-0D30-49F1-AD48-71AC0A46AEA0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Hi Chris, > On Apr 7, 2019, at 1:40 AM, Chris Marusich = wrote: >=20 > 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 =E2=80=9Ccircular=E2=80=9D dependency as part of = dev-dependencies >> (e.g. one crate pulls in an upstream crate during testing to ensure = it >> doesn=E2=80=99t break anything) we=E2=80=99ll 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. >=20 > 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? >=20 > 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=E2=80=99ll 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? >>=20 >> Yes these are a bit rare but they do exist. I don=E2=80=99t 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. >=20 > 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=E2=80=99s no harm in building the =E2=80=9Csrc=E2=80=9D = 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 = However, cargo does not perform the actual distribution of these = =E2=80=9Cexternal=E2=80=9D 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") >>>=20 >>> This probably deserves a short comment to clarify the intent. >>=20 >> Do you mean commenting on the intent of `generate-checksums` >> or the intent of the /dev/null parameter? >=20 > 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=E2=80=99t correctly handle the second parameter being a = directory and raises an error. Luckily, cargo doesn=E2=80=99t seem to care about = the contents of this checksum, as long as the declaration is there. I didn=E2=80=99t want to change the `generate-checksums` procedure just = yet since it=E2=80=99s 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: >=20 > - 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. >=20 > - 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*=E2=80=9D.= Thank you for the tips, I=E2=80=99ll keep these in mind going forward. Still pretty new to guile, its APIs, and guix=E2=80=99s utilities on top = of that! :) =E2=80=94Ivan --Apple-Mail=_27ECDF33-0D30-49F1-AD48-71AC0A46AEA0 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 Hi = Chris,

On Apr 7, 2019, at 1:40 AM, Chris Marusich = <cmmarusich@gmail.com> 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 =E2=80=9Ccircular=E2=80=9D dependency as part of = dev-dependencies
(e.g. one crate pulls in an upstream = crate during testing to ensure it
doesn=E2=80=99t break = anything) we=E2=80=99ll 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=E2=80=99ll 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=E2=80=99t 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=E2=80=99s no harm in building the =E2=80=9Csrc=E2=80=9D 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.





- =    (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=E2=80= =99t correctly handle the second parameter being a = directory
and raises an error. Luckily, cargo doesn=E2=80=99t = seem to care about the contents
of this checksum, as long as = the declaration is there.

I didn=E2=80= =99t want to change the `generate-checksums` procedure just yet = since
it=E2=80=99s 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*=E2=80=9D.

Thank you for the tips, I=E2=80=99ll keep these in mind = going forward.

= --Apple-Mail=_27ECDF33-0D30-49F1-AD48-71AC0A46AEA0--