Skip to contentSkip to conversation

hook: test -a|o is not POSIX

PATCH In review Issam Maghni via GitGitGadget 4 participants 5 messages Jan 7, 2022 – Jan 9, 2022

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 hook: test -a|o is not POSIX

From: "Issam E. Maghni" <[email protected]>

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

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

Quoted 2 lines

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

---
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(-)
templates/hooks--update.sample +2 -2
index c4d426bc6ee..6cc46ebcf3a 100755
@@ -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

Signature · 1 line
gitgitgadget
#1 lore raw
Quoted 3 lines · Issam Maghni via GitGitGadget

From: "Issam E. Maghni" <[email protected]>

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.

Quoted 1 line

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.

Quoted 7 lines
Quoted 2 lines

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.

#2 lore raw
Quoted 12 lines · Junio C Hamano
Quoted 3 lines · Issam Maghni via GitGitGadget

From: "Issam E. Maghni" <[email protected]>

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.

Quoted 8 lines

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.

#3 lore raw
In reply to Issam Maghni via GitGitGadget
Quoted 56 lines · Issam Maghni via GitGitGadget

From: "Issam E. Maghni" <[email protected]>

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

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

Quoted 2 lines

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

---
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(-)
templates/hooks--update.sample +2 -2
index c4d426bc6ee..6cc46ebcf3a 100755
@@ -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
Quoted 1 line

base-commit: 2ae0a9cb8298185a94e5998086f380a355dd8907

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

Signature · 1 line
An old man doll... just what I always wanted! - Clara
#4 lore raw
Quoted 8 lines · Bagas Sanjaya
Quoted 6 lines
templates/hooks--update.sample +1 -1
-		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.