{"thread":{"id":"26955","subject":"[PATCH] Try to remove the given path even if it can't be opened","startedAt":"2011-04-01T08:29:16Z","lastAt":"2011-04-02T20:33:40Z","messageCount":6,"participants":["Alex Riesen","Michael J Gruber","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"164862","messageId":"AANLkTikfmXiZQquWi4STTCUy0qoY9J_waJ44nrPAvB1d@mail.gmail.com","threadId":"26955","inReplyTo":null,"subject":"[PATCH] Try to remove the given path even if it can't be opened","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2011-04-01T08:29:16Z","receivedAt":"2011-04-01T08:29:16Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Consider unreadable empty directories. rmdir(2) will remove\nthem just fine, assuming the parent directory is modifiable.\n\nNoticed by Linus.\n\nSigned-off-by: Alex Riesen \n---\nOn Fri, Apr 1, 2011 at 00:01, Linus Torvalds\n wrote:\n> Which is kind of understandable, but at the same time, if it's empty,\n> a \"rmdir()\" will just work. So git gave up a bit too soon.\n...\n> Now, I realize that if the directory isn't empty, and is unreadable,\n> we really should give up (although a better error message about _why_\n> we failed may be in order) rather than try to chmod it or anything\n> like that. But the simple \"try to rmdir it\" might be a good addition\n> for the trivial case.\n\nIt is not tested, but looks trivial. The system I made it on is a Cygwin\nmachine, and a test from last master pull is still running (since two days).\nAnd sorry, it is not based on master. Should apply without problems, though.\n\n---\n dir.c | 5 ++++-\n 1 files changed, 4 insertions(+), 1 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 325fb56..7251426 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1191,8 +1191,11 @@ int remove_dir_recursively(struct strbuf *path, int flag)\n \t\treturn 0;\n\n \tdir = opendir(path->buf);\n-\tif (!dir)\n+\tif (!dir) {\n+\t\tif (rmdir(path->buf) == 0)\n+\t\t\treturn 0;\n \t\treturn -1;\n+\t}\n \tif (path->buf[original_len - 1] != '/')\n \t\tstrbuf_addch(path, '/');\n\n-- \n1.7.2.2.240.g7d094\n\n\nFrom 861871ebfe72b6839526eaa4fe8e5c4b6eec924e Mon Sep 17 00:00:00 2001\nFrom: Alex Riesen \nDate: Fri, 1 Apr 2011 09:37:07 +0200\nSubject: [PATCH] Try to remove the given path even if it can't be opened\n\nConsider unreadable empty directories. rmdir(2) will remove\nthem just fine, assuming the parent directory is modifiable.\n\nNoticed by Linus.\n\nSigned-off-by: Alex Riesen \n---\n dir.c | 5 ++++-\n 1 files changed, 4 insertions(+), 1 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 325fb56..7251426 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1191,8 +1191,11 @@ int remove_dir_recursively(struct strbuf *path, int flag)\n \t\treturn 0;\n \n \tdir = opendir(path->buf);\n-\tif (!dir)\n+\tif (!dir) {\n+\t\tif (rmdir(path->buf) == 0)\n+\t\t\treturn 0;\n \t\treturn -1;\n+\t}\n \tif (path->buf[original_len - 1] != '/')\n \t\tstrbuf_addch(path, '/');\n \n-- \n1.7.2.2.240.g7d094\n\n"},{"id":"164884","messageId":"4D95D528.6050409@drmicha.warpmail.net","threadId":"26955","inReplyTo":"AANLkTikfmXiZQquWi4STTCUy0qoY9J_waJ44nrPAvB1d@mail.gmail.com","subject":"Re: [PATCH] Try to remove the given path even if it can't be opened","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-01T13:37:44Z","receivedAt":"2011-04-01T13:37:44Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Alex Riesen venit, vidit, dixit 01.04.2011 10:29:\n> Consider unreadable empty directories. rmdir(2) will remove\n> them just fine, assuming the parent directory is modifiable.\n> \n> Noticed by Linus.\n> \n> Signed-off-by: Alex Riesen \n> ---\n> On Fri, Apr 1, 2011 at 00:01, Linus Torvalds\n> wrote:\n>> Which is kind of understandable, but at the same time, if it's empty,\n>> a \"rmdir()\" will just work. So git gave up a bit too soon.\n> ...\n>> Now, I realize that if the directory isn't empty, and is unreadable,\n>> we really should give up (although a better error message about _why_\n>> we failed may be in order) rather than try to chmod it or anything\n>> like that. But the simple \"try to rmdir it\" might be a good addition\n>> for the trivial case.\n> \n> It is not tested, but looks trivial. The system I made it on is a Cygwin\n\nFamous last words...\n\n> machine, and a test from last master pull is still running (since two days).\n> And sorry, it is not based on master. Should apply without problems, though.\n> \n> ---\n> dir.c | 5 ++++-\n> 1 files changed, 4 insertions(+), 1 deletions(-)\n> \n> diff --git a/dir.c b/dir.c\n> index 325fb56..7251426 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -1191,8 +1191,11 @@ int remove_dir_recursively(struct strbuf *path, int flag)\n> \t\treturn 0;\n> \n> \tdir = opendir(path->buf);\n> -\tif (!dir)\n> +\tif (!dir) {\n> +\t\tif (rmdir(path->buf) == 0)\n> +\t\t\treturn 0;\n> \t\treturn -1;\n> +\t}\n> \tif (path->buf[original_len - 1] != '/')\n> \t\tstrbuf_addch(path, '/');\n> \n\nHow about simply\n\nif (!dir)\n\treturn rmdir(path->buf);\n\nlike we do later on in that function?\n\nMichael\n"},{"id":"164890","messageId":"AANLkTikjfkieO9VdpEka7Hb0Fk3st1+dCMF9ti0Kg5Fw@mail.gmail.com","threadId":"26955","inReplyTo":"4D95D528.6050409@drmicha.warpmail.net","subject":"Re: [PATCH] Try to remove the given path even if it can't be opened","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2011-04-01T14:01:50Z","receivedAt":"2011-04-01T14:01:50Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On Fri, Apr 1, 2011 at 15:37, Michael J Gruber wrote:\n> How about simply\n>\n> if (!dir)\n>        return rmdir(path->buf);\n>\n> like we do later on in that function?\n>\n\nI'm used to try to keep the returned value of a function I modify,\nand I'm also used to not trust the return values of the functions\nI don't control. That's to my defense.\n\nBut you're unquestionably right.\n"},{"id":"164902","messageId":"7vy63tg7yz.fsf@alter.siamese.dyndns.org","threadId":"26955","inReplyTo":"AANLkTikfmXiZQquWi4STTCUy0qoY9J_waJ44nrPAvB1d@mail.gmail.com","subject":"Re: [PATCH] Try to remove the given path even if it can't be opened","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-01T18:08:36Z","receivedAt":"2011-04-01T18:08:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen writes:\n\n> --0016e6d9a16eca69d0049fd73526\n> Content-Type: text/plain; charset=UTF-8\n>\n> Consider unreadable empty directories. rmdir(2) will remove\n> them just fine, assuming the parent directory is modifiable.\n>\n> Noticed by Linus.\n>\n> Signed-off-by: Alex Riesen \n> ---\n\nPlease don't do an attachment that has an inline patch and then attach the\npatch itself again in base64. It is extremely annoying.\n"},{"id":"164962","messageId":"20110402200920.GA18171@blimp.dmz","threadId":"26955","inReplyTo":"7vy63tg7yz.fsf@alter.siamese.dyndns.org","subject":"[PATCH] Try to remove the given path even if it can't be opened","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2011-04-02T20:09:20Z","receivedAt":"2011-04-02T20:09:20Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Consider unreadable empty directories. rmdir(2) will remove\nthem just fine, assuming the parent directory is modifiable.\n\nNoticed by Linus.\nFix suggested by Michael Gruber and Linus.\n\nSigned-off-by: Alex Riesen \n---\nJunio C Hamano, Fri, Apr 01, 2011 20:08:36 +0200:\n> \n> Please don't do an attachment that has an inline patch and then attach the\n> patch itself again in base64. It is extremely annoying.\n\nSorry. Hard to notice on GMail.\n\nThe extended error information is a little bit tricky: there is at least four\nerror cases (opendir, stat, unlink and rmdir) and there is a closedir, which\nresets errno to output the error in the caller of remove_dir_recursively.\n\n dir.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 325fb56..532bcb6 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1192,7 +1192,7 @@ int remove_dir_recursively(struct strbuf *path, int flag)\n \n \tdir = opendir(path->buf);\n \tif (!dir)\n-\t\treturn -1;\n+\t\treturn rmdir(path->buf);\n \tif (path->buf[original_len - 1] != '/')\n \t\tstrbuf_addch(path, '/');\n \n-- \n1.7.4\n"},{"id":"164963","messageId":"7vd3l4gzq3.fsf@alter.siamese.dyndns.org","threadId":"26955","inReplyTo":"20110402200920.GA18171@blimp.dmz","subject":"Re: [PATCH] Try to remove the given path even if it can't be opened","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-02T20:33:40Z","receivedAt":"2011-04-02T20:33:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen writes:\n\n>> Please don't do an attachment that has an inline patch and then attach the\n>> patch itself again in base64. It is extremely annoying.\n>\n> Sorry. Hard to notice on GMail.\n\nThanks. I've already queued 0235017 (clean: unreadable directory may\nstill be rmdir-able if it is empty, 2011-04-01) with a trivial test.\n"}]}