Re: [PATCH v2] unpack-trees: do not abort when overwriting an existing file with the same content

[Jeff King <peff@peff.net>]

On Mon, Jan 21, 2013 at 06:40:33PM +0700, Nguyen Thai Ngoc Duy wrote:

> +	/*
> +	 * If it has the same content that we are going to overwrite,
> +	 * there's no point in complaining. We still overwrite it in
> +	 * the end though.
> +	 */
> +	if (ce &&
> +	    S_ISREG(st->st_mode) && S_ISREG(ce->ce_mode) &&
> +	    (!trust_executable_bit ||
> +	     (0100 & (ce->ce_mode ^ st->st_mode)) == 0) &&
> +	    st->st_size < SAME_CONTENT_SIZE_LIMIT &&
> +	    sha1_object_info(ce->sha1, &ce_size) == OBJ_BLOB &&
> +	    ce_size == st->st_size) {
> +		void *buffer = NULL;
> +		unsigned long size;
> +		enum object_type type;
> +		struct strbuf sb = STRBUF_INIT;
> +		int matched =
> +			strbuf_read_file(&sb, ce->name, ce_size) == ce_size &&
> +			(buffer = read_sha1_file(ce->sha1, &type, &size)) != NULL &&
> +			type == OBJ_BLOB &&
> +			size == ce_size &&
> +			!memcmp(buffer, sb.buf, size);
> +		free(buffer);
> +		strbuf_release(&sb);
> +		if (matched)
> +			return 0;
> +	}

Can you elaborate on when this code is triggered?

In the general case, shouldn't we already know the sha1 of what's on
disk in the index, and be able to just compare the hashes? And if we
don't, because the entry is start-dirty, should we be updating it
(possibly earlier, so we do not even get into the "need to write" code
path) instead of doing this ad-hoc byte comparison?

Confused...

-Peff