unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: Chris Marusich <cmmarusich@gmail.com>
To: Ivan Petkov <ivanppetkov@gmail.com>
Cc: 35318@debbugs.gnu.org
Subject: [bug#35318] [PATCH] Update cargo-build-system to expand package inputs
Date: Sat, 08 Jun 2019 11:44:55 -0700	[thread overview]
Message-ID: <87ftoj9as8.fsf@gmail.com> (raw)
In-Reply-To: <6A0A0108-A722-4D73-85B7-E61AB8230026@gmail.com> (Ivan Petkov's message of "Sun, 19 May 2019 18:00:01 -0700")

[-- Attachment #1: Type: text/plain, Size: 3516 bytes --]

Hi Ivan,

Sorry for taking so long to review this.  In short, I think these
changes are good, and if nobody has more feedback in the next few days,
I will merge it and we can see how it works in practice!

At a high level, I think these changes have the following pros and cons:

Pros:

    - We can now build Rust crates.  Fantastic!
    - The "dependencies" and "dev-dependencies" terminology is the same
      as Cargo uses, so it will be familiar to Rust programmers.
    - We avoid O(N^2) build time, where N is the number of dependencies.
    - We can use the importer to quickly import new crates and iterate.

Cons:

    - Because the "dependencies" and "dev-dependencies" are specified as
      package arguments instead of any kind of "input", they won't show
      up in some of the graphs produced by "guix package".  However, in
      theory "guix graph" could be taught to display nodes for crate
      "dependencies" and "dev-dependencies", too.
      
    - Everyone who defines a Guix package for a crate must make sure the
      origin's file name ends in ".cargo", since the cargo-build-system
      now assumes that any input ending in ".cargo" is a crate that
      should be extracted into the build's crate-dir.  This is a little
      brittle, and I wish we had a better way to check this, but I can't
      think of a better way at the moment.  Since you've updated the
      importer to always add this, it probably won't be much of an issue
      in practice, since that's the default way new crates will be added
      to Guix.  Going forward, maybe we can avoid this by just checking
      the inputs to see which ones are gzipped tarballs containing
      Cargo.toml files.

Limitations imposed by Rust/Cargo itself:

    - I'm not a Rust or Cargo expert, but my current understanding is
      that it isn't feasible to save the artifacts produced when
      building a crate for re-use when building another crate.  In the
      world of C, it is common to produce a library, and then link
      against that library when building other software later.  In Guix,
      when building a C library, we install the built artifacts (e.g.,
      .so files) into the output, so that those artifacts can be used as
      the input for another package's build.  It seems that, by Cargo's
      design, it isn't currently feasible to do the same sort of thing
      with Cargo: that is, it isn't feasible to build artifacts, install
      them somewhere for later use, and then later re-use them in
      another Cargo build.  I'd be glad to learn that I'm mistaken, but
      currently that is my understanding.

    - Related to this, I doubt that a Rust programmer will be able to
      invoke a command like "guix environment my-crate" (even if we
      teach it to understand crate "dependencies" and
      "dev-dependencies") to make all the dependencies required to build
      my-crate available.  If a Rust programmer wants to hack on
      my-crate, they'll probably still just use "cargo" to do it without
      using Guix at all.  Is there any way to avoid this and make it
      possible to get the dependencies used by Guix in the build, so
      that a Rust programmer can hack around using precisely those
      dependencies?  If this were C or Python, you could do that using
      "guix environment," but I'm not sure how this could work with Rust
      crates.

Thanks again for working on this!

-- 
Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

  parent reply	other threads:[~2019-06-08 18:46 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-19  5:34 [bug#35318] [PATCH] Update cargo-build-system to expand package inputs Ivan Petkov
2019-05-04 16:40 ` Ivan Petkov
2019-05-04 18:31   ` Danny Milosavljevic
2019-05-04 21:09     ` Ivan Petkov
2019-05-06  8:00 ` Ludovic Courtès
2019-05-06 16:04   ` Ivan Petkov
2019-05-09 23:17     ` Ivan Petkov
2019-05-15  6:08       ` Ivan Petkov
2019-05-15 12:44         ` Ludovic Courtès
2019-05-20  1:00           ` Ivan Petkov
2019-05-20 19:38             ` Ludovic Courtès
2019-05-22  2:48               ` Ivan Petkov
2019-06-08 18:44             ` Chris Marusich [this message]
2019-06-08 23:33               ` Ivan Petkov
2019-06-09 23:53                 ` Ivan Petkov
2019-06-12  1:14                   ` bug#35318: " Chris Marusich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ftoj9as8.fsf@gmail.com \
    --to=cmmarusich@gmail.com \
    --cc=35318@debbugs.gnu.org \
    --cc=ivanppetkov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).