[PATCH] Teach mailsplit about Maildir's

[PATCH] Teach mailsplit about Maildir's

[Fernando J. Pereda]

Signed-off-by: Fernando J. Pereda <ferdy@gentoo.org>
---

	I never got ACK or NACK when I sent it the first time, that's why
	I'm resending this one with (almost) no changes.

 Documentation/git-am.txt        |    8 ++-
 Documentation/git-mailsplit.txt |   13 +++-
 builtin-mailsplit.c             |  122 ++++++++++++++++++++++++++++++++------
 builtin.h                       |    2 +-
 4 files changed, 118 insertions(+), 27 deletions(-)

diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index ba79773..25cf84a 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -12,7 +12,8 @@ SYNOPSIS
 'git-am' [--signoff] [--dotest=<dir>] [--keep] [--utf8 | --no-utf8]
          [--3way] [--interactive] [--binary]
          [--whitespace=<option>] [-C<n>] [-p<n>]
-         <mbox>...
+         <mbox>|<Maildir>...
+
 'git-am' [--skip | --resolved]
 
 DESCRIPTION
@@ -23,9 +24,10 @@ current branch.
 
 OPTIONS
 -------
-<mbox>...::
+<mbox>|<Maildir>...::
 	The list of mailbox files to read patches from. If you do not
-	supply this argument, reads from the standard input.
+	supply this argument, reads from the standard input. If you supply
+	directories, they'll be treated as Maildirs.
 
 -s, --signoff::
 	Add `Signed-off-by:` line to the commit message, using
diff --git a/Documentation/git-mailsplit.txt b/Documentation/git-mailsplit.txt
index c11d6a5..abb0903 100644
--- a/Documentation/git-mailsplit.txt
+++ b/Documentation/git-mailsplit.txt
@@ -7,12 +7,15 @@ git-mailsplit - Simple UNIX mbox splitter program
 
 SYNOPSIS
 --------
-'git-mailsplit' [-b] [-f<nn>] [-d<prec>] -o<directory> [--] [<mbox>...]
+'git-mailsplit' [-b] [-f<nn>] [-d<prec>] -o<directory> [--] [<mbox>|<Maildir>...]
 
 DESCRIPTION
 -----------
-Splits a mbox file into a list of files: "0001" "0002" ..  in the specified
-directory so you can process them further from there.
+Splits a mbox file or a Maildir into a list of files: "0001" "0002" ..  in the
+specified directory so you can process them further from there.
+
+IMPORTANT: Maildir splitting relies upon filenames being sorted to output
+patches in the correct order.
 
 OPTIONS
 -------
@@ -20,6 +23,10 @@ OPTIONS
 	Mbox file to split.  If not given, the mbox is read from
 	the standard input.
 
+<Maildir>::
+	Root of the Maildir to split. This directory should contain the cur, tmp
+	and new subdirectories.
+
 <directory>::
 	Directory in which to place the individual messages.
 
diff --git a/builtin-mailsplit.c b/builtin-mailsplit.c
index 3bca855..1d096d6 100644
--- a/builtin-mailsplit.c
+++ b/builtin-mailsplit.c
@@ -6,9 +6,10 @@
  */
 #include "cache.h"
 #include "builtin.h"
+#include "path-list.h"
 
 static const char git_mailsplit_usage[] =
-"git-mailsplit [-d<prec>] [-f<n>] [-b] -o<directory> <mbox>...";
+"git-mailsplit [-d<prec>] [-f<n>] [-b] -o<directory> <mbox>|<Maildir>...";
 
 static int is_from_line(const char *line, int len)
 {
@@ -96,44 +97,106 @@ static int split_one(FILE *mbox, const char *name, int allow_bare)
 	exit(1);
 }
 
-int split_mbox(const char **mbox, const char *dir, int allow_bare, int nr_prec, int skip)
+static int populate_maildir_list(struct path_list *list, const char *path)
 {
-	char *name = xmalloc(strlen(dir) + 2 + 3 * sizeof(skip));
+	DIR *dir;
+	struct dirent *dent;
+
+	if ((dir = opendir(path)) == NULL) {
+		error("cannot diropen %s (%s)", path, strerror(errno));
+		return -1;
+	}
+
+	while ((dent = readdir(dir)) != NULL) {
+		if (dent->d_name[0] == '.')
+			continue;
+		path_list_insert(dent->d_name, list);
+	}
+
+	closedir(dir);
+
+	return 1;
+}
+
+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;
+	struct path_list list = {NULL, 0, 0, 1};
 
-	while (*mbox) {
-		const char *file = *mbox++;
-		FILE *f = !strcmp(file, "-") ? stdin : fopen(file, "r");
-		int file_done = 0;
+	snprintf(curdir, sizeof(curdir), "%s/cur", maildir);
+	if (populate_maildir_list(&list, curdir) < 0)
+		goto out;
 
-		if ( !f ) {
-			error("cannot open mbox %s", file);
+	int i;
+	for (i = 0; i < list.nr; i++) {
+		snprintf(file, sizeof(file), "%s/%s", curdir, list.items[i].path);
+		FILE *f = fopen(file, "r");
+		if (!f) {
+			error("cannot open mail %s (%s)", file, strerror(errno));
 			goto out;
 		}
 
 		if (fgets(buf, sizeof(buf), f) == NULL) {
-			if (f == stdin)
-				break; /* empty stdin is OK */
-			error("cannot read mbox %s", file);
+			error("cannot read mail %s (%s)", file, strerror(errno));
 			goto out;
 		}
 
-		while (!file_done) {
-			sprintf(name, "%s/%0*d", dir, nr_prec, ++skip);
-			file_done = split_one(f, name, allow_bare);
+		sprintf(name, "%s/%0*d", dir, nr_prec, ++skip);
+		split_one(f, name, 1);
+
+		fclose(f);
+	}
+
+	path_list_clear(&list, 1);
+
+	ret = skip;
+out:
+	return ret;
+}
+
+int split_mbox(const char *file, const char *dir, int allow_bare,
+		int nr_prec, int skip)
+{
+	char name[PATH_MAX];
+	int ret = -1;
+
+	FILE *f = !strcmp(file, "-") ? stdin : fopen(file, "r");
+	int file_done = 0;
+
+	if (!f) {
+		error("cannot open mbox %s", file);
+		goto out;
+	}
+
+	if (fgets(buf, sizeof(buf), f) == NULL) {
+		/* empty stdin is OK */
+		if (f != stdin) {
+			error("cannot read mbox %s", file);
+			goto out;
 		}
+		file_done = 1;
+	}
 
-		if (f != stdin)
-			fclose(f);
+	while (!file_done) {
+		sprintf(name, "%s/%0*d", dir, nr_prec, ++skip);
+		file_done = split_one(f, name, allow_bare);
 	}
+
+	if (f != stdin)
+		fclose(f);
+
 	ret = skip;
 out:
-	free(name);
 	return ret;
 }
+
 int cmd_mailsplit(int argc, const char **argv, const char *prefix)
 {
-	int nr = 0, nr_prec = 4, ret;
+	int nr = 0, nr_prec = 4, ret = 0;
 	int allow_bare = 0;
 	const char *dir = NULL;
 	const char **argp;
@@ -186,7 +249,26 @@ int cmd_mailsplit(int argc, const char **argv, const char *prefix)
 			argp = stdin_only;
 	}
 
-	ret = split_mbox(argp, dir, allow_bare, nr_prec, nr);
+	while (*argp) {
+		const char *arg = *argp++;
+		struct stat argstat;
+
+		if (arg[0] == '-' && arg[1] == 0) {
+			ret |= split_mbox(arg, dir, allow_bare, nr_prec, nr);
+			continue;
+		}
+
+		if (stat(arg, &argstat) == -1) {
+			error("cannot stat %s (%s)", arg, strerror(errno));
+			return 1;
+		}
+
+		if (S_ISDIR(argstat.st_mode))
+			ret |= split_maildir(arg, dir, nr_prec, nr);
+		else
+			ret |= split_mbox(arg, dir, allow_bare, nr_prec, nr);
+	}
+
 	if (ret != -1)
 		printf("%d\n", ret);
 
diff --git a/builtin.h b/builtin.h
index d3f3a74..39290d1 100644
--- a/builtin.h
+++ b/builtin.h
@@ -8,7 +8,7 @@ extern const char git_usage_string[];
 
 extern void help_unknown_cmd(const char *cmd);
 extern int mailinfo(FILE *in, FILE *out, int ks, const char *encoding, const char *msg, const char *patch);
-extern int split_mbox(const char **mbox, const char *dir, int allow_bare, int nr_prec, int skip);
+extern int split_mbox(const char *file, const char *dir, int allow_bare, int nr_prec, int skip);
 extern void stripspace(FILE *in, FILE *out);
 extern int write_tree(unsigned char *sha1, int missing_ok, const char *prefix);
 extern void prune_packed_objects(int);
-- 
1.5.2


-- 
Fernando J. Pereda Garcimartín
20BB BDC3 761A 4781 E6ED  ED0B 0A48 5B0C 60BD 28D4

Re: [PATCH] Teach mailsplit about Maildir's

[Junio C Hamano]

"Fernando J. Pereda" <ferdy@gentoo.org> writes:

> Signed-off-by: Fernando J. Pereda <ferdy@gentoo.org>
> ---
>
> 	I never got ACK or NACK when I sent it the first time, that's why
> 	I'm resending this one with (almost) no changes.

Sorry, I had a (n obviously false) impression that you retracted
this patch due to this:

> ...
> +IMPORTANT: Maildir splitting relies upon filenames being sorted to output
> +patches in the correct order.

I am sure there are many users who uses Maildir layout on this
list.  Happy with this patch?  Please speak out.

Re: [PATCH] Teach mailsplit about Maildir's

[Alex Riesen]

Junio C Hamano, Sun, May 20, 2007 20:36:25 +0200:
> > +IMPORTANT: Maildir splitting relies upon filenames being sorted to output
> > +patches in the correct order.
> 
> I am sure there are many users who uses Maildir layout on this
> list.  Happy with this patch?  Please speak out.

I do use maildirs, but I have a suggestion: --maildir or something, to
create a non-existing maildir and split into it, so that the user does
not have to pre-create it.

Re: [PATCH] Teach mailsplit about Maildir's

[Fernando J. Pereda]

On Sun, May 20, 2007 at 08:49:59PM +0200, Alex Riesen wrote:
> Junio C Hamano, Sun, May 20, 2007 20:36:25 +0200:
> > > +IMPORTANT: Maildir splitting relies upon filenames being sorted to output
> > > +patches in the correct order.
> > 
> > I am sure there are many users who uses Maildir layout on this
> > list.  Happy with this patch?  Please speak out.
> 
> I do use maildirs, but I have a suggestion: --maildir or something, to
> create a non-existing maildir and split into it, so that the user does
> not have to pre-create it.

I can't think of a use case for this...

- ferdy

-- 
Fernando J. Pereda Garcimartín
20BB BDC3 761A 4781 E6ED  ED0B 0A48 5B0C 60BD 28D4

Re: [PATCH] Teach mailsplit about Maildir's

[Alex Riesen]

Fernando J. Pereda, Sun, May 20, 2007 21:00:24 +0200:
> On Sun, May 20, 2007 at 08:49:59PM +0200, Alex Riesen wrote:
> > Junio C Hamano, Sun, May 20, 2007 20:36:25 +0200:
> > > > +IMPORTANT: Maildir splitting relies upon filenames being sorted to output
> > > > +patches in the correct order.
> > > 
> > > I am sure there are many users who uses Maildir layout on this
> > > list.  Happy with this patch?  Please speak out.
> > 
> > I do use maildirs, but I have a suggestion: --maildir or something, to
> > create a non-existing maildir and split into it, so that the user does
> > not have to pre-create it.
> 
> I can't think of a use case for this...
> 

You actually enjoy typing "mkdir tmp && git mailsplit tmp"?

Re: [PATCH] Teach mailsplit about Maildir's

[Fernando J. Pereda]

On Sun, May 20, 2007 at 10:35:34PM +0200, Alex Riesen wrote:
> Fernando J. Pereda, Sun, May 20, 2007 21:00:24 +0200:
> > On Sun, May 20, 2007 at 08:49:59PM +0200, Alex Riesen wrote:
> > > Junio C Hamano, Sun, May 20, 2007 20:36:25 +0200:
> > > > > +IMPORTANT: Maildir splitting relies upon filenames being sorted to output
> > > > > +patches in the correct order.
> > > > 
> > > > I am sure there are many users who uses Maildir layout on this
> > > > list.  Happy with this patch?  Please speak out.
> > > 
> > > I do use maildirs, but I have a suggestion: --maildir or something, to
> > > create a non-existing maildir and split into it, so that the user does
> > > not have to pre-create it.
> > 
> > I can't think of a use case for this...
> > 
> 
> You actually enjoy typing "mkdir tmp && git mailsplit tmp"?

Well, my mail client creates Maildirs for me... so I still don't see why
would you do that. I copy mails with patches to a new maildir and run
git am there...

As I said I can't think of a use case for your suggestion.


- ferdy

-- 
Fernando J. Pereda Garcimartín
20BB BDC3 761A 4781 E6ED  ED0B 0A48 5B0C 60BD 28D4

Re: [PATCH] Teach mailsplit about Maildir's

[Johan Herland]

On Sunday 20 May 2007, Junio C Hamano wrote:
> "Fernando J. Pereda" <ferdy@gentoo.org> writes:
> 
> > Signed-off-by: Fernando J. Pereda <ferdy@gentoo.org>
> > ---
> >
> > 	I never got ACK or NACK when I sent it the first time, that's why
> > 	I'm resending this one with (almost) no changes.
> 
> Sorry, I had a (n obviously false) impression that you retracted
> this patch due to this:
> 
> > ...
> > +IMPORTANT: Maildir splitting relies upon filenames being sorted to output
> > +patches in the correct order.
> 
> I am sure there are many users who uses Maildir layout on this
> list.  Happy with this patch?  Please speak out.

I use KMail with Maildir for all my mail, except for my "patches" mail folder 
where I copy patches to import into my repos (which is in mbox format). 
KMail makes it easy to mix mbox and Maildir folders, so it's not really a 
big deal for me, but I would of course prefer Maildir support if it's 
possible to get it right.

Not sure how I feel about the usefulness of the patch if it requires correct 
sorting, and most mail clients turn out to _not_ sort correctly. Of course 
I have no idea what most mail clients do, but KMail seems to get it about 
right, AFAICS (mail filename starts with timestamp, so they're at least 
sorted roughly on (arrival) date).


Have fun!

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

Re: [PATCH] Teach mailsplit about Maildir's

[Fernando J. Pereda]

On Sun, May 20, 2007 at 08:53:32PM +0200, Johan Herland wrote:
> I use KMail with Maildir for all my mail, except for my "patches" mail folder 
> where I copy patches to import into my repos (which is in mbox format). 
> KMail makes it easy to mix mbox and Maildir folders, so it's not really a 
> big deal for me, but I would of course prefer Maildir support if it's 
> possible to get it right.
> 
> Not sure how I feel about the usefulness of the patch if it requires correct 
> sorting, and most mail clients turn out to _not_ sort correctly. Of course 
> I have no idea what most mail clients do, but KMail seems to get it about 
> right, AFAICS (mail filename starts with timestamp, so they're at least 
> sorted roughly on (arrival) date).

[ I sent this only to Johan, re-sending to the rest of the people now.
Sorry Johan for the dupe ]

I discussed this with a pine user in #git and we concluded (looking at
the respective code) that both Mutt and Pine generate correct filenames
based on when that mail arrived to the Maildir.

I use Mutt, and to test it I picked a thread that didn't arrive in
order, tagged and copied it to a different Maildir, patches were
splitted in correct order.

- ferdy

-- 
Fernando J. Pereda Garcimartín
20BB BDC3 761A 4781 E6ED  ED0B 0A48 5B0C 60BD 28D4

Re: [PATCH] Teach mailsplit about Maildir's

[Johan Herland]

On Sunday 20 May 2007, Fernando J. Pereda wrote:
> On Sun, May 20, 2007 at 08:53:32PM +0200, Johan Herland wrote:
> > I use KMail with Maildir for all my mail, except for my "patches" mail folder 
> > where I copy patches to import into my repos (which is in mbox format). 
> > KMail makes it easy to mix mbox and Maildir folders, so it's not really a 
> > big deal for me, but I would of course prefer Maildir support if it's 
> > possible to get it right.
> > 
> > Not sure how I feel about the usefulness of the patch if it requires correct 
> > sorting, and most mail clients turn out to _not_ sort correctly. Of course 
> > I have no idea what most mail clients do, but KMail seems to get it about 
> > right, AFAICS (mail filename starts with timestamp, so they're at least 
> > sorted roughly on (arrival) date).
> 
> [ I sent this only to Johan, re-sending to the rest of the people now.
> Sorry Johan for the dupe ]
> 
> I discussed this with a pine user in #git and we concluded (looking at
> the respective code) that both Mutt and Pine generate correct filenames
> based on when that mail arrived to the Maildir.
> 
> I use Mutt, and to test it I picked a thread that didn't arrive in
> order, tagged and copied it to a different Maildir, patches were
> splitted in correct order.

Well, if Mutt, Pine, and KMail all get it right, then it looks like a 
useful feature to add.

Does anybody now of (current versions of) mail clients that do _not_ 
get this right?


-- 
Johan Herland, <johan@herland.net>
www.herland.net

Re: [PATCH] Teach mailsplit about Maildir's

[Johannes Schindelin]

Hi,

On Sun, 20 May 2007, Fernando J. Pereda wrote:

> On Sun, May 20, 2007 at 08:53:32PM +0200, Johan Herland wrote:
> > 
> > Not sure how I feel about the usefulness of the patch if it requires 
> > correct sorting, and most mail clients turn out to _not_ sort 
> > correctly. Of course I have no idea what most mail clients do, but 
> > KMail seems to get it about right, AFAICS (mail filename starts with 
> > timestamp, so they're at least sorted roughly on (arrival) date).
> 
> I discussed this with a pine user in #git and we concluded (looking at 
> the respective code) that both Mutt and Pine generate correct filenames 
> based on when that mail arrived to the Maildir.

FWIW this pine user was me. Yes, I finally bit the bullet and run pine 
with the maildir patches.

I cannot think of another sane way for any MUA to sort Maildirs.

Ciao,
Dscho

Re: [PATCH] Teach mailsplit about Maildir's

[Junio C Hamano]

"Fernando J. Pereda" <ferdy@gentoo.org> writes:

> -	ret = split_mbox(argp, dir, allow_bare, nr_prec, nr);
> +	while (*argp) {
> +		const char *arg = *argp++;
> +		struct stat argstat;
> +
> +		if (arg[0] == '-' && arg[1] == 0) {
> +			ret |= split_mbox(arg, dir, allow_bare, nr_prec, nr);
> +			continue;
> +		}
> +
> +		if (stat(arg, &argstat) == -1) {
> +			error("cannot stat %s (%s)", arg, strerror(errno));
> +			return 1;
> +		}
> +
> +		if (S_ISDIR(argstat.st_mode))
> +			ret |= split_maildir(arg, dir, nr_prec, nr);
> +		else
> +			ret |= split_mbox(arg, dir, allow_bare, nr_prec, nr);
> +	}
> +
>  	if (ret != -1)
>  		printf("%d\n", ret);
>  

No kidding.  ret |= stuff and then printf("%d\n", ret) would not
give us the number of commit e-mails on the standard output.

Re: [PATCH] Teach mailsplit about Maildir's

[Johannes Schindelin]

Hi,

On Sun, 20 May 2007, Junio C Hamano wrote:

> "Fernando J. Pereda" <ferdy@gentoo.org> writes:
> 
> > -	ret = split_mbox(argp, dir, allow_bare, nr_prec, nr);
> > +	while (*argp) {
> > +		const char *arg = *argp++;
> > +		struct stat argstat;
> > +
> > +		if (arg[0] == '-' && arg[1] == 0) {
> > +			ret |= split_mbox(arg, dir, allow_bare, nr_prec, nr);
> > +			continue;
> > +		}
> > +
> > +		if (stat(arg, &argstat) == -1) {
> > +			error("cannot stat %s (%s)", arg, strerror(errno));
> > +			return 1;
> > +		}
> > +
> > +		if (S_ISDIR(argstat.st_mode))
> > +			ret |= split_maildir(arg, dir, nr_prec, nr);
> > +		else
> > +			ret |= split_mbox(arg, dir, allow_bare, nr_prec, nr);
> > +	}
> > +
> >  	if (ret != -1)
> >  		printf("%d\n", ret);
> >  
> 
> No kidding.  ret |= stuff and then printf("%d\n", ret) would not
> give us the number of commit e-mails on the standard output.

Should we not just stop when split_maildir() or split_mbox() returns -1? 
And yes, we'd probably need a second variable to do take the return value.

Ciao,
Dscho

Re: [PATCH] Teach mailsplit about Maildir's

[Fernando J. Pereda]

On Mon, May 21, 2007 at 01:56:31PM +0100, Johannes Schindelin wrote:
>
> Should we not just stop when split_maildir() or split_mbox() returns -1? 
> And yes, we'd probably need a second variable to do take the return value.
>

Sure, I'll send a new patch with that.

- ferdy

-- 
Fernando J. Pereda Garcimartín
20BB BDC3 761A 4781 E6ED  ED0B 0A48 5B0C 60BD 28D4

Re: [PATCH] Teach mailsplit about Maildir's

[Fernando J. Pereda]

Signed-off-by: Fernando J. Pereda <ferdy@gentoo.org>
---
 Documentation/git-am.txt        |    8 ++-
 Documentation/git-mailsplit.txt |   13 +++-
 builtin-mailsplit.c             |  139 ++++++++++++++++++++++++++++++++-------
 builtin.h                       |    2 +-
 4 files changed, 132 insertions(+), 30 deletions(-)

diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index ba79773..25cf84a 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -12,7 +12,8 @@ SYNOPSIS
 'git-am' [--signoff] [--dotest=<dir>] [--keep] [--utf8 | --no-utf8]
          [--3way] [--interactive] [--binary]
          [--whitespace=<option>] [-C<n>] [-p<n>]
-         <mbox>...
+         <mbox>|<Maildir>...
+
 'git-am' [--skip | --resolved]
 
 DESCRIPTION
@@ -23,9 +24,10 @@ current branch.
 
 OPTIONS
 -------
-<mbox>...::
+<mbox>|<Maildir>...::
 	The list of mailbox files to read patches from. If you do not
-	supply this argument, reads from the standard input.
+	supply this argument, reads from the standard input. If you supply
+	directories, they'll be treated as Maildirs.
 
 -s, --signoff::
 	Add `Signed-off-by:` line to the commit message, using
diff --git a/Documentation/git-mailsplit.txt b/Documentation/git-mailsplit.txt
index c11d6a5..abb0903 100644
--- a/Documentation/git-mailsplit.txt
+++ b/Documentation/git-mailsplit.txt
@@ -7,12 +7,15 @@ git-mailsplit - Simple UNIX mbox splitter program
 
 SYNOPSIS
 --------
-'git-mailsplit' [-b] [-f<nn>] [-d<prec>] -o<directory> [--] [<mbox>...]
+'git-mailsplit' [-b] [-f<nn>] [-d<prec>] -o<directory> [--] [<mbox>|<Maildir>...]
 
 DESCRIPTION
 -----------
-Splits a mbox file into a list of files: "0001" "0002" ..  in the specified
-directory so you can process them further from there.
+Splits a mbox file or a Maildir into a list of files: "0001" "0002" ..  in the
+specified directory so you can process them further from there.
+
+IMPORTANT: Maildir splitting relies upon filenames being sorted to output
+patches in the correct order.
 
 OPTIONS
 -------
@@ -20,6 +23,10 @@ OPTIONS
 	Mbox file to split.  If not given, the mbox is read from
 	the standard input.
 
+<Maildir>::
+	Root of the Maildir to split. This directory should contain the cur, tmp
+	and new subdirectories.
+
 <directory>::
 	Directory in which to place the individual messages.
 
diff --git a/builtin-mailsplit.c b/builtin-mailsplit.c
index 3bca855..454f943 100644
--- a/builtin-mailsplit.c
+++ b/builtin-mailsplit.c
@@ -6,9 +6,10 @@
  */
 #include "cache.h"
 #include "builtin.h"
+#include "path-list.h"
 
 static const char git_mailsplit_usage[] =
-"git-mailsplit [-d<prec>] [-f<n>] [-b] -o<directory> <mbox>...";
+"git-mailsplit [-d<prec>] [-f<n>] [-b] -o<directory> <mbox>|<Maildir>...";
 
 static int is_from_line(const char *line, int len)
 {
@@ -96,44 +97,106 @@ static int split_one(FILE *mbox, const char *name, int allow_bare)
 	exit(1);
 }
 
-int split_mbox(const char **mbox, const char *dir, int allow_bare, int nr_prec, int skip)
+static int populate_maildir_list(struct path_list *list, const char *path)
 {
-	char *name = xmalloc(strlen(dir) + 2 + 3 * sizeof(skip));
+	DIR *dir;
+	struct dirent *dent;
+
+	if ((dir = opendir(path)) == NULL) {
+		error("cannot diropen %s (%s)", path, strerror(errno));
+		return -1;
+	}
+
+	while ((dent = readdir(dir)) != NULL) {
+		if (dent->d_name[0] == '.')
+			continue;
+		path_list_insert(dent->d_name, list);
+	}
+
+	closedir(dir);
+
+	return 1;
+}
+
+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;
+	struct path_list list = {NULL, 0, 0, 1};
 
-	while (*mbox) {
-		const char *file = *mbox++;
-		FILE *f = !strcmp(file, "-") ? stdin : fopen(file, "r");
-		int file_done = 0;
+	snprintf(curdir, sizeof(curdir), "%s/cur", maildir);
+	if (populate_maildir_list(&list, curdir) < 0)
+		goto out;
 
-		if ( !f ) {
-			error("cannot open mbox %s", file);
+	int i;
+	for (i = 0; i < list.nr; i++) {
+		snprintf(file, sizeof(file), "%s/%s", curdir, list.items[i].path);
+		FILE *f = fopen(file, "r");
+		if (!f) {
+			error("cannot open mail %s (%s)", file, strerror(errno));
 			goto out;
 		}
 
 		if (fgets(buf, sizeof(buf), f) == NULL) {
-			if (f == stdin)
-				break; /* empty stdin is OK */
-			error("cannot read mbox %s", file);
+			error("cannot read mail %s (%s)", file, strerror(errno));
 			goto out;
 		}
 
-		while (!file_done) {
-			sprintf(name, "%s/%0*d", dir, nr_prec, ++skip);
-			file_done = split_one(f, name, allow_bare);
+		sprintf(name, "%s/%0*d", dir, nr_prec, ++skip);
+		split_one(f, name, 1);
+
+		fclose(f);
+	}
+
+	path_list_clear(&list, 1);
+
+	ret = skip;
+out:
+	return ret;
+}
+
+int split_mbox(const char *file, const char *dir, int allow_bare,
+		int nr_prec, int skip)
+{
+	char name[PATH_MAX];
+	int ret = -1;
+
+	FILE *f = !strcmp(file, "-") ? stdin : fopen(file, "r");
+	int file_done = 0;
+
+	if (!f) {
+		error("cannot open mbox %s", file);
+		goto out;
+	}
+
+	if (fgets(buf, sizeof(buf), f) == NULL) {
+		/* empty stdin is OK */
+		if (f != stdin) {
+			error("cannot read mbox %s", file);
+			goto out;
 		}
+		file_done = 1;
+	}
 
-		if (f != stdin)
-			fclose(f);
+	while (!file_done) {
+		sprintf(name, "%s/%0*d", dir, nr_prec, ++skip);
+		file_done = split_one(f, name, allow_bare);
 	}
+
+	if (f != stdin)
+		fclose(f);
+
 	ret = skip;
 out:
-	free(name);
 	return ret;
 }
+
 int cmd_mailsplit(int argc, const char **argv, const char *prefix)
 {
-	int nr = 0, nr_prec = 4, ret;
+	int nr = 0, nr_prec = 4, num = 0;
 	int allow_bare = 0;
 	const char *dir = NULL;
 	const char **argp;
@@ -186,9 +249,39 @@ int cmd_mailsplit(int argc, const char **argv, const char *prefix)
 			argp = stdin_only;
 	}
 
-	ret = split_mbox(argp, dir, allow_bare, nr_prec, nr);
-	if (ret != -1)
-		printf("%d\n", ret);
+	while (*argp) {
+		const char *arg = *argp++;
+		struct stat argstat;
+		int ret = 0;
+
+		if (arg[0] == '-' && arg[1] == 0) {
+			ret = split_mbox(arg, dir, allow_bare, nr_prec, nr);
+			if (ret < 0) {
+				error("cannot split patches from stdin");
+				return 1;
+			}
+			num += ret;
+			continue;
+		}
+
+		if (stat(arg, &argstat) == -1) {
+			error("cannot stat %s (%s)", arg, strerror(errno));
+			return 1;
+		}
+
+		if (S_ISDIR(argstat.st_mode))
+			ret = split_maildir(arg, dir, nr_prec, nr);
+		else
+			ret = split_mbox(arg, dir, allow_bare, nr_prec, nr);
+
+		if (ret < 0) {
+			error("cannot split patches from %s", arg);
+			return 1;
+		}
+		num += ret;
+	}
+
+	printf("%d\n", num);
 
-	return ret == -1;
+	return 0;
 }
diff --git a/builtin.h b/builtin.h
index d3f3a74..39290d1 100644
--- a/builtin.h
+++ b/builtin.h
@@ -8,7 +8,7 @@ extern const char git_usage_string[];
 
 extern void help_unknown_cmd(const char *cmd);
 extern int mailinfo(FILE *in, FILE *out, int ks, const char *encoding, const char *msg, const char *patch);
-extern int split_mbox(const char **mbox, const char *dir, int allow_bare, int nr_prec, int skip);
+extern int split_mbox(const char *file, const char *dir, int allow_bare, int nr_prec, int skip);
 extern void stripspace(FILE *in, FILE *out);
 extern int write_tree(unsigned char *sha1, int missing_ok, const char *prefix);
 extern void prune_packed_objects(int);
-- 
1.5.2

[PATCH] Allow user to specify mailbox format for mailsplit

[Alex Riesen]

If the argument ends with a slash - assume it is a Maildir and try to
create it. Otherwise - it is an mbox.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
 Documentation/git-mailsplit.txt |    4 +++-
 builtin-mailsplit.c             |   10 ++++++++++
 2 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-mailsplit.txt b/Documentation/git-mailsplit.txt
index abb0903..2c58e09 100644
--- a/Documentation/git-mailsplit.txt
+++ b/Documentation/git-mailsplit.txt
@@ -25,7 +25,9 @@ OPTIONS
 
 <Maildir>::
 	Root of the Maildir to split. This directory should contain the cur, tmp
-	and new subdirectories.
+	and new subdirectories. If the argument ends with a slash '/'
+	the directory and new, cur and tmp subdirectories will be
+	created automatically.
 
 <directory>::
 	Directory in which to place the individual messages.
diff --git a/builtin-mailsplit.c b/builtin-mailsplit.c
index 454f943..370f7fa 100644
--- a/builtin-mailsplit.c
+++ b/builtin-mailsplit.c
@@ -253,6 +253,7 @@ int cmd_mailsplit(int argc, const char **argv, const char *prefix)
 		const char *arg = *argp++;
 		struct stat argstat;
 		int ret = 0;
+		size_t arglen = strlen(arg);
 
 		if (arg[0] == '-' && arg[1] == 0) {
 			ret = split_mbox(arg, dir, allow_bare, nr_prec, nr);
@@ -264,6 +265,15 @@ int cmd_mailsplit(int argc, const char **argv, const char *prefix)
 			continue;
 		}
 
+		if (arglen && arg[arglen-1] == '/') {
+			char *dir = malloc(arglen + 5);
+			memcpy(dir, arg, arglen);
+			mkdir(arg, 0777);
+			mkdir(strcpy(dir + arglen, "new"), 0777);
+			mkdir(strcpy(dir + arglen, "cur"), 0777);
+			mkdir(strcpy(dir + arglen, "tmp"), 0777);
+			free(dir);
+		}
 		if (stat(arg, &argstat) == -1) {
 			error("cannot stat %s (%s)", arg, strerror(errno));
 			return 1;
-- 
1.5.2.rc3.112.gc1e43

Re: [PATCH] Allow user to specify mailbox format for mailsplit

[Junio C Hamano]

Alex Riesen <raa.lkml@gmail.com> writes:

> If the argument ends with a slash - assume it is a Maildir and try to
> create it. Otherwise - it is an mbox.

Sorry, I am lost here.  git-mailsplit is to read an existing
mbox and split the pieces of e-mail into the specified output
directory.  The recent Maildir support is to make it read pieces
of e-mails from an existing Maildir (which presumably have bunch
of files that store e-mail), isn't it?

If the patch were about creating a new _output_ directory (that
is, "dir" in cmd_mailsplit), I would understand what you are
trying to do, but why would you even want to create the input
Maildir in that loop (I take that your patch is on top of
Fernando's patch)?

Re: [PATCH] Allow user to specify mailbox format for mailsplit

[Alex Riesen]

Junio C Hamano, Tue, May 22, 2007 01:19:25 +0200:
> Alex Riesen <raa.lkml@gmail.com> writes:
> 
> > If the argument ends with a slash - assume it is a Maildir and try to
> > create it. Otherwise - it is an mbox.
> 
> Sorry, I am lost here.  git-mailsplit is to read an existing
> mbox and split the pieces of e-mail into the specified output
> directory.  The recent Maildir support is to make it read pieces
> of e-mails from an existing Maildir (which presumably have bunch
> of files that store e-mail), isn't it?

Oh... Fernando, I'm very sorry. I never used mailsplit and didn't even
gave a second thought about its name. For reasons unknown, I assumed
the mbox/maildir argument is for the output.
Must be all the ozone depletion and the cosmic rays now causing
glitches in programmers brains...

> If the patch were about creating a new _output_ directory (that
> is, "dir" in cmd_mailsplit), I would understand what you are
> trying to do, but why would you even want to create the input
> Maildir in that loop (I take that your patch is on top of
> Fernando's patch)?

Yes, but don't apply it! :)

Re: [PATCH] Teach mailsplit about Maildir's

[Junio C Hamano]

"Fernando J. Pereda" <ferdy@gentoo.org> writes:

> Signed-off-by: Fernando J. Pereda <ferdy@gentoo.org>
> ---
>  Documentation/git-am.txt        |    8 ++-
>  Documentation/git-mailsplit.txt |   13 +++-
>  builtin-mailsplit.c             |  139 ++++++++++++++++++++++++++++++++-------
>  builtin.h                       |    2 +-
>  4 files changed, 132 insertions(+), 30 deletions(-)
>
> diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
> index ba79773..25cf84a 100644
> --- a/Documentation/git-am.txt
> +++ b/Documentation/git-am.txt
> @@ -12,7 +12,8 @@ SYNOPSIS
>  'git-am' [--signoff] [--dotest=<dir>] [--keep] [--utf8 | --no-utf8]
>           [--3way] [--interactive] [--binary]
>           [--whitespace=<option>] [-C<n>] [-p<n>]
> -         <mbox>...
> +         <mbox>|<Maildir>...
> +
>  'git-am' [--skip | --resolved]

Does the document still format Ok if you add a blank line there
(not a rhetorical question -- I haven't checked)?

> diff --git a/builtin-mailsplit.c b/builtin-mailsplit.c
> index 3bca855..454f943 100644
> --- a/builtin-mailsplit.c
> +++ b/builtin-mailsplit.c
> @@ -6,9 +6,10 @@
>   */
>  #include "cache.h"
>  #include "builtin.h"
> +#include "path-list.h"
>  
>  static const char git_mailsplit_usage[] =
> -"git-mailsplit [-d<prec>] [-f<n>] [-b] -o<directory> <mbox>...";
> +"git-mailsplit [-d<prec>] [-f<n>] [-b] -o<directory> <mbox>|<Maildir>...";
>  
>  static int is_from_line(const char *line, int len)
>  {
> @@ -96,44 +97,106 @@ static int split_one(FILE *mbox, const char *name, int allow_bare)
>  	exit(1);
>  }
>  
> -int split_mbox(const char **mbox, const char *dir, int allow_bare, int nr_prec, int skip)
> +static int populate_maildir_list(struct path_list *list, const char *path)
>  {
> -	char *name = xmalloc(strlen(dir) + 2 + 3 * sizeof(skip));
> +	DIR *dir;
> +	struct dirent *dent;
> +
> +	if ((dir = opendir(path)) == NULL) {
> +		error("cannot diropen %s (%s)", path, strerror(errno));
> +		return -1;
> +	}

Didn't you just fail opendir, not diropen?

> +
> +	while ((dent = readdir(dir)) != NULL) {
> +		if (dent->d_name[0] == '.')
> +			continue;
> +		path_list_insert(dent->d_name, list);
> +	}
> +
> +	closedir(dir);
> +
> +	return 1;
> +}

Usually we signal success by returning 0.

> +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;
> +	struct path_list list = {NULL, 0, 0, 1};
>  
> +	snprintf(curdir, sizeof(curdir), "%s/cur", maildir);
> +	if (populate_maildir_list(&list, curdir) < 0)
> +		goto out;
>  
> +	int i;

Decl-after-statement.

> +	for (i = 0; i < list.nr; i++) {
> +		snprintf(file, sizeof(file), "%s/%s", curdir, list.items[i].path);
> +		FILE *f = fopen(file, "r");

Likewise.

> @@ -186,9 +249,39 @@ int cmd_mailsplit(int argc, const char **argv, const char *prefix)
> ...
> +		if (ret < 0) {
> +			error("cannot split patches from %s", arg);
> +			return 1;
> +		}
> +		num += ret;
> +	}
> +
> +	printf("%d\n", num);
>  
> -	return ret == -1;
> +	return 0;
>  }

We do not signal error anymore from the command?

Re: [PATCH] Teach mailsplit about Maildir's

[Fernando J. Pereda]

On Thu, May 24, 2007 at 12:47:00PM -0700, Junio C Hamano wrote:
> "Fernando J. Pereda" <ferdy@gentoo.org> writes:
> >           [--whitespace=<option>] [-C<n>] [-p<n>]
> > -         <mbox>...
> > +         <mbox>|<Maildir>...
> > +
> >  'git-am' [--skip | --resolved]
> 
> Does the document still format Ok if you add a blank line there
> (not a rhetorical question -- I haven't checked)?

I think I tried when I first sent it, will try.

> > +
> > +	if ((dir = opendir(path)) == NULL) {
> > +		error("cannot diropen %s (%s)", path, strerror(errno));
> > +		return -1;
> > +	}
> 
> Didn't you just fail opendir, not diropen?

Ouch, will fix

> > +	return 1;
> > +}
> 
> Usually we signal success by returning 0.

Ok. Fair enough.

> 
> > +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;
> > +	struct path_list list = {NULL, 0, 0, 1};
> >  
> > +	snprintf(curdir, sizeof(curdir), "%s/cur", maildir);
> > +	if (populate_maildir_list(&list, curdir) < 0)
> > +		goto out;
> >  
> > +	int i;
> 
> Decl-after-statement.

I did it because other parts of the file do it too. Do you want me to
fix the rest of them?

> 
> > +	for (i = 0; i < list.nr; i++) {
> > +		snprintf(file, sizeof(file), "%s/%s", curdir, list.items[i].path);
> > +		FILE *f = fopen(file, "r");
> 
> Likewise.

Again, because we were already doing it in other parts of the file. Will
fix.

> 
> > @@ -186,9 +249,39 @@ int cmd_mailsplit(int argc, const char **argv, const char *prefix)
> > ...
> > +		if (ret < 0) {
> > +			error("cannot split patches from %s", arg);
> > +			return 1;
> > +		}
> > +		num += ret;
> > +	}
> > +
> > +	printf("%d\n", num);
> >  
> > -	return ret == -1;
> > +	return 0;
> >  }
> 
> We do not signal error anymore from the command?

If we reached that part of the code, there is no error to signal about.
We signal error when either split_mbox or split_maildir fail by
returning 1.

Will fix those issues and send a new patch. Thanks for reviewing it.

- ferdy

-- 
Fernando J. Pereda Garcimartín
20BB BDC3 761A 4781 E6ED  ED0B 0A48 5B0C 60BD 28D4

Re: [PATCH] Teach mailsplit about Maildir's

[Junio C Hamano]

"Fernando J. Pereda" <ferdy@gentoo.org> writes:

>> > +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;
>> > +	struct path_list list = {NULL, 0, 0, 1};
>> >  
>> > +	snprintf(curdir, sizeof(curdir), "%s/cur", maildir);
>> > +	if (populate_maildir_list(&list, curdir) < 0)
>> > +		goto out;
>> >  
>> > +	int i;
>> 
>> Decl-after-statement.
>
> I did it because other parts of the file do it too. Do you want me to
> fix the rest of them?

My "gcc -Werror -Wall -Wdeclaration-after-statement" does not
seem to find any in the existing code, but if you found any
please fix that in a separate patch.

>> > @@ -186,9 +249,39 @@ int cmd_mailsplit(int argc, const char **argv, const char *prefix)
>> > ...
>> > +		if (ret < 0) {
>> > +			error("cannot split patches from %s", arg);
>> > +			return 1;
>> > +		}
>> > +		num += ret;
>> > +	}
>> > +
>> > +	printf("%d\n", num);
>> >  
>> > -	return ret == -1;
>> > +	return 0;
>> >  }
>> 
>> We do not signal error anymore from the command?
>
> If we reached that part of the code, there is no error to signal about.
> We signal error when either split_mbox or split_maildir fail by
> returning 1.

Ok, you are right.

Thanks.

[PATCH] Teach mailsplit about Maildir's

[Fernando J. Pereda]

Signed-off-by: Fernando J. Pereda <ferdy@gentoo.org>
---

	I built the documentation again, and formatting looks ok with that
	blank line there. I think I fixed those declarations after
	statements too.

 Documentation/git-am.txt        |    8 ++-
 Documentation/git-mailsplit.txt |   13 +++-
 builtin-mailsplit.c             |  140 ++++++++++++++++++++++++++++++++-------
 builtin.h                       |    2 +-
 4 files changed, 133 insertions(+), 30 deletions(-)

diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index ba79773..25cf84a 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -12,7 +12,8 @@ SYNOPSIS
 'git-am' [--signoff] [--dotest=<dir>] [--keep] [--utf8 | --no-utf8]
          [--3way] [--interactive] [--binary]
          [--whitespace=<option>] [-C<n>] [-p<n>]
-         <mbox>...
+         <mbox>|<Maildir>...
+
 'git-am' [--skip | --resolved]
 
 DESCRIPTION
@@ -23,9 +24,10 @@ current branch.
 
 OPTIONS
 -------
-<mbox>...::
+<mbox>|<Maildir>...::
 	The list of mailbox files to read patches from. If you do not
-	supply this argument, reads from the standard input.
+	supply this argument, reads from the standard input. If you supply
+	directories, they'll be treated as Maildirs.
 
 -s, --signoff::
 	Add `Signed-off-by:` line to the commit message, using
diff --git a/Documentation/git-mailsplit.txt b/Documentation/git-mailsplit.txt
index c11d6a5..abb0903 100644
--- a/Documentation/git-mailsplit.txt
+++ b/Documentation/git-mailsplit.txt
@@ -7,12 +7,15 @@ git-mailsplit - Simple UNIX mbox splitter program
 
 SYNOPSIS
 --------
-'git-mailsplit' [-b] [-f<nn>] [-d<prec>] -o<directory> [--] [<mbox>...]
+'git-mailsplit' [-b] [-f<nn>] [-d<prec>] -o<directory> [--] [<mbox>|<Maildir>...]
 
 DESCRIPTION
 -----------
-Splits a mbox file into a list of files: "0001" "0002" ..  in the specified
-directory so you can process them further from there.
+Splits a mbox file or a Maildir into a list of files: "0001" "0002" ..  in the
+specified directory so you can process them further from there.
+
+IMPORTANT: Maildir splitting relies upon filenames being sorted to output
+patches in the correct order.
 
 OPTIONS
 -------
@@ -20,6 +23,10 @@ OPTIONS
 	Mbox file to split.  If not given, the mbox is read from
 	the standard input.
 
+<Maildir>::
+	Root of the Maildir to split. This directory should contain the cur, tmp
+	and new subdirectories.
+
 <directory>::
 	Directory in which to place the individual messages.
 
diff --git a/builtin-mailsplit.c b/builtin-mailsplit.c
index 3bca855..97ae004 100644
--- a/builtin-mailsplit.c
+++ b/builtin-mailsplit.c
@@ -6,9 +6,10 @@
  */
 #include "cache.h"
 #include "builtin.h"
+#include "path-list.h"
 
 static const char git_mailsplit_usage[] =
-"git-mailsplit [-d<prec>] [-f<n>] [-b] -o<directory> <mbox>...";
+"git-mailsplit [-d<prec>] [-f<n>] [-b] -o<directory> <mbox>|<Maildir>...";
 
 static int is_from_line(const char *line, int len)
 {
@@ -96,44 +97,107 @@ static int split_one(FILE *mbox, const char *name, int allow_bare)
 	exit(1);
 }
 
-int split_mbox(const char **mbox, const char *dir, int allow_bare, int nr_prec, int skip)
+static int populate_maildir_list(struct path_list *list, const char *path)
 {
-	char *name = xmalloc(strlen(dir) + 2 + 3 * sizeof(skip));
+	DIR *dir;
+	struct dirent *dent;
+
+	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;
+		path_list_insert(dent->d_name, list);
+	}
+
+	closedir(dir);
+
+	return 0;
+}
+
+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};
 
-	while (*mbox) {
-		const char *file = *mbox++;
-		FILE *f = !strcmp(file, "-") ? stdin : fopen(file, "r");
-		int file_done = 0;
+	snprintf(curdir, sizeof(curdir), "%s/cur", maildir);
+	if (populate_maildir_list(&list, curdir) < 0)
+		goto out;
 
-		if ( !f ) {
-			error("cannot open mbox %s", file);
+	for (i = 0; i < list.nr; i++) {
+		FILE *f;
+		snprintf(file, sizeof(file), "%s/%s", curdir, list.items[i].path);
+		f = fopen(file, "r");
+		if (!f) {
+			error("cannot open mail %s (%s)", file, strerror(errno));
 			goto out;
 		}
 
 		if (fgets(buf, sizeof(buf), f) == NULL) {
-			if (f == stdin)
-				break; /* empty stdin is OK */
-			error("cannot read mbox %s", file);
+			error("cannot read mail %s (%s)", file, strerror(errno));
 			goto out;
 		}
 
-		while (!file_done) {
-			sprintf(name, "%s/%0*d", dir, nr_prec, ++skip);
-			file_done = split_one(f, name, allow_bare);
+		sprintf(name, "%s/%0*d", dir, nr_prec, ++skip);
+		split_one(f, name, 1);
+
+		fclose(f);
+	}
+
+	path_list_clear(&list, 1);
+
+	ret = skip;
+out:
+	return ret;
+}
+
+int split_mbox(const char *file, const char *dir, int allow_bare,
+		int nr_prec, int skip)
+{
+	char name[PATH_MAX];
+	int ret = -1;
+
+	FILE *f = !strcmp(file, "-") ? stdin : fopen(file, "r");
+	int file_done = 0;
+
+	if (!f) {
+		error("cannot open mbox %s", file);
+		goto out;
+	}
+
+	if (fgets(buf, sizeof(buf), f) == NULL) {
+		/* empty stdin is OK */
+		if (f != stdin) {
+			error("cannot read mbox %s", file);
+			goto out;
 		}
+		file_done = 1;
+	}
 
-		if (f != stdin)
-			fclose(f);
+	while (!file_done) {
+		sprintf(name, "%s/%0*d", dir, nr_prec, ++skip);
+		file_done = split_one(f, name, allow_bare);
 	}
+
+	if (f != stdin)
+		fclose(f);
+
 	ret = skip;
 out:
-	free(name);
 	return ret;
 }
+
 int cmd_mailsplit(int argc, const char **argv, const char *prefix)
 {
-	int nr = 0, nr_prec = 4, ret;
+	int nr = 0, nr_prec = 4, num = 0;
 	int allow_bare = 0;
 	const char *dir = NULL;
 	const char **argp;
@@ -186,9 +250,39 @@ int cmd_mailsplit(int argc, const char **argv, const char *prefix)
 			argp = stdin_only;
 	}
 
-	ret = split_mbox(argp, dir, allow_bare, nr_prec, nr);
-	if (ret != -1)
-		printf("%d\n", ret);
+	while (*argp) {
+		const char *arg = *argp++;
+		struct stat argstat;
+		int ret = 0;
+
+		if (arg[0] == '-' && arg[1] == 0) {
+			ret = split_mbox(arg, dir, allow_bare, nr_prec, nr);
+			if (ret < 0) {
+				error("cannot split patches from stdin");
+				return 1;
+			}
+			num += ret;
+			continue;
+		}
+
+		if (stat(arg, &argstat) == -1) {
+			error("cannot stat %s (%s)", arg, strerror(errno));
+			return 1;
+		}
+
+		if (S_ISDIR(argstat.st_mode))
+			ret = split_maildir(arg, dir, nr_prec, nr);
+		else
+			ret = split_mbox(arg, dir, allow_bare, nr_prec, nr);
+
+		if (ret < 0) {
+			error("cannot split patches from %s", arg);
+			return 1;
+		}
+		num += ret;
+	}
+
+	printf("%d\n", num);
 
-	return ret == -1;
+	return 0;
 }
diff --git a/builtin.h b/builtin.h
index d3f3a74..39290d1 100644
--- a/builtin.h
+++ b/builtin.h
@@ -8,7 +8,7 @@ extern const char git_usage_string[];
 
 extern void help_unknown_cmd(const char *cmd);
 extern int mailinfo(FILE *in, FILE *out, int ks, const char *encoding, const char *msg, const char *patch);
-extern int split_mbox(const char **mbox, const char *dir, int allow_bare, int nr_prec, int skip);
+extern int split_mbox(const char *file, const char *dir, int allow_bare, int nr_prec, int skip);
 extern void stripspace(FILE *in, FILE *out);
 extern int write_tree(unsigned char *sha1, int missing_ok, const char *prefix);
 extern void prune_packed_objects(int);
-- 
1.5.2

[PATCH] Teach mailsplit about Maildir's

[Fernando J. Pereda]

Signed-off-by: Fernando J. Pereda <ferdy@gentoo.org>
---

 builtin-mailsplit.c |  107 ++++++++++++++++++++++++++++++++++++++++++---------
 builtin.h           |    2 +-
 2 files changed, 89 insertions(+), 20 deletions(-)

diff --git a/builtin-mailsplit.c b/builtin-mailsplit.c
index 3bca855..e0a283d 100644
--- a/builtin-mailsplit.c
+++ b/builtin-mailsplit.c
@@ -96,44 +96,93 @@ static int split_one(FILE *mbox, const char *name, int allow_bare)
 	exit(1);
 }
 
-int split_mbox(const char **mbox, const char *dir, int allow_bare, int nr_prec, int skip)
+int split_maildir(const char *maildir, const char *dir, int nr_prec, int skip)
 {
-	char *name = xmalloc(strlen(dir) + 2 + 3 * sizeof(skip));
+	char file[PATH_MAX];
+	char curdir[PATH_MAX];
+	char name[PATH_MAX];
+	DIR *mddir;
+	struct dirent *maildent;
 	int ret = -1;
 
-	while (*mbox) {
-		const char *file = *mbox++;
-		FILE *f = !strcmp(file, "-") ? stdin : fopen(file, "r");
-		int file_done = 0;
+	snprintf(curdir, sizeof(curdir), "%s/cur", maildir);
+	if ((mddir = opendir(curdir)) == NULL) {
+		error("cannot diropen %s (%s)", curdir, strerror(errno));
+		goto out;
+	}
+
+	while ((maildent = readdir(mddir)) != NULL) {
+		FILE *f;
+
+		snprintf(file, sizeof(file), "%s/%s",
+				curdir, maildent->d_name);
+
+		if (maildent->d_name[0] == '.')
+			continue;
 
-		if ( !f ) {
-			error("cannot open mbox %s", file);
+		f = fopen(file, "r");
+		if (!f) {
+			error("cannot open mail %s (%s)", file, strerror(errno));
 			goto out;
 		}
 
 		if (fgets(buf, sizeof(buf), f) == NULL) {
-			if (f == stdin)
-				break; /* empty stdin is OK */
-			error("cannot read mbox %s", file);
+			error("cannot read mail %s (%s)", file, strerror(errno));
 			goto out;
 		}
 
-		while (!file_done) {
-			sprintf(name, "%s/%0*d", dir, nr_prec, ++skip);
-			file_done = split_one(f, name, allow_bare);
+		sprintf(name, "%s/%0*d", dir, nr_prec, ++skip);
+		split_one(f, name, 1);
+
+		fclose(f);
+	}
+
+	closedir(mddir);
+
+	ret = skip;
+out:
+	return ret;
+}
+
+int split_mbox(const char *file, const char *dir, int allow_bare,
+		int nr_prec, int skip)
+{
+	char name[PATH_MAX];
+	int ret = -1;
+
+	FILE *f = !strcmp(file, "-") ? stdin : fopen(file, "r");
+	int file_done = 0;
+
+	if (!f) {
+		error("cannot open mbox %s", file);
+		goto out;
+	}
+
+	if (fgets(buf, sizeof(buf), f) == NULL) {
+		/* empty stdin is OK */
+		if (f != stdin) {
+			error("cannot read mbox %s", file);
+			goto out;
 		}
+		file_done = 1;
+	}
 
-		if (f != stdin)
-			fclose(f);
+	while (!file_done) {
+		sprintf(name, "%s/%0*d", dir, nr_prec, ++skip);
+		file_done = split_one(f, name, allow_bare);
 	}
+
+	if (f != stdin)
+		fclose(f);
+
 	ret = skip;
 out:
-	free(name);
 	return ret;
 }
+
 int cmd_mailsplit(int argc, const char **argv, const char *prefix)
 {
-	int nr = 0, nr_prec = 4, ret;
+	int nr = 0, nr_prec = 4, ret = 0;
 	int allow_bare = 0;
 	const char *dir = NULL;
 	const char **argp;
@@ -186,7 +235,27 @@ int cmd_mailsplit(int argc, const char **argv, const char *prefix)
 			argp = stdin_only;
 	}
 
-	ret = split_mbox(argp, dir, allow_bare, nr_prec, nr);
+	while (*argp) {
+		const char *arg = *argp++;
+		struct stat argstat;
+
+		if (arg[0] == '-' && arg[1] == 0) {
+			ret |= split_mbox(arg, dir, allow_bare, nr_prec, nr);
+			continue;
+		}
+
+		if (stat(arg, &argstat) == -1) {
+			error("cannot stat %s (%s)", arg, strerror(errno));
+			return 1;
+		}
+
+		if (S_ISDIR(argstat.st_mode)) {
+			ret |= split_maildir(arg, dir, nr_prec, nr);
+		} else {
+			ret |= split_mbox(arg, dir, allow_bare, nr_prec, nr);
+		}
+	}
+
 	if (ret != -1)
 		printf("%d\n", ret);
 
diff --git a/builtin.h b/builtin.h
index d3f3a74..39290d1 100644
--- a/builtin.h
+++ b/builtin.h
@@ -8,7 +8,7 @@ extern const char git_usage_string[];
 
 extern void help_unknown_cmd(const char *cmd);
 extern int mailinfo(FILE *in, FILE *out, int ks, const char *encoding, const char *msg, const char *patch);
-extern int split_mbox(const char **mbox, const char *dir, int allow_bare, int nr_prec, int skip);
+extern int split_mbox(const char *file, const char *dir, int allow_bare, int nr_prec, int skip);
 extern void stripspace(FILE *in, FILE *out);
 extern int write_tree(unsigned char *sha1, int missing_ok, const char *prefix);
 extern void prune_packed_objects(int);
-- 
1.5.1.2

Re: [PATCH] Teach mailsplit about Maildir's

[Fernando J. Pereda]

[-- Attachment #1: Type: text/plain, Size: 493 bytes --]

On Thu, Apr 26, 2007 at 09:24:39PM +0200, Fernando J. Pereda wrote:
> Signed-off-by: Fernando J. Pereda <ferdy@gentoo.org>
> ---
> 
>  builtin-mailsplit.c |  107 ++++++++++++++++++++++++++++++++++++++++++---------
>  builtin.h           |    2 +-
>  2 files changed, 89 insertions(+), 20 deletions(-)
>

Actually, I forgot to update the documentation, I'll send an updated
patch.

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

Re: [PATCH] Teach mailsplit about Maildir's

[Junio C Hamano]

"Fernando J. Pereda" <ferdy@gentoo.org> writes:

> +int split_maildir(const char *maildir, const char *dir, int nr_prec, int skip)
>  {
> ...
> +	while ((maildent = readdir(mddir)) != NULL) {
> +		FILE *f;
> +
> +		snprintf(file, sizeof(file), "%s/%s",
> +				curdir, maildent->d_name);
> +
> +		if (maildent->d_name[0] == '.')
> +			continue;
>  ...
> +		sprintf(name, "%s/%0*d", dir, nr_prec, ++skip);
> +		split_one(f, name, 1);
> +
> +		fclose(f);
> +	}
> +
> +	closedir(mddir);
> +
> +	ret = skip;
> +out:
> +	return ret;
> +}

I do not personally deal with maildir so I do not know for sure,
but this feels very wrong.

What order are you emitting the output?

split_mbox() is designed to number the messages the same order
as they are found in the mailbox, but the above loop relies on
readdir() to give them in a reasonable order to you, which does
not seem a right assumption to me (otherwise "/bin/ls" and
friends would not sort what they read from the filesystem would
they?).

Re: [PATCH] Teach mailsplit about Maildir's

[Fernando J. Pereda]

[-- Attachment #1: Type: text/plain, Size: 1407 bytes --]

On Fri, Apr 27, 2007 at 01:54:55AM -0700, Junio C Hamano wrote:
> "Fernando J. Pereda" <ferdy@gentoo.org> writes:
> 
> > +int split_maildir(const char *maildir, const char *dir, int nr_prec, int skip)
> >  {
> > ...
> > +	while ((maildent = readdir(mddir)) != NULL) {
> > +		FILE *f;
> > +
> > +		snprintf(file, sizeof(file), "%s/%s",
> > +				curdir, maildent->d_name);
> > +
> > +		if (maildent->d_name[0] == '.')
> > +			continue;
> >  ...
> > +		sprintf(name, "%s/%0*d", dir, nr_prec, ++skip);
> > +		split_one(f, name, 1);
> > +
> > +		fclose(f);
> > +	}
> > +
> > +	closedir(mddir);
> > +
> > +	ret = skip;
> > +out:
> > +	return ret;
> > +}
> 
> I do not personally deal with maildir so I do not know for sure,
> but this feels very wrong.
> 
> What order are you emitting the output?
> 
> split_mbox() is designed to number the messages the same order
> as they are found in the mailbox, but the above loop relies on
> readdir() to give them in a reasonable order to you, which does
> not seem a right assumption to me (otherwise "/bin/ls" and
> friends would not sort what they read from the filesystem would
> they?).

It is indeed very wrong. You can't sort them without opening and parsing
the headers. Please drop this patch.

Sorry for the noise.

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