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

From: [email-masked] (Ludovic Courtès)
Date: Fri, 05 May 2017 11:51:09 +0200

Hi!

Mathieu Othacehe <[email-masked]> skribis:

> * guix/git.scm: New file.
> * configure.ac: Check for (guile git).
> * Makefile.am: Build guix/git.scm if (guile git) is available.

[...]

> +(define* (copy-to-store cache-directory store #:key url repository)
> +  "Copy items in cache-directory to store.  URL and REPOSITORY are used
> +to forge store directory name."

Could you make ‘store’ the first parameter, as is done elsewhere?

> +(define (switch-to-ref repository ref)
> +  "Switch to REPOSITORY's branch, commit or tag specified by REF."
> +  (let* ((oid (match ref
> +                (('branch . branch)
> +                 (reference-target
> +                  (branch-lookup repository branch BRANCH-REMOTE)))
> +                (('commit . commit)
> +                 (string->oid commit))
> +                (('tag    . tag)
> +                 (reference-name->oid repository
> +                                      (string-append "refs/tags/" tag)))))
> +         (obj (object-lookup repository oid)))
> +    ;; guile-git checkout binding seems broken.
> +    (reset repository obj RESET_HARD)))

Could you add an XXX to this comment and perhaps say why this is broken?
Regardless of brokenness, it sounds safer to always hard-reset the
checkout to make sure we’re in the right state, no?  So maybe you can
even remove the comment.  :-)

> +(define* (latest-repository-commit url store
> +                                   #:key
> +                                   (ref '(branch . "origin/master")))
> +  "Return two values: the content of the git repository at URL copied into a
> +store directory and the sha1 of the top level commit in this directory.  The
> +reference to be checkout, once the repository is fetched, is specified by REF.
> +REF is pair whose key is [branch | commit | tag] and value the associated
> +data, respectively [<branch name> | <sha1> | <tag name>].

Please make ‘store’ the first argument, and add

  #:key (cache-directory (%repository-cache-directory))

OK for ‘master’ with changes along these lines.

Thank you!

Ludo’.