From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42619) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d6JAl-0007co-Pc for guix-patches@gnu.org; Thu, 04 May 2017 12:00:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d6JAh-0002xw-6L for guix-patches@gnu.org; Thu, 04 May 2017 12:00:07 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:56515) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1d6JAh-0002xm-2s for guix-patches@gnu.org; Thu, 04 May 2017 12:00:03 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1d6JAg-00033z-SH for guix-patches@gnu.org; Thu, 04 May 2017 12:00:02 -0400 Subject: bug#26777: [PATCH] guix: git: Add new module. Resent-Message-ID: From: ludo@gnu.org (Ludovic =?UTF-8?Q?Court=C3=A8s?=) References: <20170504144944.8635-1-m.othacehe@gmail.com> Date: Thu, 04 May 2017 17:59:35 +0200 In-Reply-To: <20170504144944.8635-1-m.othacehe@gmail.com> (Mathieu Othacehe's message of "Thu, 4 May 2017 16:49:44 +0200") Message-ID: <87d1bohau0.fsf@gnu.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: Mathieu Othacehe Cc: 26777@debbugs.gnu.org Hi! Mathieu Othacehe skribis: > * guix/git.scm: New file. > * configure.ac: Check for (guile git). > * Makefile.am: Build guix/git.scm if (guile git) is available. Very nice! > +(define %repository-cache-path > + (make-parameter "/var/cache/guix/checkouts")) s/path/directory/ (In GNU the convention is to use the terms =E2=80=9Cfile = name=E2=80=9D or =E2=80=9Cdirectory name=E2=80=9D, or just =E2=80=9Cfile=E2=80=9D or =E2= =80=9Cdirectory=E2=80=9D (info "(standards) GNU Manuals").) > +(define (repository-cache-directory url) > + "Return the directory associated to URL in %repository-cache-path." > + (string-append > + (%repository-cache-path) "/" > + (bytevector->base32-string (sha256 (string->utf8 url))))) This is a detail, but in general, for arguments like the cache directory, I prefer an optional argument like this: (define* (repository-cache-directory url #:optional (cache-directory (%repository-cache-direc= tory))) =E2=80=A6) > +(define (clone-with-error-handling url path) > + "Clone git repository at URL into PATH with error handling." > + (catch 'git-error > + (lambda () > + (mkdir-p path) > + (clone url path)) s/path/directory/ > + (lambda (key . parameters) > + (rmdir path) > + (error "Clone error: " parameters)))) Just let the =E2=80=98git-error=E2=80=99 through: it=E2=80=99s the caller= =E2=80=99s responsibility to handle it. Same in other procedures that catch =E2=80=98git-error=E2=80=99. If really necessary, you can add: (define-syntax-rule (false-if-git-error exp) (catch 'git-error (lambda () exp) (const #f))) > +(define* (copy-to-store cache-path #:key url repository) > + "Copy items in cache-path to store. URL and REPOSITORY are used > +to forge store directory name." > + (let* ((commit (repository->head-sha1 repository)) > + (name (url+commit->name url commit))) > + (with-store store > + (values (add-to-store store name #t "sha256" cache-path) commit)))) Please make =E2=80=98store=E2=80=99 a parameter, so that the caller can cho= ose what store they connect to. Could you send an updated patch? Thank you! Ludo=E2=80=99.