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

Duy Nguyen [off-list ref] writes:

quoted

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.