Re: [PATCH 11/27] unpack-trees: allow sparse directories
- From
Elijah Newren <newren@gmail.com>
- Date
- Jan 27, 2021, 17:36 UTC
- Message-ID
- <CABPp-BGPJgA4guWHVm3AVS=hM0fTixUpRvJe5i9NnHT-3QJMfw@mail.gmail.com>
- In-Reply-To
- <09893b4a6bbe13e61395411bdae57ce041829042.1611596534.git.gitgitgadget@gmail.com>
On Mon, Jan 25, 2021 at 9:42 AM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote:
Show 11 quoted lines
> > From: Derrick Stolee <dstolee@microsoft.com> > > The index_pos_by_traverse_info() currently throws a BUG() when a > directory entry exists exactly in the index. We need to consider that it > is possible to have a directory in a sparse index as long as that entry > is itself marked with the skip-worktree bit. > > The negation of the 'pos' variable must be conditioned to only when it > starts as negative. This is identical behavior as before when the index > is full.
The first sentence of this paragraph was really hard for me to parse. Reading the code and then reading the sentence I could make sense of it, but I struggled to do so the other way around. Is there some way to reword this? Or is this just a reading comprehension issue on my part? (It might be...)
Show 30 quoted lines
> The starts_with() condition matches because our name.buf terminates with
> a directory separator, just like our sparse directory entries.
>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
> unpack-trees.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/unpack-trees.c b/unpack-trees.c
> index 4dd99219073..b324eec2a5d 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -746,9 +746,12 @@ static int index_pos_by_traverse_info(struct name_entry *names,
> strbuf_make_traverse_path(&name, info, names->path, names->pathlen);
> strbuf_addch(&name, '/');
> pos = index_name_pos(o->src_index, name.buf, name.len);
> - if (pos >= 0)
> - BUG("This is a directory and should not exist in index");
> - pos = -pos - 1;
> + if (pos >= 0) {
> + if (!o->src_index->sparse_index ||
> + !(o->src_index->cache[pos]->ce_flags & CE_SKIP_WORKTREE))
> + BUG("This is a directory and should not exist in index");
> + } else
> + pos = -pos - 1;
> if (pos >= o->src_index->cache_nr ||
> !starts_with(o->src_index->cache[pos]->name, name.buf) ||
> (pos > 0 && starts_with(o->src_index->cache[pos-1]->name, name.buf)))
> --
> gitgitgadgetThe patch looks pretty straightforward to me.