Subject: bug#45891: closed (Re: bug#45891: [PATCH] packages: 'patch-and-repack' returns a directory when given a directory.)

From: [email-masked] (GNU bug Tracking System)
Date: Mon, 18 Jan 2021 14:57:02 +0000

Your bug report

#45891: [PATCH] packages: 'patch-and-repack' returns a directory when given a directory.

which was filed against the guix-patches package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
45891: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=45891
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#45891: [PATCH] packages: 'patch-and-repack' returns a directory when given a directory.
From: Ludovic Courtès <[email-masked]>
To: Maxim Cournoyer <[email-masked]>
Cc: [email-masked]
Date: Mon, 18 Jan 2021 15:56:49 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

Hi!

Maxim Cournoyer <[email-masked]> skribis:

> LGTM.  Feel free to push!  About my related change that we thought was
> conflicting with this one; it at least applies on top of your change,
> and only one test fails currently, I'm working on a fix.

Alright.

> Feel free to push to core-updates!

Done, thanks!

Ludo’.

Subject: [PATCH] packages: 'patch-and-repack' returns a directory when given a directory.
From: Ludovic Courtès <[email-masked]>
To: [email-masked]
Cc: Ludovic Courtès <[email-masked]>
Date: Fri, 15 Jan 2021 14:15:48 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

Previously, 'patch-and-repack' would always create a tar.xz archive as a
result, even if the input was a directory (a checkout).  This change
reduces gratuitous CPU and storage overhead.

* guix/packages.scm (patch-and-repack)[tarxz-name]: Remove 'checkout?' case.
[build](repack): New procedure, with "tar" invocation formerly at the
top level.
If SOURCE is a directory, call 'copy-recursively'; otherwise, call
'repack'.
Change NAME to ORIGINAL-FILE-NAME when it matches 'checkout?'.
---
 guix/packages.scm | 65 ++++++++++++++++++++++++++---------------------
 1 file changed, 36 insertions(+), 29 deletions(-)

Hi!

This change is a followup to a recent IRC discussion: it makes
‘patch-and-repack’ preserve the “directoriness” of its input.

It conflicts with other changes Maxim posted at
<https://issues.guix.gnu.org/45773>, though that could be addressed.

Thoughts?

Ludo’.

diff --git a/guix/packages.scm b/guix/packages.scm
index 4caaa9cb79..cd2cded9ee 100644
--- a/guix/packages.scm
+++ b/guix/packages.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020 Ludovic Courtès <[email-masked]>
+;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020, 2021 Ludovic Courtès <[email-masked]>
 ;;; Copyright © 2014, 2015, 2017, 2018 Mark H Weaver <[email-masked]>
 ;;; Copyright © 2015 Eric Bavier <[email-masked]>
 ;;; Copyright © 2016 Alex Kost <[email-masked]>
@@ -635,11 +635,9 @@ specifies modules in scope when evaluating SNIPPET."
 
   (define (tarxz-name file-name)
     ;; Return a '.tar.xz' file name based on FILE-NAME.
-    (let ((base (cond ((numeric-extension? file-name)
-                       original-file-name)
-                      ((checkout? file-name)
-                       (string-drop-right file-name 9))
-                      (else (file-sans-extension file-name)))))
+    (let ((base (if (numeric-extension? file-name)
+                    original-file-name
+                    (file-sans-extension file-name))))
       (string-append base
                      (if (equal? (file-extension base) "tar")
                          ".xz"
@@ -689,6 +687,29 @@ specifies modules in scope when evaluating SNIPPET."
                             (lambda (name)
                               (not (member name '("." "..")))))))
 
+            (define (repack directory output)
+              ;; Write to OUTPUT a compressed tarball containing DIRECTORY.
+              (unless tar-supports-sort?
+                (call-with-output-file ".file_list"
+                  (lambda (port)
+                    (for-each (lambda (name)
+                                (format port "~a~%" name))
+                              (find-files directory
+                                          #:directories? #t
+                                          #:fail-on-error? #t)))))
+
+              (apply invoke #+(file-append tar "/bin/tar")
+                     "cvfa" output
+                     ;; Avoid non-determinism in the archive.  Set the mtime
+                     ;; to 1 as is the case in the store (software like gzip
+                     ;; behaves differently when it stumbles upon mtime = 0).
+                     "--mtime=@1"
+                     "--owner=root:0" "--group=root:0"
+                     (if tar-supports-sort?
+                         `("--sort=name" ,directory)
+                         '("--no-recursion"
+                           "--files-from=.file_list"))))
+
             ;; Encoding/decoding errors shouldn't be silent.
             (fluid-set! %default-port-conversion-strategy 'error)
 
@@ -742,30 +763,16 @@ specifies modules in scope when evaluating SNIPPET."
 
               (chdir "..")
 
-              (unless tar-supports-sort?
-                (call-with-output-file ".file_list"
-                  (lambda (port)
-                    (for-each (lambda (name)
-                                (format port "~a~%" name))
-                              (find-files directory
-                                          #:directories? #t
-                                          #:fail-on-error? #t)))))
-              (apply invoke
-                     (string-append #+tar "/bin/tar")
-                     "cvfa" #$output
-                     ;; Avoid non-determinism in the archive.  Set the mtime
-                     ;; to 1 as is the case in the store (software like gzip
-                     ;; behaves differently when it stumbles upon mtime = 0).
-                     "--mtime=@1"
-                     "--owner=root:0"
-                     "--group=root:0"
-                     (if tar-supports-sort?
-                         `("--sort=name"
-                           ,directory)
-                         '("--no-recursion"
-                           "--files-from=.file_list")))))))
+              ;; If SOURCE is a directory (such as a checkout), return a
+              ;; directory.  Otherwise create a tarball.
+              (if (file-is-directory? #+source)
+                  (copy-recursively directory #$output
+                                    #:log (%make-void-port "w"))
+                  (repack directory #$output))))))
 
-    (let ((name (tarxz-name original-file-name)))
+    (let ((name (if (checkout? original-file-name)
+                    original-file-name
+                    (tarxz-name original-file-name))))
       (gexp->derivation name build
                         #:graft? #f
                         #:system system
-- 
2.30.0