Re: [PATCH] Teach mailsplit about Maildir's

7 messages, 4 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] Teach mailsplit about Maildir's

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:11

"Fernando J. Pereda" [off-list ref] 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

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:11

Hi,

On Sun, 20 May 2007, Junio C Hamano wrote:
"Fernando J. Pereda" [off-list ref] writes:
quoted
-	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

From: Fernando J. Pereda <hidden>
Date: 2016-06-15 22:43:11

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

From: Fernando J. Pereda <hidden>
Date: 2016-06-15 22:43:11

Signed-off-by: Fernando J. Pereda <redacted>
---
 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

From: Alex Riesen <hidden>
Date: 2016-06-15 22:43:12

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 <redacted>
---
 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] Teach mailsplit about Maildir's

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:12

"Fernando J. Pereda" [off-list ref] writes:
quoted hunk
Signed-off-by: Fernando J. Pereda <redacted>
---
 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)?
quoted hunk
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.
quoted hunk
@@ -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

From: Fernando J. Pereda <hidden>
Date: 2016-06-15 22:43:12

On Thu, May 24, 2007 at 12:47:00PM -0700, Junio C Hamano wrote:
"Fernando J. Pereda" [off-list ref] writes:
quoted
          [--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.
quoted
+
+	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
quoted
+	return 1;
+}
Usually we signal success by returning 0.
Ok. Fair enough.
quoted
+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?
quoted
+	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.
quoted
@@ -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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help