[v2,10/24] t/helper: initialize repository if necessary
diff mbox series

Message ID 20200222201749.937983-11-sandals@crustytoothpaste.net
State New
Headers show
Series
  • SHA-256 stage 4 implementation, part 1/3
Related show

Commit Message

brian m. carlson Feb. 22, 2020, 8:17 p.m. UTC
The repository helper is used in t5318 to read commit graphs whether
we're in a repository or not. However, without a repository, we have no
way to properly initialize the hash algorithm, meaning that the file is
misread.

Fix this by calling setup_git_directory_gently, which will read the
environment variable the testsuite sets to ensure that the correct hash
algorithm is set.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/helper/test-repository.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Junio C Hamano Feb. 24, 2020, 6:05 p.m. UTC | #1
"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> The repository helper is used in t5318 to read commit graphs whether
> we're in a repository or not. However, without a repository, we have no
> way to properly initialize the hash algorithm, meaning that the file is
> misread.
>
> Fix this by calling setup_git_directory_gently, which will read the
> environment variable the testsuite sets to ensure that the correct hash
> algorithm is set.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  t/helper/test-repository.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/t/helper/test-repository.c b/t/helper/test-repository.c
> index f7f8618445..ecc768e4cb 100644
> --- a/t/helper/test-repository.c
> +++ b/t/helper/test-repository.c
> @@ -75,6 +75,10 @@ static void test_get_commit_tree_in_graph(const char *gitdir,
>  
>  int cmd__repository(int argc, const char **argv)
>  {
> +	int nongit_ok = 0;
> +
> +	setup_git_directory_gently(&nongit_ok);

No need to initialize nongit_ok to any specific value before calling
setup_git_directory_gently() and I personally find this initialization
unhelpful to new readers, as it misleadingly hints the setup process
may be affected by the value passed in by the value in nongit_ok,
when in reality the variable is purely used as outout-only (the
first thing the function does is to clear it).

Was it necessary to work around a compiler warning or something?

>  	if (argc < 2)
>  		die("must have at least 2 arguments");
>  	if (!strcmp(argv[1], "parse_commit_in_graph")) {
brian m. carlson Feb. 25, 2020, 12:05 a.m. UTC | #2
On 2020-02-24 at 18:05:55, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> > diff --git a/t/helper/test-repository.c b/t/helper/test-repository.c
> > index f7f8618445..ecc768e4cb 100644
> > --- a/t/helper/test-repository.c
> > +++ b/t/helper/test-repository.c
> > @@ -75,6 +75,10 @@ static void test_get_commit_tree_in_graph(const char *gitdir,
> >  
> >  int cmd__repository(int argc, const char **argv)
> >  {
> > +	int nongit_ok = 0;
> > +
> > +	setup_git_directory_gently(&nongit_ok);
> 
> No need to initialize nongit_ok to any specific value before calling
> setup_git_directory_gently() and I personally find this initialization
> unhelpful to new readers, as it misleadingly hints the setup process
> may be affected by the value passed in by the value in nongit_ok,
> when in reality the variable is purely used as outout-only (the
> first thing the function does is to clear it).
> 
> Was it necessary to work around a compiler warning or something?

I don't recall, but since this seems to be the only place we do that, I
assume it's not, and I'll just drop it.

Patch
diff mbox series

diff --git a/t/helper/test-repository.c b/t/helper/test-repository.c
index f7f8618445..ecc768e4cb 100644
--- a/t/helper/test-repository.c
+++ b/t/helper/test-repository.c
@@ -75,6 +75,10 @@  static void test_get_commit_tree_in_graph(const char *gitdir,
 
 int cmd__repository(int argc, const char **argv)
 {
+	int nongit_ok = 0;
+
+	setup_git_directory_gently(&nongit_ok);
+
 	if (argc < 2)
 		die("must have at least 2 arguments");
 	if (!strcmp(argv[1], "parse_commit_in_graph")) {