* [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty @ 2007-10-26 14:15 Gerrit Pape 2007-10-26 16:01 ` Fernando J. Pereda 0 siblings, 1 reply; 20+ messages in thread From: Gerrit Pape @ 2007-10-26 14:15 UTC (permalink / raw) To: git, Junio C Hamano When saving patches to a maildir with e.g. mutt, the files are put into the new/ subdirectory of the maildir, not cur/. This makes git-am state "Nothing to do.". This patch lets git-mailsplit fallback to new/ if the cur/ subdirectory is empty. This was reported by Joey Hess through http://bugs.debian.org/447396 Signed-off-by: Gerrit Pape <pape@smarden.org> --- Documentation/git-am.txt | 3 ++- builtin-mailsplit.c | 5 +++++ 2 files changed, 7 insertions(+), 1 deletions(-) diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt index e4a6b3a..49f79f6 100644 --- a/Documentation/git-am.txt +++ b/Documentation/git-am.txt @@ -26,7 +26,8 @@ OPTIONS <mbox>|<Maildir>...:: The list of mailbox files to read patches from. If you do not supply this argument, reads from the standard input. If you supply - directories, they'll be treated as Maildirs. + directories, they'll be treated as Maildirs, which should contain + the patches either in the cur/ subdirectory, or in new/. -s, --signoff:: Add `Signed-off-by:` line to the commit message, using diff --git a/builtin-mailsplit.c b/builtin-mailsplit.c index 43fc373..eaf3cbe 100644 --- a/builtin-mailsplit.c +++ b/builtin-mailsplit.c @@ -131,6 +131,11 @@ static int split_maildir(const char *maildir, const char *dir, snprintf(curdir, sizeof(curdir), "%s/cur", maildir); if (populate_maildir_list(&list, curdir) < 0) goto out; + if (list.nr == 0) { + snprintf(curdir, sizeof(curdir), "%s/new", maildir); + if (populate_maildir_list(&list, curdir) < 0) + goto out; + } for (i = 0; i < list.nr; i++) { FILE *f; -- 1.5.3.4 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty 2007-10-26 14:15 [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty Gerrit Pape @ 2007-10-26 16:01 ` Fernando J. Pereda 2007-11-05 12:49 ` Gerrit Pape 0 siblings, 1 reply; 20+ messages in thread From: Fernando J. Pereda @ 2007-10-26 16:01 UTC (permalink / raw) To: Gerrit Pape; +Cc: git, Junio C Hamano [-- Attachment #1: Type: text/plain, Size: 693 bytes --] On Fri, Oct 26, 2007 at 02:15:39PM +0000, Gerrit Pape wrote: > When saving patches to a maildir with e.g. mutt, the files are put into > the new/ subdirectory of the maildir, not cur/. This makes git-am state > "Nothing to do.". This patch lets git-mailsplit fallback to new/ if the > cur/ subdirectory is empty. > > This was reported by Joey Hess through > http://bugs.debian.org/447396 > By that reasoning, you should make it parse both cur/ and new/. This didn't bit me because I always check mails I queue, so they ended up in cur/. Other than that, ack from me. - ferdy -- Fernando J. Pereda Garcimartín 20BB BDC3 761A 4781 E6ED ED0B 0A48 5B0C 60BD 28D4 [-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty 2007-10-26 16:01 ` Fernando J. Pereda @ 2007-11-05 12:49 ` Gerrit Pape 2007-11-05 12:58 ` Jakub Narebski ` (2 more replies) 0 siblings, 3 replies; 20+ messages in thread From: Gerrit Pape @ 2007-11-05 12:49 UTC (permalink / raw) To: Fernando J. Pereda, git, Junio C Hamano When saving patches to a maildir with e.g. mutt, the files are put into the new/ subdirectory of the maildir, not cur/. This makes git-am state "Nothing to do.". This patch lets git-mailsplit additional check new/ after reading cur/. This was reported by Joey Hess through http://bugs.debian.org/447396 Signed-off-by: Gerrit Pape <pape@smarden.org> --- On Fri, Oct 26, 2007 at 06:01:18PM +0200, Fernando J. Pereda wrote: > By that reasoning, you should make it parse both cur/ and new/. Okay. builtin-mailsplit.c | 36 ++++++++++++++++++++---------------- 1 files changed, 20 insertions(+), 16 deletions(-) diff --git a/builtin-mailsplit.c b/builtin-mailsplit.c index 74b0470..79e8ee0 100644 --- a/builtin-mailsplit.c +++ b/builtin-mailsplit.c @@ -101,19 +101,26 @@ static int populate_maildir_list(struct path_list *list, const char *path) { DIR *dir; struct dirent *dent; + char name[PATH_MAX]; + char *sub[] = { "cur", "new" }; + int i; - if ((dir = opendir(path)) == NULL) { - error("cannot opendir %s (%s)", path, strerror(errno)); - return -1; - } + for (i = 0; i < 2; ++i) { + snprintf(name, sizeof(name), "%s/%s", path, sub[i]); + if ((dir = opendir(name)) == NULL) { + error("cannot opendir %s (%s)", name, strerror(errno)); + return -1; + } - while ((dent = readdir(dir)) != NULL) { - if (dent->d_name[0] == '.') - continue; - path_list_insert(dent->d_name, list); - } + while ((dent = readdir(dir)) != NULL) { + if (dent->d_name[0] == '.') + continue; + snprintf(name, sizeof(name), "%s/%s", sub[i], dent->d_name); + path_list_insert(name, list); + } - closedir(dir); + closedir(dir); + } return 0; } @@ -122,19 +129,17 @@ static int split_maildir(const char *maildir, const char *dir, int nr_prec, int skip) { char file[PATH_MAX]; - char curdir[PATH_MAX]; char name[PATH_MAX]; int ret = -1; int i; struct path_list list = {NULL, 0, 0, 1}; - snprintf(curdir, sizeof(curdir), "%s/cur", maildir); - if (populate_maildir_list(&list, curdir) < 0) + if (populate_maildir_list(&list, maildir) < 0) goto out; for (i = 0; i < list.nr; i++) { FILE *f; - snprintf(file, sizeof(file), "%s/%s", curdir, list.items[i].path); + snprintf(file, sizeof(file), "%s/%s", maildir, list.items[i].path); f = fopen(file, "r"); if (!f) { error("cannot open mail %s (%s)", file, strerror(errno)); @@ -152,10 +157,9 @@ static int split_maildir(const char *maildir, const char *dir, fclose(f); } - path_list_clear(&list, 1); - ret = skip; out: + path_list_clear(&list, 1); return ret; } -- 1.5.3.5 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty 2007-11-05 12:49 ` Gerrit Pape @ 2007-11-05 12:58 ` Jakub Narebski 2007-11-05 21:26 ` Jeff King 2007-11-05 22:52 ` Alex Riesen 2 siblings, 0 replies; 20+ messages in thread From: Jakub Narebski @ 2007-11-05 12:58 UTC (permalink / raw) To: git [Cc: Gerrit Pape <pape@smarden.org>, git@vger.kernel.org] Gerrit Pape wrote: > + char *sub[] = { "cur", "new" }; [...] > + for (i = 0; i < 2; ++i) { Wouldn't it be better to use sizeof(sub)/sizeof(sub[0]) or it's macro equivalent ARRAY_SIZE(sub) instead of hardcoding 2 to avoid errors? -- Jakub Narebski Warsaw, Poland ShadeHawk on #git ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty 2007-11-05 12:49 ` Gerrit Pape 2007-11-05 12:58 ` Jakub Narebski @ 2007-11-05 21:26 ` Jeff King 2007-11-05 22:52 ` Alex Riesen 2 siblings, 0 replies; 20+ messages in thread From: Jeff King @ 2007-11-05 21:26 UTC (permalink / raw) To: Gerrit Pape; +Cc: Fernando J. Pereda, git, Junio C Hamano On Mon, Nov 05, 2007 at 12:49:20PM +0000, Gerrit Pape wrote: > On Fri, Oct 26, 2007 at 06:01:18PM +0200, Fernando J. Pereda wrote: > > By that reasoning, you should make it parse both cur/ and new/. > Okay. Isn't the subject line now wrong? -Peff ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty 2007-11-05 12:49 ` Gerrit Pape 2007-11-05 12:58 ` Jakub Narebski 2007-11-05 21:26 ` Jeff King @ 2007-11-05 22:52 ` Alex Riesen 2007-11-06 1:41 ` Michael Cohen 2007-11-06 8:54 ` [PATCH amend] git-mailsplit: with maildirs not only process cur/, but also new/ Gerrit Pape 2 siblings, 2 replies; 20+ messages in thread From: Alex Riesen @ 2007-11-05 22:52 UTC (permalink / raw) To: Gerrit Pape; +Cc: Fernando J. Pereda, git, Junio C Hamano Gerrit Pape, Mon, Nov 05, 2007 13:49:20 +0100: > + for (i = 0; i < 2; ++i) { > + snprintf(name, sizeof(name), "%s/%s", path, sub[i]); > + if ((dir = opendir(name)) == NULL) { > + error("cannot opendir %s (%s)", name, strerror(errno)); > + return -1; > + } Why is missing "cur" (or "new", for that matter) a fatal error? Why is it error at all? How about just ignoring the fact? ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty 2007-11-05 22:52 ` Alex Riesen @ 2007-11-06 1:41 ` Michael Cohen 2007-11-06 7:28 ` Alex Riesen 2007-11-06 8:54 ` [PATCH amend] git-mailsplit: with maildirs not only process cur/, but also new/ Gerrit Pape 1 sibling, 1 reply; 20+ messages in thread From: Michael Cohen @ 2007-11-06 1:41 UTC (permalink / raw) To: Alex Riesen; +Cc: Gerrit Pape, Fernando J. Pereda, git, Junio C Hamano On Nov 5, 2007, at 5:52 PM, Alex Riesen wrote: > Gerrit Pape, Mon, Nov 05, 2007 13:49:20 +0100: >> + for (i = 0; i < 2; ++i) { >> + snprintf(name, sizeof(name), "%s/%s", path, sub[i]); >> + if ((dir = opendir(name)) == NULL) { >> + error("cannot opendir %s (%s)", name, strerror(errno)); >> + return -1; >> + } > > Why is missing "cur" (or "new", for that matter) a fatal error? > Why is it error at all? How about just ignoring the fact? In Maildir format, cur and new hold the mails. :P -mjc ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty 2007-11-06 1:41 ` Michael Cohen @ 2007-11-06 7:28 ` Alex Riesen 2007-11-06 7:51 ` Jeff King 0 siblings, 1 reply; 20+ messages in thread From: Alex Riesen @ 2007-11-06 7:28 UTC (permalink / raw) To: Michael Cohen; +Cc: Gerrit Pape, Fernando J. Pereda, git, Junio C Hamano Michael Cohen, Tue, Nov 06, 2007 02:41:56 +0100: > On Nov 5, 2007, at 5:52 PM, Alex Riesen wrote: > >> Gerrit Pape, Mon, Nov 05, 2007 13:49:20 +0100: >>> + for (i = 0; i < 2; ++i) { >>> + snprintf(name, sizeof(name), "%s/%s", path, sub[i]); >>> + if ((dir = opendir(name)) == NULL) { >>> + error("cannot opendir %s (%s)", name, strerror(errno)); >>> + return -1; >>> + } >> >> Why is missing "cur" (or "new", for that matter) a fatal error? >> Why is it error at all? How about just ignoring the fact? > In Maildir format, cur and new hold the mails. :P So? Why *STOP* reading the mails if just one of the directories could not be opened? IOW, I suggest: + for (i = 0; i < 2; ++i) { + snprintf(name, sizeof(name), "%s/%s", path, sub[i]); + dir = opendir(name); + if (!dir) + continue; ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty 2007-11-06 7:28 ` Alex Riesen @ 2007-11-06 7:51 ` Jeff King 2007-11-06 11:01 ` Johannes Schindelin 0 siblings, 1 reply; 20+ messages in thread From: Jeff King @ 2007-11-06 7:51 UTC (permalink / raw) To: Alex Riesen Cc: Michael Cohen, Gerrit Pape, Fernando J. Pereda, git, Junio C Hamano On Tue, Nov 06, 2007 at 08:28:31AM +0100, Alex Riesen wrote: > > In Maildir format, cur and new hold the mails. :P > > So? Why *STOP* reading the mails if just one of the directories could > not be opened? IOW, I suggest: Because you are then trying to apply a patch series with some patches potentially missing? Continuing only on errno == ENOENT seems prudent. -Peff ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty 2007-11-06 7:51 ` Jeff King @ 2007-11-06 11:01 ` Johannes Schindelin 2007-11-06 15:47 ` Jeff King 0 siblings, 1 reply; 20+ messages in thread From: Johannes Schindelin @ 2007-11-06 11:01 UTC (permalink / raw) To: Jeff King Cc: Alex Riesen, Michael Cohen, Gerrit Pape, Fernando J. Pereda, git, Junio C Hamano Hi, On Tue, 6 Nov 2007, Jeff King wrote: > On Tue, Nov 06, 2007 at 08:28:31AM +0100, Alex Riesen wrote: > > > > In Maildir format, cur and new hold the mails. :P > > > > So? Why *STOP* reading the mails if just one of the directories could > > not be opened? IOW, I suggest: > > Because you are then trying to apply a patch series with some patches > potentially missing? Continuing only on errno == ENOENT seems prudent. I fail to see how the absence of one of cur/ or new/ can lead to the absence of patches. You could forget to save some patches, yes, but the presence of cur/ and new/ is no indicator for that. Ciao, Dscho ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty 2007-11-06 11:01 ` Johannes Schindelin @ 2007-11-06 15:47 ` Jeff King 2007-11-06 15:51 ` Johannes Schindelin 0 siblings, 1 reply; 20+ messages in thread From: Jeff King @ 2007-11-06 15:47 UTC (permalink / raw) To: Johannes Schindelin Cc: Alex Riesen, Michael Cohen, Gerrit Pape, Fernando J. Pereda, git, Junio C Hamano On Tue, Nov 06, 2007 at 11:01:03AM +0000, Johannes Schindelin wrote: > > > So? Why *STOP* reading the mails if just one of the directories could > > > not be opened? IOW, I suggest: > > > > Because you are then trying to apply a patch series with some patches > > potentially missing? Continuing only on errno == ENOENT seems prudent. > > I fail to see how the absence of one of cur/ or new/ can lead to the > absence of patches. You could forget to save some patches, yes, but the > presence of cur/ and new/ is no indicator for that. Read my message again. Alex is proposing ignoring errors in opening the directories; I am proposing ignoring such errors _only_ when the error is that the directory does not exist. IOW, if there is some other error in opening the directory, it should be fatal, because you might be missing patches. -Peff ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty 2007-11-06 15:47 ` Jeff King @ 2007-11-06 15:51 ` Johannes Schindelin 2007-11-06 16:35 ` Karl Hasselström 0 siblings, 1 reply; 20+ messages in thread From: Johannes Schindelin @ 2007-11-06 15:51 UTC (permalink / raw) To: Jeff King Cc: Alex Riesen, Michael Cohen, Gerrit Pape, Fernando J. Pereda, git, Junio C Hamano Hi, On Tue, 6 Nov 2007, Jeff King wrote: > On Tue, Nov 06, 2007 at 11:01:03AM +0000, Johannes Schindelin wrote: > > > > > So? Why *STOP* reading the mails if just one of the directories could > > > > not be opened? IOW, I suggest: > > > > > > Because you are then trying to apply a patch series with some patches > > > potentially missing? Continuing only on errno == ENOENT seems prudent. > > > > I fail to see how the absence of one of cur/ or new/ can lead to the > > absence of patches. You could forget to save some patches, yes, but the > > presence of cur/ and new/ is no indicator for that. > > Read my message again. Alex is proposing ignoring errors in opening the > directories; I am proposing ignoring such errors _only_ when the error > is that the directory does not exist. > > IOW, if there is some other error in opening the directory, it should be > fatal, because you might be missing patches. Yeah, sorry, I missed that. Ciao, Dscho ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty 2007-11-06 15:51 ` Johannes Schindelin @ 2007-11-06 16:35 ` Karl Hasselström 2007-11-06 16:58 ` Johannes Schindelin 2007-11-06 21:50 ` Alex Riesen 0 siblings, 2 replies; 20+ messages in thread From: Karl Hasselström @ 2007-11-06 16:35 UTC (permalink / raw) To: Johannes Schindelin Cc: Jeff King, Alex Riesen, Michael Cohen, Gerrit Pape, Fernando J. Pereda, git, Junio C Hamano On 2007-11-06 15:51:09 +0000, Johannes Schindelin wrote: > On Tue, 6 Nov 2007, Jeff King wrote: > > > On Tue, Nov 06, 2007 at 11:01:03AM +0000, Johannes Schindelin wrote: > > > > > I fail to see how the absence of one of cur/ or new/ can lead to > > > the absence of patches. You could forget to save some patches, > > > yes, but the presence of cur/ and new/ is no indicator for that. > > > > Read my message again. Alex is proposing ignoring errors in > > opening the directories; I am proposing ignoring such errors > > _only_ when the error is that the directory does not exist. > > > > IOW, if there is some other error in opening the directory, it > > should be fatal, because you might be missing patches. > > Yeah, sorry, I missed that. I think it might actually not be totally unreasonable to error out unless both directories exist. From http://www.qmail.org/qmail-manual-html/man5/maildir.html: A directory in maildir format has three subdirectories, all on the same filesystem: tmp, new, and cur. In other words, if it doesn't have these three directories, it isn't a Maildir directory. On the other hand, one could argue that requiring both dirs to exist is being too picky. -- Karl Hasselström, kha@treskal.com www.treskal.com/kalle ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty 2007-11-06 16:35 ` Karl Hasselström @ 2007-11-06 16:58 ` Johannes Schindelin 2007-11-06 21:50 ` Alex Riesen 1 sibling, 0 replies; 20+ messages in thread From: Johannes Schindelin @ 2007-11-06 16:58 UTC (permalink / raw) To: Karl Hasselström Cc: Jeff King, Alex Riesen, Michael Cohen, Gerrit Pape, Fernando J. Pereda, git, Junio C Hamano Hi, On Tue, 6 Nov 2007, Karl Hasselstr?m wrote: > On 2007-11-06 15:51:09 +0000, Johannes Schindelin wrote: > > > On Tue, 6 Nov 2007, Jeff King wrote: > > > > > On Tue, Nov 06, 2007 at 11:01:03AM +0000, Johannes Schindelin wrote: > > > > > > > I fail to see how the absence of one of cur/ or new/ can lead to > > > > the absence of patches. You could forget to save some patches, > > > > yes, but the presence of cur/ and new/ is no indicator for that. > > > > > > Read my message again. Alex is proposing ignoring errors in > > > opening the directories; I am proposing ignoring such errors > > > _only_ when the error is that the directory does not exist. > > > > > > IOW, if there is some other error in opening the directory, it > > > should be fatal, because you might be missing patches. > > > > Yeah, sorry, I missed that. > > I think it might actually not be totally unreasonable to error out > unless both directories exist. From > http://www.qmail.org/qmail-manual-html/man5/maildir.html: > > A directory in maildir format has three subdirectories, all on the > same filesystem: tmp, new, and cur. > > In other words, if it doesn't have these three directories, it isn't a > Maildir directory. > > On the other hand, one could argue that requiring both dirs to exist > is being too picky. Not only that. The recent patch for OSX' mail program would be trivial if we did not error out: the array would just contain cur, new and Messages. Ciao, Dscho ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty 2007-11-06 16:35 ` Karl Hasselström 2007-11-06 16:58 ` Johannes Schindelin @ 2007-11-06 21:50 ` Alex Riesen 1 sibling, 0 replies; 20+ messages in thread From: Alex Riesen @ 2007-11-06 21:50 UTC (permalink / raw) To: Karl Hasselström Cc: Johannes Schindelin, Jeff King, Michael Cohen, Gerrit Pape, Fernando J. Pereda, git, Junio C Hamano Karl Hasselström, Tue, Nov 06, 2007 17:35:48 +0100: > On 2007-11-06 15:51:09 +0000, Johannes Schindelin wrote: > > On Tue, 6 Nov 2007, Jeff King wrote: > > > On Tue, Nov 06, 2007 at 11:01:03AM +0000, Johannes Schindelin wrote: > > > > I fail to see how the absence of one of cur/ or new/ can lead to > > > > the absence of patches. You could forget to save some patches, > > > > yes, but the presence of cur/ and new/ is no indicator for that. > > > > > > Read my message again. Alex is proposing ignoring errors in > > > opening the directories; I am proposing ignoring such errors > > > _only_ when the error is that the directory does not exist. > > > > > > IOW, if there is some other error in opening the directory, it > > > should be fatal, because you might be missing patches. > > > > Yeah, sorry, I missed that. > > I think it might actually not be totally unreasonable to error out > unless both directories exist. From > http://www.qmail.org/qmail-manual-html/man5/maildir.html: > > A directory in maildir format has three subdirectories, all on the > same filesystem: tmp, new, and cur. > > In other words, if it doesn't have these three directories, it isn't a > Maildir directory. On the same line of reasoning, if opening a ".../cur" fails with ENOTDIR, it must be not a Maildir... > On the other hand, one could argue that requiring both dirs to exist > is being too picky. ...which MUST NOT mean it does not contain useful patches. IOW, the tool can try and apply everything it finds. If user told it to get patches from the...whatever, then the patches should it get and damn qmail. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH amend] git-mailsplit: with maildirs not only process cur/, but also new/ 2007-11-05 22:52 ` Alex Riesen 2007-11-06 1:41 ` Michael Cohen @ 2007-11-06 8:54 ` Gerrit Pape 2007-11-08 2:09 ` Junio C Hamano 1 sibling, 1 reply; 20+ messages in thread From: Gerrit Pape @ 2007-11-06 8:54 UTC (permalink / raw) To: Alex Riesen Cc: Fernando J. Pereda, git, Junio C Hamano, Jakub Narebski, Jeff King, Alex Riesen When saving patches to a maildir with e.g. mutt, the files are put into the new/ subdirectory of the maildir, not cur/. This makes git-am state "Nothing to do.". This patch lets git-mailsplit additional check new/ after reading cur/. This was reported by Joey Hess through http://bugs.debian.org/447396 Signed-off-by: Gerrit Pape <pape@smarden.org> --- On Mon, Nov 05, 2007 at 01:58:50PM +0100, Jakub Narebski wrote: > > + for (i = 0; i < 2; ++i) { > Wouldn't it be better to use sizeof(sub)/sizeof(sub[0]) or it's macro > equivalent ARRAY_SIZE(sub) instead of hardcoding 2 to avoid errors? I made the array NULL-terminated. On Mon, Nov 05, 2007 at 04:26:24PM -0500, Jeff King wrote: > Isn't the subject line now wrong? Yes, thanks. On Mon, Nov 05, 2007 at 11:52:58PM +0100, Alex Riesen wrote: > Why is missing "cur" (or "new", for that matter) a fatal error? > Why is it error at all? How about just ignoring the fact? As suggested by Jeff, I made it ignore the error on ENOENT. builtin-mailsplit.c | 38 ++++++++++++++++++++++---------------- 1 files changed, 22 insertions(+), 16 deletions(-) diff --git a/builtin-mailsplit.c b/builtin-mailsplit.c index 74b0470..46b27cd 100644 --- a/builtin-mailsplit.c +++ b/builtin-mailsplit.c @@ -101,20 +101,29 @@ static int populate_maildir_list(struct path_list *list, const char *path) { DIR *dir; struct dirent *dent; + char name[PATH_MAX]; + char *subs[] = { "cur", "new", NULL }; + char **sub; + + for (sub = subs; *sub; ++sub) { + snprintf(name, sizeof(name), "%s/%s", path, *sub); + if ((dir = opendir(name)) == NULL) { + if (errno == ENOENT) + continue; + error("cannot opendir %s (%s)", name, strerror(errno)); + return -1; + } - if ((dir = opendir(path)) == NULL) { - error("cannot opendir %s (%s)", path, strerror(errno)); - return -1; - } + while ((dent = readdir(dir)) != NULL) { + if (dent->d_name[0] == '.') + continue; + snprintf(name, sizeof(name), "%s/%s", *sub, dent->d_name); + path_list_insert(name, list); + } - while ((dent = readdir(dir)) != NULL) { - if (dent->d_name[0] == '.') - continue; - path_list_insert(dent->d_name, list); + closedir(dir); } - closedir(dir); - return 0; } @@ -122,19 +131,17 @@ static int split_maildir(const char *maildir, const char *dir, int nr_prec, int skip) { char file[PATH_MAX]; - char curdir[PATH_MAX]; char name[PATH_MAX]; int ret = -1; int i; struct path_list list = {NULL, 0, 0, 1}; - snprintf(curdir, sizeof(curdir), "%s/cur", maildir); - if (populate_maildir_list(&list, curdir) < 0) + if (populate_maildir_list(&list, maildir) < 0) goto out; for (i = 0; i < list.nr; i++) { FILE *f; - snprintf(file, sizeof(file), "%s/%s", curdir, list.items[i].path); + snprintf(file, sizeof(file), "%s/%s", maildir, list.items[i].path); f = fopen(file, "r"); if (!f) { error("cannot open mail %s (%s)", file, strerror(errno)); @@ -152,10 +159,9 @@ static int split_maildir(const char *maildir, const char *dir, fclose(f); } - path_list_clear(&list, 1); - ret = skip; out: + path_list_clear(&list, 1); return ret; } -- 1.5.3.5 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH amend] git-mailsplit: with maildirs not only process cur/, but also new/ 2007-11-06 8:54 ` [PATCH amend] git-mailsplit: with maildirs not only process cur/, but also new/ Gerrit Pape @ 2007-11-08 2:09 ` Junio C Hamano 2007-11-08 2:31 ` Jeff King ` (2 more replies) 0 siblings, 3 replies; 20+ messages in thread From: Junio C Hamano @ 2007-11-08 2:09 UTC (permalink / raw) To: Alex Riesen Cc: Gerrit Pape, Fernando J. Pereda, git, Jakub Narebski, Jeff King Gerrit Pape <pape@smarden.org> writes: > When saving patches to a maildir with e.g. mutt, the files are put into > the new/ subdirectory of the maildir, not cur/. This makes git-am state > "Nothing to do.". This patch lets git-mailsplit additional check new/ > after reading cur/. > > This was reported by Joey Hess through > http://bugs.debian.org/447396 > > Signed-off-by: Gerrit Pape <pape@smarden.org> > --- > > On Mon, Nov 05, 2007 at 01:58:50PM +0100, Jakub Narebski wrote: >> > + for (i = 0; i < 2; ++i) { >> Wouldn't it be better to use sizeof(sub)/sizeof(sub[0]) or it's macro >> equivalent ARRAY_SIZE(sub) instead of hardcoding 2 to avoid errors? > I made the array NULL-terminated. > > On Mon, Nov 05, 2007 at 04:26:24PM -0500, Jeff King wrote: >> Isn't the subject line now wrong? > Yes, thanks. > > On Mon, Nov 05, 2007 at 11:52:58PM +0100, Alex Riesen wrote: >> Why is missing "cur" (or "new", for that matter) a fatal error? >> Why is it error at all? How about just ignoring the fact? > As suggested by Jeff, I made it ignore the error on ENOENT. Looks good to me. Final acks please? ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH amend] git-mailsplit: with maildirs not only process cur/, but also new/ 2007-11-08 2:09 ` Junio C Hamano @ 2007-11-08 2:31 ` Jeff King 2007-11-08 7:24 ` Alex Riesen 2007-11-08 7:31 ` Fernando J. Pereda 2 siblings, 0 replies; 20+ messages in thread From: Jeff King @ 2007-11-08 2:31 UTC (permalink / raw) To: Junio C Hamano Cc: Alex Riesen, Gerrit Pape, Fernando J. Pereda, git, Jakub Narebski On Wed, Nov 07, 2007 at 06:09:26PM -0800, Junio C Hamano wrote: > > When saving patches to a maildir with e.g. mutt, the files are put into > > the new/ subdirectory of the maildir, not cur/. This makes git-am state > > "Nothing to do.". This patch lets git-mailsplit additional check new/ > > after reading cur/. > > Looks good to me. Final acks please? Fixed my concerns. Acked-by: Jeff King <peff@peff.net> -Peff ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH amend] git-mailsplit: with maildirs not only process cur/, but also new/ 2007-11-08 2:09 ` Junio C Hamano 2007-11-08 2:31 ` Jeff King @ 2007-11-08 7:24 ` Alex Riesen 2007-11-08 7:31 ` Fernando J. Pereda 2 siblings, 0 replies; 20+ messages in thread From: Alex Riesen @ 2007-11-08 7:24 UTC (permalink / raw) To: Junio C Hamano Cc: Gerrit Pape, Fernando J. Pereda, git, Jakub Narebski, Jeff King Junio C Hamano, Thu, Nov 08, 2007 03:09:26 +0100: > Gerrit Pape <pape@smarden.org> writes: > > > When saving patches to a maildir with e.g. mutt, the files are put into > > the new/ subdirectory of the maildir, not cur/. This makes git-am state > > "Nothing to do.". This patch lets git-mailsplit additional check new/ > > after reading cur/. > > > > This was reported by Joey Hess through > > http://bugs.debian.org/447396 > > > > Signed-off-by: Gerrit Pape <pape@smarden.org> > > --- > > > > On Mon, Nov 05, 2007 at 01:58:50PM +0100, Jakub Narebski wrote: > >> > + for (i = 0; i < 2; ++i) { > >> Wouldn't it be better to use sizeof(sub)/sizeof(sub[0]) or it's macro > >> equivalent ARRAY_SIZE(sub) instead of hardcoding 2 to avoid errors? > > I made the array NULL-terminated. > > > > On Mon, Nov 05, 2007 at 04:26:24PM -0500, Jeff King wrote: > >> Isn't the subject line now wrong? > > Yes, thanks. > > > > On Mon, Nov 05, 2007 at 11:52:58PM +0100, Alex Riesen wrote: > >> Why is missing "cur" (or "new", for that matter) a fatal error? > >> Why is it error at all? How about just ignoring the fact? > > As suggested by Jeff, I made it ignore the error on ENOENT. Better. > Looks good to me. Final acks please? Acked-by: Alex Riesen <raa.lkml@gmail.com> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH amend] git-mailsplit: with maildirs not only process cur/, but also new/ 2007-11-08 2:09 ` Junio C Hamano 2007-11-08 2:31 ` Jeff King 2007-11-08 7:24 ` Alex Riesen @ 2007-11-08 7:31 ` Fernando J. Pereda 2 siblings, 0 replies; 20+ messages in thread From: Fernando J. Pereda @ 2007-11-08 7:31 UTC (permalink / raw) To: Junio C Hamano; +Cc: Alex Riesen, Gerrit Pape, git, Jakub Narebski, Jeff King On Wed, Nov 07, 2007 at 06:09:26PM -0800, Junio C Hamano wrote: > Gerrit Pape <pape@smarden.org> writes: > > > When saving patches to a maildir with e.g. mutt, the files are put into > > the new/ subdirectory of the maildir, not cur/. This makes git-am state > > "Nothing to do.". This patch lets git-mailsplit additional check new/ > > after reading cur/. > > > > This was reported by Joey Hess through > > http://bugs.debian.org/447396 > > > > Signed-off-by: Gerrit Pape <pape@smarden.org> > > --- > > > > On Mon, Nov 05, 2007 at 01:58:50PM +0100, Jakub Narebski wrote: > >> > + for (i = 0; i < 2; ++i) { > >> Wouldn't it be better to use sizeof(sub)/sizeof(sub[0]) or it's macro > >> equivalent ARRAY_SIZE(sub) instead of hardcoding 2 to avoid errors? > > I made the array NULL-terminated. > > > > On Mon, Nov 05, 2007 at 04:26:24PM -0500, Jeff King wrote: > >> Isn't the subject line now wrong? > > Yes, thanks. > > > > On Mon, Nov 05, 2007 at 11:52:58PM +0100, Alex Riesen wrote: > >> Why is missing "cur" (or "new", for that matter) a fatal error? > >> Why is it error at all? How about just ignoring the fact? > > As suggested by Jeff, I made it ignore the error on ENOENT. > > Looks good to me. Final acks please? Fixed my concern too. Acked-by: Fernando J. Pereda <ferdy@gentoo.org> -- Fernando J. Pereda Garcimartín 20BB BDC3 761A 4781 E6ED ED0B 0A48 5B0C 60BD 28D4 ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2007-11-08 7:31 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-10-26 14:15 [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty Gerrit Pape 2007-10-26 16:01 ` Fernando J. Pereda 2007-11-05 12:49 ` Gerrit Pape 2007-11-05 12:58 ` Jakub Narebski 2007-11-05 21:26 ` Jeff King 2007-11-05 22:52 ` Alex Riesen 2007-11-06 1:41 ` Michael Cohen 2007-11-06 7:28 ` Alex Riesen 2007-11-06 7:51 ` Jeff King 2007-11-06 11:01 ` Johannes Schindelin 2007-11-06 15:47 ` Jeff King 2007-11-06 15:51 ` Johannes Schindelin 2007-11-06 16:35 ` Karl Hasselström 2007-11-06 16:58 ` Johannes Schindelin 2007-11-06 21:50 ` Alex Riesen 2007-11-06 8:54 ` [PATCH amend] git-mailsplit: with maildirs not only process cur/, but also new/ Gerrit Pape 2007-11-08 2:09 ` Junio C Hamano 2007-11-08 2:31 ` Jeff King 2007-11-08 7:24 ` Alex Riesen 2007-11-08 7:31 ` Fernando J. Pereda
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).