{"thread":{"id":"31276","subject":"Test failures in t4034","startedAt":"2012-08-18T06:03:26Z","lastAt":"2012-09-03T01:53:13Z","messageCount":10,"participants":["Brian Gernhardt","Junio C Hamano","Ramsay Jones","Johannes Sixt"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"197236","messageId":"80B6C6EE-130C-48C3-BBBB-5FCD1E7EFDEF@gernhardtsoftware.com","threadId":"31276","inReplyTo":null,"subject":"Test failures in t4034","fromName":"Brian Gernhardt","fromEmail":"brian@gernhardtsoftware.com","sentAt":"2012-08-18T06:03:26Z","receivedAt":"2012-08-18T06:03:26Z","isPatch":false,"sender":{"key":"brian@gernhardtsoftware.com","avatar":"https://avatars.githubusercontent.com/u/133455?v=4"},"body":"I've been getting a couple of test failures and finally had the time to track them down.\n\nt4034-diff-words fails tests \"22 diff driver 'bibtex'\" and \"26 diff driver 'html'\". Bisecting shows that the file started giving me errors in commit 8d96e72 \"t4034: bulk verify builtin word regex sanity\", which appears to introduce those tests. I don't see anything obviously wrong with the tests and I'm not familiar with the diff-words code, so I'm not sure what's wrong.\n\nI am running on OS X 10.8, with Xcode 4.4.1 (llvm-gcc 4.2.1).\n\nTest results follow:\n\n---------- 8< ----------\n\nexpecting success: \n\t\tcp \"$TEST_DIRECTORY/t4034/bibtex/pre\" \\\n\t\t\t\"$TEST_DIRECTORY/t4034/bibtex/post\" \\\n\t\t\t\"$TEST_DIRECTORY/t4034/bibtex/expect\" . &&\n\t\techo \"* diff=bibtex\" >.gitattributes &&\n\t\tword_diff --color-words\n\t\n--- expect\t2012-08-18 05:54:29.000000000 +0000\n+++ output.decrypted\t2012-08-18 05:54:29.000000000 +0000\n@@ -8,8 +8,8 @@\n author={Aldous, D.David},\n journal={Information Theory, IEEE Transactions on},\n volume={33Bogus.},\n- number={24},\n+ number={4},\n pages={219--223},\n- year=1987,\n- note={This is in fact a rather funny read since ethernet works well in practice. The {1987\\em pre} reference is the right one, however.},\n+ year={1987},1987,\n+ note={This is in fact a rather funny read since ethernet works well in practice. The {\\em pre} reference is the right one, however.}\n }\nnot ok - 22 diff driver 'bibtex'\n\n---------- 8< ----------\n\nexpecting success: \n\t\tcp \"$TEST_DIRECTORY/t4034/html/pre\" \\\n\t\t\t\"$TEST_DIRECTORY/t4034/html/post\" \\\n\t\t\t\"$TEST_DIRECTORY/t4034/html/expect\" . &&\n\t\techo \"* diff=html\" >.gitattributes &&\n\t\tword_diff --color-words\n\t\n--- expect\t2012-08-18 05:54:29.000000000 +0000\n+++ output.decrypted\t2012-08-18 05:54:29.000000000 +0000\n@@ -4,5 +4,5 @@\n +++ b/post\n @@ -1,3 +1,3 @@\n newattr=\"newvalue\">added content\n-\"value\"\"newvalue\">contentchanged\n-<tagnewtag>content &entity;&newentity;</tag/newtag>\n+changed\n+content &newentity;\nnot ok - 26 diff driver 'html'"},{"id":"197274","messageId":"7v7gsvquk8.fsf@alter.siamese.dyndns.org","threadId":"31276","inReplyTo":"80B6C6EE-130C-48C3-BBBB-5FCD1E7EFDEF@gernhardtsoftware.com","subject":"Re: Test failures in t4034","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-19T06:12:23Z","receivedAt":"2012-08-19T06:12:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brian Gernhardt writes:\n\n> I've been getting a couple of test failures and finally had the time to track them down.\n>\n> t4034-diff-words fails tests \"22 diff driver 'bibtex'\" and \"26\n> diff driver 'html'\". Bisecting shows that the file started giving\n> me errors in commit 8d96e72 \"t4034: bulk verify builtin word regex\n> sanity\", which appears to introduce those tests. I don't see\n> anything obviously wrong with the tests and I'm not familiar with\n> the diff-words code, so I'm not sure what's wrong.\n>\n> I am running on OS X 10.8, with Xcode 4.4.1 (llvm-gcc 4.2.1).\n>\n> Test results follow:\n>\n> ---------- 8< ----------\n>\n> expecting success: \n> \t\tcp \"$TEST_DIRECTORY/t4034/bibtex/pre\" \\\n> \t\t\t\"$TEST_DIRECTORY/t4034/bibtex/post\" \\\n> \t\t\t\"$TEST_DIRECTORY/t4034/bibtex/expect\" . &&\n> \t\techo \"* diff=bibtex\" >.gitattributes &&\n> \t\tword_diff --color-words\n> \t\n> --- expect\t2012-08-18 05:54:29.000000000 +0000\n> +++ output.decrypted\t2012-08-18 05:54:29.000000000 +0000\n> @@ -8,8 +8,8 @@\n> author={Aldous, D.David},\n> journal={Information Theory, IEEE Transactions on},\n> volume={33Bogus.},\n> - number={24},\n> + number={4},\n> pages={219--223},\n> - year=1987,\n> - note={This is in fact a rather funny read since ethernet works well in practice. The {1987\\em pre} reference is the right one, however.},\n> + year={1987},1987,\n> + note={This is in fact a rather funny read since ethernet works well in practice. The {\\em pre} reference is the right one, however.}\n> }\n> not ok - 22 diff driver 'bibtex'\n\nThanks for a report. Off the top of my head, there may be three\npossibilities.\n\n (1) The compiled binary of Git is broken on your platform and not\n formatting the escape sequence correctly. I somehow think it\n is very unlikely, as the code to do so is pretty much platform\n agonistic (color.c does not use anything fancy from system\n libraries).\n\n (2) The test script, the part that converts the escape sequence to\n human readable form, is broken---not written in a portable awk.\n\n (3) The implementation of awk on your platform was broken by your\n supplier, with the same infinite wisdom they broke the UTF-8\n pathnames on their filesystem implementation with ;-)\n\nCan you help isolating the issue first to see if it is (1) or one of\nthe other two?\n\nRun \"cd t && sh t4034-diff-words -i\" to force stop the test upon the\nfirst breakage, and inspect the \"output\" before the awk script\ntest_decode_color munges it. Does it show a red number 2 and green\nnumber 4 on the line that begins with \"number=\" (or if you have an\naccess to a box on which this test passes, grab the raw output from\nit by running this test, and make byte-for-byte comparison)? \n"},{"id":"197295","messageId":"5030FD49.6060704@ramsay1.demon.co.uk","threadId":"31276","inReplyTo":"80B6C6EE-130C-48C3-BBBB-5FCD1E7EFDEF@gernhardtsoftware.com","subject":"Re: Test failures in t4034","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2012-08-19T14:50:49Z","receivedAt":"2012-08-19T14:50:49Z","isPatch":false,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Brian Gernhardt wrote:\n> I've been getting a couple of test failures and finally had the time to track them down.\n> \n> t4034-diff-words fails tests \"22 diff driver 'bibtex'\" and \"26 diff driver 'html'\". Bisecting shows that the file started giving me errors in commit 8d96e72 \"t4034: bulk verify builtin word regex sanity\", which appears to introduce those tests. I don't see anything obviously wrong with the tests and I'm not familiar with the diff-words code, so I'm not sure what's wrong.\n> \n> I am running on OS X 10.8, with Xcode 4.4.1 (llvm-gcc 4.2.1).\n\nI had the same problem (or at least it *looks* like the same problem) on Linux\nlast year (May 2011), which turned out to be a bug in the regex routines in an\nold version of glibc. \n\nI don't know OS X at all, so this may not be relevent; does OS X use glibc?\n(I didn't think so, but ...)\n\nI sent some patches to the list which may be helpful. I can't get to gmane to\nlook up a reference, but you need to search for:\n\n [RFC/PATCH] userdiff.c: Avoid old glibc regex bug causing t4034-*.sh test failures\n\nsent on 3rd May 2011.\n\nAlso, in the same thread, a reply to Jonathan Nieder on 7th May contains a\ntest which checks whether your regex routines suffer this bug.\n\nThese patches were not applied since I didn't think this would be a common\nproblem. I simply set NO_REGEX=1 in my config.mak, since the compat/ regex\nroutines don't suffer from this problem.\n\nHTH\n\nATB,\nRamsay Jones\n"},{"id":"197289","messageId":"7va9xqq0hu.fsf@alter.siamese.dyndns.org","threadId":"31276","inReplyTo":"20449AC5-D068-46CF-B8C4-E0639FB92EF6@gernhardtsoftware.com","subject":"Re: Test failures in t4034","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-19T17:01:49Z","receivedAt":"2012-08-19T17:01:49Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brian Gernhardt writes:\n\n> I wonder if there is something non-portable about the bibtex\n> filter? (The HTML filter as well, since that errors on my machine\n> too.)\n\nYeah, that I didn't think of, but is a possibility (part of (1)\nabove).\n\nThe HTML one is \"[^<>= \\t]+\" and\nthe Bibtex one is \"[={}\\\"]|[^={}\\\" \\t]+\"\n\nand both will be used with \"|[^[:space:]]|[\\xc0-\\xff][\\x80-\\xbf]+\"\nappended and given to regcomp with REG_EXTENDED|REG_NEWLINE. \n\nNothing jumps at me that is common to these two but not shared by\nother patterns.\n"},{"id":"197300","messageId":"50315C42.5060403@kdbg.org","threadId":"31276","inReplyTo":"5030FD49.6060704@ramsay1.demon.co.uk","subject":"Re: Test failures in t4034","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-08-19T21:36:02Z","receivedAt":"2012-08-19T21:36:02Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 19.08.2012 16:50, schrieb Ramsay Jones:\n> Brian Gernhardt wrote:\n>> I've been getting a couple of test failures and finally had the\n>> time to track them down.\n>> \n>> t4034-diff-words fails tests \"22 diff driver 'bibtex'\" and \"26 diff\n>> driver 'html'\". Bisecting shows that the file started giving me\n>> errors in commit 8d96e72 \"t4034: bulk verify builtin word regex\n>> sanity\", which appears to introduce those tests. I don't see\n>> anything obviously wrong with the tests and I'm not familiar with\n>> the diff-words code, so I'm not sure what's wrong.\n>> \n>> I am running on OS X 10.8, with Xcode 4.4.1 (llvm-gcc 4.2.1).\n> \n> I had the same problem (or at least it *looks* like the same problem)\n> on Linux last year (May 2011), which turned out to be a bug in the\n> regex routines in an old version of glibc.\n\nI also had the same problem, but did not remember why I don't have it\nanymore. Now that you mention it: It was the same situation and I came\nto the same conclusion (old glibc, bogus regex implementation). I worked\nit around with NO_REGEX=YesPlease in config.mak.\n\n-- Hannes\n"},{"id":"197312","messageId":"7vboi6nzym.fsf@alter.siamese.dyndns.org","threadId":"31276","inReplyTo":"5030FD49.6060704@ramsay1.demon.co.uk","subject":"Re: Test failures in t4034","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-20T00:56:17Z","receivedAt":"2012-08-20T00:56:17Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones writes:\n\n> I had the same problem (or at least it *looks* like the same problem) on Linux\n> last year (May 2011), which turned out to be a bug in the regex routines in an\n> old version of glibc. \n>\n> I don't know OS X at all, so this may not be relevent; does OS X use glibc?\n> (I didn't think so, but ...)\n>\n> I sent some patches to the list which may be helpful. I can't get to gmane to\n> look up a reference, but you need to search for:\n>\n> [RFC/PATCH] userdiff.c: Avoid old glibc regex bug causing t4034-*.sh test failures\n>\n> sent on 3rd May 2011.\n\nThanks; that's $gmane/172676 for people who prefer easier to read\nthreading interface.\n\n> Also, in the same thread, a reply to Jonathan Nieder on 7th May contains a\n> test which checks whether your regex routines suffer this bug.\n>\n> These patches were not applied since I didn't think this would be a common\n> problem. I simply set NO_REGEX=1 in my config.mak, since the compat/ regex\n> routines don't suffer from this problem.\n\nYou also said:\n\n This is an RFC because:\n - A simple fix would be for me to put NO_REGEX=1 in my config.mak,\n since the compat/regex routines don't suffer this problem.\n - I suspect this bug is old enough that it will not affect many users.\n - I have not audited the other non-matching list expressions in\n userdiff.c\n - blame, grep and pickaxe all call regcomp() with the REG_NEWLINE\n flag, but get the regex from the user (eg from command line).\n\nI think:\n\n - the second \"this is old enough\" assumption was broken again by\n Brian this week ;-)\n\n - the first \"Use NO_REGEX if your regexp library is broken\" is a\n reasonable thing to do; is this something we may want to throw\n into the platform specific section of the top-level Makefile?\n\n - among the fourth, \"blame\" and \"grep\" goes line by line, and even\n though pickaxe is primarily meant to take multi-line pattern, I\n do not think people give multi-line pattern when they use it in\n the regexp mode. So I do not think they pose a real issue even\n though they get an arbitrary pattern from the user.\n\n - the third, combined with the fact that end user can define their\n own pattern, is a killer. We cannot really afford to let broken\n regex library to break us.\n\nI think a sensible way to go in the longer term, while we wait these\nold regexp libraries die out, is to help people to avoid building\ngit without NO_REGEX on platforms where they need it.\n\nThanks for digging an old article.\n"},{"id":"197549","messageId":"5033D573.9030103@ramsay1.demon.co.uk","threadId":"31276","inReplyTo":"7vboi6nzym.fsf@alter.siamese.dyndns.org","subject":"Re: Test failures in t4034","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2012-08-21T18:37:39Z","receivedAt":"2012-08-21T18:37:39Z","isPatch":false,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Junio C Hamano wrote:\n> Ramsay Jones writes:\n> \n>> I had the same problem (or at least it *looks* like the same problem) on Linux\n>> last year (May 2011), which turned out to be a bug in the regex routines in an\n>> old version of glibc. \n>>\n>> I don't know OS X at all, so this may not be relevent; does OS X use glibc?\n>> (I didn't think so, but ...)\n>>\n>> I sent some patches to the list which may be helpful. I can't get to gmane to\n>> look up a reference, but you need to search for:\n>>\n>> [RFC/PATCH] userdiff.c: Avoid old glibc regex bug causing t4034-*.sh test failures\n>>\n>> sent on 3rd May 2011.\n> \n> Thanks; that's $gmane/172676 for people who prefer easier to read\n> threading interface.\n> \n>> Also, in the same thread, a reply to Jonathan Nieder on 7th May contains a\n>> test which checks whether your regex routines suffer this bug.\n>>\n>> These patches were not applied since I didn't think this would be a common\n>> problem. I simply set NO_REGEX=1 in my config.mak, since the compat/ regex\n>> routines don't suffer from this problem.\n> \n> You also said:\n> \n> This is an RFC because:\n> - A simple fix would be for me to put NO_REGEX=1 in my config.mak,\n> since the compat/regex routines don't suffer this problem.\n> - I suspect this bug is old enough that it will not affect many users.\n> - I have not audited the other non-matching list expressions in\n> userdiff.c\n> - blame, grep and pickaxe all call regcomp() with the REG_NEWLINE\n> flag, but get the regex from the user (eg from command line).\n> \n> I think:\n> \n> - the second \"this is old enough\" assumption was broken again by\n> Brian this week ;-)\n> \n> - the first \"Use NO_REGEX if your regexp library is broken\" is a\n> reasonable thing to do; is this something we may want to throw\n> into the platform specific section of the top-level Makefile?\n> \n> - among the fourth, \"blame\" and \"grep\" goes line by line, and even\n> though pickaxe is primarily meant to take multi-line pattern, I\n> do not think people give multi-line pattern when they use it in\n> the regexp mode. So I do not think they pose a real issue even\n> though they get an arbitrary pattern from the user.\n> \n> - the third, combined with the fact that end user can define their\n> own pattern, is a killer. We cannot really afford to let broken\n> regex library to break us.\n> \n> I think a sensible way to go in the longer term, while we wait these\n> old regexp libraries die out, is to help people to avoid building\n> git without NO_REGEX on platforms where they need it.\n\nAgreed. Did you take a look at the second patch I mentioned above?\nI've included a rebased version (onto v1.7.12) of that patch below.\n\nNOTE: I have not even attempted to compile this version of the patch\nand I can't remember how much testing I did last year, so this is\nincluded *only* for discussion purposes ...\n\nI think that, after some testing, this (or something like it) is the\nbest that we can do. What do you think?\n\nATB,\nRamsay Jones\n\n-- >8 --\nSubject: [PATCH] test-regex: Add a test to check for a bug in the regex routines\n\nSigned-off-by: Ramsay Jones \n---\n .gitignore | 1 +\n Makefile | 1 +\n t/t0070-fundamental.sh | 5 +++++\n test-regex.c | 35 +++++++++++++++++++++++++++++++++++\n 4 files changed, 42 insertions(+)\n create mode 100644 test-regex.c\n\ndiff --git a/.gitignore b/.gitignore\nindex bb5c91e..68fe464 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -189,6 +189,7 @@\n /test-mktemp\n /test-parse-options\n /test-path-utils\n+/test-regex\n /test-revision-walking\n /test-run-command\n /test-sha1\ndiff --git a/Makefile b/Makefile\nindex 6b0c961..3b760d3 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -496,6 +496,7 @@ TEST_PROGRAMS_NEED_X += test-mergesort\n TEST_PROGRAMS_NEED_X += test-mktemp\n TEST_PROGRAMS_NEED_X += test-parse-options\n TEST_PROGRAMS_NEED_X += test-path-utils\n+TEST_PROGRAMS_NEED_X += test-regex\n TEST_PROGRAMS_NEED_X += test-revision-walking\n TEST_PROGRAMS_NEED_X += test-run-command\n TEST_PROGRAMS_NEED_X += test-scrap-cache-tree\ndiff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh\nindex 9bee8bf..da2c504 100755\n--- a/t/t0070-fundamental.sh\n+++ b/t/t0070-fundamental.sh\n@@ -25,4 +25,9 @@ test_expect_success POSIXPERM 'mktemp to unwritable directory prints filename' '\n \tgrep \"cannotwrite/test\" err\n '\n \n+test_expect_success 'check for a bug in the regex routines' '\n+\t# if this test fails, re-build git with NO_REGEX=1\n+\ttest-regex\n+'\n+\n test_done\ndiff --git a/test-regex.c b/test-regex.c\nnew file mode 100644\nindex 0000000..9259985\n--- /dev/null\n+++ b/test-regex.c\n@@ -0,0 +1,35 @@\n+#include \n+#include \n+#include \n+#include \n+#include \n+\n+static void die(const char *fmt, ...)\n+{\n+\tva_list p;\n+\n+\tva_start(p, fmt);\n+\tvfprintf(stderr, fmt, p);\n+\tva_end(p);\n+\tfputc('\\n', stderr);\n+\texit(128);\n+}\n+\n+int main(int argc, char **argv)\n+{\n+\tchar *pat = \"[^={} \\t]+\";\n+\tchar *str = \"={}\\nfred\";\n+\tregex_t r;\n+\tregmatch_t m[1];\n+\n+\tif (regcomp(&r, pat, REG_EXTENDED | REG_NEWLINE))\n+\t\tdie(\"failed regcomp() for pattern '%s'\", pat);\n+\tif (regexec(&r, str, 1, m, 0))\n+\t\tdie(\"no match of pattern '%s' to string '%s'\", pat, str);\n+\n+\t/* http://sourceware.org/bugzilla/show_bug.cgi?id=3957 */\n+\tif (m[0].rm_so == 3) /* matches '\\n' when it should not */\n+\t\texit(1);\n+\n+\texit(0);\n+}\n-- \n1.7.12\n"},{"id":"197564","messageId":"7v1uizdhi7.fsf@alter.siamese.dyndns.org","threadId":"31276","inReplyTo":"5033D573.9030103@ramsay1.demon.co.uk","subject":"Re: Test failures in t4034","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-21T22:09:36Z","receivedAt":"2012-08-21T22:09:36Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones writes:\n\n> I think that, after some testing, this (or something like it) is the\n> best that we can do. What do you think?\n>\n> ATB,\n> Ramsay Jones\n>\n> -- >8 --\n> Subject: [PATCH] test-regex: Add a test to check for a bug in the regex routines\n>\n> Signed-off-by: Ramsay Jones \n> ---\n> diff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh\n> index 9bee8bf..da2c504 100755\n> --- a/t/t0070-fundamental.sh\n> +++ b/t/t0070-fundamental.sh\n> @@ -25,4 +25,9 @@ test_expect_success POSIXPERM 'mktemp to unwritable directory prints filename' '\n> \tgrep \"cannotwrite/test\" err\n> '\n> \n> +test_expect_success 'check for a bug in the regex routines' '\n> +\t# if this test fails, re-build git with NO_REGEX=1\n> +\ttest-regex\n> +'\n\nOK.\n\n> test_done\n> diff --git a/test-regex.c b/test-regex.c\n> new file mode 100644\n> index 0000000..9259985\n> --- /dev/null\n> +++ b/test-regex.c\n> @@ -0,0 +1,35 @@\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +static void die(const char *fmt, ...)\n> +{\n> +\tva_list p;\n> +\n> +\tva_start(p, fmt);\n> +\tvfprintf(stderr, fmt, p);\n> +\tva_end(p);\n> +\tfputc('\\n', stderr);\n> +\texit(128);\n> +}\n\nLooks like a bit of overkill for only two call sites, whose output\nwe would never see because it is behind the test, but OK.\n\n> +int main(int argc, char **argv)\n> +{\n> +\tchar *pat = \"[^={} \\t]+\";\n> +\tchar *str = \"={}\\nfred\";\n> +\tregex_t r;\n> +\tregmatch_t m[1];\n> +\n> +\tif (regcomp(&r, pat, REG_EXTENDED | REG_NEWLINE))\n> +\t\tdie(\"failed regcomp() for pattern '%s'\", pat);\n> +\tif (regexec(&r, str, 1, m, 0))\n> +\t\tdie(\"no match of pattern '%s' to string '%s'\", pat, str);\n> +\n> +\t/* http://sourceware.org/bugzilla/show_bug.cgi?id=3957 */\n> +\tif (m[0].rm_so == 3) /* matches '\\n' when it should not */\n> +\t\texit(1);\n\nThis could be the third call site of die() that tells the user to\nbuild with NO_REGEX=1. Then \"cd t && sh t0070-fundamental.sh -i -v\" would\ngive that message directly to the user.\n\n> +\texit(0);\n> +}\n\nThanks.\n"},{"id":"198197","messageId":"5042494D.9040401@ramsay1.demon.co.uk","threadId":"31276","inReplyTo":"7v1uizdhi7.fsf@alter.siamese.dyndns.org","subject":"Re: Test failures in t4034","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2012-09-01T17:43:41Z","receivedAt":"2012-09-01T17:43:41Z","isPatch":false,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Junio C Hamano wrote:\n> Ramsay Jones writes:\n> \n\n[snip]\n\n>> diff --git a/test-regex.c b/test-regex.c\n>> new file mode 100644\n>> index 0000000..9259985\n>> --- /dev/null\n>> +++ b/test-regex.c\n>> @@ -0,0 +1,35 @@\n>> +#include \n>> +#include \n>> +#include \n>> +#include \n>> +#include \n>> +\n>> +static void die(const char *fmt, ...)\n>> +{\n>> +\tva_list p;\n>> +\n>> +\tva_start(p, fmt);\n>> +\tvfprintf(stderr, fmt, p);\n>> +\tva_end(p);\n>> +\tfputc('\\n', stderr);\n>> +\texit(128);\n>> +}\n> \n> Looks like a bit of overkill for only two call sites, whose output\n> we would never see because it is behind the test, but OK.\n\nYes, there was a net increase in the line count when I introduced\ndie(), but the main program flow was less cluttered by error handling.\nThe net result looked much better, so I thought it was worth it.\n\nWhat may not be too obvious, however, is that test-regex.c was written\nto be independent of git. You should be able to compile the (single) file\non any POSIX system to determine if the system regex routines suffer this\nproblem. (It was also supposed to be quiet, unless it die()-ed, and\nprovide the result via the exit code).\n\nGiven that I'm now building it as part of git, I should have simply\n#included and used the die() routine from libgit.a\n(since I'm now *relying* on test-regex being linked with libgit.a).\n\n>> +int main(int argc, char **argv)\n>> +{\n>> +\tchar *pat = \"[^={} \\t]+\";\n>> +\tchar *str = \"={}\\nfred\";\n>> +\tregex_t r;\n>> +\tregmatch_t m[1];\n>> +\n>> +\tif (regcomp(&r, pat, REG_EXTENDED | REG_NEWLINE))\n>> +\t\tdie(\"failed regcomp() for pattern '%s'\", pat);\n>> +\tif (regexec(&r, str, 1, m, 0))\n>> +\t\tdie(\"no match of pattern '%s' to string '%s'\", pat, str);\n>> +\n>> +\t/* http://sourceware.org/bugzilla/show_bug.cgi?id=3957 */\n>> +\tif (m[0].rm_so == 3) /* matches '\\n' when it should not */\n>> +\t\texit(1);\n> \n> This could be the third call site of die() that tells the user to\n> build with NO_REGEX=1. Then \"cd t && sh t0070-fundamental.sh -i -v\" would\n> give that message directly to the user.\n\nHmm, even without \"-i -v\", it's *very* clear what is going on, but sure\nit wouldn't hurt either. (Also, I wanted to be able to distinguish an exit\nvia die() from a \"test failed\" error return).\n\nSo, new (tested) version of the patch comming.\n\nATB,\nRamsay Jones\n"},{"id":"198239","messageId":"7vehmjg9di.fsf@alter.siamese.dyndns.org","threadId":"31276","inReplyTo":"5042494D.9040401@ramsay1.demon.co.uk","subject":"Re: Test failures in t4034","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-03T01:53:13Z","receivedAt":"2012-09-03T01:53:13Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones writes:\n\n> Yes, there was a net increase in the line count when I introduced\n> die(), but the main program flow was less cluttered by error handling.\n> The net result looked much better, so I thought it was worth it.\n>\n> What may not be too obvious, however, is that test-regex.c was written\n> to be independent of git.\n\nThat part I was very aware of actually; it it is a bit tricky to\ntell what the right thing to do, though. Your test itself needs to\nbe pretty much portable without the portability help you would get\ngit-compat-util.h, but the point of this kind of test is to tell if\nyou want to define preprocessor macros that may affect the behaviour\nof such compatibility layer ;-)\n\n> Given that I'm now building it as part of git, I should have simply\n> #included and used the die() routine from libgit.a\n> (since I'm now *relying* on test-regex being linked with libgit.a).\n\nOK.\n\n>>> +int main(int argc, char **argv)\n>>> +{\n>>> +\tchar *pat = \"[^={} \\t]+\";\n>>> +\tchar *str = \"={}\\nfred\";\n>>> +\tregex_t r;\n>>> +\tregmatch_t m[1];\n>>> +\n>>> +\tif (regcomp(&r, pat, REG_EXTENDED | REG_NEWLINE))\n>>> +\t\tdie(\"failed regcomp() for pattern '%s'\", pat);\n>>> +\tif (regexec(&r, str, 1, m, 0))\n>>> +\t\tdie(\"no match of pattern '%s' to string '%s'\", pat, str);\n>>> +\n>>> +\t/* http://sourceware.org/bugzilla/show_bug.cgi?id=3957 */\n>>> +\tif (m[0].rm_so == 3) /* matches '\\n' when it should not */\n>>> +\t\texit(1);\n>> \n>> This could be the third call site of die() that tells the user to\n>> build with NO_REGEX=1. Then \"cd t && sh t0070-fundamental.sh -i -v\" would\n>> give that message directly to the user.\n>\n> Hmm, even without \"-i -v\", it's *very* clear what is going on, but sure\n> it wouldn't hurt either. (Also, I wanted to be able to distinguish an exit\n> via die() from a \"test failed\" error return).\n\nOK.\n\nThanks.\n"}]}