RFC patch, 3 partsreplace_object: add mechanism to replace objects found in "refs/replace/"

The code of this mechanism has been copied from the commit graft code.

Currently this mechanism is only used from the "parse_commit_buffer" function in "commit.c". It should probably be used from "fsck.c" too.

(For information, grafts are looked up only from "parse_commit_buffer" function in "commit.c" and from "fsck_commit" in "fsck.c".)

In "parse_commit_buffer", the parent sha1s from the original commit or from a commit graft that match a ref name in "refs/replace/" are replaced by the commit sha1 that has been read in the ref.

This means that for example "git show <original commit sha1>" will display information about the original commit. If the mechanism had been called from "read_sha1_file" instead of when parents are read, then "git show <original commit sha1>" would display information about the commit that replaces the original one. This may be seen as a feature or as a bug depending on the point of view.

Anyway this implementation makes sure that the mechanism is triggered only when commit graft could be triggered, so hopefully the object reachability traverser will ignore this mechanism as it ignores the graft mechanism.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Makefile         |    1 +
 commit.c         |    7 +++-
 commit.h         |    2 +
 replace_object.c |  102 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 110 insertions(+), 2 deletions(-)
 create mode 100644 replace_object.c

Christian Couder <chriscool@tuxfamily.org> writes:

I may be reading this wrong, but if you replace the parent of the commit, that won't improve anything over the graft mechanism, which we already have.

What I was hoping to see was to give replacing commit back when a commit is asked for by normal callers, while not replacing the objects when reachability traversal (prune, pack transfer and fsck) wants to read the commit. That way:

 (1) Normal callers will see the rewritten history (which is the same as
     graft); and

 (2) We will kill the design bug of the current graft mechanism that graft
     information cannot be transferred to the other end.  By using object
     replacement, you can fetch objects reachable from refs/replace/ at
     the other end and place them in the same refs/replace/ hierarchy
     locally, if you choose to use the same altered history, or you can
     choose not to use the replacement yourself.  The resulting repository
     is fully connected either way.

 (3) We will also kill the design bug of the current graft mechanism that
     graft information hides repository corruption to fsck.  The problem
     with this is that if you and then remove the grafts, you will end up
     with a corrupt repository, because these operations do look at grafts
     (this is justified only partly because otherwise you will lose
     objects that are reachable only via grafts, but is broken at the same
     time because you can lose the true parents by letting graft hide them
     from a reachable commit).

     By doing fsck and prune always using the true reachability, and using
     refs in the refs/replace/ hierarchy for protecting objects that are
     involved in this new way of grafting, you will ensure the integrity
     of the repository.  Removal of a ref from the refs/replace/ hierarchy
     won't result in such a corruption we can have today.

And that is exactly the reason why I was hoping the hook will be at read_sha1_file() that gives you a rewritten object contents when you ask for the original object, not at parse_commit_buffer which does not give you a rewritten object, but makes you follow to a rewritten object when parsing a commit (which itself is not replaced if it is the starting point).

Doing replacement at parse_commit_buffer() time is one step too late. For example, if you have replacement information for the commit that sits at the tip of 'master' branch, I think your "git log master" will ignore that replacement information. Creating one new commit on top of it and saying "git log master" then will show the new commit, and suddenly starts showing the replacement commit for the one you used to have at the tip of the branch.

Le mercredi 7 janvier 2009, Junio C Hamano a écrit :

Yeah, but read_sha1_file is called to read all object files, not just commits. So putting the hook there will:

	1) add a lookup overhead when reading any object,
	2) make it possible to replace any object,

And there is also the following problem:

	3) this function is often called like this:

	buffer = read_sha1_file(sha1, &type, &size);
	if (!buffer)
		die("Cannot read %s", sha1_to_hex(sha1));

	so in case of error, it will give an error message with a bad sha1
	in it because the sha1 of the file that we cannot read is the sha1
	in the replace ref not the one passed to read_sha1_file.

To avoid the above problems, maybe we can try to also improve what read_sha1_file does:

1) allow callers to pass a type in the "type" argument and only lookup in 
the replace refs if we say we want a commit, but this makes calling this 
function more error prone
2) when we say we want an object with a given type, check if the object we 
read has this type (and die if not)
3) die in read_sha1_file when there is an error and we are replacing so that 
callers don't need to die themself and so that we can always report an 
accurate sha1 in the error message

Something like this (totally untested):

void *read_sha1_file(const unsigned char *sha1, enum object_type *type,
		     unsigned long *size)
{
	void *data;
	enum object_type read_type;
	int replacing = (type && *type == OBJ_COMMIT);

	/* only replace commits when we ask for one */
	if (replacing)
		sha1 = lookup_replace_object(sha1);
	data = read_object(sha1, &read_type, size);
	/* legacy behavior is to die on corrupted objects */
	if (!data && (has_loose_object(sha1) || has_packed_and_bad(sha1)))
		die("object %s is corrupted", sha1_to_hex(sha1));
	if (type && *type != OBJ_NONE && *type != read_type)
		die("type of object %s (%d) is different from the type we want (%d)",
		    sha1_to_hex(sha1), read_type, *type);
	if (!data && replacing)
		die("Cannot read commit %s", sha1_to_hex(sha1));

	return data;
}

Or perhaps it is better to leave read_sha1_file as it is and add a new function like this:

void *read_commit(const unsigned char *sha1, unsigned long *size)
{
	void *data;
	enum object_type type;

	sha1 = lookup_replace_object(sha1);
	data = read_sha1_file(sha1, &type, size);

	if (*type != OBJ_COMMIT)
		die("read_commit: object %s is not a commit", sha1_to_hex(sha1));
	if (!data)
		die("read_commit: cannot read commit %s", sha1_to_hex(sha1));

	return data;
}

The latter seems better to me. But I haven't looked much at the 60 read_sha1_file callers yet.

[...]

Yeah, I agree there are drawbacks to do it at parse_commit_buffer time.

Thanks, Christian.

Christian Couder <chriscool@tuxfamily.org> writes:

I actually see (2) as an improvement, and (1) as an associated cost.

You have refs/replace/$A that records $B, to tell git that the real object $A in the history is replaced by another object $B. The caller feeds $A in the above snippet to read_sha1_file(), and your read_sha1_file() notices that it needs to read $B instead, returns the buffer from the object $B, and reports its type and size. If there is no $B available, it may return NULL and the caller says "I asked for $A but in this repository I cannot get to it". That sounds consistent to me, but I agree it would be more helpful to report "and the reason why I cannot get to it is because you have replacement defined as $B which you do not have."

This is debatable, but can go either way.

That we already do anyway, don't we? parse_commit() gets data from read_sha1_file() and would complain if it gets a blob, etc.

I expect the use of graft and object replacement (or if you insist, "commit replacement") rather rare, and I think it is probably Ok to declare it a grave repository misconfiguration if somebody claims that $A is replaced by $B without actually having $B:

	void *read_sha1_file(sha1, type, size)
        {
		void *data;
        	unsigned char *replacement = lookup_replace_object(sha1);
                if (replacement) {
                	data = read_sha1_file(replacement, type, size);
                        if (!data)
                        	die("replacement %s not found for %s",
                                    get_sha1_hex(replacement),
                                    get_sha1_hex(sha1));
		} else {
			data = read_object(sha1, type, size);
		}
                ... existing code ...
                return data;                
        }

To disable replacement for connectivity walkers, lookup_replace_object() can look at a some global defined in environment.c, perhaps.

Junio C Hamano <gitster@pobox.com> writes:

I just had an idea: we can use this mechanism to better manage large binary files in Git, by using replacements for _blobs_.

We want to be able to have two flavours of repository: one with large blobs (media files usually), and one without. We can use stubs in the place of large binary files in 'no-megablobs' flavor, and add contents of those files via refs/replace/* for _blobs_ in 'with-megablobs' flavour. We can control which objects we want to have, and which objects to transfer.

What do you think about this (abuse of) an idea?

Hi,

On Wed, 7 Jan 2009, Christian Couder wrote:

I suspect that this sorted list should be a hashmap.

Ciao, Dscho

On Mon, 12 Jan 2009, Tim 'mithro' Ansell wrote:

Actually this idea is a bit different from lazy / on demand downloading of large blobs.

In "on demand loading" solution you have sha-1 of _full_ blob in a tree, but git knows that not having it is not a fatal error (somehow), and you can ask git to download it. This is as far as I understand the solution you proposed and partially implemented.

In "replacement blobs" solution, the (ab)use of object replacement mechanism meant originally for easier bisect, you have sha-1 of _stub_ object in a tree. If you want full (large) blob, you add replacement in refs/replace/*, replacing stub blob object (must be unique; it can for example contain some header + sha-1 of full object) with full object. If you want to have large object, you need to transfer refs/replace/*. This solution means that user needs to be aware of this mechanism, or have some wrapper (script) around it.

Different solution, and a bit different behavior wrt getting large object for the user.