[v4,3/3] ci: stop linking built-ins to the dashed versions

Commit Message

Jean-Noël Avila via GitGitGadget Sept. 21, 2020, 10:28 p.m. UTC

From: Johannes Schindelin <[e-mail]>

Since e4597aae6590 (run test suite without dashed git-commands in PATH,
2009-12-02), we stopped running our tests with `git-foo` binaries found
at the top-level directory of a freshly built source tree; instead we
have placed only `git` and selected `git-foo` commands that must be on
`$PATH` in `bin-wrappers/` and prepended that `bin-wrappers/` to the
`PATH` used in the test suite. We did that to catch the tests and
scripted Git commands that still try to use the dashed form.

Since CI jobs will not install the built Git to anywhere, and the
hardlinks we make at the top-level of the source tree for `git-add` and
friends are not even used during tests, they are pure waste of resources
these days.

Thanks to the newly invented `SKIP_DASHED_BUILT_INS` knob, we can now
skip creating these links in the source tree. So let's do that.

Note that this change introduces a subtle change of behavior: when Git's
`cmd_main()` calls `setup_path()`, it inserts the value of
`GIT_EXEC_PATH` (defaulting to `<prefix>/libexec/git-core`) at the
beginning of the environment variable `PATH`. This is necessary to find
e.g. scripted commands that are installed in that location. For the
purposes of Git's test suite, the `bin-wrappers/` scripts override
`GIT_EXEC_PATH` to point to the top-level directory of the source code.

In other words, if a scripted command had used a dashed invocation of a
built-in Git command, it would not have been caught previously, which is
fixed by this change.

Signed-off-by: Johannes Schindelin <[e-mail]>
---
 ci/lib.sh | 1 +
 1 file changed, 1 insertion(+)

Comments

Junio C Hamano Sept. 21, 2020, 10:53 p.m. UTC | #1

"Johannes Schindelin via GitGitGadget" <[e-mail]>
writes:

> From: Johannes Schindelin <[e-mail]>
>
> Since e4597aae6590 (run test suite without dashed git-commands in PATH,
> 2009-12-02), we stopped running our tests with `git-foo` binaries found
> at the top-level directory of a freshly built source tree; instead we
> have placed only `git` and selected `git-foo` commands that must be on
> `$PATH` in `bin-wrappers/` and prepended that `bin-wrappers/` to the
> `PATH` used in the test suite. We did that to catch the tests and
> scripted Git commands that still try to use the dashed form.
>
> Since CI jobs will not install the built Git to anywhere, and the
> hardlinks we make at the top-level of the source tree for `git-add` and
> friends are not even used during tests, they are pure waste of resources
> these days.

Makes perfect sense, and I do not think readers will confused like
they were by the previous round's corresponding step.

> diff --git a/ci/lib.sh b/ci/lib.sh
> index 3eefec500d..821e3660d6 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -178,6 +178,7 @@ fi
>  export DEVELOPER=1
>  export DEFAULT_TEST_TARGET=prove
>  export GIT_TEST_CLONE_2GB=true
> +export SKIP_DASHED_BUILT_INS=YesPlease

OK.  This would hopefully cover all the CI targets; we know it
covers everybody who uses DEVELOPER=1, which is a good sign ;-)

Thanks.

Patch

diff --git a/ci/lib.sh b/ci/lib.sh
index 3eefec500d..821e3660d6 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -178,6 +178,7 @@  fi
 export DEVELOPER=1
 export DEFAULT_TEST_TARGET=prove
 export GIT_TEST_CLONE_2GB=true
+export SKIP_DASHED_BUILT_INS=YesPlease
 
 case "$jobname" in
 linux-clang|linux-gcc)