diff mbox

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

Message ID 87wpja67sg.fsf@gnu.org
State New
Headers show

Commit Message

Roel Janssen Aug. 21, 2016, 4:54 a.m. UTC
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

Comments

Roel Janssen Aug. 27, 2016, 11:23 p.m. UTC | #1
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
Ludovic Courtès Aug. 28, 2016, 2:08 p.m. UTC | #2
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’.
diff mbox

Patch

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