Bug in test-lib.sh: test_create_repo() / RFC

Hi there,

running the test suite with -v for the upcoming release exposed a certain problem with test_create_repo() whose consequences I can't quite fathom at the moment. That means: I don't know whether it's maint material or forbidden fruits during rc-cycle...

Problem: Since a6d63b7 (test-lib: avoid assuming that templates/ are in the GIT_EXEC_PATH, 2009-02-04), test_create_repo() assumes to be called from a directory such that `pwd`/../templates/blt/ contains templates for git-init.

Several tests (see below) call test_create_repo() from a different directory, which means the repo is created without any of the default files (and that a mv .git/hooks .git/hooks-disabled later in the function errors out). Now, for most tests this probably doesn't matter at all but it's not nice.

RFC: I see several possible solutions:

- Make sure all tests use test_create_repo() from t/. Cumbersome and
fragile.

- Simply use $(TEST_DIRECTORY)/../templates/blt/. Nice and easy. But
uses the templates from the git repo containing t/ even when testing
against and installed git (just like now, for most of the tests).

- Teach git a "--templates-dir" option similar to "--html-path" and use
that (from the git actually being tested). Means we use the templates
belonging to the tested git; but also means we can test only git
versions containing that new option.

What do you think?

Michael

Affected tests: t0050-filesystem.sh t1007-hash-object.sh t1302-repo-version.sh t2103-update-index-ignore-missing.sh t4027-diff-submodule.sh t5300-pack-object.sh t5513-fetch-track.sh t5600-clone-fail-cleanup.sh t5601-clone.sh t5700-clone-reference.sh t5710-info-alternate.sh t6026-merge-attr.sh t7001-mv.sh t7010-setup.sh t7401-submodule-summary.sh t7506-status-submodule.sh t7508-status.sh

On Mon, Apr 20, 2009 at 04:51:18PM +0200, Michael J Gruber wrote:

If I am understanding it correctly, I think this is simply a bug introduced by a6d63b7, and the fix is maint-worthy.

And I think the right fix is:

The original code (before a6d63b7) used $(GIT_EXEC_PATH), since it handily pointed to "$(TEST_DIRECTORY)/.." (and I didn't check, but I suspect that line predates $(TEST_DIRECTORY) entirely). So I think this is just doing what the a6d63b7 should have done in the first place, instead of using `pwd` to guess at the top-level location.

One thing to consider: we now have code to test an installed version of git. Should it be using the vanilla hooks from the source directory, or should it be using the regular templates? I think that question is orthogonal to the bug you mention, though -- the current code is trying to use the hooks from the source directory, but it is just failing to do so; by fixing it, you certainly won't be breaking the test-installed-version case.

-Peff

Jeff King <peff@peff.net> writes:

Yup, I think that is the right approach.

Currently, test_create_repo() expects that templates can be found below $GIT_EXEC_DIR. This assumption fails when tests are run against a git installed somewhere else. Therefore, use $TEST_DIRECTORY as introduced in 2d84e9fb and expect templates to be present in $TEST_DIRECTORY/.. which should be the root dir of the git checkout.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
This is a patch against maint as discussed in
http://article.gmane.org/gmane.comp.version-control.git/11699 although
maint does not (yet) contain a6d63b7 which actually exposes the problem.

The patch does not apply cleanly to master and needs a different commit message there anyways. Therefore, I will send a follow-up with a patch against master. I suggest considering this "maint for master", i.e. applying to master where the bug is actually exposed.

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

Currently, test_create_repo() expects that templates can be found below `pwd`/.. This assumption fails when tests are run against a git installed somewhere else or test_create_repo() is called from subdirectiories (several tests do this). Therefore, use $TEST_DIRECTORY as introduced in 2d84e9fb and expect templates to be present in $TEST_DIRECTORY/.. which should be the root dir of the git checkout.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 t/test-lib.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Hi,

On Tue, 21 Apr 2009, Michael J Gruber wrote:

So it is a fix in need of a problem?

Ciao, Dscho

Johannes Schindelin venit, vidit, dixit 21.04.2009 11:26:

Your question is answered by the second paragraph which I wrote and you cut... I suggested applying to master only. (Admittedly, the word "only" does not appear there.)

Cheers, Michael