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

[Roman Scherer]

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

0001-build-system-Remove-code-duplication-in-Ant-Clojure-.patch
Description: Text Data

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’.

signature.asc
Description: PGP signature