{"thread":{"id":"10474","subject":"[PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty","startedAt":"2007-10-26T14:15:39Z","lastAt":"2007-11-08T07:31:01Z","messageCount":20,"participants":["Gerrit Pape","Fernando J. Pereda","Jakub Narebski","Jeff King","Alex Riesen","Michael Cohen","Johannes Schindelin","Karl Hasselström","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"57275","messageId":"20071026141539.29928.qmail@d3691352d65cf2.315fe32.mid.smarden.org","threadId":"10474","inReplyTo":null,"subject":"[PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty","fromName":"Gerrit Pape","fromEmail":"pape@smarden.org","sentAt":"2007-10-26T14:15:39Z","receivedAt":"2007-10-26T14:15:39Z","isPatch":true,"sender":{"key":"pape@smarden.org","avatar":"https://avatars.githubusercontent.com/u/143170252?v=4"},"body":"When saving patches to a maildir with e.g. mutt, the files are put into\nthe new/ subdirectory of the maildir, not cur/.  This makes git-am state\n\"Nothing to do.\".  This patch lets git-mailsplit fallback to new/ if the\ncur/ subdirectory is empty.\n\nThis was reported by Joey Hess through\n http://bugs.debian.org/447396\n\nSigned-off-by: Gerrit Pape <pape@smarden.org>\n---\n Documentation/git-am.txt |    3 ++-\n builtin-mailsplit.c      |    5 +++++\n 2 files changed, 7 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-am.txt b/Documentation/git-am.txt\nindex e4a6b3a..49f79f6 100644\n--- a/Documentation/git-am.txt\n+++ b/Documentation/git-am.txt\n@@ -26,7 +26,8 @@ OPTIONS\n <mbox>|<Maildir>...::\n \tThe list of mailbox files to read patches from. If you do not\n \tsupply this argument, reads from the standard input. If you supply\n-\tdirectories, they'll be treated as Maildirs.\n+\tdirectories, they'll be treated as Maildirs, which should contain\n+\tthe patches either in the cur/ subdirectory, or in new/.\n \n -s, --signoff::\n \tAdd `Signed-off-by:` line to the commit message, using\ndiff --git a/builtin-mailsplit.c b/builtin-mailsplit.c\nindex 43fc373..eaf3cbe 100644\n--- a/builtin-mailsplit.c\n+++ b/builtin-mailsplit.c\n@@ -131,6 +131,11 @@ static int split_maildir(const char *maildir, const char *dir,\n \tsnprintf(curdir, sizeof(curdir), \"%s/cur\", maildir);\n \tif (populate_maildir_list(&list, curdir) < 0)\n \t\tgoto out;\n+\tif (list.nr == 0) {\n+\t\tsnprintf(curdir, sizeof(curdir), \"%s/new\", maildir);\n+\t\tif (populate_maildir_list(&list, curdir) < 0)\n+\t\t\tgoto out;\n+\t}\n \n \tfor (i = 0; i < list.nr; i++) {\n \t\tFILE *f;\n-- \n1.5.3.4\n"},{"id":"57286","messageId":"20071026160118.GA5076@ferdyx.org","threadId":"10474","inReplyTo":"20071026141539.29928.qmail@d3691352d65cf2.315fe32.mid.smarden.org","subject":"Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty","fromName":"Fernando J. Pereda","fromEmail":"ferdy@gentoo.org","sentAt":"2007-10-26T16:01:18Z","receivedAt":"2007-10-26T16:01:18Z","isPatch":true,"sender":{"key":"ferdy@gentoo.org","avatar":null},"body":"On Fri, Oct 26, 2007 at 02:15:39PM +0000, Gerrit Pape wrote:\n> When saving patches to a maildir with e.g. mutt, the files are put into\n> the new/ subdirectory of the maildir, not cur/.  This makes git-am state\n> \"Nothing to do.\".  This patch lets git-mailsplit fallback to new/ if the\n> cur/ subdirectory is empty.\n> \n> This was reported by Joey Hess through\n>  http://bugs.debian.org/447396\n> \n\nBy that reasoning, you should make it parse both cur/ and new/.\n\nThis didn't bit me because I always check mails I queue, so they ended\nup in cur/.\n\nOther than that, ack from me.\n\n- ferdy\n\n-- \nFernando J. Pereda Garcimartín\n20BB BDC3 761A 4781 E6ED  ED0B 0A48 5B0C 60BD 28D4\n"},{"id":"58372","messageId":"20071105124920.17726.qmail@746e9cce42b49f.315fe32.mid.smarden.org","threadId":"10474","inReplyTo":"20071026160118.GA5076@ferdyx.org","subject":"[PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty","fromName":"Gerrit Pape","fromEmail":"pape@smarden.org","sentAt":"2007-11-05T12:49:20Z","receivedAt":"2007-11-05T12:49:20Z","isPatch":true,"sender":{"key":"pape@smarden.org","avatar":"https://avatars.githubusercontent.com/u/143170252?v=4"},"body":"When saving patches to a maildir with e.g. mutt, the files are put into\nthe new/ subdirectory of the maildir, not cur/.  This makes git-am state\n\"Nothing to do.\".  This patch lets git-mailsplit additional check new/\nafter reading cur/.\n\nThis was reported by Joey Hess through\n http://bugs.debian.org/447396\n\nSigned-off-by: Gerrit Pape <pape@smarden.org>\n---\n\nOn Fri, Oct 26, 2007 at 06:01:18PM +0200, Fernando J. Pereda wrote:\n> By that reasoning, you should make it parse both cur/ and new/.\nOkay.\n\n builtin-mailsplit.c |   36 ++++++++++++++++++++----------------\n 1 files changed, 20 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin-mailsplit.c b/builtin-mailsplit.c\nindex 74b0470..79e8ee0 100644\n--- a/builtin-mailsplit.c\n+++ b/builtin-mailsplit.c\n@@ -101,19 +101,26 @@ static int populate_maildir_list(struct path_list *list, const char *path)\n {\n \tDIR *dir;\n \tstruct dirent *dent;\n+\tchar name[PATH_MAX];\n+\tchar *sub[] = { \"cur\", \"new\" };\n+\tint i;\n \n-\tif ((dir = opendir(path)) == NULL) {\n-\t\terror(\"cannot opendir %s (%s)\", path, strerror(errno));\n-\t\treturn -1;\n-\t}\n+\tfor (i = 0; i < 2; ++i) {\n+\t\tsnprintf(name, sizeof(name), \"%s/%s\", path, sub[i]);\n+\t\tif ((dir = opendir(name)) == NULL) {\n+\t\t\terror(\"cannot opendir %s (%s)\", name, strerror(errno));\n+\t\t\treturn -1;\n+\t\t}\n \n-\twhile ((dent = readdir(dir)) != NULL) {\n-\t\tif (dent->d_name[0] == '.')\n-\t\t\tcontinue;\n-\t\tpath_list_insert(dent->d_name, list);\n-\t}\n+\t\twhile ((dent = readdir(dir)) != NULL) {\n+\t\t\tif (dent->d_name[0] == '.')\n+\t\t\t\tcontinue;\n+\t\t\tsnprintf(name, sizeof(name), \"%s/%s\", sub[i], dent->d_name);\n+\t\t\tpath_list_insert(name, list);\n+\t\t}\n \n-\tclosedir(dir);\n+\t\tclosedir(dir);\n+\t}\n \n \treturn 0;\n }\n@@ -122,19 +129,17 @@ static int split_maildir(const char *maildir, const char *dir,\n \tint nr_prec, int skip)\n {\n \tchar file[PATH_MAX];\n-\tchar curdir[PATH_MAX];\n \tchar name[PATH_MAX];\n \tint ret = -1;\n \tint i;\n \tstruct path_list list = {NULL, 0, 0, 1};\n \n-\tsnprintf(curdir, sizeof(curdir), \"%s/cur\", maildir);\n-\tif (populate_maildir_list(&list, curdir) < 0)\n+\tif (populate_maildir_list(&list, maildir) < 0)\n \t\tgoto out;\n \n \tfor (i = 0; i < list.nr; i++) {\n \t\tFILE *f;\n-\t\tsnprintf(file, sizeof(file), \"%s/%s\", curdir, list.items[i].path);\n+\t\tsnprintf(file, sizeof(file), \"%s/%s\", maildir, list.items[i].path);\n \t\tf = fopen(file, \"r\");\n \t\tif (!f) {\n \t\t\terror(\"cannot open mail %s (%s)\", file, strerror(errno));\n@@ -152,10 +157,9 @@ static int split_maildir(const char *maildir, const char *dir,\n \t\tfclose(f);\n \t}\n \n-\tpath_list_clear(&list, 1);\n-\n \tret = skip;\n out:\n+\tpath_list_clear(&list, 1);\n \treturn ret;\n }\n \n-- \n1.5.3.5\n"},{"id":"58374","messageId":"fgn429$gs9$1@ger.gmane.org","threadId":"10474","inReplyTo":"20071105124920.17726.qmail@746e9cce42b49f.315fe32.mid.smarden.org","subject":"Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-11-05T12:58:50Z","receivedAt":"2007-11-05T12:58:50Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"[Cc: Gerrit Pape <pape@smarden.org>, git@vger.kernel.org]\n\nGerrit Pape wrote:\n\n> +       char *sub[] = { \"cur\", \"new\" };\n[...]\n> +       for (i = 0; i < 2; ++i) {\n\nWouldn't it be better to use sizeof(sub)/sizeof(sub[0]) or it's macro\nequivalent ARRAY_SIZE(sub) instead of hardcoding 2 to avoid errors?\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"58422","messageId":"20071105212624.GA9520@sigill.intra.peff.net","threadId":"10474","inReplyTo":"20071105124920.17726.qmail@746e9cce42b49f.315fe32.mid.smarden.org","subject":"Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-05T21:26:24Z","receivedAt":"2007-11-05T21:26:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 05, 2007 at 12:49:20PM +0000, Gerrit Pape wrote:\n\n> On Fri, Oct 26, 2007 at 06:01:18PM +0200, Fernando J. Pereda wrote:\n> > By that reasoning, you should make it parse both cur/ and new/.\n> Okay.\n\nIsn't the subject line now wrong?\n\n-Peff\n"},{"id":"58443","messageId":"20071105225258.GC4208@steel.home","threadId":"10474","inReplyTo":"20071105124920.17726.qmail@746e9cce42b49f.315fe32.mid.smarden.org","subject":"Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-11-05T22:52:58Z","receivedAt":"2007-11-05T22:52:58Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Gerrit Pape, Mon, Nov 05, 2007 13:49:20 +0100:\n> +\tfor (i = 0; i < 2; ++i) {\n> +\t\tsnprintf(name, sizeof(name), \"%s/%s\", path, sub[i]);\n> +\t\tif ((dir = opendir(name)) == NULL) {\n> +\t\t\terror(\"cannot opendir %s (%s)\", name, strerror(errno));\n> +\t\t\treturn -1;\n> +\t\t}\n\nWhy is missing \"cur\" (or \"new\", for that matter) a fatal error?\nWhy is it error at all? How about just ignoring the fact?\n"},{"id":"58475","messageId":"635FFEC2-2489-443B-8425-DF2B58BE23C2@mac.com","threadId":"10474","inReplyTo":"20071105225258.GC4208@steel.home","subject":"Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty","fromName":"Michael Cohen","fromEmail":"michaeljosephcohen@mac.com","sentAt":"2007-11-06T01:41:56Z","receivedAt":"2007-11-06T01:41:56Z","isPatch":true,"sender":{"key":"michaeljosephcohen@mac.com","avatar":null},"body":"On Nov 5, 2007, at 5:52 PM, Alex Riesen wrote:\n\n> Gerrit Pape, Mon, Nov 05, 2007 13:49:20 +0100:\n>> +\tfor (i = 0; i < 2; ++i) {\n>> +\t\tsnprintf(name, sizeof(name), \"%s/%s\", path, sub[i]);\n>> +\t\tif ((dir = opendir(name)) == NULL) {\n>> +\t\t\terror(\"cannot opendir %s (%s)\", name, strerror(errno));\n>> +\t\t\treturn -1;\n>> +\t\t}\n>\n> Why is missing \"cur\" (or \"new\", for that matter) a fatal error?\n> Why is it error at all? How about just ignoring the fact?\nIn Maildir format, cur and new hold the mails. :P\n\n-mjc\n"},{"id":"58495","messageId":"20071106072831.GA3021@steel.home","threadId":"10474","inReplyTo":"635FFEC2-2489-443B-8425-DF2B58BE23C2@mac.com","subject":"Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-11-06T07:28:31Z","receivedAt":"2007-11-06T07:28:31Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Michael Cohen, Tue, Nov 06, 2007 02:41:56 +0100:\n> On Nov 5, 2007, at 5:52 PM, Alex Riesen wrote:\n>\n>> Gerrit Pape, Mon, Nov 05, 2007 13:49:20 +0100:\n>>> +\tfor (i = 0; i < 2; ++i) {\n>>> +\t\tsnprintf(name, sizeof(name), \"%s/%s\", path, sub[i]);\n>>> +\t\tif ((dir = opendir(name)) == NULL) {\n>>> +\t\t\terror(\"cannot opendir %s (%s)\", name, strerror(errno));\n>>> +\t\t\treturn -1;\n>>> +\t\t}\n>>\n>> Why is missing \"cur\" (or \"new\", for that matter) a fatal error?\n>> Why is it error at all? How about just ignoring the fact?\n> In Maildir format, cur and new hold the mails. :P\n\nSo? Why *STOP* reading the mails if just one of the directories could\nnot be opened? IOW, I suggest:\n\n+\tfor (i = 0; i < 2; ++i) {\n+\t\tsnprintf(name, sizeof(name), \"%s/%s\", path, sub[i]);\n+\t\tdir = opendir(name);\n+\t\tif (!dir)\n+\t\t\tcontinue;\n"},{"id":"58500","messageId":"20071106075150.GA21694@sigill.intra.peff.net","threadId":"10474","inReplyTo":"20071106072831.GA3021@steel.home","subject":"Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-06T07:51:53Z","receivedAt":"2007-11-06T07:51:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 06, 2007 at 08:28:31AM +0100, Alex Riesen wrote:\n\n> > In Maildir format, cur and new hold the mails. :P\n> \n> So? Why *STOP* reading the mails if just one of the directories could\n> not be opened? IOW, I suggest:\n\nBecause you are then trying to apply a patch series with some patches\npotentially missing? Continuing only on errno == ENOENT seems prudent.\n\n-Peff\n"},{"id":"58510","messageId":"20071106085418.14211.qmail@54d7c9212e25c5.315fe32.mid.smarden.org","threadId":"10474","inReplyTo":"20071105225258.GC4208@steel.home","subject":"[PATCH amend] git-mailsplit: with maildirs not only process cur/, but also new/","fromName":"Gerrit Pape","fromEmail":"pape@smarden.org","sentAt":"2007-11-06T08:54:18Z","receivedAt":"2007-11-06T08:54:18Z","isPatch":true,"sender":{"key":"pape@smarden.org","avatar":"https://avatars.githubusercontent.com/u/143170252?v=4"},"body":"When saving patches to a maildir with e.g. mutt, the files are put into\nthe new/ subdirectory of the maildir, not cur/.  This makes git-am state\n\"Nothing to do.\".  This patch lets git-mailsplit additional check new/\nafter reading cur/.\n\nThis was reported by Joey Hess through\n http://bugs.debian.org/447396\n\nSigned-off-by: Gerrit Pape <pape@smarden.org>\n---\n\nOn Mon, Nov 05, 2007 at 01:58:50PM +0100, Jakub Narebski wrote:\n> > +        for (i = 0; i < 2; ++i) {\n> Wouldn't it be better to use sizeof(sub)/sizeof(sub[0]) or it's macro\n> equivalent ARRAY_SIZE(sub) instead of hardcoding 2 to avoid errors?\nI made the array NULL-terminated.\n\nOn Mon, Nov 05, 2007 at 04:26:24PM -0500, Jeff King wrote:\n> Isn't the subject line now wrong?\nYes, thanks.\n\nOn Mon, Nov 05, 2007 at 11:52:58PM +0100, Alex Riesen wrote:\n> Why is missing \"cur\" (or \"new\", for that matter) a fatal error?\n> Why is it error at all? How about just ignoring the fact?\nAs suggested by Jeff, I made it ignore the error on ENOENT.\n\n builtin-mailsplit.c |   38 ++++++++++++++++++++++----------------\n 1 files changed, 22 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin-mailsplit.c b/builtin-mailsplit.c\nindex 74b0470..46b27cd 100644\n--- a/builtin-mailsplit.c\n+++ b/builtin-mailsplit.c\n@@ -101,20 +101,29 @@ static int populate_maildir_list(struct path_list *list, const char *path)\n {\n \tDIR *dir;\n \tstruct dirent *dent;\n+\tchar name[PATH_MAX];\n+\tchar *subs[] = { \"cur\", \"new\", NULL };\n+\tchar **sub;\n+\n+\tfor (sub = subs; *sub; ++sub) {\n+\t\tsnprintf(name, sizeof(name), \"%s/%s\", path, *sub);\n+\t\tif ((dir = opendir(name)) == NULL) {\n+\t\t\tif (errno == ENOENT)\n+\t\t\t\tcontinue;\n+\t\t\terror(\"cannot opendir %s (%s)\", name, strerror(errno));\n+\t\t\treturn -1;\n+\t\t}\n \n-\tif ((dir = opendir(path)) == NULL) {\n-\t\terror(\"cannot opendir %s (%s)\", path, strerror(errno));\n-\t\treturn -1;\n-\t}\n+\t\twhile ((dent = readdir(dir)) != NULL) {\n+\t\t\tif (dent->d_name[0] == '.')\n+\t\t\t\tcontinue;\n+\t\t\tsnprintf(name, sizeof(name), \"%s/%s\", *sub, dent->d_name);\n+\t\t\tpath_list_insert(name, list);\n+\t\t}\n \n-\twhile ((dent = readdir(dir)) != NULL) {\n-\t\tif (dent->d_name[0] == '.')\n-\t\t\tcontinue;\n-\t\tpath_list_insert(dent->d_name, list);\n+\t\tclosedir(dir);\n \t}\n \n-\tclosedir(dir);\n-\n \treturn 0;\n }\n \n@@ -122,19 +131,17 @@ static int split_maildir(const char *maildir, const char *dir,\n \tint nr_prec, int skip)\n {\n \tchar file[PATH_MAX];\n-\tchar curdir[PATH_MAX];\n \tchar name[PATH_MAX];\n \tint ret = -1;\n \tint i;\n \tstruct path_list list = {NULL, 0, 0, 1};\n \n-\tsnprintf(curdir, sizeof(curdir), \"%s/cur\", maildir);\n-\tif (populate_maildir_list(&list, curdir) < 0)\n+\tif (populate_maildir_list(&list, maildir) < 0)\n \t\tgoto out;\n \n \tfor (i = 0; i < list.nr; i++) {\n \t\tFILE *f;\n-\t\tsnprintf(file, sizeof(file), \"%s/%s\", curdir, list.items[i].path);\n+\t\tsnprintf(file, sizeof(file), \"%s/%s\", maildir, list.items[i].path);\n \t\tf = fopen(file, \"r\");\n \t\tif (!f) {\n \t\t\terror(\"cannot open mail %s (%s)\", file, strerror(errno));\n@@ -152,10 +159,9 @@ static int split_maildir(const char *maildir, const char *dir,\n \t\tfclose(f);\n \t}\n \n-\tpath_list_clear(&list, 1);\n-\n \tret = skip;\n out:\n+\tpath_list_clear(&list, 1);\n \treturn ret;\n }\n \n-- \n1.5.3.5\n"},{"id":"58523","messageId":"Pine.LNX.4.64.0711061100150.4362@racer.site","threadId":"10474","inReplyTo":"20071106075150.GA21694@sigill.intra.peff.net","subject":"Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-06T11:01:03Z","receivedAt":"2007-11-06T11:01:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 6 Nov 2007, Jeff King wrote:\n\n> On Tue, Nov 06, 2007 at 08:28:31AM +0100, Alex Riesen wrote:\n> \n> > > In Maildir format, cur and new hold the mails. :P\n> > \n> > So? Why *STOP* reading the mails if just one of the directories could \n> > not be opened? IOW, I suggest:\n> \n> Because you are then trying to apply a patch series with some patches \n> potentially missing? Continuing only on errno == ENOENT seems prudent.\n\nI fail to see how the absence of one of cur/ or new/ can lead to the \nabsence of patches.  You could forget to save some patches, yes, but the \npresence of cur/ and new/ is no indicator for that.\n\nCiao,\nDscho\n"},{"id":"58546","messageId":"20071106154740.GA24505@sigill.intra.peff.net","threadId":"10474","inReplyTo":"Pine.LNX.4.64.0711061100150.4362@racer.site","subject":"Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-06T15:47:40Z","receivedAt":"2007-11-06T15:47:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 06, 2007 at 11:01:03AM +0000, Johannes Schindelin wrote:\n\n> > > So? Why *STOP* reading the mails if just one of the directories could \n> > > not be opened? IOW, I suggest:\n> > \n> > Because you are then trying to apply a patch series with some patches \n> > potentially missing? Continuing only on errno == ENOENT seems prudent.\n> \n> I fail to see how the absence of one of cur/ or new/ can lead to the \n> absence of patches.  You could forget to save some patches, yes, but the \n> presence of cur/ and new/ is no indicator for that.\n\nRead my message again. Alex is proposing ignoring errors in opening the\ndirectories; I am proposing ignoring such errors _only_ when the error\nis that the directory does not exist.\n\nIOW, if there is some other error in opening the directory, it should be\nfatal, because you might be missing patches.\n\n-Peff\n"},{"id":"58547","messageId":"Pine.LNX.4.64.0711061550580.4362@racer.site","threadId":"10474","inReplyTo":"20071106154740.GA24505@sigill.intra.peff.net","subject":"Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-06T15:51:09Z","receivedAt":"2007-11-06T15:51:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 6 Nov 2007, Jeff King wrote:\n\n> On Tue, Nov 06, 2007 at 11:01:03AM +0000, Johannes Schindelin wrote:\n> \n> > > > So? Why *STOP* reading the mails if just one of the directories could \n> > > > not be opened? IOW, I suggest:\n> > > \n> > > Because you are then trying to apply a patch series with some patches \n> > > potentially missing? Continuing only on errno == ENOENT seems prudent.\n> > \n> > I fail to see how the absence of one of cur/ or new/ can lead to the \n> > absence of patches.  You could forget to save some patches, yes, but the \n> > presence of cur/ and new/ is no indicator for that.\n> \n> Read my message again. Alex is proposing ignoring errors in opening the\n> directories; I am proposing ignoring such errors _only_ when the error\n> is that the directory does not exist.\n> \n> IOW, if there is some other error in opening the directory, it should be\n> fatal, because you might be missing patches.\n\nYeah, sorry, I missed that.\n\nCiao,\nDscho\n"},{"id":"58552","messageId":"20071106163548.GA8207@diana.vm.bytemark.co.uk","threadId":"10474","inReplyTo":"Pine.LNX.4.64.0711061550580.4362@racer.site","subject":"Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-11-06T16:35:48Z","receivedAt":"2007-11-06T16:35:48Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2007-11-06 15:51:09 +0000, Johannes Schindelin wrote:\n\n> On Tue, 6 Nov 2007, Jeff King wrote:\n>\n> > On Tue, Nov 06, 2007 at 11:01:03AM +0000, Johannes Schindelin wrote:\n> >\n> > > I fail to see how the absence of one of cur/ or new/ can lead to\n> > > the absence of patches. You could forget to save some patches,\n> > > yes, but the presence of cur/ and new/ is no indicator for that.\n> >\n> > Read my message again. Alex is proposing ignoring errors in\n> > opening the directories; I am proposing ignoring such errors\n> > _only_ when the error is that the directory does not exist.\n> >\n> > IOW, if there is some other error in opening the directory, it\n> > should be fatal, because you might be missing patches.\n>\n> Yeah, sorry, I missed that.\n\nI think it might actually not be totally unreasonable to error out\nunless both directories exist. From\nhttp://www.qmail.org/qmail-manual-html/man5/maildir.html:\n\n  A directory in maildir format has three subdirectories, all on the\n  same filesystem: tmp, new, and cur.\n\nIn other words, if it doesn't have these three directories, it isn't a\nMaildir directory.\n\nOn the other hand, one could argue that requiring both dirs to exist\nis being too picky.\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"58551","messageId":"Pine.LNX.4.64.0711061658050.4362@racer.site","threadId":"10474","inReplyTo":"20071106163548.GA8207@diana.vm.bytemark.co.uk","subject":"Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-06T16:58:48Z","receivedAt":"2007-11-06T16:58:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 6 Nov 2007, Karl Hasselstr?m wrote:\n\n> On 2007-11-06 15:51:09 +0000, Johannes Schindelin wrote:\n> \n> > On Tue, 6 Nov 2007, Jeff King wrote:\n> >\n> > > On Tue, Nov 06, 2007 at 11:01:03AM +0000, Johannes Schindelin wrote:\n> > >\n> > > > I fail to see how the absence of one of cur/ or new/ can lead to\n> > > > the absence of patches. You could forget to save some patches,\n> > > > yes, but the presence of cur/ and new/ is no indicator for that.\n> > >\n> > > Read my message again. Alex is proposing ignoring errors in\n> > > opening the directories; I am proposing ignoring such errors\n> > > _only_ when the error is that the directory does not exist.\n> > >\n> > > IOW, if there is some other error in opening the directory, it\n> > > should be fatal, because you might be missing patches.\n> >\n> > Yeah, sorry, I missed that.\n> \n> I think it might actually not be totally unreasonable to error out\n> unless both directories exist. From\n> http://www.qmail.org/qmail-manual-html/man5/maildir.html:\n> \n>   A directory in maildir format has three subdirectories, all on the\n>   same filesystem: tmp, new, and cur.\n> \n> In other words, if it doesn't have these three directories, it isn't a\n> Maildir directory.\n> \n> On the other hand, one could argue that requiring both dirs to exist\n> is being too picky.\n\nNot only that.  The recent patch for OSX' mail program would be trivial \nif we did not error out: the array would just contain cur, new and \nMessages.\n\nCiao,\nDscho\n"},{"id":"58599","messageId":"20071106215058.GA3654@steel.home","threadId":"10474","inReplyTo":"20071106163548.GA8207@diana.vm.bytemark.co.uk","subject":"Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-11-06T21:50:58Z","receivedAt":"2007-11-06T21:50:58Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Karl Hasselström, Tue, Nov 06, 2007 17:35:48 +0100:\n> On 2007-11-06 15:51:09 +0000, Johannes Schindelin wrote:\n> > On Tue, 6 Nov 2007, Jeff King wrote:\n> > > On Tue, Nov 06, 2007 at 11:01:03AM +0000, Johannes Schindelin wrote:\n> > > > I fail to see how the absence of one of cur/ or new/ can lead to\n> > > > the absence of patches. You could forget to save some patches,\n> > > > yes, but the presence of cur/ and new/ is no indicator for that.\n> > >\n> > > Read my message again. Alex is proposing ignoring errors in\n> > > opening the directories; I am proposing ignoring such errors\n> > > _only_ when the error is that the directory does not exist.\n> > >\n> > > IOW, if there is some other error in opening the directory, it\n> > > should be fatal, because you might be missing patches.\n> >\n> > Yeah, sorry, I missed that.\n> \n> I think it might actually not be totally unreasonable to error out\n> unless both directories exist. From\n> http://www.qmail.org/qmail-manual-html/man5/maildir.html:\n> \n>   A directory in maildir format has three subdirectories, all on the\n>   same filesystem: tmp, new, and cur.\n> \n> In other words, if it doesn't have these three directories, it isn't a\n> Maildir directory.\n\nOn the same line of reasoning, if opening a \".../cur\" fails with ENOTDIR,\nit must be not a Maildir...\n\n> On the other hand, one could argue that requiring both dirs to exist\n> is being too picky.\n\n...which MUST NOT mean it does not contain useful patches.\nIOW, the tool can try and apply everything it finds.\nIf user told it to get patches from the...whatever, then the patches\nshould it get and damn qmail.\n"},{"id":"58788","messageId":"7vfxzh7ajt.fsf@gitster.siamese.dyndns.org","threadId":"10474","inReplyTo":"20071106085418.14211.qmail@54d7c9212e25c5.315fe32.mid.smarden.org","subject":"Re: [PATCH amend] git-mailsplit: with maildirs not only process cur/, but also new/","fromName":"Junio C Hamano","fromEmail":"junio@pobox.com","sentAt":"2007-11-08T02:09:26Z","receivedAt":"2007-11-08T02:09:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Gerrit Pape <pape@smarden.org> writes:\n\n> When saving patches to a maildir with e.g. mutt, the files are put into\n> the new/ subdirectory of the maildir, not cur/.  This makes git-am state\n> \"Nothing to do.\".  This patch lets git-mailsplit additional check new/\n> after reading cur/.\n>\n> This was reported by Joey Hess through\n>  http://bugs.debian.org/447396\n>\n> Signed-off-by: Gerrit Pape <pape@smarden.org>\n> ---\n>\n> On Mon, Nov 05, 2007 at 01:58:50PM +0100, Jakub Narebski wrote:\n>> > +        for (i = 0; i < 2; ++i) {\n>> Wouldn't it be better to use sizeof(sub)/sizeof(sub[0]) or it's macro\n>> equivalent ARRAY_SIZE(sub) instead of hardcoding 2 to avoid errors?\n> I made the array NULL-terminated.\n>\n> On Mon, Nov 05, 2007 at 04:26:24PM -0500, Jeff King wrote:\n>> Isn't the subject line now wrong?\n> Yes, thanks.\n>\n> On Mon, Nov 05, 2007 at 11:52:58PM +0100, Alex Riesen wrote:\n>> Why is missing \"cur\" (or \"new\", for that matter) a fatal error?\n>> Why is it error at all? How about just ignoring the fact?\n> As suggested by Jeff, I made it ignore the error on ENOENT.\n\nLooks good to me.  Final acks please?\n"},{"id":"58789","messageId":"20071108023109.GA3564@sigill.intra.peff.net","threadId":"10474","inReplyTo":"7vfxzh7ajt.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH amend] git-mailsplit: with maildirs not only process cur/, but also new/","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-08T02:31:10Z","receivedAt":"2007-11-08T02:31:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 07, 2007 at 06:09:26PM -0800, Junio C Hamano wrote:\n\n> > When saving patches to a maildir with e.g. mutt, the files are put into\n> > the new/ subdirectory of the maildir, not cur/.  This makes git-am state\n> > \"Nothing to do.\".  This patch lets git-mailsplit additional check new/\n> > after reading cur/.\n> \n> Looks good to me.  Final acks please?\n\nFixed my concerns.\n\nAcked-by: Jeff King <peff@peff.net>\n\n-Peff\n"},{"id":"58806","messageId":"20071108072413.GB3170@steel.home","threadId":"10474","inReplyTo":"7vfxzh7ajt.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH amend] git-mailsplit: with maildirs not only process cur/, but also new/","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-11-08T07:24:13Z","receivedAt":"2007-11-08T07:24:13Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Junio C Hamano, Thu, Nov 08, 2007 03:09:26 +0100:\n> Gerrit Pape <pape@smarden.org> writes:\n> \n> > When saving patches to a maildir with e.g. mutt, the files are put into\n> > the new/ subdirectory of the maildir, not cur/.  This makes git-am state\n> > \"Nothing to do.\".  This patch lets git-mailsplit additional check new/\n> > after reading cur/.\n> >\n> > This was reported by Joey Hess through\n> >  http://bugs.debian.org/447396\n> >\n> > Signed-off-by: Gerrit Pape <pape@smarden.org>\n> > ---\n> >\n> > On Mon, Nov 05, 2007 at 01:58:50PM +0100, Jakub Narebski wrote:\n> >> > +        for (i = 0; i < 2; ++i) {\n> >> Wouldn't it be better to use sizeof(sub)/sizeof(sub[0]) or it's macro\n> >> equivalent ARRAY_SIZE(sub) instead of hardcoding 2 to avoid errors?\n> > I made the array NULL-terminated.\n> >\n> > On Mon, Nov 05, 2007 at 04:26:24PM -0500, Jeff King wrote:\n> >> Isn't the subject line now wrong?\n> > Yes, thanks.\n> >\n> > On Mon, Nov 05, 2007 at 11:52:58PM +0100, Alex Riesen wrote:\n> >> Why is missing \"cur\" (or \"new\", for that matter) a fatal error?\n> >> Why is it error at all? How about just ignoring the fact?\n> > As suggested by Jeff, I made it ignore the error on ENOENT.\n\nBetter.\n\n> Looks good to me.  Final acks please?\n\nAcked-by: Alex Riesen <raa.lkml@gmail.com>\n"},{"id":"58808","messageId":"20071108073101.GA4875@ferdyx.org","threadId":"10474","inReplyTo":"7vfxzh7ajt.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH amend] git-mailsplit: with maildirs not only process cur/, but also new/","fromName":"Fernando J. Pereda","fromEmail":"ferdy@gentoo.org","sentAt":"2007-11-08T07:31:01Z","receivedAt":"2007-11-08T07:31:01Z","isPatch":true,"sender":{"key":"ferdy@gentoo.org","avatar":null},"body":"On Wed, Nov 07, 2007 at 06:09:26PM -0800, Junio C Hamano wrote:\n> Gerrit Pape <pape@smarden.org> writes:\n> \n> > When saving patches to a maildir with e.g. mutt, the files are put into\n> > the new/ subdirectory of the maildir, not cur/.  This makes git-am state\n> > \"Nothing to do.\".  This patch lets git-mailsplit additional check new/\n> > after reading cur/.\n> >\n> > This was reported by Joey Hess through\n> >  http://bugs.debian.org/447396\n> >\n> > Signed-off-by: Gerrit Pape <pape@smarden.org>\n> > ---\n> >\n> > On Mon, Nov 05, 2007 at 01:58:50PM +0100, Jakub Narebski wrote:\n> >> > +        for (i = 0; i < 2; ++i) {\n> >> Wouldn't it be better to use sizeof(sub)/sizeof(sub[0]) or it's macro\n> >> equivalent ARRAY_SIZE(sub) instead of hardcoding 2 to avoid errors?\n> > I made the array NULL-terminated.\n> >\n> > On Mon, Nov 05, 2007 at 04:26:24PM -0500, Jeff King wrote:\n> >> Isn't the subject line now wrong?\n> > Yes, thanks.\n> >\n> > On Mon, Nov 05, 2007 at 11:52:58PM +0100, Alex Riesen wrote:\n> >> Why is missing \"cur\" (or \"new\", for that matter) a fatal error?\n> >> Why is it error at all? How about just ignoring the fact?\n> > As suggested by Jeff, I made it ignore the error on ENOENT.\n> \n> Looks good to me.  Final acks please?\n\nFixed my concern too.\n\nAcked-by: Fernando J. Pereda <ferdy@gentoo.org>\n\n-- \nFernando J. Pereda Garcimartín\n20BB BDC3 761A 4781 E6ED  ED0B 0A48 5B0C 60BD 28D4\n"}]}