[bug#33286] Add 'list-formats' to 'guix pack'

Message ID 20181106095316.GB1206@macbook41
State New
Headers show
Series
  • [bug#33286] Add 'list-formats' to 'guix pack'
Related show

Checks

Context Check Description
cbaines/applying patch success Successfully applied

Commit Message

Efraim Flashner Nov. 6, 2018, 9:53 a.m. UTC
I was looking at the help menu for 'guix pack' and I realized that I
didn't know what formats were available and there wasn't a flag to show
them.

Comments

Danny Milosavljevic Nov. 6, 2018, 10:39 a.m. UTC | #1
Hi Efraim,

On Tue, 6 Nov 2018 11:53:16 +0200
Efraim Flashner <efraim@flashner.co.il> wrote:

> I was looking at the help menu for 'guix pack' and I realized that I
> didn't know what formats were available and there wasn't a flag to show
> them.

Good idea!  In principle LGTM, but why not (also?) always show them on "--help"?
It's kinda weird to have "--help" not show something and then having to invoke a
command again to get it.  It's not like the new output is long in proportion
to the existing output of "--help" or anything.
Efraim Flashner Nov. 6, 2018, 10:48 a.m. UTC | #2
On Tue, Nov 06, 2018 at 11:39:15AM +0100, Danny Milosavljevic wrote:
> Hi Efraim,
> 
> On Tue, 6 Nov 2018 11:53:16 +0200
> Efraim Flashner <efraim@flashner.co.il> wrote:
> 
> > I was looking at the help menu for 'guix pack' and I realized that I
> > didn't know what formats were available and there wasn't a flag to show
> > them.
> 
> Good idea!  In principle LGTM, but why not (also?) always show them on "--help"?
> It's kinda weird to have "--help" not show something and then having to invoke a
> command again to get it.  It's not like the new output is long in proportion
> to the existing output of "--help" or anything.

we also have 'guix refresh --list-updaters' and 'guix lint
--list-checkers'.

'guix system', for file-system-type, shows '(one of 'ext4', 'iso9660')'
right in the options.

also 'guix hash' allows other formats but doesn't have a list-updaters

having "(one of 'tarball', 'squashfs', 'docker')" would be easier, but
IMO my patch gives more information, specifically about squashfs and
needing to run 'docker load'.
Ludovic Courtès Nov. 6, 2018, 3:31 p.m. UTC | #3
Hello,

Efraim Flashner <efraim@flashner.co.il> skribis:

> From af9a132a662f1d703df1c32278a45d2adca146ed Mon Sep 17 00:00:00 2001
> From: Efraim Flashner <efraim@flashner.co.il>
> Date: Tue, 6 Nov 2018 11:50:48 +0200
> Subject: [PATCH] pack: List the available formats.
>
> * guix/scripts/pack.scm (show-formats): New variable.
> (%options, show-help): Add 'list-formats' option.

[...]

> @@ -551,6 +552,18 @@ please email '~a'~%")
>      (squashfs . ,squashfs-image)
>      (docker  . ,docker-image)))
>  
> +(define (show-formats)
> +  ;; Print the supported pack formats.
> +  (display (G_ "The supported formats for 'guix pack' are:"))
> +  (newline)
> +  (display (G_ "
> +  tarball       A self-contained tarball, ready to run on another machine."))
> +  (display (G_ "
> +  squashfs      A squashfs image, ready for 'cluster engines' and the like."))

I’d write “Squashfs image suitable for Singularity” and remove “A” from
each.

Also, since we have the ‘%formats’ alist right above, what about
adjusting it so that it contains each format description, and then
‘show-formats’ would just traverse it?

You could do:

  (define-record-type <image-format>
    (image-format proc description)
    image-format?
    (proc        image-format-procedure)
    (description image-format-description))

  (define %formats
    (let-syntax ((description (syntax-rules () ((_ str) str))))
      `((tarball . ,(image-format self-contained-tarball
                                  (description "foo bar")))
        …)))

The dummy ‘description’ macro is here to allow ‘xgettext’ to catch the
translatable strings without actually translating them right away;
‘show-formats’ will have to call ‘G_’ for that.

WDYT?

Ludo’.
Efraim Flashner Nov. 27, 2018, 5:26 p.m. UTC | #4
On Tue, Nov 06, 2018 at 04:31:59PM +0100, Ludovic Courtès wrote:
> Hello,
> 
> Efraim Flashner <efraim@flashner.co.il> skribis:
> 
> > From af9a132a662f1d703df1c32278a45d2adca146ed Mon Sep 17 00:00:00 2001
> > From: Efraim Flashner <efraim@flashner.co.il>
> > Date: Tue, 6 Nov 2018 11:50:48 +0200
> > Subject: [PATCH] pack: List the available formats.
> >
> > * guix/scripts/pack.scm (show-formats): New variable.
> > (%options, show-help): Add 'list-formats' option.
> 
> [...]
> 
> > @@ -551,6 +552,18 @@ please email '~a'~%")
> >      (squashfs . ,squashfs-image)
> >      (docker  . ,docker-image)))
> >  
> > +(define (show-formats)
> > +  ;; Print the supported pack formats.
> > +  (display (G_ "The supported formats for 'guix pack' are:"))
> > +  (newline)
> > +  (display (G_ "
> > +  tarball       A self-contained tarball, ready to run on another machine."))
> > +  (display (G_ "
> > +  squashfs      A squashfs image, ready for 'cluster engines' and the like."))
> 
> I’d write “Squashfs image suitable for Singularity” and remove “A” from
> each.
> 
> Also, since we have the ‘%formats’ alist right above, what about
> adjusting it so that it contains each format description, and then
> ‘show-formats’ would just traverse it?
> 
> You could do:
> 
>   (define-record-type <image-format>
>     (image-format proc description)
>     image-format?
>     (proc        image-format-procedure)
>     (description image-format-description))
> 
>   (define %formats
>     (let-syntax ((description (syntax-rules () ((_ str) str))))
>       `((tarball . ,(image-format self-contained-tarball
>                                   (description "foo bar")))
>         …)))
> 
> The dummy ‘description’ macro is here to allow ‘xgettext’ to catch the
> translatable strings without actually translating them right away;
> ‘show-formats’ will have to call ‘G_’ for that.
> 
> WDYT?
> 
> Ludo’.

I do like the idea, but I haven't been able to get it to work. In the
mean time I've committed this as db08ea40873ae20507bc40d34a56dea1b8ce8f0e
so we at least get the benefits of having it here.

Patch

diff --git a/guix/scripts/pack.scm b/guix/scripts/pack.scm
index 83bfa4ce0..9056ada6d 100644
--- a/guix/scripts/pack.scm
+++ b/guix/scripts/pack.scm
@@ -3,6 +3,7 @@ 
 ;;; Copyright © 2017, 2018 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2018 Konrad Hinsen <konrad.hinsen@fastmail.net>
 ;;; Copyright © 2018 Chris Marusich <cmmarusich@gmail.com>
+;;; Copyright © 2018 Efraim Flashner <efraim@flashner.co.il>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -551,6 +552,18 @@  please email '~a'~%")
     (squashfs . ,squashfs-image)
     (docker  . ,docker-image)))
 
+(define (show-formats)
+  ;; Print the supported pack formats.
+  (display (G_ "The supported formats for 'guix pack' are:"))
+  (newline)
+  (display (G_ "
+  tarball       A self-contained tarball, ready to run on another machine."))
+  (display (G_ "
+  squashfs      A squashfs image, ready for 'cluster engines' and the like."))
+  (display (G_ "
+  docker        A tarball ready for 'docker load'."))
+  (newline))
+
 (define %options
   ;; Specifications of the command-line options.
   (cons* (option '(#\h "help") #f #f
@@ -567,6 +580,10 @@  please email '~a'~%")
          (option '(#\f "format") #t #f
                  (lambda (opt name arg result)
                    (alist-cons 'format (string->symbol arg) result)))
+         (option '("list-formats") #f #f
+                 (lambda args
+                   (show-formats)
+                   (exit 0)))
          (option '(#\R "relocatable") #f #f
                  (lambda (opt name arg result)
                    (alist-cons 'relocatable? #t result)))
@@ -621,6 +638,8 @@  Create a bundle of PACKAGE.\n"))
   (newline)
   (display (G_ "
   -f, --format=FORMAT    build a pack in the given FORMAT"))
+  (display (G_ "
+      --list-formats     list the formats available"))
   (display (G_ "
   -R, --relocatable      produce relocatable executables"))
   (display (G_ "