Re: [PATCH v2 1/3] diff-files --raw: handle intent-to-add files correctly

From: Srinidhi Kaushik <shrinidhi.kaushik@gmail.com>
Date: Jun 24, 2020, 07:11 UTC
Message-ID: <20200624071116.GA79097@mail.clickyotomy.dev>
In-Reply-To: <640e225550886727594ca9dfaee6c9e6ea4b1014.1592916485.git.gitgitgadget@gmail.com>

Hi Johannes,

> The underlying problem is that some time ago, the (already incorrect)
> empty blob constant was replaced by the empty tree constant, by mistake. I
> contributed a patch series to fix that, and Cc:ed you you in v2 that I
> sent out earlier today.

Thanks for CC-ing me!

[...]

Show 14 quoted lines

> +			} else if (ce_intent_to_add(ce) &&
> +				   !(revs->diffopt.output_format &
> +				     ~(DIFF_FORMAT_RAW | DIFF_FORMAT_NAME_STATUS))) {
> +				struct object_id oid;
> +				int ret = lstat(ce->name, &st);
> +
> +				if (ret < 0)
> +					oidclr(&oid);
> +				else
> +					ret = index_path(istate, &oid,
> +						 ce->name, &st, 0);
> +				diff_addremove(&revs->diffopt, '+', ce->ce_mode,
> +					       &oid, ret >= 0, ce->name, 0);
> +				continue;

Instead of showing the hash for empty blobs for all entries previously, introducing this shows the hash of non-empty "i-t-a" files correctly. Nice.

[...]

Show 8 quoted lines

>  			} else if (revs->diffopt.ita_invisible_in_index &&
>  				   ce_intent_to_add(ce)) {
>  				diff_addremove(&revs->diffopt, '+', ce->ce_mode,
> -					       the_hash_algo->empty_tree, 0,
> +					       the_hash_algo->empty_blob, 0,
>  					       ce->name, 0);
>  				continue;
>  			}

Oh, I totally missed this in my patch; the change looks good!

> -  :000000 100644 0000000 $(git rev-parse --short $hash_t) A$(printf "\t")empty
> -  :000000 100644 0000000 $(git rev-parse --short $hash_t) A$(printf "\t")not-empty
> +  :000000 100644 0000000 $(git rev-parse --short $hash_e) A$(printf "\t")empty
> +  :000000 100644 0000000 $(git rev-parse --short $hash_n) A$(printf "\t")not-empty

Changing the test-case to reflect to the hash of the blob also makes sense.

[...]

Show 5 quoted lines

> > +     hash_e=$(git hash-object empty) &&
> > +     hash_n=$(git hash-object not-empty) &&
> > +     hash_t=$(git hash-object -t tree /dev/null) &&
>
> > So this is the hash of the empty tree object, and...

I guess we can get rid of the `hash_t' assignment here, because it won't be used anywhere else in the test.

Thanks.