[bug#56604] [PATCH 0/8] Update Clojure to 1.11.1.

[Roman Scherer <roman.scherer@burningswell.com>]

[-- Attachment #1.1: Type: text/plain, Size: 927 bytes --]


Hi Ludo,

thanks for taking a look. Here's a new patch. I changed the patch to not
use define-public and I'm exporting the repack-jar function now as per
your suggestion. I also updated the commit message to (hopefully) match
the changelog style. Is that correct now?

I did not add the #:jar-timestamp parameter because there is one more
difference. The strip-jar-timestamps function in the Ant build system
changes the timestamp of all files in the JAR. The
reset-class-timestamps function in the Clojure build system sets the
timestamp of only the class files.

I could add another parameter called "extension" to the
strip-jar-timestamps function if you prefer that. It would default to
"*", matching all filenames and use it with "*.class" from the Clojure
build system. I have the feeling the repack-jar function is more
flexible as is, but I'm open to doing this change or any other
suggestion you have.

Wdyt?

Roman


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-build-system-Remove-code-duplication-in-Ant-Clojure-.patch --]
[-- Type: text/x-diff, Size: 7459 bytes --]

From c3e06172044264ff871cbe8637236415bfd6077f Mon Sep 17 00:00:00 2001
From: Roman Scherer <roman@burningswell.com>
Date: Mon, 15 Aug 2022 15:29:25 +0000
Subject: [PATCH] build-system: Remove code duplication in Ant & Clojure build
 systems

* guix/build/ant-build-system.scm (repack-jar): Add repack-jar function to
unpack, modify, and repack a JAR file.
* guix/build/ant-build-system.scm (strip-jar-timestamps): Use the repack-jar
function to set the timestamps of all files in the JAR file to 1980-01-01.
* guix/build/clojure-build-system.scm (reset-class-timestamps): Use the
repack-jar function from the Ant build system to set the timestamps of all
class files in the JAR file to 1980-01-02 to prevent the Clojure compiler from
re-compiling Clojure source files.
---
 guix/build/ant-build-system.scm     | 29 ++++++++-------
 guix/build/clojure-build-system.scm | 55 +++++------------------------
 2 files changed, 26 insertions(+), 58 deletions(-)

diff --git a/guix/build/ant-build-system.scm b/guix/build/ant-build-system.scm
index fae1b47ec5..dec261cb82 100644
--- a/guix/build/ant-build-system.scm
+++ b/guix/build/ant-build-system.scm
@@ -27,7 +27,8 @@ (define-module (guix build ant-build-system)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
   #:export (%standard-phases
-            ant-build))
+            ant-build
+            repack-jar))
 
 ;; Commentary:
 ;;
@@ -195,10 +196,9 @@ (define (generate-index jar)
             outputs)
   #t)
 
-(define* (strip-jar-timestamps #:key outputs
-                               #:allow-other-keys)
-  "Unpack all jar archives, reset the timestamp of all contained files, and
-repack them.  This is necessary to ensure that archives are reproducible."
+(define (repack-jar outputs repack-fn)
+  "Unpack all jar archives, invoke repack-fn for each JAR with the directory
+it has been unpacked to, and pack them again."
   (define (repack-archive jar)
     (format #t "repacking ~a\n" jar)
     (let* ((dir (mkdtemp! "jar-contents.XXXXXX"))
@@ -206,13 +206,7 @@ (define (repack-archive jar)
       (with-directory-excursion dir
         (invoke "jar" "xf" jar))
       (delete-file jar)
-      ;; XXX: copied from (gnu build install)
-      (for-each (lambda (file)
-                  (let ((s (lstat file)))
-                    (unless (eq? (stat:type s) 'symlink)
-                      (utime file 0 0 0 0))))
-                (find-files dir #:directories? #t))
-
+      (repack-fn dir)
       ;; The jar tool will always set the timestamp on the manifest file
       ;; and the containing directory to the current time, even when we
       ;; reuse an existing manifest file.  To avoid this we use "zip"
@@ -237,6 +231,17 @@ (define (repack-archive jar)
             outputs)
   #t)
 
+(define* (strip-jar-timestamps #:key outputs
+                               #:allow-other-keys)
+  "Unpack all jar archives, reset the timestamp of all contained files, and
+repack them.  This is necessary to ensure that archives are reproducible."
+  (repack-jar outputs (lambda (dir)
+                        (for-each (lambda (file)
+                                    (let ((s (lstat file)))
+                                      (unless (eq? (stat:type s) 'symlink)
+                                        (utime file 0 0 0 0))))
+                                  (find-files dir #:directories? #t)))))
+
 (define* (check #:key target (make-flags '()) (tests? (not target))
                 (test-target "check")
                 #:allow-other-keys)
diff --git a/guix/build/clojure-build-system.scm b/guix/build/clojure-build-system.scm
index cacbefb386..b82ebc30fe 100644
--- a/guix/build/clojure-build-system.scm
+++ b/guix/build/clojure-build-system.scm
@@ -19,7 +19,7 @@
 (define-module (guix build clojure-build-system)
   #:use-module ((guix build ant-build-system)
                 #:select ((%standard-phases . %standard-phases@ant)
-                          ant-build))
+                          ant-build repack-jar))
   #:use-module (guix build clojure-utils)
   #:use-module (guix build java-utils)
   #:use-module (guix build syscalls)
@@ -112,54 +112,17 @@ (define* (check #:key
                   jar-names)))
   #t)
 
-(define (regular-jar-file? file stat)
-  "Predicate returning true if FILE is ending on '.jar'
-and STAT indicates it is a regular file."
-    (and (string-suffix? ".jar" file)
-         (eq? 'regular (stat:type stat))))
-
-;; XXX: The only difference compared to 'strip-jar-timestamps' in
-;; ant-build-system.scm is the date.  TODO: Adjust and factorize.
 (define* (reset-class-timestamps #:key outputs #:allow-other-keys)
   "Unpack all jar archives, reset the timestamp of all contained class files,
 and repack them.  This is necessary to ensure that archives are reproducible."
-  (define (repack-archive jar)
-    (format #t "resetting class timestamps and repacking ~a\n" jar)
-
-    ;; Note: .class files need to be strictly newer than source files,
-    ;; otherwise the Clojure compiler will recompile sources.
-    (let* ((early-1980 315619200) ; 1980-01-02 UTC
-           (dir (mkdtemp! "jar-contents.XXXXXX"))
-           (manifest (string-append dir "/META-INF/MANIFEST.MF")))
-      (with-directory-excursion dir
-        (invoke "jar" "xf" jar))
-      (delete-file jar)
-      (for-each (lambda (file)
-                  (let ((s (lstat file)))
-                    (unless (eq? (stat:type s) 'symlink)
-                      (when (string-match "^(.*)\\.class$" file)
-                        (utime file early-1980 early-1980)))))
-                (find-files dir #:directories? #t))
-      ;; The jar tool will always set the timestamp on the manifest file
-      ;; and the containing directory to the current time, even when we
-      ;; reuse an existing manifest file.  To avoid this we use "zip"
-      ;; instead of "jar".  It is important that the manifest appears
-      ;; first.
-      (with-directory-excursion dir
-        (let* ((files (find-files "." ".*" #:directories? #t))
-               ;; To ensure that the reference scanner can detect all
-               ;; store references in the jars we disable compression
-               ;; with the "-0" option.
-               (command (if (file-exists? manifest)
-                            `("zip" "-0" "-X" ,jar ,manifest ,@files)
-                            `("zip" "-0" "-X" ,jar ,@files))))
-          (apply invoke command)))
-      (utime jar 0 0)))
-  (for-each (match-lambda
-              ((output . directory)
-               (for-each repack-archive
-                         (find-files directory regular-jar-file?))))
-            outputs))
+  (repack-jar outputs (lambda (dir)
+                        (for-each (lambda (file)
+                                    (let ((s (lstat file))
+                                          (early-1980 315619200)) ; 1980-01-02 UTC
+                                      (unless (eq? (stat:type s) 'symlink)
+                                        (when (string-match "^(.*)\\.class$" file)
+                                          (utime file early-1980 early-1980)))))
+                                  (find-files dir #:directories? #t)))))
 
 (define-with-docs install
   "Standard 'install' phase for clojure-build-system."
-- 
2.37.2


[-- Attachment #1.3: Type: text/plain, Size: 3667 bytes --]


Ludovic Courtès <ludo@gnu.org> writes:

> Hi Roman,
>
> (Catching up after vacation…)
>
> Roman Scherer <roman.scherer@burningswell.com> skribis:
>
>> here's the promised patch to follow up with the code duplication I
>> introduced in my previous patch.
>
> Awesome.
>
>> When I run the following commands after modifying the build systems they
>> run for quite some time, because they were compiling a ton (the jdk,
>> jetty) of things.
>>
>> ./pre-inst-env guix build clojure
>> ./pre-inst-env guix build clojure-tools
>>
>> I guess this is expected, since a change in a build system might affect
>> all packages being built with it. But I was wondering if there is a way
>> to force only building the packages specified on the command line. Does
>> such a thing exists?
>
> No, it doesn’t exist, because that would be building something
> different.  In this case, building everything that depends on
> ‘ant-build-system.scm’ is unavoidable.
>
>> I was wondering what is the most efficient way to quickly iterate on
>> changes to a build system, without recompiling the whole world for that
>> build system. How would you do that?
>
> There’s no ideal solution as you’ll have to recompile the world anyway.
>
> When changing build systems, I’d usually stare at my changes for quite
> some time first, to make sure I don’t have to rebuild the world on the
> next day because of a typo.  :-)
>
> Then, for small local changes, I’d build just the bottom of the
> dependency graph to check for breakage (in this case, making sure the
> ‘strip-jar-timestamps’ phase still works as intended).  Then we can let
> ci.guix build the whole thing afterwards, and make sure nothing goes
> wrong.
>
>> From 756bfd3458ded38e1041ebb255c6b6ffe737732d Mon Sep 17 00:00:00 2001
>> From: Roman Scherer <roman@burningswell.com>
>> Date: Mon, 15 Aug 2022 15:29:25 +0000
>> Subject: [PATCH] build-system: Add repack-jar and use it in Ant & Clojure
>>  build systems
>>
>> * guix/build/ant-build-system.scm: Add repack-jar and use it in strip-jar-timestamps
>> * guix/build/clojure-build-system.scm: Use repack-jar in reset-class-timestamps
>
> Please use the ChangeLog format, specifying procedure/variable names and
> their actual changes.
>
> [...]
>
>> +(define-public (repack-jar outputs repack-fn)
>> +  "Unpack all jar archives, invoke repack-fn for each JAR with the directory
>> +it has been unpacked to, and pack them again."
>
> Instead of ‘define-public’, I’d move ‘repack-jar’ to #:export at the
> top, like the other procedures.
>
> But…
>
>> @@ -206,13 +205,7 @@ (define (repack-archive jar)
>>        (with-directory-excursion dir
>>          (invoke "jar" "xf" jar))
>>        (delete-file jar)
>> -      ;; XXX: copied from (gnu build install)
>> -      (for-each (lambda (file)
>> -                  (let ((s (lstat file)))
>> -                    (unless (eq? (stat:type s) 'symlink)
>> -                      (utime file 0 0 0 0))))
>> -                (find-files dir #:directories? #t))
>> -
>> +      (repack-fn dir)
>
> Looking at the code, the only difference between the two repack
> functions is the timestamp, right?  In that case, I’d lean towards
> adding a #:jar-timestamp parameter to ‘strip-jar-timestamps’ (rather than
> this ‘repack-fn’ argument) that’d be passed to ‘utime’.  The default for
> #:jar-timestamp would be '(0 0 0 0); for Clojure we’d set the default
> #:jar-timestamp in (guix build-system clojure) to:
>
>   #:jar-timestamp (list early-1980 early-1980)
>
> WDYT?
>
> Thanks,
> Ludo’.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 528 bytes --]