public inbox for git@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] test-lib: respect GIT_TEST_INSTALLED when querying default hash
@ 2025-07-15 18:57 Kyle Lippincott via GitGitGadget
  2025-07-15 21:27 ` Junio C Hamano
  2025-07-15 21:33 ` Justin Tobler
  0 siblings, 2 replies; 3+ messages in thread
From: Kyle Lippincott via GitGitGadget @ 2025-07-15 18:57 UTC (permalink / raw)
  To: git; +Cc: Kyle Lippincott, Kyle Lippincott

From: Kyle Lippincott <spectral@google.com>

$GIT_TEST_INSTALLED can be set to use an "installed" git instead of the
one from $GIT_BUILD_DIR. This is used by my company's internal test
infrastructure, and not using $GIT_TEST_INSTALLED when querying the
default hash meant that the tests were failing because the hash was
effectively set to the empty string (since git didn't execute).

In the two places we attempt to detect/execute git itself prior to
overriding everything and putting it in $PATH, use identical logic for
identifying the git binary to execute. This also has the effect of
including the $X suffix when querying the default hash, but that's not
strictly necessary. You don't need to specify .exe when running a binary
on Windows, just when testing whether it exists or not.

Signed-off-by: Kyle Lippincott <spectral@google.com>
---
    test-lib: respect GIT_TEST_INSTALLED when querying default hash

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2011%2Fspectral54%2Fdefault_hash_respect_git_test_installed-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2011/spectral54/default_hash_respect_git_test_installed-v1
Pull-Request: https://github.com/git/git/pull/2011

 t/test-lib.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 6dc2022ee10..621cd31ae1d 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -134,7 +134,8 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 ################################################################
 # It appears that people try to run tests without building...
-"${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git$X" >/dev/null
+GIT_BINARY="${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git$X"
+"$GIT_BINARY" >/dev/null
 if test $? != 1
 then
 	if test -n "$GIT_TEST_INSTALLED"
@@ -536,7 +537,7 @@ export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
 export GIT_COMMITTER_DATE GIT_AUTHOR_DATE
 export EDITOR
 
-GIT_TEST_BUILTIN_HASH=$("$GIT_BUILD_DIR/git" version --build-options | sed -ne 's/^default-hash: //p')
+GIT_TEST_BUILTIN_HASH=$("$GIT_BINARY" version --build-options | sed -ne 's/^default-hash: //p')
 GIT_DEFAULT_HASH="${GIT_TEST_DEFAULT_HASH:-$GIT_TEST_BUILTIN_HASH}"
 export GIT_DEFAULT_HASH
 GIT_DEFAULT_REF_FORMAT="${GIT_TEST_DEFAULT_REF_FORMAT:-files}"

base-commit: e9779f64349fbcc8d177d055208039877316e652
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] test-lib: respect GIT_TEST_INSTALLED when querying default hash
  2025-07-15 18:57 [PATCH] test-lib: respect GIT_TEST_INSTALLED when querying default hash Kyle Lippincott via GitGitGadget
@ 2025-07-15 21:27 ` Junio C Hamano
  2025-07-15 21:33 ` Justin Tobler
  1 sibling, 0 replies; 3+ messages in thread
From: Junio C Hamano @ 2025-07-15 21:27 UTC (permalink / raw)
  To: Kyle Lippincott via GitGitGadget; +Cc: git, Kyle Lippincott

"Kyle Lippincott via GitGitGadget" <gitgitgadget@gmail.com> writes:

>  ################################################################
>  # It appears that people try to run tests without building...
> -"${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git$X" >/dev/null
> +GIT_BINARY="${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git$X"
> +"$GIT_BINARY" >/dev/null
>  if test $? != 1
>  then
>  	if test -n "$GIT_TEST_INSTALLED"
> @@ -536,7 +537,7 @@ export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
>  export GIT_COMMITTER_DATE GIT_AUTHOR_DATE
>  export EDITOR
>  
> -GIT_TEST_BUILTIN_HASH=$("$GIT_BUILD_DIR/git" version --build-options | sed -ne 's/^default-hash: //p')
> +GIT_TEST_BUILTIN_HASH=$("$GIT_BINARY" version --build-options | sed -ne 's/^default-hash: //p')

Very nice.  I very much like the way this reuses what we detected earlier.

Thanks, will queue.

>  GIT_DEFAULT_HASH="${GIT_TEST_DEFAULT_HASH:-$GIT_TEST_BUILTIN_HASH}"
>  export GIT_DEFAULT_HASH
>  GIT_DEFAULT_REF_FORMAT="${GIT_TEST_DEFAULT_REF_FORMAT:-files}"
>
> base-commit: e9779f64349fbcc8d177d055208039877316e652

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] test-lib: respect GIT_TEST_INSTALLED when querying default hash
  2025-07-15 18:57 [PATCH] test-lib: respect GIT_TEST_INSTALLED when querying default hash Kyle Lippincott via GitGitGadget
  2025-07-15 21:27 ` Junio C Hamano
@ 2025-07-15 21:33 ` Justin Tobler
  1 sibling, 0 replies; 3+ messages in thread
From: Justin Tobler @ 2025-07-15 21:33 UTC (permalink / raw)
  To: Kyle Lippincott via GitGitGadget; +Cc: git, Kyle Lippincott

On 25/07/15 06:57PM, Kyle Lippincott via GitGitGadget wrote:
> From: Kyle Lippincott <spectral@google.com>
> 
> $GIT_TEST_INSTALLED can be set to use an "installed" git instead of the
> one from $GIT_BUILD_DIR. This is used by my company's internal test
> infrastructure, and not using $GIT_TEST_INSTALLED when querying the
> default hash meant that the tests were failing because the hash was
> effectively set to the empty string (since git didn't execute).
> 
> In the two places we attempt to detect/execute git itself prior to
> overriding everything and putting it in $PATH, use identical logic for
> identifying the git binary to execute. This also has the effect of
> including the $X suffix when querying the default hash, but that's not
> strictly necessary. You don't need to specify .exe when running a binary
> on Windows, just when testing whether it exists or not.

The second paragraph was a little difficult for me to parse, but I
understand the change as the following:

In c79bb70a2e (Enable SHA-256 by default in breaking changes mode,
2025-07-01), when building Git with `WITH_BREAKING_CHANGES` defined, the
default object hash format is changed from SHA1 to SHA256. To select the
correct hash used by `GIT_TEST_BUILTIN_HASH` during testing,
git-version(1) is executed with the `--build-options` flag to get the
default hash the Git binary is built with.

When running the Git testsuite with `GIT_TEST_INSTALLED` set, the Git
binary in `GIT_BUILD_DIR` is used to check for the default hash which is
incorrect. Instead the binary at `GIT_TEST_INSTALLED` should be used.

> Signed-off-by: Kyle Lippincott <spectral@google.com>
> ---
>     test-lib: respect GIT_TEST_INSTALLED when querying default hash
> 
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2011%2Fspectral54%2Fdefault_hash_respect_git_test_installed-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2011/spectral54/default_hash_respect_git_test_installed-v1
> Pull-Request: https://github.com/git/git/pull/2011
> 
>  t/test-lib.sh | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 6dc2022ee10..621cd31ae1d 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -134,7 +134,8 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>  
>  ################################################################
>  # It appears that people try to run tests without building...
> -"${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git$X" >/dev/null
> +GIT_BINARY="${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git$X"
> +"$GIT_BINARY" >/dev/null

Here we store the Git binary we should be testing with so we can use it
later. Makes sense.

>  if test $? != 1
>  then
>  	if test -n "$GIT_TEST_INSTALLED"
> @@ -536,7 +537,7 @@ export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
>  export GIT_COMMITTER_DATE GIT_AUTHOR_DATE
>  export EDITOR
>  
> -GIT_TEST_BUILTIN_HASH=$("$GIT_BUILD_DIR/git" version --build-options | sed -ne 's/^default-hash: //p')
> +GIT_TEST_BUILTIN_HASH=$("$GIT_BINARY" version --build-options | sed -ne 's/^default-hash: //p')

Now we use the correct binary to check the default hash depending on how
the tests are executed. Looks good to me :)

-Justin

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-07-15 21:39 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-15 18:57 [PATCH] test-lib: respect GIT_TEST_INSTALLED when querying default hash Kyle Lippincott via GitGitGadget
2025-07-15 21:27 ` Junio C Hamano
2025-07-15 21:33 ` Justin Tobler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox