[PATCH/WIP v3 09/31] am: commit applied patch

2015-06-18 Thread 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 pyoka...@gmail.com
---
 builtin/am.c | 51 +++
 1 file changed, 47 insertions(+), 4 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 296a5fc..dfb6f7e 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.
@@ -557,6 +560,49 @@ 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[GIT_SHA1_RAWSZ], parent[GIT_SHA1_RAWSZ],
+ commit[GIT_SHA1_RAWSZ];
+   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)
@@ -588,10 +634,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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/WIP v3 08/31] am: apply patch with git-apply

2015-06-18 Thread Paul Tan
Implement applying the patch to the index using git-apply.

Signed-off-by: Paul Tan pyoka...@gmail.com
---
 builtin/am.c | 57 -
 1 file changed, 56 insertions(+), 1 deletion(-)

diff --git a/builtin/am.c b/builtin/am.c
index d6434e4..296a5fc 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
@@ -519,6 +531,31 @@ 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.
  */
@@ -536,7 +573,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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/WIP v3 02/31] wrapper: implement xfopen()

2015-06-18 Thread 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 pyoka...@gmail.com
---
 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 bc77d77..4e69110 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -724,6 +724,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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/WIP v3 03/31] am: implement skeletal builtin am

2015-06-18 Thread 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.

Since git-am.sh cannot handle any environment modifications by
setup_git_directory(), am has to be declared as NO_SETUP in git.c. On
the other hand, to re-implement git-am.sh in builtin/am.c, we do need to
run all the git dir and work tree setup logic that git.c does for us. As
such, we work around this temporarily by copying the logic in git.c's
run_builtin(), which amounts to:

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

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

Helped-by: Junio C Hamano gits...@pobox.com
Signed-off-by: Paul Tan pyoka...@gmail.com
---

Notes:
v3

* Style fixes

* git-am.sh cannot handle the chdir() and GIT_DIR envionment variable
  that setup_git_directory() sets, so we work around it by copying the
  logic of git.c's run_builtin(), and running it only when we are using
  the builtin am.

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

diff --git a/Makefile b/Makefile
index 93e4fa2..ff9bdc0 100644
--- a/Makefile
+++ b/Makefile
@@ -811,6 +811,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 ea3c834..f30cf00 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 000..dbc8836
--- /dev/null
+++ b/builtin/am.c
@@ -0,0 +1,28 @@
+/*
+ * 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)
+{
+   /*
+* FIXME: Once all the features of git-am.sh have been re-implemented
+* in builtin/am.c, this preamble can be removed.
+*/
+   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);
+   } else {
+   prefix = setup_git_directory();
+   trace_repo_setup(prefix);
+   setup_work_tree();
+   }
+
+   return 0;
+}
diff --git a/git.c b/git.c
index e7a7713..a671535 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, NO_SETUP },
{ annotate, cmd_annotate, RUN_SETUP },
{ apply, cmd_apply, RUN_SETUP_GENTLY },
{ archive, cmd_archive },
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/WIP v3 01/31] wrapper: implement xopen()

2015-06-18 Thread 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 tbo...@web.de
Helped-by: Jeff King p...@peff.net
Signed-off-by: Paul Tan pyoka...@gmail.com
---
 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 0cc7ae8..bc77d77 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -719,6 +719,7 @@ 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 void *xmmap_gently(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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-06-18 Thread 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 pyoka...@gmail.com
---

Notes:
v3

* Moved the TODO comment to the previous patch

 builtin/am.c | 104 +--
 1 file changed, 101 insertions(+), 3 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index af68c51..e9a3687 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,8 @@ static void am_state_init(struct am_state *state)
memset(state, 0, sizeof(*state));
 
strbuf_init(state-dir, 0);
+
+   state-prec = 4;
 }
 
 /**
@@ -111,13 +122,69 @@ 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);
@@ -148,7 +215,23 @@ static void am_run(struct am_state *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)...]),
@@ -156,6 +239,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()
 };
 
@@ -185,8 +270,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;
+ 

[PATCH v4 19/19] pull: remove redirection to git-pull.sh

2015-06-18 Thread Paul Tan
At the beginning of the rewrite of git-pull.sh to C, we introduced a
redirection to git-pull.sh if the environment variable
_GIT_USE_BUILTIN_PULL was not defined in order to not break test scripts
that relied on a functional git-pull.

Now that all of git-pull's functionality has been re-implemented in
builtin/pull.c, remove this redirection, and retire the old git-pull.sh
into contrib/examples/.

Signed-off-by: Paul Tan pyoka...@gmail.com
---
 Makefile| 1 -
 builtin/pull.c  | 7 ---
 git-pull.sh = contrib/examples/git-pull.sh | 0
 3 files changed, 8 deletions(-)
 rename git-pull.sh = contrib/examples/git-pull.sh (100%)

diff --git a/Makefile b/Makefile
index 17e1136..93e4fa2 100644
--- a/Makefile
+++ b/Makefile
@@ -474,7 +474,6 @@ SCRIPT_SH += git-merge-octopus.sh
 SCRIPT_SH += git-merge-one-file.sh
 SCRIPT_SH += git-merge-resolve.sh
 SCRIPT_SH += git-mergetool.sh
-SCRIPT_SH += git-pull.sh
 SCRIPT_SH += git-quiltimport.sh
 SCRIPT_SH += git-rebase.sh
 SCRIPT_SH += git-remote-testgit.sh
diff --git a/builtin/pull.c b/builtin/pull.c
index 421a34d..722a83c 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -798,13 +798,6 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
unsigned char orig_head[GIT_SHA1_RAWSZ], curr_head[GIT_SHA1_RAWSZ];
unsigned char rebase_fork_point[GIT_SHA1_RAWSZ];
 
-   if (!getenv(_GIT_USE_BUILTIN_PULL)) {
-   const char *path = mkpath(%s/git-pull, git_exec_path());
-
-   if (sane_execvp(path, (char **)argv)  0)
-   die_errno(could not exec %s, path);
-   }
-
if (!getenv(GIT_REFLOG_ACTION))
set_reflog_message(argc, argv);
 
diff --git a/git-pull.sh b/contrib/examples/git-pull.sh
similarity index 100%
rename from git-pull.sh
rename to contrib/examples/git-pull.sh
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 14/19] pull: set reflog message

2015-06-18 Thread Paul Tan
f947413 (Use GIT_REFLOG_ACTION environment variable instead.,
2006-12-28) established git-pull's method for setting the reflog
message, which is to set the environment variable GIT_REFLOG_ACTION to
the evaluation of pull${1+ $*} if it has not already been set.

Re-implement this behavior.

Helped-by: Junio C Hamano gits...@pobox.com
Signed-off-by: Paul Tan pyoka...@gmail.com
---
 builtin/pull.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/builtin/pull.c b/builtin/pull.c
index 492bb0e..98caffe 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -169,6 +169,25 @@ static void argv_push_force(struct argv_array *arr)
 }
 
 /**
+ * Sets the GIT_REFLOG_ACTION environment variable to the concatenation of argv
+ */
+static void set_reflog_message(int argc, const char **argv)
+{
+   int i;
+   struct strbuf msg = STRBUF_INIT;
+
+   for (i = 0; i  argc; i++) {
+   if (i)
+   strbuf_addch(msg, ' ');
+   strbuf_addstr(msg, argv[i]);
+   }
+
+   setenv(GIT_REFLOG_ACTION, msg.buf, 0);
+
+   strbuf_release(msg);
+}
+
+/**
  * If pull.ff is unset, returns NULL. If pull.ff is true, returns --ff. If
  * pull.ff is false, returns --no-ff. If pull.ff is only, returns
  * --ff-only. Otherwise, if pull.ff is set to an invalid value, die with an
@@ -443,6 +462,9 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
die_errno(could not exec %s, path);
}
 
+   if (!getenv(GIT_REFLOG_ACTION))
+   set_reflog_message(argc, argv);
+
argc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0);
 
parse_repo_refspecs(argc, argv, repo, refspecs);
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 17/19] pull --rebase: exit early when the working directory is dirty

2015-06-18 Thread Paul Tan
Re-implement the behavior introduced by f9189cf (pull --rebase: exit
early when the working directory is dirty, 2008-05-21).

Signed-off-by: Paul Tan pyoka...@gmail.com
---
 builtin/pull.c | 77 +-
 1 file changed, 76 insertions(+), 1 deletion(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 1c4fb44..eb2a28f 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -14,6 +14,8 @@
 #include remote.h
 #include dir.h
 #include refs.h
+#include revision.h
+#include lockfile.h
 
 enum rebase_type {
REBASE_INVALID = -1,
@@ -296,6 +298,73 @@ static enum rebase_type config_get_rebase(void)
 }
 
 /**
+ * Returns 1 if there are unstaged changes, 0 otherwise.
+ */
+static int has_unstaged_changes(const char *prefix)
+{
+   struct rev_info rev_info;
+   int result;
+
+   init_revisions(rev_info, prefix);
+   DIFF_OPT_SET(rev_info.diffopt, IGNORE_SUBMODULES);
+   DIFF_OPT_SET(rev_info.diffopt, QUICK);
+   diff_setup_done(rev_info.diffopt);
+   result = run_diff_files(rev_info, 0);
+   return diff_result_code(rev_info.diffopt, result);
+}
+
+/**
+ * Returns 1 if there are uncommitted changes, 0 otherwise.
+ */
+static int has_uncommitted_changes(const char *prefix)
+{
+   struct rev_info rev_info;
+   int result;
+
+   if (is_cache_unborn())
+   return 0;
+
+   init_revisions(rev_info, prefix);
+   DIFF_OPT_SET(rev_info.diffopt, IGNORE_SUBMODULES);
+   DIFF_OPT_SET(rev_info.diffopt, QUICK);
+   add_head_to_pending(rev_info);
+   diff_setup_done(rev_info.diffopt);
+   result = run_diff_index(rev_info, 1);
+   return diff_result_code(rev_info.diffopt, result);
+}
+
+/**
+ * If the work tree has unstaged or uncommitted changes, dies with the
+ * appropriate message.
+ */
+static void die_on_unclean_work_tree(const char *prefix)
+{
+   struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
+   int do_die = 0;
+
+   hold_locked_index(lock_file, 0);
+   refresh_cache(REFRESH_QUIET);
+   update_index_if_able(the_index, lock_file);
+   rollback_lock_file(lock_file);
+
+   if (has_unstaged_changes(prefix)) {
+   error(_(Cannot pull with rebase: You have unstaged changes.));
+   do_die = 1;
+   }
+
+   if (has_uncommitted_changes(prefix)) {
+   if (do_die)
+   error(_(Additionally, your index contains uncommitted 
changes.));
+   else
+   error(_(Cannot pull with rebase: Your index contains 
uncommitted changes.));
+   do_die = 1;
+   }
+
+   if (do_die)
+   exit(1);
+}
+
+/**
  * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge
  * into merge_heads.
  */
@@ -751,9 +820,15 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
if (get_sha1(HEAD, orig_head))
hashclr(orig_head);
 
-   if (opt_rebase)
+   if (opt_rebase) {
+   if (is_null_sha1(orig_head)  !is_cache_unborn())
+   die(_(Updating an unborn branch with changes added to 
the index.));
+
+   die_on_unclean_work_tree(prefix);
+
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
hashclr(rebase_fork_point);
+   }
 
if (run_fetch(repo, refspecs))
return 1;
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/WIP v3 31/31] am: implement -S/--gpg-sign, commit.gpgsign

2015-06-18 Thread Paul Tan
Since 3b4e395 (am: add the --gpg-sign option, 2014-02-01), git-am.sh
supported the --gpg-sign option, and would pass it to git-commit-tree,
thus GPG-signing the commit object.

Re-implement this option in builtin/am.c.

git-commit-tree would also sign the commit by default if the
commit.gpgsign setting is true. Since we do not run commit-tree, we
re-implement this behavior by handling the commit.gpgsign setting
ourselves.

Signed-off-by: Paul Tan pyoka...@gmail.com
---
 builtin/am.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/builtin/am.c b/builtin/am.c
index 608a2da..91dae92 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -134,6 +134,8 @@ struct am_state {
 
int ignore_date;
 
+   const char *sign_commit;
+
int rebasing;
 };
 
@@ -143,6 +145,7 @@ struct am_state {
 static void am_state_init(struct am_state *state)
 {
const char *quiet;
+   int sign_commit;
 
memset(state, 0, sizeof(*state));
 
@@ -164,6 +167,9 @@ static void am_state_init(struct am_state *state)
state-scissors = SCISSORS_UNSET;
 
argv_array_init(state-git_apply_opts);
+
+   if (!git_config_get_bool(commit.gpgsign, sign_commit))
+   state-sign_commit = sign_commit ?  : NULL;
 }
 
 /**
@@ -1143,7 +1149,7 @@ static void do_commit(const struct am_state *state)
state-ignore_date ?  : state-author_date.buf, 1);
 
if (commit_tree(state-msg.buf, state-msg.len, tree, parents, commit,
-   author, NULL))
+   author, state-sign_commit))
die(_(failed to write commit object));
 
reflog_msg = getenv(GIT_REFLOG_ACTION);
@@ -1535,6 +1541,9 @@ static struct option am_options[] = {
N_(lie about committer date)),
OPT_BOOL(0, ignore-date, state.ignore_date,
N_(use current timestamp for author date)),
+   { OPTION_STRING, 'S', gpg-sign, state.sign_commit, N_(key-id),
+ N_(GPG-sign commits),
+ PARSE_OPT_OPTARG, NULL, (intptr_t)  },
OPT_HIDDEN_BOOL(0, rebasing, state.rebasing,
N_((internal use for git-rebase))),
OPT_END()
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/WIP v3 18/31] am: implement am --signoff

2015-06-18 Thread 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 pyoka...@gmail.com
---
 builtin/am.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/builtin/am.c b/builtin/am.c
index 1807d12..d45dd41 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.
@@ -71,6 +72,8 @@ struct am_state {
 
int quiet;
 
+   int sign;
+
/* override error message when patch failure occurs */
const char *resolvemsg;
 };
@@ -292,6 +295,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);
 }
 
@@ -477,6 +483,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);
@@ -636,6 +644,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;
 }
 
@@ -1007,6 +1018,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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Git completion not using ls-remote to auto-complete during push

2015-06-18 Thread SZEDER Gábor
Quoting Robert Dailey rcdailey.li...@gmail.com
 I do the following:
 
 $ git push origin :topic
 
 If I stop halfway through typing 'topic' and hit TAB, auto-completion
 does not work if I do not have a local branch by that name (sometimes
 I delete my local branch first, then I push to delete it remotely). I
 thought that git completion code was supposed to use ls-remote to auto
 complete refs used in push operations. Is this supposed to work?

It's intentional.  Running 'git ls-remote' with a far away remote can
take ages, so instead we grab the refs on the remote from the locally
stored refs under 'refs/remotes/remote/'.

See e832f5c096 (completion: avoid ls-remote in certain scenarios,
2013-05-28).  The commit message mentions that you can force
completion of remote refs via 'git ls-remote' by starting with the full
refname, i.e.  'refs/TAB', however, that seems to work only on the
left hand side of the colon in the push refspec.

Gábor

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Using clean/smudge filters with difftool

2015-06-18 Thread Florian Aspart
2015-06-18 15:26 GMT+02:00 John Keeping j...@keeping.me.uk:
 [Please don't top-post on this list.]

 On Thu, Jun 18, 2015 at 03:15:38PM +0200, Florian Aspart wrote:
 2015-06-18 14:31 GMT+02:00 Michael J Gruber g...@drmicha.warpmail.net:
  Florian Aspart venit, vidit, dixit 16.06.2015 16:11:
  Hi everyone,
 
  I created a clean filter to apply on some files before commiting them.
  The filter works correctly when I commit the file and is also applied
  when I usethe iff command line tool.
  However, when using difftool with meld, the filter is not applied and
  the different versions of the files are compared without any
  filtering.
 
  Is there a way to apply the clean/smudge filters when comparing the
  working copy of a file to the HEAD version in a gui diff tool?
 
  I'm using git version 2.4.3 under Ubuntu.
 
  Best,
  Florian
 
  Are you saying that difftool compares an uncleaned working tree file
  with a cleaned blob? That would be a bug in either difftool or the way
  we feed difftool.
 
 yes in this case difftool compares an uncleaned working tree file
 with a cleaned blob. I did not try the smudge filter to see if it
 applied in difftool.

 I think the problem comes from the way difftool is feeded, since I
 also had this problem when setting an external tool for the diff in
 the gitconfig file.

 However, I'm not sure if this is a bug or it is designed to be so.
 If the external tool changes a cleaned working tree file during the
 diff, then by saving this file the result of the cleaning filter would
 also be saved in the working tree.

 How is your filter configured?  Is it using a simple pattern (e.g.
 *.c) or is it using a file path?

 git-difftool uses `git checkout-index --all --prefix=$dir/` and I wonder
 if the prefix means that the attribute specification does not match the
 temporary file that difftool produces, so no filter is applied.


It is using a simple pattern:
*.ipynb filter=clean_ipynb
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: co-authoring commits

2015-06-18 Thread Jason Pyeron
 -Original Message-
 From: Theodore Ts'o
 Sent: Wednesday, June 17, 2015 6:52 PM
 
 On Wed, Jun 17, 2015 at 10:26:32PM +0200, Tuncer Ayaz wrote:
  
  By allowing multiple authors, you don't have to decide who's the 
  primary author, as in such situations usually there is no 
 primary at 
  all. I sometimes deliberately override the author when 
 committing and 
  add myself just as another co-author in the commit message, but as 
  others have noted it would be really great if we can just specify 
  multiple authors.
snip/
 One could imagine some frankly, quite rare example where 
 there is a team of people who votes on each commit before it 
 gets sent out and where everyone is equal and there is no 
 hierarchy.  In that case, perhaps you could set the from 
 field to a mailing list address.

This is a perfect use the signed commit by multiple persons. Git already
supports it (under the hood and in reporting).

A quick google pulled up my notes on this:

http://marc.info/?l=gitm=140845378317052w=2

$ cat merge-multisigs.sh
#!/bin/bash
(
 for i in $@
 do
  gpg --dearmor  $i
 done
) | gpg --enarmor

$ cat write-commit.ruby
#!/usr/bin/irb
require 'fileutils'
file = File.open(ARGV[0], rb)
content = file.read
header = commit #{content.length}\0
store = header + content
require 'digest/sha1'
sha1 = Digest::SHA1.hexdigest(store)
require 'zlib'
zlib_content = Zlib::Deflate.deflate(store)
path = '.git/objects/' + sha1[0,2] + '/' + sha1[2,38]
FileUtils.mkdir_p(File.dirname(path))
File.open(path, 'w') { |f| f.write zlib_content }


--
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
-   -
- Jason Pyeron  PD Inc. http://www.pdinc.us -
- Principal Consultant  10 West 24th Street #100-
- +1 (443) 269-1555 x333Baltimore, Maryland 21218   -
-   -
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- 

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Using clean/smudge filters with difftool

2015-06-18 Thread Michael J Gruber
Florian Aspart venit, vidit, dixit 16.06.2015 16:11:
 Hi everyone,
 
 I created a clean filter to apply on some files before commiting them.
 The filter works correctly when I commit the file and is also applied
 when I usethe iff command line tool.
 However, when using difftool with meld, the filter is not applied and
 the different versions of the files are compared without any
 filtering.
 
 Is there a way to apply the clean/smudge filters when comparing the
 working copy of a file to the HEAD version in a gui diff tool?
 
 I'm using git version 2.4.3 under Ubuntu.
 
 Best,
 Florian

Are you saying that difftool compares an uncleaned working tree file
with a cleaned blob? That would be a bug in either difftool or the way
we feed difftool.

Michael

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Using clean/smudge filters with difftool

2015-06-18 Thread Florian Aspart
Hi Michael,

yes in this case difftool compares an uncleaned working tree file
with a cleaned blob. I did not try the smudge filter to see if it
applied in difftool.

I think the problem comes from the way difftool is feeded, since I
also had this problem when setting an external tool for the diff in
the gitconfig file.

However, I'm not sure if this is a bug or it is designed to be so.
If the external tool changes a cleaned working tree file during the
diff, then by saving this file the result of the cleaning filter would
also be saved in the working tree.

2015-06-18 14:31 GMT+02:00 Michael J Gruber g...@drmicha.warpmail.net:
 Florian Aspart venit, vidit, dixit 16.06.2015 16:11:
 Hi everyone,

 I created a clean filter to apply on some files before commiting them.
 The filter works correctly when I commit the file and is also applied
 when I usethe iff command line tool.
 However, when using difftool with meld, the filter is not applied and
 the different versions of the files are compared without any
 filtering.

 Is there a way to apply the clean/smudge filters when comparing the
 working copy of a file to the HEAD version in a gui diff tool?

 I'm using git version 2.4.3 under Ubuntu.

 Best,
 Florian

 Are you saying that difftool compares an uncleaned working tree file
 with a cleaned blob? That would be a bug in either difftool or the way
 we feed difftool.

 Michael

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Visualizing merge conflicts after the fact (using kdiff3)

2015-06-18 Thread Michael J Gruber
Johannes Schindelin venit, vidit, dixit 16.06.2015 11:43:
 Hi Eric,
 
 On 2015-06-16 03:17, Eric Raible wrote:
 I'm running 1.9.5.msysgit.1, but this is a general git question...

 Upon returning from a vacation, I was looking at what people had been
 up to, and discovered on merge in which a colleague had resolved a merge
 incorrectly.  It turns out that he has pushed *many* merges over the past
 year which had conflicts in my code, and now I don't trust any of them.

 So naturally I want to check each of them for correctness.

 I know about git log -p -cc SHA -- path, but it really doesn't
 show just the conflicts so there's just too much noise in that output.

 I use kdiff3 to resolve conflicts, so I'm looking for a way to
 visualize these already-resolved conflicts with that tool.
 As I said, there are many merges, so the prospect of checking
 out each sha, doing the merge, and then comparing the results
 is completely untenable.

 Can anyone help?  Surely other people have wanted to review how
 conflicts were resolved w/out looking at the noise of unconflicted
 changes, right?
 
 If I was walking in your shoes, I would essentially recreate the merge 
 conflicts and then use git diff merge-commit with the resolved merge in 
 your current history.
 
 Something like this:
 
 ```bash
 mergecommit=$1
 
 # probably should verify that the working directory is clean, yadda yadda
 
 # recreate merge conflicts on an unnamed branch (Git speak: detached HEAD)
 git checkout $mergecommit^
 git merge $mergecommit^2 ||
 die This merge did not have any problem!
 
 # compare to the actual resolution as per the merge commit
 git diff $mergecommit
 ```

This type of request comes up often (for a reason). I'm wondering
whether we could support it more systematically, either by exposing the
steps above as a command, or by storing the unresolved merge somewhere
(leveraging stash or rerere).

 To list all the merge commits in the current branch, I would use the 
 command-line:
 
 ```bash
 git rev-list --author=My Colleague --parents HEAD |
 sed -n 's/ .* .*//p'
 ```
 
 (i.e. listing all the commits with their parents, then filtering just the 
 ones having more than one parent, which would include octopus merges if your 
 history has them.)

:)

--merges (aka --min-parents=2) is your friend here.

 
 Hopefully this gives you good ideas how to proceed.
 
 Ciao,
 Johannes
 

Michael
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] t3404: demonstrate CHERRY_PICK_HEAD bug

2015-06-18 Thread Johannes Schindelin
Hi Junio,

On 2015-06-17 19:33, Junio C Hamano wrote:
 Johannes Schindelin johannes.schinde...@gmx.de writes:
 
 +test_expect_failure 'rebase --continue removes CHERRY_PICK_HEAD' '
 +git checkout -b commit-to-skip 
 +for double in X 3 1
 +do
 +seq 5 | sed s/$double// seq 
 +git add seq 
 +test_tick 
 +git commit -m seq-$double
 +done 
 +git tag seq-onto 
 +git reset --hard HEAD~2 
 +git cherry-pick seq-onto 
 +test_must_fail git rebase -i seq-onto 
 
 Shouldn't this explicitly specify what fake editor is to be used,
 instead of relying on whatever the last test happened to have used?
 
 I thought this deserved to go to older maintenance track, but the
 fake editor that was used last are different between branches, so
 rebase -i fails for a wrong reason (i.e. cannot spawn the editor)
 when queued on say maint-2.2.

True. Thanks for pointing that out. Will be fixed in v2.

 +test -d .git/rebase-merge 
 +git rebase --continue 
 +git diff seq-onto 
 
 I am puzzled with this diff; what is this about?  Is it a remnant
 from an earlier debugging session, or is it making sure seq-onto is
 a valid tree-ish?

The idea is to verify that we end up with the same tree even if we exchanged 
the latest two patches. I can remove it if you want as it is not strictly 
necessary, but I would like to keep it just to make sure that we did not end up 
with an incomplete rebase.

Ciao,
Dscho
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-06-18 Thread 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 gits...@pobox.com
Helped-by: Jeff King p...@peff.net
Signed-off-by: Paul Tan pyoka...@gmail.com
---
Notes:
v3

* Style fixes

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

diff --git a/builtin/am.c b/builtin/am.c
index 7b97ea8..d6434e4 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;
 };
@@ -36,6 +59,11 @@ static void am_state_init(struct am_state *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;
 }
 
@@ -45,6 +73,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);
 }
 
 /**
@@ -94,6 +126,105 @@ static int read_state_file(struct strbuf *sb, const char 
*file, size_t hint, int
 }
 
 /**
+ * Reads a KEY=VALUE shell variable assignment from fp, and returns the VALUE
+ * in `value`. VALUE must be a quoted string, and the KEY must match `key`.
+ * Returns 0 on success, -1 on failure.
+ *
+ * This is used by read_author_script() to read the GIT_AUTHOR_* variables from
+ * the author-script.
+ */
+static int read_shell_var(struct strbuf *value, FILE *fp, const char *key)
+{
+   struct strbuf sb = STRBUF_INIT;
+   char *str;
+
+   if (strbuf_getline(sb, fp, '\n'))
+   return -1;
+
+   if (!skip_prefix(sb.buf, key, (const char **)str))
+   return -1;
+
+   if (!skip_prefix(str, =, (const char **)str))
+   return -1;
+
+   str = sq_dequote(str);
+   if (!str)
+   return -1;
+
+   strbuf_reset(value);
+   strbuf_addstr(value, str);
+
+   strbuf_release(sb);
+
+   return 0;
+}
+
+/**
+ * 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)
+{
+   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 (read_shell_var(state-author_name, fp, GIT_AUTHOR_NAME))
+   return -1;
+
+   if (read_shell_var(state-author_email, fp, GIT_AUTHOR_EMAIL))
+   return -1;
+
+   if (read_shell_var(state-author_date, fp, GIT_AUTHOR_DATE))
+   return -1;
+
+   if (fgetc(fp) != EOF)
+   return -1;
+
+   fclose(fp);
+   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;
+
+   

[PATCH/WIP v3 00/31] Make git-am a builtin

2015-06-18 Thread Paul Tan
This patch series depends on pt/pull-builtin for OPT_PASSTHRU_ARGV() and
argv_array_pushv().

This is a re-roll of [WIP v3]. Thanks Junio and Stefan for the reviews last
round.

The biggest addition this round would be the support for git-rebase. Here's
a small unscientific benchmark that rebases 50 patches:

git init 
echo initial file 
git add file 
git commit -m initial 
git tag initial 
for x in $(seq 50)
do
echo $x file 
git commit -a -m $x
done 
git checkout -b onto-rebase initial 
git commit --allow-empty -mempty 
time git rebase -q --onto onto-rebase initial master

With master:

1.53s, 1.55s, 1.17s, 1.52s, 1.22s. Avg: ~1.40s

With master + this patch series:

0.22s, 0.22s, 0.18s, 0.21s, 0.18s. Avg: ~0.20s

So this is around a 6-7x speedup.

Previous versions:

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

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 (31):
  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: don't accept patches when there's a session in progress
  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
  am: --rebasing
  am: don't use git-mailinfo if --rebasing
  am: handle stray state directory
  am: implement -k/--keep, --keep-non-patch
  am: implement --[no-]message-id, am.messageid
  am: support --keep-cr, am.keepcr
  am: implement --[no-]scissors
  am: pass git-apply's options to git-apply
  am: implement --ignore-date
  am: implement --committer-date-is-author-date
  am: implement -S/--gpg-sign, commit.gpgsign

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

-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/WIP v3 06/31] am: detect mbox patches

2015-06-18 Thread 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 sunsh...@sunshineco.com
Signed-off-by: Paul Tan pyoka...@gmail.com
---
 builtin/am.c | 98 
 1 file changed, 98 insertions(+)

diff --git a/builtin/am.c b/builtin/am.c
index e9a3687..7b97ea8 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -121,6 +121,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.
@@ -177,6 +267,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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/WIP v3 04/31] am: implement patch queue mechanism

2015-06-18 Thread 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 pyoka...@gmail.com
---
 builtin/am.c | 168 +++
 1 file changed, 168 insertions(+)

diff --git a/builtin/am.c b/builtin/am.c
index dbc8836..af68c51 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -6,6 +6,158 @@
 #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_trim(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) {
+
+   /* TODO: Patch application not implemented yet */
+
+   am_next(state);
+   }
+
+   am_destroy(state);
+}
+
+static struct am_state state;
+
+static const char * const am_usage[] = {

Re: Visualizing merge conflicts after the fact (using kdiff3)

2015-06-18 Thread Johannes Schindelin
Hi Micha,

On 2015-06-18 14:26, Michael J Gruber wrote:
 Johannes Schindelin venit, vidit, dixit 16.06.2015 11:43:

 To list all the merge commits in the current branch, I would use the 
 command-line:

 ```bash
 git rev-list --author=My Colleague --parents HEAD |
 sed -n 's/ .* .*//p'
 ```

 (i.e. listing all the commits with their parents, then filtering just the 
 ones having more than one parent, which would include octopus merges if your 
 history has them.)
 
 :)
 
 --merges (aka --min-parents=2) is your friend here.

Learnt something!

Thanks,
Dscho
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] format-patch: introduce format.outputDirectory configuration

2015-06-18 Thread Alexander Kuleshov
We can pass -o/--output-directory to the format-patch command to
store patches not in the working directory. This patch introduces
format.outputDirectory configuration option for same purpose.

The case of usage of this configuration option can be convinience
to not pass everytime -o/--output-directory if an user has pattern
to store all patches in the /patches directory for example.

The format.outputDirectory has lower priority than command line
option, so if user will set format.outputDirectory and pass the
command line option, a result will be stored in a directory that
passed to command line option.

Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com
---
 Documentation/config.txt |  4 
 builtin/log.c| 14 --
 t/t4014-format-patch.sh  |  9 +
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index fd2036c..8f6f7ed 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1247,6 +1247,10 @@ format.coverLetter::
format-patch is invoked, but in addition can be set to auto, to
generate a cover-letter only when there's more than one patch.
 
+format.outputDirectory::
+   Set a custom directory to store the resulting files instead of the
+   current working directory.
+
 filter.driver.clean::
The command which is used to convert the content of a worktree
file to a blob upon checkin.  See linkgit:gitattributes[5] for
diff --git a/builtin/log.c b/builtin/log.c
index dfb351e..22c1e46 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -687,6 +687,8 @@ enum {
COVER_AUTO
 };
 
+static const char *config_output_directory = NULL;
+
 static int git_format_config(const char *var, const char *value, void *cb)
 {
if (!strcmp(var, format.headers)) {
@@ -757,6 +759,9 @@ static int git_format_config(const char *var, const char 
*value, void *cb)
config_cover_letter = git_config_bool(var, value) ? COVER_ON : 
COVER_OFF;
return 0;
}
+   if (!strcmp(var, format.outputdirectory)) {
+   return git_config_string(config_output_directory, var, value);
+   }
 
return git_log_config(var, value, cb);
 }
@@ -1006,7 +1011,8 @@ static const char *clean_message_id(const char *msg_id)
return xmemdupz(a, z - a);
 }
 
-static const char *set_outdir(const char *prefix, const char *output_directory)
+static const char *set_outdir(const char *prefix, const char *output_directory,
+ const char *config_output_directory)
 {
if (output_directory  is_absolute_path(output_directory))
return output_directory;
@@ -1014,6 +1020,9 @@ static const char *set_outdir(const char *prefix, const 
char *output_directory)
if (!prefix || !*prefix) {
if (output_directory)
return output_directory;
+
+   if (config_output_directory)
+   return config_output_directory;
/* The user did not explicitly ask for ./ */
outdir_offset = 2;
return ./;
@@ -1368,7 +1377,8 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
init_display_notes(rev.notes_opt);
 
if (!use_stdout)
-   output_directory = set_outdir(prefix, output_directory);
+   output_directory = set_outdir(prefix, output_directory,
+ config_output_directory);
else
setup_pager();
 
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index c39e500..a4b18b5 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -40,6 +40,15 @@ test_expect_success setup '
 
 '
 
+test_expect_success format-patch format.outputDirectory option '
+   git config format.outputDirectory patches/ 
+   git format-patch master..side 
+   cnt=$(ls | wc -l) 
+   echo $cnt 
+   test $cnt = 3 
+   git config --unset format.outputDirectory
+'
+
 test_expect_success format-patch --ignore-if-in-upstream '
 
git format-patch --stdout master..side patch0 
-- 
2.4.0.383.gded6615.dirty

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/WIP v3 28/31] am: pass git-apply's options to git-apply

2015-06-18 Thread Paul Tan
git-am.sh recognizes some of git-apply's options, and would pass them to
git-apply:

* --whitespace, since 8c31cb8 (git-am: --whitespace=x option.,
  2006-02-28)

* -C, since 67dad68 (add -C[NUM] to git-am, 2007-02-08)

* -p, since 2092a1f (Teach git-am to pass -p option down to git-apply,
  2007-02-11)

* --directory, since b47dfe9 (git-am: add --directory=dir option,
  2009-01-11)

* --reject, since b80da42 (git-am: implement --reject option passed to
  git-apply, 2009-01-23)

* --ignore-space-change, --ignore-whitespace, since 86c91f9 (git apply:
  option to ignore whitespace differences, 2009-08-04)

* --exclude, since 77e9e49 (am: pass exclude down to apply, 2011-08-03)

* --include, since 58725ef (am: support --include option, 2012-03-28)

Re-implement support for these options in builtin/am.c.

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

diff --git a/builtin/am.c b/builtin/am.c
index 42efce1..3556765 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -125,6 +125,8 @@ struct am_state {
/* one of the enum scissors_type values */
int scissors;
 
+   struct argv_array git_apply_opts;
+
/* override error message when patch failure occurs */
const char *resolvemsg;
 
@@ -156,6 +158,8 @@ static void am_state_init(struct am_state *state)
git_config_get_bool(am.messageid, state-message_id);
 
state-scissors = SCISSORS_UNSET;
+
+   argv_array_init(state-git_apply_opts);
 }
 
 /**
@@ -168,6 +172,8 @@ static void am_state_release(struct am_state *state)
strbuf_release(state-author_email);
strbuf_release(state-author_date);
strbuf_release(state-msg);
+
+   argv_array_clear(state-git_apply_opts);
 }
 
 /**
@@ -377,6 +383,11 @@ static void am_load(struct am_state *state)
else
state-scissors = SCISSORS_UNSET;
 
+   read_state_file(sb, am_path(state, apply-opt), 128, 1);
+   argv_array_clear(state-git_apply_opts);
+   if (sq_dequote_to_argv_array(sb.buf, state-git_apply_opts)  0)
+   die(_(could not parse %s), am_path(state, apply-opt));
+
state-rebasing = !!file_exists(am_path(state, rebasing));
 
strbuf_release(sb);
@@ -553,6 +564,7 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
 {
unsigned char curr_head[GIT_SHA1_RAWSZ];
const char *str;
+   struct strbuf sb = STRBUF_INIT;
 
if (!patch_format)
patch_format = detect_patch_format(paths);
@@ -615,6 +627,10 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
}
write_file(am_path(state, scissors), 1, %s, str);
 
+   sq_quote_argv(sb, state-git_apply_opts.argv, 0);
+   write_file(am_path(state, apply-opt), 1, %s, sb.buf);
+   strbuf_release(sb);
+
if (state-rebasing)
write_file(am_path(state, rebasing), 1, %s, );
else
@@ -977,6 +993,8 @@ static int run_apply(const struct am_state *state, const 
char *index_file)
 
argv_array_push(cp.args, apply);
 
+   argv_array_pushv(cp.args, state-git_apply_opts.argv);
+
if (index_file)
argv_array_push(cp.args, --cached);
else
@@ -1003,6 +1021,7 @@ static int build_fake_ancestor(const struct am_state 
*state, const char *index_f
 
cp.git_cmd = 1;
argv_array_push(cp.args, apply);
+   argv_array_pushv(cp.args, state-git_apply_opts.argv);
argv_array_pushf(cp.args, --build-fake-ancestor=%s, index_file);
argv_array_push(cp.args, am_path(state, patch));
 
@@ -1459,8 +1478,35 @@ static struct option am_options[] = {
  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 0},
OPT_BOOL('c', scissors, state.scissors,
N_(strip everything before a scissors line)),
+   OPT_PASSTHRU_ARGV(0, whitespace, state.git_apply_opts, N_(action),
+   N_(pass it through git-apply),
+   0),
+   OPT_PASSTHRU_ARGV(0, ignore-space-change, state.git_apply_opts, NULL,
+   N_(pass it through git-apply),
+   PARSE_OPT_NOARG),
+   OPT_PASSTHRU_ARGV(0, ignore-whitespace, state.git_apply_opts, NULL,
+   N_(pass it through git-apply),
+   PARSE_OPT_NOARG),
+   OPT_PASSTHRU_ARGV(0, directory, state.git_apply_opts, N_(root),
+   N_(pass it through git-apply),
+   0),
+   OPT_PASSTHRU_ARGV(0, exclude, state.git_apply_opts, N_(path),
+   N_(pass it through git-apply),
+   0),
+   OPT_PASSTHRU_ARGV(0, include, state.git_apply_opts, N_(path),
+   N_(pass it through git-apply),
+   0),
+   OPT_PASSTHRU_ARGV('C', NULL, state.git_apply_opts, N_(n),
+   N_(pass it through git-apply),
+   0),
+   OPT_PASSTHRU_ARGV('p', NULL, state.git_apply_opts, N_(num),
+   N_(pass 

[PATCH/WIP v3 30/31] am: implement --committer-date-is-author-date

2015-06-18 Thread Paul Tan
Since 3f01ad6 (am: Add --committer-date-is-author-date option,
2009-01-22), git-am.sh implemented the --committer-date-is-author-date
option, which tells git-am to use the timestamp recorded in the email
message as both author and committer date.

Re-implement this option in builtin/am.c.

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

diff --git a/builtin/am.c b/builtin/am.c
index 6623b49..608a2da 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -130,6 +130,8 @@ struct am_state {
/* override error message when patch failure occurs */
const char *resolvemsg;
 
+   int committer_date_is_author_date;
+
int ignore_date;
 
int rebasing;
@@ -1136,6 +1138,10 @@ static void do_commit(const struct am_state *state)
state-ignore_date ? NULL : state-author_date.buf,
IDENT_STRICT);
 
+   if (state-committer_date_is_author_date)
+   setenv(GIT_COMMITTER_DATE,
+   state-ignore_date ?  : state-author_date.buf, 1);
+
if (commit_tree(state-msg.buf, state-msg.len, tree, parents, commit,
author, NULL))
die(_(failed to write commit object));
@@ -1524,6 +1530,9 @@ static struct option am_options[] = {
OPT_CMDMODE(0, abort, opt_resume,
N_(restore the original branch and abort the patching 
operation.),
RESUME_ABORT),
+   OPT_BOOL(0, committer-date-is-author-date,
+   state.committer_date_is_author_date,
+   N_(lie about committer date)),
OPT_BOOL(0, ignore-date, state.ignore_date,
N_(use current timestamp for author date)),
OPT_HIDDEN_BOOL(0, rebasing, state.rebasing,
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/WIP v3 19/31] cache-tree: introduce write_index_as_tree()

2015-06-18 Thread 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 pyoka...@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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/WIP v3 20/31] am: implement 3-way merge

2015-06-18 Thread Paul Tan
Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07),
git-am.sh supported the --3way option, and if set, would attempt to do a
3-way merge if the initial patch application fails.

Since 5d86861 (am -3: list the paths that needed 3-way fallback,
2012-03-28), in a 3-way merge git-am.sh would list the paths that needed
3-way fallback, so that the user can review them more carefully to spot
mismerges.

Re-implement the above in builtin/am.c.

Signed-off-by: Paul Tan pyoka...@gmail.com
---
 builtin/am.c | 147 +--
 1 file changed, 143 insertions(+), 4 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index d45dd41..e154c87 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -19,6 +19,8 @@
 #include unpack-trees.h
 #include branch.h
 #include sequencer.h
+#include revision.h
+#include merge-recursive.h
 
 /**
  * Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -70,6 +72,8 @@ struct am_state {
/* number of digits in patch filename */
int prec;
 
+   int threeway;
+
int quiet;
 
int sign;
@@ -292,6 +296,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, threeway), 2, 1);
+   state-threeway = !strcmp(sb.buf, t);
+
read_state_file(sb, am_path(state, quiet), 2, 1);
state-quiet = !strcmp(sb.buf, t);
 
@@ -481,6 +488,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, threeway), 1, state-threeway ? t : f);
+
write_file(am_path(state, quiet), 1, state-quiet ? t : f);
 
write_file(am_path(state, sign), 1, state-sign ? t : f);
@@ -666,17 +675,33 @@ 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));
 
@@ -685,8 +710,100 @@ 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[GIT_SHA1_RAWSZ], his_tree[GIT_SHA1_RAWSZ],
+ our_tree[GIT_SHA1_RAWSZ];
+   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) {
+   /*
+* List paths that needed 3-way fallback, so that the user can
+* review them with extra care to spot mismerges.
+*/
+   struct rev_info 

[PATCH/WIP v3 27/31] am: implement --[no-]scissors

2015-06-18 Thread Paul Tan
Since 017678b (am/mailinfo: Disable scissors processing by default,
2009-08-26), git-am supported the --[no-]scissors option, passing it to
git-mailinfo.

Re-implement support for this option.

Signed-off-by: Paul Tan pyoka...@gmail.com
---

Notes:
There are tests for mailinfo --scissors, but not am --scissors, or
mailinfo.scissors.

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

diff --git a/builtin/am.c b/builtin/am.c
index 1991f36..42efce1 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -87,6 +87,12 @@ enum keep_type {
KEEP_NON_PATCH  /* pass -b flag to git-mailinfo */
 };
 
+enum scissors_type {
+   SCISSORS_UNSET = -1,
+   SCISSORS_TRUE,  /* pass --scissors to git-mailinfo */
+   SCISSORS_FALSE  /* pass --no-scissors to git-mailinfo */
+};
+
 struct am_state {
/* state directory path */
struct strbuf dir;
@@ -116,6 +122,9 @@ struct am_state {
/* pass -m flag to git-mailinfo */
int message_id;
 
+   /* one of the enum scissors_type values */
+   int scissors;
+
/* override error message when patch failure occurs */
const char *resolvemsg;
 
@@ -145,6 +154,8 @@ static void am_state_init(struct am_state *state)
state-quiet = 1;
 
git_config_get_bool(am.messageid, state-message_id);
+
+   state-scissors = SCISSORS_UNSET;
 }
 
 /**
@@ -358,6 +369,14 @@ static void am_load(struct am_state *state)
read_state_file(sb, am_path(state, messageid), 2, 1);
state-message_id = !strcmp(sb.buf, t);
 
+   read_state_file(sb, am_path(state, scissors), 2, 1);
+   if (!strcmp(sb.buf, t))
+   state-scissors = SCISSORS_TRUE;
+   else if (!strcmp(sb.buf, f))
+   state-scissors = SCISSORS_FALSE;
+   else
+   state-scissors = SCISSORS_UNSET;
+
state-rebasing = !!file_exists(am_path(state, rebasing));
 
strbuf_release(sb);
@@ -581,6 +600,21 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
 
write_file(am_path(state, messageid), 1, state-message_id ? t : 
f);
 
+   switch (state-scissors) {
+   case SCISSORS_UNSET:
+   str = ;
+   break;
+   case SCISSORS_FALSE:
+   str = f;
+   break;
+   case SCISSORS_TRUE:
+   str = t;
+   break;
+   default:
+   die(BUG: invalid value for state-scissors);
+   }
+   write_file(am_path(state, scissors), 1, %s, str);
+
if (state-rebasing)
write_file(am_path(state, rebasing), 1, %s, );
else
@@ -724,6 +758,19 @@ static int parse_patch(struct am_state *state, const char 
*patch)
if (state-message_id)
argv_array_push(cp.args, -m);
 
+   switch (state-scissors) {
+   case SCISSORS_UNSET:
+   break;
+   case SCISSORS_FALSE:
+   argv_array_push(cp.args, --no-scissors);
+   break;
+   case SCISSORS_TRUE:
+   argv_array_push(cp.args, --scissors);
+   break;
+   default:
+   die(BUG: invalid value for state-scissors);
+   }
+
argv_array_push(cp.args, am_path(state, msg));
argv_array_push(cp.args, am_path(state, patch));
 
@@ -1410,6 +1457,8 @@ static struct option am_options[] = {
{ OPTION_SET_INT, 0, no-keep-cr, opt_keep_cr, NULL,
  N_(do not pass --keep-cr flag to git-mailsplit independent of 
am.keepcr),
  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 0},
+   OPT_BOOL('c', scissors, state.scissors,
+   N_(strip everything before a scissors line)),
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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/WIP v3 14/31] am: implement --abort

2015-06-18 Thread 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 pyoka...@gmail.com
---
 builtin/am.c | 95 ++--
 1 file changed, 92 insertions(+), 3 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 0d7af19..7053b8f 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -423,6 +423,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[GIT_SHA1_RAWSZ];
+
if (!patch_format)
patch_format = detect_patch_format(paths);
 
@@ -442,6 +444,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);
+   }
 }
 
 /**
@@ -450,6 +460,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);
 
@@ -460,6 +472,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, );
 }
 
 /**
@@ -665,10 +682,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);
 
@@ -844,6 +865,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[GIT_SHA1_RAWSZ], head[GIT_SHA1_RAWSZ];
+
+   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[GIT_SHA1_RAWSZ], orig_head[GIT_SHA1_RAWSZ];
+   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`.
@@ -862,7 +944,8 @@ static int 

[PATCH/WIP v3 15/31] am: don't accept patches when there's a session in progress

2015-06-18 Thread Paul Tan
Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07), git-am
would error out if the user gave it mbox(s) on the command-line, but
there was a session in progress.

Since c95b138 (Fix git-am safety checks, 2006-09-15), git-am would
detect if the user attempted to feed it a mbox via stdin, by checking if
stdin is not a tty and there is no resume command given.

Re-implement the above two safety checks.

Signed-off-by: Paul Tan pyoka...@gmail.com
---

Notes:
NOTE: there's no test for this

 builtin/am.c | 19 +--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 7053b8f..4adc487 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1003,9 +1003,24 @@ int cmd_am(int argc, const char **argv, const char 
*prefix)
if (read_index_preload(the_index, NULL)  0)
die(_(failed to read the index));
 
-   if (am_in_progress(state))
+   if (am_in_progress(state)) {
+   /*
+* Catch user error to feed us patches when there is a session
+* in progress:
+*
+* 1. mbox path(s) are provided on the command-line.
+* 2. stdin is not a tty: the user is trying to feed us a patch
+*from standard input. This is somewhat unreliable -- stdin
+*could be /dev/null for example and the caller did not
+*intend to feed us a patch but wanted to continue
+*unattended.
+*/
+   if (argc || (!opt_resume  !isatty(0)))
+   die(_(previous rebase directory %s still exists but 
mbox given.),
+   state.dir.buf);
+
am_load(state);
-   else {
+   } else {
struct string_list paths = STRING_LIST_INIT_DUP;
int i;
 
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/WIP v3 21/31] am: --rebasing

2015-06-18 Thread Paul Tan
Since 3041c32 (am: --rebasing, 2008-03-04), git-am.sh supported the
--rebasing option, which is used internally by git-rebase to tell git-am
that it is being used for its purpose. It would create the empty file
$state_dir/rebasing to help completion scripts tell if the ongoing
operation is am or rebase.

As of 0fbb95d (am: don't call mailinfo if $rebasing, 2012-06-26),
--rebasing also implies --3way as well.

Since a1549e1 (am: return control to caller, for housekeeping,
2013-05-12), git-am.sh would only clean up the state directory when it
is not --rebasing, instead deferring cleanup to git-rebase.sh.

Re-implement the above in builtin/am.c.

Signed-off-by: Paul Tan pyoka...@gmail.com
---
 builtin/am.c | 24 +---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index e154c87..9afa3bb 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -80,6 +80,8 @@ struct am_state {
 
/* override error message when patch failure occurs */
const char *resolvemsg;
+
+   int rebasing;
 };
 
 /**
@@ -305,6 +307,8 @@ static void am_load(struct am_state *state)
read_state_file(sb, am_path(state, sign), 2, 1);
state-sign = !strcmp(sb.buf, t);
 
+   state-rebasing = !!file_exists(am_path(state, rebasing));
+
strbuf_release(sb);
 }
 
@@ -484,6 +488,9 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
die(_(Failed to split patches.));
}
 
+   if (state-rebasing)
+   state-threeway = 1;
+
write_file(am_path(state, next), 1, %d, state-cur);
 
write_file(am_path(state, last), 1, %d, state-last);
@@ -494,12 +501,20 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
 
write_file(am_path(state, sign), 1, state-sign ? t : f);
 
+   if (state-rebasing)
+   write_file(am_path(state, rebasing), 1, %s, );
+   else
+   write_file(am_path(state, applying), 1, %s, );
+
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);
+   if (!state-rebasing)
+   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);
+   if (!state-rebasing)
+   delete_ref(ORIG_HEAD, NULL, 0);
}
 }
 
@@ -921,7 +936,8 @@ next:
am_next(state);
}
 
-   am_destroy(state);
+   if (!state-rebasing)
+   am_destroy(state);
 }
 
 /**
@@ -1175,6 +1191,8 @@ static struct option am_options[] = {
OPT_CMDMODE(0, abort, opt_resume,
N_(restore the original branch and abort the patching 
operation.),
RESUME_ABORT),
+   OPT_HIDDEN_BOOL(0, rebasing, state.rebasing,
+   N_((internal use for git-rebase))),
OPT_END()
 };
 
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/WIP v3 10/31] am: refresh the index at start

2015-06-18 Thread 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 pyoka...@gmail.com
---
 builtin/am.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/builtin/am.c b/builtin/am.c
index dfb6f7e..a7efe85 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.
@@ -471,6 +472,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
@@ -607,6 +622,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));
 
@@ -696,6 +713,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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/WIP v3 26/31] am: support --keep-cr, am.keepcr

2015-06-18 Thread Paul Tan
Since ad2c928 (git-am: Add command line parameter `--keep-cr` passing it
to git-mailsplit, 2010-02-27), git-am.sh supported the --keep-cr option
and would pass it to git-mailsplit.

Since e80d4cb (git-am: Add am.keepcr and --no-keep-cr to override it,
2010-02-27), git-am.sh supported the am.keepcr config setting, which
controls whether --keep-cr is on by default.

Re-implement the above in builtin/am.c.

Signed-off-by: Paul Tan pyoka...@gmail.com
---
 builtin/am.c | 30 --
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 4cec380..1991f36 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -469,7 +469,8 @@ done:
  * 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)
+static int split_patches_mbox(struct am_state *state, struct string_list 
*paths,
+   int keep_cr)
 {
struct child_process cp = CHILD_PROCESS_INIT;
struct string_list_item *item;
@@ -480,6 +481,8 @@ static int split_patches_mbox(struct am_state *state, 
struct string_list *paths)
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);
+   if (keep_cr)
+   argv_array_push(cp.args, --keep-cr);
argv_array_push(cp.args, --);
 
for_each_string_list_item(item, paths)
@@ -501,14 +504,22 @@ static int split_patches_mbox(struct am_state *state, 
struct string_list *paths)
  * set to the index of the first patch, and state-last will be set to the
  * index of the last patch.
  *
+ * Set keep_cr to 0 to convert all lines ending with \r\n to end with \n, 1
+ * to disable this behavior, -1 to use the default configured setting.
+ *
  * Returns 0 on success, -1 on failure.
  */
 static int split_patches(struct am_state *state, enum patch_format 
patch_format,
-   struct string_list *paths)
+   struct string_list *paths, int keep_cr)
 {
+   if (keep_cr  0) {
+   keep_cr = 0;
+   git_config_get_bool(am.keepcr, keep_cr);
+   }
+
switch (patch_format) {
case PATCH_FORMAT_MBOX:
-   return split_patches_mbox(state, paths);
+   return split_patches_mbox(state, paths, keep_cr);
default:
die(BUG: invalid patch_format);
}
@@ -519,7 +530,7 @@ static int split_patches(struct am_state *state, enum 
patch_format patch_format,
  * Setup a new am session for applying patches
  */
 static void am_setup(struct am_state *state, enum patch_format patch_format,
-   struct string_list *paths)
+   struct string_list *paths, int keep_cr)
 {
unsigned char curr_head[GIT_SHA1_RAWSZ];
const char *str;
@@ -535,7 +546,7 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
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) {
+   if (split_patches(state, patch_format, paths, keep_cr)  0) {
am_destroy(state);
die(_(Failed to split patches.));
}
@@ -1371,6 +1382,7 @@ enum resume_mode {
 };
 
 static struct am_state state;
+static int opt_keep_cr = -1;
 static int opt_patch_format;
 static enum resume_mode opt_resume;
 
@@ -1392,6 +1404,12 @@ static struct option am_options[] = {
N_(pass -b flag to git-mailinfo), KEEP_NON_PATCH),
OPT_BOOL('m', message-id, state.message_id,
N_(pass -m flag to git-mailinfo)),
+   { OPTION_SET_INT, 0, keep-cr, opt_keep_cr, NULL,
+ N_(pass --keep-cr flag to git-mailsplit for mbox format),
+ PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1},
+   { OPTION_SET_INT, 0, no-keep-cr, opt_keep_cr, NULL,
+ N_(do not pass --keep-cr flag to git-mailsplit independent of 
am.keepcr),
+ PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 0},
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,
@@ -1486,7 +1504,7 @@ int cmd_am(int argc, const char **argv, const char 
*prefix)
string_list_append(paths, mkpath(%s/%s, 
prefix, argv[i]));
}
 
-   am_setup(state, opt_patch_format, paths);
+   am_setup(state, opt_patch_format, paths, opt_keep_cr);
 
string_list_clear(paths, 0);
}
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/WIP v3 24/31] am: implement -k/--keep, --keep-non-patch

2015-06-18 Thread Paul Tan
Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07),
git-am.sh supported the -k/--keep option to pass the -k option to
git-mailsplit.

Since f7e5ea1 (am: learn passing -b to mailinfo, 2012-01-16), git-am.sh
supported the --keep-non-patch option to pass the -b option to
git-mailsplit.

Re-implement these two options in builtin/am.c.

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

diff --git a/builtin/am.c b/builtin/am.c
index bbef91f..b73549f 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -81,6 +81,12 @@ enum patch_format {
PATCH_FORMAT_MBOX
 };
 
+enum keep_type {
+   KEEP_FALSE = 0,
+   KEEP_TRUE,  /* pass -k flag to git-mailinfo */
+   KEEP_NON_PATCH  /* pass -b flag to git-mailinfo */
+};
+
 struct am_state {
/* state directory path */
struct strbuf dir;
@@ -104,6 +110,9 @@ struct am_state {
 
int sign;
 
+   /* one of the enum keep_type values */
+   int keep;
+
/* override error message when patch failure occurs */
const char *resolvemsg;
 
@@ -333,6 +342,14 @@ static void am_load(struct am_state *state)
read_state_file(sb, am_path(state, sign), 2, 1);
state-sign = !strcmp(sb.buf, t);
 
+   read_state_file(sb, am_path(state, keep), 2, 1);
+   if (!strcmp(sb.buf, t))
+   state-keep = KEEP_TRUE;
+   else if (!strcmp(sb.buf, b))
+   state-keep = KEEP_NON_PATCH;
+   else
+   state-keep = KEEP_FALSE;
+
state-rebasing = !!file_exists(am_path(state, rebasing));
 
strbuf_release(sb);
@@ -497,6 +514,7 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
struct string_list *paths)
 {
unsigned char curr_head[GIT_SHA1_RAWSZ];
+   const char *str;
 
if (!patch_format)
patch_format = detect_patch_format(paths);
@@ -527,6 +545,21 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
 
write_file(am_path(state, sign), 1, state-sign ? t : f);
 
+   switch (state-keep) {
+   case KEEP_FALSE:
+   str = f;
+   break;
+   case KEEP_TRUE:
+   str = t;
+   break;
+   case KEEP_NON_PATCH:
+   str = b;
+   break;
+   default:
+   die(BUG: invalid value for state-keep);
+   }
+   write_file(am_path(state, keep), 1, %s, str);
+
if (state-rebasing)
write_file(am_path(state, rebasing), 1, %s, );
else
@@ -653,6 +686,20 @@ static int parse_patch(struct am_state *state, const char 
*patch)
cp.out = xopen(am_path(state, info), O_WRONLY | O_CREAT, 0777);
 
argv_array_push(cp.args, mailinfo);
+
+   switch (state-keep) {
+   case KEEP_FALSE:
+   break;
+   case KEEP_TRUE:
+   argv_array_push(cp.args, -k);
+   break;
+   case KEEP_NON_PATCH:
+   argv_array_push(cp.args, -b);
+   break;
+   default:
+   die(BUG: invalid value for state-keep);
+   }
+
argv_array_push(cp.args, am_path(state, msg));
argv_array_push(cp.args, am_path(state, patch));
 
@@ -1326,6 +1373,10 @@ 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_SET_INT('k', keep, state.keep,
+   N_(pass -k flag to git-mailinfo), KEEP_TRUE),
+   OPT_SET_INT(0, keep-non-patch, state.keep,
+   N_(pass -b flag to git-mailinfo), KEEP_NON_PATCH),
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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/WIP v3 16/31] am: implement quiet option

2015-06-18 Thread 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 pyoka...@gmail.com
---
 builtin/am.c | 36 +---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 4adc487..5f38264 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);
@@ -85,6 +89,10 @@ static void am_state_init(struct am_state *state)
strbuf_init(state-msg, 0);
 
state-prec = 4;
+
+   quiet = getenv(GIT_QUIET);
+   if (quiet  *quiet)
+   state-quiet = 1;
 }
 
 /**
@@ -108,6 +116,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)
@@ -262,6 +286,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);
 }
 
@@ -445,6 +472,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);
@@ -654,7 +683,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,
@@ -705,7 +734,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;
@@ -736,7 +765,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
@@ -959,6 +988,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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/WIP v3 12/31] am: implement --resolved/--continue

2015-06-18 Thread 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().

Since it makes no sense for the user to run am --resolved when there is
no session in progress, we error out in this case.

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

diff --git a/builtin/am.c b/builtin/am.c
index 9d6ab2a..4381164 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -707,6 +707,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`.
  */
@@ -721,17 +749,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()
 };
 
@@ -768,6 +809,9 @@ int cmd_am(int argc, const char **argv, const char *prefix)
struct string_list paths = STRING_LIST_INIT_DUP;
int i;
 
+   if (opt_resume)
+   die(_(Resolve operation not in progress, we are not 
resuming.));
+
for (i = 0; i  argc; i++) {
if (is_absolute_path(argv[i]) || !prefix)
string_list_append(paths, argv[i]);
@@ -780,7 +824,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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/WIP v3 22/31] am: don't use git-mailinfo if --rebasing

2015-06-18 Thread Paul Tan
Since 5e835ca (rebase: do not munge commit log message, 2008-04-16),
git am --rebasing no longer gets the commit log message from the patch,
but reads it directly from the commit identified by the From  header
line.

Since 43c2325 (am: use get_author_ident_from_commit instead of mailinfo
when rebasing, 2010-06-16), git am --rebasing also gets the author name,
email and date directly from the commit.

Since 0fbb95d (am: don't call mailinfo if $rebasing, 2012-06-26), git am
--rebasing does not use git-mailinfo to get the patch body, but rather
generates it directly from the commit itself.

The above 3 commits introduced a separate parse_patch() code path in
git-am.sh's --rebasing mode that bypasses git-mailinfo. Re-implement
this code path in builtin/am.c as parse_patch_rebase().

Signed-off-by: Paul Tan pyoka...@gmail.com
---
 builtin/am.c | 155 ++-
 1 file changed, 153 insertions(+), 2 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 9afa3bb..0d7e37c 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -21,6 +21,8 @@
 #include sequencer.h
 #include revision.h
 #include merge-recursive.h
+#include revision.h
+#include log-tree.h
 
 /**
  * Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -50,6 +52,30 @@ static const char *firstline(const char *msg)
return sb.buf;
 }
 
+/**
+ * Iterate over lines, each terminated by c, in a string, without modifying the
+ * string itself. begin will be set to the beginning of the line, while end
+ * will be set to the line termination or null character at the end of the line
+ * or string. On the first call, begin and end must be initialized to the
+ * string to be iterated over. E.g.:
+ *
+ * const char *begin, *end, *str = 1\n2\n3;
+ * begin = end = str;
+ * while (!getbufline(begin, end, '\n'))
+ * ...
+ */
+static int getbufline(const char **begin, const char **end, int c)
+{
+   if (!**end)
+   return EOF;
+   else if (**end == c)
+   *begin = *end + 1;
+   else
+   *begin = *end;
+   *end = strchrnul(*begin, c);
+   return 0;
+}
+
 enum patch_format {
PATCH_FORMAT_UNKNOWN = 0,
PATCH_FORMAT_MBOX
@@ -675,6 +701,126 @@ static int parse_patch(struct am_state *state, const char 
*patch)
 }
 
 /**
+ * Sets commit_id to the commit hash where the patch was generated from.
+ * Returns 0 on success, -1 on failure.
+ */
+static int get_patch_commit_sha1(unsigned char *commit_id, const char *patch)
+{
+   struct strbuf sb = STRBUF_INIT;
+   FILE *fp = xfopen(patch, r);
+   const char *x;
+
+   if (strbuf_getline(sb, fp, '\n'))
+   return -1;
+
+   if (!skip_prefix(sb.buf, From , x))
+   return -1;
+
+   if (get_sha1_hex(x, commit_id)  0)
+   return -1;
+
+   strbuf_release(sb);
+   fclose(fp);
+   return 0;
+}
+
+/**
+ * Sets state-msg, state-author_name, state-author_email, state-author_date
+ * to the commit's respective info.
+ */
+static void get_commit_info(struct am_state *state, struct commit *commit)
+{
+   const char *buf, *begin, *end;
+
+   buf = logmsg_reencode(commit, NULL, get_commit_output_encoding());
+   begin = end = buf;
+
+   while (!getbufline(begin, end, '\n')) {
+   const char *x;
+
+   if (skip_prefix(begin, author , x)) {
+   struct ident_split split;
+   const char *date;
+
+   if (split_ident_line(split, x, end - x)  0) {
+   struct strbuf sb = STRBUF_INIT;
+
+   strbuf_add(sb, x, end - x);
+   die(_(invalid ident line: %s), sb.buf);
+   }
+
+   if (split.name_begin)
+   strbuf_add(state-author_name,
+   split.name_begin,
+   split.name_end - split.name_begin);
+
+   if (split.mail_begin)
+   strbuf_add(state-author_email,
+   split.mail_begin,
+   split.mail_end - split.mail_begin);
+
+   date = show_ident_date(split, DATE_NORMAL);
+   strbuf_addstr(state-author_date, date);
+   } else if (begin == end) {
+   end++;
+   break;
+   }
+   }
+
+   strbuf_addstr(state-msg, end);
+}
+
+/**
+ * Writes commit as a patch to $state_dir/patch.
+ */
+static void write_commit_patch(const struct am_state *state, struct commit 
*commit)
+{
+   struct rev_info rev_info;
+   FILE *fp;
+
+   fp = xfopen(am_path(state, patch), w);
+   init_revisions(rev_info, NULL);
+   rev_info.diff = 1;
+   rev_info.abbrev = 0;
+   rev_info.disable_stdin = 1;
+ 

[PATCH/WIP v3 29/31] am: implement --ignore-date

2015-06-18 Thread Paul Tan
Since a79ec62 (git-am: Add --ignore-date option, 2009-01-24), git-am.sh
supported the --ignore-date option, and would use the current timestamp
instead of the one provided in the patch if the option was set.

Re-implement this option in builtin/am.c.

Signed-off-by: Paul Tan pyoka...@gmail.com
---
 builtin/am.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/builtin/am.c b/builtin/am.c
index 3556765..6623b49 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -130,6 +130,8 @@ struct am_state {
/* override error message when patch failure occurs */
const char *resolvemsg;
 
+   int ignore_date;
+
int rebasing;
 };
 
@@ -1131,7 +1133,8 @@ static void do_commit(const struct am_state *state)
}
 
author = fmt_ident(state-author_name.buf, state-author_email.buf,
-   state-author_date.buf, IDENT_STRICT);
+   state-ignore_date ? NULL : state-author_date.buf,
+   IDENT_STRICT);
 
if (commit_tree(state-msg.buf, state-msg.len, tree, parents, commit,
author, NULL))
@@ -1521,6 +1524,8 @@ static struct option am_options[] = {
OPT_CMDMODE(0, abort, opt_resume,
N_(restore the original branch and abort the patching 
operation.),
RESUME_ABORT),
+   OPT_BOOL(0, ignore-date, state.ignore_date,
+   N_(use current timestamp for author date)),
OPT_HIDDEN_BOOL(0, rebasing, state.rebasing,
N_((internal use for git-rebase))),
OPT_END()
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/WIP v3 25/31] am: implement --[no-]message-id, am.messageid

2015-06-18 Thread Paul Tan
Since a078f73 (git-am: add --message-id/--no-message-id, 2014-11-25),
git-am.sh supported the --[no-]message-id options, and the
am.messageid setting which specifies the default option.

--[no-]message-id tells git-am whether or not the -m option should be
passed to git-mailinfo.

Re-implement this option in builtin/am.c.

Signed-off-by: Paul Tan pyoka...@gmail.com
---

Notes:
No test for am.messageid

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

diff --git a/builtin/am.c b/builtin/am.c
index b73549f..4cec380 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -113,6 +113,9 @@ struct am_state {
/* one of the enum keep_type values */
int keep;
 
+   /* pass -m flag to git-mailinfo */
+   int message_id;
+
/* override error message when patch failure occurs */
const char *resolvemsg;
 
@@ -140,6 +143,8 @@ static void am_state_init(struct am_state *state)
quiet = getenv(GIT_QUIET);
if (quiet  *quiet)
state-quiet = 1;
+
+   git_config_get_bool(am.messageid, state-message_id);
 }
 
 /**
@@ -350,6 +355,9 @@ static void am_load(struct am_state *state)
else
state-keep = KEEP_FALSE;
 
+   read_state_file(sb, am_path(state, messageid), 2, 1);
+   state-message_id = !strcmp(sb.buf, t);
+
state-rebasing = !!file_exists(am_path(state, rebasing));
 
strbuf_release(sb);
@@ -560,6 +568,8 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
}
write_file(am_path(state, keep), 1, %s, str);
 
+   write_file(am_path(state, messageid), 1, state-message_id ? t : 
f);
+
if (state-rebasing)
write_file(am_path(state, rebasing), 1, %s, );
else
@@ -700,6 +710,9 @@ static int parse_patch(struct am_state *state, const char 
*patch)
die(BUG: invalid value for state-keep);
}
 
+   if (state-message_id)
+   argv_array_push(cp.args, -m);
+
argv_array_push(cp.args, am_path(state, msg));
argv_array_push(cp.args, am_path(state, patch));
 
@@ -1377,6 +1390,8 @@ static struct option am_options[] = {
N_(pass -k flag to git-mailinfo), KEEP_TRUE),
OPT_SET_INT(0, keep-non-patch, state.keep,
N_(pass -b flag to git-mailinfo), KEEP_NON_PATCH),
+   OPT_BOOL('m', message-id, state.message_id,
+   N_(pass -m flag to git-mailinfo)),
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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/WIP v3 13/31] am: implement --skip

2015-06-18 Thread 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 pyoka...@gmail.com
---
 builtin/am.c | 121 ++-
 1 file changed, 119 insertions(+), 2 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 4381164..0d7af19 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.
@@ -735,6 +737,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`.
  */
@@ -751,7 +861,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;
@@ -760,7 +871,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
 };
 
@@ -773,6 +884,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()
 };
 
@@ -831,6 +945,9 @@ int cmd_am(int argc, const char **argv, const char 

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

2015-06-18 Thread 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 pyoka...@gmail.com
---

Notes:
Note: no tests for this

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

diff --git a/builtin/am.c b/builtin/am.c
index a7efe85..9d6ab2a 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.
@@ -486,6 +488,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
@@ -622,8 +661,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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/WIP v3 23/31] am: handle stray state directory

2015-06-18 Thread Paul Tan
Should git-am terminate unexpectedly between the point where the state
directory is created, but the next and last files are not written
yet, a stray state directory will be left behind.

As such, since b141f3c (am: handle stray $dotest directory, 2013-06-15),
git-am.sh explicitly recognizes such a stray directory, and allows the
user to remove it with am --abort.

Re-implement this feature in builtin/am.c.

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

diff --git a/builtin/am.c b/builtin/am.c
index 0d7e37c..bbef91f 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1395,6 +1395,21 @@ int cmd_am(int argc, const char **argv, const char 
*prefix)
struct string_list paths = STRING_LIST_INIT_DUP;
int i;
 
+   /*
+* Possible stray dotest directory in the independent-run case.
+*/
+   if (file_exists(state.dir.buf)  !state.rebasing) {
+   if (opt_resume == RESUME_ABORT) {
+   am_destroy(state);
+   am_state_release(state);
+   return 0;
+   }
+
+   die(_(Stray %s directory found.\n
+   Use \git am --abort\ to remove it.),
+   state.dir.buf);
+   }
+
if (opt_resume)
die(_(Resolve operation not in progress, we are not 
resuming.));
 
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/WIP v3 17/31] am: exit with user friendly message on patch failure

2015-06-18 Thread 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 pyoka...@gmail.com
---
 builtin/am.c | 26 +++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 5f38264..1807d12 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;
 };
 
 /**
@@ -636,6 +639,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.
  */
@@ -746,7 +764,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);
@@ -771,13 +789,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);
@@ -991,6 +1009,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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v4 07/10] send-email: reduce dependancies impact on parse_address_line

2015-06-18 Thread Matthieu Moy
Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr writes:

 I've some more tests, maybe I should put them all in this post ?

Yes, please post as much as you have. Ideally, this should be
automatically tested, but if you don't have time to write the automated
tests, at least having a track of what you did on the list archives can
help someone else to do it.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 1/2] t3404: demonstrate CHERRY_PICK_HEAD bug

2015-06-18 Thread Johannes Schindelin
When rev-list's --cherry option does not detect that a patch has already
been applied upstream, an interactive rebase would offer to reapply it and
consequently stop at that patch with a failure, mentioning that the diff
is empty.

Traditionally, a `git rebase --continue` simply skips the commit in such a
situation.

However, as pointed out by Gábor Szeder, this leaves a CHERRY_PICK_HEAD
behind, making the Git prompt believe that a cherry pick is still going
on. This commit adds a test case demonstrating this bug.

Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
---
 t/t3404-rebase-interactive.sh | 21 +
 1 file changed, 21 insertions(+)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index ac429a0..e5e7744 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1102,4 +1102,25 @@ test_expect_success 'rebase -i commits that overwrite 
untracked files (no ff)' '
test $(git cat-file commit HEAD | sed -ne \$p) = I
 '
 
+test_expect_failure 'rebase --continue removes CHERRY_PICK_HEAD' '
+   git checkout -b commit-to-skip 
+   for double in X 3 1
+   do
+   seq 5 | sed s/$double// seq 
+   git add seq 
+   test_tick 
+   git commit -m seq-$double
+   done 
+   git tag seq-onto 
+   git reset --hard HEAD~2 
+   git cherry-pick seq-onto 
+   set_fake_editor 
+   FAKE_LINES= test_must_fail git rebase -i seq-onto 
+   test -d .git/rebase-merge 
+   git rebase --continue 
+   git diff seq-onto 
+   test ! -d .git/rebase-merge 
+   test ! -f .git/CHERRY_PICK_HEAD
+'
+
 test_done
-- 
2.3.1.windows.1.9.g8c01ab4



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 0/2] rebase -i: Fix left-behind CHERRY_PICK_HEAD

2015-06-18 Thread Johannes Schindelin
These patches fix a bug that bites me often enough when rebasing Git for
Windows.

The symptom is that .git/CHERRY_PICK_HEAD is left behind after skipping
an already-merged patch with `git rebase --continue` instead of `git
rebase --skip`. I always prefer the former invocation because the latter
would also skip legitimate patches if there were merge conflicts, while
the former would not allow that.

Changes since v1:

- no longer uses ':' to make the comment a no-op statement

- sets the fake editor correctly in the test (I verified that by
  rebasing onto v2.2.2 and running the test with and without setting the
  fake editor)

Interdiff below the diffstat.

Johannes Schindelin (2):
  t3404: demonstrate CHERRY_PICK_HEAD bug
  rebase -i: do not leave a CHERRY_PICK_HEAD file behind

 git-rebase--interactive.sh|  6 +-
 t/t3404-rebase-interactive.sh | 21 +
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 16e0a82..5ff0f1c 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -849,7 +849,7 @@ continue)
# do we have anything to commit?
if git diff-index --cached --quiet HEAD --
then
-   : Nothing to commit -- skip this commit
+   # Nothing to commit -- skip this commit
 
test ! -f $GIT_DIR/CHERRY_PICK_HEAD ||
rm $GIT_DIR/CHERRY_PICK_HEAD ||
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 241d4d1..f3337ad 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1114,7 +1114,8 @@ test_expect_success 'rebase --continue removes 
CHERRY_PICK_HEAD' '
git tag seq-onto 
git reset --hard HEAD~2 
git cherry-pick seq-onto 
-   test_must_fail git rebase -i seq-onto 
+   set_fake_editor 
+   FAKE_LINES= test_must_fail git rebase -i seq-onto 
test -d .git/rebase-merge 
git rebase --continue 
git diff seq-onto 

-- 
2.3.1.windows.1.9.g8c01ab4


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 2/2] rebase -i: do not leave a CHERRY_PICK_HEAD file behind

2015-06-18 Thread Johannes Schindelin
When skipping commits whose changes were already applied via `git rebase
--continue`, we need to clean up said file explicitly.

The same is not true for `git rebase --skip` because that will execute
`git reset --hard` as part of the skip handling in git-rebase.sh, even
before git-rebase--interactive.sh is called.

Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
---
 git-rebase--interactive.sh| 6 +-
 t/t3404-rebase-interactive.sh | 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index dc3133f..5ff0f1c 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -849,7 +849,11 @@ continue)
# do we have anything to commit?
if git diff-index --cached --quiet HEAD --
then
-   : Nothing to commit -- skip this
+   # Nothing to commit -- skip this commit
+
+   test ! -f $GIT_DIR/CHERRY_PICK_HEAD ||
+   rm $GIT_DIR/CHERRY_PICK_HEAD ||
+   die Could not remove CHERRY_PICK_HEAD
else
if ! test -f $author_script
then
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index e5e7744..f3337ad 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1102,7 +1102,7 @@ test_expect_success 'rebase -i commits that overwrite 
untracked files (no ff)' '
test $(git cat-file commit HEAD | sed -ne \$p) = I
 '
 
-test_expect_failure 'rebase --continue removes CHERRY_PICK_HEAD' '
+test_expect_success 'rebase --continue removes CHERRY_PICK_HEAD' '
git checkout -b commit-to-skip 
for double in X 3 1
do
-- 
2.3.1.windows.1.9.g8c01ab4


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Using clean/smudge filters with difftool

2015-06-18 Thread John Keeping
[Please don't top-post on this list.]

On Thu, Jun 18, 2015 at 03:15:38PM +0200, Florian Aspart wrote:
 2015-06-18 14:31 GMT+02:00 Michael J Gruber g...@drmicha.warpmail.net:
  Florian Aspart venit, vidit, dixit 16.06.2015 16:11:
  Hi everyone,
 
  I created a clean filter to apply on some files before commiting them.
  The filter works correctly when I commit the file and is also applied
  when I usethe iff command line tool.
  However, when using difftool with meld, the filter is not applied and
  the different versions of the files are compared without any
  filtering.
 
  Is there a way to apply the clean/smudge filters when comparing the
  working copy of a file to the HEAD version in a gui diff tool?
 
  I'm using git version 2.4.3 under Ubuntu.
 
  Best,
  Florian
 
  Are you saying that difftool compares an uncleaned working tree file
  with a cleaned blob? That would be a bug in either difftool or the way
  we feed difftool.
 
 yes in this case difftool compares an uncleaned working tree file
 with a cleaned blob. I did not try the smudge filter to see if it
 applied in difftool.
 
 I think the problem comes from the way difftool is feeded, since I
 also had this problem when setting an external tool for the diff in
 the gitconfig file.
 
 However, I'm not sure if this is a bug or it is designed to be so.
 If the external tool changes a cleaned working tree file during the
 diff, then by saving this file the result of the cleaning filter would
 also be saved in the working tree.

How is your filter configured?  Is it using a simple pattern (e.g.
*.c) or is it using a file path?

git-difftool uses `git checkout-index --all --prefix=$dir/` and I wonder
if the prefix means that the attribute specification does not match the
temporary file that difftool produces, so no filter is applied.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 15/19] pull: teach git pull about --rebase

2015-06-18 Thread Paul Tan
Since cd67e4d (Teach 'git pull' about --rebase, 2007-11-28), if the
--rebase option is set, git-rebase is run instead of git-merge.

Re-implement this by introducing run_rebase(), which is called instead
of run_merge() if opt_rebase is a true value.

Since c85c792 (pull --rebase: be cleverer with rebased upstream
branches, 2008-01-26), git-pull handles the case where the upstream
branch was rebased since it was last fetched. The fork point (old remote
ref) of the branch from the upstream branch is calculated before fetch,
and then rebased from onto the new remote head (merge_head) after fetch.

Re-implement this by introducing get_merge_branch_2() and
get_merge_branch_1() to find the upstream branch for the
specified/current branch, and get_rebase_fork_point() which will find
the fork point between the upstream branch and current branch.

However, the above change created a problem where git-rebase cannot
detect commits that are already upstream, and thus may result in
unnecessary conflicts. cf65426 (pull --rebase: Avoid spurious conflicts
and reapplying unnecessary patches, 2010-08-12) fixes this by ignoring
the above old remote ref if it is contained within the merge base of the
merge head and the current branch.

This is re-implemented in run_rebase() where fork_point is not used if
it is the merge base returned by get_octopus_merge_base().

Helped-by: Stefan Beller sbel...@google.com
Helped-by: Johannes Schindelin johannes.schinde...@gmx.de
Signed-off-by: Paul Tan pyoka...@gmail.com
---
 builtin/pull.c | 247 -
 1 file changed, 245 insertions(+), 2 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 98caffe..33a7b98 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -15,6 +15,53 @@
 #include dir.h
 #include refs.h
 
+enum rebase_type {
+   REBASE_INVALID = -1,
+   REBASE_FALSE = 0,
+   REBASE_TRUE,
+   REBASE_PRESERVE
+};
+
+/**
+ * Parses the value of --rebase. If value is a false value, returns
+ * REBASE_FALSE. If value is a true value, returns REBASE_TRUE. If value is
+ * preserve, returns REBASE_PRESERVE. If value is a invalid value, dies with
+ * a fatal error if fatal is true, otherwise returns REBASE_INVALID.
+ */
+static enum rebase_type parse_config_rebase(const char *key, const char *value,
+   int fatal)
+{
+   int v = git_config_maybe_bool(pull.rebase, value);
+
+   if (!v)
+   return REBASE_FALSE;
+   else if (v  0)
+   return REBASE_TRUE;
+   else if (!strcmp(value, preserve))
+   return REBASE_PRESERVE;
+
+   if (fatal)
+   die(_(Invalid value for %s: %s), key, value);
+   else
+   error(_(Invalid value for %s: %s), key, value);
+
+   return REBASE_INVALID;
+}
+
+/**
+ * Callback for --rebase, which parses arg with parse_config_rebase().
+ */
+static int parse_opt_rebase(const struct option *opt, const char *arg, int 
unset)
+{
+   enum rebase_type *value = opt-value;
+
+   if (arg)
+   *value = parse_config_rebase(--rebase, arg, 0);
+   else
+   *value = unset ? REBASE_FALSE : REBASE_TRUE;
+   return *value == REBASE_INVALID ? -1 : 0;
+}
+
 static const char * const pull_usage[] = {
N_(git pull [options] [repository [refspec...]]),
NULL
@@ -24,7 +71,8 @@ static const char * const pull_usage[] = {
 static int opt_verbosity;
 static char *opt_progress;
 
-/* Options passed to git-merge */
+/* Options passed to git-merge or git-rebase */
+static enum rebase_type opt_rebase;
 static char *opt_diffstat;
 static char *opt_log;
 static char *opt_squash;
@@ -58,8 +106,12 @@ static struct option pull_options[] = {
N_(force progress reporting),
PARSE_OPT_NOARG),
 
-   /* Options passed to git-merge */
+   /* Options passed to git-merge or git-rebase */
OPT_GROUP(N_(Options related to merging)),
+   { OPTION_CALLBACK, 'r', rebase, opt_rebase,
+ N_(false|true|preserve),
+ N_(incorporate changes by rebasing rather than merging),
+ PARSE_OPT_OPTARG, parse_opt_rebase },
OPT_PASSTHRU('n', NULL, opt_diffstat, NULL,
N_(do not show a diffstat at the end of the merge),
PARSE_OPT_NOARG | PARSE_OPT_NONEG),
@@ -449,11 +501,194 @@ static int run_merge(void)
return ret;
 }
 
+/**
+ * Returns remote's upstream branch for the current branch. If remote is NULL,
+ * the current branch's configured default remote is used. Returns NULL if
+ * `remote` does not name a valid remote, HEAD does not point to a branch,
+ * remote is not the branch's configured remote or the branch does not have any
+ * configured upstream branch.
+ */
+static const char *get_upstream_branch(const char *remote)
+{
+   struct remote *rm;
+   struct branch *curr_branch;
+   const char *curr_branch_remote;
+
+   rm = remote_get(remote);
+   if (!rm)
+  

[PATCH v4 18/19] pull --rebase: error on no merge candidate cases

2015-06-18 Thread Paul Tan
Tweak the error messages printed by die_no_merge_candidates() to take
into account that we may be rebasing against rather than merging
with.

Signed-off-by: Paul Tan pyoka...@gmail.com
---
 builtin/pull.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index eb2a28f..421a34d 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -431,7 +431,10 @@ static void NORETURN die_no_merge_candidates(const char 
*repo, const char **refs
const char *remote = curr_branch ? curr_branch-remote_name : NULL;
 
if (*refspecs) {
-   fprintf_ln(stderr, _(There are no candidates for merging among 
the refs that you just fetched.));
+   if (opt_rebase)
+   fprintf_ln(stderr, _(There is no candidate for 
rebasing against among the refs that you just fetched.));
+   else
+   fprintf_ln(stderr, _(There are no candidates for 
merging among the refs that you just fetched.));
fprintf_ln(stderr, _(Generally this means that you provided a 
wildcard refspec which had no\n
matches on the remote end.));
} else if (repo  curr_branch  (!remote || strcmp(repo, remote))) {
@@ -441,7 +444,10 @@ static void NORETURN die_no_merge_candidates(const char 
*repo, const char **refs
repo);
} else if (!curr_branch) {
fprintf_ln(stderr, _(You are not currently on a branch.));
-   fprintf_ln(stderr, _(Please specify which branch you want to 
merge with.));
+   if (opt_rebase)
+   fprintf_ln(stderr, _(Please specify which branch you 
want to rebase against.));
+   else
+   fprintf_ln(stderr, _(Please specify which branch you 
want to merge with.));
fprintf_ln(stderr, _(See git-pull(1) for details.));
fprintf(stderr, \n);
fprintf_ln(stderr, git pull remote branch);
@@ -453,7 +459,10 @@ static void NORETURN die_no_merge_candidates(const char 
*repo, const char **refs
remote_name = remote;
 
fprintf_ln(stderr, _(There is no tracking information for the 
current branch.));
-   fprintf_ln(stderr, _(Please specify which branch you want to 
merge with.));
+   if (opt_rebase)
+   fprintf_ln(stderr, _(Please specify which branch you 
want to rebase against.));
+   else
+   fprintf_ln(stderr, _(Please specify which branch you 
want to merge with.));
fprintf_ln(stderr, _(See git-pull(1) for details.));
fprintf(stderr, \n);
fprintf_ln(stderr, git pull remote branch);
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 16/19] pull: configure --rebase via branch.name.rebase or pull.rebase

2015-06-18 Thread Paul Tan
Since cd67e4d (Teach 'git pull' about --rebase, 2007-11-28),
fetch+rebase could be set by default by defining the config variable
branch.name.rebase. This setting can be overriden on the command line
by --rebase and --no-rebase.

Since 6b37dff (pull: introduce a pull.rebase option to enable --rebase,
2011-11-06), git-pull --rebase can also be configured via the
pull.rebase configuration option.

Re-implement support for these two configuration settings by introducing
config_get_rebase() which is called before parse_options() to set the
default value of opt_rebase.

Helped-by: Stefan Beller sbel...@google.com
Helped-by: Duy Nguyen pclo...@gmail.com
Signed-off-by: Paul Tan pyoka...@gmail.com
---

Notes:
v4

* Fixed the use-after-free. Thanks Duy for catching it.

 builtin/pull.c | 35 ++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 33a7b98..1c4fb44 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -72,7 +72,7 @@ static int opt_verbosity;
 static char *opt_progress;
 
 /* Options passed to git-merge or git-rebase */
-static enum rebase_type opt_rebase;
+static enum rebase_type opt_rebase = -1;
 static char *opt_diffstat;
 static char *opt_log;
 static char *opt_squash;
@@ -266,6 +266,36 @@ static const char *config_get_ff(void)
 }
 
 /**
+ * Returns the default configured value for --rebase. It first looks for the
+ * value of branch.$curr_branch.rebase, where $curr_branch is the current
+ * branch, and if HEAD is detached or the configuration key does not exist,
+ * looks for the value of pull.rebase. If both configuration keys do not
+ * exist, returns REBASE_FALSE.
+ */
+static enum rebase_type config_get_rebase(void)
+{
+   struct branch *curr_branch = branch_get(HEAD);
+   const char *value;
+
+   if (curr_branch) {
+   char *key = xstrfmt(branch.%s.rebase, curr_branch-name);
+
+   if (!git_config_get_value(key, value)) {
+   enum rebase_type ret = parse_config_rebase(key, value, 
1);
+   free(key);
+   return ret;
+   }
+
+   free(key);
+   }
+
+   if (!git_config_get_value(pull.rebase, value))
+   return parse_config_rebase(pull.rebase, value, 1);
+
+   return REBASE_FALSE;
+}
+
+/**
  * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge
  * into merge_heads.
  */
@@ -707,6 +737,9 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
if (!opt_ff)
opt_ff = xstrdup_or_null(config_get_ff());
 
+   if (opt_rebase  0)
+   opt_rebase = config_get_rebase();
+
git_config(git_default_config, NULL);
 
if (read_cache_unmerged())
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 11/19] pull: check if in unresolved merge state

2015-06-18 Thread Paul Tan
Since d38a30d (Be more user-friendly when refusing to do something
because of conflict., 2010-01-12), git-pull will error out with
user-friendly advices if the user is in the middle of a merge or has
unmerged files.

Re-implement this behavior. While the has unmerged files case can be
handled by die_resolve_conflict(), we introduce a new function
die_conclude_merge() for printing a different error message for when
there are no unmerged files but the merge has not been finished.

Signed-off-by: Paul Tan pyoka...@gmail.com
---
 advice.c   | 8 
 advice.h   | 1 +
 builtin/pull.c | 9 +
 3 files changed, 18 insertions(+)

diff --git a/advice.c b/advice.c
index 575bec2..4965686 100644
--- a/advice.c
+++ b/advice.c
@@ -96,6 +96,14 @@ void NORETURN die_resolve_conflict(const char *me)
die(Exiting because of an unresolved conflict.);
 }
 
+void NORETURN die_conclude_merge(void)
+{
+   error(_(You have not concluded your merge (MERGE_HEAD exists).));
+   if (advice_resolve_conflict)
+   advise(_(Please, commit your changes before you can merge.));
+   die(_(Exiting because of unfinished merge.));
+}
+
 void detach_advice(const char *new_name)
 {
const char fmt[] =
diff --git a/advice.h b/advice.h
index 5ecc6c1..b341a55 100644
--- a/advice.h
+++ b/advice.h
@@ -24,6 +24,7 @@ __attribute__((format (printf, 1, 2)))
 void advise(const char *advice, ...);
 int error_resolve_conflict(const char *me);
 extern void NORETURN die_resolve_conflict(const char *me);
+void NORETURN die_conclude_merge(void);
 void detach_advice(const char *new_name);
 
 #endif /* ADVICE_H */
diff --git a/builtin/pull.c b/builtin/pull.c
index b61cff5..1e688be 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -12,6 +12,7 @@
 #include run-command.h
 #include sha1-array.h
 #include remote.h
+#include dir.h
 
 static const char * const pull_usage[] = {
N_(git pull [options] [repository [refspec...]]),
@@ -426,6 +427,14 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
if (!opt_ff)
opt_ff = xstrdup_or_null(config_get_ff());
 
+   git_config(git_default_config, NULL);
+
+   if (read_cache_unmerged())
+   die_resolve_conflict(Pull);
+
+   if (file_exists(git_path(MERGE_HEAD)))
+   die_conclude_merge();
+
if (run_fetch(repo, refspecs))
return 1;
 
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Using clean/smudge filters with difftool

2015-06-18 Thread Florian Aspart
2015-06-18 16:11 GMT+02:00 John Keeping j...@keeping.me.uk:
 On Thu, Jun 18, 2015 at 03:51:25PM +0200, Florian Aspart wrote:
 2015-06-18 15:26 GMT+02:00 John Keeping j...@keeping.me.uk:
  [Please don't top-post on this list.]
 
  On Thu, Jun 18, 2015 at 03:15:38PM +0200, Florian Aspart wrote:
  2015-06-18 14:31 GMT+02:00 Michael J Gruber g...@drmicha.warpmail.net:
   Florian Aspart venit, vidit, dixit 16.06.2015 16:11:
   Hi everyone,
  
   I created a clean filter to apply on some files before commiting them.
   The filter works correctly when I commit the file and is also applied
   when I usethe iff command line tool.
   However, when using difftool with meld, the filter is not applied and
   the different versions of the files are compared without any
   filtering.
  
   Is there a way to apply the clean/smudge filters when comparing the
   working copy of a file to the HEAD version in a gui diff tool?
  
   I'm using git version 2.4.3 under Ubuntu.
  
   Best,
   Florian
  
   Are you saying that difftool compares an uncleaned working tree file
   with a cleaned blob? That would be a bug in either difftool or the way
   we feed difftool.
  
  yes in this case difftool compares an uncleaned working tree file
  with a cleaned blob. I did not try the smudge filter to see if it
  applied in difftool.
 
  I think the problem comes from the way difftool is feeded, since I
  also had this problem when setting an external tool for the diff in
  the gitconfig file.
 
  However, I'm not sure if this is a bug or it is designed to be so.
  If the external tool changes a cleaned working tree file during the
  diff, then by saving this file the result of the cleaning filter would
  also be saved in the working tree.
 
  How is your filter configured?  Is it using a simple pattern (e.g.
  *.c) or is it using a file path?
 
  git-difftool uses `git checkout-index --all --prefix=$dir/` and I wonder
  if the prefix means that the attribute specification does not match the
  temporary file that difftool produces, so no filter is applied.

 It is using a simple pattern:
 *.ipynb filter=clean_ipynb

 I also realised that the code for file diff is very different from
 directory diff do you see any difference between git-difftool acting on
 files and with the `--dir-diff` option?

No, even with the --dir-diff option, the filter is still not applied.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Using clean/smudge filters with difftool

2015-06-18 Thread Florian Aspart
2015-06-18 16:28 GMT+02:00 John Keeping j...@keeping.me.uk:
 On Thu, Jun 18, 2015 at 04:17:52PM +0200, Florian Aspart wrote:
 2015-06-18 16:11 GMT+02:00 John Keeping j...@keeping.me.uk:
  On Thu, Jun 18, 2015 at 03:51:25PM +0200, Florian Aspart wrote:
  2015-06-18 15:26 GMT+02:00 John Keeping j...@keeping.me.uk:
   [Please don't top-post on this list.]
  
   On Thu, Jun 18, 2015 at 03:15:38PM +0200, Florian Aspart wrote:
   2015-06-18 14:31 GMT+02:00 Michael J Gruber 
   g...@drmicha.warpmail.net:
Florian Aspart venit, vidit, dixit 16.06.2015 16:11:
I created a clean filter to apply on some files before commiting 
them.
The filter works correctly when I commit the file and is also 
applied
when I usethe iff command line tool.
However, when using difftool with meld, the filter is not applied 
and
the different versions of the files are compared without any
filtering.
   
Is there a way to apply the clean/smudge filters when comparing the
working copy of a file to the HEAD version in a gui diff tool?
   
I'm using git version 2.4.3 under Ubuntu.
 
  I also realised that the code for file diff is very different from
  directory diff do you see any difference between git-difftool acting on
  files and with the `--dir-diff` option?

 No, even with the --dir-diff option, the filter is still not applied.

 I have tried to reproduce this and it works as expected for me (i.e. the
 filter is applied) both for file diff and directory diff mode:

 $ git config filter.quote.clean sed -e 's/^ //'
 $ git config filter.quote.smudge sed -e '/^ /n; s/^/ /'
 $ git config filter.quote.required true

 $ echo '*.quote filter=quote' .gitattributes
 $ cat 1.quote EOF
 one
 two
 three
 EOF
 $ git add .gitattributes 1.quote
 $ git commit -m 'Initial commit'
 $ echo four 1.quote

 Now `git-difftool` shows the differences with the filter applied.  This can be
 seen running with GIT_TRACE:

 $ GIT_TRACE=2 git difftool
 15:26:59.211541 git.c:557   trace: exec: 'git-difftool'
 15:26:59.211674 run-command.c:347   trace: run_command: 'git-difftool'
 15:26:59.338617 git.c:348   trace: built-in: git 'config' 
 '--bool' '--get' 'difftool.trustExitCode'
 15:26:59.342664 git.c:348   trace: built-in: git 'diff'
 15:26:59.344857 run-command.c:347   trace: run_command: 'sed -e '\''s/^ 
 //'\'''
 15:26:59.345383 run-command.c:195   trace: exec: '/bin/sh' '-c' 'sed -e 
 '\''s/^ //'\''' 'sed -e '\''s/^ //'\'''
 15:26:59.351077 run-command.c:347   trace: run_command: 'sed -e '\''/^ 
 /n; s/^/ /'\'''
 15:26:59.351605 run-command.c:195   trace: exec: '/bin/sh' '-c' 'sed -e 
 '\''/^ /n; s/^/ /'\''' 'sed -e '\''/^ /n; s/^/ /'\'''
 15:26:59.355716 run-command.c:347   trace: run_command: 
 'git-difftool--helper' '1.quote' '/tmp/SUEySx_1.quote' 
 '4cb29ea38f70d7c61b2a3a25b02e3bdf44905402' '100644' '1.quote' 
 '' '100644'
 15:26:59.356191 run-command.c:195   trace: exec: 'git-difftool--helper' 
 '1.quote' '/tmp/SUEySx_1.quote' '4cb29ea38f70d7c61b2a3a25b02e3bdf44905402' 
 '100644' '1.quote' '' '100644'
 15:26:59.370468 git.c:348   trace: built-in: git 'config' 
 'diff.tool'
 15:26:59.373485 git.c:348   trace: built-in: git 'config' 
 'merge.tool'
 15:26:59.378402 git.c:348   trace: built-in: git 'config' 
 'difftool.vimdiff.cmd'
 15:26:59.381424 git.c:348   trace: built-in: git 'config' 
 'mergetool.vimdiff.cmd'
 15:26:59.386623 git.c:348   trace: built-in: git 'config' 
 '--bool' 'mergetool.prompt'
 15:26:59.390198 git.c:348   trace: built-in: git 'config' 
 '--bool' 'difftool.prompt'

 I think the first run_command of `sed` is cleaning the working tree file
 to figure out *if* it differs, then the second `sed` is smudging the
 version in the index so that difftool can use it.

I'm not really understanding what your filter is doing, but I tried
your code on my machine and I get a different result when using diff
and difftool on my machine.

The diff results give me:

diff --git a/1.quote b/1.quote
index 4cb29ea..f384549 100644
--- a/1.quote
+++ b/1.quote
@@ -1,3 +1,4 @@
 one
 two
 three
+four

While the diff tool tells me that the repository file is:
 one
 two
 three

and my working copy:
one
two
three
four

In both case the the filters are called twice (cf GIT_TRACE) as in the
example your wrote.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Git completion not using ls-remote to auto-complete during push

2015-06-18 Thread Robert Dailey
On Thu, Jun 18, 2015 at 6:29 AM, SZEDER Gábor sze...@ira.uka.de wrote:
 Quoting Robert Dailey rcdailey.li...@gmail.com
 I do the following:

 $ git push origin :topic

 If I stop halfway through typing 'topic' and hit TAB, auto-completion
 does not work if I do not have a local branch by that name (sometimes
 I delete my local branch first, then I push to delete it remotely). I
 thought that git completion code was supposed to use ls-remote to auto
 complete refs used in push operations. Is this supposed to work?

 It's intentional.  Running 'git ls-remote' with a far away remote can
 take ages, so instead we grab the refs on the remote from the locally
 stored refs under 'refs/remotes/remote/'.

 See e832f5c096 (completion: avoid ls-remote in certain scenarios,
 2013-05-28).  The commit message mentions that you can force
 completion of remote refs via 'git ls-remote' by starting with the full
 refname, i.e.  'refs/TAB', however, that seems to work only on the
 left hand side of the colon in the push refspec.

 Gábor


If that's indeed the case, then completion should work. I have a
'refs/remotes/origin/topic'. Why will auto complete not work even
though this exists? Do multiple remotes cause issues (in theory there
is no reason why it should cause problems, since it should know I'm
auto-completing a ref on 'origin')?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Bug report: `git revert` on empty commit fails silently

2015-06-18 Thread Alistair Lynn
Hello gitters

Steps to reproduce:

$ git commit --allow-empty -m ‘test’
$ git revert HEAD

The latter fails silently, leaving HEAD pointing at the commit created by the 
first command.

A subsequent `git commit --allow-empty` has the revert message as the default 
commit message when starting the editor.

Hope this is the right place for bugs.

Alistair

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] mergetools: add config option to disable auto-merge

2015-06-18 Thread Mike Rappazzo
On Thu, Jun 18, 2015 at 4:43 AM David Aguilar dav...@gmail.com wrote:

 On Wed, Jun 17, 2015 at 10:27:58PM -0400, Mike Rappazzo wrote:
 
  I feel that the auto-merge takes away the context of the changes.
 
  I use araxis merge as my mergetool of choice.  I almost always immediately
  undo the auto-merge.  After taking a moment to look at each file, I will
  then (usually) use the keyboard shortcut for auto-merge.
 
  It sure would be nice to set-and-forget a config variable to remove the
  annoyance of having to undo the auto-merge.  I think that this config
  option is reasonable.  Perhaps my documentation leaves something to be
  desired.  I can take another stab at it.

 If this is the case then I would recommend making it more
 granular.  Just because Araxis' automerge is undesirable does
 not mean that some other tools' automerge is as well.
 e.g. the config variable could be mergetool.tool.automerge
 rather than the top-level mergetool.automerge variable.

I don't necessarily think that araxis' automerge is bad, but I like to look
at the before and after to understand the context of a conflict.  I can't
imagine that this is a quirk of araxis, but is probably something that
exists for any auto-merging tool.  The feature doesn't seem to be that
widely supported among the other tooling.  I only found the three to use
such a feature.

Since the automerge option is not available on every merge tool, it seems
reasonable to use mergetool.tool.automerge instead of merge.automerge.



 But, as Junio suggested, having a command-line flag to skip the
 behavior might be a better first step.  Something like,
 git mergetool --no-automerge.

 Most of Git's behavior that can be modified via configuration
 can also be modified on the command-line, so exposing this
 feature as a flag would probably be a good idea.


This makes sense, and if this change is to go forward, I will implement the
command line option.


 Even without a config variable, it can still be fire-and-forget
 convenient by using a git alias to supply the flag.

 In lieu of any of these features, another option is that you can
 override the default command by setting mergetool.araxis.cmd,
 and git mergetool will use your definition rather than its
 built-in command.  We left that escape hatch in for just this
 purpose.

I guess that if this patch does not go forward, I will have to use this
workaround.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Using clean/smudge filters with difftool

2015-06-18 Thread John Keeping
On Thu, Jun 18, 2015 at 04:17:52PM +0200, Florian Aspart wrote:
 2015-06-18 16:11 GMT+02:00 John Keeping j...@keeping.me.uk:
  On Thu, Jun 18, 2015 at 03:51:25PM +0200, Florian Aspart wrote:
  2015-06-18 15:26 GMT+02:00 John Keeping j...@keeping.me.uk:
   [Please don't top-post on this list.]
  
   On Thu, Jun 18, 2015 at 03:15:38PM +0200, Florian Aspart wrote:
   2015-06-18 14:31 GMT+02:00 Michael J Gruber g...@drmicha.warpmail.net:
Florian Aspart venit, vidit, dixit 16.06.2015 16:11:
I created a clean filter to apply on some files before commiting 
them.
The filter works correctly when I commit the file and is also applied
when I usethe iff command line tool.
However, when using difftool with meld, the filter is not applied and
the different versions of the files are compared without any
filtering.
   
Is there a way to apply the clean/smudge filters when comparing the
working copy of a file to the HEAD version in a gui diff tool?
   
I'm using git version 2.4.3 under Ubuntu.
  
  I also realised that the code for file diff is very different from
  directory diff do you see any difference between git-difftool acting on
  files and with the `--dir-diff` option?
 
 No, even with the --dir-diff option, the filter is still not applied.

I have tried to reproduce this and it works as expected for me (i.e. the
filter is applied) both for file diff and directory diff mode:

$ git config filter.quote.clean sed -e 's/^ //'
$ git config filter.quote.smudge sed -e '/^ /n; s/^/ /'
$ git config filter.quote.required true

$ echo '*.quote filter=quote' .gitattributes
$ cat 1.quote EOF
one
two
three
EOF
$ git add .gitattributes 1.quote
$ git commit -m 'Initial commit'
$ echo four 1.quote

Now `git-difftool` shows the differences with the filter applied.  This can be
seen running with GIT_TRACE:

$ GIT_TRACE=2 git difftool
15:26:59.211541 git.c:557   trace: exec: 'git-difftool'
15:26:59.211674 run-command.c:347   trace: run_command: 'git-difftool'
15:26:59.338617 git.c:348   trace: built-in: git 'config' '--bool' 
'--get' 'difftool.trustExitCode'
15:26:59.342664 git.c:348   trace: built-in: git 'diff'
15:26:59.344857 run-command.c:347   trace: run_command: 'sed -e '\''s/^ 
//'\'''
15:26:59.345383 run-command.c:195   trace: exec: '/bin/sh' '-c' 'sed -e 
'\''s/^ //'\''' 'sed -e '\''s/^ //'\'''
15:26:59.351077 run-command.c:347   trace: run_command: 'sed -e '\''/^ /n; 
s/^/ /'\'''
15:26:59.351605 run-command.c:195   trace: exec: '/bin/sh' '-c' 'sed -e 
'\''/^ /n; s/^/ /'\''' 'sed -e '\''/^ /n; s/^/ /'\'''
15:26:59.355716 run-command.c:347   trace: run_command: 
'git-difftool--helper' '1.quote' '/tmp/SUEySx_1.quote' 
'4cb29ea38f70d7c61b2a3a25b02e3bdf44905402' '100644' '1.quote' 
'' '100644'
15:26:59.356191 run-command.c:195   trace: exec: 'git-difftool--helper' 
'1.quote' '/tmp/SUEySx_1.quote' '4cb29ea38f70d7c61b2a3a25b02e3bdf44905402' 
'100644' '1.quote' '' '100644'
15:26:59.370468 git.c:348   trace: built-in: git 'config' 
'diff.tool'
15:26:59.373485 git.c:348   trace: built-in: git 'config' 
'merge.tool'
15:26:59.378402 git.c:348   trace: built-in: git 'config' 
'difftool.vimdiff.cmd'
15:26:59.381424 git.c:348   trace: built-in: git 'config' 
'mergetool.vimdiff.cmd'
15:26:59.386623 git.c:348   trace: built-in: git 'config' '--bool' 
'mergetool.prompt'
15:26:59.390198 git.c:348   trace: built-in: git 'config' '--bool' 
'difftool.prompt'

I think the first run_command of `sed` is cleaning the working tree file
to figure out *if* it differs, then the second `sed` is smudging the
version in the index so that difftool can use it.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Using clean/smudge filters with difftool

2015-06-18 Thread John Keeping
On Thu, Jun 18, 2015 at 03:51:25PM +0200, Florian Aspart wrote:
 2015-06-18 15:26 GMT+02:00 John Keeping j...@keeping.me.uk:
  [Please don't top-post on this list.]
 
  On Thu, Jun 18, 2015 at 03:15:38PM +0200, Florian Aspart wrote:
  2015-06-18 14:31 GMT+02:00 Michael J Gruber g...@drmicha.warpmail.net:
   Florian Aspart venit, vidit, dixit 16.06.2015 16:11:
   Hi everyone,
  
   I created a clean filter to apply on some files before commiting them.
   The filter works correctly when I commit the file and is also applied
   when I usethe iff command line tool.
   However, when using difftool with meld, the filter is not applied and
   the different versions of the files are compared without any
   filtering.
  
   Is there a way to apply the clean/smudge filters when comparing the
   working copy of a file to the HEAD version in a gui diff tool?
  
   I'm using git version 2.4.3 under Ubuntu.
  
   Best,
   Florian
  
   Are you saying that difftool compares an uncleaned working tree file
   with a cleaned blob? That would be a bug in either difftool or the way
   we feed difftool.
  
  yes in this case difftool compares an uncleaned working tree file
  with a cleaned blob. I did not try the smudge filter to see if it
  applied in difftool.
 
  I think the problem comes from the way difftool is feeded, since I
  also had this problem when setting an external tool for the diff in
  the gitconfig file.
 
  However, I'm not sure if this is a bug or it is designed to be so.
  If the external tool changes a cleaned working tree file during the
  diff, then by saving this file the result of the cleaning filter would
  also be saved in the working tree.
 
  How is your filter configured?  Is it using a simple pattern (e.g.
  *.c) or is it using a file path?
 
  git-difftool uses `git checkout-index --all --prefix=$dir/` and I wonder
  if the prefix means that the attribute specification does not match the
  temporary file that difftool produces, so no filter is applied.
 
 It is using a simple pattern:
 *.ipynb filter=clean_ipynb

I also realised that the code for file diff is very different from
directory diff do you see any difference between git-difftool acting on
files and with the `--dir-diff` option?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] t3404: demonstrate CHERRY_PICK_HEAD bug

2015-06-18 Thread Johannes Schindelin
Hi Junio,

On 2015-06-18 18:00, Junio C Hamano wrote:
 Johannes Schindelin johannes.schinde...@gmx.de writes:
 
 +  git diff seq-onto 

 I am puzzled with this diff; what is this about?  Is it a remnant
 from an earlier debugging session, or is it making sure seq-onto is
 a valid tree-ish?

 The idea is to verify that we end up with the same tree even if we
 exchanged the latest two patches. I can remove it if you want as it is
 not strictly necessary, but I would like to keep it just to make sure
 that we did not end up with an incomplete rebase.
 
 I agree that such a verification is a very good thing to have here.
 But you would need to ask git diff to signal that it found no
 differences with --exit-code or --quiet, I would think.
 
 Thanks.

Whoops! Of course... You want me to re-roll?

Ciao,
Dscho
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Visualizing merge conflicts after the fact (using kdiff3)

2015-06-18 Thread Junio C Hamano
Michael J Gruber g...@drmicha.warpmail.net writes:

 This type of request comes up often (for a reason). I'm wondering
 whether we could support it more systematically, either by exposing the
 steps above as a command, or by storing the unresolved merge somewhere
 (leveraging stash or rerere).

Perhaps 'tr/remerge-diff' (on 'pu') is of interest?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Improve contrib/diff-highlight to highlight unevenly-sized hunks

2015-06-18 Thread Patrick Palka
On Thu, Jun 18, 2015 at 11:50 AM, Junio C Hamano gits...@pobox.com wrote:
 Patrick Palka patr...@parcs.ath.cx writes:

 Currently the diff-highlight script does not try to highlight hunks that
 have different numbers of removed/added lines.  But we can be a little
 smarter than that, without introducing much magic and complexity.

 In the case of unevenly-sized hunks, we could still highlight the first
 few (lexicographical) add/remove pairs.  It is not uncommon for hunks to
 have common prefixes, and in such a case this change is very useful
 for spotting differences.

 Signed-off-by: Patrick Palka patr...@parcs.ath.cx
 ---

 Patrick, git shortlog --no-merges contrib/diff-highlight/ is your
 friend to see who may be able to give you a good feedback.

Sorry about that.  I admit the sending of this patch was rushed for no
good reason.


 Jeff, what do you think?

 I have this nagging feeling that it is just as likely that two
 uneven hunks align at the top as they align at the bottom, so while
 this might not hurt it may not be the right approach for a better
 solution, in the sense that when somebody really wants to do a
 better solution, this change and the original code may need to be
 ripped out and redone from scratch.

Hmm, maybe. I stuck with assuming hunks are top-aligned because it
required less code to implement :)

The benefits of a simple dumb solution like assuming hunks align at
the top or bottom is that it remains very easy to visually match up
each highlighted deleted slice with its corresponding highlighted
added slice. If we start matching up similar lines or something like
that then it seems we would have to mostly forsake this benefit.  A
stupid algorithm in this case is nice because its output is
predictable.

A direct improvement upon this patch that would not require redoing
the whole script from scratch would be to first to calculate the
highlighting assuming the hunk aligns at the top, then to calculate
the highlighting assuming the hunk aligns at the bottom, and to pick
out of the two the highlighting with the least noise.  Though we
would still be out of luck if the hunk is more complicated than being
top-aligned or bottom-aligned.

By the way, what would it take to get something like this script into
git proper?  It is IMHO immensely useful even in its current form, yet
because it's not baked into the application hardly anybody knows about
it.


  contrib/diff-highlight/diff-highlight | 26 +-
  1 file changed, 17 insertions(+), 9 deletions(-)

 diff --git a/contrib/diff-highlight/diff-highlight 
 b/contrib/diff-highlight/diff-highlight
 index ffefc31..0dfbebd 100755
 --- a/contrib/diff-highlight/diff-highlight
 +++ b/contrib/diff-highlight/diff-highlight
 @@ -88,22 +88,30 @@ sub show_hunk {
   return;
   }

 - # If we have mismatched numbers of lines on each side, we could try to
 - # be clever and match up similar lines. But for now we are simple and
 - # stupid, and only handle multi-line hunks that remove and add the same
 - # number of lines.
 - if (@$a != @$b) {
 - print @$a, @$b;
 - return;
 - }
 + # We match up the first MIN(a, b) lines on each side.
 + my $c = @$a  @$b ? @$a : @$b;

 + # Highlight each pair, and print each removed line of that pair.
   my @queue;
 - for (my $i = 0; $i  @$a; $i++) {
 + for (my $i = 0; $i  $c; $i++) {
   my ($rm, $add) = highlight_pair($a-[$i], $b-[$i]);
   print $rm;
   push @queue, $add;
   }
 +
 + # Print the remaining unmatched removed lines of the hunk.
 + for (my $i = $c; $i  @$a; $i++) {
 + print $a-[$i];
 + }
 +
 + # Print the added lines of each highlighted pair.
   print @queue;
 +
 + # Print the remaining unmatched added lines of the hunk.
 + for (my $i = $c; $i  @$b; $i++) {
 + print $b-[$i];
 + }
 +
  }

  sub highlight_pair {
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 0/2] rebase -i: Fix left-behind CHERRY_PICK_HEAD

2015-06-18 Thread Johannes Schindelin
These patches fix a bug that bites me often enough when rebasing Git for
Windows.

The symptom is that .git/CHERRY_PICK_HEAD is left behind after skipping
an already-merged patch with `git rebase --continue` instead of `git
rebase --skip`. I always prefer the former invocation because the latter
would also skip legitimate patches if there were merge conflicts, while
the former would not allow that.

Changes since v2:

- the test uses `--exit-code` to verify that the result of the rebase is
  correct

Interdiff below the diffstat.

Johannes Schindelin (2):
  t3404: demonstrate CHERRY_PICK_HEAD bug
  rebase -i: do not leave a CHERRY_PICK_HEAD file behind

 git-rebase--interactive.sh|  6 +-
 t/t3404-rebase-interactive.sh | 21 +
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index f3337ad..6bcf18b 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1118,7 +1118,7 @@ test_expect_success 'rebase --continue removes 
CHERRY_PICK_HEAD' '
FAKE_LINES= test_must_fail git rebase -i seq-onto 
test -d .git/rebase-merge 
git rebase --continue 
-   git diff seq-onto 
+   git diff --exit-code seq-onto 
test ! -d .git/rebase-merge 
test ! -f .git/CHERRY_PICK_HEAD
 '
-- 
2.3.1.windows.1.9.g8c01ab4


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 1/2] t3404: demonstrate CHERRY_PICK_HEAD bug

2015-06-18 Thread Johannes Schindelin
When rev-list's --cherry option does not detect that a patch has already
been applied upstream, an interactive rebase would offer to reapply it and
consequently stop at that patch with a failure, mentioning that the diff
is empty.

Traditionally, a `git rebase --continue` simply skips the commit in such a
situation.

However, as pointed out by Gábor Szeder, this leaves a CHERRY_PICK_HEAD
behind, making the Git prompt believe that a cherry pick is still going
on. This commit adds a test case demonstrating this bug.

Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
---
 t/t3404-rebase-interactive.sh | 21 +
 1 file changed, 21 insertions(+)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index ac429a0..6fe6c47 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1102,4 +1102,25 @@ test_expect_success 'rebase -i commits that overwrite 
untracked files (no ff)' '
test $(git cat-file commit HEAD | sed -ne \$p) = I
 '
 
+test_expect_failure 'rebase --continue removes CHERRY_PICK_HEAD' '
+   git checkout -b commit-to-skip 
+   for double in X 3 1
+   do
+   seq 5 | sed s/$double// seq 
+   git add seq 
+   test_tick 
+   git commit -m seq-$double
+   done 
+   git tag seq-onto 
+   git reset --hard HEAD~2 
+   git cherry-pick seq-onto 
+   set_fake_editor 
+   FAKE_LINES= test_must_fail git rebase -i seq-onto 
+   test -d .git/rebase-merge 
+   git rebase --continue 
+   git diff --exit-code seq-onto 
+   test ! -d .git/rebase-merge 
+   test ! -f .git/CHERRY_PICK_HEAD
+'
+
 test_done
-- 
2.3.1.windows.1.9.g8c01ab4



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 2/2] rebase -i: do not leave a CHERRY_PICK_HEAD file behind

2015-06-18 Thread Johannes Schindelin
When skipping commits whose changes were already applied via `git rebase
--continue`, we need to clean up said file explicitly.

The same is not true for `git rebase --skip` because that will execute
`git reset --hard` as part of the skip handling in git-rebase.sh, even
before git-rebase--interactive.sh is called.

Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
---
 git-rebase--interactive.sh| 6 +-
 t/t3404-rebase-interactive.sh | 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index dc3133f..5ff0f1c 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -849,7 +849,11 @@ continue)
# do we have anything to commit?
if git diff-index --cached --quiet HEAD --
then
-   : Nothing to commit -- skip this
+   # Nothing to commit -- skip this commit
+
+   test ! -f $GIT_DIR/CHERRY_PICK_HEAD ||
+   rm $GIT_DIR/CHERRY_PICK_HEAD ||
+   die Could not remove CHERRY_PICK_HEAD
else
if ! test -f $author_script
then
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 6fe6c47..6bcf18b 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1102,7 +1102,7 @@ test_expect_success 'rebase -i commits that overwrite 
untracked files (no ff)' '
test $(git cat-file commit HEAD | sed -ne \$p) = I
 '
 
-test_expect_failure 'rebase --continue removes CHERRY_PICK_HEAD' '
+test_expect_success 'rebase --continue removes CHERRY_PICK_HEAD' '
git checkout -b commit-to-skip 
for double in X 3 1
do
-- 
2.3.1.windows.1.9.g8c01ab4


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Using clean/smudge filters with difftool

2015-06-18 Thread John Keeping
On Thu, Jun 18, 2015 at 05:39:18PM +0200, Florian Aspart wrote:
 2015-06-18 16:28 GMT+02:00 John Keeping j...@keeping.me.uk:
  On Thu, Jun 18, 2015 at 04:17:52PM +0200, Florian Aspart wrote:
  2015-06-18 16:11 GMT+02:00 John Keeping j...@keeping.me.uk:
   On Thu, Jun 18, 2015 at 03:51:25PM +0200, Florian Aspart wrote:
   2015-06-18 15:26 GMT+02:00 John Keeping j...@keeping.me.uk:
[Please don't top-post on this list.]
   
On Thu, Jun 18, 2015 at 03:15:38PM +0200, Florian Aspart wrote:
2015-06-18 14:31 GMT+02:00 Michael J Gruber 
g...@drmicha.warpmail.net:
 Florian Aspart venit, vidit, dixit 16.06.2015 16:11:
 I created a clean filter to apply on some files before commiting 
 them.
 The filter works correctly when I commit the file and is also 
 applied
 when I usethe iff command line tool.
 However, when using difftool with meld, the filter is not applied 
 and
 the different versions of the files are compared without any
 filtering.

 Is there a way to apply the clean/smudge filters when comparing 
 the
 working copy of a file to the HEAD version in a gui diff tool?

 I'm using git version 2.4.3 under Ubuntu.
  
   I also realised that the code for file diff is very different from
   directory diff do you see any difference between git-difftool acting on
   files and with the `--dir-diff` option?
 
  No, even with the --dir-diff option, the filter is still not applied.
 
  I have tried to reproduce this and it works as expected for me (i.e. the
  filter is applied) both for file diff and directory diff mode:
 
  $ git config filter.quote.clean sed -e 's/^ //'
  $ git config filter.quote.smudge sed -e '/^ /n; s/^/ /'
  $ git config filter.quote.required true
 
  $ echo '*.quote filter=quote' .gitattributes
  $ cat 1.quote EOF
  one
  two
  three
  EOF
  $ git add .gitattributes 1.quote
  $ git commit -m 'Initial commit'
  $ echo four 1.quote
 
  Now `git-difftool` shows the differences with the filter applied.  This can 
  be
  seen running with GIT_TRACE:
 
  $ GIT_TRACE=2 git difftool
  15:26:59.211541 git.c:557   trace: exec: 'git-difftool'
  15:26:59.211674 run-command.c:347   trace: run_command: 'git-difftool'
  15:26:59.338617 git.c:348   trace: built-in: git 'config' 
  '--bool' '--get' 'difftool.trustExitCode'
  15:26:59.342664 git.c:348   trace: built-in: git 'diff'
  15:26:59.344857 run-command.c:347   trace: run_command: 'sed -e 
  '\''s/^ //'\'''
  15:26:59.345383 run-command.c:195   trace: exec: '/bin/sh' '-c' 'sed -e 
  '\''s/^ //'\''' 'sed -e '\''s/^ //'\'''
  15:26:59.351077 run-command.c:347   trace: run_command: 'sed -e '\''/^ 
  /n; s/^/ /'\'''
  15:26:59.351605 run-command.c:195   trace: exec: '/bin/sh' '-c' 'sed -e 
  '\''/^ /n; s/^/ /'\''' 'sed -e '\''/^ /n; s/^/ /'\'''
  15:26:59.355716 run-command.c:347   trace: run_command: 
  'git-difftool--helper' '1.quote' '/tmp/SUEySx_1.quote' 
  '4cb29ea38f70d7c61b2a3a25b02e3bdf44905402' '100644' '1.quote' 
  '' '100644'
  15:26:59.356191 run-command.c:195   trace: exec: 'git-difftool--helper' 
  '1.quote' '/tmp/SUEySx_1.quote' '4cb29ea38f70d7c61b2a3a25b02e3bdf44905402' 
  '100644' '1.quote' '' '100644'
  15:26:59.370468 git.c:348   trace: built-in: git 'config' 
  'diff.tool'
  15:26:59.373485 git.c:348   trace: built-in: git 'config' 
  'merge.tool'
  15:26:59.378402 git.c:348   trace: built-in: git 'config' 
  'difftool.vimdiff.cmd'
  15:26:59.381424 git.c:348   trace: built-in: git 'config' 
  'mergetool.vimdiff.cmd'
  15:26:59.386623 git.c:348   trace: built-in: git 'config' 
  '--bool' 'mergetool.prompt'
  15:26:59.390198 git.c:348   trace: built-in: git 'config' 
  '--bool' 'difftool.prompt'
 
  I think the first run_command of `sed` is cleaning the working tree file
  to figure out *if* it differs, then the second `sed` is smudging the
  version in the index so that difftool can use it.
 
 I'm not really understanding what your filter is doing, but I tried
 your code on my machine and I get a different result when using diff
 and difftool on my machine.

It's supposed to be adding   to the beginning of lines in the working
tree and stripping them in the repo, but I've just realised that the
instructions above are wrong since 1.quote is supposed to have leading
 's (although the test I ran before tidying it up for the email was
correct).

 The diff results give me:
 
 diff --git a/1.quote b/1.quote
 index 4cb29ea..f384549 100644
 --- a/1.quote
 +++ b/1.quote
 @@ -1,3 +1,4 @@
  one
  two
  three
 +four

It seems that `git diff` shows the difference between the clean versions
of the files, which is also shown by the GIT_TRACE output when running
it.

 While the diff tool tells me that the repository file is:
  one
  two
  three

This indicates that 

Re: Git completion not using ls-remote to auto-complete during push

2015-06-18 Thread SZEDER Gábor


Quoting Robert Dailey rcdailey.li...@gmail.com:


On Thu, Jun 18, 2015 at 6:29 AM, SZEDER Gábor sze...@ira.uka.de wrote:

Quoting Robert Dailey rcdailey.li...@gmail.com

I do the following:

$ git push origin :topic

If I stop halfway through typing 'topic' and hit TAB, auto-completion
does not work if I do not have a local branch by that name (sometimes
I delete my local branch first, then I push to delete it remotely). I
thought that git completion code was supposed to use ls-remote to auto
complete refs used in push operations. Is this supposed to work?


It's intentional.  Running 'git ls-remote' with a far away remote can
take ages, so instead we grab the refs on the remote from the locally
stored refs under 'refs/remotes/remote/'.

See e832f5c096 (completion: avoid ls-remote in certain scenarios,
2013-05-28).  The commit message mentions that you can force
completion of remote refs via 'git ls-remote' by starting with the full
refname, i.e.  'refs/TAB', however, that seems to work only on the
left hand side of the colon in the push refspec.

Gábor



If that's indeed the case, then completion should work. I have a
'refs/remotes/origin/topic'. Why will auto complete not work even
though this exists? Do multiple remotes cause issues (in theory there
is no reason why it should cause problems, since it should know I'm
auto-completing a ref on 'origin')?


The number of remotes doesn't matter.
What matters is which side of the colon the ref to be completed is.

You can complete

  git push origin refs/TAB

and

  git fetch origin refs/TAB

will even list you refs freshly queried via 'git ls-remote'.
However,

  git push origin :refs/TAB
  git push origin branch:refs/TAB

don't work, because there are no refs starting with the prefix  
':refs/' or 'branch:refs/'.


Gábor

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] t3404: demonstrate CHERRY_PICK_HEAD bug

2015-06-18 Thread Junio C Hamano
Johannes Schindelin johannes.schinde...@gmx.de writes:

 +   git diff seq-onto 
 
 I am puzzled with this diff; what is this about?  Is it a remnant
 from an earlier debugging session, or is it making sure seq-onto is
 a valid tree-ish?

 The idea is to verify that we end up with the same tree even if we
 exchanged the latest two patches. I can remove it if you want as it is
 not strictly necessary, but I would like to keep it just to make sure
 that we did not end up with an incomplete rebase.

I agree that such a verification is a very good thing to have here.
But you would need to ask git diff to signal that it found no
differences with --exit-code or --quiet, I would think.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] format-patch: introduce format.outputDirectory configuration

2015-06-18 Thread Junio C Hamano
Alexander Kuleshov kuleshovm...@gmail.com writes:

 diff --git a/Documentation/config.txt b/Documentation/config.txt
 index fd2036c..8f6f7ed 100644
 --- a/Documentation/config.txt
 +++ b/Documentation/config.txt
 @@ -1247,6 +1247,10 @@ format.coverLetter::
   format-patch is invoked, but in addition can be set to auto, to
   generate a cover-letter only when there's more than one patch.
  
 +format.outputDirectory::
 + Set a custom directory to store the resulting files instead of the
 + current working directory.
 +

After you set this configuration variable, how would you override it
and get the default behaviour back from the command line for one
time invocation?  -o ./?  That needs to be documented somewhere.

Documentation/format-patch.txt must have description on -o; that
paragraph needs to mention this new configuration variable, and it
would be a good place to document the -o ./ workaround.

 -static const char *set_outdir(const char *prefix, const char 
 *output_directory)
 +static const char *set_outdir(const char *prefix, const char 
 *output_directory,
 +   const char *config_output_directory)

This change looks ugly and unnecessary.  All the machinery after and
including the point set_outdir() is called, including reopen_stdout(),
work on output_directory variable and only that variable.

Wouldn't it work equally well to have

if (!output_directory)
output_directory = config_output_directory;

before a call to set_outdir() is made but after the configuration is
read (namely, soon after parse_options() returns), without making
any change to this function?

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] request-pull: short sha handling, manual update

2015-06-18 Thread Petr Stodulka

Hi folks,
can you someone look at it? Or were these troubles mentioned somewhere 
earlier and I miss that?


On 2.6.2015 16:14, Petr Stodulka wrote:
request-pull prints incorrectly warn messages about not found commits 
and man pages don't say
anything about todays changed behaviour. People are confused and try 
look for errors at wrong places.

At least these should be fixed/modified.

Warn massage says that commit can't be found ar remote, however there 
it is in and is missing on local repository
in 'many' cases. So I don't know if better solution is check, where 
commit is truly missing or transform warning message.

Something like:

   warn: No match for commit commit found at url or local repository.
   warn: Are you sure you have synchronized branch with remote 
repository?


..
man page could be changed like this:

---
diff --git a/Documentation/git-request-pull.txt 
b/Documentation/git-request-pull.txt

index 283577b..6d34fc7 100644
--- a/Documentation/git-request-pull.txt
+++ b/Documentation/git-request-pull.txt
@@ -73,6 +73,17 @@ then you can ask that to be pulled with
git request-pull v1.0 https://git.ko.xz/project master:for-linus


+NOTES
+-
+
+Since git version 2.0.0 is behaviour of git request-pull little 
different.

+It is recommended use of third argument for each request-pull, otherwise
+you can get error message like:
+
+   warn: No match for commit commit found at url
+   warn: Are you sure you pushed 'HEAD' there?
+
+
 GIT
 ---
 Part of the linkgit:git[1] suite

---

Second patch provides right processing of third parameter when short 
version of sha hash is used (e.g. 897a111). Now is
supported only full hash, what is different behaviour against first 
parameter or what can be found in other functions. Extra

solves one of cases of wrong warn message.

---
diff --git a/git-request-pull.sh b/git-request-pull.sh
index d5500fd..2dc735e 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -92,9 +92,11 @@ find_matching_ref='
chomp;
my ($sha1, $ref, $deref) = /^(\S+)\s+([^^]+)(\S*)$/;
my ($pattern);
+   my ($pattern2);
next unless ($sha1 eq $headrev);

$pattern=/$head\$;
+   $pattern2=^$head;
if ($ref eq $head) {
$found = $ref;
}
@@ -104,6 +106,9 @@ find_matching_ref='
if ($sha1 eq $head) {
$found = $sha1;
}
+   elsif ($sha1 =~ /$pattern2/ and (length $head) gt 7) {
+   $found = $sha1
+   }
}
if ($found) {
print $found\n;
---


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Improve contrib/diff-highlight to highlight unevenly-sized hunks

2015-06-18 Thread Junio C Hamano
Patrick Palka patr...@parcs.ath.cx writes:

 Currently the diff-highlight script does not try to highlight hunks that
 have different numbers of removed/added lines.  But we can be a little
 smarter than that, without introducing much magic and complexity.

 In the case of unevenly-sized hunks, we could still highlight the first
 few (lexicographical) add/remove pairs.  It is not uncommon for hunks to
 have common prefixes, and in such a case this change is very useful
 for spotting differences.

 Signed-off-by: Patrick Palka patr...@parcs.ath.cx
 ---

Patrick, git shortlog --no-merges contrib/diff-highlight/ is your
friend to see who may be able to give you a good feedback.

Jeff, what do you think?

I have this nagging feeling that it is just as likely that two
uneven hunks align at the top as they align at the bottom, so while
this might not hurt it may not be the right approach for a better
solution, in the sense that when somebody really wants to do a
better solution, this change and the original code may need to be
ripped out and redone from scratch.

  contrib/diff-highlight/diff-highlight | 26 +-
  1 file changed, 17 insertions(+), 9 deletions(-)

 diff --git a/contrib/diff-highlight/diff-highlight 
 b/contrib/diff-highlight/diff-highlight
 index ffefc31..0dfbebd 100755
 --- a/contrib/diff-highlight/diff-highlight
 +++ b/contrib/diff-highlight/diff-highlight
 @@ -88,22 +88,30 @@ sub show_hunk {
   return;
   }
  
 - # If we have mismatched numbers of lines on each side, we could try to
 - # be clever and match up similar lines. But for now we are simple and
 - # stupid, and only handle multi-line hunks that remove and add the same
 - # number of lines.
 - if (@$a != @$b) {
 - print @$a, @$b;
 - return;
 - }
 + # We match up the first MIN(a, b) lines on each side.
 + my $c = @$a  @$b ? @$a : @$b;
  
 + # Highlight each pair, and print each removed line of that pair.
   my @queue;
 - for (my $i = 0; $i  @$a; $i++) {
 + for (my $i = 0; $i  $c; $i++) {
   my ($rm, $add) = highlight_pair($a-[$i], $b-[$i]);
   print $rm;
   push @queue, $add;
   }
 +
 + # Print the remaining unmatched removed lines of the hunk.
 + for (my $i = $c; $i  @$a; $i++) {
 + print $a-[$i];
 + }
 +
 + # Print the added lines of each highlighted pair.
   print @queue;
 +
 + # Print the remaining unmatched added lines of the hunk.
 + for (my $i = $c; $i  @$b; $i++) {
 + print $b-[$i];
 + }
 +
  }
  
  sub highlight_pair {
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] strbuf: stop out-of-boundary warnings from Coverity

2015-06-18 Thread Junio C Hamano
Duy Nguyen pclo...@gmail.com writes:

 The last resort is simply filter out a whole class of warnings.
 Probably good enough if both patches look equally ugly.

 -- 8 --
 Subject: [PATCH] strbuf: kill strbuf_slopbuf, in favor of 

 A lot of out-of-bound access warnings on scan.coverity.com is because
 it does not realize this strbuf_slopbuf[] is in fact initialized with a
 single and promised to never change. But that promise could be broken if
 some caller attempts to write to strbuf-buf[0] write after STRBUF_INIT.

 We really can't do much about it. But we can try to put strbuf_slopbuf
 in .rodata section, where writes will be caught by the OS with memory
 protection support. The only drawback is people can't do
 buf-buf == strbuf_slopbuf any more. Luckily nobody does that in the
 current code base.
 ---

Hmph, would declaring slopbuf as const char [1] (and sprinkling
the (char *) cast) have the same effect, I wonder?

 +static inline void strbuf_terminate(struct strbuf *sb)
 +{
 + if (sb-buf[sb-len])
 + sb-buf[sb-len] = '\0';
 +}

This is so that you can call things like strbuf_rtrim() immediately
after running strbuf_init() safely, but I think it needs a comment
to save people from wondering what is going on, e.g. this is not an
optimization to avoid assigning NUL to a place that is already NUL;
a freshly initialized strbuf points at an unwritable piece of NUL
and we do not want to cause a SEGV.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Improve contrib/diff-highlight to highlight unevenly-sized hunks

2015-06-18 Thread Jeff King
On Thu, Jun 18, 2015 at 05:23:56PM -0400, Jeff King wrote:

 +# Return the individual diff-able items of the hunk, but with any
 +# diff or color prefix/suffix for each line split out (we assume that the
 +# prefix/suffix for each line will be the same).
 +sub split_hunk {
 + my ($prefix, $suffix, @r);
 + foreach my $line (@_) {
 + $line =~ /^($COLOR*[+-]$COLOR*)(.*)($COLOR*)/
 + or die eh, this is supposed to match everything!;

This isn't quite right. We'll never match the suffix, because it gets
sucked up by the greedy (.*). But making that non-greedy matches
nothing, unless we also add $ at the end.

_But_, there is still something else weird going on. I replaced this
split:

 + push @r, split(//, $2), \n;

with:

  push @r, split(/([[:space:][:punct:]])/, $2), \n;

which behaves much better. With that, 99a2cfb shows:

  -   if (!peel_ref(path, peeled)) {
  -   is_annotated = !!hashcmp(sha1, peeled);
  +   if (!peel_ref(path, peeled.hash)) {
  +   is_annotated = !!oidcmp(oid, peeled);

The latter half of both lines looks perfect. But what's that weird
highlighting of the initial if and is on those lines?

It turns out that the colored output we produce is quite odd:

  $ git show --color 99a2cfb | grep peel_ref | cat -A
  ^[[31m-^Iif (!peel_ref(path, peeled)) {^[[m$
  ^[[32m+^[[m^I^[[32mif (!peel_ref(path, peeled.hash)) {^[[m$

For the pre-image, we print the color, the -, and then the line data.
Makes sense.

For the post-image, we print the color, +, a reset, then the initial
whitespace, then the color again!

So of course the diff algorithm says hey, there's this weird color in
here. The original implementation of diff-highlight didn't care,
because it skipped leading whitespace and colors as boring. But this
one cannot do that. It pulls the strict prefix out of each line (and it
must, because it must get the same prefix for each line of a hunk).

I think git is actually wrong here; it's mingling the ANSI colors and
the actual content. Nobody ever noticed because it looks OK to a human,
and who would be foolish enough to try machine-parsing colorized diff
output?

The fix is:

diff --git a/diff.c b/diff.c
index 87b16d5..a80b5b4 100644
--- a/diff.c
+++ b/diff.c
@@ -501,9 +501,9 @@ static void emit_line_checked(const char *reset,
emit_line_0(ecbdata-opt, ws, reset, sign, line, len);
else {
/* Emit just the prefix, then the rest. */
-   emit_line_0(ecbdata-opt, set, reset, sign, , 0);
+   emit_line_0(ecbdata-opt, set, , sign, , 0);
ws_check_emit(line, len, ecbdata-ws_rule,
- ecbdata-opt-file, set, reset, ws);
+ ecbdata-opt-file, , reset, ws);
}
 }
 

But I'm a little worried it may interfere with the way the
whitespace-checker emits colors (e.g., it may emit its own colors for
the leading spaces, and we would need to re-assert our color before
showing the rest of the line). So I think you could also argue that
because of whitespace-highlighting, colorized diffs are fundamentally
going to have colors intermingled with the content and should not be
parsed this way.

All the more reason to try to move this inside diff.c, I guess.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Bug report: `git revert` on empty commit fails silently

2015-06-18 Thread Junio C Hamano
Alistair Lynn alist...@othernation.com writes:

 $ git commit --allow-empty -m ‘test’
 $ git revert HEAD

 The latter fails silently, leaving HEAD pointing at the commit created
 by the first command.

I do not necessarily think it is a bug to ignore reverting a no-op.
revert a no-op should probably fail by default, and the command
should require --force or --allow-empty.

But I agree that silently ignoring is not good.  It should warn the
user, saying that reverting no-op is nonsense, when refusing the
request.

 A subsequent `git commit --allow-empty` has the revert message as the
 default commit message when starting the editor.

And leaving a populated MERGE_MSG file to be picked up by the next
commit, which is an unrelated operation, is clearly wrong, I would
think.  If we deem the revert a no-op as a nonsense and ignore it,
we should ignore it completely and should not leave MERGE_MSG.

But leaving MERGE_MSG is internally consistent, I think.  When
revert stops due to conflicts and returns the control to the user,
it would explain the situation to the user loudly, and then after
user helps Git by resolving the conflict, the user uses commit,
and the message is picked up from MERGE_MSG.  I'd view what you saw
very similar to that situation.  Instead of seeing a conflict (with
which the command cannot automatically continue), the command saw a
no-op (which it is dubious that the user really meant to revert).
Asking the user to help and then allowing the user to signal that
s/he is now done with git commit is the right way to continue,
and for that to work seamlessly, the message has to be in MERGE_MSG.

So perhaps the only buggy part of this whole experience is that the
command silently failed, instead of explaining the situation
(i.e. No changes to revert); in case the user still does want to
commit the revert of no-op by commit --allow-empty, it did the
right thing by leaving the message in MERGE_MSG to be picked up
later.

I dunno.

 Hope this is the right place for bugs.

Yes, this is the right place for bugs.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Improve contrib/diff-highlight to highlight unevenly-sized hunks

2015-06-18 Thread Jeff King
On Thu, Jun 18, 2015 at 09:49:19PM -0700, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  ... I think you could also argue that
  because of whitespace-highlighting, colorized diffs are fundamentally
  going to have colors intermingled with the content and should not be
  parsed this way.
 
 Painting of whitespace breakages is asymmetric [*1*].  If you change
 something on a badly indented line without fixing the indentation,
 e.g.
 
   -SPTABhello-world
 +SPTABhello-people
 
 only the space-before-tab on the latter line is painted.
 
 For the purpose of your diff highlighting, however, you would want
 to treat the leading SPTABhello- on preimage and postimage
 lines as unchanged.

I do strip it off, so it is OK for it to be different in both the
pre-image and post-image. But what I can't tolerate is the
intermingling with actual data:

  +\t\t\x1b[32m;foo
  +\t\x1b[32m;bar

Those are both post-image lines. I can strip off the + from each side
to compare their inner parts to the pre-image. But the intermingled
color gets in my way. I can simply strip all colors from the whole line,
but then information is lost; how do I know where to put them back
again? It is not just add back the color at the beginning (which is
what I do with the prefix).

I think the answer is that I must strip them out, retaining the colors
found in each line along with their offset into the line, and then as I
write out the lines, re-add them at the appropriate spots (taking care
to use the original offsets, not the ones with the highlighting added
in).

  All the more reason to try to move this inside diff.c, I guess.
 
 That too, probably.

Hmm, I thought that would solve all my problems by operating on the
pre-color version without much more work. But...

 If we were to do this, I think it may make sense to separate the
 logic to compute which span of the string need to be painted in what
 color and the routine to actually emit the colored output.  That is,
 instead of letting ws-check-emit to decide which part should be in
 what color _and_ emitting the result, we should have a helper that
 reads line, len, and give us an array of spans to flag as
 whitespace violation.  Then an optional diff-highlight code would
 scan the same line, len (or a collection of line, len) without
 getting confused by the whitespace errors and annotate the spans to
 be highlighted.  After all that happens, the output routine would
 coalesce the spans and produce colored output.
 
 Or something like that ;-)

I think this array of spans is the only way to go. Otherwise whichever
markup scheme processes the hunk first ruins the data for the next
processor.

So it is a lot more work to make the two work together. The --word-diff
code would have the same issue, except that I imagine it just skips
whitespace-highlighting altogether.

The least-work thing may actually be teaching the separate
diff-highlight script to strip out the colorizing and re-add it by
offset.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Improve contrib/diff-highlight to highlight unevenly-sized hunks

2015-06-18 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 ... I think you could also argue that
 because of whitespace-highlighting, colorized diffs are fundamentally
 going to have colors intermingled with the content and should not be
 parsed this way.

Painting of whitespace breakages is asymmetric [*1*].  If you change
something on a badly indented line without fixing the indentation,
e.g.

-SPTABhello-world
+SPTABhello-people

only the space-before-tab on the latter line is painted.

For the purpose of your diff highlighting, however, you would want
to treat the leading SPTABhello- on preimage and postimage
lines as unchanged.

Which means that you shouldn't be reading a colored output without
ignoring the color-control sequences.

So I think you arrived at the correct conclusion.

 All the more reason to try to move this inside diff.c, I guess.

That too, probably.

If we were to do this, I think it may make sense to separate the
logic to compute which span of the string need to be painted in what
color and the routine to actually emit the colored output.  That is,
instead of letting ws-check-emit to decide which part should be in
what color _and_ emitting the result, we should have a helper that
reads line, len, and give us an array of spans to flag as
whitespace violation.  Then an optional diff-highlight code would
scan the same line, len (or a collection of line, len) without
getting confused by the whitespace errors and annotate the spans to
be highlighted.  After all that happens, the output routine would
coalesce the spans and produce colored output.

Or something like that ;-)


[Footnote]

*1* We recently added a knob to allow us paint them on preimage and
common lines, too, but that is not the default.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: co-authoring commits

2015-06-18 Thread Jeff King
On Thu, Jun 18, 2015 at 11:25:44PM +0200, Jakub Narębski wrote:

 Author and committer include datetime in the contents of the
 field, which is used by Git for heuristics limiting walk. Coauthor
 would have the same date as author, isn't it? If, after long
 and involved discussion, we didn't add 'generation' field (for
 easier cutting history walking), what chance adding 'coauthor'
 has.

I don't think the two situations are comparable. I would (and did) argue
that a generation field is a bad header to bake in because of what it
means (it is redundant with the graph structure).

Whereas co-author is not a fundamentally bad; it's just not something
we chose to support early on, and it would have to be added now.

 OTOH it would be nice to have support for .mailmap, and for
 grepping... but the former could conceivably be added to the trailer
 tool, the latter can be done with appropriate regexp in
 git log --grep=

I don't think we munge trailers during git log pretty-printing at all
now, but it is certainly something we could add (including mailmap-ing
them).  That doesn't seem like much more work than showing the co-author
field, and it's a lot more generally applicable (you could mailmap
S-O-B, Reviewed-by, and so forth).

Similarly, something like git shortlog would have to learn about
multiple authors under the co-author scheme. But likewise, it would
not be much more work to teach it something like:

  git shortlog --field=Reviewed-by

to handle an arbitrary trailer. And that is much more flexible.

 I wonder what would break if one used 'Name e@mai.l, Name em@i.l'
 as the author...

The normal parser we use for pretty-printing goes left-to-right and
will stop at the first , and show only the first author.

Older versions of git would then get the date wrong, complaining about
the ,. Newer versions parse the date from right-to-left to work around
such bogosities (especially things like foo bar) and so will parse
back to the second .

Fsck will definitely complain about it.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] format-patch: introduce format.outputDirectory configuration

2015-06-18 Thread Jeff King
On Thu, Jun 18, 2015 at 02:46:36PM -0700, Junio C Hamano wrote:

  If I were designing from scratch, I would consider making -o - output
  to stdout, and letting it override a previous -o (or vice versa). We
  could still do that (and make --stdout an alias for that), but I don't
  know if it is worth the trouble (it does change the behavior for anybody
  who wanted a directory called -, but IMHO it is more likely to save
  somebody a headache than create one).
 
 I agree with later -o should override an earlier one, but I do not
 necessarily agree with '-o -' should be --stdout, for a simple
 reason that -o foo is not --stdout foo.

Good point. At any rate, that was all in my designing from scratch
hypothetical, so it is doubly not worth considering.

 Perhaps something like this to replace builtin/ part of Alexander's
 patch?
 [...]
 @@ -1337,6 +1342,9 @@ int cmd_format_patch(int argc, const char **argv, const 
 char *prefix)
   die (_(--subject-prefix and -k are mutually exclusive.));
   rev.preserve_subject = keep_subject;
  
 + if (!output_directory  !use_stdout)
 + output_directory = config_output_directory;
 +

Yeah, I think that is the sanest way to do it given the constraints.

-Peff

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Improve contrib/diff-highlight to highlight unevenly-sized hunks

2015-06-18 Thread Junio C Hamano
Patrick Palka patr...@parcs.ath.cx writes:

 I have this nagging feeling that it is just as likely that two
 uneven hunks align at the top as they align at the bottom, so while
 this might not hurt it may not be the right approach for a better
 solution, in the sense that when somebody really wants to do a
 better solution, this change and the original code may need to be
 ripped out and redone from scratch.

 Hmm, maybe. I stuck with assuming hunks are top-aligned because it
 required less code to implement :)

Yeah, I understand that.

If we will need to rip out only this change but keep the original in
order to implement a future better solution, then we might be better
off not having this change (if we anticipate such a better solution
to come reasonably soon), because it would make it more work for the
final improved solution.  But if we need to rip out the original as
well as this change while we do so, then having this patch would not
make it more work, either.

So as I said, I do not think it would hurt to have this as an
incremental improvement (albeit going in a possibly wrong
direction).

Of course, it is a separate question if this change makes the output
worse, by comparing unmatched early parts of two hunks and making
nonsense highlight by calling highlight_pair() more often.  As long
as that is not an issue, I am not opposed to this change, which was
what I meant to say by this might not hurt.






--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v4 07/10] send-email: reduce dependancies impact on parse_address_line

2015-06-18 Thread Matthieu Moy
Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr writes:

 Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr writes:
 
  I've some more tests, maybe I should put them all in this post ?
 
 Yes, please post as much as you have. Ideally, this should be
 automatically tested, but if you don't have time to write the automated
 tests, at least having a track of what you did on the list archives can
 help someone else to do it.

 It may not be easily readable without colors, so there are the scripts
 at the end.

Cool. Then almost all the work is done to get an automated test. Next
step would be to add the tests itself in the code. I would do that by
adding a hidden --selfcheck option to git send-email that would compare
Mail::Address-parse($string); and split_addrs($string); for all your
testcases, and die if they do not match. Then calling it from the
testsuite would be trivial.

I can do that on top of your series if you don't have time.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Improve contrib/diff-highlight to highlight unevenly-sized hunks

2015-06-18 Thread Jeff King
On Thu, Jun 18, 2015 at 11:08:16AM -0700, Junio C Hamano wrote:

 So as I said, I do not think it would hurt to have this as an
 incremental improvement (albeit going in a possibly wrong
 direction).
 
 Of course, it is a separate question if this change makes the output
 worse, by comparing unmatched early parts of two hunks and making
 nonsense highlight by calling highlight_pair() more often.  As long
 as that is not an issue, I am not opposed to this change, which was
 what I meant to say by this might not hurt.

Yes, that is my big concern, and why I punted on mismatched-size hunks
in the first place. Now that we have a patch, it is easy enough to git
log -p | diff-highlight with the old and new versions to compare the
results.

It certainly does improve some cases. E.g.:

  -foo
  +foo 
  +bar

in a test script becomes more clear. But some of the output is not so
great. For instance, the very commit under discussion has a
confusing and useless highlight. Or take a documentation patch like
5c31acfb, where I find the highlights actively distracting. We are saved
a little by the if the whole line is different, do not highlight at
all behavior of 097128d1bc.

So I dunno. IMHO this does more harm than good, and I would not want to
use it myself. But it is somewhat a matter of taste; I am not opposed to
making it a configurable option.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Git completion not using ls-remote to auto-complete during push

2015-06-18 Thread Robert Dailey
On Thu, Jun 18, 2015 at 10:55 AM, SZEDER Gábor sze...@ira.uka.de wrote:

 Quoting Robert Dailey rcdailey.li...@gmail.com:

 On Thu, Jun 18, 2015 at 6:29 AM, SZEDER Gábor sze...@ira.uka.de wrote:

 Quoting Robert Dailey rcdailey.li...@gmail.com

 I do the following:

 $ git push origin :topic

 If I stop halfway through typing 'topic' and hit TAB, auto-completion
 does not work if I do not have a local branch by that name (sometimes
 I delete my local branch first, then I push to delete it remotely). I
 thought that git completion code was supposed to use ls-remote to auto
 complete refs used in push operations. Is this supposed to work?


 It's intentional.  Running 'git ls-remote' with a far away remote can
 take ages, so instead we grab the refs on the remote from the locally
 stored refs under 'refs/remotes/remote/'.

 See e832f5c096 (completion: avoid ls-remote in certain scenarios,
 2013-05-28).  The commit message mentions that you can force
 completion of remote refs via 'git ls-remote' by starting with the full
 refname, i.e.  'refs/TAB', however, that seems to work only on the
 left hand side of the colon in the push refspec.

 Gábor


 If that's indeed the case, then completion should work. I have a
 'refs/remotes/origin/topic'. Why will auto complete not work even
 though this exists? Do multiple remotes cause issues (in theory there
 is no reason why it should cause problems, since it should know I'm
 auto-completing a ref on 'origin')?


 The number of remotes doesn't matter.
 What matters is which side of the colon the ref to be completed is.

 You can complete

   git push origin refs/TAB

 and

   git fetch origin refs/TAB

 will even list you refs freshly queried via 'git ls-remote'.
 However,

   git push origin :refs/TAB
   git push origin branch:refs/TAB

 don't work, because there are no refs starting with the prefix ':refs/' or
 'branch:refs/'.

 Gábor


Interesting. So is it just a glaring bug that the git completion
script isn't ignoring the : and anything to the left of it? Or is this
beyond the control of the script?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/WIP v3 04/31] am: implement patch queue mechanism

2015-06-18 Thread Stefan Beller
On Thu, Jun 18, 2015 at 4:25 AM, Paul Tan pyoka...@gmail.com wrote:
 +/**
 + * Reads the contents of `file`. The third argument can be used to give a 
 hint

I would avoid `third` here. (I needed to count twice to be sure which
argument you
were referring to, as I was confused.) Also how do you abstain from
giving a hint?
(0 or negative or MAX_INT?)

So maybe

/**
 * Reads the contents of `file`. 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. If `hint` is non-zero, it is used as a
hint for initial
 * allocation to avoid reallocs.
 */

 + * 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_trim(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) {
 +
 +   /* TODO: Patch application not implemented yet */
 +
 +   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)
  {
 @@ -24,5 +176,21 @@ int cmd_am(int argc, const char **argv, const char 
 *prefix)
 setup_work_tree();
 }

 +   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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Improve contrib/diff-highlight to highlight unevenly-sized hunks

2015-06-18 Thread Jeff King
On Thu, Jun 18, 2015 at 12:28:58PM -0400, Patrick Palka wrote:

 By the way, what would it take to get something like this script into
 git proper?  It is IMHO immensely useful even in its current form, yet
 because it's not baked into the application hardly anybody knows about
 it.

I think if we were going to make it more official, it would make sense
to do it inside the diff code itself (i.e., not as a separate script),
and it might be reasonable at that point to actually do a real
character-based diff rather than the hacky prefix/suffix thing (or
possibly even integrate with the color-words patterns to find
syntactically interesting breaks). There is some discussion in the
Bugs section of contrib/diff-highlight/README.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 05/19] fsck (receive-pack): Allow demoting errors to warnings

2015-06-18 Thread Johannes Schindelin
For example, missing emails in commit and tag objects can be demoted to
mere warnings with

git config receive.fsck.missingemail=warn

The value is actually a comma-separated list.

In case that the same key is listed in multiple receive.fsck.msg-id
lines in the config, the latter configuration wins (this can happen for
example when both $HOME/.gitconfig and .git/config contain message type
settings).

As git receive-pack does not actually perform the checks, it hands off
the setting to index-pack or unpack-objects in the form of an optional
argument to the --strict option.

Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
---
 builtin/index-pack.c |  4 
 builtin/receive-pack.c   | 17 +++--
 builtin/unpack-objects.c |  5 +
 fsck.c   |  8 
 fsck.h   |  1 +
 5 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 87ae9ba..98e14fe 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1633,6 +1633,10 @@ int cmd_index_pack(int argc, const char **argv, const 
char *prefix)
} else if (!strcmp(arg, --strict)) {
strict = 1;
do_fsck_object = 1;
+   } else if (skip_prefix(arg, --strict=, arg)) {
+   strict = 1;
+   do_fsck_object = 1;
+   fsck_set_msg_types(fsck_options, arg);
} else if (!strcmp(arg, 
--check-self-contained-and-connected)) {
strict = 1;
check_self_contained_and_connected = 1;
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 94d0571..3afe8f8 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -19,6 +19,7 @@
 #include tag.h
 #include gpg-interface.h
 #include sigchain.h
+#include fsck.h
 
 static const char receive_pack_usage[] = git receive-pack git-dir;
 
@@ -36,6 +37,7 @@ static enum deny_action deny_current_branch = 
DENY_UNCONFIGURED;
 static enum deny_action deny_delete_current = DENY_UNCONFIGURED;
 static int receive_fsck_objects = -1;
 static int transfer_fsck_objects = -1;
+static struct strbuf fsck_msg_types = STRBUF_INIT;
 static int receive_unpack_limit = -1;
 static int transfer_unpack_limit = -1;
 static int advertise_atomic_push = 1;
@@ -115,6 +117,15 @@ static int receive_pack_config(const char *var, const char 
*value, void *cb)
return 0;
}
 
+   if (skip_prefix(var, receive.fsck., var)) {
+   if (is_valid_msg_type(var, value))
+   strbuf_addf(fsck_msg_types, %c%s=%s,
+   fsck_msg_types.len ? ',' : '=', var, value);
+   else
+   warning(Skipping unknown msg id '%s', var);
+   return 0;
+   }
+
if (strcmp(var, receive.fsckobjects) == 0) {
receive_fsck_objects = git_config_bool(var, value);
return 0;
@@ -1490,7 +1501,8 @@ static const char *unpack(int err_fd, struct shallow_info 
*si)
if (quiet)
argv_array_push(child.args, -q);
if (fsck_objects)
-   argv_array_push(child.args, --strict);
+   argv_array_pushf(child.args, --strict%s,
+   fsck_msg_types.buf);
child.no_stdout = 1;
child.err = err_fd;
child.git_cmd = 1;
@@ -1508,7 +1520,8 @@ static const char *unpack(int err_fd, struct shallow_info 
*si)
argv_array_pushl(child.args, index-pack,
 --stdin, hdr_arg, keep_arg, NULL);
if (fsck_objects)
-   argv_array_push(child.args, --strict);
+   argv_array_pushf(child.args, --strict%s,
+   fsck_msg_types.buf);
if (fix_thin)
argv_array_push(child.args, --fix-thin);
child.out = -1;
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 6d17040..7cc086f 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -530,6 +530,11 @@ int cmd_unpack_objects(int argc, const char **argv, const 
char *prefix)
strict = 1;
continue;
}
+   if (skip_prefix(arg, --strict=, arg)) {
+   strict = 1;
+   fsck_set_msg_types(fsck_options, arg);
+   continue;
+   }
if (starts_with(arg, --pack_header=)) {
struct pack_header *hdr;
char *c;
diff --git a/fsck.c b/fsck.c
index 7db81d2..0c7cc26 100644
--- a/fsck.c

[PATCH v5 07/19] fsck: Make fsck_ident() warn-friendly

2015-06-18 Thread Johannes Schindelin
When fsck_ident() identifies a problem with the ident, it should still
advance the pointer to the next line so that fsck can continue in the
case of a mere warning.

Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
---
 fsck.c | 49 +++--
 1 file changed, 27 insertions(+), 22 deletions(-)

diff --git a/fsck.c b/fsck.c
index 47cb686..8a1eea3 100644
--- a/fsck.c
+++ b/fsck.c
@@ -479,40 +479,45 @@ static int require_end_of_header(const void *data, 
unsigned long size,
 
 static int fsck_ident(const char **ident, struct object *obj, struct 
fsck_options *options)
 {
+   const char *p = *ident;
char *end;
 
-   if (**ident == '')
+   *ident = strchrnul(*ident, '\n');
+   if (**ident == '\n')
+   (*ident)++;
+
+   if (*p == '')
return report(options, obj, FSCK_MSG_MISSING_NAME_BEFORE_EMAIL, 
invalid author/committer line - missing space before email);
-   *ident += strcspn(*ident, \n);
-   if (**ident == '')
+   p += strcspn(p, \n);
+   if (*p == '')
return report(options, obj, FSCK_MSG_BAD_NAME, invalid 
author/committer line - bad name);
-   if (**ident != '')
+   if (*p != '')
return report(options, obj, FSCK_MSG_MISSING_EMAIL, invalid 
author/committer line - missing email);
-   if ((*ident)[-1] != ' ')
+   if (p[-1] != ' ')
return report(options, obj, 
FSCK_MSG_MISSING_SPACE_BEFORE_EMAIL, invalid author/committer line - missing 
space before email);
-   (*ident)++;
-   *ident += strcspn(*ident, \n);
-   if (**ident != '')
+   p++;
+   p += strcspn(p, \n);
+   if (*p != '')
return report(options, obj, FSCK_MSG_BAD_EMAIL, invalid 
author/committer line - bad email);
-   (*ident)++;
-   if (**ident != ' ')
+   p++;
+   if (*p != ' ')
return report(options, obj, FSCK_MSG_MISSING_SPACE_BEFORE_DATE, 
invalid author/committer line - missing space before date);
-   (*ident)++;
-   if (**ident == '0'  (*ident)[1] != ' ')
+   p++;
+   if (*p == '0'  p[1] != ' ')
return report(options, obj, FSCK_MSG_ZERO_PADDED_DATE, invalid 
author/committer line - zero-padded date);
-   if (date_overflows(strtoul(*ident, end, 10)))
+   if (date_overflows(strtoul(p, end, 10)))
return report(options, obj, FSCK_MSG_DATE_OVERFLOW, invalid 
author/committer line - date causes integer overflow);
-   if (end == *ident || *end != ' ')
+   if ((end == p || *end != ' '))
return report(options, obj, FSCK_MSG_BAD_DATE, invalid 
author/committer line - bad date);
-   *ident = end + 1;
-   if ((**ident != '+'  **ident != '-') ||
-   !isdigit((*ident)[1]) ||
-   !isdigit((*ident)[2]) ||
-   !isdigit((*ident)[3]) ||
-   !isdigit((*ident)[4]) ||
-   ((*ident)[5] != '\n'))
+   p = end + 1;
+   if ((*p != '+'  *p != '-') ||
+   !isdigit(p[1]) ||
+   !isdigit(p[2]) ||
+   !isdigit(p[3]) ||
+   !isdigit(p[4]) ||
+   (p[5] != '\n'))
return report(options, obj, FSCK_MSG_BAD_TIMEZONE, invalid 
author/committer line - bad time zone);
-   (*ident) += 6;
+   p += 6;
return 0;
 }
 
-- 
2.3.1.windows.1.9.g8c01ab4



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 04/19] fsck: Offer a function to demote fsck errors to warnings

2015-06-18 Thread Johannes Schindelin
There are legacy repositories out there whose older commits and tags
have issues that prevent pushing them when 'receive.fsckObjects' is set.
One real-life example is a commit object that has been hand-crafted to
list two authors.

Often, it is not possible to fix those issues without disrupting the
work with said repositories, yet it is still desirable to perform checks
by setting `receive.fsckObjects = true`. This commit is the first step
to allow demoting specific fsck issues to mere warnings.

The `fsck_set_msg_types()` function added by this commit parses a list
of settings in the form:

missingemail=warn,badname=warn,...

Unfortunately, the FSCK_WARN/FSCK_ERROR flag is only really heeded by
git fsck so far, but other call paths (e.g. git index-pack --strict)
error out *always* no matter what type was specified. Therefore, we need
to take extra care to set all message types to FSCK_ERROR by default in
those cases.

Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
---
 fsck.c | 82 +++---
 fsck.h | 10 ++--
 2 files changed, 87 insertions(+), 5 deletions(-)

diff --git a/fsck.c b/fsck.c
index 4595c7f..7db81d2 100644
--- a/fsck.c
+++ b/fsck.c
@@ -103,13 +103,85 @@ static int fsck_msg_type(enum fsck_msg_id msg_id,
 {
int msg_type;
 
-   msg_type = msg_id_info[msg_id].msg_type;
-   if (options-strict  msg_type == FSCK_WARN)
-   msg_type = FSCK_ERROR;
+   assert(msg_id = 0  msg_id  FSCK_MSG_MAX);
+
+   if (options-msg_type)
+   msg_type = options-msg_type[msg_id];
+   else {
+   msg_type = msg_id_info[msg_id].msg_type;
+   if (options-strict  msg_type == FSCK_WARN)
+   msg_type = FSCK_ERROR;
+   }
 
return msg_type;
 }
 
+static inline int substrcmp(const char *string, int len, const char *match)
+{
+   int match_len = strlen(match);
+   if (match_len != len)
+   return -1;
+   return memcmp(string, match, len);
+}
+
+static int parse_msg_type(const char *str, int len)
+{
+   if (len  0)
+   len = strlen(str);
+
+   if (!substrcmp(str, len, error))
+   return FSCK_ERROR;
+   else if (!substrcmp(str, len, warn))
+   return FSCK_WARN;
+   else
+   die(Unknown fsck message type: '%.*s',
+   len, str);
+}
+
+void fsck_set_msg_type(struct fsck_options *options,
+   const char *msg_id, int msg_id_len,
+   const char *msg_type, int msg_type_len)
+{
+   int id = parse_msg_id(msg_id, msg_id_len), type;
+
+   if (id  0)
+   die(Unhandled message id: %.*s, msg_id_len, msg_id);
+   type = parse_msg_type(msg_type, msg_type_len);
+
+   if (!options-msg_type) {
+   int i;
+   int *msg_type = xmalloc(sizeof(int) * FSCK_MSG_MAX);
+   for (i = 0; i  FSCK_MSG_MAX; i++)
+   msg_type[i] = fsck_msg_type(i, options);
+   options-msg_type = msg_type;
+   }
+
+   options-msg_type[id] = type;
+}
+
+void fsck_set_msg_types(struct fsck_options *options, const char *values)
+{
+   while (*values) {
+   int len = strcspn(values,  ,|), equal;
+
+   if (!len) {
+   values++;
+   continue;
+   }
+
+   for (equal = 0; equal  len; equal++)
+   if (values[equal] == '=' || values[equal] == ':')
+   break;
+
+   if (equal == len)
+   die(Missing '=': '%.*s', len, values);
+
+   fsck_set_msg_type(options, values, equal,
+   values + equal + 1, len - equal - 1);
+   values += len;
+   }
+}
+
 __attribute__((format (printf, 4, 5)))
 static int report(struct fsck_options *options, struct object *object,
enum fsck_msg_id id, const char *fmt, ...)
@@ -599,6 +671,10 @@ int fsck_object(struct object *obj, void *data, unsigned 
long size,
 
 int fsck_error_function(struct object *obj, int msg_type, const char *message)
 {
+   if (msg_type == FSCK_WARN) {
+   warning(object %s: %s, sha1_to_hex(obj-sha1), message);
+   return 0;
+   }
error(object %s: %s, sha1_to_hex(obj-sha1), message);
return 1;
 }
diff --git a/fsck.h b/fsck.h
index f6f268a..edb4540 100644
--- a/fsck.h
+++ b/fsck.h
@@ -6,6 +6,11 @@
 
 struct fsck_options;
 
+void fsck_set_msg_type(struct fsck_options *options,
+   const char *msg_id, int msg_id_len,
+   const char *msg_type, int msg_type_len);
+void fsck_set_msg_types(struct fsck_options *options, const char *values);
+
 /*
  * callback function for fsck_walk
  * type is the expected type of the object or OBJ_ANY
@@ -25,10 +30,11 @@ struct fsck_options {
fsck_walk_func walk;
fsck_error 

[PATCH v5 06/19] fsck: Report the ID of the error/warning

2015-06-18 Thread Johannes Schindelin
Some legacy code has objects with non-fatal fsck issues; To enable the
user to ignore those issues, let's print out the ID (e.g. when
encountering missingemail, the user might want to call `git config
--add receive.fsck.missingemail=warn`).

Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
---
 fsck.c  | 16 
 t/t1450-fsck.sh |  4 ++--
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/fsck.c b/fsck.c
index 0c7cc26..47cb686 100644
--- a/fsck.c
+++ b/fsck.c
@@ -190,6 +190,20 @@ void fsck_set_msg_types(struct fsck_options *options, 
const char *values)
}
 }
 
+static void append_msg_id(struct strbuf *sb, const char *msg_id)
+{
+   for (;;) {
+   char c = *(msg_id)++;
+
+   if (!c)
+   break;
+   if (c != '_')
+   strbuf_addch(sb, tolower(c));
+   }
+
+   strbuf_addstr(sb, : );
+}
+
 __attribute__((format (printf, 4, 5)))
 static int report(struct fsck_options *options, struct object *object,
enum fsck_msg_id id, const char *fmt, ...)
@@ -198,6 +212,8 @@ static int report(struct fsck_options *options, struct 
object *object,
struct strbuf sb = STRBUF_INIT;
int msg_type = fsck_msg_type(id, options), result;
 
+   append_msg_id(sb, msg_id_info[id].id_string);
+
va_start(ap, fmt);
strbuf_vaddf(sb, fmt, ap);
result = options-error_func(object, msg_type, sb.buf);
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index cfb32b6..286a643 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -231,8 +231,8 @@ test_expect_success 'tag with incorrect tag name  missing 
tagger' '
git fsck --tags 2out 
 
cat expect -EOF 
-   warning in tag $tag: invalid '\''tag'\'' name: wrong name format
-   warning in tag $tag: invalid format - expected '\''tagger'\'' line
+   warning in tag $tag: invalidtagname: invalid '\''tag'\'' name: wrong 
name format
+   warning in tag $tag: missingtaggerentry: invalid format - expected 
'\''tagger'\'' line
EOF
test_cmp expect out
 '
-- 
2.3.1.windows.1.9.g8c01ab4



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 15/19] fsck: Document the new receive.fsck.msg-id options

2015-06-18 Thread Johannes Schindelin
Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
---
 Documentation/config.txt | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 3e37b93..306ab7a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2205,6 +2205,20 @@ receive.fsckObjects::
Defaults to false. If not set, the value of `transfer.fsckObjects`
is used instead.
 
+receive.fsck.msg-id::
+   When `receive.fsckObjects` is set to true, errors can be switched
+   to warnings and vice versa by configuring the `receive.fsck.msg-id`
+   setting where the `msg-id` is the fsck message ID and the value
+   is one of `error`, `warn` or `ignore`. For convenience, fsck prefixes
+   the error/warning with the message ID, e.g. missingemail: invalid
+   author/committer line - missing email means that setting
+   `receive.fsck.missingemail = ignore` will hide that issue.
++
+This feature is intended to support working with legacy repositories
+which would not pass pushing when `receive.fsckObjects = true`, allowing
+the host to accept repositories with certain known issues but still catch
+other issues.
+
 receive.unpackLimit::
If the number of objects received in a push is below this
limit then the objects will be unpacked into loose object
-- 
2.3.1.windows.1.9.g8c01ab4



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 14/19] fsck: Allow upgrading fsck warnings to errors

2015-06-18 Thread Johannes Schindelin
The 'invalid tag name' and 'missing tagger entry' warnings can now be
upgraded to errors by specifying `invalidtagname` and
`missingtaggerentry` in the receive.fsck.msg-id config setting.

Incidentally, the missing tagger warning is now really shown as a warning
(as opposed to being reported with the error: prefix, as it used to be
the case before this commit).

Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
---
 fsck.c| 24 +---
 t/t5302-pack-index.sh |  2 +-
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/fsck.c b/fsck.c
index 0f7eb22..a5e7dfb 100644
--- a/fsck.c
+++ b/fsck.c
@@ -10,6 +10,7 @@
 #include utf8.h
 
 #define FSCK_FATAL -1
+#define FSCK_INFO -2
 
 #define FOREACH_MSG_ID(FUNC) \
/* fatal errors */ \
@@ -55,10 +56,11 @@
FUNC(HAS_DOT, WARN) \
FUNC(HAS_DOTDOT, WARN) \
FUNC(HAS_DOTGIT, WARN) \
-   FUNC(INVALID_TAG_NAME, WARN) \
-   FUNC(MISSING_TAGGER_ENTRY, WARN) \
FUNC(NULL_SHA1, WARN) \
-   FUNC(ZERO_PADDED_FILEMODE, WARN)
+   FUNC(ZERO_PADDED_FILEMODE, WARN) \
+   /* infos (reported as warnings, but ignored by default) */ \
+   FUNC(INVALID_TAG_NAME, INFO) \
+   FUNC(MISSING_TAGGER_ENTRY, INFO)
 
 #define MSG_ID(id, msg_type) FSCK_MSG_##id,
 enum fsck_msg_id {
@@ -227,6 +229,8 @@ static int report(struct fsck_options *options, struct 
object *object,
 
if (msg_type == FSCK_FATAL)
msg_type = FSCK_ERROR;
+   else if (msg_type == FSCK_INFO)
+   msg_type = FSCK_WARN;
 
append_msg_id(sb, msg_id_info[id].id_string);
 
@@ -685,15 +689,21 @@ static int fsck_tag_buffer(struct tag *tag, const char 
*data,
goto done;
}
strbuf_addf(sb, refs/tags/%.*s, (int)(eol - buffer), buffer);
-   if (check_refname_format(sb.buf, 0))
-   report(options, tag-object, FSCK_MSG_INVALID_TAG_NAME,
+   if (check_refname_format(sb.buf, 0)) {
+   ret = report(options, tag-object, FSCK_MSG_INVALID_TAG_NAME,
   invalid 'tag' name: %.*s,
   (int)(eol - buffer), buffer);
+   if (ret)
+   goto done;
+   }
buffer = eol + 1;
 
-   if (!skip_prefix(buffer, tagger , buffer))
+   if (!skip_prefix(buffer, tagger , buffer)) {
/* early tags do not contain 'tagger' lines; warn only */
-   report(options, tag-object, FSCK_MSG_MISSING_TAGGER_ENTRY, 
invalid format - expected 'tagger' line);
+   ret = report(options, tag-object, 
FSCK_MSG_MISSING_TAGGER_ENTRY, invalid format - expected 'tagger' line);
+   if (ret)
+   goto done;
+   }
else
ret = fsck_ident(buffer, tag-object, options);
 
diff --git a/t/t5302-pack-index.sh b/t/t5302-pack-index.sh
index 61bc8da..3dc5ec4 100755
--- a/t/t5302-pack-index.sh
+++ b/t/t5302-pack-index.sh
@@ -259,7 +259,7 @@ EOF
 thirtyeight=${tag#??} 
 rm -f .git/objects/${tag%$thirtyeight}/$thirtyeight 
 git index-pack --strict tag-test-${pack1}.pack 2err 
-grep ^error:.* expected .tagger. line err
+grep ^warning:.* expected .tagger. line err
 '
 
 test_done
-- 
2.3.1.windows.1.9.g8c01ab4



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 19/19] fsck: support ignoring objects in `git fsck` via fsck.skiplist

2015-06-18 Thread Johannes Schindelin
Identical to support in `git receive-pack for the config option
`receive.fsck.skiplist`, we now support ignoring given objects in
`git fsck` via `fsck.skiplist` altogether.

This is extremely handy in case of legacy repositories where it would
cause more pain to change incorrect objects than to live with them
(e.g. a duplicate 'author' line in an early commit object).

Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
---
 Documentation/config.txt |  7 +++
 builtin/fsck.c   | 10 ++
 2 files changed, 17 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5f45115..5aba63a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1261,6 +1261,13 @@ that setting `fsck.missingemail = ignore` will hide that 
issue.
 This feature is intended to support working with legacy repositories
 which cannot be repaired without disruptive changes.
 
+fsck.skipList::
+   The path to a sorted list of object names (i.e. one SHA-1 per
+   line) that are known to be broken in a non-fatal way and should
+   be ignored. This feature is useful when an established project
+   should be accepted despite early commits containing errors that
+   can be safely ignored such as invalid committer email addresses.
+
 gc.aggressiveDepth::
The depth parameter used in the delta compression
algorithm used by 'git gc --aggressive'.  This defaults
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 75fcb5f..ce538ac 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -54,6 +54,16 @@ static int fsck_config(const char *var, const char *value, 
void *cb)
return 0;
}
 
+   if (strcmp(var, fsck.skiplist) == 0) {
+   const char *path = is_absolute_path(value) ?
+   value : git_path(%s, value);
+   struct strbuf sb = STRBUF_INIT;
+   strbuf_addf(sb, skiplist=%s, path);
+   fsck_set_msg_types(fsck_obj_options, sb.buf);
+   strbuf_release(sb);
+   return 0;
+   }
+
return git_default_config(var, value, cb);
 }
 
-- 
2.3.1.windows.1.9.g8c01ab4


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 18/19] fsck: git receive-pack: support excluding objects from fsck'ing

2015-06-18 Thread Johannes Schindelin
The optional new config option `receive.fsck.skiplist` specifies the path
to a file listing the names, i.e. SHA-1s, one per line, of objects that
are to be ignored by `git receive-pack` when `receive.fsckObjects = true`.

This is extremely handy in case of legacy repositories where it would
cause more pain to change incorrect objects than to live with them
(e.g. a duplicate 'author' line in an early commit object).

The intended use case is for server administrators to inspect objects
that are reported by `git push` as being too problematic to enter the
repository, and to add the objects' SHA-1 to a (preferably sorted) file
when the objects are legitimate, i.e. when it is determined that those
problematic objects should be allowed to enter the server.

Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
---
 Documentation/config.txt|  7 ++
 builtin/receive-pack.c  |  8 ++
 fsck.c  | 54 +
 fsck.h  |  1 +
 t/t5504-fetch-receive-strict.sh | 12 +
 5 files changed, 82 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 41fd460..5f45115 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2230,6 +2230,13 @@ which would not pass pushing when `receive.fsckObjects = 
true`, allowing
 the host to accept repositories with certain known issues but still catch
 other issues.
 
+receive.fsck.skipList::
+   The path to a sorted list of object names (i.e. one SHA-1 per
+   line) that are known to be broken in a non-fatal way and should
+   be ignored. This feature is useful when an established project
+   should be accepted despite early commits containing errors that
+   can be safely ignored such as invalid committer email addresses.
+
 receive.unpackLimit::
If the number of objects received in a push is below this
limit then the objects will be unpacked into loose object
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 3afe8f8..80574f9 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -117,6 +117,14 @@ static int receive_pack_config(const char *var, const char 
*value, void *cb)
return 0;
}
 
+   if (strcmp(var, receive.fsck.skiplist) == 0) {
+   const char *path = is_absolute_path(value) ?
+   value : git_path(%s, value);
+   strbuf_addf(fsck_msg_types, %cskiplist=%s,
+   fsck_msg_types.len ? ',' : '=', path);
+   return 0;
+   }
+
if (skip_prefix(var, receive.fsck., var)) {
if (is_valid_msg_type(var, value))
strbuf_addf(fsck_msg_types, %c%s=%s,
diff --git a/fsck.c b/fsck.c
index a5e7dfb..9b8981e 100644
--- a/fsck.c
+++ b/fsck.c
@@ -8,6 +8,7 @@
 #include fsck.h
 #include refs.h
 #include utf8.h
+#include sha1-array.h
 
 #define FSCK_FATAL -1
 #define FSCK_INFO -2
@@ -122,6 +123,43 @@ static int fsck_msg_type(enum fsck_msg_id msg_id,
return msg_type;
 }
 
+static void init_skiplist(struct fsck_options *options, const char *path)
+{
+   static struct sha1_array skiplist = SHA1_ARRAY_INIT;
+   int sorted, fd;
+   char buffer[41];
+   unsigned char sha1[20];
+
+   if (options-skiplist)
+   sorted = options-skiplist-sorted;
+   else {
+   sorted = 1;
+   options-skiplist = skiplist;
+   }
+
+   fd = open(path, O_RDONLY);
+   if (fd  0)
+   die(Could not open skip list: %s, path);
+   for (;;) {
+   int result = read_in_full(fd, buffer, sizeof(buffer));
+   if (result  0)
+   die_errno(Could not read '%s', path);
+   if (!result)
+   break;
+   if (get_sha1_hex(buffer, sha1) || buffer[40] != '\n')
+   die(Invalid SHA-1: %s, buffer);
+   sha1_array_append(skiplist, sha1);
+   if (sorted  skiplist.nr  1 
+   hashcmp(skiplist.sha1[skiplist.nr - 2],
+   sha1)  0)
+   sorted = 0;
+   }
+   close(fd);
+
+   if (sorted)
+   skiplist.sorted = 1;
+}
+
 static inline int substrcmp(const char *string, int len, const char *match)
 {
int match_len = strlen(match);
@@ -193,6 +231,18 @@ void fsck_set_msg_types(struct fsck_options *options, 
const char *values)
if (values[equal] == '=' || values[equal] == ':')
break;
 
+   if (!substrcmp(values, equal, skiplist)) {
+   char *path = xstrndup(values + equal + 1,
+   len - equal - 1);
+
+   if (equal == len)
+   die(skiplist requires a path);
+   init_skiplist(options, 

[PATCH v5 16/19] fsck: Support demoting errors to warnings

2015-06-18 Thread Johannes Schindelin
We already have support in `git receive-pack` to deal with some legacy
repositories which have non-fatal issues.

Let's make `git fsck` itself useful with such repositories, too, by
allowing users to ignore known issues, or at least demote those issues
to mere warnings.

Example: `git -c fsck.missingemail=ignore fsck` would hide
problems with missing emails in author, committer and tagger lines.

In the same spirit that `git receive-pack`'s usage of the fsck machinery
differs from `git fsck`'s – some of the non-fatal warnings in `git fsck`
are fatal with `git receive-pack` when receive.fsckObjects = true, for
example – we strictly separate the fsck.msg-id from the
receive.fsck.msg-id settings.

Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
---
 Documentation/config.txt | 11 +++
 builtin/fsck.c   | 12 
 t/t1450-fsck.sh  | 11 +++
 3 files changed, 34 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 306ab7a..41fd460 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1250,6 +1250,17 @@ filter.driver.smudge::
object to a worktree file upon checkout.  See
linkgit:gitattributes[5] for details.
 
+fsck.msg-id::
+   Allows overriding the message type (error, warn or ignore) of a
+   specific message ID such as `missingemail`.
++
+For convenience, fsck prefixes the error/warning with the message ID,
+e.g.  missingemail: invalid author/committer line - missing email means
+that setting `fsck.missingemail = ignore` will hide that issue.
++
+This feature is intended to support working with legacy repositories
+which cannot be repaired without disruptive changes.
+
 gc.aggressiveDepth::
The depth parameter used in the delta compression
algorithm used by 'git gc --aggressive'.  This defaults
diff --git a/builtin/fsck.c b/builtin/fsck.c
index fff38fe..6de9f3e 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -46,6 +46,16 @@ static int show_dangling = 1;
 #define DIRENT_SORT_HINT(de) ((de)-d_ino)
 #endif
 
+static int fsck_config(const char *var, const char *value, void *cb)
+{
+   if (skip_prefix(var, fsck., var)) {
+   fsck_set_msg_type(fsck_obj_options, var, -1, value, -1);
+   return 0;
+   }
+
+   return git_default_config(var, value, cb);
+}
+
 static void objreport(struct object *obj, const char *msg_type,
const char *err)
 {
@@ -646,6 +656,8 @@ int cmd_fsck(int argc, const char **argv, const char 
*prefix)
include_reflogs = 0;
}
 
+   git_config(fsck_config, NULL);
+
fsck_head_link();
fsck_object_dir(get_object_directory());
 
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 286a643..fe4bb03 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -287,6 +287,17 @@ test_expect_success 'rev-list --verify-objects with bad 
sha1' '
grep -q error: sha1 mismatch 63ff 
out
 '
 
+test_expect_success 'force fsck to ignore double author' '
+   git cat-file commit HEAD basis 
+   sed s/^author .*/,/ basis | tr , \\n multiple-authors 
+   new=$(git hash-object -t commit -w --stdin multiple-authors) 
+   test_when_finished remove_object $new 
+   git update-ref refs/heads/bogus $new 
+   test_when_finished git update-ref -d refs/heads/bogus 
+   test_must_fail git fsck 
+   git -c fsck.multipleauthors=ignore fsck
+'
+
 _bz='\0'
 _bz5=$_bz$_bz$_bz$_bz$_bz
 _bz20=$_bz5$_bz5$_bz5$_bz5
-- 
2.3.1.windows.1.9.g8c01ab4



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 17/19] fsck: Introduce `git fsck --quick`

2015-06-18 Thread Johannes Schindelin
This option avoids unpacking each and all objects, and just verifies the
connectivity. In particular with large repositories, this speeds up the
operation, at the expense of missing corrupt blobs and ignoring
unreachable objects, if any.

Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
---
 Documentation/git-fsck.txt |  7 ++-
 builtin/fsck.c |  7 ++-
 t/t1450-fsck.sh| 22 ++
 3 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-fsck.txt b/Documentation/git-fsck.txt
index 25c431d..b98fb43 100644
--- a/Documentation/git-fsck.txt
+++ b/Documentation/git-fsck.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 
 [verse]
 'git fsck' [--tags] [--root] [--unreachable] [--cache] [--no-reflogs]
-[--[no-]full] [--strict] [--verbose] [--lost-found]
+[--[no-]full] [--quick] [--strict] [--verbose] [--lost-found]
 [--[no-]dangling] [--[no-]progress] [object*]
 
 DESCRIPTION
@@ -60,6 +60,11 @@ index file, all SHA-1 references in `refs` namespace, and 
all reflogs
object pools.  This is now default; you can turn it off
with --no-full.
 
+--quick::
+   Check only the connectivity of tags, commits and tree objects. By
+   avoiding to unpack blobs, this speeds up the operation, at the
+   expense of missing corrupt objects.
+
 --strict::
Enable more strict checking, namely to catch a file mode
recorded with g+w bit set, which was created by older
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 6de9f3e..75fcb5f 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -23,6 +23,7 @@ static int show_tags;
 static int show_unreachable;
 static int include_reflogs = 1;
 static int check_full = 1;
+static int quick;
 static int check_strict;
 static int keep_cache_objects;
 static struct fsck_options fsck_walk_options = FSCK_OPTIONS_DEFAULT;
@@ -181,6 +182,8 @@ static void check_reachable_object(struct object *obj)
if (!(obj-flags  HAS_OBJ)) {
if (has_sha1_pack(obj-sha1))
return; /* it is in pack - forget about it */
+   if (quick  has_sha1_file(obj-sha1))
+   return;
printf(missing %s %s\n, typename(obj-type), 
sha1_to_hex(obj-sha1));
errors_found |= ERROR_REACHABLE;
return;
@@ -623,6 +626,7 @@ static struct option fsck_opts[] = {
OPT_BOOL(0, cache, keep_cache_objects, N_(make index objects head 
nodes)),
OPT_BOOL(0, reflogs, include_reflogs, N_(make reflogs head nodes 
(default))),
OPT_BOOL(0, full, check_full, N_(also consider packs and alternate 
objects)),
+   OPT_BOOL(0, quick, quick, N_(check only connectivity)),
OPT_BOOL(0, strict, check_strict, N_(enable more strict checking)),
OPT_BOOL(0, lost-found, write_lost_and_found,
N_(write dangling objects in 
.git/lost-found)),
@@ -659,7 +663,8 @@ int cmd_fsck(int argc, const char **argv, const char 
*prefix)
git_config(fsck_config, NULL);
 
fsck_head_link();
-   fsck_object_dir(get_object_directory());
+   if (!quick)
+   fsck_object_dir(get_object_directory());
 
prepare_alt_odb();
for (alt = alt_odb_list; alt; alt = alt-next) {
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index fe4bb03..471e2ea 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -431,4 +431,26 @@ test_expect_success 'fsck notices ref pointing to missing 
tag' '
test_must_fail git -C missing fsck
 '
 
+test_expect_success 'fsck --quick' '
+   rm -rf quick 
+   git init quick 
+   (
+   cd quick 
+   touch empty 
+   git add empty 
+   test_commit empty 
+   empty=.git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391 
+   rm -f $empty 
+   echo invalid $empty 
+   test_must_fail git fsck --strict 
+   git fsck --strict --quick 
+   tree=$(git rev-parse HEAD:) 
+   suffix=${tree#??} 
+   tree=.git/objects/${tree%$suffix}/$suffix 
+   rm -f $tree 
+   echo invalid $tree 
+   test_must_fail git fsck --strict --quick
+   )
+'
+
 test_done
-- 
2.3.1.windows.1.9.g8c01ab4



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/2] rebase -i: Fix left-behind CHERRY_PICK_HEAD

2015-06-18 Thread Johannes Schindelin
On 2015-06-18 21:45, Junio C Hamano wrote:

 This round looks good, except one trivial nit (below), which I'll
 locally squash-in a fix for.

Thanks,
Dscho
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/2] rebase -i: Fix left-behind CHERRY_PICK_HEAD

2015-06-18 Thread Junio C Hamano
Thanks.

This round looks good, except one trivial nit (below), which I'll
locally squash-in a fix for.

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index fb1b571..6938e5e 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1052,7 +1052,7 @@ test_expect_failure 'rebase --continue removes 
CHERRY_PICK_HEAD' '
git reset --hard HEAD~2 
git cherry-pick seq-onto 
set_fake_editor 
-   FAKE_LINES= test_must_fail git rebase -i seq-onto 
+   test_must_fail env FAKE_LINES= git rebase -i seq-onto 
test -d .git/rebase-merge 
git rebase --continue 
git diff --exit-code seq-onto 
-- 
2.4.4-569-gdcc90bb

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] format-patch: introduce format.outputDirectory configuration

2015-06-18 Thread Jeff King
On Thu, Jun 18, 2015 at 10:13:37AM -0700, Junio C Hamano wrote:

  -static const char *set_outdir(const char *prefix, const char 
  *output_directory)
  +static const char *set_outdir(const char *prefix, const char 
  *output_directory,
  + const char *config_output_directory)
 
 This change looks ugly and unnecessary.  All the machinery after and
 including the point set_outdir() is called, including reopen_stdout(),
 work on output_directory variable and only that variable.
 
 Wouldn't it work equally well to have
 
   if (!output_directory)
   output_directory = config_output_directory;
 
 before a call to set_outdir() is made but after the configuration is
 read (namely, soon after parse_options() returns), without making
 any change to this function?

Don't we load the config before parsing options here? In that case, we
can use our usual strategy to just set output_directory (which is
already a static global) from the config callback, and everything Just
Works.

We do have to bump the definition of output_directory up above the
config callback, like so (while we are here, we might also want to
drop the unnecessary static initializers, which violate our style guide):

diff --git a/builtin/log.c b/builtin/log.c
index e67671e..77c06f7 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -37,6 +37,10 @@ static int use_mailmap_config;
 static const char *fmt_patch_subject_prefix = PATCH;
 static const char *fmt_pretty;
 
+static FILE *realstdout = NULL;
+static const char *output_directory = NULL;
+static int outdir_offset;
+
 static const char * const builtin_log_usage[] = {
N_(git log [options] [revision-range] [[--] path...]),
N_(git show [options] object...),
@@ -752,14 +756,12 @@ static int git_format_config(const char *var, const char 
*value, void *cb)
config_cover_letter = git_config_bool(var, value) ? COVER_ON : 
COVER_OFF;
return 0;
}
+   if (!strcmp(var, format.outputdirectory))
+   return git_config_string(output_directory, var, value);
 
return git_log_config(var, value, cb);
 }
 
-static FILE *realstdout = NULL;
-static const char *output_directory = NULL;
-static int outdir_offset;
-
 static int reopen_stdout(struct commit *commit, const char *subject,
 struct rev_info *rev, int quiet)
 {
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] format-patch: introduce format.outputDirectory configuration

2015-06-18 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 Jeff King p...@peff.net writes:

 This change looks ugly and unnecessary.  All the machinery after and
 including the point set_outdir() is called, including reopen_stdout(),
 work on output_directory variable and only that variable.
 
 Wouldn't it work equally well to have
 
 if (!output_directory)
 output_directory = config_output_directory;
 
 before a call to set_outdir() is made but after the configuration is
 read (namely, soon after parse_options() returns), without making
 any change to this function?

 Don't we load the config before parsing options here? In that case, we
 can use our usual strategy to just set output_directory (which is
 already a static global) from the config callback, and everything Just
 Works.

 We do have to bump the definition of output_directory up above the
 config callback, like so (while we are here, we might also want to
 drop the unnecessary static initializers, which violate our style guide):

 You would also need to remove the oh you gave me -o twice? check,
 and change the semantics to later -o overrides an earlier one,
 wouldn't you?  Otherwise you would never be able to override what
 you read from the config, I am afraid.

By the way, I actually think later -o overrides an earlier one is
a good change by itself, regardless of this new configuration.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 00/19] Make git-pull a builtin

2015-06-18 Thread Junio C Hamano
Paul Tan pyoka...@gmail.com writes:

 This is a re-roll of [v3]. It squashes in Ramsay's patch fix some sparse
 warnings, and fixes the use-before-free reported by Duy. Thanks a lot for
 dealing with my mess :-).

 Other than that, there are no other changes as I'm working on the git-am side
 of things.

I didn't look carefully, but does that mean 04/19 has the what if
you start from a subdirectory and are still using the scripted one?
issue we discussed recently for am?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


  1   2   >