From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
To: "Ludovic Courtès" <ludo@gnu.org>
Cc: Josselin Poiret <dev@jpoiret.xyz>,
Simon Tournier <zimon.toutoune@gmail.com>,
Mathieu Othacehe <othacehe@gnu.org>,
Tobias Geerinckx-Rice <me@tobias.gr>,
Ricardo Wurmus <rekado@elephly.net>,
65866@debbugs.gnu.org, Christopher Baines <guix@cbaines.net>
Subject: [bug#65866] [PATCH 0/8] Add built-in builder for Git checkouts
Date: Wed, 20 Sep 2023 13:50:09 -0400 [thread overview]
Message-ID: <87ttro4tv2.fsf_-_@gmail.com> (raw)
In-Reply-To: <2cd5b127be6d64e640e569f262cef3bbb89f58a6.1694441831.git.ludo@gnu.org> ("Ludovic Courtès"'s message of "Mon, 11 Sep 2023 16:25:25 +0200")
Hello!
Ludovic Courtès <ludo@gnu.org> writes:
> Fixes <https://issues.guix.gnu.org/63331>.
>
> Longer-term this will remove Git from the derivation graph when its sole
> use is to perform a checkout for a fixed-output derivation, thereby
> breaking dependency cycles that can arise in these situations.
>
> * guix/git-download.scm (git-fetch): Rename to…
> (git-fetch/in-band): … this. Deal with GIT or GUILE being #f.
Nitpick, but I find this usage of dynamic default argument on top of
default arguments inelegant; see my comments below for an
alternative.
> (git-fetch/built-in, built-in-builders*, git-fetch): New procedures.
> * tests/builders.scm ("git-fetch, file URI"): New test.
> ---
> guix/git-download.scm | 68 +++++++++++++++++++++++++++++++++++++------
> tests/builders.scm | 29 +++++++++++++++++-
> 2 files changed, 87 insertions(+), 10 deletions(-)
>
> diff --git a/guix/git-download.scm b/guix/git-download.scm
> index f1f19397c6..505dff0a89 100644
> --- a/guix/git-download.scm
> +++ b/guix/git-download.scm
> @@ -27,6 +27,7 @@ (define-module (guix git-download)
> #:use-module (guix records)
> #:use-module (guix packages)
> #:use-module (guix modules)
> + #:use-module ((guix derivations) #:select (raw-derivation))
> #:autoload (guix build-system gnu) (standard-packages)
> #:autoload (guix download) (%download-fallback-test)
> #:autoload (git bindings) (libgit2-init!)
> @@ -78,15 +79,19 @@ (define (git-package)
> (let ((distro (resolve-interface '(gnu packages version-control))))
> (module-ref distro 'git-minimal)))
>
> -(define* (git-fetch ref hash-algo hash
> - #:optional name
> - #:key (system (%current-system)) (guile (default-guile))
> - (git (git-package)))
> - "Return a fixed-output derivation that fetches REF, a <git-reference>
> -object. The output is expected to have recursive hash HASH of type
> -HASH-ALGO (a symbol). Use NAME as the file name, or a generic name if #f."
> +(define* (git-fetch/in-band ref hash-algo hash
> + #:optional name
> + #:key (system (%current-system))
> + (guile (default-guile))
> + (git (git-package)))
> + "Return a fixed-output derivation that performs a Git checkout of REF, using
> +GIT and GUILE (thus, said derivation depends on GIT and GUILE).
> +
> +This method is deprecated in favor of the \"builtin:git-download\" builder.
> +It will be removed when versions of guix-daemon implementing
> +\"builtin:git-download\" will be sufficiently widespread."
> (define inputs
> - `(("git" ,git)
> + `(("git" ,(or git (git-package)))
Instead of using 'or' here to ensure git has a value, the default values
should have been copied to the new definition of git-fetch.
>
> ;; When doing 'git clone --recursive', we need sed, grep, etc. to be
> ;; available so that 'git submodule' works.
> @@ -154,7 +159,8 @@ (define* (git-fetch ref hash-algo hash
> #:recursive? recursive?
> #:git-command "git")))))
>
> - (mlet %store-monad ((guile (package->derivation guile system)))
> + (mlet %store-monad ((guile (package->derivation (or guile (default-guile))
> + system)))
> (gexp->derivation (or name "git-checkout") build
>
> ;; Use environment variables and a fixed script name so
> @@ -181,6 +187,50 @@ (define* (git-fetch ref hash-algo hash
> #:recursive? #t
> #:guile-for-build guile)))
>
> +(define* (git-fetch/built-in ref hash-algo hash
> + #:optional name
> + #:key (system (%current-system)))
> + "Return a fixed-output derivation without any dependency that performs a Git
> +checkout of REF, using the \"builtin:git-download\" derivation builder."
> + (raw-derivation (or name "git-checkout") "builtin:git-download" '()
> + #:system system
> + #:hash-algo hash-algo
> + #:hash hash
> + #:recursive? #t
> + #:env-vars
> + `(("url" . ,(object->string
> + (match (%download-fallback-test)
> + ('content-addressed-mirrors
> + "https://example.org/does-not-exist")
> + (_
> + (git-reference-url ref)))))
> + ("commit" . ,(git-reference-commit ref))
> + ("recursive?" . ,(object->string
> + (git-reference-recursive? ref))))
> + #:leaked-env-vars '("http_proxy" "https_proxy"
> + "LC_ALL" "LC_MESSAGES" "LANG"
> + "COLUMNS")
> + #:local-build? #t))
> +
> +(define built-in-builders*
> + (store-lift built-in-builders))
> +
> +(define* (git-fetch ref hash-algo hash
> + #:optional name
> + #:key (system (%current-system))
> + guile git)
As mentioned above, I'd have kept the default values for guile and git
here.
> + "Return a fixed-output derivation that fetches REF, a <git-reference>
> +object. The output is expected to have recursive hash HASH of type
> +HASH-ALGO (a symbol). Use NAME as the file name, or a generic name if #f."
> + (mlet %store-monad ((builtins (built-in-builders*)))
> + (if (member "git-download" builtins)
> + (git-fetch/built-in ref hash-algo hash name
> + #:system system)
> + (git-fetch/in-band ref hash-algo hash name
> + #:system system
> + #:guile guile
> + #:git git))))
> +
> (define (git-version version revision commit)
> "Return the version string for packages using git-download."
> ;; git-version is almost exclusively executed while modules are being loaded.
> diff --git a/tests/builders.scm b/tests/builders.scm
> index 0b5577c7a3..619caa5f31 100644
> --- a/tests/builders.scm
> +++ b/tests/builders.scm
> @@ -1,5 +1,5 @@
> ;;; GNU Guix --- Functional package management for GNU
> -;;; Copyright © 2012, 2013, 2014, 2015, 2018, 2019, 2021 Ludovic Courtès <ludo@gnu.org>
> +;;; Copyright © 2012-2015, 2018-2019, 2021, 2023 Ludovic Courtès <ludo@gnu.org>
> ;;; Copyright © 2021 Lars-Dominik Braun <lars@6xq.net>
> ;;;
> ;;; This file is part of GNU Guix.
> @@ -20,6 +20,7 @@
>
> (define-module (tests builders)
> #:use-module (guix download)
> + #:use-module (guix git-download)
> #:use-module (guix build-system)
> #:use-module (guix build-system gnu)
> #:use-module (guix build gnu-build-system)
> @@ -31,9 +32,12 @@ (define-module (tests builders)
> #:use-module (guix base32)
> #:use-module (guix derivations)
> #:use-module (gcrypt hash)
> + #:use-module ((guix hash) #:select (file-hash*))
> #:use-module (guix tests)
> + #:use-module (guix tests git)
> #:use-module (guix packages)
> #:use-module (gnu packages bootstrap)
> + #:use-module ((ice-9 ftw) #:select (scandir))
> #:use-module (ice-9 match)
> #:use-module (ice-9 textual-ports)
> #:use-module (srfi srfi-1)
> @@ -84,6 +88,29 @@ (define url-fetch*
> (and (file-exists? out)
> (valid-path? %store out))))
>
> +(test-equal "git-fetch, file URI"
> + '("." ".." "a.txt" "b.scm")
> + (let ((nonce (random-text)))
> + (with-temporary-git-repository directory
> + `((add "a.txt" ,nonce)
> + (add "b.scm" "#t")
> + (commit "Commit.")
> + (tag "v1.0.0" "The tag."))
> + (run-with-store %store
> + (mlet* %store-monad ((hash
> + -> (file-hash* directory
> + #:algorithm (hash-algorithm sha256)
> + #:recursive? #t))
> + (drv (git-fetch
> + (git-reference
> + (url (string-append "file://" directory))
> + (commit "v1.0.0"))
> + 'sha256 hash
> + "git-fetch-test")))
> + (mbegin %store-monad
> + (built-derivations (list drv))
> + (return (scandir (derivation->output-path drv)))))))))
> +
> (test-assert "gnu-build-system"
> (build-system? gnu-build-system))
Pretty neat test! LGTM. You can add a 'Reviewed-by:' git trailer in
Magit easily with 'C-u C-c C-r' :-)
--
Thanks,
Maxim
next prev parent reply other threads:[~2023-09-20 18:07 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-11 14:23 [bug#65866] [PATCH 0/8] Add built-in builder for Git checkouts Ludovic Courtès
2023-09-11 14:25 ` [bug#65866] [PATCH 1/8] git-download: Move fallback code to (guix build git) Ludovic Courtès
2023-09-20 16:05 ` [bug#65866] [PATCH 0/8] Add built-in builder for Git checkouts Maxim Cournoyer
2023-09-20 16:40 ` Simon Tournier
2023-09-22 21:53 ` Ludovic Courtès
2023-09-11 14:25 ` [bug#65866] [PATCH 2/8] git-download: Honor the ‘GUIX_DOWNLOAD_FALLBACK_TEST’ environment variable Ludovic Courtès
2023-09-20 16:07 ` [bug#65866] [PATCH 0/8] Add built-in builder for Git checkouts Maxim Cournoyer
2023-09-11 14:25 ` [bug#65866] [PATCH 3/8] perform-download: Remove unused one-argument clause Ludovic Courtès
2023-09-20 16:09 ` [bug#65866] [PATCH 0/8] Add built-in builder for Git checkouts Maxim Cournoyer
2023-09-11 14:25 ` [bug#65866] [PATCH 4/8] daemon: Add “git-download” built-in builder Ludovic Courtès
2023-09-20 17:32 ` [bug#65866] [PATCH 0/8] Add built-in builder for Git checkouts Maxim Cournoyer
2023-09-21 7:42 ` Ludovic Courtès
2023-09-22 22:27 ` [bug#65866] [PATCH v2 " Ludovic Courtès
2023-09-22 22:27 ` [bug#65866] [PATCH v2 1/8] git-download: Move fallback code to (guix build git) Ludovic Courtès
2023-09-25 8:15 ` Simon Tournier
2023-09-25 9:24 ` [bug#65866] [PATCH 0/8] Add built-in builder for Git checkouts Ludovic Courtès
2023-09-25 12:13 ` Simon Tournier
2023-09-22 22:27 ` [bug#65866] [PATCH v2 2/8] git-download: Honor the ‘GUIX_DOWNLOAD_FALLBACK_TEST’ environment variable Ludovic Courtès
2023-09-22 22:27 ` [bug#65866] [PATCH v2 3/8] perform-download: Remove unused one-argument clause Ludovic Courtès
2023-09-22 22:28 ` [bug#65866] [PATCH v2 4/8] daemon: Add “git-download” built-in builder Ludovic Courtès
2023-09-22 22:28 ` [bug#65866] [PATCH v2 5/8] build: Add dependency on Git Ludovic Courtès
2023-09-25 13:59 ` Simon Tournier
2023-09-26 14:05 ` [bug#65866] Bootstrapping without the daemon and all that Ludovic Courtès
2023-09-26 17:04 ` Simon Tournier
2023-10-12 10:54 ` Ludovic Courtès
2023-10-16 8:46 ` Simon Tournier
2023-09-22 22:28 ` [bug#65866] [PATCH v2 6/8] perform-download: Use the ‘git’ command captured at configure time Ludovic Courtès
2023-09-22 22:28 ` [bug#65866] [PATCH v2 7/8] git-download: Use “builtin:git-download” when available Ludovic Courtès
2023-09-25 8:33 ` Simon Tournier
2023-09-25 9:23 ` [bug#65866] [PATCH 0/8] Add built-in builder for Git checkouts Ludovic Courtès
2023-09-25 12:37 ` Simon Tournier
2023-09-25 12:48 ` Simon Tournier
2023-09-25 15:49 ` Maxim Cournoyer
2023-09-26 15:44 ` bug#65866: " Ludovic Courtès
2023-09-26 17:13 ` [bug#65866] " Simon Tournier
2023-10-01 15:02 ` Ludovic Courtès
2023-10-16 9:11 ` [bug#65866] Toward RFC? (was Re: [bug#65866] [PATCH 0/8] Add built-in builder for Git checkouts) Simon Tournier
2023-10-30 15:12 ` Ludovic Courtès
2023-09-22 22:28 ` [bug#65866] [PATCH v2 8/8] tests: Assume ‘git’ is always available Ludovic Courtès
2023-09-11 14:25 ` [bug#65866] [PATCH 5/8] build: Add dependency on Git Ludovic Courtès
2023-09-20 17:57 ` [bug#65866] [PATCH 0/8] Add built-in builder for Git checkouts Maxim Cournoyer
2023-09-22 22:00 ` Ludovic Courtès
2023-09-25 15:59 ` Maxim Cournoyer
2023-09-11 14:25 ` [bug#65866] [PATCH 6/8] perform-git-download: Use the ‘git’ command captured at configure time Ludovic Courtès
2023-09-20 17:34 ` [bug#65866] [PATCH 0/8] Add built-in builder for Git checkouts Maxim Cournoyer
2023-09-11 14:25 ` [bug#65866] [PATCH 7/8] git-download: Use “builtin:git-download” when available Ludovic Courtès
2023-09-20 17:50 ` Maxim Cournoyer [this message]
2023-09-22 21:58 ` [bug#65866] [PATCH 0/8] Add built-in builder for Git checkouts Ludovic Courtès
2023-09-25 15:56 ` Maxim Cournoyer
2023-09-11 14:25 ` [bug#65866] [PATCH 8/8] tests: Assume ‘git’ is always available Ludovic Courtès
2023-09-20 17:59 ` [bug#65866] [PATCH 0/8] Add built-in builder for Git checkouts Maxim Cournoyer
[not found] <877ctljs0m.fsf@inria.fr>
[not found] ` <87pm2osrot.fsf@gnu.org>
2023-09-11 15:16 ` [bug#65866] bug#63331: Guile-GnuTLS/Git circular dependency and built-in git checkouts Vivien Kraus via Guix-patches via
2023-09-11 20:57 ` [bug#65866] [PATCH 0/8] Add built-in builder for Git checkouts Ludovic Courtès
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=87ttro4tv2.fsf_-_@gmail.com \
--to=maxim.cournoyer@gmail.com \
--cc=65866@debbugs.gnu.org \
--cc=dev@jpoiret.xyz \
--cc=guix@cbaines.net \
--cc=ludo@gnu.org \
--cc=me@tobias.gr \
--cc=othacehe@gnu.org \
--cc=rekado@elephly.net \
--cc=zimon.toutoune@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).