Hi,
of course I messed up and did not fix the Cc: list... now fixed ;-)
On Mon, 21 May 2018, Johannes Schindelin wrote:
Show 115 quoted lines
> Hi Chris,
>
> On Mon, 21 May 2018, Christian Couder wrote:
>
> > From: David Turner <dturner@twopensource.com>
>
> 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@twosigma.com/
>
> So maybe update it here, too, to
>
> From: David Turner <novalis@novalis.org>
>
> ?
>
> > 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@twopensource.com>
> > Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> > ---
>
> 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,
> DschoJohannes Schindelin <johannes.schindelin@gmx.de>