Subject: Re: [PATCH 2/2] commit-graph tests: fix cryptic unportable "dd" invocation

From: SZEDER Gábor <[email-masked]>
Date: Fri, 22 Feb 2019 11:50:53 +0100

On Thu, Feb 21, 2019 at 11:26:26PM +0100, Ævar Arnfjörð Bjarmason wrote:
> >> -	dd of="$objdir/info/commit-graph" bs=1 seek="$zero_pos" count=0 &&
> >> +	perl -we 'truncate $ARGV[0], $ARGV[1] if -s $ARGV[0] > $ARGV[1]' \
> >> +		$objdir/info/commit-graph $zero_pos &&
> >
> > This will make Dscho unhappy :)
> 
> Sorry Dscho :)
> 
> Although this is a one-off in one test, as opposed to a new "perl -e" in
> test-lib-functions.sh
> 
> > Is there a problem with:
> >
> >   dd if=/dev/null of="$objdir/info/commit-graph" bs=1 seek="$zero_pos"
> >
> > ?
> >
> > To my understanding of the specs it's well-defined what it should do,
> > even when $zero_pos is larget than the file size,  it's shorter,
> > simpler, and doesn't introduce yet another Perl dependency.
> 
> I tried that as a one-off and it indeed works as a "truncate" on NetBSD
> & GNU.
> 
> My reading of POSIX "dd" and "lseek" docs is that we'd need some similar
> guard if we're going to be paranoid about a $zero_pos value past the end
> of the file. It doesn't look like that's portable, my assumption from
> reading the docs is that the seek=* will devolve without a stat() check
> on some "dd" implementations to an "lseek".

Could you point to the part of the specs where your assumption comes
from?  The specs are quite clear on what should happen:

  If the size of the seek plus the size of the input file is less than
  the previous size of the output file, the output file shall be
  shortened by the copy. If the input file is empty and either the
  size of the seek is greater than the previous size of the output
  file or the output file did not previously exist, the size of the
  output file shall be set to the file offset after the seek.

IOW no such guard is necessary.

I checked the man pages of FreeBSD's, NetBSD's, OpenBSD's and Solaris'
'dd', and they are clearly following the specs in this respect.  I
tried NetBSD 6.0's and 8.0's 'dd', and both behave as advertised.

And using 'dd' doesn't add a condition after statement...

> I'm not going to submit a re-roll of this because it works, and I'd
> still trust Perl's truncate(...) portability over dd.
>
> But more importantly because it takes me *ages* to fully re-test
> anything on the slow BSD VMs I have access to, and I already tore town
> my one-off hacking env there after testing these patches...
> 
> >>  	generate_zero_bytes $(($orig_size - $zero_pos)) >>"$objdir/info/commit-graph" &&
> >>  	test_must_fail git commit-graph verify 2>test_err &&
> >>  	grep -v "^+" test_err >err &&
> >> --
> >> 2.21.0.rc0.258.g878e2cd30e
> >>