[PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty

Previous thread: [PATCH] hooks-pre-commit: use \t, rather than a literal TAB in regexp by Jim Meyering on Friday, October 26, 2007 - 3:48 am. (1 message)

Next thread: Trouble with case-insensitive filesystems by Rocco Rutte on Friday, October 26, 2007 - 7:52 am. (5 messages)
From: Gerrit Pape
Date: Friday, October 26, 2007 - 7:15 am

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

-

From: Fernando J. Pereda
Date: Friday, October 26, 2007 - 9:01 am

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

--=20
Fernando J. Pereda Garcimart=EDn
20BB BDC3 761A 4781 E6ED  ED0B 0A48 5B0C 60BD 28D4
From: Gerrit Pape
Date: Monday, November 5, 2007 - 5:49 am

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>
---

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 < ...
From: Jeff King
Date: Monday, November 5, 2007 - 2:26 pm

Isn't the subject line now wrong?

-Peff
-

From: Alex Riesen
Date: Monday, November 5, 2007 - 3:52 pm

Why is missing "cur" (or "new", for that matter) a fatal error?
Why is it error at all? How about just ignoring the fact?

-

From: Gerrit Pape
Date: Tuesday, November 6, 2007 - 1:54 am

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>
---

I made the array NULL-terminated.

Yes, thanks.

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, ...
From: Junio C Hamano
Date: Wednesday, November 7, 2007 - 7:09 pm

Looks good to me.  Final acks please?
-

From: Jeff King
Date: Wednesday, November 7, 2007 - 7:31 pm

Fixed my concerns.

Acked-by: Jeff King <peff@peff.net>

-Peff
-

From: Alex Riesen
Date: Thursday, November 8, 2007 - 12:24 am

Acked-by: Alex Riesen <raa.lkml@gmail.com>

-

From: Fernando J. Pereda
Date: Thursday, November 8, 2007 - 12:31 am

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

-

From: Michael Cohen
Date: Monday, November 5, 2007 - 6:41 pm

In Maildir format, cur and new hold the mails. :P

-mjc
-

From: Alex Riesen
Date: Tuesday, November 6, 2007 - 12:28 am

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;
-

From: Jeff King
Date: Tuesday, November 6, 2007 - 12:51 am

Because you are then trying to apply a patch series with some patches
potentially missing? Continuing only on errno == ENOENT seems prudent.

-Peff
-

From: Johannes Schindelin
Date: Tuesday, November 6, 2007 - 4:01 am

Hi,


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

-

From: Jeff King
Date: Tuesday, November 6, 2007 - 8:47 am

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
-

From: Johannes Schindelin
Date: Tuesday, November 6, 2007 - 8:51 am

Hi,


Yeah, sorry, I missed that.

Ciao,
Dscho

-

From: Karl
Date: Tuesday, November 6, 2007 - 9:35 am

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
-

From: Johannes Schindelin
Date: Tuesday, November 6, 2007 - 9:58 am

Hi,


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

-

From: Alex Riesen
Date: Tuesday, November 6, 2007 - 2:50 pm

On the same line of reasoning, if opening a ".../cur" fails with ENOTDIR,

...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.

-

Previous thread: [PATCH] hooks-pre-commit: use \t, rather than a literal TAB in regexp by Jim Meyering on Friday, October 26, 2007 - 3:48 am. (1 message)

Next thread: Trouble with case-insensitive filesystems by Rocco Rutte on Friday, October 26, 2007 - 7:52 am. (5 messages)