Re: [PATCH v2 2/3] rebase -i: re-fix short SHA-1 collision

Johannes Schindelin [off-list ref] writes:

quoted

True.  Doesn't rev-parse have an appropriate option for this kind of
thing that gets rid of the need for "cut" in the first place?

You mean `git rev-parse --short=4`? That does something _sligthly_
different: it tries to shorten the OID to 4 characters _unless that would
be ambiguous_. In our case, it _will_ be ambiguous. That's why I use
`cut`.

Ah, yes of course; we want ambiguous prefix.  I think a more
thorough test would be to see that the output with --short=$n (where
n is the length of the abbreviated object name in $colliding_sha1)
is longer than $colliding_sha1 and the output prefix-matches
$colliding_sha1 iow, something like

	abbreviated=$(git rev-parse --short=7 HEAD) &&
	case "$abbreviated" in
	"$colliding_sha1"?*) : happy ;;
	*) false ;;
	esac &&
	...

which would make sure that we are testing colliding case.

As to the crash in `rev-parse` _after_ printing out the OID: yes, there is
a possibility for that. But that regression test is not about `rev-parse`,
so it is actually a good thing that it would not trigger on such a bug ;-)

No, I do not think this test should be about rev-parse working
correctly---just that if it is easy enough to make the test robust
enough against such a breakage, it would be nice to do so, that's
all.

I'm not Eric but I suspect his primary point was not about worrying
about rev-parse crashing but more about avoiding to add a pattern
less experienced developers can copy&paste without thinking enough
to realize why it would be OK here and not OK in the context of the
tests they are adding.  That would be what I would worry about more
than rev-parse crashing in the part of the test under discussion.

Thanks.