unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#66961] [PATCH] scripts: hash: Handle repository with different VCS folders.
@ 2023-11-05 23:00 Simon Tournier
  2023-11-14 13:44 ` Ludovic Courtès
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Simon Tournier @ 2023-11-05 23:00 UTC (permalink / raw)
  To: 66961
  Cc: Simon Tournier, Christopher Baines, Josselin Poiret,
	Ludovic Courtès, Mathieu Othacehe, Ricardo Wurmus,
	Simon Tournier, Tobias Geerinckx-Rice

Fixes <https://issues.guix.gnu.org/issue/65979>.
Reported by Simon Tournier <zimon.toutoune@gmail.com>

* guix/hash.scm (vcs-file?): Add optional argument for passing VCS kind of the
file/repository.
(file-hash*): Adjust accordingly.
* guix/scripts/hash.scm (guix-hash)[file-hash]: Detect VCS kind of the
file/repository and passes it.

Change-Id: I8e286c3426ddefd664dc3a471d5a09e309824faa
---
 guix/hash.scm         | 18 ++++++++++++------
 guix/scripts/hash.scm | 18 +++++++++++++-----
 2 files changed, 25 insertions(+), 11 deletions(-)

Hi,

I re-resend this patch for more attractions in case I have missed
something. :-)  It had already been send more than 2 weeks ago as an answer to
#65979.

It fixes a corner case of "guix hash".

--8<---------------cut here---------------start------------->8---
$ git clone https://github.com/s-andrews/FastQC /tmp/FastQC
$ git -C /tmp/FastQC/ checkout v0.11.9

$ find /tmp/FastQC/ -type d -name ".git" -print
/tmp/FastQC/.git
$ find /tmp/FastQC/ -type d -name ".svn" -print
/tmp/FastQC/Help/1 Introduction/.svn
/tmp/FastQC/Help/3 Analysis Modules/.svn
/tmp/FastQC/Help/2 Basic Operations/.svn

$ guix hash -rx /tmp/FastQC
0jyk90kg6s62w3dn6qjx9nrawjs12qx172lii0yxbvsfylhnx479

$ grep -A 15 'define-public fastqc' gnu/packages/bioinformatics.scm | grep -C 1 base32
       (sha256
        (base32
         "00y9drm0bkpxw8xfl8ysss18jmnhj8blgqgr6fpa58rkpfcbg8qk"))

$ ./pre-inst-env guix hash -rx /tmp/FastQC
00y9drm0bkpxw8xfl8ysss18jmnhj8blgqgr6fpa58rkpfcbg8qk
--8<---------------cut here---------------end--------------->8---

And it does not introduce any significant penalty:

--8<---------------cut here---------------start------------->8---
$ time guix hash -rx .
093w70scf2v64bdbln5m2xjm63pzfib903w1mcb2smj32g0w8y63

real	0m9.730s
user	0m20.468s
sys	0m1.487s

$ time ./pre-inst-env guix hash -rx .
093w70scf2v64bdbln5m2xjm63pzfib903w1mcb2smj32g0w8y63

real	0m9.632s
user	0m20.320s
sys	0m1.541s
--8<---------------cut here---------------end--------------->8---


WDYT?

Cheers,
simon


diff --git a/guix/hash.scm b/guix/hash.scm
index 3cb68e5c44..8fff51e8f1 100644
--- a/guix/hash.scm
+++ b/guix/hash.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
 ;;; Copyright © 2022 Maxime Devos <maximedevos@telenet.be>
+;;; Copyright © 2023 Simon Tournier <zimon.toutoune@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -25,21 +26,26 @@ (define-module (guix hash)
   #:export (vcs-file?
             file-hash*))
 
-(define (vcs-file? file stat)
-  "Returns true if FILE is a version control system file."
+(define* (vcs-file? file stat
+                    #:optional
+                    (vcses (list ".bzr" ".git" ".hg" ".svn" "CVS")))
+  "Returns true if FILE matches a version control system from the list VCSES."
   (case (stat:type stat)
     ((directory)
-     (member (basename file) '(".bzr" ".git" ".hg" ".svn" "CVS")))
+     (member (basename file) vcses))
     ((regular)
-     ;; Git sub-modules have a '.git' file that is a regular text file.
-     (string=? (basename file) ".git"))
+     (if (member ".git" vcses)
+         ;; Git sub-modules have a '.git' file that is a regular text file.
+         (string=? (basename file) ".git")
+         #f))
     (else
      #f)))
 
 (define* (file-hash* file #:key
                      (algorithm (hash-algorithm sha256))
                      (recursive? 'auto)
-                     (select? (negate vcs-file?)))
+                     (select? (negate (lambda (file stat)
+                                        (vcs-file? file stat)))))
   "Compute the hash of FILE with ALGORITHM.
 
 Symbolic links are only dereferenced if RECURSIVE? is false.
diff --git a/guix/scripts/hash.scm b/guix/scripts/hash.scm
index 7197d3965c..ed96e6a7e1 100644
--- a/guix/scripts/hash.scm
+++ b/guix/scripts/hash.scm
@@ -3,7 +3,7 @@
 ;;; Copyright © 2013 Nikita Karetnikov <nikita@karetnikov.org>
 ;;; Copyright © 2016 Jan Nieuwenhuizen <janneke@gnu.org>
 ;;; Copyright © 2018 Tim Gesthuizen <tim.gesthuizen@yahoo.de>
-;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
+;;; Copyright © 2021, 2023 Simon Tournier <zimon.toutoune@gmail.com>
 ;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
 ;;;
 ;;; This file is part of GNU Guix.
@@ -181,9 +181,6 @@ (define-command (guix-hash . args)
                             (_ #f))
                            (reverse opts)))
          (fmt  (assq-ref opts 'format))
-         (select? (if (assq-ref opts 'exclude-vcs?)
-                      (negate vcs-file?)
-                      (const #t)))
          (algorithm (assoc-ref opts 'hash-algorithm))
          (serializer (assoc-ref opts 'serializer)))
 
@@ -193,7 +190,18 @@ (define-command (guix-hash . args)
       (catch 'system-error
         (lambda _
           (with-error-handling
-            (serializer file algorithm select?)))
+            (let* ((vcses (fold (lambda (vcs result)
+                                  (if (file-exists? (string-append file "/" vcs))
+                                      (cons vcs result)
+                                      result))
+                                '()
+                                (list ".bzr" ".git" ".hg" ".svn" "CVS")))
+                   (select? (if (assq-ref opts 'exclude-vcs?)
+                                (negate (lambda (file stat)
+                                          (vcs-file? file stat
+                                                     vcses)))
+                                (const #t))))
+              (serializer file algorithm select?))))
         (lambda args
           (leave (G_ "~a ~a~%")
                  file

base-commit: 08d94fe20eca47b69678b3eced8749dd02c700a4
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [bug#66961] [PATCH] scripts: hash: Handle repository with different VCS folders.
  2023-11-05 23:00 [bug#66961] [PATCH] scripts: hash: Handle repository with different VCS folders Simon Tournier
@ 2023-11-14 13:44 ` Ludovic Courtès
  2023-11-29 20:36   ` Simon Tournier
  2023-11-29 20:31 ` [bug#66961] [PATCH v2] " Simon Tournier
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Ludovic Courtès @ 2023-11-14 13:44 UTC (permalink / raw)
  To: Simon Tournier
  Cc: Josselin Poiret, Mathieu Othacehe, Tobias Geerinckx-Rice, 66961,
	Ricardo Wurmus, Christopher Baines

Hi!

Simon Tournier <zimon.toutoune@gmail.com> skribis:

> Fixes <https://issues.guix.gnu.org/issue/65979>.
> Reported by Simon Tournier <zimon.toutoune@gmail.com>
>
> * guix/hash.scm (vcs-file?): Add optional argument for passing VCS kind of the
> file/repository.
> (file-hash*): Adjust accordingly.
> * guix/scripts/hash.scm (guix-hash)[file-hash]: Detect VCS kind of the
> file/repository and passes it.
>
> Change-Id: I8e286c3426ddefd664dc3a471d5a09e309824faa

[...]

> +(define* (vcs-file? file stat
> +                    #:optional
> +                    (vcses (list ".bzr" ".git" ".hg" ".svn" "CVS")))
> +  "Returns true if FILE matches a version control system from the list VCSES."

How about ‘vcs-directories’ rather than ‘vcses’?

Also, you can add:

  (define %known-vcs-directories
    '(".bzr" …))

and use it to avoid repeating it.

> +            (let* ((vcses (fold (lambda (vcs result)
> +                                  (if (file-exists? (string-append file "/" vcs))
> +                                      (cons vcs result)
> +                                      result))
> +                                '()
> +                                (list ".bzr" ".git" ".hg" ".svn" "CVS")))
> +                   (select? (if (assq-ref opts 'exclude-vcs?)
> +                                (negate (lambda (file stat)
> +                                          (vcs-file? file stat
> +                                                     vcses)))
> +                                (const #t))))

Maybe you can have:

  (define (vcs-predicate directory)
    (define directories
      (filter (lambda (metadata-directory)
                (file-exists? (in-vicinity directory metadata-directory)))
              %known-vcs-directories))

    (lambda (file stat)
      (vcs-file? file stat directories)))

and then use that above?  (That way we don’t stat these things when
‘exclude-vcs?’ is #false.)

Thanks,
Ludo’.




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#66961] [PATCH v2] scripts: hash: Handle repository with different VCS folders.
  2023-11-05 23:00 [bug#66961] [PATCH] scripts: hash: Handle repository with different VCS folders Simon Tournier
  2023-11-14 13:44 ` Ludovic Courtès
@ 2023-11-29 20:31 ` Simon Tournier
  2023-11-29 20:54 ` [bug#66961] [PATCH v3] " Simon Tournier
  2023-12-02 13:11 ` [bug#66961] [PATCH v4] " Simon Tournier
  3 siblings, 0 replies; 10+ messages in thread
From: Simon Tournier @ 2023-11-29 20:31 UTC (permalink / raw)
  To: 66961
  Cc: Simon Tournier, Christopher Baines, Josselin Poiret,
	Ludovic Courtès, Mathieu Othacehe, Ricardo Wurmus,
	Simon Tournier, Tobias Geerinckx-Rice

Fixes <https://issues.guix.gnu.org/issue/65979>.
Reported by Simon Tournier <zimon.toutoune@gmail.com>

* guix/hash.scm (%vcs-directories): New variable.
(vcs-file?): Add optional argument for passing VCS kind of the
file/repository.
(file-hash*): Adjust accordingly.
(vcs-exclude?): New procedure and export it.
* guix/scripts/hash.scm (guix-hash): Use it.

Change-Id: I8e286c3426ddefd664dc3a471d5a09e309824faa
---
 guix/hash.scm         | 34 ++++++++++++++++++++++++++++------
 guix/scripts/hash.scm |  2 +-
 2 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/guix/hash.scm b/guix/hash.scm
index 3cb68e5c44..650def0696 100644
--- a/guix/hash.scm
+++ b/guix/hash.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
 ;;; Copyright © 2022 Maxime Devos <maximedevos@telenet.be>
+;;; Copyright © 2023 Simon Tournier <zimon.toutoune@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -23,23 +24,44 @@ (define-module (guix hash)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-11)
   #:export (vcs-file?
+            vcs-exclude?
             file-hash*))
 
-(define (vcs-file? file stat)
-  "Returns true if FILE is a version control system file."
+(define %vcs-directories
+  ;; Directory used for determining the kind of VCS.
+  (list ".bzr" ".git" ".hg" ".svn" "CVS"))
+
+(define* (vcs-file? file stat
+                    #:optional
+                    (vcs-directories %vcs-directories))
+  "Return true if FILE matches a version control system from the list
+VCSES-DIRECTORIES."
   (case (stat:type stat)
     ((directory)
-     (member (basename file) '(".bzr" ".git" ".hg" ".svn" "CVS")))
+     (member (basename file) vcs-directories))
     ((regular)
-     ;; Git sub-modules have a '.git' file that is a regular text file.
-     (string=? (basename file) ".git"))
+     (if (member ".git" vcs-directories)
+         ;; Git sub-modules have a '.git' file that is a regular text file.
+         (string=? (basename file) ".git")
+         #f))
     (else
      #f)))
 
+(define (vcs-exclude? directory)
+  "Return a procedure excluding content if DIRECTORY contains supported VCS."
+  (define vcs-directories
+    (filter (lambda (vcs)
+              (file-exists? (in-vicinity directory vcs)))
+            %vcs-directories))
+
+  (lambda (file stat)
+    (not (vcs-file? file stat vcs-directories))))
+
 (define* (file-hash* file #:key
                      (algorithm (hash-algorithm sha256))
                      (recursive? 'auto)
-                     (select? (negate vcs-file?)))
+                     (select? (negate (lambda (file stat)
+                                        (vcs-file? file stat)))))
   "Compute the hash of FILE with ALGORITHM.
 
 Symbolic links are only dereferenced if RECURSIVE? is false.
diff --git a/guix/scripts/hash.scm b/guix/scripts/hash.scm
index 7197d3965c..b6af9d5649 100644
--- a/guix/scripts/hash.scm
+++ b/guix/scripts/hash.scm
@@ -182,7 +182,7 @@ (define-command (guix-hash . args)
                            (reverse opts)))
          (fmt  (assq-ref opts 'format))
          (select? (if (assq-ref opts 'exclude-vcs?)
-                      (negate vcs-file?)
+                      (vcs-exclude? file)
                       (const #t)))
          (algorithm (assoc-ref opts 'hash-algorithm))
          (serializer (assoc-ref opts 'serializer)))

base-commit: cd46757c1a0f886848fbb6828c028dd2a2532767
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [bug#66961] [PATCH] scripts: hash: Handle repository with different VCS folders.
  2023-11-14 13:44 ` Ludovic Courtès
@ 2023-11-29 20:36   ` Simon Tournier
  2023-11-29 20:52     ` Simon Tournier
  0 siblings, 1 reply; 10+ messages in thread
From: Simon Tournier @ 2023-11-29 20:36 UTC (permalink / raw)
  To: Ludovic Courtès
  Cc: Josselin Poiret, Mathieu Othacehe, Tobias Geerinckx-Rice, 66961,
	Ricardo Wurmus, Christopher Baines

Hi Ludo,

On mar., 14 nov. 2023 at 14:44, Ludovic Courtès <ludo@gnu.org> wrote:

> How about ‘vcs-directories’ rather than ‘vcses’?
>
> Also, you can add:
>
>   (define %known-vcs-directories
>     '(".bzr" …))
>
> and use it to avoid repeating it.

[...]

> Maybe you can have:
>
>   (define (vcs-predicate directory)
>     (define directories
>       (filter (lambda (metadata-directory)
>                 (file-exists? (in-vicinity directory metadata-directory)))
>               %known-vcs-directories))
>
>     (lambda (file stat)
>       (vcs-file? file stat directories)))
>
> and then use that above?  (That way we don’t stat these things when
> ‘exclude-vcs?’ is #false.)

Thanks for the feedback.  See v2.

Cheers,
simon

PS: Sorry, I have sent twice v2; missed an option with my new
experimental setup. :-)




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#66961] [PATCH] scripts: hash: Handle repository with different VCS folders.
  2023-11-29 20:36   ` Simon Tournier
@ 2023-11-29 20:52     ` Simon Tournier
  0 siblings, 0 replies; 10+ messages in thread
From: Simon Tournier @ 2023-11-29 20:52 UTC (permalink / raw)
  To: Ludovic Courtès
  Cc: Josselin Poiret, Mathieu Othacehe, Tobias Geerinckx-Rice, 66961,
	Ricardo Wurmus, Christopher Baines

On mer., 29 nov. 2023 at 21:36, Simon Tournier <zimon.toutoune@gmail.com> wrote:

> Thanks for the feedback.  See v2.

[...]

> PS: Sorry, I have sent twice v2; missed an option with my new
> experimental setup. :-)

Sorry for the mess.  I have sent the correct one; v2 is obviously-
broken.  Therefore, please see v3.  Again sorry for the mess.

Cheers,
simon




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#66961] [PATCH v3] scripts: hash: Handle repository with different VCS folders.
  2023-11-05 23:00 [bug#66961] [PATCH] scripts: hash: Handle repository with different VCS folders Simon Tournier
  2023-11-14 13:44 ` Ludovic Courtès
  2023-11-29 20:31 ` [bug#66961] [PATCH v2] " Simon Tournier
@ 2023-11-29 20:54 ` Simon Tournier
  2023-12-02 10:39   ` Ludovic Courtès
  2023-12-02 13:11 ` [bug#66961] [PATCH v4] " Simon Tournier
  3 siblings, 1 reply; 10+ messages in thread
From: Simon Tournier @ 2023-11-29 20:54 UTC (permalink / raw)
  To: 66961
  Cc: Simon Tournier, Christopher Baines, Josselin Poiret,
	Ludovic Courtès, Mathieu Othacehe, Ricardo Wurmus,
	Simon Tournier, Tobias Geerinckx-Rice

Fixes <https://issues.guix.gnu.org/issue/65979>.
Reported by Simon Tournier <zimon.toutoune@gmail.com>

* guix/hash.scm (%vcs-directories): New variable.
(vcs-file?): Add optional argument for passing VCS kind of the
file/repository.
(file-hash*): Adjust accordingly.
(vcs-exclude?): New procedure and export it.
* guix/scripts/hash.scm (guix-hash)[file-hash]: Use it.

Change-Id: I8e286c3426ddefd664dc3a471d5a09e309824faa
---
 guix/hash.scm         | 34 ++++++++++++++++++++++++++++------
 guix/scripts/hash.scm | 10 +++++-----
 2 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/guix/hash.scm b/guix/hash.scm
index 3cb68e5c44..650def0696 100644
--- a/guix/hash.scm
+++ b/guix/hash.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
 ;;; Copyright © 2022 Maxime Devos <maximedevos@telenet.be>
+;;; Copyright © 2023 Simon Tournier <zimon.toutoune@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -23,23 +24,44 @@ (define-module (guix hash)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-11)
   #:export (vcs-file?
+            vcs-exclude?
             file-hash*))
 
-(define (vcs-file? file stat)
-  "Returns true if FILE is a version control system file."
+(define %vcs-directories
+  ;; Directory used for determining the kind of VCS.
+  (list ".bzr" ".git" ".hg" ".svn" "CVS"))
+
+(define* (vcs-file? file stat
+                    #:optional
+                    (vcs-directories %vcs-directories))
+  "Return true if FILE matches a version control system from the list
+VCSES-DIRECTORIES."
   (case (stat:type stat)
     ((directory)
-     (member (basename file) '(".bzr" ".git" ".hg" ".svn" "CVS")))
+     (member (basename file) vcs-directories))
     ((regular)
-     ;; Git sub-modules have a '.git' file that is a regular text file.
-     (string=? (basename file) ".git"))
+     (if (member ".git" vcs-directories)
+         ;; Git sub-modules have a '.git' file that is a regular text file.
+         (string=? (basename file) ".git")
+         #f))
     (else
      #f)))
 
+(define (vcs-exclude? directory)
+  "Return a procedure excluding content if DIRECTORY contains supported VCS."
+  (define vcs-directories
+    (filter (lambda (vcs)
+              (file-exists? (in-vicinity directory vcs)))
+            %vcs-directories))
+
+  (lambda (file stat)
+    (not (vcs-file? file stat vcs-directories))))
+
 (define* (file-hash* file #:key
                      (algorithm (hash-algorithm sha256))
                      (recursive? 'auto)
-                     (select? (negate vcs-file?)))
+                     (select? (negate (lambda (file stat)
+                                        (vcs-file? file stat)))))
   "Compute the hash of FILE with ALGORITHM.
 
 Symbolic links are only dereferenced if RECURSIVE? is false.
diff --git a/guix/scripts/hash.scm b/guix/scripts/hash.scm
index 7197d3965c..8982bb37b9 100644
--- a/guix/scripts/hash.scm
+++ b/guix/scripts/hash.scm
@@ -3,7 +3,7 @@
 ;;; Copyright © 2013 Nikita Karetnikov <nikita@karetnikov.org>
 ;;; Copyright © 2016 Jan Nieuwenhuizen <janneke@gnu.org>
 ;;; Copyright © 2018 Tim Gesthuizen <tim.gesthuizen@yahoo.de>
-;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
+;;; Copyright © 2021, 2023 Simon Tournier <zimon.toutoune@gmail.com>
 ;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
 ;;;
 ;;; This file is part of GNU Guix.
@@ -181,9 +181,6 @@ (define-command (guix-hash . args)
                             (_ #f))
                            (reverse opts)))
          (fmt  (assq-ref opts 'format))
-         (select? (if (assq-ref opts 'exclude-vcs?)
-                      (negate vcs-file?)
-                      (const #t)))
          (algorithm (assoc-ref opts 'hash-algorithm))
          (serializer (assoc-ref opts 'serializer)))
 
@@ -193,7 +190,10 @@ (define-command (guix-hash . args)
       (catch 'system-error
         (lambda _
           (with-error-handling
-            (serializer file algorithm select?)))
+            (let ((select? (if (assq-ref opts 'exclude-vcs?)
+                               (vcs-exclude? file)
+                               (const #t))))
+              (serializer file algorithm select?))))
         (lambda args
           (leave (G_ "~a ~a~%")
                  file

base-commit: cd46757c1a0f886848fbb6828c028dd2a2532767
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [bug#66961] [PATCH v3] scripts: hash: Handle repository with different VCS folders.
  2023-11-29 20:54 ` [bug#66961] [PATCH v3] " Simon Tournier
@ 2023-12-02 10:39   ` Ludovic Courtès
  2023-12-02 13:10     ` Simon Tournier
  0 siblings, 1 reply; 10+ messages in thread
From: Ludovic Courtès @ 2023-12-02 10:39 UTC (permalink / raw)
  To: Simon Tournier
  Cc: Josselin Poiret, Mathieu Othacehe, Tobias Geerinckx-Rice, 66961,
	Ricardo Wurmus, Christopher Baines

Hi,

Simon Tournier <zimon.toutoune@gmail.com> skribis:

> Fixes <https://issues.guix.gnu.org/issue/65979>.
> Reported by Simon Tournier <zimon.toutoune@gmail.com>
>
> * guix/hash.scm (%vcs-directories): New variable.
> (vcs-file?): Add optional argument for passing VCS kind of the
> file/repository.
> (file-hash*): Adjust accordingly.
> (vcs-exclude?): New procedure and export it.
> * guix/scripts/hash.scm (guix-hash)[file-hash]: Use it.
>
> Change-Id: I8e286c3426ddefd664dc3a471d5a09e309824faa

[...]

> +(define (vcs-exclude? directory)
> +  "Return a procedure excluding content if DIRECTORY contains supported VCS."

Nitpick: I would call it ‘vcs-file-predicate’ for consistency, with
a docstring along these lines:

  Return a two-argument procedure that returns true for version-control
  metadata directories such as '.git' found in DIRECTORY.

> +  (lambda (file stat)
> +    (not (vcs-file? file stat vcs-directories))))

… and thus removing ‘not’ here

> +            (let ((select? (if (assq-ref opts 'exclude-vcs?)
> +                               (vcs-exclude? file)

… and changing the above to (negate (vcs-file-predicate file)).

(“Return #t for VCS files” sounds clearer to me than “return true except
for VCS files” in terms of API design.)

OK with these changes!

Thanks,
Ludo’.




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#66961] [PATCH v3] scripts: hash: Handle repository with different VCS folders.
  2023-12-02 10:39   ` Ludovic Courtès
@ 2023-12-02 13:10     ` Simon Tournier
  0 siblings, 0 replies; 10+ messages in thread
From: Simon Tournier @ 2023-12-02 13:10 UTC (permalink / raw)
  To: Ludovic Courtès
  Cc: Josselin Poiret, Mathieu Othacehe, Tobias Geerinckx-Rice, 66961,
	Ricardo Wurmus, Christopher Baines

Hi,

On sam., 02 déc. 2023 at 11:39, Ludovic Courtès <ludo@gnu.org> wrote:

> Nitpick: I would call it ‘vcs-file-predicate’ for consistency,

Hum, I thought the convention for ’predicate’ was ending by ’?’.
Therefore, personally I do not find this name consistent.  Anyway…

>                                                                with
> a docstring along these lines:
>
>   Return a two-argument procedure that returns true for version-control
>   metadata directories such as '.git' found in DIRECTORY.

…did that way in v4.

Thanks for your comments.


Cheers,
simon




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#66961] [PATCH v4] scripts: hash: Handle repository with different VCS folders.
  2023-11-05 23:00 [bug#66961] [PATCH] scripts: hash: Handle repository with different VCS folders Simon Tournier
                   ` (2 preceding siblings ...)
  2023-11-29 20:54 ` [bug#66961] [PATCH v3] " Simon Tournier
@ 2023-12-02 13:11 ` Simon Tournier
  2023-12-22 16:19   ` Ludovic Courtès
  3 siblings, 1 reply; 10+ messages in thread
From: Simon Tournier @ 2023-12-02 13:11 UTC (permalink / raw)
  To: 66961
  Cc: Simon Tournier, Christopher Baines, Josselin Poiret,
	Ludovic Courtès, Mathieu Othacehe, Ricardo Wurmus,
	Simon Tournier, Tobias Geerinckx-Rice

Fixes <https://issues.guix.gnu.org/issue/65979>.
Reported by Simon Tournier <zimon.toutoune@gmail.com>

* guix/hash.scm (%vcs-directories): New variable.
(vcs-file?): Add optional argument for passing VCS kind of the
file/repository.
(file-hash*): Adjust accordingly.
(vcs-file-predicate): New procedure and export it.
* guix/scripts/hash.scm (guix-hash)[file-hash]: Use it.

Change-Id: I8e286c3426ddefd664dc3a471d5a09e309824faa
---
 guix/hash.scm         | 35 +++++++++++++++++++++++++++++------
 guix/scripts/hash.scm | 10 +++++-----
 2 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/guix/hash.scm b/guix/hash.scm
index 3cb68e5c44..81f35d63df 100644
--- a/guix/hash.scm
+++ b/guix/hash.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
 ;;; Copyright © 2022 Maxime Devos <maximedevos@telenet.be>
+;;; Copyright © 2023 Simon Tournier <zimon.toutoune@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -23,23 +24,45 @@ (define-module (guix hash)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-11)
   #:export (vcs-file?
+            vcs-file-predicate
             file-hash*))
 
-(define (vcs-file? file stat)
-  "Returns true if FILE is a version control system file."
+(define %vcs-directories
+  ;; Directory used for determining the kind of VCS.
+  (list ".bzr" ".git" ".hg" ".svn" "CVS"))
+
+(define* (vcs-file? file stat
+                    #:optional
+                    (vcs-directories %vcs-directories))
+  "Return true if FILE matches a version control system from the list
+VCSES-DIRECTORIES."
   (case (stat:type stat)
     ((directory)
-     (member (basename file) '(".bzr" ".git" ".hg" ".svn" "CVS")))
+     (member (basename file) vcs-directories))
     ((regular)
-     ;; Git sub-modules have a '.git' file that is a regular text file.
-     (string=? (basename file) ".git"))
+     (if (member ".git" vcs-directories)
+         ;; Git sub-modules have a '.git' file that is a regular text file.
+         (string=? (basename file) ".git")
+         #f))
     (else
      #f)))
 
+(define (vcs-file-predicate directory)
+  "Return a two-argument procedure that returns true when version-control
+metadata directories such as '.git' is found in DIRECTORY."
+  (define vcs-directories
+    (filter (lambda (vcs)
+              (file-exists? (in-vicinity directory vcs)))
+            %vcs-directories))
+
+  (lambda (file stat)
+    (vcs-file? file stat vcs-directories)))
+
 (define* (file-hash* file #:key
                      (algorithm (hash-algorithm sha256))
                      (recursive? 'auto)
-                     (select? (negate vcs-file?)))
+                     (select? (negate (lambda (file stat)
+                                        (vcs-file? file stat)))))
   "Compute the hash of FILE with ALGORITHM.
 
 Symbolic links are only dereferenced if RECURSIVE? is false.
diff --git a/guix/scripts/hash.scm b/guix/scripts/hash.scm
index 7197d3965c..dec037ed3f 100644
--- a/guix/scripts/hash.scm
+++ b/guix/scripts/hash.scm
@@ -3,7 +3,7 @@
 ;;; Copyright © 2013 Nikita Karetnikov <nikita@karetnikov.org>
 ;;; Copyright © 2016 Jan Nieuwenhuizen <janneke@gnu.org>
 ;;; Copyright © 2018 Tim Gesthuizen <tim.gesthuizen@yahoo.de>
-;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
+;;; Copyright © 2021, 2023 Simon Tournier <zimon.toutoune@gmail.com>
 ;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
 ;;;
 ;;; This file is part of GNU Guix.
@@ -181,9 +181,6 @@ (define-command (guix-hash . args)
                             (_ #f))
                            (reverse opts)))
          (fmt  (assq-ref opts 'format))
-         (select? (if (assq-ref opts 'exclude-vcs?)
-                      (negate vcs-file?)
-                      (const #t)))
          (algorithm (assoc-ref opts 'hash-algorithm))
          (serializer (assoc-ref opts 'serializer)))
 
@@ -193,7 +190,10 @@ (define-command (guix-hash . args)
       (catch 'system-error
         (lambda _
           (with-error-handling
-            (serializer file algorithm select?)))
+            (let ((select? (if (assq-ref opts 'exclude-vcs?)
+                               (negate (vcs-file-predicate file))
+                               (const #t))))
+              (serializer file algorithm select?))))
         (lambda args
           (leave (G_ "~a ~a~%")
                  file

base-commit: aeb494322ca9dec4a4d66a7d063239c8536bd538
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [bug#66961] [PATCH v4] scripts: hash: Handle repository with different VCS folders.
  2023-12-02 13:11 ` [bug#66961] [PATCH v4] " Simon Tournier
@ 2023-12-22 16:19   ` Ludovic Courtès
  0 siblings, 0 replies; 10+ messages in thread
From: Ludovic Courtès @ 2023-12-22 16:19 UTC (permalink / raw)
  To: Simon Tournier
  Cc: Josselin Poiret, Mathieu Othacehe, Tobias Geerinckx-Rice, 66961,
	Ricardo Wurmus, Christopher Baines

Hi,

Simon Tournier <zimon.toutoune@gmail.com> skribis:

> Fixes <https://issues.guix.gnu.org/issue/65979>.
> Reported by Simon Tournier <zimon.toutoune@gmail.com>
>
> * guix/hash.scm (%vcs-directories): New variable.
> (vcs-file?): Add optional argument for passing VCS kind of the
> file/repository.
> (file-hash*): Adjust accordingly.
> (vcs-file-predicate): New procedure and export it.
> * guix/scripts/hash.scm (guix-hash)[file-hash]: Use it.
>
> Change-Id: I8e286c3426ddefd664dc3a471d5a09e309824faa

Sorry for the long delay—LGTM!

Thanks,
Ludo’.




^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-12-22 16:20 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-05 23:00 [bug#66961] [PATCH] scripts: hash: Handle repository with different VCS folders Simon Tournier
2023-11-14 13:44 ` Ludovic Courtès
2023-11-29 20:36   ` Simon Tournier
2023-11-29 20:52     ` Simon Tournier
2023-11-29 20:31 ` [bug#66961] [PATCH v2] " Simon Tournier
2023-11-29 20:54 ` [bug#66961] [PATCH v3] " Simon Tournier
2023-12-02 10:39   ` Ludovic Courtès
2023-12-02 13:10     ` Simon Tournier
2023-12-02 13:11 ` [bug#66961] [PATCH v4] " Simon Tournier
2023-12-22 16:19   ` Ludovic Courtès

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).