[PATCH/WIP v2 00/19] Make git-am a builtin

[PATCH/WIP v2 00/19] Make git-am a builtin

[Paul Tan]

This is a re-roll of [v1]. Thanks Junio, Torsten, Jeff, Eric for the reviews
last round.

Previous versions:

[v1] http://thread.gmane.org/gmane.comp.version-control.git/270048

git-am is a commonly used command for applying a series of patches from a
mailbox to the current branch. Currently, it is implemented by the shell script
git-am.sh. However, compared to C, shell scripts have certain deficiencies:
they need to spawn a lot of processes, introduce a lot of dependencies and
cannot take advantage of git's internal caches.

This WIP patch series rewrites git-am.sh into optimized C builtin/am.c, and is
part of my GSoC project to rewrite git-pull and git-am into C builtins[1].

[1] https://gist.github.com/pyokagan/1b7b0d1f4dab6ba3cef1

Paul Tan (19):
  wrapper: implement xopen()
  wrapper: implement xfopen()
  am: implement skeletal builtin am
  am: implement patch queue mechanism
  am: split out mbox/maildir patches with git-mailsplit
  am: detect mbox patches
  am: extract patch, message and authorship with git-mailinfo
  am: apply patch with git-apply
  am: commit applied patch
  am: refresh the index at start
  am: refuse to apply patches if index is dirty
  am: implement --resolved/--continue
  am: implement --skip
  am: implement --abort
  am: implement quiet option
  am: exit with user friendly message on patch failure
  am: implement am --signoff
  cache-tree: introduce write_index_as_tree()
  am: implement 3-way merge

 Makefile          |    1 +
 builtin.h         |    1 +
 builtin/am.c      | 1214 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 cache-tree.c      |   29 +-
 cache-tree.h      |    1 +
 git-compat-util.h |    2 +
 git.c             |    1 +
 wrapper.c         |   43 ++
 8 files changed, 1280 insertions(+), 12 deletions(-)
 create mode 100644 builtin/am.c

-- 
2.1.4

[PATCH/WIP v2 01/19] wrapper: implement xopen()

[Paul Tan]

A common usage pattern of open() is to check if it was successful, and
die() if it was not:

	int fd = open(path, O_WRONLY | O_CREAT, 0777);
	if (fd < 0)
		die_errno(_("Could not open '%s' for writing."), path);

Implement a wrapper function xopen() that does the above so that we can
save a few lines of code, and make the die() messages consistent.

Helped-by: Torsten Bögershausen <tboegi@web.de>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---

Notes:
    v2
    
    * retry on EINTR
    
    * mode is now an optional argument in xopen(). We use the mode argument
      only if O_CREAT is specified in oflag.

 git-compat-util.h |  1 +
 wrapper.c         | 25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/git-compat-util.h b/git-compat-util.h
index 17584ad..95cc278 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -718,6 +718,7 @@ extern char *xstrndup(const char *str, size_t len);
 extern void *xrealloc(void *ptr, size_t size);
 extern void *xcalloc(size_t nmemb, size_t size);
 extern void *xmmap(void *start, size_t length, int prot, int flags, int fd, off_t offset);
+extern int xopen(const char *path, int flags, ...);
 extern ssize_t xread(int fd, void *buf, size_t len);
 extern ssize_t xwrite(int fd, const void *buf, size_t len);
 extern ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
diff --git a/wrapper.c b/wrapper.c
index c1a663f..82658b3 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -189,6 +189,31 @@ void *xcalloc(size_t nmemb, size_t size)
 # endif
 #endif
 
+/**
+ * xopen() is the same as open(), but it die()s if the open() fails.
+ */
+int xopen(const char *path, int oflag, ...)
+{
+	mode_t mode = 0;
+	va_list ap;
+
+	va_start(ap, oflag);
+	if (oflag & O_CREAT)
+		mode = va_arg(ap, mode_t);
+	va_end(ap);
+
+	assert(path);
+
+	for (;;) {
+		int fd = open(path, oflag, mode);
+		if (fd >= 0)
+			return fd;
+		if (errno == EINTR)
+			continue;
+		die_errno(_("could not open '%s'"), path);
+	}
+}
+
 /*
  * xread() is the same a read(), but it automatically restarts read()
  * operations with a recoverable error (EAGAIN and EINTR). xread()
-- 
2.1.4

[PATCH/WIP v2 02/19] wrapper: implement xfopen()

[Paul Tan]

A common usage pattern of fopen() is to check if it succeeded, and die()
if it failed:

	FILE *fp = fopen(path, "w");
	if (!fp)
		die_errno(_("could not open '%s' for writing"), path);

Implement a wrapper function xfopen() for the above, so that we can save
a few lines of code and make the die() messages consistent.

Signed-off-by: Paul Tan <pyokagan@gmail.com>
---

Notes:
    v2
    
    * Removed the error message distinction between reading and writing.
    
    * Handle EINTR.

 git-compat-util.h |  1 +
 wrapper.c         | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/git-compat-util.h b/git-compat-util.h
index 95cc278..03ea3a2 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -723,6 +723,7 @@ extern ssize_t xread(int fd, void *buf, size_t len);
 extern ssize_t xwrite(int fd, const void *buf, size_t len);
 extern ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
 extern int xdup(int fd);
+extern FILE *xfopen(const char *path, const char *mode);
 extern FILE *xfdopen(int fd, const char *mode);
 extern int xmkstemp(char *template);
 extern int xmkstemp_mode(char *template, int mode);
diff --git a/wrapper.c b/wrapper.c
index 82658b3..9692460 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -336,6 +336,24 @@ int xdup(int fd)
 	return ret;
 }
 
+/**
+ * xfopen() is the same as fopen(), but it die()s if the fopen() fails.
+ */
+FILE *xfopen(const char *path, const char *mode)
+{
+	assert(path);
+	assert(mode);
+
+	for (;;) {
+		FILE *fp = fopen(path, mode);
+		if (fp)
+			return fp;
+		if (errno == EINTR)
+			continue;
+		die_errno(_("could not open '%s'"), path);
+	}
+}
+
 FILE *xfdopen(int fd, const char *mode)
 {
 	FILE *stream = fdopen(fd, mode);
-- 
2.1.4

[PATCH/WIP v2 03/19] am: implement skeletal builtin am

[Paul Tan]

For the purpose of rewriting git-am.sh into a C builtin, implement a
skeletal builtin/am.c that redirects to $GIT_EXEC_PATH/git-am if the
environment variable _GIT_USE_BUILTIN_AM is not defined. Since in the
Makefile git-am.sh takes precedence over builtin/am.c,
$GIT_EXEC_PATH/git-am will contain the shell script git-am.sh, and thus
this allows us to fall back on the functional git-am.sh when running the
test suite for tests that depend on a working git-am implementation.

This redirection will be removed when all the features of git-am.sh have
been re-implemented in builtin/am.c.

Signed-off-by: Paul Tan <pyokagan@gmail.com>
---

Notes:
    v2
    
    * Declare struct am_state state as static.

 Makefile     |  1 +
 builtin.h    |  1 +
 builtin/am.c | 20 ++++++++++++++++++++
 git.c        |  1 +
 4 files changed, 23 insertions(+)
 create mode 100644 builtin/am.c

diff --git a/Makefile b/Makefile
index 54ec511..b82ba0e 100644
--- a/Makefile
+++ b/Makefile
@@ -812,6 +812,7 @@ LIB_OBJS += xdiff-interface.o
 LIB_OBJS += zlib.o
 
 BUILTIN_OBJS += builtin/add.o
+BUILTIN_OBJS += builtin/am.o
 BUILTIN_OBJS += builtin/annotate.o
 BUILTIN_OBJS += builtin/apply.o
 BUILTIN_OBJS += builtin/archive.o
diff --git a/builtin.h b/builtin.h
index b87df70..d50c9d1 100644
--- a/builtin.h
+++ b/builtin.h
@@ -30,6 +30,7 @@ extern int textconv_object(const char *path, unsigned mode, const unsigned char
 extern int is_builtin(const char *s);
 
 extern int cmd_add(int argc, const char **argv, const char *prefix);
+extern int cmd_am(int argc, const char **argv, const char *prefix);
 extern int cmd_annotate(int argc, const char **argv, const char *prefix);
 extern int cmd_apply(int argc, const char **argv, const char *prefix);
 extern int cmd_archive(int argc, const char **argv, const char *prefix);
diff --git a/builtin/am.c b/builtin/am.c
new file mode 100644
index 0000000..0ccbe33
--- /dev/null
+++ b/builtin/am.c
@@ -0,0 +1,20 @@
+/*
+ * Builtin "git am"
+ *
+ * Based on git-am.sh by Junio C Hamano.
+ */
+#include "cache.h"
+#include "builtin.h"
+#include "exec_cmd.h"
+
+int cmd_am(int argc, const char **argv, const char *prefix)
+{
+	if (!getenv("_GIT_USE_BUILTIN_AM")) {
+		const char *path = mkpath("%s/git-am", git_exec_path());
+
+		if (sane_execvp(path, (char**) argv) < 0)
+			die_errno("could not exec %s", path);
+	}
+
+	return 0;
+}
diff --git a/git.c b/git.c
index 44374b1..42328ed 100644
--- a/git.c
+++ b/git.c
@@ -370,6 +370,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 
 static struct cmd_struct commands[] = {
 	{ "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
+	{ "am", cmd_am, RUN_SETUP | NEED_WORK_TREE },
 	{ "annotate", cmd_annotate, RUN_SETUP },
 	{ "apply", cmd_apply, RUN_SETUP_GENTLY },
 	{ "archive", cmd_archive },
-- 
2.1.4

Re: [PATCH/WIP v2 03/19] am: implement skeletal builtin am

[Junio C Hamano]

Paul Tan <pyokagan@gmail.com> writes:

> +int cmd_am(int argc, const char **argv, const char *prefix)
> +{
> +	if (!getenv("_GIT_USE_BUILTIN_AM")) {
> +		const char *path = mkpath("%s/git-am", git_exec_path());
> +
> +		if (sane_execvp(path, (char**) argv) < 0)

Style: 

		if (sane_execvp(path, (char**)argv) < 0)

> +			die_errno("could not exec %s", path);
> +	}
> +
> +	return 0;
> +}
> diff --git a/git.c b/git.c
> index 44374b1..42328ed 100644
> --- a/git.c
> +++ b/git.c
> @@ -370,6 +370,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
>  
>  static struct cmd_struct commands[] = {
>  	{ "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
> +	{ "am", cmd_am, RUN_SETUP | NEED_WORK_TREE },

Would this, especially having RUN_SETUP, keep the same behaviour
when the command is run from a subdirectory of a working tree?
e.g.

	save messages to ./inbox
        $ cd sub/dir
        $ git am ../../inbox



>  	{ "annotate", cmd_annotate, RUN_SETUP },
>  	{ "apply", cmd_apply, RUN_SETUP_GENTLY },
>  	{ "archive", cmd_archive },

Re: [PATCH/WIP v2 03/19] am: implement skeletal builtin am

[Paul Tan]

On Mon, Jun 15, 2015 at 6:08 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Paul Tan <pyokagan@gmail.com> writes:
>> diff --git a/git.c b/git.c
>> index 44374b1..42328ed 100644
>> --- a/git.c
>> +++ b/git.c
>> @@ -370,6 +370,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
>>
>>  static struct cmd_struct commands[] = {
>>       { "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
>> +     { "am", cmd_am, RUN_SETUP | NEED_WORK_TREE },
>
> Would this, especially having RUN_SETUP, keep the same behaviour
> when the command is run from a subdirectory of a working tree?
> e.g.
>
>         save messages to ./inbox
>         $ cd sub/dir
>         $ git am ../../inbox
>

Yes, in 05/19, where the splitting of patches is implemented, we
prefix the mbox paths with the path to the working tree.

There are also tests in t4150 to catch this, introduced in bb034f8
(am: read from the right mailbox when started from a subdirectory,
2008-03-04).

Thanks,
Paul

Re: [PATCH/WIP v2 03/19] am: implement skeletal builtin am

[Junio C Hamano]

Paul Tan <pyokagan@gmail.com> writes:

> On Mon, Jun 15, 2015 at 6:08 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Paul Tan <pyokagan@gmail.com> writes:
>>> diff --git a/git.c b/git.c
>>> index 44374b1..42328ed 100644
>>> --- a/git.c
>>> +++ b/git.c
>>> @@ -370,6 +370,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
>>>
>>>  static struct cmd_struct commands[] = {
>>>       { "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
>>> +     { "am", cmd_am, RUN_SETUP | NEED_WORK_TREE },
>>
>> Would this, especially having RUN_SETUP, keep the same behaviour
>> when the command is run from a subdirectory of a working tree?
>> e.g.
>>
>>         save messages to ./inbox
>>         $ cd sub/dir
>>         $ git am ../../inbox
>>
>
> Yes, in 05/19, where the splitting of patches is implemented, we
> prefix the mbox paths with the path to the working tree.

I wasn't wondering about your new code.

The scripted Porcelain is spawned after applying patches 1-3 from
here, when you do not have _GIT_USE_BUILTIN_AM exported.  Haven't
RUN_SETUP code did its thing by that time?

Re: [PATCH/WIP v2 03/19] am: implement skeletal builtin am

[Paul Tan]

On Tue, Jun 16, 2015 at 1:14 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Paul Tan <pyokagan@gmail.com> writes:
>
>> On Mon, Jun 15, 2015 at 6:08 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Paul Tan <pyokagan@gmail.com> writes:
>>>> diff --git a/git.c b/git.c
>>>> index 44374b1..42328ed 100644
>>>> --- a/git.c
>>>> +++ b/git.c
>>>> @@ -370,6 +370,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
>>>>
>>>>  static struct cmd_struct commands[] = {
>>>>       { "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
>>>> +     { "am", cmd_am, RUN_SETUP | NEED_WORK_TREE },
>>>
>>> Would this, especially having RUN_SETUP, keep the same behaviour
>>> when the command is run from a subdirectory of a working tree?
>>> e.g.
>>>
>>>         save messages to ./inbox
>>>         $ cd sub/dir
>>>         $ git am ../../inbox
>>>
>>
>> Yes, in 05/19, where the splitting of patches is implemented, we
>> prefix the mbox paths with the path to the working tree.
>
> I wasn't wondering about your new code.
>
> The scripted Porcelain is spawned after applying patches 1-3 from
> here, when you do not have _GIT_USE_BUILTIN_AM exported.  Haven't
> RUN_SETUP code did its thing by that time?

Ah right, the RUN_SETUP code would have chdir()-ed to the working
directory root, so git-am.sh will be unable to find the original
working directory. To aid it, we would have to chdir() back to the
original working directory, and unset GIT_DIR.

Thanks,
Paul

Re: [PATCH/WIP v2 03/19] am: implement skeletal builtin am

[Junio C Hamano]

Paul Tan <pyokagan@gmail.com> writes:

>> The scripted Porcelain is spawned after applying patches 1-3 from
>> here, when you do not have _GIT_USE_BUILTIN_AM exported.  Haven't
>> RUN_SETUP code did its thing by that time?
>
> Ah right, the RUN_SETUP code would have chdir()-ed to the working
> directory root, so git-am.sh will be unable to find the original
> working directory. To aid it, we would have to chdir() back to the
> original working directory, and unset GIT_DIR.

I do not think that is a correct workaround, though.  GIT_DIR may
have come from the end user, i.e.

	$ GIT_WORK_TREE=somewhere GIT_DIR=somewhere.else git am ...

As the RUN_SETUP|REQUIRE_WORK_TREE bit is merely a convenence in
git.c, one workable way to keep these dual implementations is to do
what built-in commands used to do before these were invented.
Perhaps studying how cmd_diff(), which is run from git.c without
either RUN_SETUP or NEED_WORK_TREE, and taking good bits from it
would help.  I think the implementation roughly would look like
this:

	int cmd_am(int ac, const char **av, const char *prefix)
	{
                /*
                 * NEEDSWORK: once we retire the dual-mode
                 * implementation, this preamble can be removed...                
                 */
		if (... want to do scripted ...) {
                	... spawn the scripted thing ...
		}
		prefix = setup_git_directory();
                setup_work_tree();
                /* ... up to this point */

		... your real "git am in C" comes here ...
	}

Re: [PATCH/WIP v2 03/19] am: implement skeletal builtin am

[Paul Tan]

On Tue, Jun 16, 2015 at 1:54 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Paul Tan <pyokagan@gmail.com> writes:
>
>>> The scripted Porcelain is spawned after applying patches 1-3 from
>>> here, when you do not have _GIT_USE_BUILTIN_AM exported.  Haven't
>>> RUN_SETUP code did its thing by that time?
>>
>> Ah right, the RUN_SETUP code would have chdir()-ed to the working
>> directory root, so git-am.sh will be unable to find the original
>> working directory. To aid it, we would have to chdir() back to the
>> original working directory, and unset GIT_DIR.
>
> I do not think that is a correct workaround, though.  GIT_DIR may
> have come from the end user, i.e.
>
>         $ GIT_WORK_TREE=somewhere GIT_DIR=somewhere.else git am ...

Ah, forgot about that ><

> As the RUN_SETUP|REQUIRE_WORK_TREE bit is merely a convenence in
> git.c, one workable way to keep these dual implementations is to do
> what built-in commands used to do before these were invented.
> Perhaps studying how cmd_diff(), which is run from git.c without
> either RUN_SETUP or NEED_WORK_TREE, and taking good bits from it
> would help.  I think the implementation roughly would look like
> this:
>
>         int cmd_am(int ac, const char **av, const char *prefix)
>         {
>                 /*
>                  * NEEDSWORK: once we retire the dual-mode
>                  * implementation, this preamble can be removed...
>                  */
>                 if (... want to do scripted ...) {
>                         ... spawn the scripted thing ...
>                 }
>                 prefix = setup_git_directory();
>                 setup_work_tree();
>                 /* ... up to this point */
>
>                 ... your real "git am in C" comes here ...
>         }
>

Ah OK. Just to be sure, I took a look at run_builtin() in git.c, and
indeed it only just does:

prefix = setup_git_directory();
trace_repo_setup(prefix);
setup_work_tree();

which is not bad at all. Thanks.

Regards,
Paul

[PATCH/WIP v2 04/19] am: implement patch queue mechanism

[Paul Tan]

git-am applies a series of patches. If the process terminates
abnormally, we want to be able to resume applying the series of patches.
This requires the session state to be saved in a persistent location.

Implement the mechanism of a "patch queue", represented by 2 integers --
the index of the current patch we are applying and the index of the last
patch, as well as its lifecycle through the following functions:

* am_setup(), which will set up the state directory
  $GIT_DIR/rebase-apply. As such, even if the process exits abnormally,
  the last-known state will still persist.

* am_load(), which is called if there is an am session in
  progress, to load the last known state from the state directory so we
  can resume applying patches.

* am_run(), which will do the actual patch application. After applying a
  patch, it calls am_next() to increment the current patch index. The
  logic for applying and committing a patch is not implemented yet.

* am_destroy(), which is finally called when we successfully applied all
  the patches in the queue, to clean up by removing the state directory
  and its contents.

Signed-off-by: Paul Tan <pyokagan@gmail.com>
---

Notes:
    v2
    
    * Declare struct am_state as static

 builtin/am.c | 164 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 164 insertions(+)

diff --git a/builtin/am.c b/builtin/am.c
index 0ccbe33..f061d21 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -6,6 +6,154 @@
 #include "cache.h"
 #include "builtin.h"
 #include "exec_cmd.h"
+#include "parse-options.h"
+#include "dir.h"
+
+struct am_state {
+	/* state directory path */
+	struct strbuf dir;
+
+	/* current and last patch numbers, 1-indexed */
+	int cur;
+	int last;
+};
+
+/**
+ * Initializes am_state with the default values.
+ */
+static void am_state_init(struct am_state *state)
+{
+	memset(state, 0, sizeof(*state));
+
+	strbuf_init(&state->dir, 0);
+}
+
+/**
+ * Release memory allocated by an am_state.
+ */
+static void am_state_release(struct am_state *state)
+{
+	strbuf_release(&state->dir);
+}
+
+/**
+ * Returns path relative to the am_state directory.
+ */
+static inline const char *am_path(const struct am_state *state, const char *path)
+{
+	return mkpath("%s/%s", state->dir.buf, path);
+}
+
+/**
+ * Returns 1 if there is an am session in progress, 0 otherwise.
+ */
+static int am_in_progress(const struct am_state *state)
+{
+	struct stat st;
+
+	if (lstat(state->dir.buf, &st) < 0 || !S_ISDIR(st.st_mode))
+		return 0;
+	if (lstat(am_path(state, "last"), &st) || !S_ISREG(st.st_mode))
+		return 0;
+	if (lstat(am_path(state, "next"), &st) || !S_ISREG(st.st_mode))
+		return 0;
+	return 1;
+}
+
+/**
+ * Reads the contents of `file`. The third argument can be used to give a hint
+ * about the file size, to avoid reallocs. Returns number of bytes read on
+ * success, -1 if the file does not exist. If trim is set, trailing whitespace
+ * will be removed from the file contents.
+ */
+static int read_state_file(struct strbuf *sb, const char *file, size_t hint, int trim)
+{
+	strbuf_reset(sb);
+	if (strbuf_read_file(sb, file, hint) >= 0) {
+		if (trim)
+			strbuf_rtrim(sb);
+
+		return sb->len;
+	}
+
+	if (errno == ENOENT)
+		return -1;
+
+	die_errno(_("could not read '%s'"), file);
+}
+
+/**
+ * Loads state from disk.
+ */
+static void am_load(struct am_state *state)
+{
+	struct strbuf sb = STRBUF_INIT;
+
+	read_state_file(&sb, am_path(state, "next"), 8, 1);
+	state->cur = strtol(sb.buf, NULL, 10);
+
+	read_state_file(&sb, am_path(state, "last"), 8, 1);
+	state->last = strtol(sb.buf, NULL, 10);
+
+	strbuf_release(&sb);
+}
+
+/**
+ * Remove the am_state directory.
+ */
+static void am_destroy(const struct am_state *state)
+{
+	struct strbuf sb = STRBUF_INIT;
+
+	strbuf_addstr(&sb, state->dir.buf);
+	remove_dir_recursively(&sb, 0);
+	strbuf_release(&sb);
+}
+
+/**
+ * Setup a new am session for applying patches
+ */
+static void am_setup(struct am_state *state)
+{
+	if (mkdir(state->dir.buf, 0777) < 0 && errno != EEXIST)
+		die_errno(_("failed to create directory '%s'"), state->dir.buf);
+
+	write_file(am_path(state, "next"), 1, "%d", state->cur);
+
+	write_file(am_path(state, "last"), 1, "%d", state->last);
+}
+
+/**
+ * Increments the patch pointer, and cleans am_state for the application of the
+ * next patch.
+ */
+static void am_next(struct am_state *state)
+{
+	state->cur++;
+	write_file(am_path(state, "next"), 1, "%d", state->cur);
+}
+
+/**
+ * Applies all queued patches.
+ */
+static void am_run(struct am_state *state)
+{
+	while (state->cur <= state->last)
+		am_next(state);
+
+	am_destroy(state);
+}
+
+static struct am_state state;
+
+static const char * const am_usage[] = {
+	N_("git am [options] [(<mbox>|<Maildir>)...]"),
+	NULL
+};
+
+static struct option am_options[] = {
+	OPT_END()
+};
 
 int cmd_am(int argc, const char **argv, const char *prefix)
 {
@@ -16,5 +164,21 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 			die_errno("could not exec %s", path);
 	}
 
+	git_config(git_default_config, NULL);
+
+	am_state_init(&state);
+	strbuf_addstr(&state.dir, git_path("rebase-apply"));
+
+	argc = parse_options(argc, argv, prefix, am_options, am_usage, 0);
+
+	if (am_in_progress(&state))
+		am_load(&state);
+	else
+		am_setup(&state);
+
+	am_run(&state);
+
+	am_state_release(&state);
+
 	return 0;
 }
-- 
2.1.4

Re: [PATCH/WIP v2 04/19] am: implement patch queue mechanism

[Stefan Beller]

On Thu, Jun 11, 2015 at 3:21 AM, Paul Tan <pyokagan@gmail.com> wrote:
> git-am applies a series of patches. If the process terminates
> abnormally, we want to be able to resume applying the series of patches.
> This requires the session state to be saved in a persistent location.
>
> Implement the mechanism of a "patch queue", represented by 2 integers --
> the index of the current patch we are applying and the index of the last
> patch, as well as its lifecycle through the following functions:
>
> * am_setup(), which will set up the state directory
>   $GIT_DIR/rebase-apply. As such, even if the process exits abnormally,
>   the last-known state will still persist.
>
> * am_load(), which is called if there is an am session in
>   progress, to load the last known state from the state directory so we
>   can resume applying patches.
>
> * am_run(), which will do the actual patch application. After applying a
>   patch, it calls am_next() to increment the current patch index. The
>   logic for applying and committing a patch is not implemented yet.
>
> * am_destroy(), which is finally called when we successfully applied all
>   the patches in the queue, to clean up by removing the state directory
>   and its contents.
>
> Signed-off-by: Paul Tan <pyokagan@gmail.com>
> ---
>
> Notes:
>     v2
>
>     * Declare struct am_state as static
>
>  builtin/am.c | 164 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 164 insertions(+)
>
> diff --git a/builtin/am.c b/builtin/am.c
> index 0ccbe33..f061d21 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -6,6 +6,154 @@
>  #include "cache.h"
>  #include "builtin.h"
>  #include "exec_cmd.h"
> +#include "parse-options.h"
> +#include "dir.h"
> +
> +struct am_state {

Did you mean to declare all the functions below to be static or the
struct as well?
Reading further, you declared it static below. I thought maybe it'd be
useful to have definition
and declaration up here, but having all declarations further below may
be even better.



> +       /* state directory path */
> +       struct strbuf dir;
> +
> +       /* current and last patch numbers, 1-indexed */
> +       int cur;
> +       int last;
> +};
> +
> +/**
> + * Initializes am_state with the default values.
> + */
> +static void am_state_init(struct am_state *state)
> +{
> +       memset(state, 0, sizeof(*state));
> +
> +       strbuf_init(&state->dir, 0);
> +}
> +
> +/**
> + * Release memory allocated by an am_state.
> + */
> +static void am_state_release(struct am_state *state)
> +{
> +       strbuf_release(&state->dir);
> +}
> +
> +/**
> + * Returns path relative to the am_state directory.
> + */
> +static inline const char *am_path(const struct am_state *state, const char *path)
> +{
> +       return mkpath("%s/%s", state->dir.buf, path);
> +}
> +
> +/**
> + * Returns 1 if there is an am session in progress, 0 otherwise.
> + */
> +static int am_in_progress(const struct am_state *state)
> +{
> +       struct stat st;
> +
> +       if (lstat(state->dir.buf, &st) < 0 || !S_ISDIR(st.st_mode))
> +               return 0;
> +       if (lstat(am_path(state, "last"), &st) || !S_ISREG(st.st_mode))
> +               return 0;
> +       if (lstat(am_path(state, "next"), &st) || !S_ISREG(st.st_mode))
> +               return 0;
> +       return 1;
> +}
> +
> +/**
> + * Reads the contents of `file`. The third argument can be used to give a hint
> + * about the file size, to avoid reallocs. Returns number of bytes read on
> + * success, -1 if the file does not exist. If trim is set, trailing whitespace
> + * will be removed from the file contents.
> + */
> +static int read_state_file(struct strbuf *sb, const char *file, size_t hint, int trim)
> +{
> +       strbuf_reset(sb);
> +       if (strbuf_read_file(sb, file, hint) >= 0) {
> +               if (trim)
> +                       strbuf_rtrim(sb);
> +
> +               return sb->len;
> +       }
> +
> +       if (errno == ENOENT)
> +               return -1;
> +
> +       die_errno(_("could not read '%s'"), file);
> +}
> +
> +/**
> + * Loads state from disk.
> + */
> +static void am_load(struct am_state *state)
> +{
> +       struct strbuf sb = STRBUF_INIT;
> +
> +       read_state_file(&sb, am_path(state, "next"), 8, 1);
> +       state->cur = strtol(sb.buf, NULL, 10);
> +
> +       read_state_file(&sb, am_path(state, "last"), 8, 1);
> +       state->last = strtol(sb.buf, NULL, 10);
> +
> +       strbuf_release(&sb);
> +}
> +
> +/**
> + * Remove the am_state directory.
> + */
> +static void am_destroy(const struct am_state *state)
> +{
> +       struct strbuf sb = STRBUF_INIT;
> +
> +       strbuf_addstr(&sb, state->dir.buf);
> +       remove_dir_recursively(&sb, 0);
> +       strbuf_release(&sb);
> +}
> +
> +/**
> + * Setup a new am session for applying patches
> + */
> +static void am_setup(struct am_state *state)
> +{
> +       if (mkdir(state->dir.buf, 0777) < 0 && errno != EEXIST)
> +               die_errno(_("failed to create directory '%s'"), state->dir.buf);
> +
> +       write_file(am_path(state, "next"), 1, "%d", state->cur);
> +
> +       write_file(am_path(state, "last"), 1, "%d", state->last);
> +}
> +
> +/**
> + * Increments the patch pointer, and cleans am_state for the application of the
> + * next patch.
> + */
> +static void am_next(struct am_state *state)
> +{
> +       state->cur++;
> +       write_file(am_path(state, "next"), 1, "%d", state->cur);
> +}
> +
> +/**
> + * Applies all queued patches.
> + */
> +static void am_run(struct am_state *state)
> +{
> +       while (state->cur <= state->last)
> +               am_next(state);
> +
> +       am_destroy(state);
> +}
> +
> +static struct am_state state;
> +
> +static const char * const am_usage[] = {
> +       N_("git am [options] [(<mbox>|<Maildir>)...]"),
> +       NULL
> +};
> +
> +static struct option am_options[] = {
> +       OPT_END()
> +};
>
>  int cmd_am(int argc, const char **argv, const char *prefix)
>  {
> @@ -16,5 +164,21 @@ int cmd_am(int argc, const char **argv, const char *prefix)
>                         die_errno("could not exec %s", path);
>         }
>
> +       git_config(git_default_config, NULL);
> +
> +       am_state_init(&state);
> +       strbuf_addstr(&state.dir, git_path("rebase-apply"));
> +
> +       argc = parse_options(argc, argv, prefix, am_options, am_usage, 0);
> +
> +       if (am_in_progress(&state))
> +               am_load(&state);
> +       else
> +               am_setup(&state);
> +
> +       am_run(&state);
> +
> +       am_state_release(&state);
> +
>         return 0;
>  }
> --
> 2.1.4
>

Re: [PATCH/WIP v2 04/19] am: implement patch queue mechanism

[Paul Tan]

On Fri, Jun 12, 2015 at 1:39 AM, Stefan Beller <sbeller@google.com> wrote:
> On Thu, Jun 11, 2015 at 3:21 AM, Paul Tan <pyokagan@gmail.com> wrote:
>> Notes:
>>     v2
>>
>>     * Declare struct am_state as static
>>
>>  builtin/am.c | 164 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 164 insertions(+)
>>
>> diff --git a/builtin/am.c b/builtin/am.c
>> index 0ccbe33..f061d21 100644
>> --- a/builtin/am.c
>> +++ b/builtin/am.c
>> @@ -6,6 +6,154 @@
>>  #include "cache.h"
>>  #include "builtin.h"
>>  #include "exec_cmd.h"
>> +#include "parse-options.h"
>> +#include "dir.h"
>> +
>> +struct am_state {
>
> Did you mean to declare all the functions below to be static or the
> struct as well?

Well, everything in this file, with the exception of the cmd_am()
entry point, should have static linkage.

The changelog comment was referring to [1], but I should have made it
clearer. Sorry if it was confusing.

[1] http://thread.gmane.org/gmane.comp.version-control.git/270048/focus=270205

> Reading further, you declared it static below. I thought maybe it'd be
> useful to have definition
> and declaration up here, but having all declarations further below may
> be even better.

Right, I aimed to have a strict separation between "git-am: the
functionality" and "git-am: the command-line interface", where the
latter depends on the former, and not the other way round (or have
circular dependencies). The former perhaps could even be moved into
libgit.a in the future.

Thanks,
Paul

[PATCH/WIP v2 05/19] am: split out mbox/maildir patches with git-mailsplit

[Paul Tan]

git-am.sh supports mbox, stgit and mercurial patches. Re-implement
support for splitting out mbox/maildirs using git-mailsplit, while also
implementing the framework required to support other patch formats in
the future.

Re-implement support for the --patch-format option (since a5a6755
(git-am foreign patch support: introduce patch_format, 2009-05-27)) to
allow the user to choose between the different patch formats.

Signed-off-by: Paul Tan <pyokagan@gmail.com>
---

Notes:
    v2
    
    * Declare int opt_patchformat as static.
    
    * Fix up indentation style for the switch()

 builtin/am.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 103 insertions(+), 4 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index f061d21..5198a8e 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -8,6 +8,12 @@
 #include "exec_cmd.h"
 #include "parse-options.h"
 #include "dir.h"
+#include "run-command.h"
+
+enum patch_format {
+	PATCH_FORMAT_UNKNOWN = 0,
+	PATCH_FORMAT_MBOX
+};
 
 struct am_state {
 	/* state directory path */
@@ -16,6 +22,9 @@ struct am_state {
 	/* current and last patch numbers, 1-indexed */
 	int cur;
 	int last;
+
+	/* number of digits in patch filename */
+	int prec;
 };
 
 /**
@@ -26,6 +35,7 @@ static void am_state_init(struct am_state *state)
 	memset(state, 0, sizeof(*state));
 
 	strbuf_init(&state->dir, 0);
+	state->prec = 4;
 }
 
 /**
@@ -111,13 +121,67 @@ static void am_destroy(const struct am_state *state)
 }
 
 /**
+ * Splits out individual patches from `paths`, where each path is either a mbox
+ * file or a Maildir. Return 0 on success, -1 on failure.
+ */
+static int split_patches_mbox(struct am_state *state, struct string_list *paths)
+{
+	struct child_process cp = CHILD_PROCESS_INIT;
+	struct string_list_item *item;
+	struct strbuf last = STRBUF_INIT;
+
+	cp.git_cmd = 1;
+	argv_array_push(&cp.args, "mailsplit");
+	argv_array_pushf(&cp.args, "-d%d", state->prec);
+	argv_array_pushf(&cp.args, "-o%s", state->dir.buf);
+	argv_array_push(&cp.args, "-b");
+	argv_array_push(&cp.args, "--");
+
+	for_each_string_list_item(item, paths)
+		argv_array_push(&cp.args, item->string);
+
+	if (capture_command(&cp, &last, 8))
+		return -1;
+
+	state->cur = 1;
+	state->last = strtol(last.buf, NULL, 10);
+
+	return 0;
+}
+
+/**
+ * Splits out individual patches, of patch_format, contained within paths.
+ * These patches will be stored in the state directory, with each patch's
+ * filename being its index, padded to state->prec digits. state->cur will be
+ * set to the index of the first patch, and state->last will be set to the
+ * index of the last patch. Returns 0 on success, -1 on failure.
+ */
+static int split_patches(struct am_state *state, enum patch_format patch_format,
+		struct string_list *paths)
+{
+	switch (patch_format) {
+	case PATCH_FORMAT_MBOX:
+		return split_patches_mbox(state, paths);
+	default:
+		die("BUG: invalid patch_format");
+	}
+	return -1;
+}
+
+/**
  * Setup a new am session for applying patches
  */
-static void am_setup(struct am_state *state)
+static void am_setup(struct am_state *state, enum patch_format patch_format,
+		struct string_list *paths)
 {
 	if (mkdir(state->dir.buf, 0777) < 0 && errno != EEXIST)
 		die_errno(_("failed to create directory '%s'"), state->dir.buf);
 
+	if (split_patches(state, patch_format, paths) < 0) {
+		am_destroy(state);
+		die(_("Failed to split patches."));
+	}
+
 	write_file(am_path(state, "next"), 1, "%d", state->cur);
 
 	write_file(am_path(state, "last"), 1, "%d", state->last);
@@ -138,13 +202,33 @@ static void am_next(struct am_state *state)
  */
 static void am_run(struct am_state *state)
 {
-	while (state->cur <= state->last)
+	while (state->cur <= state->last) {
+
+		/* TODO: Patch application not implemented yet */
+
 		am_next(state);
+	}
 
 	am_destroy(state);
 }
 
+/**
+ * parse_options() callback that validates and sets opt->value to the
+ * PATCH_FORMAT_* enum value corresponding to `arg`.
+ */
+static int parse_opt_patchformat(const struct option *opt, const char *arg, int unset)
+{
+	int *opt_value = opt->value;
+
+	if (!strcmp(arg, "mbox"))
+		*opt_value = PATCH_FORMAT_MBOX;
+	else
+		return -1;
+	return 0;
+}
+
 static struct am_state state;
+static int opt_patch_format;
 
 static const char * const am_usage[] = {
 	N_("git am [options] [(<mbox>|<Maildir>)...]"),
@@ -152,6 +236,8 @@ static const char * const am_usage[] = {
 };
 
 static struct option am_options[] = {
+	OPT_CALLBACK(0, "patch-format", &opt_patch_format, N_("format"),
+		N_("format the patch(es) are in"), parse_opt_patchformat),
 	OPT_END()
 };
 
@@ -173,8 +259,21 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 
 	if (am_in_progress(&state))
 		am_load(&state);
-	else
-		am_setup(&state);
+	else {
+		struct string_list paths = STRING_LIST_INIT_DUP;
+		int i;
+
+		for (i = 0; i < argc; i++) {
+			if (is_absolute_path(argv[i]) || !prefix)
+				string_list_append(&paths, argv[i]);
+			else
+				string_list_append(&paths, mkpath("%s/%s", prefix, argv[i]));
+		}
+
+		am_setup(&state, opt_patch_format, &paths);
+
+		string_list_clear(&paths, 0);
+	}
 
 	am_run(&state);
 
-- 
2.1.4

Re: [PATCH/WIP v2 05/19] am: split out mbox/maildir patches with git-mailsplit

[Stefan Beller]

On Thu, Jun 11, 2015 at 3:21 AM, Paul Tan <pyokagan@gmail.com> wrote:
> git-am.sh supports mbox, stgit and mercurial patches. Re-implement
> support for splitting out mbox/maildirs using git-mailsplit, while also
> implementing the framework required to support other patch formats in
> the future.
>
> Re-implement support for the --patch-format option (since a5a6755
> (git-am foreign patch support: introduce patch_format, 2009-05-27)) to
> allow the user to choose between the different patch formats.
>
> Signed-off-by: Paul Tan <pyokagan@gmail.com>
> ---
>
> Notes:
>     v2
>
>     * Declare int opt_patchformat as static.
>
>     * Fix up indentation style for the switch()
>
>  builtin/am.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 103 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/am.c b/builtin/am.c
> index f061d21..5198a8e 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -8,6 +8,12 @@
>  #include "exec_cmd.h"
>  #include "parse-options.h"
>  #include "dir.h"
> +#include "run-command.h"
> +
> +enum patch_format {
> +       PATCH_FORMAT_UNKNOWN = 0,
> +       PATCH_FORMAT_MBOX
> +};
>
>  struct am_state {
>         /* state directory path */
> @@ -16,6 +22,9 @@ struct am_state {
>         /* current and last patch numbers, 1-indexed */
>         int cur;
>         int last;
> +
> +       /* number of digits in patch filename */
> +       int prec;
>  };
>
>  /**
> @@ -26,6 +35,7 @@ static void am_state_init(struct am_state *state)
>         memset(state, 0, sizeof(*state));
>
>         strbuf_init(&state->dir, 0);
> +       state->prec = 4;
>  }
>
>  /**
> @@ -111,13 +121,67 @@ static void am_destroy(const struct am_state *state)
>  }
>
>  /**
> + * Splits out individual patches from `paths`, where each path is either a mbox
> + * file or a Maildir. Return 0 on success, -1 on failure.
> + */
> +static int split_patches_mbox(struct am_state *state, struct string_list *paths)
> +{
> +       struct child_process cp = CHILD_PROCESS_INIT;
> +       struct string_list_item *item;
> +       struct strbuf last = STRBUF_INIT;
> +
> +       cp.git_cmd = 1;
> +       argv_array_push(&cp.args, "mailsplit");
> +       argv_array_pushf(&cp.args, "-d%d", state->prec);
> +       argv_array_pushf(&cp.args, "-o%s", state->dir.buf);
> +       argv_array_push(&cp.args, "-b");
> +       argv_array_push(&cp.args, "--");
> +
> +       for_each_string_list_item(item, paths)
> +               argv_array_push(&cp.args, item->string);
> +
> +       if (capture_command(&cp, &last, 8))
> +               return -1;
> +
> +       state->cur = 1;
> +       state->last = strtol(last.buf, NULL, 10);
> +
> +       return 0;
> +}
> +
> +/**
> + * Splits out individual patches, of patch_format, contained within paths.
> + * These patches will be stored in the state directory, with each patch's
> + * filename being its index, padded to state->prec digits. state->cur will be
> + * set to the index of the first patch, and state->last will be set to the
> + * index of the last patch. Returns 0 on success, -1 on failure.
> + */
> +static int split_patches(struct am_state *state, enum patch_format patch_format,
> +               struct string_list *paths)
> +{
> +       switch (patch_format) {
> +       case PATCH_FORMAT_MBOX:
> +               return split_patches_mbox(state, paths);
> +       default:
> +               die("BUG: invalid patch_format");
> +       }
> +       return -1;
> +}
> +
> +/**
>   * Setup a new am session for applying patches
>   */
> -static void am_setup(struct am_state *state)
> +static void am_setup(struct am_state *state, enum patch_format patch_format,
> +               struct string_list *paths)
>  {
>         if (mkdir(state->dir.buf, 0777) < 0 && errno != EEXIST)
>                 die_errno(_("failed to create directory '%s'"), state->dir.buf);
>
> +       if (split_patches(state, patch_format, paths) < 0) {
> +               am_destroy(state);
> +               die(_("Failed to split patches."));
> +       }
> +
>         write_file(am_path(state, "next"), 1, "%d", state->cur);
>
>         write_file(am_path(state, "last"), 1, "%d", state->last);
> @@ -138,13 +202,33 @@ static void am_next(struct am_state *state)
>   */
>  static void am_run(struct am_state *state)
>  {
> -       while (state->cur <= state->last)
> +       while (state->cur <= state->last) {
> +
> +               /* TODO: Patch application not implemented yet */
> +
>                 am_next(state);
> +       }

When reviewing the previous patch I did look at this loop for awhile confused,
if you want to apply patches in am_next(state) and thought there might be
a better approach.

Maybe you want to move this chunk with the TODO into the previous patch,
so it's clear after reading the documentation of am_run, that the actual am is
missing there.


>
>         am_destroy(state);
>  }
>
> +/**
> + * parse_options() callback that validates and sets opt->value to the
> + * PATCH_FORMAT_* enum value corresponding to `arg`.
> + */
> +static int parse_opt_patchformat(const struct option *opt, const char *arg, int unset)
> +{
> +       int *opt_value = opt->value;
> +
> +       if (!strcmp(arg, "mbox"))
> +               *opt_value = PATCH_FORMAT_MBOX;
> +       else
> +               return -1;
> +       return 0;
> +}
> +
>  static struct am_state state;
> +static int opt_patch_format;
>
>  static const char * const am_usage[] = {
>         N_("git am [options] [(<mbox>|<Maildir>)...]"),
> @@ -152,6 +236,8 @@ static const char * const am_usage[] = {
>  };
>
>  static struct option am_options[] = {
> +       OPT_CALLBACK(0, "patch-format", &opt_patch_format, N_("format"),
> +               N_("format the patch(es) are in"), parse_opt_patchformat),
>         OPT_END()
>  };
>
> @@ -173,8 +259,21 @@ int cmd_am(int argc, const char **argv, const char *prefix)
>
>         if (am_in_progress(&state))
>                 am_load(&state);
> -       else
> -               am_setup(&state);
> +       else {
> +               struct string_list paths = STRING_LIST_INIT_DUP;
> +               int i;
> +
> +               for (i = 0; i < argc; i++) {
> +                       if (is_absolute_path(argv[i]) || !prefix)
> +                               string_list_append(&paths, argv[i]);
> +                       else
> +                               string_list_append(&paths, mkpath("%s/%s", prefix, argv[i]));
> +               }
> +
> +               am_setup(&state, opt_patch_format, &paths);
> +
> +               string_list_clear(&paths, 0);
> +       }
>
>         am_run(&state);
>
> --
> 2.1.4
>

Re: [PATCH/WIP v2 05/19] am: split out mbox/maildir patches with git-mailsplit

[Paul Tan]

On Fri, Jun 12, 2015 at 1:45 AM, Stefan Beller <sbeller@google.com> wrote:
> On Thu, Jun 11, 2015 at 3:21 AM, Paul Tan <pyokagan@gmail.com> wrote:
>> @@ -138,13 +202,33 @@ static void am_next(struct am_state *state)
>>   */
>>  static void am_run(struct am_state *state)
>>  {
>> -       while (state->cur <= state->last)
>> +       while (state->cur <= state->last) {
>> +
>> +               /* TODO: Patch application not implemented yet */
>> +
>>                 am_next(state);
>> +       }
>
> When reviewing the previous patch I did look at this loop for awhile confused,
> if you want to apply patches in am_next(state) and thought there might be
> a better approach.
>
> Maybe you want to move this chunk with the TODO into the previous patch,
> so it's clear after reading the documentation of am_run, that the actual am is
> missing there.

Ah right, this is a mistake. This comment should be in the previous patch.

Thanks,
Paul

[PATCH/WIP v2 06/19] am: detect mbox patches

[Paul Tan]

Since 15ced75 (git-am foreign patch support: autodetect some patch
formats, 2009-05-27), git-am.sh is able to autodetect mbox, stgit and
mercurial patches through heuristics.

Re-implement support for autodetecting mbox/maildir files.

Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---

Notes:
    v2
    
    * Various small code tweaks suggested by Eric.

 builtin/am.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 98 insertions(+)

diff --git a/builtin/am.c b/builtin/am.c
index 5198a8e..7379b97 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -120,6 +120,96 @@ static void am_destroy(const struct am_state *state)
 	strbuf_release(&sb);
 }
 
+/*
+ * Returns 1 if the file looks like a piece of email a-la RFC2822, 0 otherwise.
+ * We check this by grabbing all the non-indented lines and seeing if they look
+ * like they begin with valid header field names.
+ */
+static int is_email(const char *filename)
+{
+	struct strbuf sb = STRBUF_INIT;
+	FILE *fp = xfopen(filename, "r");
+	int ret = 1;
+
+	while (!strbuf_getline(&sb, fp, '\n')) {
+		const char *x;
+
+		strbuf_rtrim(&sb);
+
+		if (!sb.len)
+			break; /* End of header */
+
+		/* Ignore indented folded lines */
+		if (*sb.buf == '\t' || *sb.buf == ' ')
+			continue;
+
+		/* It's a header if it matches the regexp "^[!-9;-~]+:" */
+		for (x = sb.buf; *x; x++) {
+			if (('!' <= *x && *x <= '9') || (';' <= *x && *x <= '~'))
+				continue;
+			if (*x == ':' && x != sb.buf)
+				break;
+			ret = 0;
+			goto done;
+		}
+	}
+
+done:
+	fclose(fp);
+	strbuf_release(&sb);
+	return ret;
+}
+
+/**
+ * Attempts to detect the patch_format of the patches contained in `paths`,
+ * returning the PATCH_FORMAT_* enum value. Returns PATCH_FORMAT_UNKNOWN if
+ * detection fails.
+ */
+static int detect_patch_format(struct string_list *paths)
+{
+	enum patch_format ret = PATCH_FORMAT_UNKNOWN;
+	struct strbuf l1 = STRBUF_INIT;
+	struct strbuf l2 = STRBUF_INIT;
+	struct strbuf l3 = STRBUF_INIT;
+	FILE *fp;
+
+	/*
+	 * We default to mbox format if input is from stdin and for directories
+	 */
+	if (!paths->nr || !strcmp(paths->items->string, "-") ||
+	    is_directory(paths->items->string)) {
+		ret = PATCH_FORMAT_MBOX;
+		goto done;
+	}
+
+	/*
+	 * Otherwise, check the first 3 lines of the first patch, starting
+	 * from the first non-blank line, to try to detect its format.
+	 */
+	fp = xfopen(paths->items->string, "r");
+	while (!strbuf_getline(&l1, fp, '\n')) {
+		strbuf_trim(&l1);
+		if (l1.len)
+			break;
+	}
+	strbuf_getline(&l2, fp, '\n');
+	strbuf_trim(&l2);
+	strbuf_getline(&l3, fp, '\n');
+	strbuf_trim(&l3);
+	fclose(fp);
+
+	if (starts_with(l1.buf, "From ") || starts_with(l1.buf, "From: "))
+		ret = PATCH_FORMAT_MBOX;
+	else if (l1.len && l2.len && l3.len && is_email(paths->items->string))
+		ret = PATCH_FORMAT_MBOX;
+
+done:
+	strbuf_release(&l1);
+	strbuf_release(&l2);
+	strbuf_release(&l3);
+	return ret;
+}
+
 /**
  * Splits out individual patches from `paths`, where each path is either a mbox
  * file or a Maildir. Return 0 on success, -1 on failure.
@@ -174,6 +264,14 @@ static int split_patches(struct am_state *state, enum patch_format patch_format,
 static void am_setup(struct am_state *state, enum patch_format patch_format,
 		struct string_list *paths)
 {
+	if (!patch_format)
+		patch_format = detect_patch_format(paths);
+
+	if (!patch_format) {
+		fprintf_ln(stderr, _("Patch format detection failed."));
+		exit(128);
+	}
+
 	if (mkdir(state->dir.buf, 0777) < 0 && errno != EEXIST)
 		die_errno(_("failed to create directory '%s'"), state->dir.buf);
 
-- 
2.1.4

[PATCH/WIP v2 07/19] am: extract patch, message and authorship with git-mailinfo

[Paul Tan]

For the purpose of applying the patch and committing the results,
implement extracting the patch data, commit message and authorship from
an e-mail message using git-mailinfo.

git-mailinfo is run as a separate process, but ideally in the future,
we should be be able to access its functionality directly without
spawning a new process.

Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---

Notes:
    v2
    
    * use die_errno()
    
    * use '%*d' as the format specifier for msgnum()

 builtin/am.c | 228 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 228 insertions(+)

diff --git a/builtin/am.c b/builtin/am.c
index 7379b97..a1db474 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -9,6 +9,23 @@
 #include "parse-options.h"
 #include "dir.h"
 #include "run-command.h"
+#include "quote.h"
+
+/**
+ * Returns 1 if the file is empty or does not exist, 0 otherwise.
+ */
+static int is_empty_file(const char *filename)
+{
+	struct stat st;
+
+	if (stat(filename, &st) < 0) {
+		if (errno == ENOENT)
+			return 1;
+		die_errno(_("could not stat %s"), filename);
+	}
+
+	return !st.st_size;
+}
 
 enum patch_format {
 	PATCH_FORMAT_UNKNOWN = 0,
@@ -23,6 +40,12 @@ struct am_state {
 	int cur;
 	int last;
 
+	/* commit message and metadata */
+	struct strbuf author_name;
+	struct strbuf author_email;
+	struct strbuf author_date;
+	struct strbuf msg;
+
 	/* number of digits in patch filename */
 	int prec;
 };
@@ -35,6 +58,10 @@ static void am_state_init(struct am_state *state)
 	memset(state, 0, sizeof(*state));
 
 	strbuf_init(&state->dir, 0);
+	strbuf_init(&state->author_name, 0);
+	strbuf_init(&state->author_email, 0);
+	strbuf_init(&state->author_date, 0);
+	strbuf_init(&state->msg, 0);
 	state->prec = 4;
 }
 
@@ -44,6 +71,10 @@ static void am_state_init(struct am_state *state)
 static void am_state_release(struct am_state *state)
 {
 	strbuf_release(&state->dir);
+	strbuf_release(&state->author_name);
+	strbuf_release(&state->author_email);
+	strbuf_release(&state->author_date);
+	strbuf_release(&state->msg);
 }
 
 /**
@@ -93,6 +124,95 @@ static int read_state_file(struct strbuf *sb, const char *file, size_t hint, int
 }
 
 /**
+ * Parses the "author script" `filename`, and sets state->author_name,
+ * state->author_email and state->author_date accordingly. We are strict with
+ * our parsing, as the author script is supposed to be eval'd, and loosely
+ * parsing it may not give the results the user expects.
+ *
+ * The author script is of the format:
+ *
+ * 	GIT_AUTHOR_NAME='$author_name'
+ * 	GIT_AUTHOR_EMAIL='$author_email'
+ * 	GIT_AUTHOR_DATE='$author_date'
+ *
+ * where $author_name, $author_email and $author_date are quoted.
+ */
+static int read_author_script(struct am_state *state)
+{
+	char *value;
+	struct strbuf sb = STRBUF_INIT;
+	const char *filename = am_path(state, "author-script");
+	FILE *fp = fopen(filename, "r");
+	if (!fp) {
+		if (errno == ENOENT)
+			return 0;
+		die_errno(_("could not open '%s' for reading"), filename);
+	}
+
+	if (strbuf_getline(&sb, fp, '\n'))
+		return -1;
+	if (!skip_prefix(sb.buf, "GIT_AUTHOR_NAME=", (const char**) &value))
+		return -1;
+	value = sq_dequote(value);
+	if (!value)
+		return -1;
+	strbuf_reset(&state->author_name);
+	strbuf_addstr(&state->author_name, value);
+
+	if (strbuf_getline(&sb, fp, '\n'))
+		return -1;
+	if (!skip_prefix(sb.buf, "GIT_AUTHOR_EMAIL=", (const char**) &value))
+		return -1;
+	value = sq_dequote(value);
+	if (!value)
+		return -1;
+	strbuf_reset(&state->author_email);
+	strbuf_addstr(&state->author_email, value);
+
+	if (strbuf_getline(&sb, fp, '\n'))
+		return -1;
+	if (!skip_prefix(sb.buf, "GIT_AUTHOR_DATE=", (const char**) &value))
+		return -1;
+	value = sq_dequote(value);
+	if (!value)
+		return -1;
+	strbuf_reset(&state->author_date);
+	strbuf_addstr(&state->author_date, value);
+
+	if (fgetc(fp) != EOF)
+		return -1;
+
+	fclose(fp);
+	strbuf_release(&sb);
+	return 0;
+}
+
+/**
+ * Saves state->author_name, state->author_email and state->author_date in
+ * `filename` as an "author script", which is the format used by git-am.sh.
+ */
+static void write_author_script(const struct am_state *state)
+{
+	static const char fmt[] = "GIT_AUTHOR_NAME=%s\n"
+		"GIT_AUTHOR_EMAIL=%s\n"
+		"GIT_AUTHOR_DATE=%s\n";
+	struct strbuf author_name = STRBUF_INIT;
+	struct strbuf author_email = STRBUF_INIT;
+	struct strbuf author_date = STRBUF_INIT;
+
+	sq_quote_buf(&author_name, state->author_name.buf);
+	sq_quote_buf(&author_email, state->author_email.buf);
+	sq_quote_buf(&author_date, state->author_date.buf);
+
+	write_file(am_path(state, "author-script"), 1, fmt,
+			author_name.buf, author_email.buf, author_date.buf);
+
+	strbuf_release(&author_name);
+	strbuf_release(&author_email);
+	strbuf_release(&author_date);
+}
+
+/**
  * Loads state from disk.
  */
 static void am_load(struct am_state *state)
@@ -105,6 +225,11 @@ static void am_load(struct am_state *state)
 	read_state_file(&sb, am_path(state, "last"), 8, 1);
 	state->last = strtol(sb.buf, NULL, 10);
 
+	if (read_author_script(state) < 0)
+		die(_("could not parse author script"));
+
+	read_state_file(&state->msg, am_path(state, "final-commit"), 0, 0);
+
 	strbuf_release(&sb);
 }
 
@@ -293,6 +418,98 @@ static void am_next(struct am_state *state)
 {
 	state->cur++;
 	write_file(am_path(state, "next"), 1, "%d", state->cur);
+
+	strbuf_reset(&state->author_name);
+	strbuf_reset(&state->author_email);
+	strbuf_reset(&state->author_date);
+	unlink(am_path(state, "author-script"));
+
+	strbuf_reset(&state->msg);
+	unlink(am_path(state, "final-commit"));
+}
+
+/**
+ * Returns the filename of the current patch.
+ */
+static const char *msgnum(const struct am_state *state)
+{
+	static struct strbuf sb = STRBUF_INIT;
+
+	strbuf_reset(&sb);
+	strbuf_addf(&sb, "%0*d", state->prec, state->cur);
+
+	return sb.buf;
+}
+
+/**
+ * Parses `patch` using git-mailinfo. state->msg will be set to the patch
+ * message. state->author_name, state->author_email, state->author_date will be
+ * set to the patch author's name, email and date respectively. The patch's
+ * body will be written to "$state_dir/patch", where $state_dir is the state
+ * directory.
+ *
+ * Returns 1 if the patch should be skipped, 0 otherwise.
+ */
+static int parse_patch(struct am_state *state, const char *patch)
+{
+	FILE *fp;
+	struct child_process cp = CHILD_PROCESS_INIT;
+	struct strbuf sb = STRBUF_INIT;
+
+	cp.git_cmd = 1;
+	cp.in = xopen(patch, O_RDONLY, 0);
+	cp.out = xopen(am_path(state, "info"), O_WRONLY | O_CREAT, 0777);
+
+	argv_array_push(&cp.args, "mailinfo");
+	argv_array_push(&cp.args, am_path(state, "msg"));
+	argv_array_push(&cp.args, am_path(state, "patch"));
+
+	if (run_command(&cp) < 0)
+		die("could not parse patch");
+
+	close(cp.in);
+	close(cp.out);
+
+	/* Extract message and author information */
+	fp = xfopen(am_path(state, "info"), "r");
+	while (!strbuf_getline(&sb, fp, '\n')) {
+		const char *x;
+
+		if (skip_prefix(sb.buf, "Subject: ", &x)) {
+			if (state->msg.len)
+				strbuf_addch(&state->msg, '\n');
+			strbuf_addstr(&state->msg, x);
+		} else if (skip_prefix(sb.buf, "Author: ", &x)) {
+			if (state->author_name.len)
+				strbuf_addch(&state->author_name, '\n');
+			strbuf_addstr(&state->author_name, x);
+		} else if (skip_prefix(sb.buf, "Email: ", &x)) {
+			if (state->author_email.len)
+				strbuf_addch(&state->author_email, '\n');
+			strbuf_addstr(&state->author_email, x);
+		} else if (skip_prefix(sb.buf, "Date: ", &x)) {
+			if (state->author_date.len)
+				strbuf_addch(&state->author_date, '\n');
+			strbuf_addstr(&state->author_date, x);
+		}
+	}
+	fclose(fp);
+
+	/* Skip pine's internal folder data */
+	if (!strcmp(state->author_name.buf, "Mail System Internal Data"))
+		return 1;
+
+	if (is_empty_file(am_path(state, "patch")))
+		die(_("Patch is empty. Was it split wrong?\n"
+		"If you would prefer to skip this patch, instead run \"git am --skip\".\n"
+		"To restore the original branch and stop patching run \"git am --abort\"."));
+
+	strbuf_addstr(&state->msg, "\n\n");
+	if (strbuf_read_file(&state->msg, am_path(state, "msg"), 0) < 0)
+		die_errno(_("could not read '%s'"), am_path(state, "msg"));
+	stripspace(&state->msg, 0);
+
+	return 0;
 }
 
 /**
@@ -301,9 +518,20 @@ static void am_next(struct am_state *state)
 static void am_run(struct am_state *state)
 {
 	while (state->cur <= state->last) {
+		const char *patch = am_path(state, msgnum(state));
+
+		if (!file_exists(patch))
+			goto next;
+
+		if (parse_patch(state, patch))
+			goto next; /* patch should be skipped */
+
+		write_author_script(state);
+		write_file(am_path(state, "final-commit"), 1, "%s", state->msg.buf);
 
 		/* TODO: Patch application not implemented yet */
 
+next:
 		am_next(state);
 	}
 
-- 
2.1.4

Re: [PATCH/WIP v2 07/19] am: extract patch, message and authorship with git-mailinfo

[Junio C Hamano]

Paul Tan <pyokagan@gmail.com> writes:

> For the purpose of applying the patch and committing the results,
> implement extracting the patch data, commit message and authorship from
> an e-mail message using git-mailinfo.
>
> git-mailinfo is run as a separate process, but ideally in the future,
> we should be be able to access its functionality directly without
> spawning a new process.
>
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Helped-by: Jeff King <peff@peff.net>
> Signed-off-by: Paul Tan <pyokagan@gmail.com>
> ---
>
> Notes:
>     v2
>     
>     * use die_errno()
>     
>     * use '%*d' as the format specifier for msgnum()
>
>  builtin/am.c | 228 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 228 insertions(+)
>
> diff --git a/builtin/am.c b/builtin/am.c
> index 7379b97..a1db474 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -9,6 +9,23 @@
>  #include "parse-options.h"
>  #include "dir.h"
>  #include "run-command.h"
> +#include "quote.h"
> +
> +/**
> + * Returns 1 if the file is empty or does not exist, 0 otherwise.
> + */
> +static int is_empty_file(const char *filename)
> +{
> +	struct stat st;
> +
> +	if (stat(filename, &st) < 0) {
> +		if (errno == ENOENT)
> +			return 1;
> +		die_errno(_("could not stat %s"), filename);
> +	}
> +
> +	return !st.st_size;
> +}
>  
>  enum patch_format {
>  	PATCH_FORMAT_UNKNOWN = 0,
> @@ -23,6 +40,12 @@ struct am_state {
>  	int cur;
>  	int last;
>  
> +	/* commit message and metadata */
> +	struct strbuf author_name;
> +	struct strbuf author_email;
> +	struct strbuf author_date;
> +	struct strbuf msg;
> +
>  	/* number of digits in patch filename */
>  	int prec;
>  };
> @@ -35,6 +58,10 @@ static void am_state_init(struct am_state *state)
>  	memset(state, 0, sizeof(*state));
>  
>  	strbuf_init(&state->dir, 0);
> +	strbuf_init(&state->author_name, 0);
> +	strbuf_init(&state->author_email, 0);
> +	strbuf_init(&state->author_date, 0);
> +	strbuf_init(&state->msg, 0);
>  	state->prec = 4;
>  }
>  
> @@ -44,6 +71,10 @@ static void am_state_init(struct am_state *state)
>  static void am_state_release(struct am_state *state)
>  {
>  	strbuf_release(&state->dir);
> +	strbuf_release(&state->author_name);
> +	strbuf_release(&state->author_email);
> +	strbuf_release(&state->author_date);
> +	strbuf_release(&state->msg);
>  }
>  
>  /**
> @@ -93,6 +124,95 @@ static int read_state_file(struct strbuf *sb, const char *file, size_t hint, int
>  }
>  
>  /**
> + * Parses the "author script" `filename`, and sets state->author_name,
> + * state->author_email and state->author_date accordingly. We are strict with
> + * our parsing, as the author script is supposed to be eval'd, and loosely
> + * parsing it may not give the results the user expects.
> + *
> + * The author script is of the format:
> + *
> + * 	GIT_AUTHOR_NAME='$author_name'

It seems that you have SP * SP TAB GIT_AUTHOR_... here; lose SP
before the TAB?

> +	if (!skip_prefix(sb.buf, "GIT_AUTHOR_NAME=", (const char**) &value))

Style:

	if (!skip_prefix(sb.buf, "GIT_AUTHOR_NAME=", (const char **)&value))

[PATCH/WIP v2 08/19] am: apply patch with git-apply

[Paul Tan]

Implement applying the patch to the index using git-apply.

Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 builtin/am.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 54 insertions(+), 1 deletion(-)

diff --git a/builtin/am.c b/builtin/am.c
index a1db474..b725a74 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -27,6 +27,18 @@ static int is_empty_file(const char *filename)
 	return !st.st_size;
 }
 
+/**
+ * Returns the first line of msg
+ */
+static const char *firstline(const char *msg)
+{
+	static struct strbuf sb = STRBUF_INIT;
+
+	strbuf_reset(&sb);
+	strbuf_add(&sb, msg, strchrnul(msg, '\n') - msg);
+	return sb.buf;
+}
+
 enum patch_format {
 	PATCH_FORMAT_UNKNOWN = 0,
 	PATCH_FORMAT_MBOX
@@ -512,6 +524,29 @@ static int parse_patch(struct am_state *state, const char *patch)
 	return 0;
 }
 
+/*
+ * Applies current patch with git-apply. Returns 0 on success, -1 otherwise.
+ */
+static int run_apply(const struct am_state *state)
+{
+	struct child_process cp = CHILD_PROCESS_INIT;
+
+	cp.git_cmd = 1;
+
+	argv_array_push(&cp.args, "apply");
+	argv_array_push(&cp.args, "--index");
+	argv_array_push(&cp.args, am_path(state, "patch"));
+
+	if (run_command(&cp))
+		return -1;
+
+	/* Reload index as git-apply will have modified it. */
+	discard_cache();
+	read_cache();
+
+	return 0;
+}
+
 /**
  * Applies all queued patches.
  */
@@ -529,7 +564,25 @@ static void am_run(struct am_state *state)
 		write_author_script(state);
 		write_file(am_path(state, "final-commit"), 1, "%s", state->msg.buf);
 
-		/* TODO: Patch application not implemented yet */
+		printf_ln(_("Applying: %s"), firstline(state->msg.buf));
+
+		if (run_apply(state) < 0) {
+			int value;
+
+			printf_ln(_("Patch failed at %s %s"), msgnum(state),
+					firstline(state->msg.buf));
+
+			if (!git_config_get_bool("advice.amworkdir", &value) && !value)
+				printf_ln(_("The copy of the patch that failed is found in: %s"),
+						am_path(state, "patch"));
+
+			exit(128);
+		}
+
+		/*
+		 * TODO: After the patch has been applied to the index with
+		 * git-apply, we need to make commit as well.
+		 */
 
 next:
 		am_next(state);
-- 
2.1.4

[PATCH/WIP v2 09/19] am: commit applied patch

[Paul Tan]

Implement do_commit(), which commits the index which contains the
results of applying the patch, along with the extracted commit message
and authorship information.

Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 builtin/am.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 46 insertions(+), 4 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index b725a74..ecc6d29 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -10,6 +10,9 @@
 #include "dir.h"
 #include "run-command.h"
 #include "quote.h"
+#include "cache-tree.h"
+#include "refs.h"
+#include "commit.h"
 
 /**
  * Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -548,6 +551,48 @@ static int run_apply(const struct am_state *state)
 }
 
 /**
+ * Commits the current index with state->msg as the commit message and
+ * state->author_name, state->author_email and state->author_date as the author
+ * information.
+ */
+static void do_commit(const struct am_state *state)
+{
+	unsigned char tree[20], parent[20], commit[20];
+	unsigned char *ptr;
+	struct commit_list *parents = NULL;
+	const char *reflog_msg, *author;
+	struct strbuf sb = STRBUF_INIT;
+
+	if (write_cache_as_tree(tree, 0, NULL))
+		die(_("git write-tree failed to write a tree"));
+
+	if (!get_sha1_commit("HEAD", parent)) {
+		ptr = parent;
+		commit_list_insert(lookup_commit(parent), &parents);
+	} else {
+		ptr = NULL;
+		fprintf_ln(stderr, _("applying to an empty history"));
+	}
+
+	author = fmt_ident(state->author_name.buf, state->author_email.buf,
+			state->author_date.buf, IDENT_STRICT);
+
+	if (commit_tree(state->msg.buf, state->msg.len, tree, parents, commit,
+				author, NULL))
+		die(_("failed to write commit object"));
+
+	reflog_msg = getenv("GIT_REFLOG_ACTION");
+	if (!reflog_msg)
+		reflog_msg = "am";
+
+	strbuf_addf(&sb, "%s: %s", reflog_msg, firstline(state->msg.buf));
+
+	update_ref(sb.buf, "HEAD", commit, ptr, 0, UPDATE_REFS_DIE_ON_ERR);
+
+	strbuf_release(&sb);
+}
+
+/**
  * Applies all queued patches.
  */
 static void am_run(struct am_state *state)
@@ -579,10 +624,7 @@ static void am_run(struct am_state *state)
 			exit(128);
 		}
 
-		/*
-		 * TODO: After the patch has been applied to the index with
-		 * git-apply, we need to make commit as well.
-		 */
+		do_commit(state);
 
 next:
 		am_next(state);
-- 
2.1.4

[PATCH/WIP v2 10/19] am: refresh the index at start

[Paul Tan]

If a file is unchanged but stat-dirty, git-apply may erroneously fail to
apply patches, thinking that they conflict with a dirty working tree.

As such, since 2a6f08a (am: refresh the index at start and --resolved,
2011-08-15), git-am will refresh the index before applying patches.
Re-implement this behavior.

Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 builtin/am.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/builtin/am.c b/builtin/am.c
index ecc6d29..417bfde 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -13,6 +13,7 @@
 #include "cache-tree.h"
 #include "refs.h"
 #include "commit.h"
+#include "lockfile.h"
 
 /**
  * Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -457,6 +458,20 @@ static const char *msgnum(const struct am_state *state)
 }
 
 /**
+ * Refresh and write index.
+ */
+static void refresh_and_write_cache(void)
+{
+	static struct lock_file lock_file;
+
+	hold_locked_index(&lock_file, 1);
+	refresh_cache(REFRESH_QUIET);
+	if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
+		die(_("unable to write index file"));
+	rollback_lock_file(&lock_file);
+}
+
+/**
  * Parses `patch` using git-mailinfo. state->msg will be set to the patch
  * message. state->author_name, state->author_email, state->author_date will be
  * set to the patch author's name, email and date respectively. The patch's
@@ -597,6 +612,8 @@ static void do_commit(const struct am_state *state)
  */
 static void am_run(struct am_state *state)
 {
+	refresh_and_write_cache();
+
 	while (state->cur <= state->last) {
 		const char *patch = am_path(state, msgnum(state));
 
@@ -678,6 +695,9 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, prefix, am_options, am_usage, 0);
 
+	if (read_index_preload(&the_index, NULL) < 0)
+		die(_("failed to read the index"));
+
 	if (am_in_progress(&state))
 		am_load(&state);
 	else {
-- 
2.1.4

[PATCH/WIP v2 11/19] am: refuse to apply patches if index is dirty

[Paul Tan]

Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07), git-am
will refuse to apply patches if the index is dirty. Re-implement this
behavior.

Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 builtin/am.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/builtin/am.c b/builtin/am.c
index 417bfde..234762c 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -14,6 +14,8 @@
 #include "refs.h"
 #include "commit.h"
 #include "lockfile.h"
+#include "diff.h"
+#include "diffcore.h"
 
 /**
  * Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -472,6 +474,43 @@ static void refresh_and_write_cache(void)
 }
 
 /**
+ * Returns 1 if the index differs from HEAD, 0 otherwise. When on an unborn
+ * branch, returns 1 if there are entries in the index, 0 otherwise. If an
+ * strbuf is provided, the space-separated list of files that differ will be
+ * appended to it.
+ */
+static int index_has_changes(struct strbuf *sb)
+{
+	unsigned char head[GIT_SHA1_RAWSZ];
+	int i;
+
+	if (!get_sha1_tree("HEAD", head)) {
+		struct diff_options opt;
+
+		diff_setup(&opt);
+		DIFF_OPT_SET(&opt, EXIT_WITH_STATUS);
+		if (!sb)
+			DIFF_OPT_SET(&opt, QUICK);
+		do_diff_cache(head, &opt);
+		diffcore_std(&opt);
+		for (i = 0; sb && i < diff_queued_diff.nr; i++) {
+			if (i)
+				strbuf_addch(sb, ' ');
+			strbuf_addstr(sb, diff_queued_diff.queue[i]->two->path);
+		}
+		diff_flush(&opt);
+		return DIFF_OPT_TST(&opt, HAS_CHANGES) != 0;
+	} else {
+		for (i = 0; sb && i < active_nr; i++) {
+			if (i)
+				strbuf_addch(sb, ' ');
+			strbuf_addstr(sb, active_cache[i]->name);
+		}
+		return !!active_nr;
+	}
+}
+
+/**
  * Parses `patch` using git-mailinfo. state->msg will be set to the patch
  * message. state->author_name, state->author_email, state->author_date will be
  * set to the patch author's name, email and date respectively. The patch's
@@ -612,8 +651,15 @@ static void do_commit(const struct am_state *state)
  */
 static void am_run(struct am_state *state)
 {
+	struct strbuf sb = STRBUF_INIT;
+
 	refresh_and_write_cache();
 
+	if (index_has_changes(&sb))
+		die(_("Dirty index: cannot apply patches (dirty: %s)"), sb.buf);
+
+	strbuf_release(&sb);
+
 	while (state->cur <= state->last) {
 		const char *patch = am_path(state, msgnum(state));
 
-- 
2.1.4

[PATCH/WIP v2 12/19] am: implement --resolved/--continue

[Paul Tan]

Since 0c15cc9 (git-am: --resolved., 2005-11-16), git-am supported
resuming from a failed patch application. The user will manually apply
the patch, and the run git am --resolved which will then commit the
resulting index. Re-implement this feature by introducing am_resolve().

Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 builtin/am.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/builtin/am.c b/builtin/am.c
index 234762c..935ffcb 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -697,6 +697,34 @@ next:
 }
 
 /**
+ * Resume the current am session after patch application failure. The user did
+ * all the hard work, and we do not have to do any patch application. Just
+ * trust and commit what the user has in the index and working tree.
+ */
+static void am_resolve(struct am_state *state)
+{
+	printf_ln(_("Applying: %s"), firstline(state->msg.buf));
+
+	if (!index_has_changes(NULL)) {
+		printf_ln(_("No changes - did you forget to use 'git add'?\n"
+			"If there is nothing left to stage, chances are that something else\n"
+			"already introduced the same changes; you might want to skip this patch."));
+		exit(128);
+	}
+
+	if (unmerged_cache()) {
+		printf_ln(_("You still have unmerged paths in your index.\n"
+			"Did you forget to use 'git add'?"));
+		exit(128);
+	}
+
+	do_commit(state);
+
+	am_next(state);
+	am_run(state);
+}
+
+/**
  * parse_options() callback that validates and sets opt->value to the
  * PATCH_FORMAT_* enum value corresponding to `arg`.
  */
@@ -711,17 +739,30 @@ static int parse_opt_patchformat(const struct option *opt, const char *arg, int
 	return 0;
 }
 
+enum resume_mode {
+	RESUME_FALSE = 0,
+	RESUME_RESOLVED
+};
+
 static struct am_state state;
 static int opt_patch_format;
+static enum resume_mode opt_resume;
 
 static const char * const am_usage[] = {
 	N_("git am [options] [(<mbox>|<Maildir>)...]"),
+	N_("git am [options] --continue"),
 	NULL
 };
 
 static struct option am_options[] = {
 	OPT_CALLBACK(0, "patch-format", &opt_patch_format, N_("format"),
 		N_("format the patch(es) are in"), parse_opt_patchformat),
+	OPT_CMDMODE(0, "continue", &opt_resume,
+		N_("continue applying patches after resolving a conflict"),
+		RESUME_RESOLVED),
+	OPT_CMDMODE('r', "resolved", &opt_resume,
+		N_("synonyms for --continue"),
+		RESUME_RESOLVED),
 	OPT_END()
 };
 
@@ -762,7 +803,16 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 		string_list_clear(&paths, 0);
 	}
 
-	am_run(&state);
+	switch (opt_resume) {
+	case RESUME_FALSE:
+		am_run(&state);
+		break;
+	case RESUME_RESOLVED:
+		am_resolve(&state);
+		break;
+	default:
+		die("BUG: invalid resume value");
+	}
 
 	am_state_release(&state);
 
-- 
2.1.4

[PATCH/WIP v2 13/19] am: implement --skip

[Paul Tan]

Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07), git-am
supported resuming from a failed patch application by skipping the
current patch. Re-implement this feature by introducing am_skip().

Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 builtin/am.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 119 insertions(+), 2 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 935ffcb..1434e68 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -16,6 +16,8 @@
 #include "lockfile.h"
 #include "diff.h"
 #include "diffcore.h"
+#include "unpack-trees.h"
+#include "branch.h"
 
 /**
  * Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -725,6 +727,114 @@ static void am_resolve(struct am_state *state)
 }
 
 /**
+ * Performs a checkout fast-forward from `head` to `remote`. If `reset` is
+ * true, any unmerged entries will be discarded. Returns 0 on success, -1 on
+ * failure.
+ */
+static int fast_forward_to(struct tree *head, struct tree *remote, int reset)
+{
+	struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
+	struct unpack_trees_options opts;
+	struct tree_desc t[2];
+
+	if (parse_tree(head) || parse_tree(remote))
+		return -1;
+
+	hold_locked_index(lock_file, 1);
+
+	refresh_cache(REFRESH_QUIET);
+
+	memset(&opts, 0, sizeof(opts));
+	opts.head_idx = 1;
+	opts.src_index = &the_index;
+	opts.dst_index = &the_index;
+	opts.update = 1;
+	opts.merge = 1;
+	opts.reset = reset;
+	opts.fn = twoway_merge;
+	init_tree_desc(&t[0], head->buffer, head->size);
+	init_tree_desc(&t[1], remote->buffer, remote->size);
+
+	if (unpack_trees(2, t, &opts)) {
+		rollback_lock_file(lock_file);
+		return -1;
+	}
+
+	if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
+		die(_("unable to write new index file"));
+
+	return 0;
+}
+
+/**
+ * Clean the index without touching entries that are not modified between
+ * `head` and `remote`.
+ */
+static int clean_index(const unsigned char *head, const unsigned char *remote)
+{
+	struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
+	struct tree *head_tree, *remote_tree, *index_tree;
+	unsigned char index[GIT_SHA1_RAWSZ];
+	struct pathspec pathspec;
+
+	head_tree = parse_tree_indirect(head);
+	if (!head_tree)
+		return error(_("Could not parse object '%s'."), sha1_to_hex(head));
+
+	remote_tree = parse_tree_indirect(remote);
+	if (!remote_tree)
+		return error(_("Could not parse object '%s'."), sha1_to_hex(remote));
+
+	read_cache_unmerged();
+
+	if (fast_forward_to(head_tree, head_tree, 1))
+		return -1;
+
+	if (write_cache_as_tree(index, 0, NULL))
+		return -1;
+
+	index_tree = parse_tree_indirect(index);
+	if (!index_tree)
+		return error(_("Could not parse object '%s'."), sha1_to_hex(index));
+
+	if (fast_forward_to(index_tree, remote_tree, 0))
+		return -1;
+
+	memset(&pathspec, 0, sizeof(pathspec));
+
+	hold_locked_index(lock_file, 1);
+
+	if (read_tree(remote_tree, 0, &pathspec)) {
+		rollback_lock_file(lock_file);
+		return -1;
+	}
+
+	if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
+		die(_("unable to write new index file"));
+
+	remove_branch_state();
+
+	return 0;
+}
+
+/**
+ * Resume the current am session by skipping the current patch.
+ */
+static void am_skip(struct am_state *state)
+{
+	unsigned char head[GIT_SHA1_RAWSZ];
+
+	if (get_sha1("HEAD", head))
+		hashcpy(head, EMPTY_TREE_SHA1_BIN);
+
+	if (clean_index(head, head))
+		die(_("failed to clean index"));
+
+	am_next(state);
+	am_run(state);
+}
+
+/**
  * parse_options() callback that validates and sets opt->value to the
  * PATCH_FORMAT_* enum value corresponding to `arg`.
  */
@@ -741,7 +851,8 @@ static int parse_opt_patchformat(const struct option *opt, const char *arg, int
 
 enum resume_mode {
 	RESUME_FALSE = 0,
-	RESUME_RESOLVED
+	RESUME_RESOLVED,
+	RESUME_SKIP
 };
 
 static struct am_state state;
@@ -750,7 +861,7 @@ static enum resume_mode opt_resume;
 
 static const char * const am_usage[] = {
 	N_("git am [options] [(<mbox>|<Maildir>)...]"),
-	N_("git am [options] --continue"),
+	N_("git am [options] (--continue | --skip)"),
 	NULL
 };
 
@@ -763,6 +874,9 @@ static struct option am_options[] = {
 	OPT_CMDMODE('r', "resolved", &opt_resume,
 		N_("synonyms for --continue"),
 		RESUME_RESOLVED),
+	OPT_CMDMODE(0, "skip", &opt_resume,
+		N_("skip the current patch"),
+		RESUME_SKIP),
 	OPT_END()
 };
 
@@ -810,6 +924,9 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 	case RESUME_RESOLVED:
 		am_resolve(&state);
 		break;
+	case RESUME_SKIP:
+		am_skip(&state);
+		break;
 	default:
 		die("BUG: invalid resume value");
 	}
-- 
2.1.4

[PATCH/WIP v2 14/19] am: implement --abort

[Paul Tan]

Since 3e5057a (git am --abort, 2008-07-16), git-am supported the --abort
option that will rewind HEAD back to the original commit. Re-implement
this feature through am_abort().

Since 7b3b7e3 (am --abort: keep unrelated commits since the last failure
and warn, 2010-12-21), to prevent commits made since the last failure
from being lost, git-am will not rewind HEAD back to the original
commit if HEAD moved since the last failure. Re-implement this through
safe_to_abort().

Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 builtin/am.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 92 insertions(+), 3 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 1434e68..cdc07ab 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -409,6 +409,8 @@ static int split_patches(struct am_state *state, enum patch_format patch_format,
 static void am_setup(struct am_state *state, enum patch_format patch_format,
 		struct string_list *paths)
 {
+	unsigned char curr_head[20];
+
 	if (!patch_format)
 		patch_format = detect_patch_format(paths);
 
@@ -428,6 +430,14 @@ static void am_setup(struct am_state *state, enum patch_format patch_format,
 	write_file(am_path(state, "next"), 1, "%d", state->cur);
 
 	write_file(am_path(state, "last"), 1, "%d", state->last);
+
+	if (!get_sha1("HEAD", curr_head)) {
+		write_file(am_path(state, "abort-safety"), 1, "%s", sha1_to_hex(curr_head));
+		update_ref("am", "ORIG_HEAD", curr_head, NULL, 0, UPDATE_REFS_DIE_ON_ERR);
+	} else {
+		write_file(am_path(state, "abort-safety"), 1, "%s", "");
+		delete_ref("ORIG_HEAD", NULL, 0);
+	}
 }
 
 /**
@@ -436,6 +446,8 @@ static void am_setup(struct am_state *state, enum patch_format patch_format,
  */
 static void am_next(struct am_state *state)
 {
+	unsigned char head[GIT_SHA1_RAWSZ];
+
 	state->cur++;
 	write_file(am_path(state, "next"), 1, "%d", state->cur);
 
@@ -446,6 +458,11 @@ static void am_next(struct am_state *state)
 
 	strbuf_reset(&state->msg);
 	unlink(am_path(state, "final-commit"));
+
+	if (!get_sha1("HEAD", head))
+		write_file(am_path(state, "abort-safety"), 1, "%s", sha1_to_hex(head));
+	else
+		write_file(am_path(state, "abort-safety"), 1, "%s", "");
 }
 
 /**
@@ -655,10 +672,14 @@ static void am_run(struct am_state *state)
 {
 	struct strbuf sb = STRBUF_INIT;
 
+	unlink(am_path(state, "dirtyindex"));
+
 	refresh_and_write_cache();
 
-	if (index_has_changes(&sb))
+	if (index_has_changes(&sb)) {
+		write_file(am_path(state, "dirtyindex"), 1, "t");
 		die(_("Dirty index: cannot apply patches (dirty: %s)"), sb.buf);
+	}
 
 	strbuf_release(&sb);
 
@@ -834,6 +855,67 @@ static void am_skip(struct am_state *state)
 	am_run(state);
 }
 
+static int safe_to_abort(const struct am_state *state)
+{
+	struct strbuf sb = STRBUF_INIT;
+	unsigned char abort_safety[20], head[20];
+
+	if (file_exists(am_path(state, "dirtyindex")))
+		return 0;
+
+	if (read_state_file(&sb, am_path(state, "abort-safety"), 40, 1) > 0) {
+		if (get_sha1_hex(sb.buf, abort_safety))
+			die(_("could not parse %s"), am_path(state, "abort_safety"));
+	} else
+		hashclr(abort_safety);
+
+	if (get_sha1("HEAD", head))
+		hashclr(head);
+
+	if (!hashcmp(head, abort_safety))
+		return 1;
+
+	error(_("You seem to have moved HEAD since the last 'am' failure.\n"
+		"Not rewinding to ORIG_HEAD"));
+
+	return 0;
+}
+
+/**
+ * Aborts the current am session if it is safe to do so.
+ */
+static void am_abort(struct am_state *state)
+{
+	unsigned char curr_head[20], orig_head[20];
+	int has_curr_head, has_orig_head;
+	const char *curr_branch;
+
+	if (!safe_to_abort(state)) {
+		am_destroy(state);
+		return;
+	}
+
+	curr_branch = resolve_refdup("HEAD", 0, curr_head, NULL);
+	has_curr_head = !is_null_sha1(curr_head);
+	if (!has_curr_head)
+		hashcpy(curr_head, EMPTY_TREE_SHA1_BIN);
+
+	has_orig_head = !get_sha1("ORIG_HEAD", orig_head);
+	if (!has_orig_head)
+		hashcpy(orig_head, EMPTY_TREE_SHA1_BIN);
+
+	clean_index(curr_head, orig_head);
+
+	if (has_orig_head)
+		update_ref("am --abort", "HEAD", orig_head,
+				has_curr_head ? curr_head : NULL, 0,
+				UPDATE_REFS_DIE_ON_ERR);
+	else if (curr_branch)
+		delete_ref(curr_branch, NULL, REF_NODEREF);
+
+	am_destroy(state);
+}
+
 /**
  * parse_options() callback that validates and sets opt->value to the
  * PATCH_FORMAT_* enum value corresponding to `arg`.
@@ -852,7 +934,8 @@ static int parse_opt_patchformat(const struct option *opt, const char *arg, int
 enum resume_mode {
 	RESUME_FALSE = 0,
 	RESUME_RESOLVED,
-	RESUME_SKIP
+	RESUME_SKIP,
+	RESUME_ABORT
 };
 
 static struct am_state state;
@@ -861,7 +944,7 @@ static enum resume_mode opt_resume;
 
 static const char * const am_usage[] = {
 	N_("git am [options] [(<mbox>|<Maildir>)...]"),
-	N_("git am [options] (--continue | --skip)"),
+	N_("git am [options] (--continue | --skip | --abort)"),
 	NULL
 };
 
@@ -877,6 +960,9 @@ static struct option am_options[] = {
 	OPT_CMDMODE(0, "skip", &opt_resume,
 		N_("skip the current patch"),
 		RESUME_SKIP),
+	OPT_CMDMODE(0, "abort", &opt_resume,
+		N_("restore the original branch and abort the patching operation."),
+		RESUME_ABORT),
 	OPT_END()
 };
 
@@ -927,6 +1013,9 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 	case RESUME_SKIP:
 		am_skip(&state);
 		break;
+	case RESUME_ABORT:
+		am_abort(&state);
+		break;
 	default:
 		die("BUG: invalid resume value");
 	}
-- 
2.1.4

[PATCH/WIP v2 15/19] am: implement quiet option

[Paul Tan]

Since 0e987a1 (am, rebase: teach quiet option, 2009-06-16), git-am
supported the --quiet option and GIT_QUIET environment variable, and
when told to be quiet, would only speak on failure. Re-implement this by
introducing the say() function, which works like fprintf_ln(), but would
only write to the stream when state->quiet is false.

Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 builtin/am.c | 36 +++++++++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index cdc07ab..795b672 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -68,6 +68,8 @@ struct am_state {
 
 	/* number of digits in patch filename */
 	int prec;
+
+	int quiet;
 };
 
 /**
@@ -75,6 +77,8 @@ struct am_state {
  */
 static void am_state_init(struct am_state *state)
 {
+	const char *quiet;
+
 	memset(state, 0, sizeof(*state));
 
 	strbuf_init(&state->dir, 0);
@@ -83,6 +87,10 @@ static void am_state_init(struct am_state *state)
 	strbuf_init(&state->author_date, 0);
 	strbuf_init(&state->msg, 0);
 	state->prec = 4;
+
+	quiet = getenv("GIT_QUIET");
+	if (quiet && *quiet)
+		state->quiet = 1;
 }
 
 /**
@@ -106,6 +114,22 @@ static inline const char *am_path(const struct am_state *state, const char *path
 }
 
 /**
+ * If state->quiet is false, calls fprintf(fp, fmt, ...), and appends a newline
+ * at the end.
+ */
+static void say(const struct am_state *state, FILE *fp, const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap, fmt);
+	if (!state->quiet) {
+		vfprintf(fp, fmt, ap);
+		putc('\n', fp);
+	}
+	va_end(ap);
+}
+
+/**
  * Returns 1 if there is an am session in progress, 0 otherwise.
  */
 static int am_in_progress(const struct am_state *state)
@@ -250,6 +274,9 @@ static void am_load(struct am_state *state)
 
 	read_state_file(&state->msg, am_path(state, "final-commit"), 0, 0);
 
+	read_state_file(&sb, am_path(state, "quiet"), 2, 1);
+	state->quiet = !strcmp(sb.buf, "t");
+
 	strbuf_release(&sb);
 }
 
@@ -431,6 +458,8 @@ static void am_setup(struct am_state *state, enum patch_format patch_format,
 
 	write_file(am_path(state, "last"), 1, "%d", state->last);
 
+	write_file(am_path(state, "quiet"), 1, state->quiet ? "t" : "f");
+
 	if (!get_sha1("HEAD", curr_head)) {
 		write_file(am_path(state, "abort-safety"), 1, "%s", sha1_to_hex(curr_head));
 		update_ref("am", "ORIG_HEAD", curr_head, NULL, 0, UPDATE_REFS_DIE_ON_ERR);
@@ -644,7 +673,7 @@ static void do_commit(const struct am_state *state)
 		commit_list_insert(lookup_commit(parent), &parents);
 	} else {
 		ptr = NULL;
-		fprintf_ln(stderr, _("applying to an empty history"));
+		say(state, stderr, _("applying to an empty history"));
 	}
 
 	author = fmt_ident(state->author_name.buf, state->author_email.buf,
@@ -695,7 +724,7 @@ static void am_run(struct am_state *state)
 		write_author_script(state);
 		write_file(am_path(state, "final-commit"), 1, "%s", state->msg.buf);
 
-		printf_ln(_("Applying: %s"), firstline(state->msg.buf));
+		say(state, stdout, _("Applying: %s"), firstline(state->msg.buf));
 
 		if (run_apply(state) < 0) {
 			int value;
@@ -726,7 +755,7 @@ next:
  */
 static void am_resolve(struct am_state *state)
 {
-	printf_ln(_("Applying: %s"), firstline(state->msg.buf));
+	say(state, stdout, _("Applying: %s"), firstline(state->msg.buf));
 
 	if (!index_has_changes(NULL)) {
 		printf_ln(_("No changes - did you forget to use 'git add'?\n"
@@ -949,6 +978,7 @@ static const char * const am_usage[] = {
 };
 
 static struct option am_options[] = {
+	OPT__QUIET(&state.quiet, N_("be quiet")),
 	OPT_CALLBACK(0, "patch-format", &opt_patch_format, N_("format"),
 		N_("format the patch(es) are in"), parse_opt_patchformat),
 	OPT_CMDMODE(0, "continue", &opt_resume,
-- 
2.1.4

[PATCH/WIP v2 16/19] am: exit with user friendly message on patch failure

[Paul Tan]

Since ced9456 (Give the user a hint for how to continue in the case that
git-am fails because it requires user intervention, 2006-05-02), git-am
prints additional information on how the user can re-invoke git-am to
resume patch application after resolving the failure. Re-implement this
through the die_user_resolve() function.

Since cc12005 (Make git rebase interactive help match documentation.,
2006-05-13), git-am supports the --resolvemsg option which is used by
git-rebase to override the message printed out when git-am fails.
Re-implement this option.

Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 builtin/am.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 795b672..4cd21b8 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -70,6 +70,9 @@ struct am_state {
 	int prec;
 
 	int quiet;
+
+	/* override error message when patch failure occurs */
+	const char *resolvemsg;
 };
 
 /**
@@ -629,6 +632,21 @@ static int parse_patch(struct am_state *state, const char *patch)
 	return 0;
 }
 
+/**
+ * Dies with a user-friendly message on how to proceed after resolving the
+ * problem. This message can be overridden with state->resolvemsg.
+ */
+static void NORETURN die_user_resolve(const struct am_state *state)
+{
+	if (state->resolvemsg)
+		printf_ln("%s", state->resolvemsg);
+	else
+		printf_ln(_("When you have resolved this problem, run \"git am --continue\".\n"
+			"If you prefer to skip this patch, run \"git am --skip\" instead.\n"
+			"To restore the original branch and stop patching, run \"git am --abort\"."));
+	exit(128);
+}
+
 /*
  * Applies current patch with git-apply. Returns 0 on success, -1 otherwise.
  */
@@ -736,7 +754,7 @@ static void am_run(struct am_state *state)
 				printf_ln(_("The copy of the patch that failed is found in: %s"),
 						am_path(state, "patch"));
 
-			exit(128);
+			die_user_resolve(state);
 		}
 
 		do_commit(state);
@@ -761,13 +779,13 @@ static void am_resolve(struct am_state *state)
 		printf_ln(_("No changes - did you forget to use 'git add'?\n"
 			"If there is nothing left to stage, chances are that something else\n"
 			"already introduced the same changes; you might want to skip this patch."));
-		exit(128);
+		die_user_resolve(state);
 	}
 
 	if (unmerged_cache()) {
 		printf_ln(_("You still have unmerged paths in your index.\n"
 			"Did you forget to use 'git add'?"));
-		exit(128);
+		die_user_resolve(state);
 	}
 
 	do_commit(state);
@@ -981,6 +999,8 @@ static struct option am_options[] = {
 	OPT__QUIET(&state.quiet, N_("be quiet")),
 	OPT_CALLBACK(0, "patch-format", &opt_patch_format, N_("format"),
 		N_("format the patch(es) are in"), parse_opt_patchformat),
+	OPT_STRING(0, "resolvemsg", &state.resolvemsg, NULL,
+		N_("override error message when patch failure occurs")),
 	OPT_CMDMODE(0, "continue", &opt_resume,
 		N_("continue applying patches after resolving a conflict"),
 		RESUME_RESOLVED),
-- 
2.1.4

[PATCH/WIP v2 17/19] am: implement am --signoff

[Paul Tan]

Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07), git-am
supported the --signoff option which will append a signoff at the end of
the commit messsage. Re-implement this feature by calling
append_signoff() if the option is set.

Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 builtin/am.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/builtin/am.c b/builtin/am.c
index 4cd21b8..71fda16 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -18,6 +18,7 @@
 #include "diffcore.h"
 #include "unpack-trees.h"
 #include "branch.h"
+#include "sequencer.h"
 
 /**
  * Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -73,6 +74,8 @@ struct am_state {
 
 	/* override error message when patch failure occurs */
 	const char *resolvemsg;
+
+	int sign;
 };
 
 /**
@@ -280,6 +283,9 @@ static void am_load(struct am_state *state)
 	read_state_file(&sb, am_path(state, "quiet"), 2, 1);
 	state->quiet = !strcmp(sb.buf, "t");
 
+	read_state_file(&sb, am_path(state, "sign"), 2, 1);
+	state->sign = !strcmp(sb.buf, "t");
+
 	strbuf_release(&sb);
 }
 
@@ -463,6 +469,8 @@ static void am_setup(struct am_state *state, enum patch_format patch_format,
 
 	write_file(am_path(state, "quiet"), 1, state->quiet ? "t" : "f");
 
+	write_file(am_path(state, "sign"), 1, state->sign ? "t" : "f");
+
 	if (!get_sha1("HEAD", curr_head)) {
 		write_file(am_path(state, "abort-safety"), 1, "%s", sha1_to_hex(curr_head));
 		update_ref("am", "ORIG_HEAD", curr_head, NULL, 0, UPDATE_REFS_DIE_ON_ERR);
@@ -629,6 +637,9 @@ static int parse_patch(struct am_state *state, const char *patch)
 		die_errno(_("could not read '%s'"), am_path(state, "msg"));
 	stripspace(&state->msg, 0);
 
+	if (state->sign)
+		append_signoff(&state->msg, 0, 0);
+
 	return 0;
 }
 
@@ -997,6 +1008,8 @@ static const char * const am_usage[] = {
 
 static struct option am_options[] = {
 	OPT__QUIET(&state.quiet, N_("be quiet")),
+	OPT_BOOL('s', "signoff", &state.sign,
+		N_("add a Signed-off-by line to the commit message")),
 	OPT_CALLBACK(0, "patch-format", &opt_patch_format, N_("format"),
 		N_("format the patch(es) are in"), parse_opt_patchformat),
 	OPT_STRING(0, "resolvemsg", &state.resolvemsg, NULL,
-- 
2.1.4

[PATCH/WIP v2 18/19] cache-tree: introduce write_index_as_tree()

[Paul Tan]

A caller may wish to write a temporary index as a tree. However,
write_cache_as_tree() assumes that the index was read from, and will
write to, the default index file path. Introduce write_index_as_tree()
which removes this limitation by allowing the caller to specify its own
index_state and index file path.

Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 cache-tree.c | 29 +++++++++++++++++------------
 cache-tree.h |  1 +
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/cache-tree.c b/cache-tree.c
index 32772b9..feace8b 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -592,7 +592,7 @@ static struct cache_tree *cache_tree_find(struct cache_tree *it, const char *pat
 	return it;
 }
 
-int write_cache_as_tree(unsigned char *sha1, int flags, const char *prefix)
+int write_index_as_tree(unsigned char *sha1, struct index_state *index_state, const char *index_path, int flags, const char *prefix)
 {
 	int entries, was_valid, newfd;
 	struct lock_file *lock_file;
@@ -603,23 +603,23 @@ int write_cache_as_tree(unsigned char *sha1, int flags, const char *prefix)
 	 */
 	lock_file = xcalloc(1, sizeof(struct lock_file));
 
-	newfd = hold_locked_index(lock_file, 1);
+	newfd = hold_lock_file_for_update(lock_file, index_path, LOCK_DIE_ON_ERROR);
 
-	entries = read_cache();
+	entries = read_index_from(index_state, index_path);
 	if (entries < 0)
 		return WRITE_TREE_UNREADABLE_INDEX;
 	if (flags & WRITE_TREE_IGNORE_CACHE_TREE)
-		cache_tree_free(&(active_cache_tree));
+		cache_tree_free(&index_state->cache_tree);
 
-	if (!active_cache_tree)
-		active_cache_tree = cache_tree();
+	if (!index_state->cache_tree)
+		index_state->cache_tree = cache_tree();
 
-	was_valid = cache_tree_fully_valid(active_cache_tree);
+	was_valid = cache_tree_fully_valid(index_state->cache_tree);
 	if (!was_valid) {
-		if (cache_tree_update(&the_index, flags) < 0)
+		if (cache_tree_update(index_state, flags) < 0)
 			return WRITE_TREE_UNMERGED_INDEX;
 		if (0 <= newfd) {
-			if (!write_locked_index(&the_index, lock_file, COMMIT_LOCK))
+			if (!write_locked_index(index_state, lock_file, COMMIT_LOCK))
 				newfd = -1;
 		}
 		/* Not being able to write is fine -- we are only interested
@@ -631,14 +631,14 @@ int write_cache_as_tree(unsigned char *sha1, int flags, const char *prefix)
 	}
 
 	if (prefix) {
-		struct cache_tree *subtree =
-			cache_tree_find(active_cache_tree, prefix);
+		struct cache_tree *subtree;
+		subtree = cache_tree_find(index_state->cache_tree, prefix);
 		if (!subtree)
 			return WRITE_TREE_PREFIX_ERROR;
 		hashcpy(sha1, subtree->sha1);
 	}
 	else
-		hashcpy(sha1, active_cache_tree->sha1);
+		hashcpy(sha1, index_state->cache_tree->sha1);
 
 	if (0 <= newfd)
 		rollback_lock_file(lock_file);
@@ -646,6 +646,11 @@ int write_cache_as_tree(unsigned char *sha1, int flags, const char *prefix)
 	return 0;
 }
 
+int write_cache_as_tree(unsigned char *sha1, int flags, const char *prefix)
+{
+	return write_index_as_tree(sha1, &the_index, get_index_file(), flags, prefix);
+}
+
 static void prime_cache_tree_rec(struct cache_tree *it, struct tree *tree)
 {
 	struct tree_desc desc;
diff --git a/cache-tree.h b/cache-tree.h
index aa7b3e4..41c5746 100644
--- a/cache-tree.h
+++ b/cache-tree.h
@@ -46,6 +46,7 @@ int update_main_cache_tree(int);
 #define WRITE_TREE_UNMERGED_INDEX (-2)
 #define WRITE_TREE_PREFIX_ERROR (-3)
 
+int write_index_as_tree(unsigned char *sha1, struct index_state *index_state, const char *index_path, int flags, const char *prefix);
 int write_cache_as_tree(unsigned char *sha1, int flags, const char *prefix);
 void prime_cache_tree(struct index_state *, struct tree *);
 
-- 
2.1.4

[PATCH/WIP v2 19/19] am: implement 3-way merge

[Paul Tan]

Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07), git-am
supported the --3way option, and if set, would attempt to do a 3-way
merge if the initial patch application fails. Re-implement this feature
through the fall_back_threeway() function.

Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 builtin/am.c | 133 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 129 insertions(+), 4 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 71fda16..8566d22 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -19,6 +19,7 @@
 #include "unpack-trees.h"
 #include "branch.h"
 #include "sequencer.h"
+#include "merge-recursive.h"
 
 /**
  * Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -76,6 +77,8 @@ struct am_state {
 	const char *resolvemsg;
 
 	int sign;
+
+	int threeway;
 };
 
 /**
@@ -659,16 +662,32 @@ static void NORETURN die_user_resolve(const struct am_state *state)
 }
 
 /*
- * Applies current patch with git-apply. Returns 0 on success, -1 otherwise.
+ * Applies current patch with git-apply. Returns 0 on success, -1 otherwise. If
+ * index_file is not NULL, the patch will be applied to that index.
  */
-static int run_apply(const struct am_state *state)
+static int run_apply(const struct am_state *state, const char *index_file)
 {
 	struct child_process cp = CHILD_PROCESS_INIT;
 
 	cp.git_cmd = 1;
 
+	if (index_file)
+		argv_array_pushf(&cp.env_array, "GIT_INDEX_FILE=%s", index_file);
+
+	/*
+	 * If we are allowed to fall back on 3-way merge, don't give false
+	 * errors during the initial attempt.
+	 */
+	if (state->threeway && !index_file) {
+		cp.no_stdout = 1;
+		cp.no_stderr = 1;
+	}
+
 	argv_array_push(&cp.args, "apply");
-	argv_array_push(&cp.args, "--index");
+	if (index_file)
+		argv_array_push(&cp.args, "--cached");
+	else
+		argv_array_push(&cp.args, "--index");
 	argv_array_push(&cp.args, am_path(state, "patch"));
 
 	if (run_command(&cp))
@@ -676,8 +695,92 @@ static int run_apply(const struct am_state *state)
 
 	/* Reload index as git-apply will have modified it. */
 	discard_cache();
+	read_cache_from(index_file ? index_file : get_index_file());
+
+	return 0;
+}
+
+/*
+ * Builds a index that contains just the blobs needed for a 3way merge.
+ */
+static int build_fake_ancestor(const struct am_state *state, const char *index_file)
+{
+	struct child_process cp = CHILD_PROCESS_INIT;
+
+	cp.git_cmd = 1;
+	argv_array_push(&cp.args, "apply");
+	argv_array_pushf(&cp.args, "--build-fake-ancestor=%s", index_file);
+	argv_array_push(&cp.args, am_path(state, "patch"));
+
+	if (run_command(&cp))
+		return -1;
+
+	return 0;
+}
+
+/**
+ * Attempt a threeway merge, using index_path as the temporary index.
+ */
+static int fall_back_threeway(const struct am_state *state, const char *index_path)
+{
+	unsigned char orig_tree[20], his_tree[20], our_tree[20];
+	const unsigned char *bases[1] = {orig_tree};
+	struct merge_options o;
+	struct commit *result;
+
+	if (get_sha1("HEAD", our_tree) < 0)
+		hashcpy(our_tree, EMPTY_TREE_SHA1_BIN);
+
+	if (build_fake_ancestor(state, index_path))
+		return error("could not build fake ancestor");
+
+	discard_cache();
+	read_cache_from(index_path);
+
+	if (write_index_as_tree(orig_tree, &the_index, index_path, 0, NULL))
+		return error(_("Repository lacks necessary blobs to fall back on 3-way merge."));
+
+	say(state, stdout, _("Using index info to reconstruct a base tree..."));
+
+	if (!state->quiet) {
+		struct child_process cp = CHILD_PROCESS_INIT;
+		cp.git_cmd = 1;
+		argv_array_pushf(&cp.env_array, "GIT_INDEX_FILE=%s", index_path);
+		argv_array_pushl(&cp.args, "diff-index", "--cached",
+				"--diff-filter=AM", "--name-status", "HEAD", NULL);
+		run_command(&cp);
+	}
+
+	if (run_apply(state, index_path))
+		return error(_("Did you hand edit your patch?\n"
+				"It does not apply to blobs recorded in its index."));
+
+	if (write_index_as_tree(his_tree, &the_index, index_path, 0, NULL))
+		return error("could not write tree");
+
+	say(state, stdout, _("Falling back to patching base and 3-way merge..."));
+
+	discard_cache();
 	read_cache();
 
+	/*
+	 * This is not so wrong. Depending on which base we picked, orig_tree
+	 * may be wildly different from ours, but his_tree has the same set of
+	 * wildly different changes in parts the patch did not touch, so
+	 * recursive ends up canceling them, saying that we reverted all those
+	 * changes.
+	 */
+
+	init_merge_options(&o);
+
+	o.branch1 = "HEAD";
+	o.branch2 = firstline(state->msg.buf);
+	if (state->quiet)
+		o.verbosity = 0;
+
+	if (merge_recursive_generic(&o, our_tree, his_tree, 1, bases, &result))
+		return error(_("Failed to merge in the changes."));
+
 	return 0;
 }
 
@@ -743,6 +846,7 @@ static void am_run(struct am_state *state)
 
 	while (state->cur <= state->last) {
 		const char *patch = am_path(state, msgnum(state));
+		int apply_status;
 
 		if (!file_exists(patch))
 			goto next;
@@ -755,7 +859,26 @@ static void am_run(struct am_state *state)
 
 		say(state, stdout, _("Applying: %s"), firstline(state->msg.buf));
 
-		if (run_apply(state) < 0) {
+		apply_status = run_apply(state, NULL);
+
+		if (apply_status && state->threeway) {
+			struct strbuf sb = STRBUF_INIT;
+
+			strbuf_addstr(&sb, am_path(state, "patch-merge-index"));
+			apply_status = fall_back_threeway(state, sb.buf);
+			strbuf_release(&sb);
+
+			/*
+			 * Applying the patch to an earlier tree and merging
+			 * the result may have produced the same tree as ours.
+			 */
+			if (!apply_status && !index_has_changes(NULL)) {
+				say(state, stdout, _("No changes -- Patch already applied."));
+				goto next;
+			}
+		}
+
+		if (apply_status) {
 			int value;
 
 			printf_ln(_("Patch failed at %s %s"), msgnum(state),
@@ -1007,6 +1130,8 @@ static const char * const am_usage[] = {
 };
 
 static struct option am_options[] = {
+	OPT_BOOL('3', "3way", &state.threeway,
+		N_("allow fall back on 3way merging if needed")),
 	OPT__QUIET(&state.quiet, N_("be quiet")),
 	OPT_BOOL('s', "signoff", &state.sign,
 		N_("add a Signed-off-by line to the commit message")),
-- 
2.1.4