Hi, of course I messed up and did not fix the Cc: list... now fixed ;-) On Mon, 21 May 2018, Johannes Schindelin wrote: > Hi Chris, > > On Mon, 21 May 2018, Christian Couder wrote: > > > From: David Turner <dturner@xxxxxxxxxxxxxxxx> > > I vaguely remember that Dave suggested using a different email address > these days... > > *clicketyclick* > > ah, yes: > https://public-inbox.org/git/1474935093-26757-3-git-send-email-dturner@xxxxxxxxxxxx/ > > So maybe update it here, too, to > > From: David Turner <novalis@xxxxxxxxxxx> > > ? > > > So that they work under alternate ref storage backends. > > > > This will be really needed when such alternate ref storage backends are > > developed. But this could already help by making clear to readers that > > some tests do not depend on which ref backend is used. > > > > This patch just takes care of many low hanging fruits. It does not try > > to completely solves the issue. > > > > Signed-off-by: David Turner <dturner@xxxxxxxxxxxxxxxx> > > Signed-off-by: Christian Couder <chriscool@xxxxxxxxxxxxx> > > --- > > This is great. I am all for making the tests better ;-) > > > diff --git a/t/lib-t6000.sh b/t/lib-t6000.sh > > index 3f2d873fec..b8567cdf94 100644 > > --- a/t/lib-t6000.sh > > +++ b/t/lib-t6000.sh > > @@ -4,11 +4,10 @@ mkdir -p .git/refs/tags > > > > >sed.script > > > > -# Answer the sha1 has associated with the tag. The tag must exist in .git/refs/tags > > +# Answer the sha1 has associated with the tag. The tag must exist under refs/tags > > tag () { > > _tag=$1 > > - test -f ".git/refs/tags/$_tag" || error "tag: \"$_tag\" does not exist" > > - cat ".git/refs/tags/$_tag" > > + git rev-parse --verify "refs/tags/$_tag" || error "tag: \"$_tag\" does not exist" > > Line longer than 80 columns... > > > diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh > > index 0680dec808..886a9e3b72 100755 > > --- a/t/t5500-fetch-pack.sh > > +++ b/t/t5500-fetch-pack.sh > > @@ -30,7 +30,7 @@ add () { > > test_tick && > > commit=$(echo "$text" | git commit-tree $tree $parents) && > > eval "$name=$commit; export $name" && > > - echo $commit > .git/refs/heads/$branch && > > + git update-ref refs/heads/$branch $commit && > > I think we have to be careful here to quote both "refs/heads/$branch" and > "$commit" here, as a bug that introduces spaces into $commit or $branch > would have been caught earlier, but not now, unless we quote. > > This goes for all introduced `update-ref` calls. > > Maybe even for some `git rev-parse --verify` calls. > > > diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh > > index 8f5c811dd7..c3b89ae783 100755 > > --- a/t/t9903-bash-prompt.sh > > +++ b/t/t9903-bash-prompt.sh > > @@ -148,7 +148,7 @@ test_expect_success 'prompt - inside .git directory' ' > > test_expect_success 'prompt - deep inside .git directory' ' > > printf " (GIT_DIR!)" >expected && > > ( > > - cd .git/refs/heads && > > + cd .git/objects && > > __git_ps1 >"$actual" > > ) && > > test_cmp expected "$actual" > > -- > > This one looks wrong. > > Upon cursory review, one might be tempted to assume that the file is now > written into .git/objects/ instead of .git/refs/heads/. And the patch > context provided in the email is not enough to see (gawd, I hate > mail-based patch review, it really takes all my Git tools away from me). > The trick is that `actual` points at an absolute path: > > #!/bin/sh > # > # Copyright (c) 2012 SZEDER Gábor > # > > test_description='test git-specific bash prompt functions' > > . ./lib-bash.sh > > . "$GIT_BUILD_DIR/contrib/completion/git-prompt.sh" > > actual="$TRASH_DIRECTORY/actual" > [...] > > So the remaining question (which probably wants to be added to the commit > message together with a hint that `actual` points at an absolute path) is: > Why not `cd .git` instead? > > Ciao, > Dscho
