git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: git add —ignore-errors causes --renormalize

From
SZEDER Gábor <szeder.dev@gmail.com>
Date
Jan 29, 2019, 21:35 UTC
Message-ID
<20190129213533.GE13764@szeder.dev>
In-Reply-To
<20190117162711.GA7935@sigill.intra.peff.net>
On Thu, Jan 17, 2019 at 11:27:11AM -0500, Jeff King wrote:
Show 42 quoted lines
> On Thu, Jan 17, 2019 at 06:22:05PM +0300, Dmitriy Smirnov wrote:
> 
> > Calling `git add —ignore-errors` appears to be equal to calling `git add —renormalize`:
> > 
> > Main.java is saved with CRLF in repo
> > git config core.autocrlf = input
> > 
> > $ src  git:(master) echo line >> Main.java  
> > $ src  git:(master) git add --ignore-errors Main.java  
> > $ src  git:(master) git commit -m "Ignore errors"                          
> > [master cf24b3b] Ignore errors
> >  1 file changed, 13 insertions(+), 12 deletions(-)
> > 
> > 
> > The reason appears to be wrong bit mask usage
> > 
> > #define ADD_CACHE_IGNORE_ERRORS    4
> > and
> > #define HASH_RENORMALIZE  4
> > 
> > Looks like a regression since 2.16.0 - 9472935d81eaf9faed771878c9df0216ae0d9045
> 
> Thanks for a very clear report! The patch below should fix it.
> 
> -- >8 --
> Subject: [PATCH] add: use separate ADD_CACHE_RENORMALIZE flag
> 
> Commit 9472935d81 (add: introduce "--renormalize", 2017-11-16) taught
> git-add to pass HASH_RENORMALIZE to add_to_index(), which then passes
> the flag along to index_path(). However, the flags taken by
> add_to_index() and the ones taken by index_path() are distinct
> namespaces. We cannot take HASH_* flags in add_to_index(), because they
> overlap with the ADD_CACHE_* flags we already take (in this case,
> HASH_RENORMALIZE conflicts with ADD_CACHE_IGNORE_ERRORS).
> 
> We can solve this by adding a new ADD_CACHE_RENORMALIZE flag, and using
> it to set HASH_RENORMALIZE within add_to_index(). In order to make it
> clear that these two flags come from distinct sets, let's also change
> the name "newflags" in the function to "hash_flags".
> 
> Reported-by: Dmitriy Smirnov <dmitriy.smirnov@jetbrains.com>
> Signed-off-by: Jeff King <peff@peff.net>

t0025 with '--stress' usually fails within seconds when run on the merge of 'jk/add-ignore-errors-bit-assignment-fix' and 'master' [1]:

  + echo *.txt text=auto
  + git add --renormalize *.txt
  + cat
  + sed -e s/     / /g -e s/  */ /g
  + sort
  + git ls-files --eol
  + test_cmp expect actual
  + diff -u expect actual
  --- expect      2019-01-29 21:27:34.043344898 +0000
  +++ actual      2019-01-29 21:27:34.055345252 +0000
  @@ -1,3 +1,3 @@
  -i/lf w/crlf attr/text=auto CRLF.txt
  +i/crlf w/crlf attr/text=auto CRLF.txt
   i/lf w/lf attr/text=auto LF.txt
  -i/lf w/mixed attr/text=auto CRLF_mix_LF.txt
  +i/mixed w/mixed attr/text=auto CRLF_mix_LF.txt
  error: last command exited with $?=1
  not ok 2 - renormalize CRLF in repo
  #       
  #               echo "*.txt text=auto" >.gitattributes &&
  #               git add --renormalize "*.txt" &&
  #               cat >expect <<-\EOF &&
  #               i/lf w/crlf attr/text=auto CRLF.txt
  #               i/lf w/lf attr/text=auto LF.txt
  #               i/lf w/mixed attr/text=auto CRLF_mix_LF.txt
  #               EOF
  #               git ls-files --eol |
  #               sed -e "s/      / /g" -e "s/  */ /g" |
  #               sort >actual &&
  #               test_cmp expect actual
  #       
[1] 'jk/add-ignore-errors-bit-assignment-fix' doesn't contains
    '--stress' yet, hence the merge to 'master'.
Show 80 quoted lines
> ---
>  builtin/add.c               | 2 +-
>  cache.h                     | 1 +
>  read-cache.c                | 8 ++++----
>  t/t0025-crlf-renormalize.sh | 9 +++++++++
>  4 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/builtin/add.c b/builtin/add.c
> index 12247b48fd..f481ae548e 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -137,7 +137,7 @@ static int renormalize_tracked_files(const struct pathspec *pathspec, int flags)
>  			continue; /* do not touch non blobs */
>  		if (pathspec && !ce_path_match(&the_index, ce, pathspec, NULL))
>  			continue;
> -		retval |= add_file_to_cache(ce->name, flags | HASH_RENORMALIZE);
> +		retval |= add_file_to_cache(ce->name, flags | ADD_CACHE_RENORMALIZE);
>  	}
>  
>  	return retval;
> diff --git a/cache.h b/cache.h
> index 49713cc5a5..11b19505fc 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -745,6 +745,7 @@ extern int index_name_pos(const struct index_state *, const char *name, int name
>  #define ADD_CACHE_JUST_APPEND 8		/* Append only; tree.c::read_tree() */
>  #define ADD_CACHE_NEW_ONLY 16		/* Do not replace existing ones */
>  #define ADD_CACHE_KEEP_CACHE_TREE 32	/* Do not invalidate cache-tree */
> +#define ADD_CACHE_RENORMALIZE 64        /* Pass along HASH_RENORMALIZE */
>  extern int add_index_entry(struct index_state *, struct cache_entry *ce, int option);
>  extern void rename_index_entry_at(struct index_state *, int pos, const char *new_name);
>  
> diff --git a/read-cache.c b/read-cache.c
> index bfff271a3d..9783c493a3 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -703,10 +703,10 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
>  	int intent_only = flags & ADD_CACHE_INTENT;
>  	int add_option = (ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE|
>  			  (intent_only ? ADD_CACHE_NEW_ONLY : 0));
> -	int newflags = HASH_WRITE_OBJECT;
> +	int hash_flags = HASH_WRITE_OBJECT;
>  
> -	if (flags & HASH_RENORMALIZE)
> -		newflags |= HASH_RENORMALIZE;
> +	if (flags & ADD_CACHE_RENORMALIZE)
> +		hash_flags |= HASH_RENORMALIZE;
>  
>  	if (!S_ISREG(st_mode) && !S_ISLNK(st_mode) && !S_ISDIR(st_mode))
>  		return error(_("%s: can only add regular files, symbolic links or git-directories"), path);
> @@ -762,7 +762,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
>  		}
>  	}
>  	if (!intent_only) {
> -		if (index_path(istate, &ce->oid, path, st, newflags)) {
> +		if (index_path(istate, &ce->oid, path, st, hash_flags)) {
>  			discard_cache_entry(ce);
>  			return error(_("unable to index file '%s'"), path);
>  		}
> diff --git a/t/t0025-crlf-renormalize.sh b/t/t0025-crlf-renormalize.sh
> index 9d9e02a211..e13363ade5 100755
> --- a/t/t0025-crlf-renormalize.sh
> +++ b/t/t0025-crlf-renormalize.sh
> @@ -27,4 +27,13 @@ test_expect_success 'renormalize CRLF in repo' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'ignore-errors not mistaken for renormalize' '
> +	git reset --hard &&
> +	echo "*.txt text=auto" >.gitattributes &&
> +	git ls-files --eol >expect &&
> +	git add --ignore-errors "*.txt" &&
> +	git ls-files --eol >actual &&
> +	test_cmp expect actual
> +'
> +
>  test_done
> -- 
> 2.20.1.689.g635a1dda8a
>
Previous: Torsten BögershausenNext: Jeff King
Message 5 of 6 in “git add —ignore-errors causes --renormalize”
  1. Dmitriy SmirnovJan 17, 2019
  2. Jeff KingJan 17, 2019
  3. Jeff KingJan 17, 2019
  4. Torsten BögershausenJan 19, 2019
  5. SZEDER GáborJan 29, 2019
  6. Jeff KingJan 29, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.