| Message ID | cover.1657994905.git.roman@burningswell.com |
|---|---|
| Headers | show |
| Series | Update Clojure to 1.11.1. | expand |
Sorry, somehow I messed up the the subject lines a bit :/
> This phase makes sure the timestamp of compiled class files is set to a later > point in time than the timestamp of the corresponding Clojure source files. If Please check that this doesn't create reproducibility issues if you haven't already (I haven't looked at the related patches.) r0man schreef op za 16-07-2022 om 20:17 [+0200]: > Are we sure that AOT compiling all Clojure source files by default is > a good > idea, instead of just compiling user declared namespaces which > Leiningen and > friends are doing? WDYT? I think it's a good idea for the same reason that incremental compilation is good and the current Rust packaging is bad, but I realise this is not supported in general for Clojure so we might often need to disable it. Anyway, possibly related previous e-mails: https://issues.guix.gnu.org/53765#47 and other AOT mails > + '(;; Disabled AOT, because of failing test: No implementation > of > + ;; method: :conj-flat of protocol: > + ;; #'instaparse.auto-flatten-seq/ConjFlat found for class: > + ;; instaparse.auto_flatten_seq.AutoFlattenSeq It had been suggested that disabling AOT for test files sometimes is sufficient, is this the case here? Greetongs, Maxime.
Hi, r0man <roman@burningswell.com> skribis: > This phase makes sure the timestamp of compiled class files is set to a later > point in time than the timestamp of the corresponding Clojure source files. If > the timestamps of the class and source files are the same, the Clojure > compiler will compile the sources again which can lead to issues. This problem > has been discussed here [1]. The suggested solution was to keep/adjust the > timestamps of the class files. Sounds reasonable. It’s a bummer though that the whole phase is pasted from ant-build-system.scm, the only difference being the timestamps (1980 instead of 1970). I added a TODO comment in clojure-build-system.scm when applying the patch. Could you follow up with a patch to factorize that? > Btw, I was a bit surprised that in Guix Clojure packages are AOT compiled. The > general wisdom in the Clojure community seems to be to avoid AOT compilation > when distributing libraries, and only AOT compiling Uberjars for final > deployment. Due to issues like I mentioned in clojure-instaparse. > > Are we sure that AOT compiling all Clojure source files by default is a good > idea, instead of just compiling user declared namespaces which Leiningen and > friends are doing? WDYT? Not much, but as you might have seen in ./etc/teams.scm, the project is finally being structured as teams. There’s an opportunity for you to start a Clojure team and to take the lead! :-) As a first step, I’d recommend getting in touch with people who have worked on ‘clojure-build-system’ and packaged things in the past. > gnu: clojure-tools-cli: Update to 1.0.206. > gnu: clojure-tools-gitlibs: Update to 2.4.181. > gnu: clojure-tools-deps-alpha: Update to 0.14.1212. > gnu: clojure-tools: Update to 1.11.1.1149. > gnu: clojure: Update to 1.11.1. > gnu: clojure-algo-generic: Fix test failing under AOT in Clojure 1.11.1. > gnu: clojure-core-match: Update to 1.0.0. > gnu: clojure-instaparse: Update to 1.4.12 (disabled AOT). I adjusted all the commit logs to follow our conventions; please consider doing this next time: https://guix.gnu.org/manual/devel/en/html_node/Submitting-Patches.html The instaparse patch missed the hash update so I did that too. Thanks! Ludo’.
Hi Maxim and Ludo, thanks for your replies. Sorry that I was not answering earlier. The heat killed my laptop and I am having trouble to get set up again. I follow up when I have my new laptop. Thanks On Sat, Jul 23, 2022, 00:12 GNU bug Tracking System <help-debbugs@gnu.org> wrote: > Your bug report > > #56604: [PATCH 0/8] Update Clojure to 1.11.1. > > 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 56604@debbugs.gnu.org. > > -- > 56604: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=56604 > GNU Bug Tracking System > Contact help-debbugs@gnu.org with problems > > > > ---------- Forwarded message ---------- > From: "Ludovic Courtès" <ludo@gnu.org> > To: r0man <roman@burningswell.com> > Cc: 56604-done@debbugs.gnu.org > Bcc: > Date: Sat, 23 Jul 2022 00:11:05 +0200 > Subject: Re: bug#56604: [PATCH 0/8] Update Clojure to 1.11.1. > Hi, > > r0man <roman@burningswell.com> skribis: > > > This phase makes sure the timestamp of compiled class files is set to a > later > > point in time than the timestamp of the corresponding Clojure source > files. If > > the timestamps of the class and source files are the same, the Clojure > > compiler will compile the sources again which can lead to issues. This > problem > > has been discussed here [1]. The suggested solution was to keep/adjust > the > > timestamps of the class files. > > Sounds reasonable. It’s a bummer though that the whole phase is pasted > from ant-build-system.scm, the only difference being the timestamps > (1980 instead of 1970). > > I added a TODO comment in clojure-build-system.scm when applying the > patch. Could you follow up with a patch to factorize that? > > > Btw, I was a bit surprised that in Guix Clojure packages are AOT > compiled. The > > general wisdom in the Clojure community seems to be to avoid AOT > compilation > > when distributing libraries, and only AOT compiling Uberjars for final > > deployment. Due to issues like I mentioned in clojure-instaparse. > > > > Are we sure that AOT compiling all Clojure source files by default is a > good > > idea, instead of just compiling user declared namespaces which Leiningen > and > > friends are doing? WDYT? > > Not much, but as you might have seen in ./etc/teams.scm, the project is > finally being structured as teams. There’s an opportunity for you to > start a Clojure team and to take the lead! :-) > > As a first step, I’d recommend getting in touch with people who have > worked on ‘clojure-build-system’ and packaged things in the past. > > > gnu: clojure-tools-cli: Update to 1.0.206. > > gnu: clojure-tools-gitlibs: Update to 2.4.181. > > gnu: clojure-tools-deps-alpha: Update to 0.14.1212. > > gnu: clojure-tools: Update to 1.11.1.1149. > > gnu: clojure: Update to 1.11.1. > > gnu: clojure-algo-generic: Fix test failing under AOT in Clojure > 1.11.1. > > gnu: clojure-core-match: Update to 1.0.0. > > gnu: clojure-instaparse: Update to 1.4.12 (disabled AOT). > > I adjusted all the commit logs to follow our conventions; please > consider doing this next time: > > https://guix.gnu.org/manual/devel/en/html_node/Submitting-Patches.html > > The instaparse patch missed the hash update so I did that too. > > Thanks! > > Ludo’. > > > > > ---------- Forwarded message ---------- > From: r0man <roman@burningswell.com> > To: guix-patches@gnu.org > Cc: > Bcc: > Date: Sat, 16 Jul 2022 20:17:34 +0200 > Subject: [PATCH 0/8] Update Clojure to 1.11.1. > Hello Guix, > > This patch series updates Clojure and it's packages to 1.11.1. > > It also adds the 'reset-class-timestamps phase to the Clojure build system. > > This phase makes sure the timestamp of compiled class files is set to a > later > point in time than the timestamp of the corresponding Clojure source > files. If > the timestamps of the class and source files are the same, the Clojure > compiler will compile the sources again which can lead to issues. This > problem > has been discussed here [1]. The suggested solution was to keep/adjust the > timestamps of the class files. > > [1] https://www.mail-archive.com/clojure@googlegroups.com/msg99928.html > > Could you please review this? > > In the patch for clojure-instaparse, I disabled the AOT compilation > because of > a known issue. Is that ok? > > Btw, I was a bit surprised that in Guix Clojure packages are AOT compiled. > The > general wisdom in the Clojure community seems to be to avoid AOT > compilation > when distributing libraries, and only AOT compiling Uberjars for final > deployment. Due to issues like I mentioned in clojure-instaparse. > > Are we sure that AOT compiling all Clojure source files by default is a > good > idea, instead of just compiling user declared namespaces which Leiningen > and > friends are doing? WDYT? > > Thanks Roman. > > r0man (8): > gnu: clojure-tools-cli: Update to 1.0.206. > gnu: clojure-tools-gitlibs: Update to 2.4.181. > gnu: clojure-tools-deps-alpha: Update to 0.14.1212. > gnu: clojure-tools: Update to 1.11.1.1149. > gnu: clojure: Update to 1.11.1. > gnu: clojure-algo-generic: Fix test failing under AOT in Clojure 1.11.1. > gnu: clojure-core-match: Update to 1.0.0. > gnu: clojure-instaparse: Update to 1.4.12 (disabled AOT). > > gnu/packages/clojure.scm | 185 +++++++++++++++++++--------- > guix/build/clojure-build-system.scm | 53 +++++++- > 2 files changed, 178 insertions(+), 60 deletions(-) > > -- > 2.36.1 > >