Odd behavior with --dry-run and --upgrade

Odd behavior with --dry-run and --upgrade

[Roel Janssen]

Dear Guix,

For some time now, running `guix package --dry-run --upgrade' results in
build actions involving grafting.  For a dry-run, I find that really
odd.  I believe the correct behavior should be what can be achieved
with: `guix package --dry-run --no-grafts --upgrade'.

Do you think the behavior of --dry-run should be changed to not graft
things by default?  And if so, do you have a pointer where I could find
the problematic code?  I couldn't find anything particularly odd because
the code involved has a `dry-run?' condition pretty much everywhere it
matters.

Kind regards,
Roel Janssen

Re: Odd behavior with --dry-run and --upgrade

[Leo Famulari]

On Sat, Jul 23, 2016 at 05:11:03PM +0200, Roel Janssen wrote:
> For some time now, running `guix package --dry-run --upgrade' results in
> build actions involving grafting.  For a dry-run, I find that really
> odd.  I believe the correct behavior should be what can be achieved
> with: `guix package --dry-run --no-grafts --upgrade'.

Right, it has to build the ungrafted things in order to do a dry-run of
the grafts.

> Do you think the behavior of --dry-run should be changed to not graft
> things by default?  And if so, do you have a pointer where I could find
> the problematic code?  I couldn't find anything particularly odd because
> the code involved has a `dry-run?' condition pretty much everywhere it
> matters.

I think we should be able to do --dry-runs of grafting. But I was also
confused by the effect of the grafts at first. I'll wait for the
opinions of more experienced developers :)

Re: Odd behavior with --dry-run and --upgrade

[Alex Kost]

Roel Janssen (2016-07-23 18:11 +0300) wrote:

> Dear Guix,
>
> For some time now, running `guix package --dry-run --upgrade' results in
> build actions involving grafting.  For a dry-run, I find that really
> odd.  I believe the correct behavior should be what can be achieved
> with: `guix package --dry-run --no-grafts --upgrade'.

I'm totally agree with this; nowadays I always use --dry-run with
--no-grafts option.  As a user I expect that --dry-run means no building
at all.

BTW it's not just about ‘guix package --dry-run --upgrade’, it relates
to all commands, for example ‘guix build --dry-run foo’, etc.

OTOH, if a future ‘--dry-run’ would mean what ‘--dry-run --no-grafts’
means now, than how to achieve what ‘--dry-run’ means now?  Or rather:
does anyone use just --dry-run (without --no-grafts)?  Is it really
useful?

-- 
Alex

Re: Odd behavior with --dry-run and --upgrade

[Ludovic Courtès]

Hi!

Alex Kost <alezost@gmail.com> skribis:

> Roel Janssen (2016-07-23 18:11 +0300) wrote:
>
>> Dear Guix,
>>
>> For some time now, running `guix package --dry-run --upgrade' results in
>> build actions involving grafting.  For a dry-run, I find that really
>> odd.  I believe the correct behavior should be what can be achieved
>> with: `guix package --dry-run --no-grafts --upgrade'.
>
> I'm totally agree with this; nowadays I always use --dry-run with
> --no-grafts option.

Same here…

> As a user I expect that --dry-run means no building at all.
>
> BTW it's not just about ‘guix package --dry-run --upgrade’, it relates
> to all commands, for example ‘guix build --dry-run foo’, etc.
>
> OTOH, if a future ‘--dry-run’ would mean what ‘--dry-run --no-grafts’
> means now, than how to achieve what ‘--dry-run’ means now?  Or rather:
> does anyone use just --dry-run (without --no-grafts)?  Is it really
> useful?

In theory it could be useful for ‘guix build’, since it’s a “low level”
tool and people using it may want to be able to distinguish between
grafted and non-grafted results.

But honestly, I think changing ‘--dry-run’ to do ‘--dry-run --no-grafts’
would be fine, and probably better than the current situation.

Thoughts?

Ludo’.

Re: Odd behavior with --dry-run and --upgrade

[Andreas Enge]

On Mon, Jul 25, 2016 at 12:18:41AM +0200, Ludovic Courtès wrote:
> > I'm totally agree with this; nowadays I always use --dry-run with
> > --no-grafts option.
> Same here…
> > As a user I expect that --dry-run means no building at all.
> But honestly, I think changing ‘--dry-run’ to do ‘--dry-run --no-grafts’
> would be fine, and probably better than the current situation.

Yes, this is what I would expect. We could also have a "--dry-run --grafts"
option if anybody complains about the missing feature.

Andreas

Re: Odd behavior with --dry-run and --upgrade

[Roel Janssen]

Ludovic Courtès writes:

> Hi!
>
> Alex Kost <alezost@gmail.com> skribis:
>
>> Roel Janssen (2016-07-23 18:11 +0300) wrote:
>>
>>> Dear Guix,
>>>
>>> For some time now, running `guix package --dry-run --upgrade' results in
>>> build actions involving grafting.  For a dry-run, I find that really
>>> odd.  I believe the correct behavior should be what can be achieved
>>> with: `guix package --dry-run --no-grafts --upgrade'.
>>
>> I'm totally agree with this; nowadays I always use --dry-run with
>> --no-grafts option.
>
> Same here…
>
>> As a user I expect that --dry-run means no building at all.
>>
>> BTW it's not just about ‘guix package --dry-run --upgrade’, it relates
>> to all commands, for example ‘guix build --dry-run foo’, etc.
>>
>> OTOH, if a future ‘--dry-run’ would mean what ‘--dry-run --no-grafts’
>> means now, than how to achieve what ‘--dry-run’ means now?  Or rather:
>> does anyone use just --dry-run (without --no-grafts)?  Is it really
>> useful?
>
> In theory it could be useful for ‘guix build’, since it’s a “low level”
> tool and people using it may want to be able to distinguish between
> grafted and non-grafted results.
>
> But honestly, I think changing ‘--dry-run’ to do ‘--dry-run --no-grafts’
> would be fine, and probably better than the current situation.

Could you provide some insight in where I should be looking to att the
check to 'graft?'?

Kind regards,
Roel Janssen

Re: Odd behavior with --dry-run and --upgrade

[Ludovic Courtès]

[-- Attachment #1: Type: text/plain, Size: 1773 bytes --]

Roel Janssen <roel@gnu.org> skribis:

> Ludovic Courtès writes:
>
>> Hi!
>>
>> Alex Kost <alezost@gmail.com> skribis:
>>
>>> Roel Janssen (2016-07-23 18:11 +0300) wrote:
>>>
>>>> Dear Guix,
>>>>
>>>> For some time now, running `guix package --dry-run --upgrade' results in
>>>> build actions involving grafting.  For a dry-run, I find that really
>>>> odd.  I believe the correct behavior should be what can be achieved
>>>> with: `guix package --dry-run --no-grafts --upgrade'.
>>>
>>> I'm totally agree with this; nowadays I always use --dry-run with
>>> --no-grafts option.
>>
>> Same here…
>>
>>> As a user I expect that --dry-run means no building at all.
>>>
>>> BTW it's not just about ‘guix package --dry-run --upgrade’, it relates
>>> to all commands, for example ‘guix build --dry-run foo’, etc.
>>>
>>> OTOH, if a future ‘--dry-run’ would mean what ‘--dry-run --no-grafts’
>>> means now, than how to achieve what ‘--dry-run’ means now?  Or rather:
>>> does anyone use just --dry-run (without --no-grafts)?  Is it really
>>> useful?
>>
>> In theory it could be useful for ‘guix build’, since it’s a “low level”
>> tool and people using it may want to be able to distinguish between
>> grafted and non-grafted results.
>>
>> But honestly, I think changing ‘--dry-run’ to do ‘--dry-run --no-grafts’
>> would be fine, and probably better than the current situation.
>
> Could you provide some insight in where I should be looking to att the
> check to 'graft?'?

Everything that relates to command-line argument processing is in (guix
scripts build), for the common options, and then in each (guix scripts
*) module.

Roughly, the change I suggest would be along these lines:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 670 bytes --]

diff --git a/guix/scripts/build.scm b/guix/scripts/build.scm
index a02a0d5..daa60b9 100644
--- a/guix/scripts/build.scm
+++ b/guix/scripts/build.scm
@@ -541,7 +541,8 @@ must be one of 'package', 'all', or 'transitive'~%")
                    (alist-cons 'file arg result)))
          (option '(#\n "dry-run") #f #f
                  (lambda (opt name arg result)
-                   (alist-cons 'dry-run? #t result)))
+                   (alist-cons 'dry-run? #t
+                               (alist-cons 'graft? #f result))))
          (option '(#\r "root") #t #f
                  (lambda (opt name arg result)
                    (alist-cons 'gc-root arg result)))

[-- Attachment #3: Type: text/plain, Size: 229 bytes --]


However, since --dry-run is processed separately in each command, this
change should probably be duplicated.

Would you like to look into it?

Something similar should be done in the Emacs interface.

Thanks,
Ludo’.

Re: Odd behavior with --dry-run and --upgrade

[Roel Janssen]

Ludovic Courtès writes:

> Roel Janssen <roel@gnu.org> skribis:
>
>> Ludovic Courtès writes:
>>
>>> Hi!
>>>
>>> Alex Kost <alezost@gmail.com> skribis:
>>>
>>>> Roel Janssen (2016-07-23 18:11 +0300) wrote:
>>>>
>>>>> Dear Guix,
>>>>>
>>>>> For some time now, running `guix package --dry-run --upgrade' results in
>>>>> build actions involving grafting.  For a dry-run, I find that really
>>>>> odd.  I believe the correct behavior should be what can be achieved
>>>>> with: `guix package --dry-run --no-grafts --upgrade'.
>>>>
>>>> I'm totally agree with this; nowadays I always use --dry-run with
>>>> --no-grafts option.
>>>
>>> Same here…
>>>
>>>> As a user I expect that --dry-run means no building at all.
>>>>
>>>> BTW it's not just about ‘guix package --dry-run --upgrade’, it relates
>>>> to all commands, for example ‘guix build --dry-run foo’, etc.
>>>>
>>>> OTOH, if a future ‘--dry-run’ would mean what ‘--dry-run --no-grafts’
>>>> means now, than how to achieve what ‘--dry-run’ means now?  Or rather:
>>>> does anyone use just --dry-run (without --no-grafts)?  Is it really
>>>> useful?
>>>
>>> In theory it could be useful for ‘guix build’, since it’s a “low level”
>>> tool and people using it may want to be able to distinguish between
>>> grafted and non-grafted results.
>>>
>>> But honestly, I think changing ‘--dry-run’ to do ‘--dry-run --no-grafts’
>>> would be fine, and probably better than the current situation.
>>
>> Could you provide some insight in where I should be looking to att the
>> check to 'graft?'?
>
> Everything that relates to command-line argument processing is in (guix
> scripts build), for the common options, and then in each (guix scripts
> *) module.
>
> Roughly, the change I suggest would be along these lines:
>

Aha.  Disabling grafting when the `--dry-run' switch is provided seems
like exactly what we want to do.  Should we add a `--enable-grafts' too?

> However, since --dry-run is processed separately in each command, this
> change should probably be duplicated.
>
> Would you like to look into it?

Yes!  Please allow me some time though.

> Something similar should be done in the Emacs interface.

I'm not familiar with the code of the Emacs interface.  Any other
takers for it?  Otherwise I will look into it, but that will take even
more time :)

Kind regards,
Roel Janssen

Re: Odd behavior with --dry-run and --upgrade

[Ludovic Courtès]

Roel Janssen <roel@gnu.org> skribis:

> Ludovic Courtès writes:
>
>> Roel Janssen <roel@gnu.org> skribis:
>>
>>> Ludovic Courtès writes:

[...]

>> Everything that relates to command-line argument processing is in (guix
>> scripts build), for the common options, and then in each (guix scripts
>> *) module.
>>
>> Roughly, the change I suggest would be along these lines:
>>
>
> Aha.  Disabling grafting when the `--dry-run' switch is provided seems
> like exactly what we want to do.  Should we add a `--enable-grafts' too?

I don’t think so.  Given that they’re about security updated, I think
grafts should always be enabled by default, except for --dry-run.

>> However, since --dry-run is processed separately in each command, this
>> change should probably be duplicated.
>>
>> Would you like to look into it?
>
> Yes!  Please allow me some time though.

Sure, no rush!

>> Something similar should be done in the Emacs interface.
>
> I'm not familiar with the code of the Emacs interface.  Any other
> takers for it?  Otherwise I will look into it, but that will take even
> more time :)

Maybe Alex can give a hand?  :-)

Thank you for looking into it!

Ludo’.

Re: Odd behavior with --dry-run and --upgrade

[Alex Kost]

Ludovic Courtès (2016-07-26 12:50 +0300) wrote:

> Roel Janssen <roel@gnu.org> skribis:
>
>> Ludovic Courtès writes:
[...]
>>> But honestly, I think changing ‘--dry-run’ to do ‘--dry-run --no-grafts’
>>> would be fine, and probably better than the current situation.
>>
>> Could you provide some insight in where I should be looking to att the
>> check to 'graft?'?
>
> Everything that relates to command-line argument processing is in (guix
> scripts build), for the common options, and then in each (guix scripts
> *) module.
>
> Roughly, the change I suggest would be along these lines:
>
>
> diff --git a/guix/scripts/build.scm b/guix/scripts/build.scm
> index a02a0d5..daa60b9 100644
> --- a/guix/scripts/build.scm
> +++ b/guix/scripts/build.scm
> @@ -541,7 +541,8 @@ must be one of 'package', 'all', or 'transitive'~%")
>                     (alist-cons 'file arg result)))
>           (option '(#\n "dry-run") #f #f
>                   (lambda (opt name arg result)
> -                   (alist-cons 'dry-run? #t result)))
> +                   (alist-cons 'dry-run? #t
> +                               (alist-cons 'graft? #f result))))
>           (option '(#\r "root") #t #f
>                   (lambda (opt name arg result)
>                     (alist-cons 'gc-root arg result)))
>
> However, since --dry-run is processed separately in each command, this
> change should probably be duplicated.
>
> Would you like to look into it?
>
> Something similar should be done in the Emacs interface.

What would be "something similar" here?  For CLI it's easy to set
‘graft?’ option as you suggest, and later 'guix-package', 'guix-system'
and other similar procedures from (guix scripts ...) modules
parameterize ‘%graft?’ according to this option.

The only way I see for the Emacs interface is to modify
"emacs/guix-main.scm" to parameterize ‘%graft?’ as well and to set it
depending on the current value of ‘dry-run’.  AFAICT this
parameterization should be added to:

- 'process-package-actions': it is responsible for operations with
  profiles (installing/upgrading/removing packages);

- 'package-source-build-derivation': it is responsible for building
  package sources.

If my understanding is correct, I can make a patch for this.

I can also add 'grafts' option that will appear in prompts (in the
mode-line along with 'substitutes' and 'dry-run'), but I'm not sure if
it will be useful since dry-run will disable grafts anyway.

-- 
Alex

Re: Odd behavior with --dry-run and --upgrade

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> The only way I see for the Emacs interface is to modify
> "emacs/guix-main.scm" to parameterize ‘%graft?’ as well and to set it
> depending on the current value of ‘dry-run’.  AFAICT this
> parameterization should be added to:
>
> - 'process-package-actions': it is responsible for operations with
>   profiles (installing/upgrading/removing packages);
>
> - 'package-source-build-derivation': it is responsible for building
>   package sources.
>
> If my understanding is correct, I can make a patch for this.

Yes, it sounds good.

> I can also add 'grafts' option that will appear in prompts (in the
> mode-line along with 'substitutes' and 'dry-run'), but I'm not sure if
> it will be useful since dry-run will disable grafts anyway.

Yeah, I don’t think it’s useful.

Thanks,
Ludo’.

Re: Odd behavior with --dry-run and --upgrade

[Alex Kost]

[-- Attachment #1: Type: text/plain, Size: 643 bytes --]

Ludovic Courtès (2016-07-26 15:41 +0300) wrote:

> Roel Janssen <roel@gnu.org> skribis:
>
>> Ludovic Courtès writes:
[...]
>>> Something similar should be done in the Emacs interface.
>>
>> I'm not familiar with the code of the Emacs interface.  Any other
>> takers for it?  Otherwise I will look into it, but that will take even
>> more time :)
>
> Maybe Alex can give a hand?  :-)

I have zero knowledge in grafting, but if I checked it right, the
attached patch should disable grafting for the emacs interface (when
dry-run is on).  If you think it should be a part of a bigger
dry-run+no-grafts patch, please use it.


[-- Attachment #2: 0001-emacs-Disable-grafts-when-dry-run-is-enabled.patch --]
[-- Type: text/x-patch, Size: 3268 bytes --]

From d7747453bf31a616d414dce293fc0556d601abcb Mon Sep 17 00:00:00 2001
From: Alex Kost <alezost@gmail.com>
Date: Wed, 27 Jul 2016 14:55:50 +0300
Subject: [PATCH] emacs: Disable grafts when dry-run is enabled.

* emacs/guix-main.scm (process-package-actions): Set grafting according
to 'dry-run?'.
* guix/scripts.scm (build-package): Likewise.
---
 emacs/guix-main.scm | 20 +++++++++++---------
 guix/scripts.scm    |  4 +++-
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/emacs/guix-main.scm b/emacs/guix-main.scm
index cbf7cdc..040932f 100644
--- a/emacs/guix-main.scm
+++ b/emacs/guix-main.scm
@@ -46,6 +46,7 @@
  (guix)
  (guix combinators)
  (guix git-download)
+ (guix grafts)
  (guix packages)
  (guix profiles)
  (guix licenses)
@@ -930,15 +931,16 @@ OUTPUTS is a list of package outputs (may be an empty list)."
          (new-manifest (manifest-perform-transaction
                         manifest transaction)))
     (unless (and (null? install) (null? remove))
-      (with-store store
-        (set-build-options store
-                           #:print-build-trace #f
-                           #:use-substitutes? use-substitutes?)
-        (show-manifest-transaction store manifest transaction
-                                   #:dry-run? dry-run?)
-        (build-and-use-profile store profile new-manifest
-                               #:use-substitutes? use-substitutes?
-                               #:dry-run? dry-run?)))))
+      (parameterize ((%graft? (not dry-run?)))
+        (with-store store
+          (set-build-options store
+                             #:print-build-trace #f
+                             #:use-substitutes? use-substitutes?)
+          (show-manifest-transaction store manifest transaction
+                                     #:dry-run? dry-run?)
+          (build-and-use-profile store profile new-manifest
+                                 #:use-substitutes? use-substitutes?
+                                 #:dry-run? dry-run?))))))
 
 (define (delete-generations* profile generations)
   "Delete GENERATIONS from PROFILE.
diff --git a/guix/scripts.scm b/guix/scripts.scm
index d84375f..2ed1eeb 100644
--- a/guix/scripts.scm
+++ b/guix/scripts.scm
@@ -1,7 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013, 2014, 2015 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2014 Deck Pickard <deck.r.pickard@gmail.com>
-;;; Copyright © 2015 Alex Kost <alezost@gmail.com>
+;;; Copyright © 2015, 2016 Alex Kost <alezost@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -19,6 +19,7 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (guix scripts)
+  #:use-module (guix grafts)
   #:use-module (guix utils)
   #:use-module (guix ui)
   #:use-module (guix store)
@@ -106,6 +107,7 @@ true."
   "Build PACKAGE using BUILD-OPTIONS acceptable by 'set-build-options'.
 Show what and how will/would be built."
   (mbegin %store-monad
+    (set-grafting (not dry-run?))
     (apply set-build-options*
            #:use-substitutes? use-substitutes?
            (strip-keyword-arguments '(#:dry-run?) build-options))
-- 
2.8.3

Re: Odd behavior with --dry-run and --upgrade

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> I have zero knowledge in grafting, but if I checked it right, the
> attached patch should disable grafting for the emacs interface (when
> dry-run is on).  If you think it should be a part of a bigger
> dry-run+no-grafts patch, please use it.

Two separate patches is fine, IMO.

> From d7747453bf31a616d414dce293fc0556d601abcb Mon Sep 17 00:00:00 2001
> From: Alex Kost <alezost@gmail.com>
> Date: Wed, 27 Jul 2016 14:55:50 +0300
> Subject: [PATCH] emacs: Disable grafts when dry-run is enabled.
>
> * emacs/guix-main.scm (process-package-actions): Set grafting according
> to 'dry-run?'.
> * guix/scripts.scm (build-package): Likewise.

[...]

>  (define-module (guix scripts)
> +  #:use-module (guix grafts)
>    #:use-module (guix utils)
>    #:use-module (guix ui)
>    #:use-module (guix store)
> @@ -106,6 +107,7 @@ true."
>    "Build PACKAGE using BUILD-OPTIONS acceptable by 'set-build-options'.
>  Show what and how will/would be built."
>    (mbegin %store-monad
> +    (set-grafting (not dry-run?))
>      (apply set-build-options*
>             #:use-substitutes? use-substitutes?
>             (strip-keyword-arguments '(#:dry-run?) build-options))

Here it might be best to do something like this:

  (mlet %store-monad ((grafting? ((lift0 %graft? %store-monad))))
    (set-grafting (and (not dry-run?) grafting?))
    …)

This would make sure we don’t enable grafting if it turned out to be
disabled.

WDYT?

Ludo’.

Re: Odd behavior with --dry-run and --upgrade

[Alex Kost]

Ludovic Courtès (2016-07-28 01:19 +0300) wrote:

> Alex Kost <alezost@gmail.com> skribis:
>
>> I have zero knowledge in grafting, but if I checked it right, the
>> attached patch should disable grafting for the emacs interface (when
>> dry-run is on).  If you think it should be a part of a bigger
>> dry-run+no-grafts patch, please use it.
>
> Two separate patches is fine, IMO.

OK, then I'll commit this patch when it will be ready.

>> From d7747453bf31a616d414dce293fc0556d601abcb Mon Sep 17 00:00:00 2001
>> From: Alex Kost <alezost@gmail.com>
>> Date: Wed, 27 Jul 2016 14:55:50 +0300
>> Subject: [PATCH] emacs: Disable grafts when dry-run is enabled.
>>
>> * emacs/guix-main.scm (process-package-actions): Set grafting according
>> to 'dry-run?'.
>> * guix/scripts.scm (build-package): Likewise.
>
> [...]
>
>>  (define-module (guix scripts)
>> +  #:use-module (guix grafts)
>>    #:use-module (guix utils)
>>    #:use-module (guix ui)
>>    #:use-module (guix store)
>> @@ -106,6 +107,7 @@ true."
>>    "Build PACKAGE using BUILD-OPTIONS acceptable by 'set-build-options'.
>>  Show what and how will/would be built."
>>    (mbegin %store-monad
>> +    (set-grafting (not dry-run?))
>>      (apply set-build-options*
>>             #:use-substitutes? use-substitutes?
>>             (strip-keyword-arguments '(#:dry-run?) build-options))
>
> Here it might be best to do something like this:
>
>   (mlet %store-monad ((grafting? ((lift0 %graft? %store-monad))))
>     (set-grafting (and (not dry-run?) grafting?))
>     …)
>
> This would make sure we don’t enable grafting if it turned out to be
> disabled.
>
> WDYT?

OK, you know better :-)

However, I tried it and it doesn't work for me (note: I know nothing
about monads, gexps, etc.).  When I try this:

   (mbegin %store-monad
     (set-grafting #f)
     …)

the grafting doesn't happen, but when I try this:

   (mlet %store-monad ((grafting? ((lift0 %graft? %store-monad))))
     (set-grafting #f)
     …)

grafting happens anyway.  I have no idea what the problem is.

-- 
Alex

Re: Odd behavior with --dry-run and --upgrade

[Ludovic Courtès]

[-- Attachment #1: Type: text/plain, Size: 1914 bytes --]

Alex Kost <alezost@gmail.com> skribis:

> Ludovic Courtès (2016-07-28 01:19 +0300) wrote:
>
>> Alex Kost <alezost@gmail.com> skribis:

[...]

>>> From d7747453bf31a616d414dce293fc0556d601abcb Mon Sep 17 00:00:00 2001
>>> From: Alex Kost <alezost@gmail.com>
>>> Date: Wed, 27 Jul 2016 14:55:50 +0300
>>> Subject: [PATCH] emacs: Disable grafts when dry-run is enabled.
>>>
>>> * emacs/guix-main.scm (process-package-actions): Set grafting according
>>> to 'dry-run?'.
>>> * guix/scripts.scm (build-package): Likewise.
>>
>> [...]
>>
>>>  (define-module (guix scripts)
>>> +  #:use-module (guix grafts)
>>>    #:use-module (guix utils)
>>>    #:use-module (guix ui)
>>>    #:use-module (guix store)
>>> @@ -106,6 +107,7 @@ true."
>>>    "Build PACKAGE using BUILD-OPTIONS acceptable by 'set-build-options'.
>>>  Show what and how will/would be built."
>>>    (mbegin %store-monad
>>> +    (set-grafting (not dry-run?))
>>>      (apply set-build-options*
>>>             #:use-substitutes? use-substitutes?
>>>             (strip-keyword-arguments '(#:dry-run?) build-options))
>>
>> Here it might be best to do something like this:
>>
>>   (mlet %store-monad ((grafting? ((lift0 %graft? %store-monad))))
>>     (set-grafting (and (not dry-run?) grafting?))
>>     …)
>>
>> This would make sure we don’t enable grafting if it turned out to be
>> disabled.
>>
>> WDYT?
>
> OK, you know better :-)
>
> However, I tried it and it doesn't work for me (note: I know nothing
> about monads, gexps, etc.).  When I try this:
>
>    (mbegin %store-monad
>      (set-grafting #f)
>      …)
>
> the grafting doesn't happen, but when I try this:
>
>    (mlet %store-monad ((grafting? ((lift0 %graft? %store-monad))))
>      (set-grafting #f)
>      …)
>
> grafting happens anyway.  I have no idea what the problem is.

Hmm.  Not sure why.  What about this:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 1185 bytes --]

--- a/guix/scripts.scm
+++ b/guix/scripts.scm
@@ -22,6 +22,7 @@
   #:use-module (guix utils)
   #:use-module (guix ui)
   #:use-module (guix store)
+  #:use-module (guix grafts)
   #:use-module (guix monads)
   #:use-module (guix packages)
   #:use-module (guix derivations)
@@ -105,11 +106,14 @@ true."
                         #:rest build-options)
   "Build PACKAGE using BUILD-OPTIONS acceptable by 'set-build-options'.
 Show what and how will/would be built."
-  (mbegin %store-monad
+  (mlet %store-monad ((grafting? ((lift0 %graft? %store-monad))))
     (apply set-build-options*
            #:use-substitutes? use-substitutes?
            (strip-keyword-arguments '(#:dry-run?) build-options))
-    (mlet %store-monad ((derivation (package->derivation package)))
+    (mlet %store-monad ((derivation (package->derivation package
+                                                         #:graft?
+                                                         (and (not dry-run?)
+                                                              grafting?))))
       (mbegin %store-monad
         (maybe-build (list derivation)
                      #:use-substitutes? use-substitutes?

[-- Attachment #3: Type: text/plain, Size: 49 bytes --]


Using #:graft? is cleaner anyway.

Ludo’.

Re: Odd behavior with --dry-run and --upgrade

[Alex Kost]

Ludovic Courtès (2016-07-28 16:01 +0300) wrote:

> Alex Kost <alezost@gmail.com> skribis:
[...]
>> However, I tried it and it doesn't work for me (note: I know nothing
>> about monads, gexps, etc.).  When I try this:
>>
>>    (mbegin %store-monad
>>      (set-grafting #f)
>>      …)
>>
>> the grafting doesn't happen, but when I try this:
>>
>>    (mlet %store-monad ((grafting? ((lift0 %graft? %store-monad))))
>>      (set-grafting #f)
>>      …)
>>
>> grafting happens anyway.  I have no idea what the problem is.
>
> Hmm.  Not sure why.  What about this:

This approach works, thanks!

> --- a/guix/scripts.scm
> +++ b/guix/scripts.scm
> @@ -22,6 +22,7 @@
>    #:use-module (guix utils)
>    #:use-module (guix ui)
>    #:use-module (guix store)
> +  #:use-module (guix grafts)
>    #:use-module (guix monads)
>    #:use-module (guix packages)
>    #:use-module (guix derivations)
> @@ -105,11 +106,14 @@ true."
>                          #:rest build-options)
>    "Build PACKAGE using BUILD-OPTIONS acceptable by 'set-build-options'.
>  Show what and how will/would be built."
> -  (mbegin %store-monad
> +  (mlet %store-monad ((grafting? ((lift0 %graft? %store-monad))))
>      (apply set-build-options*
>             #:use-substitutes? use-substitutes?
>             (strip-keyword-arguments '(#:dry-run?) build-options))
> -    (mlet %store-monad ((derivation (package->derivation package)))
> +    (mlet %store-monad ((derivation (package->derivation package
> +                                                         #:graft?
> +                                                         (and (not dry-run?)
> +                                                              grafting?))))
>        (mbegin %store-monad
>          (maybe-build (list derivation)
>                       #:use-substitutes? use-substitutes?
>
> Using #:graft? is cleaner anyway.

Indeed!  Since you said it's OK to make separate patches for this
"dry-run + no-grafts" change (for CLI and Emacs UI), I committed it as
a82a201.

-- 
Alex

Re: Odd behavior with --dry-run and --upgrade

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> Ludovic Courtès (2016-07-28 16:01 +0300) wrote:

[...]

>> --- a/guix/scripts.scm
>> +++ b/guix/scripts.scm
>> @@ -22,6 +22,7 @@
>>    #:use-module (guix utils)
>>    #:use-module (guix ui)
>>    #:use-module (guix store)
>> +  #:use-module (guix grafts)
>>    #:use-module (guix monads)
>>    #:use-module (guix packages)
>>    #:use-module (guix derivations)
>> @@ -105,11 +106,14 @@ true."
>>                          #:rest build-options)
>>    "Build PACKAGE using BUILD-OPTIONS acceptable by 'set-build-options'.
>>  Show what and how will/would be built."
>> -  (mbegin %store-monad
>> +  (mlet %store-monad ((grafting? ((lift0 %graft? %store-monad))))
>>      (apply set-build-options*
>>             #:use-substitutes? use-substitutes?
>>             (strip-keyword-arguments '(#:dry-run?) build-options))
>> -    (mlet %store-monad ((derivation (package->derivation package)))
>> +    (mlet %store-monad ((derivation (package->derivation package
>> +                                                         #:graft?
>> +                                                         (and (not dry-run?)
>> +                                                              grafting?))))
>>        (mbegin %store-monad
>>          (maybe-build (list derivation)
>>                       #:use-substitutes? use-substitutes?
>>
>> Using #:graft? is cleaner anyway.
>
> Indeed!  Since you said it's OK to make separate patches for this
> "dry-run + no-grafts" change (for CLI and Emacs UI), I committed it as
> a82a201.

Perfect, thanks!

Ludo’.

Re: Odd behavior with --dry-run and --upgrade

[Roel Janssen]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: 0001-guix-scripts-Disable-grafting-on-dry-runs.patch --]
[-- Type: text/x-patch, Size: 3756 bytes --]

From cf5a01aba3c08b7c62048bff3e0f1df80dc0b4dc Mon Sep 17 00:00:00 2001
From: Roel Janssen <roel@gnu.org>
Date: Sun, 21 Aug 2016 06:48:52 +0200
Subject: [PATCH] guix scripts: Disable grafting on dry runs.

* guix/scripts/archive.scm, guix/scripts/build.scm,
  guix/scripts/environment.scm, guix/scripts/system.scm,
  guix/scripts/package.scm: Disable grafting when a dry-run parameter is given.
---
 guix/scripts/archive.scm     | 2 +-
 guix/scripts/build.scm       | 2 +-
 guix/scripts/environment.scm | 2 +-
 guix/scripts/package.scm     | 3 ++-
 guix/scripts/system.scm      | 2 +-
 5 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/guix/scripts/archive.scm b/guix/scripts/archive.scm
index e06c38a..8c7322d 100644
--- a/guix/scripts/archive.scm
+++ b/guix/scripts/archive.scm
@@ -162,7 +162,7 @@ Export/import one or more packages from/to the store.\n"))
                    (alist-cons 'expression arg result)))
          (option '(#\n "dry-run") #f #f
                  (lambda (opt name arg result)
-                   (alist-cons 'dry-run? #t result)))
+                   (alist-cons 'dry-run? #t (alist-cons 'graft? #f result))))
 
          %standard-build-options))
 
diff --git a/guix/scripts/build.scm b/guix/scripts/build.scm
index a02a0d5..9a113b4 100644
--- a/guix/scripts/build.scm
+++ b/guix/scripts/build.scm
@@ -541,7 +541,7 @@ must be one of 'package', 'all', or 'transitive'~%")
                    (alist-cons 'file arg result)))
          (option '(#\n "dry-run") #f #f
                  (lambda (opt name arg result)
-                   (alist-cons 'dry-run? #t result)))
+                   (alist-cons 'dry-run? #t (alist-cons 'graft? #f result))))
          (option '(#\r "root") #t #f
                  (lambda (opt name arg result)
                    (alist-cons 'gc-root arg result)))
diff --git a/guix/scripts/environment.scm b/guix/scripts/environment.scm
index 9f72b7b..0c69bfc 100644
--- a/guix/scripts/environment.scm
+++ b/guix/scripts/environment.scm
@@ -226,7 +226,7 @@ COMMAND or an interactive shell in that environment.\n"))
                    (alist-cons 'ad-hoc? #t result)))
          (option '(#\n "dry-run") #f #f
                  (lambda (opt name arg result)
-                   (alist-cons 'dry-run? #t result)))
+                   (alist-cons 'dry-run? #t (alist-cons 'graft? #f result))))
          (option '(#\s "system") #t #f
                  (lambda (opt name arg result)
                    (alist-cons 'system arg
diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index 2a751a4..fd42cdb 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -486,7 +486,8 @@ kind of search path~%")
                            #f)))
          (option '(#\n "dry-run") #f #f
                  (lambda (opt name arg result arg-handler)
-                   (values (alist-cons 'dry-run? #t result)
+                   (values (alist-cons 'dry-run? #t
+                                       (alist-cons 'graft? #f result))
                            #f)))
          (option '("bootstrap") #f #f
                  (lambda (opt name arg result arg-handler)
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 4b53c3d..a9fe7d5 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -750,7 +750,7 @@ Build the operating system declared in FILE according to ACTION.\n"))
 
          (option '(#\n "dry-run") #f #f
                  (lambda (opt name arg result)
-                   (alist-cons 'dry-run? #t result)))
+                   (alist-cons 'dry-run? #t (alist-cons 'graft? #f result))))
          (option '(#\s "system") #t #f
                  (lambda (opt name arg result)
                    (alist-cons 'system arg
-- 
2.9.3


[-- Attachment #2: Type: text/plain, Size: 2208 bytes --]


Ludovic Courtès writes:

> Roel Janssen <roel@gnu.org> skribis:
>
>> Ludovic Courtès writes:
>>
>>> Hi!
>>>
>>> Alex Kost <alezost@gmail.com> skribis:
>>>
>>>> Roel Janssen (2016-07-23 18:11 +0300) wrote:
>>>>
>>>>> Dear Guix,
>>>>>
>>>>> For some time now, running `guix package --dry-run --upgrade' results in
>>>>> build actions involving grafting.  For a dry-run, I find that really
>>>>> odd.  I believe the correct behavior should be what can be achieved
>>>>> with: `guix package --dry-run --no-grafts --upgrade'.
>>>>
>>>> I'm totally agree with this; nowadays I always use --dry-run with
>>>> --no-grafts option.
>>>
>>> Same here…
>>>
>>>> As a user I expect that --dry-run means no building at all.
>>>>
>>>> BTW it's not just about ‘guix package --dry-run --upgrade’, it relates
>>>> to all commands, for example ‘guix build --dry-run foo’, etc.
>>>>
>>>> OTOH, if a future ‘--dry-run’ would mean what ‘--dry-run --no-grafts’
>>>> means now, than how to achieve what ‘--dry-run’ means now?  Or rather:
>>>> does anyone use just --dry-run (without --no-grafts)?  Is it really
>>>> useful?
>>>
>>> In theory it could be useful for ‘guix build’, since it’s a “low level”
>>> tool and people using it may want to be able to distinguish between
>>> grafted and non-grafted results.
>>>
>>> But honestly, I think changing ‘--dry-run’ to do ‘--dry-run --no-grafts’
>>> would be fine, and probably better than the current situation.
>>
>> Could you provide some insight in where I should be looking to att the
>> check to 'graft?'?
>
> Everything that relates to command-line argument processing is in (guix
> scripts build), for the common options, and then in each (guix scripts
> *) module.
>
> Roughly, the change I suggest would be along these lines:
>
>
> However, since --dry-run is processed separately in each command, this
> change should probably be duplicated.
>
> Would you like to look into it?
>
> Something similar should be done in the Emacs interface.
>
> Thanks,
> Ludo’.

Sorry for the delay.  The attached patch applies your idea to all
subcommands that have a --dry-run option.  Is this what you had in mind?

Kind regards,
Roel Janssen

Re: Odd behavior with --dry-run and --upgrade

[Roel Janssen]

Roel Janssen writes:

> Ludovic Courtès writes:
>
>> Roel Janssen <roel@gnu.org> skribis:
>>
>>> Ludovic Courtès writes:
>>>
>>>> Hi!
>>>>
>>>> Alex Kost <alezost@gmail.com> skribis:
>>>>
>>>>> Roel Janssen (2016-07-23 18:11 +0300) wrote:
>>>>>
>>>>>> Dear Guix,
>>>>>>
>>>>>> For some time now, running `guix package --dry-run --upgrade' results in
>>>>>> build actions involving grafting.  For a dry-run, I find that really
>>>>>> odd.  I believe the correct behavior should be what can be achieved
>>>>>> with: `guix package --dry-run --no-grafts --upgrade'.
>>>>>
>>>>> I'm totally agree with this; nowadays I always use --dry-run with
>>>>> --no-grafts option.
>>>>
>>>> Same here…
>>>>
>>>>> As a user I expect that --dry-run means no building at all.
>>>>>
>>>>> BTW it's not just about ‘guix package --dry-run --upgrade’, it relates
>>>>> to all commands, for example ‘guix build --dry-run foo’, etc.
>>>>>
>>>>> OTOH, if a future ‘--dry-run’ would mean what ‘--dry-run --no-grafts’
>>>>> means now, than how to achieve what ‘--dry-run’ means now?  Or rather:
>>>>> does anyone use just --dry-run (without --no-grafts)?  Is it really
>>>>> useful?
>>>>
>>>> In theory it could be useful for ‘guix build’, since it’s a “low level”
>>>> tool and people using it may want to be able to distinguish between
>>>> grafted and non-grafted results.
>>>>
>>>> But honestly, I think changing ‘--dry-run’ to do ‘--dry-run --no-grafts’
>>>> would be fine, and probably better than the current situation.
>>>
>>> Could you provide some insight in where I should be looking to att the
>>> check to 'graft?'?
>>
>> Everything that relates to command-line argument processing is in (guix
>> scripts build), for the common options, and then in each (guix scripts
>> *) module.
>>
>> Roughly, the change I suggest would be along these lines:
>>
>>
>> However, since --dry-run is processed separately in each command, this
>> change should probably be duplicated.
>>
>> Would you like to look into it?
>>
>> Something similar should be done in the Emacs interface.
>>
>> Thanks,
>> Ludo’.
>
> Sorry for the delay.  The attached patch applies your idea to all
> subcommands that have a --dry-run option.  Is this what you had in mind?
>
> Kind regards,
> Roel Janssen

Is it OK to push this change?

Kind regards,
Roel Janssen

Re: Odd behavior with --dry-run and --upgrade

[Ludovic Courtès]

Hi Roel,

Roel Janssen <roel@gnu.org> skribis:

> From cf5a01aba3c08b7c62048bff3e0f1df80dc0b4dc Mon Sep 17 00:00:00 2001
> From: Roel Janssen <roel@gnu.org>
> Date: Sun, 21 Aug 2016 06:48:52 +0200
> Subject: [PATCH] guix scripts: Disable grafting on dry runs.
>
> * guix/scripts/archive.scm, guix/scripts/build.scm,
>   guix/scripts/environment.scm, guix/scripts/system.scm,
>   guix/scripts/package.scm: Disable grafting when a dry-run parameter is given.

LGTM, please push.

Thank you!

Ludo’.