Subject: [bug#28765] [PATCH] config: Conditionally configure daemon offload script.

From: [email-masked] (Ludovic Courtès)
Date: Tue, 10 Oct 2017 16:17:46 +0200

Eric Bavier <[email-masked]> skribis:

> On Tue, 10 Oct 2017 08:49:49 +0200
> [email-masked] (Ludovic Courtès) wrote:
>
>> Hello,
>> 
>> Eric Bavier <[email-masked]> skribis:
>> 
>> > On Mon, 09 Oct 2017 23:07:10 +0200
>> > [email-masked] (Ludovic Courtès) wrote:
>> >  
>> >> Eric Bavier <[email-masked]> skribis:
>> >>   
>> >> > From 984e324370c2c17d8d1a982adf2884112c9e64b7 Mon Sep 17 00:00:00 2001
>> >> > From: Eric Bavier <[email-masked]>
>> >> > Date: Mon, 9 Oct 2017 13:58:04 -0500
>> >> > Subject: [PATCH] config: Conditionally configure daemon offload script.    
>> >>                    ^
>> >> Nitpick: rather “build:”, which is for all things build-related.
>> >>   
>> >> > * config-daemon.ac (nix/scripts/offload): Configure only if offloading enabled.    
>> >> 
>> >> It LGTM, but is it helpful?  guix-daemon does not invoke ‘guix offload’
>> >> when it’s missing, as can be seen in guix-daemon.cc:
>> >> 
>> >>   #ifdef HAVE_DAEMON_OFFLOAD_HOOK
>> >>         /* Use our build hook for distributed builds by default.  */
>> >>   …  
>> >
>> > Or maybe something like this would be preferable, to avoid exporting
>> > NIX_BUILD_HOOK if Guix has been configured to disable the daemon
>> > offload hook?
>> >
>> > --- a/build-aux/pre-inst-env.in
>> > +++ b/build-aux/pre-inst-env.in
>> > @@ -50,13 +50,9 @@ NIX_LIBEXEC_DIR="@abs_top_builddir@/nix/scripts" # for 'guix-authenticate'
>> >  export NIX_ROOT_FINDER NIX_SUBSTITUTERS NIX_LIBEXEC_DIR
>> >  
>> >  NIX_BUILD_HOOK="$abs_top_builddir/nix/scripts/offload"
>> > -if [ -x "$NIX_BUILD_HOOK" ]
>> > -then
>> > -    export NIX_BUILD_HOOK
>> > -else
>> > -    # No offloading support.
>> > -    unset NIX_BUILD_HOOK
>> > -fi
>> > +@BUILD_DAEMON_OFFLOAD_TRUE@export NIX_BUILD_HOOK
>> > +@BUILD_DAEMON_OFFLOAD_FALSE@# No offloading support.
>> > +@BUILD_DAEMON_OFFLOAD_FALSE@unset NIX_BUILD_HOOK  
>> 
>> It’s nicer (you’re welcome to push this change!), but it’s equivalent to
>> what’s already here, no?
>
> It's not quite equivalent.  The current situation would end up always
> exporting NIX_BUILD_HOOK because it is always generated by
> config.status.

Oooh, got it.

>> Is there a problem that we are trying to solve in the first place, or is
>> it more about making things nicer?
>
> The problem is libstore/build.cc executing NIX_BUILD_HOOK even if the
> daemon offload hook is disabled, i.e. when guile-ssh is missing.
>
> Make sense?

Yes, definitely.

Then you can definitely commit the pre-inst-env.in patch; the
config-daemon.ac patch can’t hurt either.

Thank you!

Ludo’.