Re: [PATCH] clone: Skip pack-*.keep files when cloning locally

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jens Lindström <jl@xxxxxxxxx> writes:

> On Fri, Jun 28, 2013 at 8:38 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
>>> The pack-*.keep files are temporary, and serve no purpose in the
>>> clone.
>>
>> They are not temporary, actually. A user can deliberatey create a
>> "keep" marker after packing with a good set of parameters, so that
>> the resulting pack will be kept, instead of letting a later repack
>> (with faster set of parameters) remove and replace it with less
>> optimal results.
>
> Ah, I see.  Was (obviously) not aware of that.  It would perhaps be a
> good idea to be able to differentiate between such permanent keep
> files and the temporary ones created by built-in commands.

I am not sure if we should care that deeply about them in the first
place.  For these temporary ".keep" lockfiles to matter, you have to
be doing "clone --local --no-hardlinks", which is a "cp -R" that
bypasses the usual Git transport mechanism, while there is still
activity in the source repository (a fetch is slurping new objects
into a newly created pack with such a temporary ".keep" lockfile,
the refs may not have been updated yet).  The result is expected to
have inconsistencies even if ".keep" were readable in such a copy.

> Also, even if some keep files are permanent in the source repository,
> is it always a good idea to copy them over to the clone?

The local "cheap cp -R clone" are primarily used by people copying
their own private repository that is in a quiescent state, and in
that set-up, copying ".keep" by default has been a good idea.  Of
course, after copying to a new repository, if they want to repack
with different parameters, they need to unplug ".keep" files
manually, and it can be argued that the default could have been not
to copy and we could have forced those who want to reuse a tight
pack they created earlier to manually copy or create the ".keep"
files instead.
>
> On Fri, Jun 28, 2013 at 10:38 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
>> That is, something like this, perhaps?
>
> Comments:
>
> With this patch, it still fails with --local, when the link() call
> fails.

Oh, I wasn't even attempting to look at the hardlink codepath in the
"something like this" illustration patch ;-).

Besides, I think you can make a hardlink to a file that you cannot
read.

If you apply only the parts of the patch that adds the test, without
applying either your patch or mine to builtin/clone.c, and remove
"--no-hardlinks" to force the hardlink codepath, what happens?  We
create an unreadable $keepname in strictsrc repository and I think
the local clone (which is not "cp -R" but something like "find"
piped to "cpio -pluadm") would make a "copy" of it just fine.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]