Re: [PATCH 7/8] t: support GIT_TEST_WRITE_REV_INDEX
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Jan 12, 2021, 17:18 UTC
- Message-ID
- <875z425b17.fsf@evledraar.gmail.com>
- In-Reply-To
- <3525c4d114c8655953aca44d8effa1652ccc93d3.1610129989.git.me@ttaylorr.com>
On Fri, Jan 08 2021, Taylor Blau wrote:
Show 21 quoted lines
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---
> builtin/index-pack.c | 5 ++++-
> builtin/pack-objects.c | 2 ++
> pack-revindex.h | 2 ++
> t/README | 3 +++
> 4 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> index 03408250b1..0bde325a8b 100644
> --- a/builtin/index-pack.c
> +++ b/builtin/index-pack.c
> @@ -1748,7 +1748,10 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
> if (prefix && chdir(prefix))
> die(_("Cannot come back to cwd"));
>
> - rev_index = !!(opts.flags & (WRITE_REV_VERIFY | WRITE_REV));
> + if (git_env_bool(GIT_TEST_WRITE_REV_INDEX, 0))
> + rev_index = 1;
> + else
> + rev_index = !!(opts.flags & (WRITE_REV_VERIFY | WRITE_REV));Why not make an explicit GIT_TEST_WRITE_REV_INDEX=false meaningful? It's also sometimes handy to turn these off in the tests.
rev_index = git_env_bool("GIT_TEST_WRITE_REV_INDEX",
!!(opts.flags & (WRITE_REV_VERIFY | WRITE_REV)));> +#define GIT_TEST_WRITE_REV_INDEX "GIT_TEST_WRITE_REV_INDEX"
Micro style nit: FWIW I'm not a fan of this macro->string indirection a few GIT_TEST_* names have had since 859fdc0c3c (commit-graph: define GIT_TEST_COMMIT_GRAPH, 2018-08-29).
Most of them just use git_env_bool("GIT_TEST_[...]") which IMO makes it easier to eyeball a "git grep"
> +GIT_TEST_WRITE_REV_INDEX=<boolean>, when true enables the > +'pack.writeReverseIndex' setting. > +
Re the git_env_bool() default value comment above: I see our other boolean GIT_TEST_* docs say "when true", but mostly they mean things like:
GIT_TEST_WRITE_REV_INDEX=<boolean>, when set, configures the
'pack.writeReverseIndex' setting. Defaults to 'false'.