{"thread":{"id":"45945","subject":"[PATCH] fixup! log: add exhaustive tests for pattern style options & config","startedAt":"2017-05-12T18:44:53Z","lastAt":"2017-05-16T06:15:35Z","messageCount":11,"participants":["Johannes Schindelin","Junio C Hamano","Jonathan Nieder","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"319511","messageId":"92c10618c688bb8cb1f31ee2a93110c581974468.1494586245.git.johannes.schindelin@gmx.de","threadId":"45945","inReplyTo":null,"subject":"[PATCH] fixup! log: add exhaustive tests for pattern style options & config","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-05-12T10:50:55Z","receivedAt":"2017-05-12T18:44:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"On Windows, `(1|2)` is not a valid file name, and therefore the tag\ncannot be created as expected by the new test.\n\nSo simply skip this test on Windows.\n\nSigned-off-by: Johannes Schindelin \n---\nPublished-As: https://github.com/dscho/git/releases/tag/exhaustive-grep-tests-fixup-v1\nFetch-It-Via: git fetch https://github.com/dscho/git exhaustive-grep-tests-fixup-v1\n\n t/t4202-log.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 36321f19061..6f108e99b7b 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -296,7 +296,7 @@ test_expect_success 'log with grep.patternType configuration and command line' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'log with various grep.patternType configurations & command-lines' '\n+test_expect_success !MINGW 'log with various grep.patternType configurations & command-lines' '\n \tgit init pattern-type &&\n \t(\n \t\tcd pattern-type &&\n\nbase-commit: 3760a479060228867a31eed443334b30124465b9\n-- \n2.12.2.windows.2.800.gede8f145e06\n"},{"id":"319578","messageId":"xmqqtw4p4sgr.fsf@gitster.mtv.corp.google.com","threadId":"45945","inReplyTo":"92c10618c688bb8cb1f31ee2a93110c581974468.1494586245.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH] fixup! log: add exhaustive tests for pattern style options & config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-12T22:32:04Z","receivedAt":"2017-05-12T22:32:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> On Windows, `(1|2)` is not a valid file name, and therefore the tag\n> cannot be created as expected by the new test.\n>\n> So simply skip this test on Windows.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n\nWill queue as an emergency fix on 'pu' for now, but I'd expect\n(read: ask) Ævar to squash this in when the series is rerolled.\n\nThanks.\n\n> Published-As: https://github.com/dscho/git/releases/tag/exhaustive-grep-tests-fixup-v1\n> Fetch-It-Via: git fetch https://github.com/dscho/git exhaustive-grep-tests-fixup-v1\n>\n> t/t4202-log.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/t/t4202-log.sh b/t/t4202-log.sh\n> index 36321f19061..6f108e99b7b 100755\n> --- a/t/t4202-log.sh\n> +++ b/t/t4202-log.sh\n> @@ -296,7 +296,7 @@ test_expect_success 'log with grep.patternType configuration and command line' '\n> \ttest_cmp expect actual\n> '\n> \n> -test_expect_success 'log with various grep.patternType configurations & command-lines' '\n> +test_expect_success !MINGW 'log with various grep.patternType configurations & command-lines' '\n> \tgit init pattern-type &&\n> \t(\n> \t\tcd pattern-type &&\n>\n> base-commit: 3760a479060228867a31eed443334b30124465b9\n"},{"id":"319584","messageId":"20170512234414.GF27400@aiede.svl.corp.google.com","threadId":"45945","inReplyTo":"92c10618c688bb8cb1f31ee2a93110c581974468.1494586245.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH] fixup! log: add exhaustive tests for pattern style options & config","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-05-12T23:44:14Z","receivedAt":"2017-05-12T23:44:22Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Johannes Schindelin wrote:\n\n> On Windows, `(1|2)` is not a valid file name, and therefore the tag\n> cannot be created as expected by the new test.\n>\n> So simply skip this test on Windows.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n\nReviewed-by: Jonathan Nieder \n\nI wouldn't be surprised if there are filesystems used places other\nthan MINGW that also can't handle this test. Isn't this what some\ntests use a FUNNYNAMES prerequisite for?\n\nIn this example, it's the pipe that's not allowed, not the\nparenthesis, right? (At least I have some memories of naming files\nwith some parentheses.) Would something like\n\n\ttest PIPE_IN_FILENAME '\n\t\t>\"a|b\" &&\n\t\ttest -f \"a|b\"\n\t'\n\nwork?\n\nThanks,\nJonathan\n"},{"id":"319614","messageId":"CACBZZX7MXh_9mG1EROZVEEGapBwjzRWzr3S57f6rWLnQe9L+XA@mail.gmail.com","threadId":"45945","inReplyTo":"92c10618c688bb8cb1f31ee2a93110c581974468.1494586245.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH] fixup! log: add exhaustive tests for pattern style options & config","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T13:20:17Z","receivedAt":"2017-05-13T13:20:43Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Fri, May 12, 2017 at 12:50 PM, Johannes Schindelin\n wrote:\n> On Windows, `(1|2)` is not a valid file name, and therefore the tag\n> cannot be created as expected by the new test.\n>\n> So simply skip this test on Windows.\n\nThanks for the hotfix. I'll fix this in my v2, but do it differently\nin such a way that I can still run these tests on windows.\n\nI.e. the actual test here just needs these odd characters in the\ncommit message. It's just an unintended implementation detail of\ntest_commit that a tag is being created.\n\n> Signed-off-by: Johannes Schindelin \n> ---\n> Published-As: https://github.com/dscho/git/releases/tag/exhaustive-grep-tests-fixup-v1\n> Fetch-It-Via: git fetch https://github.com/dscho/git exhaustive-grep-tests-fixup-v1\n>\n> t/t4202-log.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/t/t4202-log.sh b/t/t4202-log.sh\n> index 36321f19061..6f108e99b7b 100755\n> --- a/t/t4202-log.sh\n> +++ b/t/t4202-log.sh\n> @@ -296,7 +296,7 @@ test_expect_success 'log with grep.patternType configuration and command line' '\n> test_cmp expect actual\n> '\n>\n> -test_expect_success 'log with various grep.patternType configurations & command-lines' '\n> +test_expect_success !MINGW 'log with various grep.patternType configurations & command-lines' '\n> git init pattern-type &&\n> (\n> cd pattern-type &&\n>\n> base-commit: 3760a479060228867a31eed443334b30124465b9\n> --\n> 2.12.2.windows.2.800.gede8f145e06\n"},{"id":"319615","messageId":"CACBZZX4LXkCCkHTaEw5YKsVHFcE6GsE3JRzLR5gd56DxGHxptQ@mail.gmail.com","threadId":"45945","inReplyTo":"20170512234414.GF27400@aiede.svl.corp.google.com","subject":"Re: [PATCH] fixup! log: add exhaustive tests for pattern style options & config","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-13T13:22:30Z","receivedAt":"2017-05-13T13:22:58Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sat, May 13, 2017 at 1:44 AM, Jonathan Nieder wrote:\n> Johannes Schindelin wrote:\n>\n>> On Windows, `(1|2)` is not a valid file name, and therefore the tag\n>> cannot be created as expected by the new test.\n>>\n>> So simply skip this test on Windows.\n>>\n>> Signed-off-by: Johannes Schindelin \n>> ---\n>\n> Reviewed-by: Jonathan Nieder \n>\n> I wouldn't be surprised if there are filesystems used places other\n> than MINGW that also can't handle this test. Isn't this what some\n> tests use a FUNNYNAMES prerequisite for?\n>\n> In this example, it's the pipe that's not allowed, not the\n> parenthesis, right? (At least I have some memories of naming files\n> with some parentheses.) Would something like\n>\n> test PIPE_IN_FILENAME '\n> >\"a|b\" &&\n> test -f \"a|b\"\n> '\n>\n> work?\n\nIt would, but as indicated upthread I'll just amend this so the odd\ntag/filename won't be needed, since the test doesn't actually use\nthat.\n\n(B.t.w. I meant \"[odd looking] tag or file\" in my last E-Mail in this\nthread, not just \"[odd looking tag]\")>\n"},{"id":"319629","messageId":"alpine.DEB.2.21.1.1705132037050.146734@virtualbox","threadId":"45945","inReplyTo":"20170512234414.GF27400@aiede.svl.corp.google.com","subject":"Re: [PATCH] fixup! log: add exhaustive tests for pattern style options & config","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-05-13T19:03:35Z","receivedAt":"2017-05-13T19:03:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Jonathan,\n\nOn Fri, 12 May 2017, Jonathan Nieder wrote:\n\n> Johannes Schindelin wrote:\n> \n> > On Windows, `(1|2)` is not a valid file name, and therefore the tag\n> > cannot be created as expected by the new test.\n> >\n> > So simply skip this test on Windows.\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> \n> Reviewed-by: Jonathan Nieder \n> \n> I wouldn't be surprised if there are filesystems used places other\n> than MINGW that also can't handle this test. Isn't this what some\n> tests use a FUNNYNAMES prerequisite for?\n\nSome tests do that. And they all implement that prereq themselves,\ndifferently.\n\nt3600 tries to create a file whose name contains a tab.\n\nt4135 tries to create a file whose name contains a tab, but only if the\nMINGW prereq is set.\n\nt9903 tries to create a file whose name contains a newline.\n\nIt seems, therefore, that the existing FUNNYNAMES are already in\ndisagreement, and as you point out below, the file name in question\ncontains neither a tab nor a newline. So we would introduce *yet another*\ndisagreeing FUNNYNAMES.\n\nBesides, in some hopefully not so far future, our default refs backend may\nnot depend on the current platform's file system's idionsyncracies. So\nmaybe we do not even want the MINGW nor the FUNNYNAMES prereq...\n\n> In this example, it's the pipe that's not allowed, not the\n> parenthesis, right? (At least I have some memories of naming files\n> with some parentheses.)\n\nYou would be correct in that assumptions. Long names on NTFS can contain\nparentheses.\n\n> Would something like\n> \n> \ttest PIPE_IN_FILENAME '\n> \t\t>\"a|b\" &&\n> \t\ttest -f \"a|b\"\n> \t'\n> \n> work?\n\nNo.\n\nRemember: on Windows, there is no Unix shell.\n\n(Actually, there is Bash on Ubuntu on Windows, and with the Creators\nUpdate, it became truly awesome, I am a big fan of it. But that is besides\nthe point here: Git for Windows needs to support Windows versions older\nthan Windows 10, where Bash on Ubuntu on Windows is simply unavailable.)\n\nSo what Git for Windows has to do is quite a burden: we ship with MSYS2,\nkind of a light-weight version of Cygwin, to run our shell scripts. (I\nwould wager a bet that at least a third of the bug reports relate to the\nPOSIX emulation layer we ship to allow us to run Bash and Perl, but even\nthat is only half the truth: tracking down those bugs takes easily the\nmajority of my bug hunting/fixing time, hands down.)\n\nThe test suite is no exception. I may have ranted about this about six\ndozen times already, so I'll save me the time. Just pretend that you now\nread seven long paragraphs describing how much it sucks to run the test\nsuite on Windows.\n\nTake home message: Unix shell scripting is good for personal use. Don't\nuse it in applications, not if you want to stay portable. Just don't.\n\nBack to the subject: The MSYS2 emulation layer inherits a neat trick from\nCygwin, where it *can* create file names containing pipe symbols. They\nwill be quietly mapped into a private UTF-8 page, and when Cygwin or MSYS2\nread the file back, the file name maps from this page back to ASCII\ntransparently.\n\nThat strategy is all good and dandy, as long as you stay within the POSIX\nemulation layer.\n\nGit for Windows avoids the POSIX emulation layer as much as possible, for\nspeed, and also for robustness.\n\nWhich means that Git does *not* map the file names using said private\nUTF-8 code page. And therefore, your test would succeed (because the shell\nscript would stay within the POSIX emulation layer, which creates that\nfile using above-mentioned strategy), but Git (being a regular Win32\nprogram) *still* would fail to create said file.\n\nCiao,\nDscho\n"},{"id":"319740","messageId":"xmqqbmquyiae.fsf@gitster.mtv.corp.google.com","threadId":"45945","inReplyTo":"CACBZZX7MXh_9mG1EROZVEEGapBwjzRWzr3S57f6rWLnQe9L+XA@mail.gmail.com","subject":"Re: [PATCH] fixup! log: add exhaustive tests for pattern style options & config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-15T02:18:17Z","receivedAt":"2017-05-15T02:18:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Thanks for the hotfix. I'll fix this in my v2, but do it differently\n> in such a way that I can still run these tests on windows.\n>\n> I.e. the actual test here just needs these odd characters in the\n> commit message. It's just an unintended implementation detail of\n> test_commit that a tag is being created.\n\nMy knee-jerk reaction matched Dscho's, but grep is about contents,\nand we should be able to test this if we used a sensible tagnames or\ndidn't use any. Glad to see somebody can step back and think ;-)\n"},{"id":"319778","messageId":"alpine.DEB.2.21.1.1705151418000.146734@virtualbox","threadId":"45945","inReplyTo":"xmqqbmquyiae.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] fixup! log: add exhaustive tests for pattern style options & config","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-05-15T12:19:27Z","receivedAt":"2017-05-15T12:19:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 15 May 2017, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n> \n> > Thanks for the hotfix. I'll fix this in my v2, but do it differently\n> > in such a way that I can still run these tests on windows.\n> >\n> > I.e. the actual test here just needs these odd characters in the\n> > commit message. It's just an unintended implementation detail of\n> > test_commit that a tag is being created.\n> \n> My knee-jerk reaction matched Dscho's, but grep is about contents,\n> and we should be able to test this if we used a sensible tagnames or\n> didn't use any. Glad to see somebody can step back and think ;-)\n\nMaybe somebody should step back even further and think even more, as we\ncould adjust test_commit to mangle the argument into a tag name that is\nlegal even with a refs backend relying on NTFS.\n\nCiao,\nDscho"},{"id":"319816","messageId":"20170515190415.GM27400@aiede.svl.corp.google.com","threadId":"45945","inReplyTo":"alpine.DEB.2.21.1.1705132037050.146734@virtualbox","subject":"Re: [PATCH] fixup! log: add exhaustive tests for pattern style options & config","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-05-15T19:04:15Z","receivedAt":"2017-05-15T19:04:23Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJohannes Schindelin wrote:\n> On Fri, 12 May 2017, Jonathan Nieder wrote:\n\n>> Would something like\n>>\n>> \ttest PIPE_IN_FILENAME '\n>> \t\t>\"a|b\" &&\n>> \t\ttest -f \"a|b\"\n>> \t'\n>>\n>> work?\n[...]\n> Back to the subject: The MSYS2 emulation layer inherits a neat trick from\n> Cygwin, where it *can* create file names containing pipe symbols. They\n> will be quietly mapped into a private UTF-8 page, and when Cygwin or MSYS2\n> read the file back, the file name maps from this page back to ASCII\n> transparently.\n>\n> That strategy is all good and dandy, as long as you stay within the POSIX\n> emulation layer.\n>\n> Git for Windows avoids the POSIX emulation layer as much as possible, for\n> speed, and also for robustness.\n>\n> Which means that Git does *not* map the file names using said private\n> UTF-8 code page. And therefore, your test would succeed (because the shell\n> script would stay within the POSIX emulation layer, which creates that\n> file using above-mentioned strategy), but Git (being a regular Win32\n> program) *still* would fail to create said file.\n\nWow. Thanks for a clear explaination.\n\nI'll be keeping a copy of this message handy for the next time I'm\nconfused about filename handling in the testsuite on Windows.\n\nSincerely,\nJonathan\n"},{"id":"319856","messageId":"xmqq37c56334.fsf@gitster.mtv.corp.google.com","threadId":"45945","inReplyTo":"alpine.DEB.2.21.1.1705151418000.146734@virtualbox","subject":"Re: [PATCH] fixup! log: add exhaustive tests for pattern style options & config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-16T00:46:23Z","receivedAt":"2017-05-16T00:46:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> On Mon, 15 May 2017, Junio C Hamano wrote:\n>\n>> My knee-jerk reaction matched Dscho's, but grep is about contents,\n>> and we should be able to test this if we used a sensible tagnames or\n>> didn't use any. Glad to see somebody can step back and think ;-)\n>\n> Maybe somebody should step back even further and think even more, as we\n> could adjust test_commit to mangle the argument into a tag name that is\n> legal even with a refs backend relying on NTFS.\n\nPerhaps, but I am not sure if that is needed. \n\nThe point of the helper is to serve as a simple \"we are building a\ntoy sample history by only adding a one-liner new file\" convenience\nhelper, and I think it is sensible to keep its definition simple.\nThe callers (like the ones being added in the rerolled patch under\ndiscussion) with special needs can supply tagname when the default\none is not suitable.\n\nIn hindsight, perhaps it would have been better if the default for\nthe helper were _not_ to create any tag (and callers who care about\ntags can optionally tell it to add tag, or tag the resulting commit\nthemselves), but that is lamenting water under the bridge.\n"},{"id":"319894","messageId":"CACBZZX4GZnpVzhL-bDSxmsH_JWZjxOPzpEYyZC=d+wLPLs6Kpg@mail.gmail.com","threadId":"45945","inReplyTo":"xmqq37c56334.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] fixup! log: add exhaustive tests for pattern style options & config","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-16T06:15:06Z","receivedAt":"2017-05-16T06:15:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, May 16, 2017 at 2:46 AM, Junio C Hamano wrote:\n> Johannes Schindelin writes:\n>\n>> On Mon, 15 May 2017, Junio C Hamano wrote:\n>>\n>>> My knee-jerk reaction matched Dscho's, but grep is about contents,\n>>> and we should be able to test this if we used a sensible tagnames or\n>>> didn't use any. Glad to see somebody can step back and think ;-)\n>>\n>> Maybe somebody should step back even further and think even more, as we\n>> could adjust test_commit to mangle the argument into a tag name that is\n>> legal even with a refs backend relying on NTFS.\n>\n> Perhaps, but I am not sure if that is needed.\n>\n> The point of the helper is to serve as a simple \"we are building a\n> toy sample history by only adding a one-liner new file\" convenience\n> helper, and I think it is sensible to keep its definition simple.\n> The callers (like the ones being added in the rerolled patch under\n> discussion) with special needs can supply tagname when the default\n> one is not suitable.\n>\n> In hindsight, perhaps it would have been better if the default for\n> the helper were _not_ to create any tag (and callers who care about\n> tags can optionally tell it to add tag, or tag the resulting commit\n> themselves), but that is lamenting water under the bridge.\n\nThis works, but I wonder if it's worth it to solve this one-off issue:\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 5ee124332a..4cab67c410 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -195,7 +195,15 @@ test_commit () {\n test_tick\n fi &&\n git ${indir:+ -C \"$indir\"} commit $signoff -m \"$1\" &&\n- git ${indir:+ -C \"$indir\"} tag \"${4:-$1}\"\n+ if test -n \"$4\"\n+ then\n+ git ${indir:+ -C \"$indir\"} tag \"$4\"\n+ elif test -n \"$(echo $1 | tr -d A-Za-z0-9/~_.#-)\"\n+ then\n+ error \"Implicitly created tag '$1' looks unusual,\nprobably fails outside *nix\"\n+ else\n+ git ${indir:+ -C \"$indir\"} tag \"$1\"\n+ fi\n }\n\n # Call test_merge with the arguments \" \", where \n"}]}