Re: [PATCH v3 1/3] t5323: test cases for git-pack-redundant

From: "Torsten Bögershausen" <tboegi@web.de>
To: "SZEDER Gábor" <szeder.dev@gmail.com>,
	"Jiang Xin" <worldhello.net@gmail.com>
Cc: Sun Chao <sunchao9@huawei.com>, Git List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>,
	Jiang Xin <zhiyou.jx@alibaba-inc.com>
Subject: Re: [PATCH v3 1/3] t5323: test cases for git-pack-redundant
Date: Thu, 10 Jan 2019 13:25:35 +0100	[thread overview]
Message-ID: <700045da-2d91-a3c7-aaf2-c9d7bdd86a51@web.de> (raw)
In-Reply-To: <20190110115704.GL4673@szeder.dev>

On 10.01.19 12:57, SZEDER Gábor wrote:
> On Thu, Jan 10, 2019 at 11:28:34AM +0800, Jiang Xin wrote:
>> SZEDER Gábor <szeder.dev@gmail.com> 于2019年1月9日周三 下午8:56写道：
>>>> +             sed -e "s#^.*/pack-\(.*\)\.\(idx\|pack\)#\1#g" | \
>>>
>>> This sed command doesn't seem to work on macOS (on Travis CI), and
>>> causes the test to fail with:
>>>
>>
>> It works if rewrite as follows:
>>
>>     git pack-redundant --all >out &&
>>     sed -E -e "s#.*/pack-(.*)\.(idx|pack)#\1#" out | \
>>
>> Without `-E`, MasOS has to write two seperate sed commands, such as:
>>
>>     git pack-redundant --all >out &&
>>     sed -e "s#.*/pack-\(.*\)\.idx#\1#" out | \
>>     sed -e "s#.*/pack-\(.*\)\.pack#\1#"
>>
>> Option '-E' is an alias for -r in GNU sed 4.2  (added in 4.2, not documented
>> unti 4.3), released on May 11 2009.  I prefer the `-E` version.
> 
> Is 'sed -E' portable enough, e.g. to the various BSDs, Solaris, and
> whatnot?  I don't know, but POSIX doesn't mention it, there is not a
> single instance of it in our current codebase, and it appears that
> we've never used it before, either.  OTOH,

If we can use "two seperate sed commands" i would (really) prefer to so,
to avoid "sed -E".
My conclusion is that it is not portable enough.
> 't/check-non-portable-shell.pl' doesn't catch it as non-portable
> construct...

Good point.
Actually that script only checks "known non-portable" options.
Every time somebody finds a non-portable option, we update it.
A growing blacklist, so to say.
May be we should have a white list instead.