Re: [PATCH 10/16] mktree: overwrite duplicate entries

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2366 bytes --]

On Tue, Jun 11, 2024 at 06:24:42PM +0000, Victoria Dye via GitGitGadget wrote:
> From: Victoria Dye <vdye@github.com>
> 
> If multiple tree entries with the same name are provided as input to
> 'mktree', only write the last one to the tree. Entries are considered
> duplicates if they have identical names (*not* considering mode); if a blob
> and a tree with the same name are provided, only the last one will be
> written to the tree. A tree with duplicate entries is invalid (per 'git
> fsck'), so that condition should be avoided wherever possible.
> 
> Signed-off-by: Victoria Dye <vdye@github.com>
> ---
>  Documentation/git-mktree.txt |  8 ++++---
>  builtin/mktree.c             | 45 ++++++++++++++++++++++++++++++++----
>  t/t1010-mktree.sh            | 36 +++++++++++++++++++++++++++--
>  3 files changed, 80 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/git-mktree.txt b/Documentation/git-mktree.txt
> index fb07e40cef0..afbc846d077 100644
> --- a/Documentation/git-mktree.txt
> +++ b/Documentation/git-mktree.txt
> @@ -43,9 +43,11 @@ OPTIONS
>  INPUT FORMAT
>  ------------
>  Tree entries may be specified in any of the formats compatible with the
> -`--index-info` option to linkgit:git-update-index[1]. The order of the tree
> -entries is normalized by `mktree` so pre-sorting the input by path is not
> -required.
> +`--index-info` option to linkgit:git-update-index[1].
> +
> +The order of the tree entries is normalized by `mktree` so pre-sorting the input
> +by path is not required. Multiple entries provided with the same path are
> +deduplicated, with only the last one specified added to the tree.

Hm. I'm not sure whether this is a good idea. With git-mktree(1) being
part of our plumbing layer, you can expect that it's mostly going to be
fed input from scripts. And any script that generates duplicate tree
entries is broken, but we now start to paper over such brokenness
without giving the user any indicator of this. As user of git-mktree(1)
in Gitaly I can certainly say that I'd rather want to see it die instead
of silently fixing my inputs so that I start to notice my own bugs.

So without seeing a strong motivating usecase for this feature I'd think
that git-mktree(1) should reject such inputs and return an error such
that the user can fix their tooling.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]