rust-build-system: Unvendor *-sys libraries in phase?

rust-build-system: Unvendor *-sys libraries in phase?

[John Soo]

Hi guix,

After working on a few rust packages, it looks like there could be another step on the process.  There are a number of libraries in the crates registry that wrap and vendor c libraries - libgit2, openssl, or jemalloc for example. The wrapping libraries, for reference, are usually called <c-library-name>-sys - libgit2-sys for example.

In the current rust-build-system, the onus of removing the vendored code falls on the ultimate consumer of the library. So, for instance, cbindgen would have to define a phase to remove the vendored code in a *-sys library even if that library is a transitive dependency.

What would you all think about adding a phase to the standard phases of the rust-build-system? I’m imagining some phase that would delete the vendored code and performed the other necessary steps to use the c libraries in /gnu/store.

Wdyt?

John

Re: rust-build-system: Unvendor *-sys libraries in phase?

[Efraim Flashner]

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

On Fri, Jan 24, 2020 at 01:49:21PM -0800, John Soo wrote:
> Hi guix,
> 
> After working on a few rust packages, it looks like there could be another step on the process.  There are a number of libraries in the crates registry that wrap and vendor c libraries - libgit2, openssl, or jemalloc for example. The wrapping libraries, for reference, are usually called <c-library-name>-sys - libgit2-sys for example.
> 
> In the current rust-build-system, the onus of removing the vendored code falls on the ultimate consumer of the library. So, for instance, cbindgen would have to define a phase to remove the vendored code in a *-sys library even if that library is a transitive dependency.
> 
> What would you all think about adding a phase to the standard phases of the rust-build-system? I’m imagining some phase that would delete the vendored code and performed the other necessary steps to use the c libraries in /gnu/store.
> 
> Wdyt?

IMO the correct way to do it would be in the crate source that we
download. We regularly add snippets to remove vendored code, this should
be no different. The only real difference is that right now the
cargo-build-system is "too smart" and checks the #:cargo-inputs to make
sure they actually are crates before untarring them and putting them in
the guix-vendor dor. Currently the 'patch-and-rezip phase always uses xz
for compression, and crates always use gzip, so we cannot use patches or
snippets on crates. IMO the easiest way around this is to
unconditionally untar anything in #:cargo-inputs into guix-vendor/ and
continue on.

Then the only thing left is to set certain environment variables, some
of which can probably be moved to the cargo-build-system if they don't
require an actual path. LIB(GIT|SSH)2_SYS_USE_PKG_CONFIG can move,
JEMALLOC_OVERRIDE and LIBCLANG_PATH can't.

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

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

Re: rust-build-system: Unvendor *-sys libraries in phase?

[John Soo]

Hi Efraim,


> IMO the correct way to do it would be in the crate source that we
> download. We regularly add snippets to remove vendored code, this should
> be no different.

Totally agree. It seems like a challenge to me to do the other required work since all the building happens only when building the top level package. I will totally defer to you here, since you have the knowledge and experience. 

> The only real difference is that right now the
> cargo-build-system is "too smart" and checks the #:cargo-inputs to make
> sure they actually are crates before untarring them and putting them in
> the guix-vendor dor. Currently the 'patch-and-rezip phase always uses xz
> for compression, and crates always use gzip, so we cannot use patches or
> snippets on crates.

Can that be fixed in any way? I actually have no idea. 

> Then the only thing left is to set certain environment variables, some
> of which can probably be moved to the cargo-build-system if they don't
> require an actual path.

Ahh right. I hadn’t realized that restriction. Though couldn’t end variables with paths be thunked or g-expressions or be done through propagated paths?  I’m not sure, just thinking aloud.

Thanks for your maintenance, as always.

John

Re: rust-build-system: Unvendor *-sys libraries in phase?

[Efraim Flashner]

[-- Attachment #1.1: Type: text/plain, Size: 1950 bytes --]

On Sat, Jan 25, 2020 at 11:01:31AM -0800, John Soo wrote:
> 
> Hi Efraim,
> 
> 
> > IMO the correct way to do it would be in the crate source that we
> > download. We regularly add snippets to remove vendored code, this should
> > be no different.
> 
> Totally agree. It seems like a challenge to me to do the other required work since all the building happens only when building the top level package. I will totally defer to you here, since you have the knowledge and experience. 
> 
> > The only real difference is that right now the
> > cargo-build-system is "too smart" and checks the #:cargo-inputs to make
> > sure they actually are crates before untarring them and putting them in
> > the guix-vendor dor. Currently the 'patch-and-rezip phase always uses xz
> > for compression, and crates always use gzip, so we cannot use patches or
> > snippets on crates.
> 
> Can that be fixed in any way? I actually have no idea. 
> 

I didn't mean to actually fix it, but it seems that just eliminating
directories is enough to make it work.

I've attached a simple diff against cargo-build-system and rust-libz-sys
and rust-libgit2-sys which removes the bundled source from both crates
and builds rust-libgit2-sys without complaints.

> > Then the only thing left is to set certain environment variables, some
> > of which can probably be moved to the cargo-build-system if they don't
> > require an actual path.
> 
> Ahh right. I hadn’t realized that restriction. Though couldn’t end variables with paths be thunked or g-expressions or be done through propagated paths?  I’m not sure, just thinking aloud.

This one I'm not sure about

> 
> Thanks for your maintenance, as always.
> 
> John

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #1.2: allow-patched-cargo-sources.diff --]
[-- Type: text/plain, Size: 4756 bytes --]

diff --git a/gnu/packages/crates-io.scm b/gnu/packages/crates-io.scm
index 8155bd7a94..ff2f9c52ef 100644
--- a/gnu/packages/crates-io.scm
+++ b/gnu/packages/crates-io.scm
@@ -5603,10 +5603,13 @@ values of all the exported APIs match the platform that libc is compiled for.")
       (origin
         (method url-fetch)
         (uri (crate-uri "libgit2-sys" version))
-        (file-name (string-append name "-" version ".crate"))
+        (file-name (string-append name "-" version ".tar.gz"))
         (sha256
          (base32
-          "0l9fvki7qxsl97vgzqwlv75nl213a5vxw7b1jaik97ala356pv6r"))))
+          "0l9fvki7qxsl97vgzqwlv75nl213a5vxw7b1jaik97ala356pv6r"))
+        (modules '((guix build utils)))
+        (snippet
+         '(begin (delete-file-recursively "libgit2") #t))))
     (build-system cargo-build-system)
     (arguments
      `(#:cargo-inputs
@@ -5623,15 +5626,15 @@ values of all the exported APIs match the platform that libc is compiled for.")
            (lambda* (#:key inputs #:allow-other-keys)
              (let ((openssl (assoc-ref inputs "openssl")))
                (setenv "OPENSSL_DIR" openssl))
-             (delete-file-recursively "libgit2")
-             (delete-file-recursively
-               (string-append "guix-vendor/rust-libgit2-sys-"
-                              ,(package-version rust-libgit2-sys-0.10)
-                              ".crate/libgit2"))
-             (delete-file-recursively
-               (string-append "guix-vendor/rust-libz-sys-"
-                              ,(package-version rust-libz-sys-1.0)
-                              ".crate/src/zlib"))
+             ;(delete-file-recursively "libgit2")
+             ;(delete-file-recursively
+             ;  (string-append "guix-vendor/rust-libgit2-sys-"
+             ;                 ,(package-version rust-libgit2-sys-0.10)
+             ;                 ".crate/libgit2"))
+             ;(delete-file-recursively
+             ;  (string-append "guix-vendor/rust-libz-sys-"
+             ;                 ,(package-version rust-libz-sys-1.0)
+             ;                 ".crate/src/zlib"))
              (delete-file-recursively
                (string-append "guix-vendor/rust-libssh2-sys-"
                               ,(package-version rust-libssh2-sys-0.2)
@@ -6591,10 +6594,13 @@ types as proposed in RFC 1158.")
       (origin
         (method url-fetch)
         (uri (crate-uri "libz-sys" version))
-        (file-name (string-append name "-" version ".crate"))
+        (file-name (string-append name "-" version ".tar.gz"))
         (sha256
          (base32
-          "1gjycyl2283525abks98bhxa4r259m617xfm5z52p3p3c8ry9d9f"))))
+          "1gjycyl2283525abks98bhxa4r259m617xfm5z52p3p3c8ry9d9f"))
+        (modules '((guix build utils)))
+        (snippet
+         '(begin (delete-file-recursively "src/zlib") #t))))
     (build-system cargo-build-system)
     (arguments
      `(#:cargo-inputs
@@ -6603,16 +6609,17 @@ types as proposed in RFC 1158.")
         ("rust-cc" ,rust-cc-1.0)
         ("rust-pkg-config" ,rust-pkg-config-0.3)
         ("rust-vcpkg" ,rust-vcpkg-0.2))
-       #:phases
-       (modify-phases %standard-phases
-         (add-after 'configure 'delete-vendored-zlib
-           (lambda _
-             (delete-file-recursively "src/zlib")
-             (delete-file-recursively
-               (string-append "guix-vendor/rust-libz-sys-"
-                              ,(package-version rust-libz-sys-1.0)
-                              ".crate/src/zlib"))
-             #t)))))
+       ;#:phases
+       ;(modify-phases %standard-phases
+       ;  (add-after 'configure 'delete-vendored-zlib
+       ;    (lambda _
+       ;      (delete-file-recursively "src/zlib")
+       ;      (delete-file-recursively
+       ;        (string-append "guix-vendor/rust-libz-sys-"
+       ;                       ,(package-version rust-libz-sys-1.0)
+       ;                       ".crate/src/zlib"))
+       ;      #t)))
+       ))
     (native-inputs
      `(("pkg-config" ,pkg-config)
        ("zlib" ,zlib)))
diff --git a/guix/build/cargo-build-system.scm b/guix/build/cargo-build-system.scm
index 8a8d74ee1b..c2046e5f5a 100644
--- a/guix/build/cargo-build-system.scm
+++ b/guix/build/cargo-build-system.scm
@@ -58,7 +58,7 @@
 (define (crate-src? path)
   "Check if PATH refers to a crate source, namely a gzipped tarball with a
 Cargo.toml file present at its root."
-    (and (gzip-file? path)
+    (and (not (directory-exists? path)) ; not a tarball
          ;; First we print out all file names within the tarball to see if it
          ;; looks like the source of a crate. However, the tarball will include
          ;; an extra path component which we would like to ignore (since we're

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

Re: rust-build-system: Unvendor *-sys libraries in phase?

[John Soo]

Hi Efraim,

> I didn't mean to actually fix it, but it seems that just eliminating
> directories is enough to make it work.
>
> I've attached a simple diff against cargo-build-system and rust-libz-sys
> and rust-libgit2-sys which removes the bundled source from both crates
> and builds rust-libgit2-sys without complaints.

This looks good. Can the phases doing the same thing in the arguments
of libgit, etc. be removed too?

Re: rust-build-system: Unvendor *-sys libraries in phase?

[Efraim Flashner]

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

On Mon, Jan 27, 2020 at 02:44:13PM +0000, John Soo wrote:
> Hi Efraim,
> 
> > I didn't mean to actually fix it, but it seems that just eliminating
> > directories is enough to make it work.
> >
> > I've attached a simple diff against cargo-build-system and rust-libz-sys
> > and rust-libgit2-sys which removes the bundled source from both crates
> > and builds rust-libgit2-sys without complaints.
> 
> This looks good. Can the phases doing the same thing in the arguments
> of libgit, etc. be removed too?

I have a set of patches locally to unbundle a bunch of the other -sys
libraries and move the PKG_CONFIG environment variables to
cargo-build-system. I'll go ahead and push them after I make sure the
different packages build.

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

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