[bug#51427] [PATCH] nix: libstore: Do not remove unused links when deleting specific items.

[Liliana Marie Prikler]

Hi,

Am Donnerstag, den 28.10.2021, 16:16 +0200 schrieb Ludovic Courtès:
> Hi,
> 
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
> 
> > Deleting unused links can be a very costly operation, especially on
> > rotative hard drives.  As removing single store items is often used
> > for experimentation rather than for cleaning purposes, this change
> > allows it to run without the links cleanup.
> > 
> > * nix/libstore/gc.cc (LocalStore::collectGarbage): Do not clean up
> > links when
> > the specified action is GCOptions::gcDeleteSpecific.
> > ---
> >  nix/libstore/gc.cc | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/nix/libstore/gc.cc b/nix/libstore/gc.cc
> > index e1d0765154..7d872d8cc1 100644
> > --- a/nix/libstore/gc.cc
> > +++ b/nix/libstore/gc.cc
> > @@ -771,7 +771,7 @@ void LocalStore::collectGarbage(const GCOptions
> > & options, GCResults & results)
> >      deleteGarbage(state, state.trashDir);
> >  
> >      /* Clean up the links directory. */
> > -    if (options.action == GCOptions::gcDeleteDead ||
> > options.action == GCOptions::gcDeleteSpecific) {
> > +    if (options.action == GCOptions::gcDeleteDead) {
> 
> I believe the effect is that ‘guix gc -D /gnu/store/…-disk-image’
> would remove nothing: /gnu/store/.links would still contain a copy of
> that big disk image, so as a result, you’ve freed zero bytes.
> 
> Am I right?
I think that might be the point.  As Maxim said, single items are
(likely) not removed for cleaning purposes, so freeing the disk image
has little effect.  Plus, you could invoke it like

  guix gc -D dead-item dead-item live-item dead-item

It would fail at live-item and then not continue to free the links of
the two dead items prior.

So there's a few things we could do here:

1. simply fail and have the user deal with it (including the option of
doing a normal `guix gc' or `guix gc -C 1')
2. remember which paths were live and dead and always clean up the
links, only reporting errors afterwards
3. add an option to explicitly check the .links directory (which
defaults to true for the current things, but could also be used to
clean links after a liveness check or after a do-nothing `guix gc -F').
4. ...

WDYT?