{"thread":{"id":"42766","subject":"[PATCH] t7610 test for mktemp existence","startedAt":"2016-07-02T19:01:59Z","lastAt":"2016-07-06T19:15:30Z","messageCount":4,"participants":["Armin Kunaschik","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"290752","messageId":"57780F9F.3050802@googlemail.com","threadId":"42766","inReplyTo":null,"subject":"[PATCH] t7610 test for mktemp existence","fromName":"Armin Kunaschik","fromEmail":"megabreit@googlemail.com","sentAt":"2016-07-02T19:01:51Z","receivedAt":"2016-07-02T19:01:59Z","isPatch":true,"sender":{"key":"megabreit@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/10429218?v=4"},"body":"Subject: t7610: test for mktemp before test execution\n\nmktemp is not available on all platforms, so the test\n'temporary filenames are used with mergetool.writeToTemp'\nfails there.\nThis patch does not replace mktemp but just disables\nthe test that otherwise would fail.\nmergetool checks itself before executing mktemp and\nreports an error.\n\nSigned-off-by: Armin Kunaschik <megabreit@googlemail.com>\n\n---\n\ndiff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\nindex 76306cf..9279bf5 100755\n--- a/t/t7610-mergetool.sh\n+++ b/t/t7610-mergetool.sh\n@@ -589,7 +589,12 @@ test_expect_success 'filenames seen by tools start with ./' '\n \tgit reset --hard master >/dev/null 2>&1\n '\n\n-test_expect_success 'temporary filenames are used with mergetool.writeToTemp' '\n+test_lazy_prereq MKTEMP '\n+\ttempdir=$(mktemp -d -t foo.XXX) &&\n+\ttest -d \"$tempdir\"\n+'\n+\n+test_expect_success MKTEMP 'temporary filenames are used with mergetool.writeToTemp' '\n \tgit checkout -b test16 branch1 &&\n \ttest_config mergetool.writeToTemp true &&\n \ttest_config mergetool.myecho.cmd \"echo \\\"\\$LOCAL\\\"\" &&\n\n"},{"id":"290898","messageId":"xmqqr3b6sk08.fsf@gitster.mtv.corp.google.com","threadId":"42766","inReplyTo":"57780F9F.3050802@googlemail.com","subject":"Re: [PATCH] t7610 test for mktemp existence","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-06T18:23:51Z","receivedAt":"2016-07-06T18:24:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Armin Kunaschik <megabreit@googlemail.com> writes:\n\n> Subject: t7610: test for mktemp before test execution\n>\n> mktemp is not available on all platforms, so the test\n> 'temporary filenames are used with mergetool.writeToTemp'\n> fails there.\n> This patch does not replace mktemp but just disables\n> the test that otherwise would fail.\n> mergetool checks itself before executing mktemp and\n> reports an error.\n\nThanks.\n\n> Signed-off-by: Armin Kunaschik <megabreit@googlemail.com>\n>\n> ---\n>\n> diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\n> index 76306cf..9279bf5 100755\n> --- a/t/t7610-mergetool.sh\n> +++ b/t/t7610-mergetool.sh\n> @@ -589,7 +589,12 @@ test_expect_success 'filenames seen by tools start with ./' '\n>  \tgit reset --hard master >/dev/null 2>&1\n>  '\n>\n> -test_expect_success 'temporary filenames are used with mergetool.writeToTemp' '\n> +test_lazy_prereq MKTEMP '\n> +\ttempdir=$(mktemp -d -t foo.XXX) &&\n> +\ttest -d \"$tempdir\"\n> +'\n\nThis makes me wonder what would happen to the leftover directory,\nthough.  Would it be a better idea to clean it up as well, i.e.\n\n\ttempdir=$(mktemp -d -t foo.XXXXXX) &&\n\ttest -d \"$tempdir\" &&\n        rmdir \"$tempdir\"\n\n"},{"id":"290903","messageId":"20160706190158.GA31148@sigill.intra.peff.net","threadId":"42766","inReplyTo":"xmqqr3b6sk08.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] t7610 test for mktemp existence","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-06T19:01:58Z","receivedAt":"2016-07-06T19:02:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 06, 2016 at 11:23:51AM -0700, Junio C Hamano wrote:\n\n> > -test_expect_success 'temporary filenames are used with mergetool.writeToTemp' '\n> > +test_lazy_prereq MKTEMP '\n> > +\ttempdir=$(mktemp -d -t foo.XXX) &&\n> > +\ttest -d \"$tempdir\"\n> > +'\n> \n> This makes me wonder what would happen to the leftover directory,\n> though.  Would it be a better idea to clean it up as well, i.e.\n> \n> \ttempdir=$(mktemp -d -t foo.XXXXXX) &&\n> \ttest -d \"$tempdir\" &&\n>         rmdir \"$tempdir\"\n\nLazy prereq's are computed inside a temporary directory[1] that is\nautomatically cleaned up, so I think the code here does not have to\nworry about it.\n\n-Peff\n\n[1] Coincidentally, I recently wanted to have a lazy prereq check\n    _another_ prereq inside it. But it turns out you cannot do this:\n\n      test_lazy_prereq RECURSE_INNER '\n        echo inner >file\n      '\n      test_lazy_prereq RECURSE_OUTER '\n        echo outer >file &&\n\ttest_have_prereq RECURSE_INNER &&\n\techo outer >expect &&\n\ttest_cmp expeect file\n      '\n      test_expect_success 'lazy prereqs can recurse' '\n\ttest_have_prereq RECURSE_OUTER\n      '\n\n    because they both use the same temporary directory (so beyond having\n    \"outer\" see \"inner\" in the file, it actually complains when the\n    inner check removes the directory entirely).\n\n    The fix for that is simple: give the tempdir a unique name. But I\n    think this kind of recursion is still not OK in the shell because\n    without the \"local\" keyword, we have no concept of stack variables,\n    and so stomp on the globals with each iteration.\n\n    Anyway. Not a big deal, and I ended up simplifying my tests not to\n    need it. But I was certainly surprised and confused by it at the\n    time, so I figured it was worth sharing the knowledge. :)\n"},{"id":"290905","messageId":"xmqqinwishme.fsf@gitster.mtv.corp.google.com","threadId":"42766","inReplyTo":"xmqqr3b6sk08.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] t7610 test for mktemp existence","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-06T19:15:21Z","receivedAt":"2016-07-06T19:15:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> -test_expect_success 'temporary filenames are used with mergetool.writeToTemp' '\n>> +test_lazy_prereq MKTEMP '\n>> +\ttempdir=$(mktemp -d -t foo.XXX) &&\n>> +\ttest -d \"$tempdir\"\n>> +'\n>\n> This makes me wonder what would happen to the leftover directory,\n> though.  Would it be a better idea to clean it up as well, i.e.\n>\n> \ttempdir=$(mktemp -d -t foo.XXXXXX) &&\n> \ttest -d \"$tempdir\" &&\n>         rmdir \"$tempdir\"\n\nIt turns out that this is not necessary since day one of\nlazy_prereq.  I forgot what I wrote in 04083f27 (test: allow\nprerequisite to be evaluated lazily, 2012-07-26).\n\nThanks.\n"}]}