Re: [PATCH v3 1/2] t: add a test helper for getting hostname

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 19, 2024, 20:31 UTC
Message-ID: <xmqq8r2eneut.fsf@gitster.g>
In-Reply-To: <20240319183722.211300-2-ignacio@iencinas.com>

Ignacio Encinas <ignacio@iencinas.com> writes:

Show 54 quoted lines

> diff --git a/Makefile b/Makefile
> index 4e255c81f223..561d7a1fa268 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -863,6 +863,7 @@ TEST_BUILTINS_OBJS += test-userdiff.o
>  TEST_BUILTINS_OBJS += test-wildmatch.o
>  TEST_BUILTINS_OBJS += test-windows-named-pipe.o
>  TEST_BUILTINS_OBJS += test-write-cache.o
> +TEST_BUILTINS_OBJS += test-xgethostname.o
>  TEST_BUILTINS_OBJS += test-xml-encode.o
>  
>  # Do not add more tests here unless they have extra dependencies. Add
> diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
> index 482a1e58a4b6..9318900a2981 100644
> --- a/t/helper/test-tool.c
> +++ b/t/helper/test-tool.c
> @@ -86,6 +86,7 @@ static struct test_cmd cmds[] = {
>  	{ "truncate", cmd__truncate },
>  	{ "userdiff", cmd__userdiff },
>  	{ "urlmatch-normalization", cmd__urlmatch_normalization },
> +	{ "xgethostname", cmd__xgethostname },
>  	{ "xml-encode", cmd__xml_encode },
>  	{ "wildmatch", cmd__wildmatch },
>  #ifdef GIT_WINDOWS_NATIVE
> diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
> index b1be7cfcf593..075d34bd3c0a 100644
> --- a/t/helper/test-tool.h
> +++ b/t/helper/test-tool.h
> @@ -79,6 +79,7 @@ int cmd__trace2(int argc, const char **argv);
>  int cmd__truncate(int argc, const char **argv);
>  int cmd__userdiff(int argc, const char **argv);
>  int cmd__urlmatch_normalization(int argc, const char **argv);
> +int cmd__xgethostname(int argc, const char **argv);
>  int cmd__xml_encode(int argc, const char **argv);
>  int cmd__wildmatch(int argc, const char **argv);
>  #ifdef GIT_WINDOWS_NATIVE
> diff --git a/t/helper/test-xgethostname.c b/t/helper/test-xgethostname.c
> new file mode 100644
> index 000000000000..285746aef54a
> --- /dev/null
> +++ b/t/helper/test-xgethostname.c
> @@ -0,0 +1,12 @@
> +#include "test-tool.h"
> +
> +int cmd__xgethostname(int argc, const char **argv)
> +{
> +	char hostname[HOST_NAME_MAX + 1];
> +
> +	if (xgethostname(hostname, sizeof(hostname)))
> +		die("unable to get the host name");
> +
> +	puts(hostname);
> +	return 0;
> +}

OK. Given that xgethostname() is meant as a safe (and improved) replacement for the underlying gethostname(), I would have used gethostname() as the name for the above, but that alone is not big enough reason for another update.

Show 18 quoted lines

> diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
> index 18fe1c25e6a0..613c766e2bb4 100755
> --- a/t/t6500-gc.sh
> +++ b/t/t6500-gc.sh
> @@ -395,7 +395,6 @@ test_expect_success 'background auto gc respects lock for all operations' '
>  
>  	# now fake a concurrent gc that holds the lock; we can use our
>  	# shell pid so that it looks valid.
> -	hostname=$(hostname || echo unknown) &&
>  	shell_pid=$$ &&
>  	if test_have_prereq MINGW && test -f /proc/$shell_pid/winpid
>  	then
> @@ -404,7 +403,7 @@ test_expect_success 'background auto gc respects lock for all operations' '
>  		# the Windows PID in this case.
>  		shell_pid=$(cat /proc/$shell_pid/winpid)
>  	fi &&
> -	printf "%d %s" "$shell_pid" "$hostname" >.git/gc.pid &&
> +	printf "%d %s" "$shell_pid" "$(test-tool xgethostname)" >.git/gc.pid &&

We should replace the "hostname || echo unknown" in the original, instead of doing this change, as it loses the exit status from the "test-tool xgethostname" command.

Thanks.

>  	# our gc should exit zero without doing anything
>  	run_and_wait_for_auto_gc &&