[PATCH] hook: test -a|o is not POSIX

[PATCH] hook: test -a|o is not POSIX

[Issam Maghni via GitGitGadget]

From: "Issam E. Maghni" <issam.e.maghni@mailbox.org>

I faced `test: too many arguments` when building using sbase [1]

This is due to a non-POSIX syntax `test ... -a ...` and `test … -o …`.

> The XSI extensions specifying the -a and -o binary primaries and the
> '(' and ')' operators have been marked obsolescent.
[2]

[1] https://core.suckless.org/sbase/
[2] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html

Signed-off-by: Issam E. Maghni <issam.e.maghni@mailbox.org>
---
    shell: test -a|o is not POSIX
    
    I faced test: too many arguments when building using sbase
    [https://core.suckless.org/sbase/]. This is due to a non-POSIX syntax
    test ... -a ... and test … -o ….
    
    > The XSI extensions specifying the -a and -o binary primaries and the
    > '(' and ')' operators have been marked obsolescent.
    
    https://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1172%2Fconcatime%2Fpatch-1-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1172/concatime/patch-1-v1
Pull-Request: https://github.com/git/git/pull/1172

 templates/hooks--update.sample | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/templates/hooks--update.sample b/templates/hooks--update.sample
index c4d426bc6ee..6cc46ebcf3a 100755
--- a/templates/hooks--update.sample
+++ b/templates/hooks--update.sample
@@ -37,7 +37,7 @@ if [ -z "$GIT_DIR" ]; then
 	exit 1
 fi
 
-if [ -z "$refname" -o -z "$oldrev" -o -z "$newrev" ]; then
+if [ -z "$refname" ] || [ -z "$oldrev" ] || [ -z "$newrev" ]; then
 	echo "usage: $0 <ref> <oldrev> <newrev>" >&2
 	exit 1
 fi
@@ -95,7 +95,7 @@ case "$refname","$newrev_type" in
 		;;
 	refs/heads/*,commit)
 		# branch
-		if [ "$oldrev" = "$zero" -a "$denycreatebranch" = "true" ]; then
+		if [ "$oldrev" = "$zero" ] && [ "$denycreatebranch" = "true" ]; then
 			echo "*** Creating a branch is not allowed in this repository" >&2
 			exit 1
 		fi

base-commit: 2ae0a9cb8298185a94e5998086f380a355dd8907
-- 
gitgitgadget

Re: [PATCH] hook: test -a|o is not POSIX

[Junio C Hamano]

"Issam Maghni via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: "Issam E. Maghni" <issam.e.maghni@mailbox.org>
>
> I faced `test: too many arguments` when building using sbase [1]

Building "git" with sbase?  It is curious how a loosely written
sample hook script can cause build failures on a platform with a
more strict userspace.

> This is due to a non-POSIX syntax `test ... -a ...` and `test … -o …`.

That's all correct.  The formatting of the above line feels a bit
off, though.

>> The XSI extensions specifying the -a and -o binary primaries and the
>> '(' and ')' operators have been marked obsolescent.
> [2]
>
> [1] https://core.suckless.org/sbase/
> [2] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html
>

Perhaps...

	The sample update script in the templates directory uses the
	`-o` and `-a` binary primaries of the "test" command, which
	are marked obsolescent in the recent versions of POSIX.

...would be sufficient, as 'sbase' would not be the only source of
the userspace whose 'test' lack the -o/-a primaries.

Re: [PATCH] hook: test -a|o is not POSIX

[Issam E. Maghni]

> On 01/07/2022 4:39 PM Junio C Hamano <gitster@pobox.com> wrote:
> 
> "Issam Maghni via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
> > From: "Issam E. Maghni" <issam.e.maghni@mailbox.org>
> >
> > I faced `test: too many arguments` when building using sbase [1]
> 
> Building "git" with sbase?  It is curious how a loosely written
> sample hook script can cause build failures on a platform with a
> more strict userspace.

Sorry, I rather meant using git with sbase. I’ve submitted at least a
half dozen patches similar to this one. My brain is in auto-mode.

> Perhaps...
> 
> 	The sample update script in the templates directory uses the
> 	`-o` and `-a` binary primaries of the "test" command, which
> 	are marked obsolescent in the recent versions of POSIX.
> 
> ...would be sufficient, as 'sbase' would not be the only source of
> the userspace whose 'test' lack the -o/-a primaries.

You’re right, this is much cleaner.

I force-pushed the changes in https://github.com/git/git/pull/1172.
I must admit, I’m not familiar (yet) to submitting patches through
mailing lists.

Re: [PATCH] hook: test -a|o is not POSIX

[Bagas Sanjaya]

On 07/01/22 11.03, Issam Maghni via GitGitGadget wrote:
> From: "Issam E. Maghni" <issam.e.maghni@mailbox.org>
> 
> I faced `test: too many arguments` when building using sbase [1]
> 
> This is due to a non-POSIX syntax `test ... -a ...` and `test … -o …`.
> 
>> The XSI extensions specifying the -a and -o binary primaries and the
>> '(' and ')' operators have been marked obsolescent.
> [2]
> 
> [1] https://core.suckless.org/sbase/
> [2] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html
> 
> Signed-off-by: Issam E. Maghni <issam.e.maghni@mailbox.org>
> ---
>      shell: test -a|o is not POSIX
>      
>      I faced test: too many arguments when building using sbase
>      [https://core.suckless.org/sbase/]. This is due to a non-POSIX syntax
>      test ... -a ... and test … -o ….
>      
>      > The XSI extensions specifying the -a and -o binary primaries and the
>      > '(' and ')' operators have been marked obsolescent.
>      
>      https://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html
> 
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1172%2Fconcatime%2Fpatch-1-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1172/concatime/patch-1-v1
> Pull-Request: https://github.com/git/git/pull/1172
> 
>   templates/hooks--update.sample | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/templates/hooks--update.sample b/templates/hooks--update.sample
> index c4d426bc6ee..6cc46ebcf3a 100755
> --- a/templates/hooks--update.sample
> +++ b/templates/hooks--update.sample
> @@ -37,7 +37,7 @@ if [ -z "$GIT_DIR" ]; then
>   	exit 1
>   fi
>   
> -if [ -z "$refname" -o -z "$oldrev" -o -z "$newrev" ]; then
> +if [ -z "$refname" ] || [ -z "$oldrev" ] || [ -z "$newrev" ]; then
>   	echo "usage: $0 <ref> <oldrev> <newrev>" >&2
>   	exit 1
>   fi
> @@ -95,7 +95,7 @@ case "$refname","$newrev_type" in
>   		;;
>   	refs/heads/*,commit)
>   		# branch
> -		if [ "$oldrev" = "$zero" -a "$denycreatebranch" = "true" ]; then
> +		if [ "$oldrev" = "$zero" ] && [ "$denycreatebranch" = "true" ]; then
>   			echo "*** Creating a branch is not allowed in this repository" >&2
>   			exit 1
>   		fi
> 
> base-commit: 2ae0a9cb8298185a94e5998086f380a355dd8907

 From the patch above, the title should be "replace non-POSIX test -a & test -o with logical operator".

-- 
An old man doll... just what I always wanted! - Clara

Re: [PATCH] hook: test -a|o is not POSIX

[Junio C Hamano]

Bagas Sanjaya <bagasdotme@gmail.com> writes:

>> -		if [ "$oldrev" = "$zero" -a "$denycreatebranch" = "true" ]; then
>> +		if [ "$oldrev" = "$zero" ] && [ "$denycreatebranch" = "true" ]; then
>>   			echo "*** Creating a branch is not allowed in this repository" >&2
>>   			exit 1
>>   		fi
>> base-commit: 2ae0a9cb8298185a94e5998086f380a355dd8907
>
> From the patch above, the title should be "replace non-POSIX test -a & test -o with logical operator".

I think "should" is a bit too strong a word.  While yours is a bit
more explicit on _what_ the solution is, "hook: test -a|o is not
POSIX" already implies that we are fixing that non-posix-ness by
rewriting, and it is obvious (cf. Documentation/CodingGuidelines)
what the right rewrite should be.

One thing the original does a bit better tha yours is that it tell
us _where_ the problem is.  So, perhaps

    sample hook: use "test ... &&/|| test ..." instead of "test -a/-o"

But I find the original just fine.