From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Bavier Subject: Re: [PATCH] gc: Add option --keep-going. Date: Thu, 17 Nov 2016 22:49:03 -0600 Message-ID: <20161117224903.46deb2e6@centurylink.net> References: <1479307034-20585-1-git-send-email-h.goebel@crazy-compilers.com> Reply-To: bavier@member.fsf.org Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:37965) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c7b6s-0003bN-7K for guix-devel@gnu.org; Thu, 17 Nov 2016 23:49:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c7b6p-0002cl-2e for guix-devel@gnu.org; Thu, 17 Nov 2016 23:49:10 -0500 Received: from mail.centurylink.net ([205.219.233.9]:8863 helo=smtp.centurylink.net) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c7b6o-0002cK-Qj for guix-devel@gnu.org; Thu, 17 Nov 2016 23:49:07 -0500 In-Reply-To: <1479307034-20585-1-git-send-email-h.goebel@crazy-compilers.com> List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: Hartmut Goebel Cc: guix-devel@gnu.org Just a few comments/suggestions. On Wed, 16 Nov 2016 15:37:14 +0100 Hartmut Goebel 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; >