| Message ID | 20190730104658.GC21431@E2140 |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#36841,v3] build/cargo-build-system: Patch cargo checksums. | expand |
Hi Efraim, > On Jul 30, 2019, at 3:46 AM, Efraim Flashner <efraim@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
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@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
Pushed with an update to the documentation
diff --git a/guix/build/cargo-build-system.scm b/guix/build/cargo-build-system.scm index f38de16cf7..7d363a18a5 100644 --- a/guix/build/cargo-build-system.scm +++ b/guix/build/cargo-build-system.scm @@ -2,6 +2,7 @@ ;;; Copyright © 2016 David Craven <david@craven.ch> ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com> ;;; Copyright © 2019 Ivan Petkov <ivanppetkov@gmail.com> +;;; Copyright © 2019 Efraim Flashner <efraim@flashner.co.il> ;;; ;;; This file is part of GNU Guix. ;;; @@ -39,6 +40,21 @@ ;; ;; Code: +;; TODO: Move this to (guix build cargo-utils). Will cause a full rebuild +;; of all rust compilers. + +(define (generate-all-checksums dir-name) + (for-each + (lambda (filename) + (let* ((dir (dirname filename)) + (checksum-file (string-append dir "/.cargo-checksum.json"))) + (when (file-exists? checksum-file) (delete-file checksum-file)) + (display (string-append + "patch-cargo-checksums: generate-checksums for " + dir "\n")) + (generate-checksums dir))) + (find-files dir-name "Cargo.toml$"))) + (define (manifest-targets) "Extract all targets from the Cargo.toml manifest" (let* ((port (open-input-pipe "cargo read-manifest")) @@ -94,8 +110,7 @@ Cargo.toml file present at its root." ;; so that we can generate any cargo checksums. ;; The --strip-components argument is needed to prevent creating ;; an extra directory within `crate-dir`. - (invoke "tar" "xvf" path "-C" crate-dir "--strip-components" "1") - (generate-checksums crate-dir))))) + (invoke "tar" "xvf" path "-C" crate-dir "--strip-components" "1"))))) inputs) ;; Configure cargo to actually use this new directory. @@ -121,6 +136,31 @@ directory = '" port) (setenv "CC" (string-append (assoc-ref inputs "gcc") "/bin/gcc")) #t) +;; The Cargo.lock file tells the build system which crates are required for +;; building and hardcodes their version and checksum. In order to build with +;; the inputs we provide, we need to recreate the file with our inputs. +(define* (update-cargo-lock #:key + (vendor-dir "guix-vendor") + #:allow-other-keys) + "Regenerate the Cargo.lock file with the current build inputs." + (when (file-exists? "Cargo.lock") + (begin + ;; Unfortunately we can't generate a Cargo.lock file until the checksums + ;; are generated, so we have an extra round of generate-all-checksums here. + (generate-all-checksums vendor-dir) + (delete-file "Cargo.lock") + (invoke "cargo" "generate-lockfile"))) + #t) + +;; After the 'patch-generated-file-shebangs phase any vendored crates who have +;; their shebangs patched 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." + (generate-all-checksums vendor-dir) + #t) + (define* (build #:key skip-build? (cargo-build-flags '("--release")) @@ -162,7 +202,9 @@ directory = '" port) (replace 'configure configure) (replace 'build build) (replace 'check check) - (replace 'install install))) + (replace 'install install) + (add-after 'configure 'update-cargo-lock update-cargo-lock) + (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)