git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / 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
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 index

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

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git