{"thread":{"id":"36316","subject":"AIX fixes","startedAt":"2014-03-29T15:38:59Z","lastAt":"2014-03-31T17:35:48Z","messageCount":6,"participants":["Charles Bailey","Jens Lehmann","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"238051","messageId":"1396107541-22974-1-git-send-email-cbailey32@bloomberg.net","threadId":"36316","inReplyTo":null,"subject":"AIX fixes","fromName":"Charles Bailey","fromEmail":"cbailey32@bloomberg.net","sentAt":"2014-03-29T15:38:59Z","receivedAt":"2014-03-29T15:38:59Z","isPatch":false,"sender":{"key":"cbailey32@bloomberg.net","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"[PATCH 1/2] Remove inline from git_fnmatch in dir.c\n\nThere are currently a few issues with building on AIX. These two patches\naddress two of them. The first removes 'inline' from a function in\ndir.c. The function has grown such that I don't really see a benefit in\nexplicitly encouraging the compiler to inline. (As it is in a .c file, it's\nonly going to be inlined for sophisticated toolchains doing LTO or\nsimilar for other translation units and with this sophistication the\ninline hinting behaviour is probably not so important.)\n\nThe problem with having this function declared inline is a compliance\nissue.\n\n6.7.4 of C99 says:\n> An inline definition of a function with external linkage shall not\n> contain a definition of a modifiable object with static storage\n> duration, and shall not contain a reference to an identifier with\n> internal linkage.\n\ngit_fnmatch contains calls to ps_strncmp and ps_strcmp which are all\ndeclared static so violate this and xlC complains about this.\n\n[PATCH 2/2] Don't rely on strerror text when testing rmdir failure\n\nThe second issue is that AIX doesn't distinguish between EEXIST and\nWNOTEMPTY so two tests that rely on the exact text of strerror for\nrmdir's failure to remove a non-empty directory fail. My personal take\nwas that the exact text of strerror was not too important but we can\ntest the leading portion of the error message which is under the control\nof Git and verifies that the readdir function reported a failure.\n\nI also have an issue where two low level tests (t0020-crlf and\nt0022-crlf-rename) fail (and perhaps later tests, too) unless I\nde-inline ce_path_match and dir_path_match from dir.h but as I cannot\nyet explain what the issue is or why this is a fix, I'm holding onto\nthis one for now.\n\nCharles.\n"},{"id":"238052","messageId":"1396107541-22974-2-git-send-email-cbailey32@bloomberg.net","threadId":"36316","inReplyTo":"1396107541-22974-1-git-send-email-cbailey32@bloomberg.net","subject":"[PATCH 1/2] Remove inline from git_fnmatch in dir.c","fromName":"Charles Bailey","fromEmail":"cbailey32@bloomberg.net","sentAt":"2014-03-29T15:39:00Z","receivedAt":"2014-03-29T15:39:00Z","isPatch":true,"sender":{"key":"cbailey32@bloomberg.net","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"Now that it calls a static inline function, it cannot be an inline\ndefinition with external linkage. Remove inline and make it an\nexternal definition.\n\nSigned-off-by: Charles Bailey <cbailey32@bloomberg.net>\n---\n dir.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 99f5303..eb6f581 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -54,9 +54,9 @@ int fnmatch_icase(const char *pattern, const char *string, int flags)\n \t\t\t NULL);\n }\n \n-inline int git_fnmatch(const struct pathspec_item *item,\n-\t\t       const char *pattern, const char *string,\n-\t\t       int prefix)\n+int git_fnmatch(const struct pathspec_item *item,\n+\t\tconst char *pattern, const char *string,\n+\t\tint prefix)\n {\n \tif (prefix > 0) {\n \t\tif (ps_strncmp(item, pattern, string, prefix))\n-- \n1.8.5.1.2.ge5d1dab\n"},{"id":"238053","messageId":"1396107541-22974-3-git-send-email-cbailey32@bloomberg.net","threadId":"36316","inReplyTo":"1396107541-22974-1-git-send-email-cbailey32@bloomberg.net","subject":"[PATCH 2/2] Don't rely on strerror text when testing rmdir failure","fromName":"Charles Bailey","fromEmail":"cbailey32@bloomberg.net","sentAt":"2014-03-29T15:39:01Z","receivedAt":"2014-03-29T15:39:01Z","isPatch":true,"sender":{"key":"cbailey32@bloomberg.net","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"AIX doesn't make a distiction between EEXIST and ENOTEMPTY so relying on\nthe strerror string for the rmdir failure is fragile. Just test that the\nstart of the string matches the Git controlled \"failed to rmdir...\"\nerror. The exact text of the OS generated error string isn't important\nto the test.\n\nSigned-off-by: Charles Bailey <cbailey32@bloomberg.net>\n---\n t/t3600-rm.sh | 5 ++---\n t/t7001-mv.sh | 3 +--\n 2 files changed, 3 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t3600-rm.sh b/t/t3600-rm.sh\nindex 3d30581..23eed17 100755\n--- a/t/t3600-rm.sh\n+++ b/t/t3600-rm.sh\n@@ -709,10 +709,9 @@ test_expect_success 'checking out a commit after submodule removal needs manual\n \tgit commit -m \"submodule removal\" submod &&\n \tgit checkout HEAD^ &&\n \tgit submodule update &&\n-\tgit checkout -q HEAD^ 2>actual &&\n+\tgit checkout -q HEAD^ 2>/dev/null &&\n \tgit checkout -q master 2>actual &&\n-\techo \"warning: unable to rmdir submod: Directory not empty\" >expected &&\n-\ttest_i18ncmp expected actual &&\n+\ttest_i18ngrep \"^warning: unable to rmdir submod:\" actual &&\n \tgit status -s submod >actual &&\n \techo \"?? submod/\" >expected &&\n \ttest_cmp expected actual &&\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex 215d43d..34fb1af 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -447,8 +447,7 @@ test_expect_success 'checking out a commit before submodule moved needs manual u\n \tgit mv sub sub2 &&\n \tgit commit -m \"moved sub to sub2\" &&\n \tgit checkout -q HEAD^ 2>actual &&\n-\techo \"warning: unable to rmdir sub2: Directory not empty\" >expected &&\n-\ttest_i18ncmp expected actual &&\n+\ttest_i18ngrep \"^warning: unable to rmdir sub2:\" actual &&\n \tgit status -s sub2 >actual &&\n \techo \"?? sub2/\" >expected &&\n \ttest_cmp expected actual &&\n-- \n1.8.5.1.2.ge5d1dab\n"},{"id":"238055","messageId":"5336EB5C.6040103@web.de","threadId":"36316","inReplyTo":"1396107541-22974-3-git-send-email-cbailey32@bloomberg.net","subject":"Re: [PATCH 2/2] Don't rely on strerror text when testing rmdir failure","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2014-03-29T15:48:44Z","receivedAt":"2014-03-29T15:48:44Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 29.03.2014 16:39, schrieb Charles Bailey:\n> AIX doesn't make a distiction between EEXIST and ENOTEMPTY so relying on\n> the strerror string for the rmdir failure is fragile. Just test that the\n> start of the string matches the Git controlled \"failed to rmdir...\"\n> error. The exact text of the OS generated error string isn't important\n> to the test.\n\nMakes sense.\n\n> Signed-off-by: Charles Bailey <cbailey32@bloomberg.net>\n> ---\n>  t/t3600-rm.sh | 5 ++---\n>  t/t7001-mv.sh | 3 +--\n>  2 files changed, 3 insertions(+), 5 deletions(-)\n> \n> diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh\n> index 3d30581..23eed17 100755\n> --- a/t/t3600-rm.sh\n> +++ b/t/t3600-rm.sh\n> @@ -709,10 +709,9 @@ test_expect_success 'checking out a commit after submodule removal needs manual\n>  \tgit commit -m \"submodule removal\" submod &&\n>  \tgit checkout HEAD^ &&\n>  \tgit submodule update &&\n> -\tgit checkout -q HEAD^ 2>actual &&\n> +\tgit checkout -q HEAD^ 2>/dev/null &&\n\nIsn't this unrelated to the strerror issue you are fixing here?\nWhy not just drop the redirection completely? But maybe I'm just\nbeing to pedantic here ;-)\n\n>  \tgit checkout -q master 2>actual &&\n> -\techo \"warning: unable to rmdir submod: Directory not empty\" >expected &&\n> -\ttest_i18ncmp expected actual &&\n> +\ttest_i18ngrep \"^warning: unable to rmdir submod:\" actual &&\n>  \tgit status -s submod >actual &&\n>  \techo \"?? submod/\" >expected &&\n>  \ttest_cmp expected actual &&\n> diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\n> index 215d43d..34fb1af 100755\n> --- a/t/t7001-mv.sh\n> +++ b/t/t7001-mv.sh\n> @@ -447,8 +447,7 @@ test_expect_success 'checking out a commit before submodule moved needs manual u\n>  \tgit mv sub sub2 &&\n>  \tgit commit -m \"moved sub to sub2\" &&\n>  \tgit checkout -q HEAD^ 2>actual &&\n> -\techo \"warning: unable to rmdir sub2: Directory not empty\" >expected &&\n> -\ttest_i18ncmp expected actual &&\n> +\ttest_i18ngrep \"^warning: unable to rmdir sub2:\" actual &&\n>  \tgit status -s sub2 >actual &&\n>  \techo \"?? sub2/\" >expected &&\n>  \ttest_cmp expected actual &&\n> \n"},{"id":"238056","messageId":"20140329155817.GA23238@hashpling.org","threadId":"36316","inReplyTo":"5336EB5C.6040103@web.de","subject":"Re: [PATCH 2/2] Don't rely on strerror text when testing rmdir failure","fromName":"Charles Bailey","fromEmail":"cbailey32@bloomberg.net","sentAt":"2014-03-29T15:58:17Z","receivedAt":"2014-03-29T15:58:17Z","isPatch":true,"sender":{"key":"cbailey32@bloomberg.net","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"On Sat, Mar 29, 2014 at 04:48:44PM +0100, Jens Lehmann wrote:\n> Am 29.03.2014 16:39, schrieb Charles Bailey:\n> > diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh\n> > index 3d30581..23eed17 100755\n> > --- a/t/t3600-rm.sh\n> > +++ b/t/t3600-rm.sh\n> > @@ -709,10 +709,9 @@ test_expect_success 'checking out a commit after submodule removal needs manual\n> >  \tgit commit -m \"submodule removal\" submod &&\n> >  \tgit checkout HEAD^ &&\n> >  \tgit submodule update &&\n> > -\tgit checkout -q HEAD^ 2>actual &&\n> > +\tgit checkout -q HEAD^ 2>/dev/null &&\n> \n> Isn't this unrelated to the strerror issue you are fixing here?\n> Why not just drop the redirection completely? But maybe I'm just\n> being to pedantic here ;-)\n\nWell, it's a write to 'actual' and the next thing that tests the\ncontents of 'actual' is the thing that I'm fixing so it's almost\nrelated. (See context kept below.)\n\nI changed the redirection here while investigating the bug. The\nredirected output was being overwritten immediately and this was a\nmore explicit way to write \"I don't care about whatever goes to stderr\nfrom this command\" which confused me less that merely overwriting the\nfile on the next line, but perhaps simply not redirecting is better. I\nreally didn't give it much thought.\n\n> \n> >  \tgit checkout -q master 2>actual &&\n> > -\techo \"warning: unable to rmdir submod: Directory not empty\" >expected &&\n> > -\ttest_i18ncmp expected actual &&\n> > +\ttest_i18ngrep \"^warning: unable to rmdir submod:\" actual &&\n\nCharles.\n"},{"id":"238115","messageId":"xmqqvbuu6zkb.fsf@gitster.dls.corp.google.com","threadId":"36316","inReplyTo":"5336EB5C.6040103@web.de","subject":"Re: [PATCH 2/2] Don't rely on strerror text when testing rmdir failure","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-31T17:35:48Z","receivedAt":"2014-03-31T17:35:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann <Jens.Lehmann@web.de> writes:\n\n> Am 29.03.2014 16:39, schrieb Charles Bailey:\n>> AIX doesn't make a distiction between EEXIST and ENOTEMPTY so relying on\n>> the strerror string for the rmdir failure is fragile. Just test that the\n>> start of the string matches the Git controlled \"failed to rmdir...\"\n>> error. The exact text of the OS generated error string isn't important\n>> to the test.\n>\n> Makes sense.\n>\n>> Signed-off-by: Charles Bailey <cbailey32@bloomberg.net>\n>> ---\n>>  t/t3600-rm.sh | 5 ++---\n>>  t/t7001-mv.sh | 3 +--\n>>  2 files changed, 3 insertions(+), 5 deletions(-)\n>> \n>> diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh\n>> index 3d30581..23eed17 100755\n>> --- a/t/t3600-rm.sh\n>> +++ b/t/t3600-rm.sh\n>> @@ -709,10 +709,9 @@ test_expect_success 'checking out a commit after submodule removal needs manual\n>>  \tgit commit -m \"submodule removal\" submod &&\n>>  \tgit checkout HEAD^ &&\n>>  \tgit submodule update &&\n>> -\tgit checkout -q HEAD^ 2>actual &&\n>> +\tgit checkout -q HEAD^ 2>/dev/null &&\n>\n> Isn't this unrelated to the strerror issue you are fixing here?\n> Why not just drop the redirection completely? But maybe I'm just\n> being to pedantic here ;-)\n\nNo, that sounds like a very reasonable suggestion.  Especially given\nthat the redirection destination is overwritten immediately after.\n\nIn general tests should not have to squelch their standard error\noutput with 2>/dev/null; that is a job for the test harness, and\nthey will be shown in the output of \"./t3600-rm -v\" to serve as\nanchor point while finding where a test goes wrong, which is a good\nthing.\n"}]}