{"thread":{"id":"32352","subject":"[PATCH 1/1] tests: Allow customization of how say_color() prints","startedAt":"2012-12-15T19:12:06Z","lastAt":"2012-12-18T01:42:30Z","messageCount":4,"participants":["Ramsay Jones","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"204939","messageId":"50CCCB86.5080701@ramsay1.demon.co.uk","threadId":"32352","inReplyTo":null,"subject":"[PATCH 1/1] tests: Allow customization of how say_color() prints","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2012-12-15T19:12:06Z","receivedAt":"2012-12-15T19:12:06Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nSince commit 7bc0911d (\"test-lib: Fix say_color () not to interpret\n\\a\\b\\c in the message\", 11-10-2012), the \"--no-color\" version of\nsay_color() has been using the (bash builtin) printf function, rather\nthan echo, to print the testsuite output. Due to an intermittent (and\nrare) failure of the printf builtin function on some older versions\nof cygwin, this leads to several (currently 7) test failures.\n\nIn order the fix the test failures, we provide a means to customize\nthe function used by say_color() to print the output. The function\nis customized using GIT_TEST_PRINT[_LN] variables, which are set by\ndefault to keep the current behaviour unchanged, but may used from\n(say) the config.mak file to re-instate the use of echo. This could\nbe done by adding the following to config.mak:\n\n    GIT_TEST_PRINT=echo -nE\n    GIT_TEST_PRINT_LN=echo -E\n    export GIT_TEST_PRINT GIT_TEST_PRINT_LN\n\nNote that the GIT_TEST_PRINT variable is used in the \"--color\" version\nof say_color(), and does not provide the line termination character.\nIn contrast, the GIT_TEST_PRINT_LN variable is used by the \"--no-color\"\nversion of say_color() and does provide the line termination.\n\nSigned-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>\n---\n Makefile      |  6 ++++++\n t/test-lib.sh | 13 +++++++++++--\n 2 files changed, 17 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 736ecd4..4f7803e 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2603,6 +2603,12 @@ GIT-BUILD-OPTIONS: FORCE\n ifdef GIT_TEST_OPTS\n \t@echo GIT_TEST_OPTS=\\''$(subst ','\\'',$(subst ','\\'',$(GIT_TEST_OPTS)))'\\' >>$@\n endif\n+ifdef GIT_TEST_PRINT\n+\t@echo GIT_TEST_PRINT=\\''$(subst ','\\'',$(subst ','\\'',$(GIT_TEST_PRINT)))'\\' >>$@\n+endif\n+ifdef GIT_TEST_PRINT_LN\n+\t@echo GIT_TEST_PRINT_LN=\\''$(subst ','\\'',$(subst ','\\'',$(GIT_TEST_PRINT_LN)))'\\' >>$@\n+endif\n ifdef GIT_TEST_CMP\n \t@echo GIT_TEST_CMP=\\''$(subst ','\\'',$(subst ','\\'',$(GIT_TEST_CMP)))'\\' >>$@\n endif\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex f50f834..9dcf3c1 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -202,6 +202,15 @@ do\n \tesac\n done\n \n+if test -z \"$GIT_TEST_PRINT\"\n+then\n+\tGIT_TEST_PRINT=\"printf %s\"\n+fi\n+if test -z \"$GIT_TEST_PRINT_LN\"\n+then\n+\tGIT_TEST_PRINT_LN=\"printf %s\\n\"\n+fi\n+\n if test -n \"$color\"\n then\n \tsay_color () {\n@@ -221,7 +230,7 @@ then\n \t\t\ttest -n \"$quiet\" && return;;\n \t\tesac\n \t\tshift\n-\t\tprintf \"%s\" \"$*\"\n+\t\t$GIT_TEST_PRINT \"$*\"\n \t\ttput sgr0\n \t\techo\n \t\t)\n@@ -230,7 +239,7 @@ else\n \tsay_color() {\n \t\ttest -z \"$1\" && test -n \"$quiet\" && return\n \t\tshift\n-\t\tprintf \"%s\\n\" \"$*\"\n+\t\t$GIT_TEST_PRINT_LN \"$*\"\n \t}\n fi\n \n-- \n1.8.0\n"},{"id":"204957","messageId":"7vobhuqzdc.fsf@alter.siamese.dyndns.org","threadId":"32352","inReplyTo":"50CCCB86.5080701@ramsay1.demon.co.uk","subject":"Re: [PATCH 1/1] tests: Allow customization of how say_color() prints","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-16T06:34:23Z","receivedAt":"2012-12-16T06:34:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:\n\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index f50f834..9dcf3c1 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -202,6 +202,15 @@ do\n>  \tesac\n>  done\n>  \n> +if test -z \"$GIT_TEST_PRINT\"\n> +then\n> +\tGIT_TEST_PRINT=\"printf %s\"\n> +fi\n> +if test -z \"$GIT_TEST_PRINT_LN\"\n> +then\n> +\tGIT_TEST_PRINT_LN=\"printf %s\\n\"\n> +fi\n> +\n>  if test -n \"$color\"\n>  then\n>  \tsay_color () {\n> @@ -221,7 +230,7 @@ then\n>  \t\t\ttest -n \"$quiet\" && return;;\n>  \t\tesac\n>  \t\tshift\n> -\t\tprintf \"%s\" \"$*\"\n> +\t\t$GIT_TEST_PRINT \"$*\"\n>  \t\ttput sgr0\n>  \t\techo\n>  \t\t)\n> @@ -230,7 +239,7 @@ else\n>  \tsay_color() {\n>  \t\ttest -z \"$1\" && test -n \"$quiet\" && return\n>  \t\tshift\n> -\t\tprintf \"%s\\n\" \"$*\"\n> +\t\t$GIT_TEST_PRINT_LN \"$*\"\n>  \t}\n>  fi\n\nAs you said, this is ugly and also unwieldy in that I do not see an\neasy way for a platform/builder to define something that needs to\npass a parameter with $IFS in it in these two variables.\n\nWhy does your printf die in the first place???\n"},{"id":"205082","messageId":"50CF9D4C.9080706@ramsay1.demon.co.uk","threadId":"32352","inReplyTo":"7vobhuqzdc.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/1] tests: Allow customization of how say_color() prints","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2012-12-17T22:31:40Z","receivedAt":"2012-12-17T22:31:40Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Junio C Hamano wrote:\n> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:\n> \n>> diff --git a/t/test-lib.sh b/t/test-lib.sh\n>> index f50f834..9dcf3c1 100644\n>> --- a/t/test-lib.sh\n>> +++ b/t/test-lib.sh\n>> @@ -202,6 +202,15 @@ do\n>>  \tesac\n>>  done\n>>  \n>> +if test -z \"$GIT_TEST_PRINT\"\n>> +then\n>> +\tGIT_TEST_PRINT=\"printf %s\"\n>> +fi\n>> +if test -z \"$GIT_TEST_PRINT_LN\"\n>> +then\n>> +\tGIT_TEST_PRINT_LN=\"printf %s\\n\"\n>> +fi\n>> +\n>>  if test -n \"$color\"\n>>  then\n>>  \tsay_color () {\n>> @@ -221,7 +230,7 @@ then\n>>  \t\t\ttest -n \"$quiet\" && return;;\n>>  \t\tesac\n>>  \t\tshift\n>> -\t\tprintf \"%s\" \"$*\"\n>> +\t\t$GIT_TEST_PRINT \"$*\"\n>>  \t\ttput sgr0\n>>  \t\techo\n>>  \t\t)\n>> @@ -230,7 +239,7 @@ else\n>>  \tsay_color() {\n>>  \t\ttest -z \"$1\" && test -n \"$quiet\" && return\n>>  \t\tshift\n>> -\t\tprintf \"%s\\n\" \"$*\"\n>> +\t\t$GIT_TEST_PRINT_LN \"$*\"\n>>  \t}\n>>  fi\n> \n> As you said, this is ugly and also unwieldy in that I do not see an\n> easy way for a platform/builder to define something that needs to\n> pass a parameter with $IFS in it in these two variables.\n\nYes, I spent 10 minutes trying to decide if I should send this patch\nat all ... (ie how much public humiliation could I take :-D )\n\n> Why does your printf die in the first place???\n\nI really don't know. I'm not sure I will ever know. A couple of years\nago, when I was trying to debug the (harmless) \"--color\" spew, I found\n(via google, etc) numerous accounts of similar problems, with various\nworkarounds for specific problems. One such account claimed that the\ncause of the problem was an official \"fix\" from Microsoft (as part of\na service pack) which worked just fine on Windows Vista (and later) but\nhad this known side-effect on XP. Since it fixed the problem it was\nintended to fix, even on XP, and the unfortunate \"side-effect\" on XP\nshould be quite rare, they decided to apply it on XP anyway. :(\n\nHmm, on reflection, it would probably be best if you just drop this patch.\nI can keep it locally and apply it on top of any branch I want to test.\n(Actually, it would be easier to simply revert commit 7bc0911d.)\n\nSorry for wasting your time.\n\nATB,\nRamsay Jones\n"},{"id":"205100","messageId":"7v38z4jfuh.fsf@alter.siamese.dyndns.org","threadId":"32352","inReplyTo":"50CF9D4C.9080706@ramsay1.demon.co.uk","subject":"Re: [PATCH 1/1] tests: Allow customization of how say_color() prints","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-18T01:42:30Z","receivedAt":"2012-12-18T01:42:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:\n\n> Junio C Hamano wrote:\n> ...\n>> Why does your printf die in the first place???\n>\n> I really don't know. ...\n>\n> Sorry for wasting your time.\n\nNot a waste. I was hoping somebody (not necessarily you) may be able\nto come up with a cleaner solution.  Unfortunately it hasn't\nhappened (yet), but discussing issues on the list is often not a\nwaste.\n\nWe could introduce git_test_print and git_test_println shell\nfunctions that default to the current \"printf\", and let the users\noverride these by including a custom scriptllet from t/test-lib.sh,\nor something.\n"}]}