Re: [PATCH] scripts: hash: Add --git option. WIP

[Ludovic Courtès]

Hello,

Jan Nieuwenhuizen <address@hidden> skribis:

> Attached is a patch to get the hash of a git archive without having to
> clean the tree or do a clean checkout.
>
> Using
>
>     guix hash -gr .
>
> procudes the same hash as doing something like
>
>     git clone . tmp && guix hash -rx tmp && rm -r tmp

Very useful!

> I marked it as WIP because while it is already "handy" as it is, I
> consider adding a commit argument and imply --recursive, like so
>
>    guix hash --git HEAD
>    guix hash --git v0.13
>
> WDYT?

This can always be added in a later patch.

> From cfc9e557db6fe6c9aece68cfc5153ec9481a45a4 Mon Sep 17 00:00:00 2001
> From: Jan Nieuwenhuizen <address@hidden>
> Date: Thu, 23 Nov 2017 04:30:13 +0100
> Subject: [PATCH] scripts: hash: Add --git option.  WIP
>
> Using
>
>     guix hash -gr .
>
> procudes the same hash as doing something like
>
>     git clone . tmp && guix hash -rx tmp && rm -r tmp
>
> * guix/git.scm (git-ls-files): New function.
> * guix/scripts/hash.scm (%options, show-help): Add `--git'.
> (guix-hash)[git-file?]: New function.

[...]

> +(define (git-ls-files directory)

What about ‘git-file-list’ or ‘git-controlled-files’ rather?
With a docstring too.  :-)

> +  (define (git-file? directory)
> +    (let* ((files (git-ls-files directory))
> +           (directories (delete-duplicates (map dirname files)))
> +           (prefix (if (string-suffix? "/" directory) directory
> +                       (string-append directory "/")))
> +           (prefix-length (string-length prefix)))
> +      (lambda (file stat)
> +        (case (stat:type stat)
> +          ((directory)
> +           (member (string-drop file prefix-length) directories))
> +          ((regular)
> +           (member (string-drop file prefix-length) files))
> +          (else
> +           #f)))))

This appears to duplicate ‘git-predicate’ from (guix git-download),
which was carefully optimized for large trees by Chris.

What about:

  1. Rewrite ‘git-predicate’ to use the new ‘git-file-list’ (currently
     is shells out ‘git’);

  2. Moving ‘git-predicate’ to (guix git).

?

I understand this is probably more than you were willing to do ;-), but
it should be beneficial.

> +         (select? (cond
> +                   ((assq-ref opts 'exclude-vcs?)
> +                    (negate vcs-file?))
> +                   ((assq-ref opts 'git?)
> +                    (git-file? (car args)))
> +                   (else
> +                    (const #t)))))

I think we should use ‘match’ to gracefully handle any errors:

  ((assq-ref opts 'git?)
   (match args
     ((file) (git-predicate file))
     (_      (const #f))))

Thanks,
Ludo’.