{"thread":{"id":"57204","subject":"[PATCH] hook: test -a|o is not POSIX","startedAt":"2022-01-07T04:03:46Z","lastAt":"2022-01-09T18:32:28Z","messageCount":5,"participants":["Issam Maghni via GitGitGadget","Junio C Hamano","Issam E. Maghni","Bagas Sanjaya"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"445690","messageId":"pull.1172.git.git.1641528221530.gitgitgadget@gmail.com","threadId":"57204","inReplyTo":null,"subject":"[PATCH] hook: test -a|o is not POSIX","fromName":"Issam Maghni via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-07T04:03:41Z","receivedAt":"2022-01-07T04:03:46Z","isPatch":true,"sender":{"key":"name:Issam Maghni","avatar":null},"body":"From: \"Issam E. Maghni\" \n\nI faced `test: too many arguments` when building using sbase [1]\n\nThis is due to a non-POSIX syntax `test ... -a ...` and `test … -o …`.\n\n> The XSI extensions specifying the -a and -o binary primaries and the\n> '(' and ')' operators have been marked obsolescent.\n[2]\n\n[1] https://core.suckless.org/sbase/\n[2] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html\n\nSigned-off-by: Issam E. Maghni \n---\n shell: test -a|o is not POSIX\n \n I faced test: too many arguments when building using sbase\n [https://core.suckless.org/sbase/]. This is due to a non-POSIX syntax\n test ... -a ... and test … -o ….\n \n > The XSI extensions specifying the -a and -o binary primaries and the\n > '(' and ')' operators have been marked obsolescent.\n \n https://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1172%2Fconcatime%2Fpatch-1-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1172/concatime/patch-1-v1\nPull-Request: https://github.com/git/git/pull/1172\n\n templates/hooks--update.sample | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/templates/hooks--update.sample b/templates/hooks--update.sample\nindex c4d426bc6ee..6cc46ebcf3a 100755\n--- a/templates/hooks--update.sample\n+++ b/templates/hooks--update.sample\n@@ -37,7 +37,7 @@ if [ -z \"$GIT_DIR\" ]; then\n \texit 1\n fi\n \n-if [ -z \"$refname\" -o -z \"$oldrev\" -o -z \"$newrev\" ]; then\n+if [ -z \"$refname\" ] || [ -z \"$oldrev\" ] || [ -z \"$newrev\" ]; then\n \techo \"usage: $0 \" >&2\n \texit 1\n fi\n@@ -95,7 +95,7 @@ case \"$refname\",\"$newrev_type\" in\n \t\t;;\n \trefs/heads/*,commit)\n \t\t# branch\n-\t\tif [ \"$oldrev\" = \"$zero\" -a \"$denycreatebranch\" = \"true\" ]; then\n+\t\tif [ \"$oldrev\" = \"$zero\" ] && [ \"$denycreatebranch\" = \"true\" ]; then\n \t\t\techo \"*** Creating a branch is not allowed in this repository\" >&2\n \t\t\texit 1\n \t\tfi\n\nbase-commit: 2ae0a9cb8298185a94e5998086f380a355dd8907\n-- \ngitgitgadget\n"},{"id":"445750","messageId":"xmqqr19j6x92.fsf@gitster.g","threadId":"57204","inReplyTo":"pull.1172.git.git.1641528221530.gitgitgadget@gmail.com","subject":"Re: [PATCH] hook: test -a|o is not POSIX","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-01-07T21:39:53Z","receivedAt":"2022-01-07T21:39:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Issam Maghni via GitGitGadget\" writes:\n\n> From: \"Issam E. Maghni\" \n>\n> I faced `test: too many arguments` when building using sbase [1]\n\nBuilding \"git\" with sbase? It is curious how a loosely written\nsample hook script can cause build failures on a platform with a\nmore strict userspace.\n\n> This is due to a non-POSIX syntax `test ... -a ...` and `test … -o …`.\n\nThat's all correct. The formatting of the above line feels a bit\noff, though.\n\n>> The XSI extensions specifying the -a and -o binary primaries and the\n>> '(' and ')' operators have been marked obsolescent.\n> [2]\n>\n> [1] https://core.suckless.org/sbase/\n> [2] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html\n>\n\nPerhaps...\n\n\tThe sample update script in the templates directory uses the\n\t`-o` and `-a` binary primaries of the \"test\" command, which\n\tare marked obsolescent in the recent versions of POSIX.\n\n...would be sufficient, as 'sbase' would not be the only source of\nthe userspace whose 'test' lack the -o/-a primaries.\n"},{"id":"445758","messageId":"1303411829.1252938.1641594678116@office.mailbox.org","threadId":"57204","inReplyTo":"xmqqr19j6x92.fsf@gitster.g","subject":"Re: [PATCH] hook: test -a|o is not POSIX","fromName":"Issam E. Maghni","fromEmail":"issam.e.maghni@mailbox.org","sentAt":"2022-01-07T22:31:18Z","receivedAt":"2022-01-07T22:31:28Z","isPatch":true,"sender":{"key":"issam.e.maghni@mailbox.org","avatar":null},"body":"\n> On 01/07/2022 4:39 PM Junio C Hamano wrote:\n> \n> \"Issam Maghni via GitGitGadget\" writes:\n> \n> > From: \"Issam E. Maghni\" \n> >\n> > I faced `test: too many arguments` when building using sbase [1]\n> \n> Building \"git\" with sbase? It is curious how a loosely written\n> sample hook script can cause build failures on a platform with a\n> more strict userspace.\n\nSorry, I rather meant using git with sbase. I’ve submitted at least a\nhalf dozen patches similar to this one. My brain is in auto-mode.\n\n> Perhaps...\n> \n> \tThe sample update script in the templates directory uses the\n> \t`-o` and `-a` binary primaries of the \"test\" command, which\n> \tare marked obsolescent in the recent versions of POSIX.\n> \n> ...would be sufficient, as 'sbase' would not be the only source of\n> the userspace whose 'test' lack the -o/-a primaries.\n\nYou’re right, this is much cleaner.\n\nI force-pushed the changes in https://github.com/git/git/pull/1172.\nI must admit, I’m not familiar (yet) to submitting patches through\nmailing lists.\n"},{"id":"445808","messageId":"9c7aeb5b-87b2-a51e-c08d-8dfc75ecf588@gmail.com","threadId":"57204","inReplyTo":"pull.1172.git.git.1641528221530.gitgitgadget@gmail.com","subject":"Re: [PATCH] hook: test -a|o is not POSIX","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2022-01-09T08:14:11Z","receivedAt":"2022-01-09T08:26:09Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"On 07/01/22 11.03, Issam Maghni via GitGitGadget wrote:\n> From: \"Issam E. Maghni\" \n> \n> I faced `test: too many arguments` when building using sbase [1]\n> \n> This is due to a non-POSIX syntax `test ... -a ...` and `test … -o …`.\n> \n>> The XSI extensions specifying the -a and -o binary primaries and the\n>> '(' and ')' operators have been marked obsolescent.\n> [2]\n> \n> [1] https://core.suckless.org/sbase/\n> [2] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html\n> \n> Signed-off-by: Issam E. Maghni \n> ---\n> shell: test -a|o is not POSIX\n> \n> I faced test: too many arguments when building using sbase\n> [https://core.suckless.org/sbase/]. This is due to a non-POSIX syntax\n> test ... -a ... and test … -o ….\n> \n> > The XSI extensions specifying the -a and -o binary primaries and the\n> > '(' and ')' operators have been marked obsolescent.\n> \n> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html\n> \n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1172%2Fconcatime%2Fpatch-1-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1172/concatime/patch-1-v1\n> Pull-Request: https://github.com/git/git/pull/1172\n> \n> templates/hooks--update.sample | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/templates/hooks--update.sample b/templates/hooks--update.sample\n> index c4d426bc6ee..6cc46ebcf3a 100755\n> --- a/templates/hooks--update.sample\n> +++ b/templates/hooks--update.sample\n> @@ -37,7 +37,7 @@ if [ -z \"$GIT_DIR\" ]; then\n> \texit 1\n> fi\n> \n> -if [ -z \"$refname\" -o -z \"$oldrev\" -o -z \"$newrev\" ]; then\n> +if [ -z \"$refname\" ] || [ -z \"$oldrev\" ] || [ -z \"$newrev\" ]; then\n> \techo \"usage: $0 \" >&2\n> \texit 1\n> fi\n> @@ -95,7 +95,7 @@ case \"$refname\",\"$newrev_type\" in\n> \t\t;;\n> \trefs/heads/*,commit)\n> \t\t# branch\n> -\t\tif [ \"$oldrev\" = \"$zero\" -a \"$denycreatebranch\" = \"true\" ]; then\n> +\t\tif [ \"$oldrev\" = \"$zero\" ] && [ \"$denycreatebranch\" = \"true\" ]; then\n> \t\t\techo \"*** Creating a branch is not allowed in this repository\" >&2\n> \t\t\texit 1\n> \t\tfi\n> \n> base-commit: 2ae0a9cb8298185a94e5998086f380a355dd8907\n\n From the patch above, the title should be \"replace non-POSIX test -a & test -o with logical operator\".\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"},{"id":"445810","messageId":"xmqqmtk43glm.fsf@gitster.g","threadId":"57204","inReplyTo":"9c7aeb5b-87b2-a51e-c08d-8dfc75ecf588@gmail.com","subject":"Re: [PATCH] hook: test -a|o is not POSIX","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-01-09T18:32:21Z","receivedAt":"2022-01-09T18:32:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bagas Sanjaya writes:\n\n>> -\t\tif [ \"$oldrev\" = \"$zero\" -a \"$denycreatebranch\" = \"true\" ]; then\n>> +\t\tif [ \"$oldrev\" = \"$zero\" ] && [ \"$denycreatebranch\" = \"true\" ]; then\n>> \t\t\techo \"*** Creating a branch is not allowed in this repository\" >&2\n>> \t\t\texit 1\n>> \t\tfi\n>> base-commit: 2ae0a9cb8298185a94e5998086f380a355dd8907\n>\n> From the patch above, the title should be \"replace non-POSIX test -a & test -o with logical operator\".\n\nI think \"should\" is a bit too strong a word. While yours is a bit\nmore explicit on _what_ the solution is, \"hook: test -a|o is not\nPOSIX\" already implies that we are fixing that non-posix-ness by\nrewriting, and it is obvious (cf. Documentation/CodingGuidelines)\nwhat the right rewrite should be.\n\nOne thing the original does a bit better tha yours is that it tell\nus _where_ the problem is. So, perhaps\n\n sample hook: use \"test ... &&/|| test ...\" instead of \"test -a/-o\"\n\nBut I find the original just fine.\n\n\n"}]}