Re: [PATCH] tests: don't give unportable ">" to "test" built-in, use -gt
On Aug 06 2017, Ævar Arnfjörð Bjarmason wrote: > diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh > index 0ef7b94394..0e2e57aa3d 100755 > --- a/t/t7004-tag.sh > +++ b/t/t7004-tag.sh > @@ -1887,7 +1887,7 @@ EOF" > run_with_limited_stack git tag --contains HEAD >actual && > test_cmp expect actual && > run_with_limited_stack git tag --no-contains HEAD >actual && > - test_line_count ">" 10 actual > + test_line_count "-gt" 10 actual Maybe also remove the quotes, they are no longer needed. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: [PATCH] tests: don't give unportable ">" to "test" built-in, use -gt
Ævar Arnfjörð Bjarmason writes:
> Change an argument to test_line_count (which'll ultimately be turned
> into a "test" expression) to use "-gt" instead of ">" for an
> arithmetic test.
>
> This broken on e.g. OpenBSD as of v2.13.0 with my commit
> ac3f5a3468 ("ref-filter: add --no-contains option to
> tag/branch/for-each-ref", 2017-03-24).
>
> Upstream just worked around it by patching git and didn't tell us
I'm tempted to do s/Upstream/Downstream/ while queuing, but please
tell me I am confused.
> about it, I discovered this when reading various Git packaging
> implementations: https://github.com/openbsd/ports/commit/7e48bf88a20
>
> Signed-off-by: Ævar Arnfjörð Bjarmason
> ---
>
> David, it would be great to get a quick bug report to
> git@vger.kernel.org if you end up having to monkeypatch something
> we've done. We won't bite, promise :)
>
> As shown in that linked Github commit OpenBSD has another recent
> workaround in turning on DIR_HAS_BSD_GROUP_SEMANTICS and skipping a
> related test, maybe René can make more sense of that?
>
> There's more patches in their ports which indicate possible bugs of
> ours: https://github.com/openbsd/ports/tree/master/devel/git/patches/
>
> t/t7004-tag.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> index 0ef7b94394..0e2e57aa3d 100755
> --- a/t/t7004-tag.sh
> +++ b/t/t7004-tag.sh
> @@ -1887,7 +1887,7 @@ EOF"
> run_with_limited_stack git tag --contains HEAD >actual &&
> test_cmp expect actual &&
> run_with_limited_stack git tag --no-contains HEAD >actual &&
> - test_line_count ">" 10 actual
> + test_line_count "-gt" 10 actual
> '
>
> test_expect_success '--format should list tags as per format given' '
Re: [PATCH] tests: don't give unportable ">" to "test" built-in, use -gt
On Mon, Aug 07 2017, René Scharfe jotted:
> Am 07.08.2017 um 03:18 schrieb brian m. carlson:
>> On Sun, Aug 06, 2017 at 11:38:50PM +, Ævar Arnfjörð Bjarmason wrote:
>>> Change an argument to test_line_count (which'll ultimately be turned
>>> into a "test" expression) to use "-gt" instead of ">" for an
>>> arithmetic test.
>>>
>>> This broken on e.g. OpenBSD as of v2.13.0 with my commit
>>> ac3f5a3468 ("ref-filter: add --no-contains option to
>>> tag/branch/for-each-ref", 2017-03-24).
>>>
>>> Upstream just worked around it by patching git and didn't tell us
>>> about it, I discovered this when reading various Git packaging
>>> implementations: https://github.com/openbsd/ports/commit/7e48bf88a20
>>>
>>> Signed-off-by: Ævar Arnfjörð Bjarmason
>>> ---
>>>
>>> David, it would be great to get a quick bug report to
>>> git@vger.kernel.org if you end up having to monkeypatch something
>>> we've done. We won't bite, promise :)
>>>
>>> As shown in that linked Github commit OpenBSD has another recent
>>> workaround in turning on DIR_HAS_BSD_GROUP_SEMANTICS and skipping a
>>> related test, maybe René can make more sense of that?
>>
>> I've confirmed using the NetBSD 7.1 man pages that NetBSD will also want
>> DIR_HAS_BSD_GROUP_SEMANTICS. MirBSD will also, according to its man
>> pages.
>>
>> As I understand it, the only consequence of not setting this flag on BSD
>> systems is that some directories will be setgid, which, while ugly and
>> useless, should have no negative effect.
>
> Right; specifically it's for newly created subdirectories of shared
> directories, which we want to be owned by the same group as their
> parent. That's the default on BSDs, and we have to set the setgid bit to
> turn on that semantic only on other systems, e.g. on Linux.
>
> 81a24b52c1 (Do not use GUID on dir in git init --shared=all on FreeBSD)
> introduced DIR_HAS_BSD_GROUP_SEMANTICS with the rationale that setting
> setgid on directories may not even be allowed for normal users. I can't
> reproduce this (t1301 succeeds for me on FreeBSD 10.3 even without that
> build flag), but apparently at least in some configurations it's not just
> a cosmetic issue.
>
> The skipped test 'init in long base path' in t0001 is a different kettle
> of fish. getcwd(3) on OpenBSD respects permissions on the parent
> directories up to root. E.g. after "chmod 711 /home" normal users would
> get EACCES when they'd call getcwd(3) in their homes there, while e.g.
> on Linux and FreeBSD they'd successfully get their current working dir.
Good summary. As openbsd-tech points out it seems the bug is ours, if
I'm understanding this correctly:
http://marc.info/?l=openbsd-tech&m=149625768825173&w=2
[EACCES]
Search permission was denied for the current directory, or read or
search permission was denied for a directory above the current
directory in the file hierarchy.
Re: [PATCH] tests: don't give unportable ">" to "test" built-in, use -gt
Am 07.08.2017 um 03:18 schrieb brian m. carlson:
> On Sun, Aug 06, 2017 at 11:38:50PM +, Ævar Arnfjörð Bjarmason wrote:
>> Change an argument to test_line_count (which'll ultimately be turned
>> into a "test" expression) to use "-gt" instead of ">" for an
>> arithmetic test.
>>
>> This broken on e.g. OpenBSD as of v2.13.0 with my commit
>> ac3f5a3468 ("ref-filter: add --no-contains option to
>> tag/branch/for-each-ref", 2017-03-24).
>>
>> Upstream just worked around it by patching git and didn't tell us
>> about it, I discovered this when reading various Git packaging
>> implementations: https://github.com/openbsd/ports/commit/7e48bf88a20
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason
>> ---
>>
>> David, it would be great to get a quick bug report to
>> git@vger.kernel.org if you end up having to monkeypatch something
>> we've done. We won't bite, promise :)
>>
>> As shown in that linked Github commit OpenBSD has another recent
>> workaround in turning on DIR_HAS_BSD_GROUP_SEMANTICS and skipping a
>> related test, maybe René can make more sense of that?
>
> I've confirmed using the NetBSD 7.1 man pages that NetBSD will also want
> DIR_HAS_BSD_GROUP_SEMANTICS. MirBSD will also, according to its man
> pages.
>
> As I understand it, the only consequence of not setting this flag on BSD
> systems is that some directories will be setgid, which, while ugly and
> useless, should have no negative effect.
Right; specifically it's for newly created subdirectories of shared
directories, which we want to be owned by the same group as their
parent. That's the default on BSDs, and we have to set the setgid bit to
turn on that semantic only on other systems, e.g. on Linux.
81a24b52c1 (Do not use GUID on dir in git init --shared=all on FreeBSD)
introduced DIR_HAS_BSD_GROUP_SEMANTICS with the rationale that setting
setgid on directories may not even be allowed for normal users. I can't
reproduce this (t1301 succeeds for me on FreeBSD 10.3 even without that
build flag), but apparently at least in some configurations it's not just
a cosmetic issue.
The skipped test 'init in long base path' in t0001 is a different kettle
of fish. getcwd(3) on OpenBSD respects permissions on the parent
directories up to root. E.g. after "chmod 711 /home" normal users would
get EACCES when they'd call getcwd(3) in their homes there, while e.g.
on Linux and FreeBSD they'd successfully get their current working dir.
René
Re: [PATCH] tests: don't give unportable ">" to "test" built-in, use -gt
Ævar Arnfjörð Bjarmason writes:
> Change an argument to test_line_count (which'll ultimately be turned
> into a "test" expression) to use "-gt" instead of ">" for an
> arithmetic test.
>
> This broken on e.g. OpenBSD as of v2.13.0 with my commit
> ac3f5a3468 ("ref-filter: add --no-contains option to
> tag/branch/for-each-ref", 2017-03-24).
>
> Upstream just worked around it by patching git and didn't tell us
> about it, I discovered this when reading various Git packaging
> implementations: https://github.com/openbsd/ports/commit/7e48bf88a20
Thanks for finding and relaying this.
> Signed-off-by: Ævar Arnfjörð Bjarmason
> ---
>
> David, it would be great to get a quick bug report to
> git@vger.kernel.org if you end up having to monkeypatch something
> we've done. We won't bite, promise :)
Yeah. I was hoping that your new list for platform porters would
help communicate these issues easier for those who are a bit shy to
be on this general development list ;-)
> As shown in that linked Github commit OpenBSD has another recent
> workaround in turning on DIR_HAS_BSD_GROUP_SEMANTICS and skipping a
> related test, maybe René can make more sense of that?
>
> There's more patches in their ports which indicate possible bugs of
> ours: https://github.com/openbsd/ports/tree/master/devel/git/patches/
>
> t/t7004-tag.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> index 0ef7b94394..0e2e57aa3d 100755
> --- a/t/t7004-tag.sh
> +++ b/t/t7004-tag.sh
> @@ -1887,7 +1887,7 @@ EOF"
> run_with_limited_stack git tag --contains HEAD >actual &&
> test_cmp expect actual &&
> run_with_limited_stack git tag --no-contains HEAD >actual &&
> - test_line_count ">" 10 actual
> + test_line_count "-gt" 10 actual
> '
>
> test_expect_success '--format should list tags as per format given' '
Re: [PATCH] tests: don't give unportable ">" to "test" built-in, use -gt
On Sun, Aug 06, 2017 at 11:38:50PM +, Ævar Arnfjörð Bjarmason wrote:
> Change an argument to test_line_count (which'll ultimately be turned
> into a "test" expression) to use "-gt" instead of ">" for an
> arithmetic test.
>
> This broken on e.g. OpenBSD as of v2.13.0 with my commit
> ac3f5a3468 ("ref-filter: add --no-contains option to
> tag/branch/for-each-ref", 2017-03-24).
>
> Upstream just worked around it by patching git and didn't tell us
> about it, I discovered this when reading various Git packaging
> implementations: https://github.com/openbsd/ports/commit/7e48bf88a20
>
> Signed-off-by: Ævar Arnfjörð Bjarmason
> ---
>
> David, it would be great to get a quick bug report to
> git@vger.kernel.org if you end up having to monkeypatch something
> we've done. We won't bite, promise :)
>
> As shown in that linked Github commit OpenBSD has another recent
> workaround in turning on DIR_HAS_BSD_GROUP_SEMANTICS and skipping a
> related test, maybe René can make more sense of that?
I've confirmed using the NetBSD 7.1 man pages that NetBSD will also want
DIR_HAS_BSD_GROUP_SEMANTICS. MirBSD will also, according to its man
pages.
As I understand it, the only consequence of not setting this flag on BSD
systems is that some directories will be setgid, which, while ugly and
useless, should have no negative effect.
--
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204
signature.asc
Description: PGP signature
[PATCH] tests: don't give unportable ">" to "test" built-in, use -gt
Change an argument to test_line_count (which'll ultimately be turned
into a "test" expression) to use "-gt" instead of ">" for an
arithmetic test.
This broken on e.g. OpenBSD as of v2.13.0 with my commit
ac3f5a3468 ("ref-filter: add --no-contains option to
tag/branch/for-each-ref", 2017-03-24).
Upstream just worked around it by patching git and didn't tell us
about it, I discovered this when reading various Git packaging
implementations: https://github.com/openbsd/ports/commit/7e48bf88a20
Signed-off-by: Ævar Arnfjörð Bjarmason
---
David, it would be great to get a quick bug report to
git@vger.kernel.org if you end up having to monkeypatch something
we've done. We won't bite, promise :)
As shown in that linked Github commit OpenBSD has another recent
workaround in turning on DIR_HAS_BSD_GROUP_SEMANTICS and skipping a
related test, maybe René can make more sense of that?
There's more patches in their ports which indicate possible bugs of
ours: https://github.com/openbsd/ports/tree/master/devel/git/patches/
t/t7004-tag.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 0ef7b94394..0e2e57aa3d 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1887,7 +1887,7 @@ EOF"
run_with_limited_stack git tag --contains HEAD >actual &&
test_cmp expect actual &&
run_with_limited_stack git tag --no-contains HEAD >actual &&
- test_line_count ">" 10 actual
+ test_line_count "-gt" 10 actual
'
test_expect_success '--format should list tags as per format given' '
--
2.14.0.rc1.383.gd1ce394fe2
