bug#30104: [PATCH] gnu: ant-bootstrap: Make build reproducible.

[Ludovic Courtès]

Hello!

Gábor Boskovits <address@hidden> skribis:

> * gnu/packages/java.scm 
> (ant-bootstrap)[arguments]<#:phases>[strip-jar-timestamps]:
> New phase to reset jar file timestamps.
> <#:imported-modules>: Import guix build syscalls, gnu-build-system-modules.
> <#:modules>: Import guix build: gnu-build-system, utils, syscalls.
> [native-inputs]: Add unzip, zip.

Great that you’re tackling this issue!

I have very minor comments:

> +         (add-after 'build 'strip-jar-timestamps ;based on ant-build-system
> +           (lambda* (#:key outputs #:allow-other-keys)
> +             (define (repack-archive jar)
> +               (let* ((dir (mkdtemp! "jar-contents.XXXXXX"))
> +                      (manifest (string-append dir 
> "/META-INF/MANIFESTS.MF")))
> +                 (and (with-directory-excursion dir
> +                        (invoke "unzip" jar))
> +                      (delete-file jar)

‘invoke’ throws an exception upon failure, and the return value of
‘delete-file’ is unspecified, so you shouldn’t rely on it.

Thus, it should be written without ‘and’:

  (with-directory-excursion
    (invoke "unzip" jar))
  (delete-file jar)
  …

> +                      ;; It is important that the manifes appears first.
                                                            ^
Typo.

> +                      (utime jar 0 0)

I think this is unnecessary because guix-daemon resets timestamps on all
the file upon build completion.

> +             (every repack-archive
> +                    (find-files
> +                     (string-append (assoc-ref %outputs "out") "/lib")
> +                     "\\.jar$"))))

Use ‘for-each’ instead of ‘every’ (because the return value of
‘repack-archive’ is undefined/unused), and add a trailing #t to denote
success.

If Ricardo and Julien don’t have anything to add, I think you can go
ahead and push with these changes.

Thank you!

Ludo’.