{"thread":{"id":"11772","subject":"[PATCH] filter-branch.sh: remove temporary directory on failure","startedAt":"2008-01-28T21:16:02Z","lastAt":"2008-01-30T11:51:31Z","messageCount":3,"participants":["Brandon Casey","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"66759","messageId":"479E4612.6030006@nrlssc.navy.mil","threadId":"11772","inReplyTo":null,"subject":"[PATCH] filter-branch.sh: remove temporary directory on failure","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-01-28T21:16:02Z","receivedAt":"2008-01-28T21:16:02Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey \n\nOne of the first things filter-branch does is to create a temporary\ndirectory. This directory is eventually removed by the script during\nnormal operation, but is not removed if the script encounters an error.\n\nSet a trap to remove it when the script terminates for any reason.\n\nSigned-off-by: Brandon Casey \n---\n\n\nEven though this directory may be useful for debugging when you encounter\na problem, I wonder if the normal \"error\" will be _user_ error. In which\ncase the user will adjust the command line parameters and try to rerun.\nCurrently the user would then receive the message\n\n \"$tempdir already exists, please remove it\"\n\nThen, rm -rf the tempdir, then rerun.\n\nIs this necessary? Maybe anyone with the know-how to debug would also have the\nknow-how to comment out the 'trap' in the script (or direct someone asking for\nhelp on the mailing list to do so).\n\nIf we really want to leave this temporary directory around on failure, maybe\nwe can at least remove it for some simple errors like:\n\n line 198: die \"Namespace $orig_namespace not empty\"\n line 217: die \"Which ref do you want to rewrite?\"\n etc.\n\nby moving the 'trap - 0' higher up.\n\n-brandon\n\n\n git-filter-branch.sh | 5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex ebf05ca..75970a9 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -189,6 +189,9 @@ cd \"$tempdir/t\" &&\n workdir=\"$(pwd)\" ||\n die \"\"\n \n+# Remove tempdir on exit\n+trap 'cd ../..; rm -rf \"$tempdir\"' 0\n+\n # Make sure refs/original is empty\n git for-each-ref > \"$tempdir\"/backup-refs\n while read sha1 type name\n@@ -406,6 +409,8 @@ fi\n cd ../..\n rm -rf \"$tempdir\"\n \n+trap - 0\n+\n unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE\n test -z \"$ORIG_GIT_DIR\" || GIT_DIR=\"$ORIG_GIT_DIR\" && export GIT_DIR\n test -z \"$ORIG_GIT_WORK_TREE\" || GIT_WORK_TREE=\"$ORIG_GIT_WORK_TREE\" &&\n-- \n1.5.4.rc5.2.g9a6d-dirty\n"},{"id":"66884","messageId":"7v3asfkgii.fsf@gitster.siamese.dyndns.org","threadId":"11772","inReplyTo":"479E4612.6030006@nrlssc.navy.mil","subject":"Re: [PATCH] filter-branch.sh: remove temporary directory on failure","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-30T05:51:33Z","receivedAt":"2008-01-30T05:51:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Casey writes:\n\n> Even though this directory may be useful for debugging when you encounter\n> a problem, I wonder if the normal \"error\" will be _user_ error. In which\n> case the user will adjust the command line parameters and try to rerun.\n> Currently the user would then receive the message\n>\n> \"$tempdir already exists, please remove it\"\n>\n> Then, rm -rf the tempdir, then rerun.\n>\n> Is this necessary? Maybe anyone with the know-how to debug would also have the\n> know-how to comment out the 'trap' in the script (or direct someone asking for\n> help on the mailing list to do so).\n\nMakes sense to me. Dscho?\n"},{"id":"66913","messageId":"alpine.LSU.1.00.0801301151050.23907@racer.site","threadId":"11772","inReplyTo":"7v3asfkgii.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] filter-branch.sh: remove temporary directory on failure","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-01-30T11:51:31Z","receivedAt":"2008-01-30T11:51:31Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 29 Jan 2008, Junio C Hamano wrote:\n\n> Brandon Casey writes:\n> \n> > Even though this directory may be useful for debugging when you \n> > encounter a problem, I wonder if the normal \"error\" will be _user_ \n> > error. In which case the user will adjust the command line parameters \n> > and try to rerun. Currently the user would then receive the message\n> >\n> > \"$tempdir already exists, please remove it\"\n> >\n> > Then, rm -rf the tempdir, then rerun.\n> >\n> > Is this necessary? Maybe anyone with the know-how to debug would also \n> > have the know-how to comment out the 'trap' in the script (or direct \n> > someone asking for help on the mailing list to do so).\n> \n> Makes sense to me. Dscho?\n\nTo me, too.\n\nAcked-by: Johannes Schindelin \n\nThanks,\nDscho\n"}]}