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

From: Duy Nguyen <pclouds@gmail.com>
Date: Feb 20, 2014, 13:15 UTC
Message-ID: <CACsJy8B11ZjLgLiE=S1sqZ09z2GtbwGfdJRuQ0AC_ppnQsQvcQ@mail.gmail.com>
In-Reply-To: <xmqq38je9739.fsf@gitster.dls.corp.google.com>

On Thu, Feb 20, 2014 at 3:32 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 25 quoted lines

> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
> (Only nitpicks during this round of review).
>
>> +     if (get_device_or_die(path) != get_device_or_die(get_git_dir())) {
>> +             strbuf_reset(&sb);
>> +             strbuf_addf(&sb, "%s/locked", sb_repo.buf);
>> +             write_file(sb.buf, 1, "located on a different file system\n");
>> +             keep_locked = 1;
>> +     } else {
>> +             strbuf_reset(&sb);
>> +             strbuf_addf(&sb, "%s/link", sb_repo.buf);
>> +             link(sb_git.buf, sb.buf); /* it's ok to fail */
>
> If so, perhaps tell that to the code by saying something like
>
>                 (void) link(...);
>
> ?
>
> 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.

-- 
Duy