From: Jeff King <peff@peff.net>
Date: Thu, 15 Jan 2026 20:09:03 GMT
Subject: Re: [PATCH v3] sparse-checkout: optimize string_list construction
Message-ID: <20260115200903.GB1053259@coredump.intra.peff.net>
In-Reply-To: <CAPvEtreX9sGHUn7+Y0kLo_VnK7Y=OYLq-kz-+np3bu1QtoEpnA@mail.gmail.com>

On Thu, Jan 15, 2026 at 06:45:35PM +0530, Amisha Chhajed wrote:

> I was also very curious about the presence of
> string_list_remove_duplicates in the original code, from my
> understanding string_list_insert already removed duplicates and
> string_list_remove_duplicates was still present with it.

Yes, I don't think you could have duplicates when inserting with
string_list_insert(). Of course your patch removes that, which means
we're falling back on the notion that the hashmap cannot have
duplicates, either.

I think our hashmap _does_ allow duplicate entries, though. The
insertion code in insert_recursive_pattern() avoids duplicates in
parent_hashmap, but adds its arguments directly to recursive_hashmap.

So I think you could get duplicates with something like:

  git init
  git sparse-checkout set --cone
  git sparse-checkout add --stdin <<\EOF
  foo
  bar
  foo
  EOF

Before your patch, that produces this .git/info/sparse-checkout file:

  /*
  !/*/
  /bar/
  /foo/

and after we get:

  /*
  !/*/
  /bar/
  /foo/
  /foo/

So I think we do want to retain the duplicate suppression. Switching
from insert() to append() is still good, as long as we keep the
remove_duplicates() lines.

-Peff