Re: [PATCH 1/2] index-pack: Create .keep files with same permissions and .pack/.idx

On Friday 01 April 2011, Shawn Pearce wrote:

The only problem is a cpio based clone, which may link the objects
directory before the refs, and miss linking the new pack but wind up
linking the new ref, making the clone corrupt. But that is a bug in
the cpio clone implementation. Using file:// to use the classical pipe
is safe here, because the refs are scanned before the objects are.
IMHO, if you think clone during push is unsafe because of this, we
should fix the cpio clone path to do a `git ls-remote` on the source,
cache the refs in memory, copy the objects, then write out a
packed-refs file containing the refs we snapshotted *before* linking
the objects directory into the new clone.

Looking at clone_local() in builtin/clone.c (which I guess is the
culprit here), wouldn't we fix this simply by swapping the two parts
of the function, so that the refs part is done before the objects
part? Like this:


static const struct ref *clone_local(const char *src_repo,
				     const char *dest_repo)
{
	const struct ref *ret;
	struct strbuf src = STRBUF_INIT;
	struct strbuf dest = STRBUF_INIT;
	struct remote *remote;
	struct transport *transport;

	remote = remote_get(src_repo);
	transport = transport_get(remote, src_repo);
	ret = transport_get_remote_refs(transport);
	transport_disconnect(transport);

	if (option_shared)
		add_to_alternates_file(src_repo);
	else {
		strbuf_addf(&src, "%s/objects", src_repo);
		strbuf_addf(&dest, "%s/objects", dest_repo);
		copy_or_link_directory(&src, &dest);
		strbuf_release(&src);
		strbuf_release(&dest);
	}

	if (0 <= option_verbosity)
		printf("done.\n");
	return ret;
}


Have fun! :)

...Johan

-- 
Johan Herland, [off-list ref]
www.herland.net