{"thread":{"id":"24367","subject":"[PATCH/RFC] test-lib: clean up trash* directories on SIGINT","startedAt":"2010-07-11T09:51:02Z","lastAt":"2010-07-12T08:56:52Z","messageCount":7,"participants":["Ævar Arnfjörð Bjarmason","Andreas Schwab","Nicolas Pitre","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"145321","messageId":"AANLkTimnrLlprdScYDKHs_lIvFKc3K0n8U5vTzG_df2k@mail.gmail.com","threadId":"24367","inReplyTo":null,"subject":"[PATCH/RFC] test-lib: clean up trash* directories on SIGINT","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-11T09:51:02Z","receivedAt":"2010-07-11T09:51:02Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Is there any reason not to remove trash directories on SIGINT? It's\nannoying that trash directories are left when I cancel a test run.\n\nI can submit an actual patch along these lines once Junio applies some\nof my other stuff which will probably conflict with any finalized\npatch.\n\nIs \"trap 'foo' INT\" a bashism? Is \"trap 'foo' SIGINT\" more portable?\nHow about the exit code from an INT handler, what should it be?\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex ac496aa..e4a0fc9 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -222,6 +222,17 @@ die () {\n GIT_EXIT_OK=\n trap 'die' EXIT\n\n+git_interrupted () {\n+       test -d \"$remove_trash\" &&\n+       cd \"$(dirname \"$remove_trash\")\" &&\n+       rm -rf \"$(basename \"$remove_trash\")\"\n+\n+       # What my shell normal exits on on sigint\n+       exit 130\n+}\n+\n+trap 'git_interrupted' INT\n+\n # The semantics of the editor variables are that of invoking\n # sh -c \"$EDITOR \\\"$@\\\"\" files ...\n #\n"},{"id":"145324","messageId":"m21vba9qrq.fsf@igel.home","threadId":"24367","inReplyTo":"AANLkTimnrLlprdScYDKHs_lIvFKc3K0n8U5vTzG_df2k@mail.gmail.com","subject":"Re: [PATCH/RFC] test-lib: clean up trash* directories on SIGINT","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2010-07-11T11:55:37Z","receivedAt":"2010-07-11T11:55:37Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> Is \"trap 'foo' INT\" a bashism?\n\nIt's the norm.\n\n> Is \"trap 'foo' SIGINT\" more portable?\n\n<http://www.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_28_03>\n\"Implementations may permit names with the SIG prefix or ignore case in\nsignal names as an extension.\"\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"145339","messageId":"alpine.LFD.2.00.1007112327150.10598@xanadu.home","threadId":"24367","inReplyTo":"AANLkTimnrLlprdScYDKHs_lIvFKc3K0n8U5vTzG_df2k@mail.gmail.com","subject":"Re: [PATCH/RFC] test-lib: clean up trash* directories on SIGINT","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2010-07-12T03:32:26Z","receivedAt":"2010-07-12T03:32:26Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Sun, 11 Jul 2010, Ævar Arnfjörð Bjarmason wrote:\n\n> Is there any reason not to remove trash directories on SIGINT? It's\n> annoying that trash directories are left when I cancel a test run.\n\nWhat about tests that would fail due to some infinite loop?  The test \nwould never complete until you manually interrupt it, and in that case \nthe trash directory content might be valuable.  \n\nYou may do 'make clean' in the test directory to clean it.\n\n\nNicolas\n"},{"id":"145342","messageId":"AANLkTindliEmS2XhH8B1dUyo-v-CzpLQBqZakOaNnARW@mail.gmail.com","threadId":"24367","inReplyTo":"alpine.LFD.2.00.1007112327150.10598@xanadu.home","subject":"Re: [PATCH/RFC] test-lib: clean up trash* directories on SIGINT","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-12T05:42:54Z","receivedAt":"2010-07-12T05:42:54Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, Jul 12, 2010 at 03:32, Nicolas Pitre <nico@fluxnic.net> wrote:\n> On Sun, 11 Jul 2010, Ævar Arnfjörð Bjarmason wrote:\n>\n>> Is there any reason not to remove trash directories on SIGINT? It's\n>> annoying that trash directories are left when I cancel a test run.\n>\n> What about tests that would fail due to some infinite loop?  The test\n> would never complete until you manually interrupt it, and in that case\n> the trash directory content might be valuable.\n\nUnless that's due to some unreproducable heisenbug you could get the\ntrash directory by running the test with --debug.\n\n> You may do 'make clean' in the test directory to clean it.\n\nI know, but I think not cleaning them up on SIGINT is a\nmisfeature. These trash directories are explicitly temporary, and\nshould be cleaned up as the shell exists.\n"},{"id":"145347","messageId":"20100712061608.GA6140@burratino","threadId":"24367","inReplyTo":"AANLkTindliEmS2XhH8B1dUyo-v-CzpLQBqZakOaNnARW@mail.gmail.com","subject":"Re: [PATCH/RFC] test-lib: clean up trash* directories on SIGINT","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-12T06:16:08Z","receivedAt":"2010-07-12T06:16:08Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Ævar,\n\nÆvar Arnfjörð Bjarmason wrote:\n\n> Unless that's due to some unreproducable heisenbug you could get the\n> trash directory by running the test with --debug.\n\nSo what should I do after noticing a heisenbug?  I guess I can suspend\nwith ^Z, but I would prefer not to have to train my fingers away from ^C.\n\n> These trash directories are explicitly temporary, and\n> should be cleaned up as the shell exists.\n\nWhen tests learned to remove the trash directory in test_done in\nv1.6.1-rc1~336^2~1 (Enable parallel tests, 2008-08-08), that was not\nthe rationale; it was rather to avoid too many trash directories\npiling up.  If a test exits by mistake with ‘exit’ or fails with ‘-i’,\nthe one or two scratch directories involved are still kept for\ndebugging.\n\nSo you can see why a person would be reluctant to change this for\naesthetic reasons only.\n"},{"id":"145351","messageId":"AANLkTinGsIqy-rG8um1E7_zfla1KwsO2Z26JnchXaRLd@mail.gmail.com","threadId":"24367","inReplyTo":"20100712061608.GA6140@burratino","subject":"Re: [PATCH/RFC] test-lib: clean up trash* directories on SIGINT","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-12T08:16:26Z","receivedAt":"2010-07-12T08:16:26Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, Jul 12, 2010 at 06:16, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Hi Ævar,\n>\n> Ævar Arnfjörð Bjarmason wrote:\n>\n>> Unless that's due to some unreproducable heisenbug you could get the\n>> trash directory by running the test with --debug.\n>\n> So what should I do after noticing a heisenbug?\n\nRun the test again with --debug. The test library and the commands\nit's calling don't live in the same address space. A \"heisenbug\" will\nprobably be just as reproducable with --debug and without --debug.\n\n>> These trash directories are explicitly temporary, and\n>> should be cleaned up as the shell exists.\n>\n> When tests learned to remove the trash directory in test_done in\n> v1.6.1-rc1~336^2~1 (Enable parallel tests, 2008-08-08), that was not\n> the rationale; it was rather to avoid too many trash directories\n> piling up.  If a test exits by mistake with ‘exit’ or fails with ‘-i’,\n> the one or two scratch directories involved are still kept for\n> debugging.\n\nThat's the original rationale, which is fair enough. I think the\nbehavior should be to clean up temporary files when the test exits,\nnot just when it's done executing.\n\n> So you can see why a person would be reluctant to change this for\n> aesthetic reasons only.\n\nThe reason I want to change it is that as I'm hacking Git I often run\nthe tests but kill them before they've completed, because if I'm\ne.g. testing the test library itself I'm content with having only 10\ntests pass before I continue improving it.\n\nIf I keep doing that (along with --jobs 20 and --shuffle) I'll\neventually pile up ~200 MB of trash. That'll exceed the limits of my\nramdisk and I'll have to grudgingly do rm -rf trash* again.\n\nBut maybe people such as yourself depend on the current behavior with\nregards to SIGINT. I don't feel strongly about it, and having to\nremove trash is a minor annoyance.\n\nIf it's being used it should be kept. I just assumed that it was an\nomission in the cleanup routines.\n"},{"id":"145352","messageId":"20100712085652.GA6837@burratino","threadId":"24367","inReplyTo":"AANLkTinGsIqy-rG8um1E7_zfla1KwsO2Z26JnchXaRLd@mail.gmail.com","subject":"Re: [PATCH/RFC] test-lib: clean up trash* directories on SIGINT","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-12T08:56:52Z","receivedAt":"2010-07-12T08:56:52Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n> On Mon, Jul 12, 2010 at 06:16, Jonathan Nieder <jrnieder@gmail.com> wrote:\n\n>> So what should I do after noticing a heisenbug?\n>\n> Run the test again with --debug.\n\nSorry, I wasn’t clear.  The sequence I was imagining (and have\nenountered) is:\n\n $ make test\n [...]\n not ok - 35 something that really should have been okay\n ... more text streams by ...\n\n-- wait, what?  I’ve never seen that fail before.  ^C\n\nIf I catch it early enough, I can inspect the trash directory and\nfigure out what happened.  Some bugs are hard to reproduce (and it\navoids waiting for tests to run again).  But I can use ^Z instead.\n\n> If I keep doing that (along with --jobs 20 and --shuffle) I'll\n> eventually pile up ~200 MB of trash. That'll exceed the limits of my\n> ramdisk and I'll have to grudgingly do rm -rf trash* again.\n\nGiven this rationale, I don’t personally mind the change (though it\nis still not clear why not to run “make clean” from t/ between runs\ninstead).\n\nI would be more worried if you were proposing making the --immediate\noption remove the trash directory, too.  I often use that option with\nthe express intent of populating a test repository which is no more\ntemporary than the .o files and copies of binaries in the build tree\nare.\n\nThanks for the explanation,\nJonathan\n"}]}