On Fri, Aug 1, 2025 at 9:11 AM Lucas Seiki Oshiro
<lucasseikioshiro@xxxxxxxxx> wrote:
> Add the --format flag to git-repo-info. By using this flag, the users
> can choose the format for obtaining the data they requested.
>
> Given that this command can be used for generating input for other
> applications and for being read by end users, it requires at least two
> formats: one for being read by humans and other for being read by
> machines. Some other Git commands also have two output formats, notably
> git-config which was the inspiration for the two formats that were
> chosen here:
>
> - keyvalue, where the retrieved data is printed one per line, using =
> for delimiting the key and the value. This is the default format,
> targeted for end users.
> - nul, where the retrieved data is separated by null characters, using
> the newline character for delimiting the key and the value. This
> format is targeted for being read by machines.
>
> Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@xxxxxxxxx>
> ---
> diff --git a/t/t1900-repo.sh b/t/t1900-repo.sh
> @@ -21,10 +21,17 @@ test_repo_info () {
> - test_expect_success "$label" '
> - eval "$init_command $repo_name" &&
> + test_expect_success "keyvalue: $label" '
> + eval "$init_command keyvalue-$repo_name" &&
> echo "$key=$expected_value" >expected &&
> - git -C $repo_name repo info "$key" >actual &&
> + git -C keyvalue-$repo_name repo info "$key" >actual &&
> + test_cmp expected actual
> + '
> +
> + test_expect_success "nul: $label" '
> + eval "$init_command nul-$repo_name" &&
> + printf "%s\n%s\0" "$key" "$expected_value" >expected &&
> + git -C nul-$repo_name repo info --format=nul "$key" >actual &&
> test_cmp expected actual
> '
Due to the embedded NUL's in this new "nul" test, I'm pretty sure you
want to be using `test_cmp_bin` here as suggested previously[*].
[*]: https://lore.kernel.org/git/CAPig+cQn7c5+k06yHOD2jxYTGnny7is=fbo4tOw26eD+4zX-Jw@xxxxxxxxxxxxxx/
> @@ -45,6 +52,7 @@ test_repo_info 'shallow repository = false is retrieved correctly' '
> test_repo_info 'shallow repository = true is retrieved correctly' '
> + test_when_finished "rm -rf remote" &&
> git init remote &&
> echo x >remote/x &&
> git -C remote add x &&
For what it's worth, it would be clearer to turn the removal of
"remote" into a "make sure we have a clean-slate for what we are about
to do" rather than making it an after-the-fact cleanup. That is:
test_repo_info 'shallow repository = true is retrieved correctly' '
rm -rf remote &&
git init remote &&
...
'
Alternatively, since the "remote" repository is static in the sense
that it is the same for both the "keyvalue" and "nul" cases, it would
be even clearer to just separate it out into its own "setup"-style
test:
test_expect_success 'setup remote' '
git init remote &&
echo x >remote/x &&
...
'
test_repo_info 'shallow repository = true is retrieved correctly' \
'git clone --depth 1 "file://$PWD/remote"' 'shallow'
'layout.shallow' 'true'
> @@ -79,4 +87,11 @@ test_expect_success 'output is returned correctly when two keys are requested' '
> +test_expect_success 'git-repo-info aborts when requesting an invalid format' '
> + test_when_finished "rm -f err expected" &&
> + echo "fatal: invalid format '\'foo\''" >expected &&
> + test_must_fail git repo info --format=foo 2>err &&
> + test_cmp expected err
> +'
Do we need to perform this `test_when_finished` cleanup? As mentioned
in earlier reviews, we don't usually perform cleanup unnecessarily
since doing so slows down the test suite and makes it more difficult
to debug a failing test.
Also, didn't patch [2/5] already add this exact test?
