All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: "Jeff King" <peff@peff.net>, "Eric Wong" <e@80x24.org>,
	"René Scharfe" <l.s.r@web.de>,
	git@vger.kernel.org
Subject: Re: [PATCH] test-lib: stricter unzip(1) check
Date: Mon, 18 Jul 2016 11:20:19 -0700	[thread overview]
Message-ID: <xmqqtwfmkduk.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <alpine.DEB.2.20.1607181536540.3472@virtualbox> (Johannes Schindelin's message of "Mon, 18 Jul 2016 15:52:31 +0200 (CEST)")

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hrm. That sounds a little magical, and fragile, to me. What if the next
> person's unzip returns 0 and *still* cannot handle -a?

That is a very sensible line of thought.

> I'd rather do something like

... but the patch presented as an alternative does not seem to
follow that line of thought.  After reading that sensible line of
thought I would have expected to see an auto-detection of the path
and support for features we care about.

Stepping back a bit, why do we even care if "unzip -a" works on
"../$zipfile" and converts things correctly in that check_zip() test
in t5003 in the first place?  It looks more like a test on "unzip"
than making sure we correctly generate a zip archive to me...

> -- snipsnap --
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 0055ebb..5b9521e 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -929,7 +929,8 @@ yes () {
>  }
>  
>  # Fix some commands on Windows
> -case $(uname -s) in
> +uname_s=$(uname -s)
> +case $uname_s in
>  *MINGW*)
>  	# Windows has its own (incompatible) sort and find
>  	sort () {
> @@ -1100,6 +1101,7 @@ test_lazy_prereq SANITY '
>  	return $status
>  '
>  
> +test FreeBSD != $uname_s || GIT_UNZIP=${GIT_UNZIP:-/usr/local/bin/unzip}
>  GIT_UNZIP=${GIT_UNZIP:-unzip}
>  test_lazy_prereq UNZIP '
>  	"$GIT_UNZIP" -v

  reply	other threads:[~2016-07-18 18:20 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-18  6:44 [PATCH] test-lib: stricter unzip(1) check Eric Wong
2016-07-18 13:04 ` Jeff King
2016-07-18 13:52   ` Johannes Schindelin
2016-07-18 18:20     ` Junio C Hamano [this message]
2016-07-18 18:56       ` Jeff King
2016-07-18 19:17         ` Junio C Hamano
2016-07-19 11:27       ` Johannes Schindelin
2016-07-19 17:26         ` Junio C Hamano
2016-07-18 19:43   ` Junio C Hamano
2016-07-18 20:03     ` Eric Wong
2016-07-18 20:19       ` Junio C Hamano
2016-07-18 21:19         ` Eric Wong
2016-07-18 21:27           ` Junio C Hamano
2016-07-18 23:41     ` Jeff King

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

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

  git send-email \
    --in-reply-to=xmqqtwfmkduk.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=e@80x24.org \
    --cc=git@vger.kernel.org \
    --cc=l.s.r@web.de \
    --cc=peff@peff.net \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.