On 2020-01-13 at 13:41:44, Eric Sunshine wrote:
> On Mon, Jan 13, 2020 at 7:40 AM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> > I suspect that t3404 also has a bug, since the object IDs that are
> > supposed to collide do not, according to my instrumentation of the test.
> > I'm unsure what the intended collision was and consequently haven't
> > fixed it.  However, it does work with SHA-256 as it stands and is no
> > more or less functional than with SHA-1, so I've removed the
> > prerequisite.
> 
> The test itself is fine, but it is one of those unfortunate cases of
> checking for absence of something (which is a wide net). As explained
> by the commit message[1] of the patch which added the test, the
> collision occurred only between short OID's. The patch[2] which fixed
> the problem did so by avoiding short OID's in the scripted
> implementation of `git rebase -i` (and also flipped the test from
> `text_expect_failure` to `test_expect_success`).
> 
> The test, as currently implemented, is very much specific to SHA-1
> since the FAKE_COMMIT_MESSAGE="collide2 ac4f2ee" it uses only produces
> a collision with short OID's when SHA-1 is the hashing function, so
> the prerequisite is correct and serves as documentation (even if it
> doesn't affect the outcome of the test). Removing that prerequisite
> should only be done if the test is updated with a different
> FAKE_COMMIT_MESSAGE which causes a short OID collision when SHA-256 is
> used.

I'll take another look.  When I looked at the output, it looked like
they didn't collide anymore even under SHA-1, but perhaps I instrumented
the test wrong and therefore got the wrong result.  Thanks for double
checking.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204