Re: [PATCH 1/3] git reset --hard gives clean working tree

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

The above needs a bit of clarifying follow-up.

Some operations (e.g. "apply --index" and "checkout another-branch") want to make sure that the path in the working tree "matches" what is in the index before proceeding. The reason why they require a match is because they are going to update what is in the index and then update what is in the working tree to match the result by checking the updated index entry out to the working tree--if the working tree and the index are different before they start their operation, that means you have some changes you made in the working tree since you checked it out of the index, and their operation will lose such changes.

Normally, this verification is done by ce_match_stat() and friends, whose correct operation relies on an earlier refresh_index(), which in turn makes sure that the result of running the contents in the working tree through convert_to_git() matches what is in the index.

When your convert_to_working_tree() and convert_to_git() do not round-trip, however, the result of convert_to_git() on the working tree contents would not match what is in the index. That is inconvenient, and it is something you may want to relax to help such a broken situation. Immediately after you "git checkout" (or "git reset --hard"), you haven't made any changes, and you should be able to "git checkout another" to go to another branch.

For this reason, I am perfectly OK with an approach to teach the callers that currently use ce_uptodate() as the only way to make sure that there is no modification to a given path (and refuse to work on it if ce_uptodate() says the path is modified) that it is also OK to clobber a path that does not pass the ce_uptodate() check as long as the result of running convert_to_working_tree() on the indexed contents matches what is in the working tree. These callers are currently overly strict and you will be relaxing their overly strict check to help this broken situation.

Perhaps we can introduce a new function can_clobber() that has the same function signature as ce_uptodate() and update the callers in apply and unpack-trees (there may be others) to call it instead when they want to see if they can clobber the working tree file that corresponds to the cache entry.

For implementing the can_clobber() function, you can use something along the lines of compare_with_fd() helper function I introduced in [1] and do something like this, perhaps.

When I send an illustration patch and say "totally untested", I usually start from the real source file and send "git diff" output after making changes to the source file, and I may even have at least compiled the modified result. The following however is typed directly into my mail program without touching any existing source file, so it is truly untested--caveat emptor.

/*
 * We are about to do some operation to the index entry, and
 * write the result out to the working tree.  Would we lose
 * some local change that exist only in the working tree by
 * doing so?  Return 1 if we can safely clobber the working
 * tree file (i.e. no changes) and return 0 if we can't (i.e.
 * there are some changes).
 */
int can_clobber(struct cache_entry *ce)
{
        int fd, match = 0;
        enum object_type type;
	unsigned long size;
        void *data;

        /*
         * Does another "git add -f" of the path result in the
         * identical blob in the index?  If so, the working tree
         * file is expendable.
         */
	if (ce_uptodate(ce))
        	return 1;
	fd = open(ce->name, O_RDONLY);
        if (fd < 0)
		return 0;

	data = read_sha1_file(ce->sha1, &type, &size);
	if (type == OBJ_BLOB) {
		struct strbuf worktree = STRBUF_INIT;
		/*
                 * Does another "git checkout -- path"
		 * recreate what we see in the working tree?
                 * If so, the working tree file is expendable.
                 */
		if (convert_to_working_tree(ce->name, data, size,
					    &worktree)) {
			free(data);
			data = strbuf_detach(&worktree, &size);
		}
		if (!compare_with_fd(data, size, fd))
			match = 1;
	}
	free(data);
	close(fd);

        return match;
}

[Reference]

*1* http://thread.gmane.org/gmane.comp.version-control.git/284352/focus=285341