bug#26777: [PATCH] guix: git: Add new module.

[Ludovic Courtès]

Hi!

Mathieu Othacehe <address@hidden> 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’.