From: Eric Bavier <ericbavier@centurylink.net>
To: Hartmut Goebel <h.goebel@crazy-compilers.com>
Cc: guix-devel@gnu.org
Subject: Re: [PATCH] gc: Add option --keep-going.
Date: Thu, 17 Nov 2016 22:49:03 -0600 [thread overview]
Message-ID: <20161117224903.46deb2e6@centurylink.net> (raw)
In-Reply-To: <1479307034-20585-1-git-send-email-h.goebel@crazy-compilers.com>
Just a few comments/suggestions.
On Wed, 16 Nov 2016 15:37:14 +0100
Hartmut Goebel <h.goebel@crazy-compilers.com> wrote:
> * guix/scripts/gc.scm (show-help, %options): Add option -k/--keep-going.
> (guix-gc): Pass value off option --keep-going on to delete-paths.
^
Changelog continuation lines should start in column 0.
> * guix/store.scm (%protocol-version): Increment to 17.
> (delete-paths) New key-word parameter `keep-going?`, pass it on to
> run-rc.
> (run-gc): New key-word parameter `keep-going?`, send value of
> keep-going? to the daemon.
> * nix/libstore/store-api.hh (GCOptions): Add boolean keepGoing.
> * nix/libstore/worker-protocol.hh (PROTOCOL_VERSION) Increment to 17.
> * nix/nix-daemon/nix-daemon.cc (performOp)[wopCollectGarbage] Read
> keepGoing.
> * nix/libstore/gc.cc (LocalStore::collectGarbage) If keepGoing is true
> print an error message instead of throwing an error.
> * doc/guix.texi (Invoking guix gc): Document option --keep-going.
> ---
> doc/guix.texi | 8 +++++++-
> guix/scripts/gc.scm | 9 ++++++++-
> guix/store.scm | 12 ++++++++----
> nix/libstore/gc.cc | 7 ++++++-
> nix/libstore/store-api.hh | 3 +++
> nix/libstore/worker-protocol.hh | 2 +-
> nix/nix-daemon/nix-daemon.cc | 2 ++
> 7 files changed, 35 insertions(+), 8 deletions(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index a3eba58..b8362d6 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -2098,7 +2098,13 @@ nothing and exit immediately.
> @itemx -d
> Attempt to delete all the store files and directories specified as
> arguments. This fails if some of the files are not in the store, or if
> -they are still live.
> +they are still live (with behaviour can be changed with
> +@option{--keep-going}).
How about: "Unless the @option{--keep-going} option is given this will
fail if some of the files are not in the store, or if they are still
live."
> +
> +@item --keep-going
> +@itemx -k
> +Keep going when @option{--delete} is given and some of the store files
^
maybe "any of the store files or directories"?
> +and directories specified as arguments fail to re removed.
^
"be"
>
> @item --list-failures
> List store items corresponding to cached build failures.
> diff --git a/guix/scripts/gc.scm b/guix/scripts/gc.scm
> index bdfee43..778e9a7 100644
> --- a/guix/scripts/gc.scm
> +++ b/guix/scripts/gc.scm
> @@ -72,6 +72,9 @@ Invoke the garbage collector.\n"))
> --clear-failures remove PATHS from the set of cached failures"))
> (newline)
> (display (_ "
> + -k, --keep-going keep going when some of th pathes can not be deleted"))
"keep going if paths cannot be deleted"
> + (newline)
> + (display (_ "
> -h, --help display this help and exit"))
> (display (_ "
> -V, --version display version information and exit"))
> @@ -107,6 +110,9 @@ Invoke the garbage collector.\n"))
> (lambda (opt name arg result)
> (alist-cons 'action 'delete
> (alist-delete 'action result))))
> + (option '(#\k "keep-going") #f #f
> + (lambda (opt name arg result)
> + (alist-cons 'keep-going? #t result)))
> (option '("optimize") #f #f
> (lambda (opt name arg result)
> (alist-cons 'action 'optimize
> @@ -228,7 +234,8 @@ Invoke the garbage collector.\n"))
> (let-values (((paths freed) (collect-garbage store)))
> (info (_ "freed ~h bytes~%") freed))))))
> ((delete)
> - (delete-paths store (map direct-store-path paths)))
> + (delete-paths store (map direct-store-path paths)
> + #:keep-going? (assoc-ref opts 'keep-going?)))
> ((list-references)
> (list-relatives references))
> ((list-requisites)
> diff --git a/guix/store.scm b/guix/store.scm
> index 2023875..4276db4 100644
> --- a/guix/store.scm
> +++ b/guix/store.scm
> @@ -137,7 +137,7 @@
> direct-store-path
> log-file))
>
> -(define %protocol-version #x110)
> +(define %protocol-version #x111)
>
> (define %worker-magic-1 #x6e697863) ; "nixc"
> (define %worker-magic-2 #x6478696f) ; "dxio"
> @@ -938,7 +938,7 @@ is not an atomic operation.) When CHECK-CONTENTS? is true, check the contents
> of store items; this can take a lot of time."
> (not (verify store check-contents? repair?)))))
>
> -(define (run-gc server action to-delete min-freed)
> +(define* (run-gc server action to-delete min-freed #:key (keep-going? #f))
> "Perform the garbage-collector operation ACTION, one of the
> `gc-action' values. When ACTION is `delete-specific', the TO-DELETE is
> the list of store paths to delete. IGNORE-LIVENESS? should always be
> @@ -956,6 +956,8 @@ and the number of bytes freed."
> ;; Obsolete `use-atime' and `max-atime' parameters.
> (write-int 0 s)
> (write-int 0 s))
> + (when (>= (nix-server-minor-version server) 17)
> + (write-arg boolean keep-going? s))
>
> ;; Loop until the server is done sending error output.
> (let loop ((done? (process-stderr server)))
> @@ -993,12 +995,14 @@ then collect at least MIN-FREED bytes. Return the paths that were
> collected, and the number of bytes freed."
> (run-gc server (gc-action delete-dead) '() min-freed))
>
> -(define* (delete-paths server paths #:optional (min-freed (%long-long-max)))
> +(define* (delete-paths server paths #:optional (min-freed (%long-long-max))
> + #:key (keep-going? #f))
> "Delete PATHS from the store at SERVER, if they are no longer
> referenced. If MIN-FREED is non-zero, then stop after at least
> MIN-FREED bytes have been collected. Return the paths that were
> collected, and the number of bytes freed."
> - (run-gc server (gc-action delete-specific) paths min-freed))
> + (run-gc server (gc-action delete-specific) paths min-freed
> + #:keep-going? keep-going?))
>
> (define (import-paths server port)
> "Import the set of store paths read from PORT into SERVER's store. An error
> diff --git a/nix/libstore/gc.cc b/nix/libstore/gc.cc
> index 72eff52..6f2d8f7 100644
> --- a/nix/libstore/gc.cc
> +++ b/nix/libstore/gc.cc
> @@ -662,8 +662,13 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results)
> foreach (PathSet::iterator, i, options.pathsToDelete) {
> assertStorePath(*i);
> tryToDelete(state, *i);
> - if (state.dead.find(*i) == state.dead.end())
> + if (state.dead.find(*i) == state.dead.end()) {
> + if (options.keepGoing) {
> + printMsg(lvlError, format("cannot delete path `%1%' since it is still alive") % *i);
^
Is error formatting appropriate, or is there a warning level we can use?
> + } else {
> throw Error(format("cannot delete path `%1%' since it is still alive") % *i);
> + }
> + }
> }
>
> } else if (options.maxFreed > 0) {
> diff --git a/nix/libstore/store-api.hh b/nix/libstore/store-api.hh
> index fa78d59..8b0f521 100644
> --- a/nix/libstore/store-api.hh
> +++ b/nix/libstore/store-api.hh
> @@ -50,6 +50,9 @@ struct GCOptions
> /* Stop after at least `maxFreed' bytes have been freed. */
> unsigned long long maxFreed;
>
> + /* keep going even if some of the paths can not be collected */
> + bool keepGoing;
> +
> GCOptions();
> };
>
> diff --git a/nix/libstore/worker-protocol.hh b/nix/libstore/worker-protocol.hh
> index 99c1ee2..8faf193 100644
> --- a/nix/libstore/worker-protocol.hh
> +++ b/nix/libstore/worker-protocol.hh
> @@ -6,7 +6,7 @@ namespace nix {
> #define WORKER_MAGIC_1 0x6e697863
> #define WORKER_MAGIC_2 0x6478696f
>
> -#define PROTOCOL_VERSION 0x110
> +#define PROTOCOL_VERSION 0x111
> #define GET_PROTOCOL_MAJOR(x) ((x) & 0xff00)
> #define GET_PROTOCOL_MINOR(x) ((x) & 0x00ff)
>
> diff --git a/nix/nix-daemon/nix-daemon.cc b/nix/nix-daemon/nix-daemon.cc
> index a1fce25..b04cece 100644
> --- a/nix/nix-daemon/nix-daemon.cc
> +++ b/nix/nix-daemon/nix-daemon.cc
> @@ -526,6 +526,8 @@ static void performOp(bool trusted, unsigned int clientVersion,
> readInt(from);
> readInt(from);
> }
> + if (GET_PROTOCOL_MINOR(clientVersion) >= 17)
> + options.keepGoing = readInt(from) != 0;
^
Check tabbing/spacing here.
>
> GCResults results;
>
next prev parent reply other threads:[~2016-11-18 4:49 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-16 14:37 [PATCH] gc: Add option --keep-going Hartmut Goebel
2016-11-18 4:49 ` Eric Bavier [this message]
2016-11-24 9:28 ` Hartmut Goebel
2016-11-27 21:07 ` Ludovic Courtès
2016-11-18 23:15 ` Ludovic Courtès
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161117224903.46deb2e6@centurylink.net \
--to=ericbavier@centurylink.net \
--cc=bavier@member.fsf.org \
--cc=guix-devel@gnu.org \
--cc=h.goebel@crazy-compilers.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.