Re: [PATCH v3 24/25] prune: strategies for linked checkouts

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 20, 2014, 19:55 UTC
Message-ID: <xmqqd2ih5zk4.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CACsJy8B11ZjLgLiE=S1sqZ09z2GtbwGfdJRuQ0AC_ppnQsQvcQ@mail.gmail.com>

Duy Nguyen <pclouds@gmail.com> writes:

Show 13 quoted lines

>> But why is it OK to fail in the first place?  If we couldn't link,
>> don't you want to fall back to the lock codepath?  After all, the
>> "are we on the same device?" check is to cope with the case where
>> we cannot link and that alternate codepath is supposed to be
>> prepared to handle the "ah, we cannot link" case correctly, no?
>
> Filesystems not supporting hardlinks are one reason. The idea behind
> "locked" is that the new checkout could be on a portable device and we
> don't want to prune its metadata just because the device is not
> plugged in. Checking same device help but even that can't guarantee no
> false positives, unless your only mount point is /. So no I don't
> really think we should go lock whenever link() fails, that would just
> make fewer checkouts prunable.

It just looked wrong to have a logic that goes like this:

        if (we detect one case link() can never possibly work) {
                do something for a case we do not use link;
        } else {
                prepare parameters that is used when we do link;
                do link() but ignore error;
        }

and felt that I would have understood if it were more like this:

        prepare parameters that is used when we do link;
        if (try link() and it fails) {
                do something for a case we do not use link;
        }

but I did not mean that was the only alternative to correct the "wrongness".

I do not think I yet know the motivation behind this step well enough to further comment---until I read the earlier parts of the series that lead to this step, so I'll do that first.

Thanks.