[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#37978] [PATCH] guix: new command "guix time-machine"
From: |
Ludovic Courtès |
Subject: |
[bug#37978] [PATCH] guix: new command "guix time-machine" |
Date: |
Wed, 06 Nov 2019 14:53:40 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) |
Hello Konrad!
Konrad Hinsen <address@hidden> skribis:
> * guix/scripts/time-machine.scm: New file.
> * guix/scripts/pull.scm: Export function channel-list.
> * guix/inferior.scm: New function cached-guix-filetree-for-channels.
> * doc/guix.texi: Document "git time-machine"
Awesome. :-)
Please also add time-machine.scm to Makefile.am. In the commit log,
you’ll get bonus points if you mention the modified entities in
parentheses (see ‘git log’ for examples.) :-)
> @@ -247,6 +247,7 @@ Utilities
> * Invoking guix container:: Process isolation.
> * Invoking guix weather:: Assessing substitute availability.
> * Invoking guix processes:: Listing client processes.
> +* Invoking guix time-machine:: Running an older version of Guix.
How about moving this section a bit higher, because it’s more widely
useful than ‘guix processes’ for instance? Actually, it could go under
“Package Management” right before “Inferiors”, WDYT?
> The @command{guix describe --format=channels} command can even generate this
> -list of channels directly (@pxref{Invoking guix describe}).
> +list of channels directly (@pxref{Invoking guix describe}). The resulting
> +file can be used with the -C options of @command{guix pull}
> +(@pxref{Invoking guix pull}) or @command{guix time-machine}
> +(@pxref{Invoking guix time-machine}).
Nitpick: Please write two spaces after an end-of-sentence period.
> +The general syntax is:
> +
> +@example
> +guix time-machine @var{channels} -- @var{command} @var {arg}@dots{}
> +@end example
I think it should be “guix time-machine @var{options}@dots{} -- …”,
right?
IIUC, if you run:
guix time-machine -- build hello
you build “hello” with the latest master, right?
Perhaps it would be good to add an example like this one actually, WDYT?
> +where @var{command} and @var{arg}@dots{} are passed unmodified to the
> +@command{guix} command in its old version. The @var{channels} that define
> +this version can be specified using the following options:
Perhaps add “like for @command{guix pull}”.
> -(define* (inferior-for-channels channels
> - #:key
> - (cache-directory (%inferior-cache-directory))
> - (ttl (* 3600 24 30)))
> - "Return an inferior for CHANNELS, a list of channels. Use the cache at
> -CACHE-DIRECTORY, where entries can be reclaimed after TTL seconds. This
> -procedure opens a new connection to the build daemon.
> -
> -This is a convenience procedure that people may use in manifests passed to
> -'guix package -m', for instance."
> +(define* (cached-guix-filetree-for-channels channels
> + #:key
> + (cache-directory
> (%inferior-cache-directory))
> + (ttl (* 3600 24 30)))
> + "Return a directory containing a guix filetree defined by CHANNELS, a list
> of channels.
> +The directory is a subdirectory of CACHE-DIRECTORY, where entries can be
> reclaimed after TTL seconds.
> +This procedure opens a new connection to the build daemon."
How about (1) calling it ‘cached-channel-instance’ (or similar), and (2)
not opening a connection to the daemon?
Regarding (2), it means that procedure would be a monadic procedure and
it’s up to the user to do with-store + run-with-store or whatever. The
general convention is to not open new connections on behalf of the user
(‘inferior-for-channels’ is one of the only exceptions to the rule
because it’s a convenience function for use in manifests.)
Perhaps this change should be a separate patch.
> +(define (guix-time-machine . args)
> + (with-error-handling
> + (let* ((opts (parse-args args))
> + (channels (channel-list opts))
> + (command-line (assoc-ref opts 'exec))
> + (directory (cached-guix-filetree-for-channels channels))
> + (executable (string-append directory "/bin/guix")))
> + (apply system* (cons executable command-line)))))
I think this should be:
(apply execl executable command-line)
so that we don’t create an extra process and actually get the exit code
for that sub-process.
Thanks,
Ludo’.
- [bug#37978] [PATCH] guix: new command "guix time-machine",
Ludovic Courtès <=