git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	Sebastian Staudt <koraktor@gmail.com>,
	Josh Steadmon <steadmon@google.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH 2/2] commit-graph tests: fix cryptic unportable "dd" invocation
Date: Fri, 22 Feb 2019 11:50:53 +0100	[thread overview]
Message-ID: <20190222105053.GU1622@szeder.dev> (raw)
In-Reply-To: <878sy86anh.fsf@evledraar.gmail.com>

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
> >>

  reply	other threads:[~2019-02-22 10:51 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-19 23:29 [ANNOUNCE] Git v2.21.0-rc2 Junio C Hamano
2019-02-20  0:43 ` Randall S. Becker
2019-02-20 19:41   ` Junio C Hamano
2019-02-20 20:46     ` Johannes Schindelin
2019-02-21 13:10     ` Duy Nguyen
2019-02-21 23:55       ` brian m. carlson
2019-02-22  9:13         ` Duy Nguyen
2019-02-21 15:54     ` Randall S. Becker
2019-02-21 20:04       ` Junio C Hamano
2019-02-21 21:06       ` SZEDER Gábor
2019-02-21 21:30         ` Randall S. Becker
2019-02-21 19:59     ` Randall S. Becker
2019-02-21 10:46 ` Git for Windows v2.21.0-rc2, was " Johannes Schindelin
2019-02-21 19:28 ` [PATCH 0/2] BSD portability fixes for 2.21.0-rc2 Ævar Arnfjörð Bjarmason
2019-02-22  0:37   ` Josh Steadmon
2019-02-21 19:28 ` [PATCH 1/2] tests: fix unportable "\?" and "\+" regex syntax Ævar Arnfjörð Bjarmason
2019-02-22  5:00   ` Junio C Hamano
2019-02-21 19:28 ` [PATCH 2/2] commit-graph tests: fix cryptic unportable "dd" invocation Ævar Arnfjörð Bjarmason
2019-02-21 20:43   ` SZEDER Gábor
2019-02-21 22:26     ` Ævar Arnfjörð Bjarmason
2019-02-22 10:50       ` SZEDER Gábor [this message]
2019-02-22 14:34         ` Ævar Arnfjörð Bjarmason
2019-02-22 18:30           ` Junio C Hamano
2019-02-22 18:35             ` Todd Zullinger
2019-02-22 19:23               ` [PATCH v3] commit-graph tests: fix " Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190222105053.GU1622@szeder.dev \
    --to=szeder.dev@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=koraktor@gmail.com \
    --cc=steadmon@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).