From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp1 ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id yBi8Eht6LmE7IgAAgWs5BA (envelope-from ) for ; Tue, 31 Aug 2021 20:51:07 +0200 Received: from aspmx1.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp1 with LMTPS id +PjdDRt6LmEkVgAAbx9fmQ (envelope-from ) for ; Tue, 31 Aug 2021 18:51:07 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id C7DE4275BF for ; Tue, 31 Aug 2021 20:51:06 +0200 (CEST) Received: from localhost ([::1]:47562 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mL8qf-0003cd-OJ for larch@yhetil.org; Tue, 31 Aug 2021 14:51:05 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:41436) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mL8qc-0003cS-6B for guix-patches@gnu.org; Tue, 31 Aug 2021 14:51:02 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:52073) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mL8qb-0002N8-VL for guix-patches@gnu.org; Tue, 31 Aug 2021 14:51:01 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mL8qb-0002LK-RR for guix-patches@gnu.org; Tue, 31 Aug 2021 14:51:01 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#50274] [PATCH] guix: git: Adds feature to download git repository to the store. Resent-From: Sarah Morgensen Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Tue, 31 Aug 2021 18:51:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 50274 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: jgart Cc: Julien Lepiller , 50274@debbugs.gnu.org Received: via spool by 50274-submit@debbugs.gnu.org id=B50274.16304358578995 (code B ref 50274); Tue, 31 Aug 2021 18:51:01 +0000 Received: (at 50274) by debbugs.gnu.org; 31 Aug 2021 18:50:57 +0000 Received: from localhost ([127.0.0.1]:35386 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mL8qW-0002L0-V4 for submit@debbugs.gnu.org; Tue, 31 Aug 2021 14:50:57 -0400 Received: from out0.migadu.com ([94.23.1.103]:27320) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mL8qT-0002Kp-Mf for 50274@debbugs.gnu.org; Tue, 31 Aug 2021 14:50:55 -0400 X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mgsn.dev; s=key1; t=1630435851; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=Ib47Ra2325KK7553QF+khNFLNaHPJ4+coN+eofxN5Yo=; b=G8QAVglXSJtVHJ+m2mvThR74OHFYA5c0YUO1B+lVTwwceij5g2X0UYEpK2fgxxav50DjhH kGMFMZhFgvIgXrDoUh2xn2L/QKeKx2bgVMBeICgxdZamp6XlLX+iURFj1RiudB3qW0zIk0 NAswGw8qM0xf4GgWFNwm4UyrdLfhC5E= From: Sarah Morgensen References: <20210830163918.19419-1-jgart@dismail.de> Date: Tue, 31 Aug 2021 11:50:48 -0700 In-Reply-To: <20210830163918.19419-1-jgart@dismail.de> (jgart@dismail.de's message of "Mon, 30 Aug 2021 12:39:19 -0400 (1 day, 1 hour, 24 minutes ago)") Message-ID: <86a6kx1m9j.fsf@mgsn.dev> MIME-Version: 1.0 Content-Type: text/plain X-Migadu-Auth-User: iskarian@mgsn.dev X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+larch=yhetil.org@gnu.org Sender: "Guix-patches" X-Migadu-Flow: FLOW_IN ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1630435867; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:resent-cc:resent-from:resent-sender: resent-message-id:in-reply-to:in-reply-to:references:references: list-id:list-help:list-unsubscribe:list-subscribe:list-post: dkim-signature; bh=Ib47Ra2325KK7553QF+khNFLNaHPJ4+coN+eofxN5Yo=; b=XRaw47aBRWyDAXoly0o7AoO+bcy6QUuPCJ8sNIP1olDGEgCk7gaDXkClI2WOG1Y8g8M0Po wQsxvP0ZDJZzP9WSMVejGmaKweF4kHv1Wn3fULR0DYI32TnqmDf55fzlIjcnCCoxldEAY4 orV3AzcD2bzu684P6NL/SBvh36W6khvwN5smm85+DTI3WNwKd5PjelYd/mp+uOhS4nNgs+ pXqdfFA8J7Y2BEOjFBwgMJhONQRSqgk1BeRL4MHKc5mOwFabGwufE9AmAJSSxQdtn0CN9f qe/182R9u6puDqI4chY/R4i8WwN0XNAdQyX2llkVSiDlWTxFQcYO9ZdoJc/wpg== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1630435867; a=rsa-sha256; cv=none; b=u8ekCZJZAbjbU3bkFHr3UGSLQNDkYzUbLmnWDXEueOYO5zF9EArrNqWKuZzy025z6r32Co jyOVGKyMPULk3Tf00o+EbKNf5DK9MTmtSF8ciMQCD178mFNEFlsLxzz1nTjQ8kfNplLmdQ WVKzc2JyQ2Anmp7xg2KUmuuOKnKzx3cXZWshPgt56jT42kBpc3GyifvynifBTcWeEsv/no l4GmS5na4E/A5RS3jKRlj0r8e4Rwg83iCWlVKCDfTovb+zBR51A72p43oNzVnYjGi4UD/r 8lFZKk7nUYxHVCJmhXoISf/4XVsjkx24rimsIV6AaNJ1Q9Sby8vbcRm25BpH9A== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=mgsn.dev header.s=key1 header.b=G8QAVglX; dmarc=fail reason="SPF not aligned (relaxed)" header.from=mgsn.dev (policy=none); spf=pass (aspmx1.migadu.com: domain of guix-patches-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-patches-bounces@gnu.org X-Migadu-Spam-Score: 0.18 Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=mgsn.dev header.s=key1 header.b=G8QAVglX; dmarc=fail reason="SPF not aligned (relaxed)" header.from=mgsn.dev (policy=none); spf=pass (aspmx1.migadu.com: domain of guix-patches-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-patches-bounces@gnu.org X-Migadu-Queue-Id: C7DE4275BF X-Spam-Score: 0.18 X-Migadu-Scanner: scn0.migadu.com X-TUID: /jyEuPVJZJcq Hello, Thanks for the patch! I've been wanting a feature like this. I haven't tested your patch, but just reading it, I have a few comments... jgart writes: > From: Julien Lepiller > > * 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 the ChangeLog entry. > --- > 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. > > (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. > + > + (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 of this? That already takes care of temporary directories, ignores .git, and reduces duplication if you download different tags from the same repository: guix download -c v0.1.3 example.com/repo guix download -c v2.1.0 example.com/repo > ;; 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. > + (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't conflict 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 perhaps removing 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'. See guix/import.scm for how this works. > (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 you wrote above. An oversight? > + (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 the help message. I'm also not sure what it's supposed to mean here. 'Recursive' would have no meaning outside of fetching a repo, and for fetching a repo, 'recursive' should mean that the fetch is recursive, and would be applied above. > + (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 consider incorporating the first 1-3 patches from [0]? This would make it easier to, say, add other repository formats later. It would make all this roughly (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