GNU bug report logs - #36841
[PATCH] build/cargo-build-system: Patch cargo checksums.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 36841 in the body.
You can then email your comments to 36841 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Efraim Flashner <efraim <at> flashner.co.il>
To: guix-patches <at> gnu.org
Cc: Efraim Flashner <efraim <at> flashner.co.il>
Subject: [PATCH] build/cargo-build-system: Patch cargo checksums.
Date: Mon, 29 Jul 2019 22:04:22 +0300

* guix/build/cargo-build-system.scm (patch-cargo-checksums): New phase.
(%standard-phases): Add 'patch-cargo-checksums after
'patch-generated-file-shebangs.
---
 guix/build/cargo-build-system.scm | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/guix/build/cargo-build-system.scm b/guix/build/cargo-build-system.scm
index f38de16cf7..8e1ee62f65 100644
--- a/guix/build/cargo-build-system.scm
+++ b/guix/build/cargo-build-system.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2016 David Craven <david <at> craven.ch>
 ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe <at> gmail.com>
 ;;; Copyright © 2019 Ivan Petkov <ivanppetkov <at> gmail.com>
+;;; Copyright © 2019 Efraim Flashner <efraim <at> flashner.co.il>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -121,6 +122,23 @@ directory = '" port)
   (setenv "CC" (string-append (assoc-ref inputs "gcc") "/bin/gcc"))
   #t)
 
+;; After patching the 'patch-generated-file-shebangs phase any vendored crates
+;; will have a mismatch on their checksum.
+(define* (patch-cargo-checksums #:key
+                                (vendor-dir "guix-vendor")
+                                #:allow-other-keys)
+  "Patch the checksums of the vendored crates after patching their shebangs."
+  (for-each
+    (lambda (filename)
+      (delete-file filename)
+      (let* ((dir (dirname filename)))
+        (display (string-append
+                   "patch-cargo-checksums: generate-checksums for "
+                   dir "\n"))
+        (generate-checksums dir)))
+    (find-files "guix-vendor" ".cargo-checksum.json"))
+  #t)
+
 (define* (build #:key
                 skip-build?
                 (cargo-build-flags '("--release"))
@@ -162,7 +180,8 @@ directory = '" port)
     (replace 'configure configure)
     (replace 'build build)
     (replace 'check check)
-    (replace 'install install)))
+    (replace 'install install)
+    (add-after 'patch-generated-file-shebangs 'patch-cargo-checksums patch-cargo-checksums)))
 
 (define* (cargo-build #:key inputs (phases %standard-phases)
                       #:allow-other-keys #:rest args)
-- 
2.22.0

Message #8 received at 36841 <at> debbugs.gnu.org (full text, mbox):

From: Ivan Petkov <ivanppetkov <at> gmail.com>
To: Efraim Flashner <efraim <at> flashner.co.il>
Cc: 36841 <at> debbugs.gnu.org
Subject: Re: [bug#36841] [PATCH] build/cargo-build-system: Patch cargo
 checksums.
Date: Mon, 29 Jul 2019 18:44:31 -0700

Hi Efraim,

> On Jul 29, 2019, at 12:04 PM, Efraim Flashner <efraim <at> flashner.co.il> wrote:
> 
> +;; After patching the 'patch-generated-file-shebangs phase any vendored crates
> +;; will have a mismatch on their checksum.
> +(define* (patch-cargo-checksums #:key
> +                                (vendor-dir "guix-vendor")
> +                                #:allow-other-keys)

[snip]

> +    (replace 'install install)
> +    (add-after 'patch-generated-file-shebangs 'patch-cargo-checksums patch-cargo-checksums)))

I can’t quite remember the order the phases run in off the top of my head. Would it be possible to
make the configure/checksum generation phase run after shebang-patching (or ensure the patching
happens first)? It would avoid having to checksum all the files twice that way…

—Ivan

Message #13 received at 36841-done <at> debbugs.gnu.org (full text, mbox):

From: Efraim Flashner <efraim <at> flashner.co.il>
To: Ivan Petkov <ivanppetkov <at> gmail.com>
Cc: 36841-done <at> debbugs.gnu.org
Subject: Re: [bug#36841] [PATCH] build/cargo-build-system: Patch cargo
 checksums.
Date: Tue, 30 Jul 2019 08:59:03 +0300

[Message part 1 (text/plain, inline)]

On Mon, Jul 29, 2019 at 06:44:31PM -0700, Ivan Petkov wrote:
> Hi Efraim,
> 
> > On Jul 29, 2019, at 12:04 PM, Efraim Flashner <efraim <at> flashner.co.il> wrote:
> > 
> > +;; After patching the 'patch-generated-file-shebangs phase any vendored crates
> > +;; will have a mismatch on their checksum.
> > +(define* (patch-cargo-checksums #:key
> > +                                (vendor-dir "guix-vendor")
> > +                                #:allow-other-keys)
> 
> [snip]
> 
> > +    (replace 'install install)
> > +    (add-after 'patch-generated-file-shebangs 'patch-cargo-checksums patch-cargo-checksums)))
> 
> I can’t quite remember the order the phases run in off the top of my head. Would it be possible to
> make the configure/checksum generation phase run after shebang-patching (or ensure the patching
> happens first)? It would avoid having to checksum all the files twice that way…
> 
> —Ivan

I thought about it a bit more after I sent the patch, and I'm pretty
sure this is only needed when there's a Cargo.lock file in the build
directory. So in actuality it should be more like:

(when (file-exists? "Cargo.lock")
  (begin
    (delete-file "Cargo.lock")
    (invoke "cargo" "generate-lockfile")
    (patch-cargo-checksums ...)))

I'm going to close this bug/patch and re-submit it when I've given it a
bit more work so it doesn't do the expensive compute-checksums
computation on all builds.

-- 
Efraim Flashner   <efraim <at> 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

[signature.asc (application/pgp-signature, inline)]

Message #16 received at 36841 <at> debbugs.gnu.org (full text, mbox):

From: Efraim Flashner <efraim <at> flashner.co.il>
To: Ivan Petkov <ivanppetkov <at> gmail.com>
Cc: 36841 <at> debbugs.gnu.org
Subject: Re: [bug#36841] [PATCH] build/cargo-build-system: Patch cargo
 checksums.
Date: Tue, 30 Jul 2019 11:17:57 +0300

[Message part 1 (text/plain, inline)]

On Mon, Jul 29, 2019 at 06:44:31PM -0700, Ivan Petkov wrote:
> Hi Efraim,
> 
> > On Jul 29, 2019, at 12:04 PM, Efraim Flashner <efraim <at> flashner.co.il> wrote:
> > 
> > +;; After patching the 'patch-generated-file-shebangs phase any vendored crates
> > +;; will have a mismatch on their checksum.
> > +(define* (patch-cargo-checksums #:key
> > +                                (vendor-dir "guix-vendor")
> > +                                #:allow-other-keys)
> 
> [snip]
> 
> > +    (replace 'install install)
> > +    (add-after 'patch-generated-file-shebangs 'patch-cargo-checksums patch-cargo-checksums)))
> 
> I can’t quite remember the order the phases run in off the top of my head. Would it be possible to
> make the configure/checksum generation phase run after shebang-patching (or ensure the patching
> happens first)? It would avoid having to checksum all the files twice that way…

The 'configure phase could be renamed the plop-vendored-crates-into-place
phase. It actually can't come after the 'patch-generated-file-shebangs
phase since then there won't be any vendored crates to patch.

If we remove the generate-checksums call from 'configure then there
won't be a .cargo-checksum.json to remove and regenerate during the
'patch-cargo-checksums phase, so I've changed that to search for
"Cargo.toml$" and not delete it. Not as robust as "for each top-level
directory in the 'vendor-dir'", but should be good enough.

-- 
Efraim Flashner   <efraim <at> 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

[0001-build-cargo-build-system-Patch-cargo-checksums.patch (text/plain, attachment)]

[signature.asc (application/pgp-signature, inline)]

Message #19 received at 36841 <at> debbugs.gnu.org (full text, mbox):

From: Efraim Flashner <efraim <at> flashner.co.il>
To: Ivan Petkov <ivanppetkov <at> gmail.com>
Cc: 36841 <at> debbugs.gnu.org
Subject: Re: [bug#36841] [PATCH v3] build/cargo-build-system: Patch cargo
 checksums.
Date: Tue, 30 Jul 2019 13:46:58 +0300

[Message part 1 (text/plain, inline)]

On Tue, Jul 30, 2019 at 11:17:57AM +0300, Efraim Flashner wrote:
> On Mon, Jul 29, 2019 at 06:44:31PM -0700, Ivan Petkov wrote:
> > Hi Efraim,
> > 
> > > On Jul 29, 2019, at 12:04 PM, Efraim Flashner <efraim <at> flashner.co.il> wrote:
> > > 
> > > +;; After patching the 'patch-generated-file-shebangs phase any vendored crates
> > > +;; will have a mismatch on their checksum.
> > > +(define* (patch-cargo-checksums #:key
> > > +                                (vendor-dir "guix-vendor")
> > > +                                #:allow-other-keys)
> > 
> > [snip]
> > 
> > > +    (replace 'install install)
> > > +    (add-after 'patch-generated-file-shebangs 'patch-cargo-checksums patch-cargo-checksums)))
> > 
> > I can’t quite remember the order the phases run in off the top of my head. Would it be possible to
> > make the configure/checksum generation phase run after shebang-patching (or ensure the patching
> > happens first)? It would avoid having to checksum all the files twice that way…
> 
> The 'configure phase could be renamed the plop-vendored-crates-into-place
> phase. It actually can't come after the 'patch-generated-file-shebangs
> phase since then there won't be any vendored crates to patch.
> 
> If we remove the generate-checksums call from 'configure then there
> won't be a .cargo-checksum.json to remove and regenerate during the
> 'patch-cargo-checksums phase, so I've changed that to search for
> "Cargo.toml$" and not delete it. Not as robust as "for each top-level
> directory in the 'vendor-dir'", but should be good enough.
> 
This one I'm pretty happy with. The checksums are only generated twice
when there's a Cargo.lock file present and I've factored out the
function to generate all the checksums. When that's moved to (guix build
cargo-utils) it can be used by the rust compilers and icecat.


-- 
Efraim Flashner   <efraim <at> 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

[0001-build-cargo-build-system-Patch-cargo-checksums.patch (text/plain, attachment)]

[signature.asc (application/pgp-signature, inline)]

Message #22 received at 36841 <at> debbugs.gnu.org (full text, mbox):

From: Ivan Petkov <ivanppetkov <at> gmail.com>
To: Efraim Flashner <efraim <at> flashner.co.il>
Cc: 36841 <at> debbugs.gnu.org
Subject: Re: [bug#36841] [PATCH v3] build/cargo-build-system: Patch cargo
 checksums.
Date: Wed, 31 Jul 2019 20:00:00 -0700

[Message part 1 (text/plain, inline)]

Hi Efraim,

> On Jul 30, 2019, at 3:46 AM, Efraim Flashner <efraim <at> flashner.co.il> wrote:
> 
> This one I'm pretty happy with. The checksums are only generated twice
> when there's a Cargo.lock file present and I've factored out the
> function to generate all the checksums. When that's moved to (guix build
> cargo-utils) it can be used by the rust compilers and icecat.

Overall the patch makes sense to me!

However, I am curious what are some of the situations in which you’re encountering
a Cargo.lock file? In a system like guix which maintains all dependencies immutably
and consistently, the Cargo.lock file is virtually useless (in fact it *could* be harmful
if an application is released with a Cargo.lock file pinning to a particular vulnerable
dependency which needs to be updated, requiring patching of the Cargo.lock file).

I’d be willing to go as far as suggest we unconditionally delete any Cargo.lock file
in source tarballs and let cargo generate its own replacement using the vendor
directory we have supplied. (Imports from crates.io <http://crates.io/> also never include a Cargo.lock
file, so this may only pertain if we’re performing a direct source import…)

—Ivan

[Message part 2 (text/html, inline)]

Message #25 received at 36841 <at> debbugs.gnu.org (full text, mbox):

From: Efraim Flashner <efraim <at> flashner.co.il>
To: Ivan Petkov <ivanppetkov <at> gmail.com>
Cc: 36841 <at> debbugs.gnu.org
Subject: Re: [bug#36841] [PATCH v3] build/cargo-build-system: Patch cargo
 checksums.
Date: Thu, 1 Aug 2019 14:15:26 +0300

[Message part 1 (text/plain, inline)]

On Wed, Jul 31, 2019 at 08:00:00PM -0700, Ivan Petkov wrote:
> Hi Efraim,
> 
> > On Jul 30, 2019, at 3:46 AM, Efraim Flashner <efraim <at> flashner.co.il> wrote:
> > 
> > This one I'm pretty happy with. The checksums are only generated twice
> > when there's a Cargo.lock file present and I've factored out the
> > function to generate all the checksums. When that's moved to (guix build
> > cargo-utils) it can be used by the rust compilers and icecat.
> 
> Overall the patch makes sense to me!
> 
> However, I am curious what are some of the situations in which you’re encountering
> a Cargo.lock file? In a system like guix which maintains all dependencies immutably
> and consistently, the Cargo.lock file is virtually useless (in fact it *could* be harmful
> if an application is released with a Cargo.lock file pinning to a particular vulnerable
> dependency which needs to be updated, requiring patching of the Cargo.lock file).

One is the package that I'm actually targeting, https://github.com/chfi/rust-qtlreaper/ ,
and three of the others are rust-regex and rust-compiler-builtins and
rust-env-logger. All three of them I got from $(guix import crate foo).
`guix import crate env-logger`, for example, returns this:
https://static.crates.io/crates/env_logger/env_logger-0.6.2.crate

> 
> I’d be willing to go as far as suggest we unconditionally delete any Cargo.lock file
> in source tarballs and let cargo generate its own replacement using the vendor
> directory we have supplied. (Imports from crates.io <http://crates.io/> also never include a Cargo.lock
> file, so this may only pertain if we’re performing a direct source import…)

This is basically what my 'update-cargo-lock phase does. Otherwise we
end up packaging arbitrary versions of crates to satisfy whatever
version they were using when they last updated their Cargo.lock.

> 
> —Ivan

-- 
Efraim Flashner   <efraim <at> 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

[signature.asc (application/pgp-signature, inline)]

Message #28 received at 36841-done <at> debbugs.gnu.org (full text, mbox):

From: Efraim Flashner <efraim <at> flashner.co.il>
To: Ivan Petkov <ivanppetkov <at> gmail.com>
Cc: 36841-done <at> debbugs.gnu.org
Subject: Re: [bug#36841] [PATCH v3] build/cargo-build-system: Patch cargo
 checksums.
Date: Sun, 4 Aug 2019 11:57:12 +0300

[Message part 1 (text/plain, inline)]

Pushed with an update to the documentation


-- 
Efraim Flashner   <efraim <at> 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

[signature.asc (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.