[bug#36404] [PATCH v5 0/4] Add 'guix deploy'.

[Jakob L. Kreuze]

"Thompson, David" <address@hidden> writes:

> Replace "path" with "file name". Lots of people use them
> interchangeably, but GNU makes a clear distinction between the two
> terms.

Ah, good to know. Updated.

Ludovic Courtès <address@hidden> writes:

> Please add this file to po/guix/POTFILES.in so it can be subject to
> localization.
>
>> +(define %default-options
>> +  '((system . ,(%current-system))
>> +    (substitutes? . #t)
>> +    (build-hook? . #t)
>> +    (graft? . #t)
>> +    (debug . 0)
>> +    (verbosity . 2)))
>
> ‘verbosity’ should probably be 1 (only ‘guix build’ and ‘guix system
> build’ default to 2.)
>
>> +      (for-each (lambda (machine)
>> +                  (format #t "building ~a... " (machine-display-name 
>> machine))
>> +                  (run-with-store store (build-machine machine))
>> +                  (display "done\n"))
>> +                machines)
>> +      (for-each (lambda (machine)
>> +                  (format #t "deploying to ~a... " (machine-display-name 
>> machine))
>> +                  (run-with-store store (deploy-machine machine))
>> +                  (display "done\n"))
>> +                machines))))
>
> For i18n purposes and also to get consistent output, please avoid
> ‘format #t’ and instead write:
>
>   (info (G_ "deploying ~a…~%") (machine-display-name machine))
>
> I think you can omit the “done” message.
>
> As a matter of style, it’s clearer IMO to have only one ‘run-with-store’
> call in the whole program.

As in, create a monadic expression with 'mapm' to evaluate the multiple
calls to '(deploy-machine machine)' in sequence, and then pass that to
'run-with-store'?

> In the SSH case, ‘deploy-machine’ should roughly translate to:
> 
>   (remote-eval #~(switch-to-system #$os) machine)
> 
> Thus, ‘build-machine’ is unnecessary: the actual build of OS is
> automatically triggered by ‘remote-eval’, either locally or remotely,
> depending on #:build-locally?.
> 
> So I believe you can remove ‘build-machine’ altogether.

Thanks for pointing that out; I meant to ask about that since it's kinda
vestigial at this point, but wasn't sure if it would be better to have it for
the UI. But I went ahead and removed it, since we already have code for
showing what derivations are going to be built, etc.

> It’s a bit verbose, but I’d suggest using SRFI-34/35 instead, like so:
> 
>   (raise (condition
>           (&message (message "unsupported machine configuration type"))))
> 
> That way, if you also add the file to po/guix/POTFILES.in, i18n will do
> its magic.  :-)

In the end, I generalized the various configuration-related error messages
into a 'maybe-raise-unsupported-configuration-error' that uses
SRFI-35. Hopefully that's alright -- I believe the manual specifies the
behavior enough that one more detailed message is better than two.

> Yay!
>
> You can add a copyright line for you at the top of guix.texi.
>
>> +@section Invoking @code{guix deploy}
>> +
>> +We've already seen @code{operating-system} declarations used to manage a
>> +machine's configuration locally.  Suppose you need to configure multiple
>> +machines, though---perhaps you're managing a service on the web that's
>> +comprised of several servers.  @command{guix deploy} enables you to use 
>> those
>> +same @code{operating-system} declarations to manage multiple remote hosts at
>> +once as a logical ``deployment''.
>
> Perhaps add something like:
>
>   @quotation Note
>   The functionality described in this section is still under development
>   and is subject to change.  Get in touch with us on
>   @email{guix-devel@@gnu.org}!
>   @end quotation
>
> That way, if we make a Guix release before this is all stabilized,
> we make sure people have appropriate expectations.  :-)

I like it!

>> +complex deployment may involve, for example, starting virtual machines 
>> through
>> +a VPS provider.  In such a case, a different @var{environment} type would be
>      ^^^
> I would write “Virtual Private Server (VPS)”.
>
> I hope the nitpicking level is acceptable, let me know.  I’m really
> excited to see this land in master!

Oh, I appreciate this level of attention to detail. The hardest part of
technical writing for me is having my writing fit in with the writing around
it when contributing to an existing document, so these kinds of comments from
someone more familiar with the manual are great.

Jakob L. Kreuze (4):
  ssh: Add 'identity' keyword to 'open-ssh-session'.
  gnu: Add machine type for deployment specifications.
  Add 'guix deploy'.
  doc: Add section for 'guix deploy'.

 Makefile.am             |   4 +-
 doc/guix.texi           | 114 +++++++++++++
 gnu/local.mk            |   5 +-
 gnu/machine.scm         | 107 ++++++++++++
 gnu/machine/ssh.scm     | 369 ++++++++++++++++++++++++++++++++++++++++
 guix/scripts/deploy.scm |  84 +++++++++
 guix/ssh.scm            |  10 +-
 po/guix/POTFILES.in     |   2 +
 8 files changed, 689 insertions(+), 6 deletions(-)
 create mode 100644 gnu/machine.scm
 create mode 100644 gnu/machine/ssh.scm
 create mode 100644 guix/scripts/deploy.scm

-- 
2.22.0

signature.asc
Description: PGP signature