From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39641) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1evBG4-0001BJ-4U for guix-patches@gnu.org; Sun, 11 Mar 2018 20:24:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1evBFz-00029S-3J for guix-patches@gnu.org; Sun, 11 Mar 2018 20:24:08 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:47957) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1evBFy-00029I-VO for guix-patches@gnu.org; Sun, 11 Mar 2018 20:24:03 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1evBFy-0003Wn-NM for guix-patches@gnu.org; Sun, 11 Mar 2018 20:24:02 -0400 Subject: bug#30351: [PATCH] gnu: mcron2: Correct and enhance the wrap phase. Resent-To: guix-patches@gnu.org Resent-Message-ID: From: Maxim Cournoyer References: <874lmwwkgn.fsf@gmail.com> <87h8qq4gpg.fsf@gnu.org> Date: Sun, 11 Mar 2018 20:23:47 -0400 In-Reply-To: <87h8qq4gpg.fsf@gnu.org> ("Ludovic \=\?utf-8\?Q\?Court\=C3\=A8s\=22'\?\= \=\?utf-8\?Q\?s\?\= message of "Fri, 09 Feb 2018 11:09:47 +0100") Message-ID: <87d10af8fg.fsf@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: Ludovic =?UTF-8?Q?Court=C3=A8s?= Cc: 30351-done@debbugs.gnu.org Hello! ludo@gnu.org (Ludovic Court=C3=A8s) writes: > Hi Maxim, > > Maxim Cournoyer skribis: > >> As a follow up to my previous patch to mcron2, this completes the wrap >> phases and fix a small issue where the mcron modules were installed to >> share/guile/site/2.0 instead of 2.2. > > Good catch. I=E2=80=99ve committed that change separately. > > BTW, I encourage you to submit changes to bug-mcron@gnu.org. Mcron is > now maintained by Mathieu Lirzin, who is familiar with current Guile and > certainly interested in receiving improvements like this. > > (Another thing that should be done would be install .go files in > $libdir/guile/2.2/site-ccache.) It's already all fixed upstream! I verified myself. I guess we could poke Mathieu for a new release ;). >> For those using dyndns services, a job like the following should now >> work without having to propagate Guile or GnuTLS in your user/system >> profile[0]: > > [...] > >>>>From 385343b1370d87e6104ebe2ef473bf2d1e31f2f2 Mon Sep 17 00:00:00 2001 >> From: Maxim Cournoyer >> Date: Sun, 4 Feb 2018 14:05:40 -0500 >> Subject: [PATCH] gnu: mcron2: Correct and enhance the wrap phase. >> >> * gnu/packages/guile.scm (mcron2)[inputs]: Rename "guile-2.2" to just "g= uile". >> [phases]: Install mcron2 modules to guile/site/2.2 instead of guile/site= /2.0. >> Add Guile 2.2 and GnuTLS modules to the wrap phase. > > [...] > >> (add-after 'install 'wrap-mcron >> - (lambda* (#:key outputs #:allow-other-keys) >> - ;; Wrap the 'mcron' command to refer to the right >> - ;; modules. >> - (let* ((out (assoc-ref outputs "out")) >> - (bin (string-append out "/bin")) >> - (site (string-append >> - out "/share/guile/site"))) >> - (match (scandir site) >> - (("." ".." version) >> - (let ((modules (string-append site "/" version))) >> - (wrap-program (string-append bin "/mcron") >> - `("GUILE_LOAD_PATH" ":" prefix >> - (,modules)) >> - `("GUILE_LOAD_COMPILED_PATH" ":" prefix >> - (,modules))) >> - #t)))))))))))) >> + (lambda* (#:key inputs outputs #:allow-other-keys) >> + ;; Wrap the 'mcron' command to refer to the right module= s. We >> + ;; also include Guile 2.2 modules and GnuTLS, so that Gu= ile >> + ;; libraries can be used in mcron jobs without having to >> + ;; propagate those in a user profile. >> + (let* ((site-dir "/share/guile/site/2.2") >> + (ccache-dir "/lib/guile/2.2/ccache") >> + (mcron (assoc-ref outputs "out")) >> + (mcron-bin (string-append mcron "/bin/mcron")) >> + (mcron-modules (string-append mcron site-dir)) >> + (guile (assoc-ref inputs "guile")) >> + (guile-modules (string-append guile site-dir)) >> + (guile-ccache (string-append guile ccache-dir)) >> + (gnutls (assoc-ref inputs "gnutls")) >> + (gnutls-modules (string-append gnutls site-dir)) >> + (gnutls-ccache (string-append gnutls ccache-dir))) >> + (wrap-program mcron-bin >> + `("GUILE_LOAD_PATH" ":" prefix >> + (,mcron-modules ,guile-modules ,gnutls-modules)) >> + `("GUILE_LOAD_COMPILED_PATH" ":" prefix >> + (,mcron-modules ,guile-ccache ,gnutls-ccache))) >> + #t))))))))) > > A couple of issues here: the =E2=80=98scandir=E2=80=99 trick above allowe= d us to not > hard-code =E2=80=9C2.2=E2=80=9D. I think it=E2=80=99d be nice to try to = preserve such things; > it=E2=80=99ll be less pain down the road. > > Second issue is about adding GnuTLS to the search path: it=E2=80=99s not = the > right place for that. What if someone wants Guile-JSON? And Guile-Git? > And=E2=80=A6 You get the idea. :-) > > Instead I=E2=80=99d suggest writing your mcron job along these lines: > > #~(begin > (add-to-load-path (string-append #+gnutls "/share/guile/site/" > (effective-version))) > (use-modules (web client)) > =E2=80=A6) > > (See (guix download) for an example of this hack.) Thanks for sharing this idea; it makes sense. I hadn't thought about manipulating the load-path at run time. > The extra boilerplate is admittedly not great, so I=E2=80=99d like to add= a > =E2=80=98with-extensions=E2=80=99 or =E2=80=98with-imported-packages=E2= =80=99 form that would be like > =E2=80=99with-imported-modules=E2=80=99 but for Guile =E2=80=9Cextensions= =E2=80=9D like GnuTLS. I don't think it's that bad as it is. Any enhancements welcome, of course :). > Last thing: it=E2=80=99s not necessary to put Guile=E2=80=99s own module = directories in > the search path. They=E2=80=99re already there by default. Tried, and indeed it works without it. Thanks. I must have gotten confused somewhere. > So overall I think I=E2=80=99m arguing for the status quo. Would that wo= rk for > you? I'm happy with status quo as well. Thanks for lending your sharp eyes to this review! Maxim