[PATCH 1/2] gnu: Add cargo.

[PATCH 1/2] gnu: Add cargo.

[David Craven]

* gnu/packages/rust.scm (cargo): New variable.
---
 gnu/packages/rust.scm | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/gnu/packages/rust.scm b/gnu/packages/rust.scm
index 8aa867317..c6c4f3171 100644
--- a/gnu/packages/rust.scm
+++ b/gnu/packages/rust.scm
@@ -23,11 +23,15 @@
   #:use-module (gnu packages bootstrap)
   #:use-module (gnu packages cmake)
   #:use-module (gnu packages compression)
+  #:use-module (gnu packages curl)
   #:use-module (gnu packages elf)
   #:use-module (gnu packages gcc)
   #:use-module (gnu packages jemalloc)
   #:use-module (gnu packages llvm)
+  #:use-module (gnu packages pkg-config)
   #:use-module (gnu packages python)
+  #:use-module (gnu packages ssh)
+  #:use-module (gnu packages tls)
   #:use-module (gnu packages version-control)
   #:use-module (guix build-system gnu)
   #:use-module (guix build-system trivial)
@@ -265,3 +269,54 @@ safety and thread safety guarantees.")
     (home-page "https://www.rust-lang.org")
     ;; Dual licensed.
     (license (list license:asl2.0 license:expat))))
+
+(define-public cargo
+  (package
+    (name "cargo")
+    (version (cargo-version (rustc-version %rust-bootstrap-binaries-version)))
+    (source (origin
+              (method url-fetch)
+              ;; Use a cargo tarball with vendored dependencies and a cargo
+              ;; config file.
+              (uri (string-append
+                    "https://github.com/dvc94ch/cargo"
+                    "/archive/" version "-cargo-vendor.tar.gz"))
+              (file-name (string-append name "-" version ".tar.gz"))
+              (sha256
+               (base32
+                "0hpix5hwz10pm1wh65gimhsy9nxjvy7yikgbpw8afwglqr3bl856"))))
+    (build-system cargo-build-system)
+    (inputs
+     `(("cmake" ,cmake)
+       ("curl" ,curl)
+       ("libgit2" ,libgit2)
+       ("libssh2" ,libssh2)
+       ("openssl" ,openssl)
+       ("pkg-config" ,pkg-config)
+       ("python-2" ,python-2)
+       ("zlib" ,zlib)))
+    (arguments
+     `(#:cargo ,cargo-bootstrap
+       #:tests? #f ; FIXME
+       #:phases
+       (modify-phases %standard-phases
+         ;; Avoid cargo complaining about missmatched checksums.
+         (delete 'patch-source-shebangs)
+         (delete 'patch-generated-file-shebangs)
+         (delete 'patch-usr-bin-file)
+         ;; Set CARGO_HOME to use the vendored dependencies.
+         (add-after 'unpack 'set-cargo-home
+           (lambda* (#:key inputs #:allow-other-keys)
+             (let* ((gcc (assoc-ref inputs "gcc"))
+                    (cc (string-append gcc "/bin/gcc")))
+               (setenv "CARGO_HOME" (string-append (getcwd) "/cargohome"))
+               (setenv "CMAKE_C_COMPILER" cc)
+               (setenv "CC" cc))
+             #t)))))
+    (home-page "https://github.com/rust-lang/cargo")
+    (synopsis "Build tool and package manager for Rust")
+    (description "Cargo downloads your Rust project’s dependencies and compiles
+your project.")
+    ;; Cargo is dual licensed Apache and MIT. Also contains
+    ;; code from openssl which is GPL2 with linking exception.
+    (license (list license:asl2.0 license:expat license:gpl2+))))
-- 
2.11.0

[PATCH 2/2] build-system: cargo: Use correct cargo.

[David Craven]

* gnu/packages/rust.scm (cargo-bootstrap): Make private.
* guix/build-system/cargo.scm (default-cargo): Use cargo.
* guix/build/cargo-build-system.scm (install): Pass correct path to
  --root.
---
 gnu/packages/rust.scm             | 4 +---
 guix/build-system/cargo.scm       | 3 +--
 guix/build/cargo-build-system.scm | 2 +-
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/gnu/packages/rust.scm b/gnu/packages/rust.scm
index c6c4f3171..215030a7e 100644
--- a/gnu/packages/rust.scm
+++ b/gnu/packages/rust.scm
@@ -121,9 +121,7 @@
 which can in turn be used to build the final Rust compiler.")
     (license license:asl2.0)))
 
-;; FIXME: Make private once cargo is packaged. Is currently used by the
-;; cargo-build-system.
-(define-public cargo-bootstrap
+(define cargo-bootstrap
   (package
     (name "cargo-bootstrap")
     (version (cargo-version %rust-bootstrap-binaries-version))
diff --git a/guix/build-system/cargo.scm b/guix/build-system/cargo.scm
index 8d835dda1..1ee28bcfe 100644
--- a/guix/build-system/cargo.scm
+++ b/guix/build-system/cargo.scm
@@ -46,8 +46,7 @@ to NAME and VERSION."
   "Return the default Cargo package."
   ;; Lazily resolve the binding to avoid a circular dependency.
   (let ((rust (resolve-interface '(gnu packages rust))))
-    ;; FIXME: Package cargo and replace cargo-bootstrap with cargo.
-    (module-ref rust 'cargo-bootstrap)))
+    (module-ref rust 'cargo)))
 
 (define (default-rustc)
   "Return the default Rustc package."
diff --git a/guix/build/cargo-build-system.scm b/guix/build/cargo-build-system.scm
index 460d829b3..6357812f9 100644
--- a/guix/build/cargo-build-system.scm
+++ b/guix/build/cargo-build-system.scm
@@ -85,7 +85,7 @@
     ;; When the package includes executables we install
     ;; it using cargo install. This fails when the crate
     ;; doesn't contain an executable.
-    (system* "cargo" "install" "--root" bin)
+    (system* "cargo" "install" "--root" out)
     #t))
 
 (define %standard-phases
-- 
2.11.0

Re: [PATCH 1/2] gnu: Add cargo.

[Chris Marusich]

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

David Craven <david@craven.ch> writes:

> +(define-public cargo
> +  (package
> +    (name "cargo")
> +    (version (cargo-version (rustc-version %rust-bootstrap-binaries-version)))
> +    (source (origin
> +              (method url-fetch)
> +              ;; Use a cargo tarball with vendored dependencies and a cargo

What does "vendored dependencies" mean?  Are there dependencies included
in this tarball which are not managed via Guix?

> +    (inputs
> +     `(("cmake" ,cmake)
> +       ("curl" ,curl)
> +       ("libgit2" ,libgit2)
> +       ("libssh2" ,libssh2)
> +       ("openssl" ,openssl)
> +       ("pkg-config" ,pkg-config)
> +       ("python-2" ,python-2)
> +       ("zlib" ,zlib)))

Should some of these be native-inputs?  Like cmake or pkg-config?

> +    (arguments
> +     `(#:cargo ,cargo-bootstrap
> +       #:tests? #f ; FIXME

Why don't the tests work?

> +       #:phases
> +       (modify-phases %standard-phases
> +         ;; Avoid cargo complaining about missmatched checksums.
> +         (delete 'patch-source-shebangs)
> +         (delete 'patch-generated-file-shebangs)
> +         (delete 'patch-usr-bin-file)

Don't we have to patch the shebangs to make them work?

> +         ;; Set CARGO_HOME to use the vendored dependencies.
> +         (add-after 'unpack 'set-cargo-home
> +           (lambda* (#:key inputs #:allow-other-keys)
> +             (let* ((gcc (assoc-ref inputs "gcc"))
> +                    (cc (string-append gcc "/bin/gcc")))
> +               (setenv "CARGO_HOME" (string-append (getcwd) "/cargohome"))
> +               (setenv "CMAKE_C_COMPILER" cc)
> +               (setenv "CC" cc))
> +             #t)))))
> +    (home-page "https://github.com/rust-lang/cargo")
> +    (synopsis "Build tool and package manager for Rust")
> +    (description "Cargo downloads your Rust project’s dependencies and compiles
> +your project.")

Perhaps we can give a better description?  The guide has more info;
maybe we can adapt some of that info to a more detailed description:

http://doc.crates.io/guide.html

> +    ;; Cargo is dual licensed Apache and MIT. Also contains
> +    ;; code from openssl which is GPL2 with linking exception.
> +    (license (list license:asl2.0 license:expat license:gpl2+))))

Your comment says "GPL2" but the code says "gpl2+".  Doesn't "gpl2+"
mean "GPL2 or later"?  I don't think those are the same thing.

-- 
Chris

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

Re: [PATCH 1/2] gnu: Add cargo.

[David Craven]

> What does "vendored dependencies" mean?  Are there dependencies included
> in this tarball which are not managed via Guix?

Rust crates are included as source in the vendor subfolder. It uses
the cargo vendor subcommand [0] for building cargo offline. I don't
think that people want this merged, I only published it in case
someone is interested and as an example of how you could deploy a
webapplication written in rust.

[0] https://github.com/alexcrichton/cargo-vendor

> Should some of these be native-inputs?  Like cmake or pkg-config?

Cargo is a build tool, it uses cmake and pkg-config at runtime to
check for c dependencies and compile c code (like for building the
bundled llvm in rustc).

guix gc --references /gnu/store/g88cylsl8x8d4jvpqgaas1dwws77d2c1-cargo-0.15.0
/gnu/store/4pnp5scrgmjp21flaihmgbckwrz6z4g3-libgcrypt-1.7.3
/gnu/store/a64w9dq219a8d9k4mfz76mnzph9wsvfj-zlib-1.2.8
/gnu/store/cdi08kw7r6r684w8mk0xq0dkgpjhfpmd-gcc-4.9.4-lib
/gnu/store/iwgi9001dmmihrjg4rqhd6pa6788prjw-glibc-2.24
/gnu/store/k9rds2a8fbk7qn8dak5y6lwmyq7ds437-libssh2-1.7.0
/gnu/store/liib5wid6rx9rkss78spc7wcqzwb1g2k-openssl-1.0.2j
/gnu/store/nblsf2i13xvxwsy947wmf7zyv6viadwl-curl-7.50.3

> Why don't the tests work?

Haven't looked into it yet, I thought ng0 wanted to experiment with
the rust build system and needed an up-to-date cargo.

> Don't we have to patch the shebangs to make them work?

Nope. Patching shebangs at this stage is only needed for the build to
work. The build works, all scripts that end up in the output directory
are patched after the install phase.

> Perhaps we can give a better description?  The guide has more info;
> maybe we can adapt some of that info to a more detailed description:

Sure.

> Your comment says "GPL2" but the code says "gpl2+".  Doesn't "gpl2+"
> mean "GPL2 or later"?  I don't think those are the same thing.

Ah yes, it is only GPL2.

I'd also need to import the cargo-build-system in this commit.

Re: [PATCH 1/2] gnu: Add cargo.

[Chris Marusich]

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

David Craven <david@craven.ch> writes:

>> What does "vendored dependencies" mean?  Are there dependencies included
>> in this tarball which are not managed via Guix?
>
> Rust crates are included as source in the vendor subfolder. It uses
> the cargo vendor subcommand [0] for building cargo offline. I don't
> think that people want this merged, I only published it in case
> someone is interested and as an example of how you could deploy a
> webapplication written in rust.
>
> [0] https://github.com/alexcrichton/cargo-vendor

I'm not sure I follow.  I downloaded

  https://github.com/dvc94ch/cargo/archive/0.15.0-cargo-vendor.tar.gz

and checked out its "vendor" directory - it looks like there's a lot of
source code for other packages included right in there.  I understand
that this is some sort of mechanism to enable cargo to build things
offline, but is it appropriate to put all of those other packages'
source into the "cargo" package that is going to be built by Guix?  I
haven't been following the Cargo/Rust in Guix discussion thread, so it's
entirely possible I just missed something that was already discussed.

>> Should some of these be native-inputs?  Like cmake or pkg-config?
>
> Cargo is a build tool, it uses cmake and pkg-config at runtime to
> check for c dependencies and compile c code (like for building the
> bundled llvm in rustc).
>
> guix gc --references /gnu/store/g88cylsl8x8d4jvpqgaas1dwws77d2c1-cargo-0.15.0
> /gnu/store/4pnp5scrgmjp21flaihmgbckwrz6z4g3-libgcrypt-1.7.3
> /gnu/store/a64w9dq219a8d9k4mfz76mnzph9wsvfj-zlib-1.2.8
> /gnu/store/cdi08kw7r6r684w8mk0xq0dkgpjhfpmd-gcc-4.9.4-lib
> /gnu/store/iwgi9001dmmihrjg4rqhd6pa6788prjw-glibc-2.24
> /gnu/store/k9rds2a8fbk7qn8dak5y6lwmyq7ds437-libssh2-1.7.0
> /gnu/store/liib5wid6rx9rkss78spc7wcqzwb1g2k-openssl-1.0.2j
> /gnu/store/nblsf2i13xvxwsy947wmf7zyv6viadwl-curl-7.50.3

OK.  That makes sense.  But I wonder why cmake and pkg-config are
missing from the list of references if that is the case?

>> Why don't the tests work?
>
> Haven't looked into it yet, I thought ng0 wanted to experiment with
> the rust build system and needed an up-to-date cargo.

OK.

>> Don't we have to patch the shebangs to make them work?
>
> Nope. Patching shebangs at this stage is only needed for the build to
> work. The build works, all scripts that end up in the output directory
> are patched after the install phase.

I see.

> I'd also need to import the cargo-build-system in this commit.

I tried applying these two patches and, yes, I got an error because
cargo-build-system wasn't present.

-- 
Chris

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

Re: [PATCH 1/2] gnu: Add cargo.

[David Craven]

> I understand that this is some sort of mechanism to enable cargo to build things
> offline, but is it appropriate to put all of those other packages'
> source into the "cargo" package that is going to be built by Guix?  I
> haven't been following the Cargo/Rust in Guix discussion thread, so it's
> entirely possible I just missed something that was already discussed.

No it is not appropriate. There has been a lot of discussion if it is ok to take
shortcuts or not, I think the general opinion is that it is not ok.
But it is a way to
build cargo without a lot of work, since there are a couple of issues that need
to be addressed by the cargo-build-system, specifically the importer.

> OK.  That makes sense.  But I wonder why cmake and pkg-config are
> missing from the list of references if that is the case?

Wow, I don't know what I was thinking, I was convinced that cmake and pkg-config
where retained inputs :) They should be propagated-inputs then... Thanks!

Re: [PATCH 1/2] gnu: Add cargo.

[Chris Marusich]

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

David Craven <david@craven.ch> writes:

>> I understand that this is some sort of mechanism to enable cargo to build things
>> offline, but is it appropriate to put all of those other packages'
>> source into the "cargo" package that is going to be built by Guix?  I
>> haven't been following the Cargo/Rust in Guix discussion thread, so it's
>> entirely possible I just missed something that was already discussed.
>
> No it is not appropriate. There has been a lot of discussion if it is ok to take
> shortcuts or not, I think the general opinion is that it is not ok.
> But it is a way to
> build cargo without a lot of work, since there are a couple of issues that need
> to be addressed by the cargo-build-system, specifically the importer.

I see.  Since I haven't been involved in the discussion so far, I think
I'll defer to others for further comment on that topic.

>> OK.  That makes sense.  But I wonder why cmake and pkg-config are
>> missing from the list of references if that is the case?
>
> Wow, I don't know what I was thinking, I was convinced that cmake and pkg-config
> where retained inputs :) They should be propagated-inputs
> then... Thanks!

Although making them propagated-inputs might work, another solution
might be to use 'wrap-program' to wrap cargo's executable(s).  For
example, this is what we did when we packaged Asunder:

https://lists.gnu.org/archive/html/guix-devel/2016-12/msg00707.html

Wrapping the program might not be appropriate in every case (see email
thread discussion), but if it's appropriate in this case, it's a great
way to avoid propagated-inputs.

-- 
Chris

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

Re: [PATCH 1/2] gnu: Add cargo.

[Ludovic Courtès]

Hi!

Chris Marusich <cmmarusich@gmail.com> skribis:

> David Craven <david@craven.ch> writes:
>
>>> I understand that this is some sort of mechanism to enable cargo to build things
>>> offline, but is it appropriate to put all of those other packages'
>>> source into the "cargo" package that is going to be built by Guix?  I
>>> haven't been following the Cargo/Rust in Guix discussion thread, so it's
>>> entirely possible I just missed something that was already discussed.
>>
>> No it is not appropriate. There has been a lot of discussion if it is ok to take
>> shortcuts or not, I think the general opinion is that it is not ok.
>> But it is a way to
>> build cargo without a lot of work, since there are a couple of issues that need
>> to be addressed by the cargo-build-system, specifically the importer.
>
> I see.  Since I haven't been involved in the discussion so far, I think
> I'll defer to others for further comment on that topic.
>
>>> OK.  That makes sense.  But I wonder why cmake and pkg-config are
>>> missing from the list of references if that is the case?
>>
>> Wow, I don't know what I was thinking, I was convinced that cmake and pkg-config
>> where retained inputs :) They should be propagated-inputs
>> then... Thanks!
>
> Although making them propagated-inputs might work, another solution
> might be to use 'wrap-program' to wrap cargo's executable(s).  For
> example, this is what we did when we packaged Asunder:
>
> https://lists.gnu.org/archive/html/guix-devel/2016-12/msg00707.html
>
> Wrapping the program might not be appropriate in every case (see email
> thread discussion), but if it's appropriate in this case, it's a great
> way to avoid propagated-inputs.

Agreed, though that shouldn’t block the patch.

David, could you address this cmake/pkg-config issue (one way or the
other) and add answers to some of the questions Chris asked as comments
in the code?  LGTM with these changes!

Thank you!

Ludo’.

Re: [PATCH 2/2] build-system: cargo: Use correct cargo.

[Ludovic Courtès]

David Craven <david@craven.ch> skribis:

> * gnu/packages/rust.scm (cargo-bootstrap): Make private.
> * guix/build-system/cargo.scm (default-cargo): Use cargo.
> * guix/build/cargo-build-system.scm (install): Pass correct path to
>   --root.

Please push once patch #1 is in.  Thanks!

Ludo’.