Web lists-archives.com

Re: [PATCH 2/7] t: introduce tests for unexpected object types




On Fri, Apr 05, 2019 at 08:42:29PM +0200, SZEDER Gábor wrote:

> > > Don't run git commands upstream of a pipe, because the pipe hides
> > > their exit code.  This applies to several other tests below as well.
> > 
> > I disagree with that rule here. We're not testing "cat-file" in any
> > meaningful way, but just getting some stock output from a known-good
> > commit.
> 
> It makes auditing harder and sets bad example.

I disagree that it's a bad example. Just because a reader might not
realize that it is sometimes OK and sometimes not, does not make it bad
to do so in the OK case. It means the reader needs to understand the
rule in order to correctly apply it.

I am sympathetic to the auditing issue, though.

I dunno. In this case it is not too bad to do:

  git cat-file commit $commit >good-commit &&
  perl ... <good-commit >broken-commit

but I am not sure I am on board with a blanket rule of "git must never
be on the left-hand side of a pipe".

> > > Wouldn't a 'sed' one-liner suffice, so we won't have yet another perl
> > > dependency?
> > 
> > Heh, this was actually the subject of much discussion before the patches
> > hit the list. If you can write such a one-liner that is both readable
> > and portable, please share it. I got disgusted with sed and suggested
> > this perl.
> 
> Hm, so the trivial s/// with '\n' in the replacement part is not
> portable, then?  Oh, well.

I don't think it is, but I could be wrong. POSIX does say that "\n"
matches a newline in the pattern space, but nothing about it on the RHS
of a substitution. I have a vague feeling of running into problems in
the past, but I could just be misremembering.

We also tried matching /^tree/ and using "a\" to append a line, but it
was both hard to read and hit portability issues with bsd sed.

-Peff