Re: [PATCH 02/18] t9300: check ref existence using git-rev-parse rather than FS check
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-04-21 06:01:39
On Mon, Apr 19 2021, Han-Wen Nienhuys via GitGitGadget wrote:
quoted hunk ↗ jump to hunk
From: Han-Wen Nienhuys <redacted> Signed-off-by: Han-Wen Nienhuys <redacted> --- t/t9300-fast-import.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index 5c47ac4465cb..087ddf097036 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh@@ -392,7 +392,7 @@ test_expect_success 'B: accept branch name "TEMP_TAG"' ' git gc git prune" && git fast-import <input && - test -f .git/TEMP_TAG && + git rev-parse TEMP_TAG && test $(git rev-parse main) = $(git rev-parse TEMP_TAG^) '
It seems to me that this breaks the test, to the extent that it's
testing something we care about at all.
I.e. reading ea08a6fd19 (Actually allow TAG_FIXUP branches in
fast-import, 2007-08-02) the whole point is to test that a "TEMP_TAG"
ref is accepted by fast-import, as opposed to "refs/heads/TEMP_TAG".
So we're trying to check that the refstore accepts a concept of a name
like HEAD called BLAHBLAH or whatever, that lives at the "top-level".
For something unrelated I was trying to figure out how to detect that
the other day and couldn't come up with anything I was happy with,
e.g. this seems to work:
# Rely on for-each-ref not listing it...
git for-each-ref >gfe &&
! grep TEMP_TAG gfe &&
# ... but rev-parse grokking it
git rev-parse TEMP_TAG
Or, rely on rev-parse emitting a warning about it:
$ git update-ref refs/heads/TEMP_TAG HEAD
# A bug? We emit both an error and a warning with
# --symbolic-full-name?
$ git rev-parse --symbolic-full-name TEMP_TAG
warning: refname 'TEMP_TAG' is ambiguous.
error: refname 'TEMP_TAG' is ambiguous
$ echo $?
0
IOW re my
https://lore.kernel.org/git/87wnt2th1v.fsf@evledraar.gmail.com/ (local) this
seems like exactly the sort of edge case where we want to dig a bit more
and assert that we support (or explicitly don't support) this "mode"
under reftable.
In any case, if we're not testing for .git/TEMP_TAG anymore I think your
addition of "git rev-parse TEMP_TAG" is entirely redundant to the ad-hoc
"test_cmp" that comes after it, when would we not be able to rev-parse
FOO, but succeed with FOO^ ?