GNU bug report logs - #26777
[PATCH] guix: git: Add new module.

Previous Next

Full log

View this message in rfc822 format

From: ludo <at> gnu.org (Ludovic Courtès)
To: Mathieu Othacehe <m.othacehe <at> gmail.com>
Cc: 26777 <at> debbugs.gnu.org
Subject: bug#26777: [PATCH] guix: git: Add new module.
Date: Thu, 04 May 2017 17:59:35 +0200

Hi!

Mathieu Othacehe <m.othacehe <at> gmail.com> 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 “file name”
or “directory name”, or just “file” or “directory” (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-directory)))
    …)

> +(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 ‘git-error’ through: it’s the caller’s responsibility to
handle it.  Same in other procedures that catch ‘git-error’.

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 ‘store’ a parameter, so that the caller can choose what
store they connect to.

Could you send an updated patch?

Thank you!

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.