[PATCH] guix: git: Adds feature to download git repository to the store.

OpenSubmitted by jgart.
Details
4 participants
  • Sarah Morgensen
  • jgart
  • Ludovic Courtès
  • Maxime Devos
Owner
unassigned
Severity
normal
J
(address . guix-patches@gnu.org)(name . Julien Lepiller)(address . julien@lepiller.eu)
20210830163918.19419-1-jgart@dismail.de
From: Julien Lepiller <julien@lepiller.eu>
* guix/git.scm (download-git-to-store): Download Git repository fromURL at COMMIT to STORE, either under NAME or URL's basename if omitted.Write progress reports to LOG. RECURSIVE? has the same effect as thesame-named parameter of 'git-fetch'.
* guix/scripts/download.scm (download-git-to-store*): Adds cli option.Examples:guix download --git-commit=v0.1.1 github.com/anaseto/gruid-tcellguix download -c v0.1.1 https://github.com/anaseto/gruid-tcell--- guix/git.scm | 24 +++++++++++++++++- guix/scripts/download.scm | 51 ++++++++++++++++++++++++++++++++------- 2 files changed, 65 insertions(+), 10 deletions(-)
Toggle diff (164 lines)diff --git a/guix/git.scm b/guix/git.scmindex 9c6f326c36..4c70782b97 100644--- a/guix/git.scm+++ b/guix/git.scm@@ -28,6 +28,7 @@ #:use-module (gcrypt hash) #:use-module ((guix build utils) #:select (mkdir-p delete-file-recursively))+ #:use-module ((guix build git) #:select (git-fetch)) #:use-module (guix store) #:use-module (guix utils) #:use-module (guix records)@@ -43,6 +44,7 @@ #:use-module (srfi srfi-11) #:use-module (srfi srfi-34) #:use-module (srfi srfi-35)+ #:use-module (web uri) #:export (%repository-cache-directory honor-system-x509-certificates! @@ -61,7 +63,9 @@ git-checkout-url git-checkout-branch git-checkout-commit- git-checkout-recursive?))+ git-checkout-recursive?++ download-git-to-store)) (define %repository-cache-directory (make-parameter (string-append (cache-directory #:ensure? #f)@@ -614,6 +618,24 @@ objects: 'ancestor (meaning that OLD is an ancestor of NEW), 'descendant, or #:recursive? recursive? #:log-port (current-error-port))))) +(define* (download-git-to-store store url commit+ #:optional (name (basename url))+ #:key (log (current-error-port)) recursive?)+ "Download Git repository from URL at COMMIT to STORE, either under NAME or+URL's basename if omitted. Write progress reports to LOG. RECURSIVE? has the+same effect as the same-named parameter of 'git-fetch'."+ (define uri+ (string->uri url))++ (call-with-temporary-directory+ (lambda (temp)+ (let ((result+ (parameterize ((current-output-port log))+ (git-fetch url commit temp+ #:recursive? recursive?))))+ (and result+ (add-to-store store name #t "sha256" temp))))))+ ;; Local Variables: ;; eval: (put 'with-repository 'scheme-indent-function 2) ;; End:diff --git a/guix/scripts/download.scm b/guix/scripts/download.scmindex 5a91390358..6253ecaa5c 100644--- a/guix/scripts/download.scm+++ b/guix/scripts/download.scm@@ -26,15 +26,19 @@ #:use-module (guix base32) #:autoload (guix base64) (base64-encode) #:use-module ((guix download) #:hide (url-fetch))+ #:use-module ((guix git) #:select (download-git-to-store)) #:use-module ((guix build download) #:select (url-fetch)) #:use-module ((guix progress) #:select (current-terminal-columns))+ #:use-module ((guix serialization)+ #:select (write-file)) #:use-module ((guix build syscalls) #:select (terminal-columns)) #:use-module (web uri) #:use-module (ice-9 match) #:use-module (srfi srfi-1)+ #:use-module (srfi srfi-11) #:use-module (srfi srfi-14) #:use-module (srfi srfi-26) #:use-module (srfi srfi-37)@@ -76,12 +80,20 @@ (ensure-valid-store-file-name (basename url)) #:verify-certificate? verify-certificate?))) +(define* (download-git-to-store* url commit #:key recursive?)+ (with-store store+ (download-git-to-store store url commit+ (ensure-valid-store-file-name (basename url))+ #:recursive? recursive?)))+ (define %default-options ;; Alist of default option values. `((format . ,bytevector->nix-base32-string) (hash-algorithm . ,(hash-algorithm sha256)) (verify-certificate? . #t)- (download-proc . ,download-to-store*)))+ (download-proc . ,download-to-store*)+ (git-download-proc . ,download-git-to-store*)+ (commit . #f))) (define (show-help) (display (G_ "Usage: guix download [OPTION] URL@@ -100,6 +112,9 @@ and 'base16' ('hex' and 'hexadecimal' can be used as well).\n")) do not validate the certificate of HTTPS servers ")) (format #t (G_ " -o, --output=FILE download to FILE"))+ (format #t (G_ "+ -c, --git-commit=COMMIT+ download a Git repository")) (newline) (display (G_ " -h, --help display this help and exit"))@@ -143,6 +158,9 @@ and 'base16' ('hex' and 'hexadecimal' can be used as well).\n")) (lambda* (url #:key verify-certificate?) (download-to-file url arg)) (alist-delete 'download result))))+ (option '(#\c "git-commit") #t #f+ (lambda (opt name arg result)+ (alist-cons 'commit arg result))) (option '(#\h "help") #f #f (lambda args@@ -182,16 +200,31 @@ and 'base16' ('hex' and 'hexadecimal' can be used as well).\n")) (leave (G_ "~a: failed to parse URI~%") arg))) (fetch (assq-ref opts 'download-proc))+ (git-fetch (assq-ref opts 'git-download-proc))+ (commit (assq-ref opts 'commit)) (path (parameterize ((current-terminal-columns (terminal-columns)))- (fetch (uri->string uri)- #:verify-certificate?- (assq-ref opts 'verify-certificate?))))- (hash (call-with-input-file- (or path- (leave (G_ "~a: download failed~%")- arg))- (cute port-hash (assoc-ref opts 'hash-algorithm) <>)))+ (if commit+ (git-fetch (uri->string uri) commit)+ (fetch (uri->string uri)+ #:verify-certificate?+ (assq-ref opts 'verify-certificate?)))))+ (hash (if (or (assq-ref opts 'recursive) commit)+ (let-values (((port get-hash)+ (open-hash-port+ (assoc-ref opts 'hash-algorithm))))+ (write-file path port+ #:select?+ (if commit+ (lambda (file stat) (not (equal? (basename file) ".git")))+ (const #t)))+ (force-output port)+ (get-hash))+ (call-with-input-file+ (or path+ (leave (G_ "~a: download failed~%")+ arg))+ (cute port-hash (assoc-ref opts 'hash-algorithm) <>)))) (fmt (assq-ref opts 'format))) (format #t "~a~%~a~%" path (fmt hash)) #t)))-- 2.33.0
S
S
Sarah Morgensen wrote on 31 Aug 20:50 +0200
(name . jgart)(address . jgart@dismail.de)
86a6kx1m9j.fsf@mgsn.dev
Hello,
Thanks for the patch! I've been wanting a feature like this. I haven'ttested your patch, but just reading it, I have a few comments...
jgart <jgart@dismail.de> writes:
Toggle quote (12 lines)> From: Julien Lepiller <julien@lepiller.eu>>> * guix/git.scm (download-git-to-store): Download Git repository from> URL at COMMIT to STORE, either under NAME or URL's basename if omitted.> Write progress reports to LOG. RECURSIVE? has the same effect as the> same-named parameter of 'git-fetch'.>> * guix/scripts/download.scm (download-git-to-store*): Adds cli option.> Examples:> guix download --git-commit=v0.1.1 github.com/anaseto/gruid-tcell> guix download -c v0.1.1 https://github.com/anaseto/gruid-tcell
These examples should probably be in the main commit message, not theChangeLog entry.
Toggle quote (34 lines)> ---> guix/git.scm | 24 +++++++++++++++++-> guix/scripts/download.scm | 51 ++++++++++++++++++++++++++++++++-------> 2 files changed, 65 insertions(+), 10 deletions(-)>> diff --git a/guix/git.scm b/guix/git.scm> index 9c6f326c36..4c70782b97 100644> --- a/guix/git.scm> +++ b/guix/git.scm> @@ -28,6 +28,7 @@> #:use-module (gcrypt hash)> #:use-module ((guix build utils)> #:select (mkdir-p delete-file-recursively))> + #:use-module ((guix build git) #:select (git-fetch))> #:use-module (guix store)> #:use-module (guix utils)> #:use-module (guix records)> @@ -43,6 +44,7 @@> #:use-module (srfi srfi-11)> #:use-module (srfi srfi-34)> #:use-module (srfi srfi-35)> + #:use-module (web uri)> #:export (%repository-cache-directory> honor-system-x509-certificates!> > @@ -61,7 +63,9 @@> git-checkout-url> git-checkout-branch> git-checkout-commit> - git-checkout-recursive?))> + git-checkout-recursive?> +> + download-git-to-store))
Your patch has a number of tabs in it; they should be converted to spaces.
Toggle quote (16 lines)> > (define %repository-cache-directory> (make-parameter (string-append (cache-directory #:ensure? #f)> @@ -614,6 +618,24 @@ objects: 'ancestor (meaning that OLD is an ancestor of NEW), 'descendant, or> #:recursive? recursive?> #:log-port (current-error-port)))))> > +(define* (download-git-to-store store url commit> + #:optional (name (basename url))> + #:key (log (current-error-port)) recursive?)> + "Download Git repository from URL at COMMIT to STORE, either under NAME or> +URL's basename if omitted. Write progress reports to LOG. RECURSIVE? has the> +same effect as the same-named parameter of 'git-fetch'."> + (define uri> + (string->uri url))
You've defined uri but not used it anywhere.
Toggle quote (11 lines)> +> + (call-with-temporary-directory> + (lambda (temp)> + (let ((result> + (parameterize ((current-output-port log))> + (git-fetch url commit temp> + #:recursive? recursive?))))> + (and result> + (add-to-store store name #t "sha256" temp))))))> +
Is there a reason for not using latest-repository-commit instead ofthis? That already takes care of temporary directories, ignores .git,and reduces duplication if you download different tags from the samerepository:
guix download -c v0.1.3 example.com/repo
guix download -c v2.1.0 example.com/repo
Toggle quote (54 lines)> ;; Local Variables:> ;; eval: (put 'with-repository 'scheme-indent-function 2)> ;; End:> diff --git a/guix/scripts/download.scm b/guix/scripts/download.scm> index 5a91390358..6253ecaa5c 100644> --- a/guix/scripts/download.scm> +++ b/guix/scripts/download.scm> @@ -26,15 +26,19 @@> #:use-module (guix base32)> #:autoload (guix base64) (base64-encode)> #:use-module ((guix download) #:hide (url-fetch))> + #:use-module ((guix git) #:select (download-git-to-store))> #:use-module ((guix build download)> #:select (url-fetch))> #:use-module ((guix progress)> #:select (current-terminal-columns))> + #:use-module ((guix serialization)> + #:select (write-file))> #:use-module ((guix build syscalls)> #:select (terminal-columns))> #:use-module (web uri)> #:use-module (ice-9 match)> #:use-module (srfi srfi-1)> + #:use-module (srfi srfi-11)> #:use-module (srfi srfi-14)> #:use-module (srfi srfi-26)> #:use-module (srfi srfi-37)> @@ -76,12 +80,20 @@> (ensure-valid-store-file-name (basename url))> #:verify-certificate? verify-certificate?)))> > +(define* (download-git-to-store* url commit #:key recursive?)> + (with-store store> + (download-git-to-store store url commit> + (ensure-valid-store-file-name (basename url))> + #:recursive? recursive?)))> +> (define %default-options> ;; Alist of default option values.> `((format . ,bytevector->nix-base32-string)> (hash-algorithm . ,(hash-algorithm sha256))> (verify-certificate? . #t)> - (download-proc . ,download-to-store*)))> + (download-proc . ,download-to-store*)> + (git-download-proc . ,download-git-to-store*)> + (commit . #f)))> > (define (show-help)> (display (G_ "Usage: guix download [OPTION] URL> @@ -100,6 +112,9 @@ and 'base16' ('hex' and 'hexadecimal' can be used as well).\n"))> do not validate the certificate of HTTPS servers "))> (format #t (G_ "> -o, --output=FILE download to FILE"))
This option exists, so it would make sense for
guix download -c v0.1 example.com/repo -o ~/src/repo
to work as well.
Toggle quote (4 lines)> + (format #t (G_ "> + -c, --git-commit=COMMIT> + download a Git repository"))
Would it be possible to find a way to express this option so it wouldn'tconflict with potential other repositories (SVN, hg, etc)?
Maybe something like
guix download --git -c v0.1 example.com/repo guix download --svn -c v0.1 example.com/repo
But in SVN, "revisions" are used instead of "commits," so perhapsremoving that terminology would be better. We could do
guix download --git example.com/repo v0.1 guix download --svn example.com/repo v0.1
Or! We could use (fake) sub-commands like so:
guix download git example.com/repo -c v0.1
This feels nice, since it matches e.g. 'guix import go'. Seeguix/import.scm for how this works.
Toggle quote (32 lines)> (newline)> (display (G_ "> -h, --help display this help and exit"))> @@ -143,6 +158,9 @@ and 'base16' ('hex' and 'hexadecimal' can be used as well).\n"))> (lambda* (url #:key verify-certificate?)> (download-to-file url arg))> (alist-delete 'download result))))> + (option '(#\c "git-commit") #t #f> + (lambda (opt name arg result)> + (alist-cons 'commit arg result)))> > (option '(#\h "help") #f #f> (lambda args> @@ -182,16 +200,31 @@ and 'base16' ('hex' and 'hexadecimal' can be used as well).\n"))> (leave (G_ "~a: failed to parse URI~%")> arg)))> (fetch (assq-ref opts 'download-proc))> + (git-fetch (assq-ref opts 'git-download-proc))> + (commit (assq-ref opts 'commit))> (path (parameterize ((current-terminal-columns> (terminal-columns)))> - (fetch (uri->string uri)> - #:verify-certificate?> - (assq-ref opts 'verify-certificate?))))> - (hash (call-with-input-file> - (or path> - (leave (G_ "~a: download failed~%")> - arg))> - (cute port-hash (assoc-ref opts 'hash-algorithm) <>)))> + (if commit> + (git-fetch (uri->string uri) commit)
You don't actually seem to use the download-git-to-store procedure youwrote above. An oversight?
Toggle quote (5 lines)> + (fetch (uri->string uri)> + #:verify-certificate?> + (assq-ref opts 'verify-certificate?)))))> + (hash (if (or (assq-ref opts 'recursive) commit)
The 'recursive' option is used here, but not specified in options or thehelp message. I'm also not sure what it's supposed to mean here.'Recursive' would have no meaning outside of fetching a repo, and forfetching a repo, 'recursive' should mean that the fetch is recursive,and would be applied above.
Toggle quote (16 lines)> + (let-values (((port get-hash)> + (open-hash-port> + (assoc-ref opts 'hash-algorithm))))> + (write-file path port> + #:select?> + (if commit> + (lambda (file stat) (not (equal? (basename file) ".git")))> + (const #t)))> + (force-output port)> + (get-hash))> + (call-with-input-file> + (or path> + (leave (G_ "~a: download failed~%")> + arg))> + (cute port-hash (assoc-ref opts 'hash-algorithm) <>))))
Rather than special-casing repositories, perhaps you'd considerincorporating the first 1-3 patches from [0]? This would make it easierto, say, add other repository formats later. It would make all thisroughly
(hash (and path (file-hash* path #:algorithm (assq-ref opts 'hash-algorithm))))
modulo error reporting, of course.
[0] https://issues.guix.gnu.org/50072
--Sarah
S
S
Sarah Morgensen wrote on 31 Aug 21:08 +0200
(name . jgart)(address . jgart@dismail.de)
865yvl1lg6.fsf@mgsn.dev
Hi again,
Sarah Morgensen <iskarian@mgsn.dev> writes:
Toggle quote (19 lines)>> (fetch (assq-ref opts 'download-proc))>> + (git-fetch (assq-ref opts 'git-download-proc))>> + (commit (assq-ref opts 'commit))>> (path (parameterize ((current-terminal-columns>> (terminal-columns)))>> - (fetch (uri->string uri)>> - #:verify-certificate?>> - (assq-ref opts 'verify-certificate?))))>> - (hash (call-with-input-file>> - (or path>> - (leave (G_ "~a: download failed~%")>> - arg))>> - (cute port-hash (assoc-ref opts 'hash-algorithm) <>)))>> + (if commit>> + (git-fetch (uri->string uri) commit)>> You don't actually seem to use the download-git-to-store procedure you> wrote above. An oversight?
Please disregard this comment. I read too fast and didn't catch thatgit-fetch took the value of 'git-download-proc :)
--Sarah
J
(name . Sarah Morgensen)(address . iskarian@mgsn.dev)
20210831153029.GC15239@gac.attlocal.net
Toggle quote (3 lines)> Please disregard this comment. I read too fast and didn't catch that> git-fetch took the value of 'git-download-proc :)
No worries! :) _________________________________________ / 3B1D 7F19 E36B B60C 0F5B 2CA9 A52A A2B4 \\ 77B6 DD35 / ----------------------------------------- \ ^__^ \ (oo)\_______ (__)\ )\/\ ||----w | || ||
J
(name . Sarah Morgensen)(address . iskarian@mgsn.dev)
20210831160630.GB24419@gac.attlocal.net
On Tue, 31 Aug 2021 15:30:29 -0400 jgart <jgart@dismail.de> wrote:
Hi Sarah,
I think Julien will send the patch with the tabs to spaces conversion later.
Thanks for the review.
M
M
Maxime Devos wrote on 1 Sep 00:28 +0200
533946f3c605e64a17cb34208551d578765d283a.camel@telenet.be
Sarah Morgensen schreef op di 31-08-2021 om 11:50 [-0700]:
Toggle quote (3 lines)> > Your patch has a number of tabs in it; they should be converted to spaces.
Is this documented somewhere? I'm not aware of such a rule.
Greetings,Maxime
-----BEGIN PGP SIGNATURE-----
iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYS6tFhccbWF4aW1lZGV2b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7qwtAP9M/oHf67O9hJ0mscHL41a0XG/tNyAfJEfaAe7ZbvRG4wEAhVxmP6AaKLOpZ0N1aIwutrymlFmYQ4Hd5j871OqMIwk==Y+Om-----END PGP SIGNATURE-----

S
S
Sarah Morgensen wrote on 3 Sep 03:37 +0200
(name . Maxime Devos)(address . maximedevos@telenet.be)
86tuj2xwvt.fsf@mgsn.dev
Hi Maxime,
Maxime Devos <maximedevos@telenet.be> writes:
Toggle quote (6 lines)> Sarah Morgensen schreef op di 31-08-2021 om 11:50 [-0700]:>> >> Your patch has a number of tabs in it; they should be converted to spaces.>> Is this documented somewhere? I'm not aware of such a rule.
I cannot find it explicitly stated anywhere, but etc/indent-code.el doesreplace tabs with spaces:
Toggle snippet (9 lines) ;; Indent all of FILE-NAME. (find-file file-name) (let ((indent-tabs-mode nil)) (untabify (point-min) (point-max)) (indent-region (point-min) (point-max)) (save-buffer) (message "Done!")))
So I assumed it's policy. Perhaps the documentation should explicitlynote that spaces are Official Policy?
--Sarah
L
L
Ludovic Courtès wrote on 24 Sep 14:25 +0200
Re: bug#50274: [PATCH] guix: git: Adds feature to download git repository to the store.
(name . jgart)(address . jgart@dismail.de)
87y27mma7m.fsf@gnu.org
Hi Julien & jgart,
jgart <jgart@dismail.de> skribis:
Toggle quote (12 lines)> From: Julien Lepiller <julien@lepiller.eu>>> * guix/git.scm (download-git-to-store): Download Git repository from> URL at COMMIT to STORE, either under NAME or URL's basename if omitted.> Write progress reports to LOG. RECURSIVE? has the same effect as the> same-named parameter of 'git-fetch'.>> * guix/scripts/download.scm (download-git-to-store*): Adds cli option.> Examples:> guix download --git-commit=v0.1.1 github.com/anaseto/gruid-tcell> guix download -c v0.1.1 https://github.com/anaseto/gruid-tcell
Very useful!
Toggle quote (7 lines)> +(define* (download-git-to-store store url commit> + #:optional (name (basename url))> + #:key (log (current-error-port)) recursive?)> + "Download Git repository from URL at COMMIT to STORE, either under NAME or> +URL's basename if omitted. Write progress reports to LOG. RECURSIVE? has the> +same effect as the same-named parameter of 'git-fetch'."
Can we use ‘latest-repository-commit’ instead? The difference is thatit’ll populate ~/.cache/guix/checkouts, but I think that’s fine.
OTOH, if we want to make it easier to support other VCSes, we can chooseto not use (guix git) at all and instead use ‘git-fetch’ in (guixgit-download), ‘hg-fetch’ in (guix hg-download), etc. This code wouldgo to (guix scripts hash).
WDYT?
[...]
Toggle quote (4 lines)> + (option '(#\c "git-commit") #t #f> + (lambda (opt name arg result)> + (alist-cons 'commit arg result)))
For consistency with package transformation options, I’d call it‘--commit’. This option would still apply if/when other VCSes aresupported.
This would also need an update of the manual.
Thanks,Ludo’.
S
S
Sarah Morgensen wrote on 26 Sep 08:24 +0200
Re: [bug#50274] [PATCH] guix: git: Adds feature to download git repository to the store.
(name . Ludovic Courtès)(address . ludo@gnu.org)
868rzjvope.fsf@mgsn.dev
Hi,
Ludovic Courtès <ludo@gnu.org> writes:
Toggle quote (17 lines)>> +(define* (download-git-to-store store url commit>> + #:optional (name (basename url))>> + #:key (log (current-error-port)) recursive?)>> + "Download Git repository from URL at COMMIT to STORE, either under NAME or>> +URL's basename if omitted. Write progress reports to LOG. RECURSIVE? has the>> +same effect as the same-named parameter of 'git-fetch'.">> Can we use ‘latest-repository-commit’ instead? The difference is that> it’ll populate ~/.cache/guix/checkouts, but I think that’s fine.>> OTOH, if we want to make it easier to support other VCSes, we can choose> to not use (guix git) at all and instead use ‘git-fetch’ in (guix> git-download), ‘hg-fetch’ in (guix hg-download), etc. This code would> go to (guix scripts hash).>> WDYT?
Would using 'git-fetch' mean that it's already in the store (andtherefore won't be redownloaded) when it's subsequently used in asource? That would be even better than latest-repository-commit!
(Presumably --with-commit and friends also use 'git-fetch'?)
--Sarah
L
L
Ludovic Courtès wrote on 30 Sep 22:03 +0200
(name . Sarah Morgensen)(address . iskarian@mgsn.dev)
871r554yrq.fsf@gnu.org
Hi,
Sarah Morgensen <iskarian@mgsn.dev> skribis:
Toggle quote (23 lines)> Ludovic Courtès <ludo@gnu.org> writes:>>>> +(define* (download-git-to-store store url commit>>> + #:optional (name (basename url))>>> + #:key (log (current-error-port)) recursive?)>>> + "Download Git repository from URL at COMMIT to STORE, either under NAME or>>> +URL's basename if omitted. Write progress reports to LOG. RECURSIVE? has the>>> +same effect as the same-named parameter of 'git-fetch'.">>>> Can we use ‘latest-repository-commit’ instead? The difference is that>> it’ll populate ~/.cache/guix/checkouts, but I think that’s fine.>>>> OTOH, if we want to make it easier to support other VCSes, we can choose>> to not use (guix git) at all and instead use ‘git-fetch’ in (guix>> git-download), ‘hg-fetch’ in (guix hg-download), etc. This code would>> go to (guix scripts hash).>>>> WDYT?>> Would using 'git-fetch' mean that it's already in the store (and> therefore won't be redownloaded) when it's subsequently used in a> source? That would be even better than latest-repository-commit!
Yes, you’re right.
Toggle quote (2 lines)> (Presumably --with-commit and friends also use 'git-fetch'?)
No, they use ‘latest-repository-commit’ (via <git-checkout>) becauseit’s a case where you can’t use an <origin> because the content hash isnot known in advance (and the commit is also unknown when you use‘--with-branch’).
Ludo’.
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send email to 50274@debbugs.gnu.org