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

[Junio C Hamano <gitster@pobox.com>]

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

> Besides, it is OK if your status and diff says your worktree is
> dirty immediately after cloning in such a broken situation, I would
> think.  In fact, it may even be preferable to do so, in order to
> indicate that there is something unusual going on.

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