Hi Junio, On Tue, 21 Jan 2020, Junio C Hamano wrote: > Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes: > > > On Mon, Jan 20, 2020 at 3:04 PM Johannes Schindelin > > <Johannes.Schindelin@xxxxxx> wrote: > >> On Mon, 20 Jan 2020, Eric Sunshine wrote: > >> > On Fri, Jan 17, 2020 at 6:38 PM Johannes Schindelin via GitGitGadget > >> > <gitgitgadget@xxxxxxxxx> wrote: > >> > > + test $colliding_sha1 = "$(git rev-parse HEAD | cut -c 1-7)" && > >> > > >> > How much do we care that this is introducing new code with git > >> > upstream of a pipe (considering recent efforts to eradicate such > >> > usage)? Same question regarding several other new instances introduce > >> > by this patch. > >> > >> I would argue that the test case will fail if the `git` call fails. So I > >> am not overly concerned if that `git` call is upstream of a pipe. > > > > Unless the git command crashes _after_ it produces the correct output... > > 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`. 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 ;-) Ciao, Dscho
