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

Re: [PATCH v2] cache-tree: do not try to use an invalidated subtree info to build a tree

From
David Turner <dturner@twopensource.com>
Date
Sep 3, 2014, 02:56 UTC
Message-ID
<1409712991.3057.10.camel@leckie>
In-Reply-To
<xmqqvbp5y7ef.fsf_-_@gitster.dls.corp.google.com>
On Tue, 2014-09-02 at 15:39 -0700, Junio C Hamano wrote:
Show 85 quoted lines
> We punt from repairing the cache-tree during a branch switching if
> it involves having to create a new tree object that does not yet
> exist in the object store.  "mkdir dir && >dir/file && git add dir"
> followed by "git checkout" is one example, when a tree that records
> the state of such "dir/" is not in the object store.
> 
> However, after discovering that we do not have a tree object that
> records the state of "dir/", the caller failed to remember the fact
> that it noticed the cache-tree entry it received for "dir/" is
> invalidated, it already knows it should not be populating the level
> callchain to stop the code to attempt populating the level that has
> "dir/" as its immediate subdirectory, and it is not an error at all
> for the sublevel cache-tree entry gave it a bogus object name it
> shouldn't even look at.
> 
> This led the caller detect and report a non-existent error.  The end
> result was the same and we avoided stuffing a non-existent tree to
> the cache-tree, but we shouldn't have issued an alarming error
> message to the user.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> 
>  * Second try.  The level that has intent-to-add entries needs to be
>    kept invalidated but the level above it needs to treat as if the
>    i-t-a entries do not exist and build the whole tree; a directory
>    that does not yet have corresponding tree object while repairing
>    the cache-tree needs to invalidate itself *and* propagate the
>    (in)validity upwards.  They have to be treated differently but
>    the first attempt failed to do so.
> 
>  cache-tree.c          | 7 ++++++-
>  t/t0090-cache-tree.sh | 8 ++++++++
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/cache-tree.c b/cache-tree.c
> index f951d7d..57597ac 100644
> --- a/cache-tree.c
> +++ b/cache-tree.c
> @@ -316,6 +316,7 @@ static int update_one(struct cache_tree *it,
>  		int pathlen, entlen;
>  		const unsigned char *sha1;
>  		unsigned mode;
> +		int expected_missing = 0;
>  
>  		path = ce->name;
>  		pathlen = ce_namelen(ce);
> @@ -332,8 +333,10 @@ static int update_one(struct cache_tree *it,
>  			i += sub->count;
>  			sha1 = sub->cache_tree->sha1;
>  			mode = S_IFDIR;
> -			if (sub->cache_tree->entry_count < 0)
> +			if (sub->cache_tree->entry_count < 0) {
>  				to_invalidate = 1;
> +				expected_missing = 1;
> +			}
>  		}
>  		else {
>  			sha1 = ce->sha1;
> @@ -343,6 +346,8 @@ static int update_one(struct cache_tree *it,
>  		}
>  		if (mode != S_IFGITLINK && !missing_ok && !has_sha1_file(sha1)) {
>  			strbuf_release(&buffer);
> +			if (expected_missing)
> +				return -1;
>  			return error("invalid object %06o %s for '%.*s'",
>  				mode, sha1_to_hex(sha1), entlen+baselen, path);
>  		}
> diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
> index 48c4240..f9648a8 100755
> --- a/t/t0090-cache-tree.sh
> +++ b/t/t0090-cache-tree.sh
> @@ -210,4 +210,12 @@ test_expect_success 'partial commit gives cache-tree' '
>  	test_cache_tree
>  '
>  
> +test_expect_success 'no phantom error when switching trees' '
> +	mkdir newdir &&
> +	>newdir/one &&
> +	git add newdir/one &&
> +	git checkout 2>errors &&
> +	! test -s errors
> +'
> +
>  test_done
LGTM.
Previous: Junio C HamanoNext: Eric Sunshine
Message 27 of 28 in “cache-tree: Create/update cache-tree on checkout”
  1. 1/4 cache-tree: Create/update cache-tree on checkoutDavid Turner, Jul 12, 2014
  2. 2/4 test-dump-cache-tree: invalid trees are not errorsDavid Turner, Jul 12, 2014
  3. 3/4 cache-tree: subdirectory testsDavid Turner, Jul 12, 2014
  4. 4/4 cache-tree: Write updated cache-tree after commitDavid Turner, Jul 12, 2014
  5. Duy NguyenJul 13, 2014
  6. Junio C HamanoJul 14, 2014
  7. Ramsay JonesJul 14, 2014
  8. Junio C HamanoJul 14, 2014
  9. Ramsay JonesJul 14, 2014
  10. Junio C HamanoJul 14, 2014
  11. David TurnerJul 14, 2014
  12. Duy NguyenJul 15, 2014
  13. Junio C HamanoJul 15, 2014
  14. Duy NguyenJul 15, 2014
  15. Junio C HamanoJul 15, 2014
  16. Duy NguyenJul 16, 2014
  17. Junio C HamanoJul 16, 2014
  18. Junio C HamanoJul 14, 2014
  19. lockfile: allow reopening a closed but still locked fileJunio C Hamano, Jul 14, 2014
  20. John KeepingAug 31, 2014
  21. David TurnerSep 1, 2014
  22. John KeepingSep 1, 2014
  23. Junio C HamanoSep 2, 2014
  24. Junio C HamanoSep 2, 2014
  25. cache-tree: propagate invalidation up when puntingJunio C Hamano, Sep 2, 2014
  26. cache-tree: do not try to use an invalidated subtree info to build a treeJunio C Hamano, Sep 2, 2014
  27. David TurnerSep 3, 2014
  28. Eric SunshineSep 3, 2014

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.