Re: [PATCH] add_to_index(): convert forgotten HASH_RENORMALIZE check

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 2804 bytes --]

Hi Peff,

On Wed, 6 Feb 2019, Jeff King wrote:

> On Wed, Feb 06, 2019 at 11:42:43AM +0100, SZEDER Gábor wrote:
> 
> > I reported this and Peff looked into it on the way to Git Merge, but
> > not working solution yet.
> > 
> > https://public-inbox.org/git/20190129225121.GD1895@sigill.intra.peff.net/T/#u
> 
> Oof. Well, now I know why my attempts to fix the test failed. It was not
> my new test that was failing at all, but rather the existing test. Which
> implies that I severely bungled the actual code change.
> 
> Armed with that knowledge, it was pretty easy to find said bungling. The
> fix is below.
> 
> Junio, this should go on top of jk/add-ignore-errors-bit-assignment-fix
> as soon as possible, as the regression is already in master. And I'll go
> find a brown paper bag. ;)

Thank you *so* much for the quick fix!
Dscho

> -- >8 --
> Subject: [PATCH] add_to_index(): convert forgotten HASH_RENORMALIZE check
> 
> Commit 9e5da3d055 (add: use separate ADD_CACHE_RENORMALIZE flag,
> 2019-01-17) switched out using HASH_RENORMALIZE in our flags field for a
> new ADD_CACHE_RENORMALIZE flag. However, it forgot to convert one of the
> checks for HASH_RENORMALIZE into the new flag, which totally broke "git
> add --renormalize".
> 
> To make matters even more confusing, the resulting code would racily
> pass the tests!  The forgotten check was responsible for defeating the
> up-to-date check of the index entry. That meant that "git add
> --renormalize" would refuse to renormalize things that appeared
> stat-clean. But most of the time the test commands run fast enough that
> the file mtime is the same as the index mtime. And thus we err on the
> conservative side and re-hash the file, which is what "--renormalize"
> would have wanted.
> 
> But if you're unlucky and cross that one-second boundary between writing
> the file and writing the index (which is more likely to happen on a slow
> or heavily-loaded system), then the file appears stat-clean. And
> "--renormalize" would effectively do nothing.
> 
> The fix is straightforward: convert this check to use the right flag.
> 
> Noticed-by: SZEDER Gábor <szeder.dev@gmail.com>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  read-cache.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/read-cache.c b/read-cache.c
> index 9783c493a3..accc059951 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -746,7 +746,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
>  	if (ignore_case) {
>  		adjust_dirname_case(istate, ce->name);
>  	}
> -	if (!(flags & HASH_RENORMALIZE)) {
> +	if (!(flags & ADD_CACHE_RENORMALIZE)) {
>  		alias = index_file_exists(istate, ce->name,
>  					  ce_namelen(ce), ignore_case);
>  		if (alias &&
> -- 
> 2.20.1.1122.g2972e48916
> 
>