diff mbox

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

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

Commit Message

Ludovic Courtès July 26, 2016, 9:50 a.m. UTC
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’.

Comments

Roel Janssen July 26, 2016, 10:20 a.m. UTC | #1
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
Ludovic Courtès July 26, 2016, 12:41 p.m. UTC | #2
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’.
Alex Kost July 26, 2016, 1:41 p.m. UTC | #3
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.
Ludovic Courtès July 26, 2016, 2:37 p.m. UTC | #4
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’.
diff mbox

Patch

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)))