GNU bug report logs - #28765
[PATCH] config: Conditionally configure daemon offload script.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 28765 in the body.
You can then email your comments to 28765 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Eric Bavier <bavier <at> cray.com>
To: "guix-patches <at> gnu.org" <guix-patches <at> gnu.org>
Subject: [PATCH] config: Conditionally configure daemon offload script.
Date: Mon, 9 Oct 2017 19:08:21 +0000

[Message part 1 (text/plain, inline)]

This prevents pre-inst-env from exporting NIX_BUILD_HOOK when guile-ssh is not available.

Eric Bavier, Scientific Libraries, Cray Inc.

[0001-config-Conditionally-configure-daemon-offload-script.patch (text/x-patch, attachment)]

Message #8 received at 28765 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Eric Bavier <bavier <at> cray.com>
Cc: 28765 <at> debbugs.gnu.org
Subject: Re: [bug#28765] [PATCH] config: Conditionally configure daemon
 offload script.
Date: Mon, 09 Oct 2017 23:07:10 +0200

Eric Bavier <bavier <at> cray.com> skribis:

> From 984e324370c2c17d8d1a982adf2884112c9e64b7 Mon Sep 17 00:00:00 2001
> From: Eric Bavier <bavier <at> cray.com>
> 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.  */
  …

Thanks,
Ludo’.

Message #11 received at 28765 <at> debbugs.gnu.org (full text, mbox):

From: Eric Bavier <ericbavier <at> centurylink.net>
To: ludo <at> gnu.org (Ludovic Courtès)
Cc: Eric Bavier <bavier <at> cray.com>, 28765 <at> debbugs.gnu.org
Subject: Re: [bug#28765] [PATCH] config: Conditionally configure daemon
 offload script.
Date: Mon, 9 Oct 2017 19:08:01 -0500

On Mon, 09 Oct 2017 23:07:10 +0200
ludo <at> gnu.org (Ludovic Courtès) wrote:

> Eric Bavier <bavier <at> cray.com> skribis:
> 
> > From 984e324370c2c17d8d1a982adf2884112c9e64b7 Mon Sep 17 00:00:00 2001
> > From: Eric Bavier <bavier <at> cray.com>
> > 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 <at> export NIX_BUILD_HOOK
+@BUILD_DAEMON_OFFLOAD_FALSE@# No offloading support.
+@BUILD_DAEMON_OFFLOAD_FALSE <at> unset NIX_BUILD_HOOK
 
 # The 'guix-register' program.
 GUIX_REGISTER="$abs_top_builddir/guix-register"

Message #14 received at 28765 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Eric Bavier <ericbavier <at> centurylink.net>
Cc: Eric Bavier <bavier <at> cray.com>, 28765 <at> debbugs.gnu.org
Subject: Re: [bug#28765] [PATCH] config: Conditionally configure daemon
 offload script.
Date: Tue, 10 Oct 2017 08:49:49 +0200

Hello,

Eric Bavier <ericbavier <at> centurylink.net> skribis:

> On Mon, 09 Oct 2017 23:07:10 +0200
> ludo <at> gnu.org (Ludovic Courtès) wrote:
>
>> Eric Bavier <bavier <at> cray.com> skribis:
>> 
>> > From 984e324370c2c17d8d1a982adf2884112c9e64b7 Mon Sep 17 00:00:00 2001
>> > From: Eric Bavier <bavier <at> cray.com>
>> > 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 <at> export NIX_BUILD_HOOK
> +@BUILD_DAEMON_OFFLOAD_FALSE@# No offloading support.
> +@BUILD_DAEMON_OFFLOAD_FALSE <at> unset NIX_BUILD_HOOK

It’s nicer (you’re welcome to push this change!), but it’s equivalent to
what’s already here, no?

Is there a problem that we are trying to solve in the first place, or is
it more about making things nicer?

Thank you,
Ludo’.

Message #17 received at 28765 <at> debbugs.gnu.org (full text, mbox):

From: Eric Bavier <ericbavier <at> centurylink.net>
To: ludo <at> gnu.org (Ludovic Courtès)
Cc: Eric Bavier <bavier <at> cray.com>, 28765 <at> debbugs.gnu.org
Subject: Re: [bug#28765] [PATCH] config: Conditionally configure daemon
 offload script.
Date: Tue, 10 Oct 2017 08:00:19 -0500

On Tue, 10 Oct 2017 08:49:49 +0200
ludo <at> gnu.org (Ludovic Courtès) wrote:

> Hello,
> 
> Eric Bavier <ericbavier <at> centurylink.net> skribis:
> 
> > On Mon, 09 Oct 2017 23:07:10 +0200
> > ludo <at> gnu.org (Ludovic Courtès) wrote:
> >  
> >> Eric Bavier <bavier <at> cray.com> skribis:
> >>   
> >> > From 984e324370c2c17d8d1a982adf2884112c9e64b7 Mon Sep 17 00:00:00 2001
> >> > From: Eric Bavier <bavier <at> cray.com>
> >> > 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 <at> export NIX_BUILD_HOOK
> > +@BUILD_DAEMON_OFFLOAD_FALSE@# No offloading support.
> > +@BUILD_DAEMON_OFFLOAD_FALSE <at> 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.
> 
> 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?

`~Eric

Message #20 received at 28765 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Eric Bavier <ericbavier <at> centurylink.net>
Cc: Eric Bavier <bavier <at> cray.com>, 28765 <at> debbugs.gnu.org
Subject: Re: [bug#28765] [PATCH] config: Conditionally configure daemon
 offload script.
Date: Tue, 10 Oct 2017 16:17:46 +0200

Eric Bavier <ericbavier <at> centurylink.net> skribis:

> On Tue, 10 Oct 2017 08:49:49 +0200
> ludo <at> gnu.org (Ludovic Courtès) wrote:
>
>> Hello,
>> 
>> Eric Bavier <ericbavier <at> centurylink.net> skribis:
>> 
>> > On Mon, 09 Oct 2017 23:07:10 +0200
>> > ludo <at> gnu.org (Ludovic Courtès) wrote:
>> >  
>> >> Eric Bavier <bavier <at> cray.com> skribis:
>> >>   
>> >> > From 984e324370c2c17d8d1a982adf2884112c9e64b7 Mon Sep 17 00:00:00 2001
>> >> > From: Eric Bavier <bavier <at> cray.com>
>> >> > 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 <at> export NIX_BUILD_HOOK
>> > +@BUILD_DAEMON_OFFLOAD_FALSE@# No offloading support.
>> > +@BUILD_DAEMON_OFFLOAD_FALSE <at> 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’.

Message #25 received at 28765-done <at> debbugs.gnu.org (full text, mbox):

From: Eric Bavier <ericbavier <at> centurylink.net>
To: ludo <at> gnu.org (Ludovic Courtès)
Cc: 28765-done <at> debbugs.gnu.org
Subject: Re: [bug#28765] [PATCH] config: Conditionally configure daemon
 offload script.
Date: Tue, 10 Oct 2017 23:18:31 -0500

On Tue, 10 Oct 2017 16:17:46 +0200
ludo <at> gnu.org (Ludovic Courtès) wrote:

> Eric Bavier <ericbavier <at> centurylink.net> skribis:
> 
> > On Tue, 10 Oct 2017 08:49:49 +0200
> > ludo <at> gnu.org (Ludovic Courtès) wrote:
> >  
> >> Hello,
> >> 
> >> Eric Bavier <ericbavier <at> centurylink.net> skribis:
> >>   
> >> > On Mon, 09 Oct 2017 23:07:10 +0200
> >> > ludo <at> gnu.org (Ludovic Courtès) wrote:
> >> >    
> >> >> Eric Bavier <bavier <at> cray.com> skribis:
> >> >>     
> >> >> > From 984e324370c2c17d8d1a982adf2884112c9e64b7 Mon Sep 17 00:00:00 2001
> >> >> > From: Eric Bavier <bavier <at> cray.com>
> >> >> > 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 <at> export NIX_BUILD_HOOK
> >> > +@BUILD_DAEMON_OFFLOAD_FALSE@# No offloading support.
> >> > +@BUILD_DAEMON_OFFLOAD_FALSE <at> 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.

pre-inst-env patch pushed as 7740228e3523e3e0e4c007eb1f1b224575d16574

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.