Re: [PATCH 08/16] mktree: add a --literally option

[Junio C Hamano <gitster@pobox.com>]

"Victoria Dye via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Victoria Dye <vdye@github.com>
>
> Add the '--literally' option to 'git mktree' to allow constructing a tree
> with invalid contents. For now, the only change this represents compared to
> the normal 'git mktree' behavior is no longer sorting the inputs; in later
> commits, deduplicaton and path validation will be added to the command and
> '--literally' will skip those as well.

Hmph, the end state of the the series as a whole may be good, but
the above makes me wonder if we broke bisectability with the
previous step 07/16 where we introduced type checks without touching
any existing tests?

> Certain tests use 'git mktree' to intentionally generate corrupt trees.
> Update these tests to use '--literally' so that they continue functioning
> properly when additional input cleanup & validation is added to the base
> command. Note that, because 'mktree --literally' does not sort entries, some
> of the tests are updated to provide their inputs in tree order; otherwise,
> the test would fail with an "incorrect order" error instead of the error the
> test expects.

Makes sense.

> diff --git a/Documentation/git-mktree.txt b/Documentation/git-mktree.txt
> index 507682ed23e..fb07e40cef0 100644
> --- a/Documentation/git-mktree.txt
> +++ b/Documentation/git-mktree.txt
> @@ -9,7 +9,7 @@ git-mktree - Build a tree-object from formatted tree entries
>  SYNOPSIS
>  --------
>  [verse]
> -'git mktree' [-z] [--missing] [--batch]
> +'git mktree' [-z] [--missing] [--literally] [--batch]
>  
>  DESCRIPTION
>  -----------
> @@ -27,6 +27,13 @@ OPTIONS
>  	object.  This option has no effect on the treatment of gitlink entries
>  	(aka "submodules") which are always allowed to be missing.
>  
> +--literally::
> +	Create the tree from the tree entries provided to stdin in the order
> +	they are provided without performing additional sorting, deduplication,
> +	or path validation on them. This option is primarily useful for creating
> +	invalid tree objects to use in tests of how Git deals with various forms
> +	of tree corruption.
> +

OK.

> diff --git a/builtin/mktree.c b/builtin/mktree.c
> index 5530257252d..48019448c1f 100644
> --- a/builtin/mktree.c
> +++ b/builtin/mktree.c
> @@ -45,11 +45,11 @@ static void release_tree_entry_array(struct tree_entry_array *arr)
>  }
>  
>  static void append_to_tree(unsigned mode, struct object_id *oid, const char *path,
> -			   struct tree_entry_array *arr)
> +			   struct tree_entry_array *arr, int literally)
>  {
>  	struct tree_entry *ent;
>  	size_t len = strlen(path);
> -	if (strchr(path, '/'))
> +	if (!literally && strchr(path, '/'))
>  		die("path %s contains slash", path);

;-).

A tree_entry with a slash in it.  Our fsck should be catching them
already, but this will allow constructing a test case more easily.