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

Re: [PATCH 3/4] object-name: reject trees found in the index

From
Josh Steadmon <steadmon@google.com>
Date
Apr 14, 2022, 18:57 UTC
Message-ID
<Ylhui1cBGDca4m19@google.com>
In-Reply-To
<f5da532767367844af7ecc477e1c8434a5454fd1.1649349442.git.gitgitgadget@gmail.com>
On 2022.04.07 16:37, Derrick Stolee via GitGitGadget wrote:
Show 9 quoted lines
> From: Derrick Stolee <derrickstolee@github.com>
> 
> The get_oid_with_context_1() method is used when parsing revision
> arguments. One particular case is to take a ":<path>" string and search
> the index for the given path.
> 
> In the case of a sparse index, this might find a sparse directory entry,
> in which case the contained object is a tree. In the case of a full
> index, this search within the index would fail.

Another case where my naive understanding of sparse-indexes caused a lot of confusion for me. I don't know if we want to add reminders everywhere in this series, but there should probably be at least one commit message in the series that points out that sparse indexes are not subsets of full indexes.

Show 75 quoted lines
> In order to maintain identical return state as in a full index, inspect
> the discovered cache entry to see if it is a sparse directory and reject
> it. This requires being careful around the only_to_die option to be sure
> we die only at the correct time.
> 
> This changes the behavior of 'git show :<sparse-dir>', but does not
> bring it entirely into alignment with a full index case. It specifically
> hits the wrong error message within diagnose_invalid_index_path(). That
> error message will be corrected in a future change.
> 
> Signed-off-by: Derrick Stolee <derrickstolee@github.com>
> ---
>  object-name.c                            | 19 ++++++++++++++++++-
>  t/t1092-sparse-checkout-compatibility.sh | 11 ++---------
>  2 files changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/object-name.c b/object-name.c
> index f0e327f91f5..2dc5d2549b8 100644
> --- a/object-name.c
> +++ b/object-name.c
> @@ -1881,6 +1881,20 @@ static char *resolve_relative_path(struct repository *r, const char *rel)
>  			   rel);
>  }
>  
> +static int reject_tree_in_index(struct repository *repo,
> +				int only_to_die,
> +				const struct cache_entry *ce,
> +				int stage,
> +				const char *prefix,
> +				const char *cp)
> +{
> +	if (!S_ISSPARSEDIR(ce->ce_mode))
> +		return 0;
> +	if (only_to_die)
> +		diagnose_invalid_index_path(repo, stage, prefix, cp);
> +	return -1;
> +}
> +
>  static enum get_oid_result get_oid_with_context_1(struct repository *repo,
>  				  const char *name,
>  				  unsigned flags,
> @@ -1955,9 +1969,12 @@ static enum get_oid_result get_oid_with_context_1(struct repository *repo,
>  			    memcmp(ce->name, cp, namelen))
>  				break;
>  			if (ce_stage(ce) == stage) {
> +				free(new_path);
> +				if (reject_tree_in_index(repo, only_to_die, ce,
> +							 stage, prefix, cp))
> +					return -1;
>  				oidcpy(oid, &ce->oid);
>  				oc->mode = ce->ce_mode;
> -				free(new_path);
>  				return 0;
>  			}
>  			pos++;
> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
> index f6a14e08b81..9d32361110d 100755
> --- a/t/t1092-sparse-checkout-compatibility.sh
> +++ b/t/t1092-sparse-checkout-compatibility.sh
> @@ -1164,15 +1164,8 @@ test_expect_success 'show (cached blobs/trees)' '
>  	test_must_fail git -C full-checkout show :folder1/ &&
>  	test_must_fail git -C sparse-checkout show :folder1/ &&
>  
> -	git -C sparse-index show :folder1/ >actual &&
> -	git -C full-checkout show HEAD:folder1 >expect &&
> -
> -	# The output of "git show" includes the way we referenced the
> -	# objects, so strip that out.
> -	test_line_count = 4 actual &&
> -	tail -n 2 actual >actual-trunc &&
> -	tail -n 2 expect >expect-trunc &&
> -	test_cmp expect-trunc actual-trunc
> +	test_must_fail git -C sparse-index show :folder1/ 2>err &&
> +	grep "is in the index, but not at stage 0" err
>  '

It might be worth a note that we're demonstrating the current behavior here, but this is not the desired end-state. In other words, explicitly note that this is the "wrong error message" referred to in the commit message.

Show 5 quoted lines
>  
>  test_expect_success 'submodule handling' '
> -- 
> gitgitgadget
> 
Previous: Derrick Stolee via GitGitGadgetNext: Derrick Stolee
Message 9 of 26 in “Sparse index integration with 'git show'”
  1. 0/4 Sparse index integration with 'git show'Derrick Stolee via GitGitGadget, Apr 7, 2022
  2. 1/4 t1092: add compatibility tests for 'git show'Derrick Stolee via GitGitGadget, Apr 7, 2022
  3. Josh SteadmonApr 14, 2022
  4. Derrick StoleeApr 18, 2022
  5. 2/4 show: integrate with the sparse indexDerrick Stolee via GitGitGadget, Apr 7, 2022
  6. Josh SteadmonApr 14, 2022
  7. Derrick StoleeApr 18, 2022
  8. 3/4 object-name: reject trees found in the indexDerrick Stolee via GitGitGadget, Apr 7, 2022
  9. Josh SteadmonApr 14, 2022
  10. Derrick StoleeApr 18, 2022
  11. 4/4 object-name: diagnose trees in index properlyDerrick Stolee via GitGitGadget, Apr 7, 2022
  12. Philip OakleyApr 7, 2022
  13. Junio C HamanoApr 12, 2022
  14. Derrick StoleeApr 12, 2022
  15. Junio C HamanoApr 12, 2022
  16. Josh SteadmonApr 14, 2022
  17. Junio C HamanoApr 14, 2022
  18. Derrick StoleeApr 18, 2022
  19. 0/5 Sparse index integration with 'git show'Derrick Stolee via GitGitGadget, Apr 26, 2022
  20. 1/5 t1092: add compatibility tests for 'git show'Derrick Stolee via GitGitGadget, Apr 26, 2022
  21. 2/5 show: integrate with the sparse indexDerrick Stolee via GitGitGadget, Apr 26, 2022
  22. 3/5 object-name: reject trees found in the indexDerrick Stolee via GitGitGadget, Apr 26, 2022
  23. 4/5 object-name: diagnose trees in index properlyDerrick Stolee via GitGitGadget, Apr 26, 2022
  24. 5/5 rev-parse: integrate with sparse indexDerrick Stolee via GitGitGadget, Apr 26, 2022
  25. Junio C HamanoApr 26, 2022
  26. Derrick StoleeApr 27, 2022

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.