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

2015-06-14 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..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 
Signed-off-by: Paul Tan 
---

Notes:
v3

* Now that parse_config_rebase() takes care of the die()-ing and
  error()-ing, we only need one function again. Yay!

* The free()s is ugly though. Ideally, I would like to have a xstrfmt()
  function that returns a static buffer.

* We now don't lookup the pull.rebase config if the --rebase option is
  provided on the command-line.

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

diff --git a/builtin/pull.c b/builtin/pull.c
index 8915947..e4098d0 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,35 @@ 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)) {
+   free(key);
+   return parse_config_rebase(key, value, 1);
+   }
+
+   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 +736,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 v3 14/19] pull: set reflog message

2015-06-14 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 
Signed-off-by: Paul Tan 
---
 builtin/pull.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/builtin/pull.c b/builtin/pull.c
index a2dd0ba..a2c900e 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 v3 18/19] pull --rebase: error on no merge candidate cases

2015-06-14 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 
---
 builtin/pull.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index a379c1f..b78c67c 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -430,7 +430,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))) {
@@ -440,7 +443,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  ");
@@ -452,7 +458,10 @@ static void NORETURN die_no_merge_candidates(const char 
*repo, const char **refs
remote_name = "";
 
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  ");
-- 
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 v3 08/19] pull: pass git-fetch's options to git-fetch

2015-06-14 Thread Paul Tan
Since eb2a8d9 (pull: handle git-fetch's options as well, 2015-06-02),
git-pull knows about and handles git-fetch's options, passing them to
git-fetch. Re-implement this behavior.

Since 29609e6 (pull: do nothing on --dry-run, 2010-05-25) git-pull
supported the --dry-run option, exiting after git-fetch if --dry-run is
set. Re-implement this behavior.

Signed-off-by: Paul Tan 
---
 builtin/pull.c | 95 ++
 1 file changed, 95 insertions(+)

diff --git a/builtin/pull.c b/builtin/pull.c
index 0442da9..731e2a6 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -32,6 +32,21 @@ static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
 static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
 static char *opt_gpg_sign;
 
+/* Options passed to git-fetch */
+static char *opt_all;
+static char *opt_append;
+static char *opt_upload_pack;
+static int opt_force;
+static char *opt_tags;
+static char *opt_prune;
+static char *opt_recurse_submodules;
+static int opt_dry_run;
+static char *opt_keep;
+static char *opt_depth;
+static char *opt_unshallow;
+static char *opt_update_shallow;
+static char *opt_refmap;
+
 static struct option pull_options[] = {
/* Shared options */
OPT__VERBOSITY(&opt_verbosity),
@@ -82,6 +97,46 @@ static struct option pull_options[] = {
N_("GPG sign commit"),
PARSE_OPT_OPTARG),
 
+   /* Options passed to git-fetch */
+   OPT_GROUP(N_("Options related to fetching")),
+   OPT_PASSTHRU(0, "all", &opt_all, 0,
+   N_("fetch from all remotes"),
+   PARSE_OPT_NOARG),
+   OPT_PASSTHRU('a', "append", &opt_append, 0,
+   N_("append to .git/FETCH_HEAD instead of overwriting"),
+   PARSE_OPT_NOARG),
+   OPT_PASSTHRU(0, "upload-pack", &opt_upload_pack, N_("path"),
+   N_("path to upload pack on remote end"),
+   0),
+   OPT__FORCE(&opt_force, N_("force overwrite of local branch")),
+   OPT_PASSTHRU('t', "tags", &opt_tags, 0,
+   N_("fetch all tags and associated objects"),
+   PARSE_OPT_NOARG),
+   OPT_PASSTHRU('p', "prune", &opt_prune, 0,
+   N_("prune remote-tracking branches no longer on remote"),
+   PARSE_OPT_NOARG),
+   OPT_PASSTHRU(0, "recurse-submodules", &opt_recurse_submodules,
+   N_("on-demand"),
+   N_("control recursive fetching of submodules"),
+   PARSE_OPT_OPTARG),
+   OPT_BOOL(0, "dry-run", &opt_dry_run,
+   N_("dry run")),
+   OPT_PASSTHRU('k', "keep", &opt_keep, 0,
+   N_("keep downloaded pack"),
+   PARSE_OPT_NOARG),
+   OPT_PASSTHRU(0, "depth", &opt_depth, N_("depth"),
+   N_("deepen history of shallow clone"),
+   0),
+   OPT_PASSTHRU(0, "unshallow", &opt_unshallow, 0,
+   N_("convert to a complete repository"),
+   PARSE_OPT_NONEG | PARSE_OPT_NOARG),
+   OPT_PASSTHRU(0, "update-shallow", &opt_update_shallow, 0,
+   N_("accept refs that update .git/shallow"),
+   PARSE_OPT_NOARG),
+   OPT_PASSTHRU(0, "refmap", &opt_refmap, N_("refmap"),
+   N_("specify fetch refmap"),
+   PARSE_OPT_NONEG),
+
OPT_END()
 };
 
@@ -100,6 +155,16 @@ static void argv_push_verbosity(struct argv_array *arr)
 }
 
 /**
+ * Pushes "-f" switches into arr to match the opt_force level.
+ */
+static void argv_push_force(struct argv_array *arr)
+{
+   int force = opt_force;
+   while (force-- > 0)
+   argv_array_push(arr, "-f");
+}
+
+/**
  * Parses argv into [ [...]], returning their values in `repo`
  * as a string and `refspecs` as a null-terminated array of strings. If `repo`
  * is not provided in argv, it is set to NULL.
@@ -131,6 +196,33 @@ static int run_fetch(const char *repo, const char 
**refspecs)
if (opt_progress)
argv_array_push(&args, opt_progress);
 
+   /* Options passed to git-fetch */
+   if (opt_all)
+   argv_array_push(&args, opt_all);
+   if (opt_append)
+   argv_array_push(&args, opt_append);
+   if (opt_upload_pack)
+   argv_array_push(&args, opt_upload_pack);
+   argv_push_force(&args);
+   if (opt_tags)
+   argv_array_push(&args, opt_tags);
+   if (opt_prune)
+   argv_array_push(&args, opt_prune);
+   if (opt_recurse_submodules)
+

[PATCH v3 10/19] pull: support pull.ff config

2015-06-14 Thread Paul Tan
Since b814da8 (pull: add pull.ff configuration, 2014-01-15), git-pull.sh
would lookup the configuration value of "pull.ff", and set the flag
"--ff" if its value is "true", "--no-ff" if its value is "false" and
"--ff-only" if its value is "only".

Re-implement this behavior.

Signed-off-by: Paul Tan 
---

Notes:
v3

* style fixes

 builtin/pull.c | 29 +
 1 file changed, 29 insertions(+)

diff --git a/builtin/pull.c b/builtin/pull.c
index 83691fc..0ddb964 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -167,6 +167,32 @@ static void argv_push_force(struct argv_array *arr)
 }
 
 /**
+ * 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
+ * error.
+ */
+static const char *config_get_ff(void)
+{
+   const char *value;
+
+   if (git_config_get_value("pull.ff", &value))
+   return NULL;
+
+   switch (git_config_maybe_bool("pull.ff", value)) {
+   case 0:
+   return "--no-ff";
+   case 1:
+   return "--ff";
+   }
+
+   if (!strcmp(value, "only"))
+   return "--ff-only";
+
+   die(_("Invalid value for pull.ff: %s"), value);
+}
+
+/**
  * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge
  * into merge_heads.
  */
@@ -397,6 +423,9 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
 
parse_repo_refspecs(argc, argv, &repo, &refspecs);
 
+   if (!opt_ff)
+   opt_ff = xstrdup_or_null(config_get_ff());
+
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 v3 05/19] pull: implement fetch + merge

2015-06-14 Thread Paul Tan
Implement the fetch + merge functionality of git-pull, by first running
git-fetch with the repo and refspecs provided on the command line, then
running git-merge on FETCH_HEAD to merge the fetched refs into the
current branch.

Helped-by: Junio C Hamano 
Signed-off-by: Paul Tan 
---

Notes:
v3

* Catch bug where there is are refspecs but no repo.

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

diff --git a/builtin/pull.c b/builtin/pull.c
index cabeed4..9157536 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -9,8 +9,10 @@
 #include "builtin.h"
 #include "parse-options.h"
 #include "exec_cmd.h"
+#include "run-command.h"
 
 static const char * const pull_usage[] = {
+   N_("git pull [options] [ [...]]"),
NULL
 };
 
@@ -18,8 +20,61 @@ static struct option pull_options[] = {
OPT_END()
 };
 
+/**
+ * Parses argv into [ [...]], returning their values in `repo`
+ * as a string and `refspecs` as a null-terminated array of strings. If `repo`
+ * is not provided in argv, it is set to NULL.
+ */
+static void parse_repo_refspecs(int argc, const char **argv, const char **repo,
+   const char ***refspecs)
+{
+   if (argc > 0) {
+   *repo = *argv++;
+   argc--;
+   } else
+   *repo = NULL;
+   *refspecs = argv;
+}
+
+/**
+ * Runs git-fetch, returning its exit status. `repo` and `refspecs` are the
+ * repository and refspecs to fetch, or NULL if they are not provided.
+ */
+static int run_fetch(const char *repo, const char **refspecs)
+{
+   struct argv_array args = ARGV_ARRAY_INIT;
+   int ret;
+
+   argv_array_pushl(&args, "fetch", "--update-head-ok", NULL);
+   if (repo) {
+   argv_array_push(&args, repo);
+   argv_array_pushv(&args, refspecs);
+   } else if (*refspecs)
+   die("BUG: refspecs without repo?");
+   ret = run_command_v_opt(args.argv, RUN_GIT_CMD);
+   argv_array_clear(&args);
+   return ret;
+}
+
+/**
+ * Runs git-merge, returning its exit status.
+ */
+static int run_merge(void)
+{
+   int ret;
+   struct argv_array args = ARGV_ARRAY_INIT;
+
+   argv_array_pushl(&args, "merge", NULL);
+   argv_array_push(&args, "FETCH_HEAD");
+   ret = run_command_v_opt(args.argv, RUN_GIT_CMD);
+   argv_array_clear(&args);
+   return ret;
+}
+
 int cmd_pull(int argc, const char **argv, const char *prefix)
 {
+   const char *repo, **refspecs;
+
if (!getenv("_GIT_USE_BUILTIN_PULL")) {
const char *path = mkpath("%s/git-pull", git_exec_path());
 
@@ -29,5 +84,10 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 
argc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0);
 
-   return 0;
+   parse_repo_refspecs(argc, argv, &repo, &refspecs);
+
+   if (run_fetch(repo, refspecs))
+   return 1;
+
+   return run_merge();
 }
-- 
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 v3 03/19] argv-array: implement argv_array_pushv()

2015-06-14 Thread Paul Tan
When we have a null-terminated array, it would be useful to convert it
or append it to an argv_array for further manipulation.

Implement argv_array_pushv() which will push a null-terminated array of
strings on to an argv_array.

Signed-off-by: Paul Tan 
---
 Documentation/technical/api-argv-array.txt | 3 +++
 argv-array.c   | 6 ++
 argv-array.h   | 1 +
 3 files changed, 10 insertions(+)

diff --git a/Documentation/technical/api-argv-array.txt 
b/Documentation/technical/api-argv-array.txt
index 1a79781..8076172 100644
--- a/Documentation/technical/api-argv-array.txt
+++ b/Documentation/technical/api-argv-array.txt
@@ -46,6 +46,9 @@ Functions
Format a string and push it onto the end of the array. This is a
convenience wrapper combining `strbuf_addf` and `argv_array_push`.
 
+`argv_array_pushv`::
+   Push a null-terminated array of strings onto the end of the array.
+
 `argv_array_pop`::
Remove the final element from the array. If there are no
elements in the array, do nothing.
diff --git a/argv-array.c b/argv-array.c
index 256741d..eaed477 100644
--- a/argv-array.c
+++ b/argv-array.c
@@ -49,6 +49,12 @@ void argv_array_pushl(struct argv_array *array, ...)
va_end(ap);
 }
 
+void argv_array_pushv(struct argv_array *array, const char **argv)
+{
+   for (; *argv; argv++)
+   argv_array_push(array, *argv);
+}
+
 void argv_array_pop(struct argv_array *array)
 {
if (!array->argc)
diff --git a/argv-array.h b/argv-array.h
index c65e6e8..a2fa0aa 100644
--- a/argv-array.h
+++ b/argv-array.h
@@ -17,6 +17,7 @@ __attribute__((format (printf,2,3)))
 void argv_array_pushf(struct argv_array *, const char *fmt, ...);
 LAST_ARG_MUST_BE_NULL
 void argv_array_pushl(struct argv_array *, ...);
+void argv_array_pushv(struct argv_array *, const char **);
 void argv_array_pop(struct argv_array *);
 void argv_array_clear(struct argv_array *);
 
-- 
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 v3 01/19] parse-options-cb: implement parse_opt_passthru()

2015-06-14 Thread Paul Tan
Certain git commands, such as git-pull, are simply wrappers around other
git commands like git-fetch, git-merge and git-rebase. As such, these
wrapper commands will typically need to "pass through" command-line
options of the commands they wrap.

Implement the parse_opt_passthru() parse-options callback, which will
reconstruct the command-line option into an char* string, such that it
can be passed to another git command.

Helped-by: Johannes Schindelin 
Helped-by: Junio C Hamano 
Helped-by: Stefan Beller 
Signed-off-by: Paul Tan 
---

Notes:
v3

* Reverted back to returning newly-allocated strings. Junio raised the
  concern that it may not be such a good idea to burden the users of the
  API to always use X.buf to access the string. Personally I believe
  that memory management safety is usually more important, but I don't
  have strong feelings about this.

* Extracted out the "option reconstruction" logic into a private
  function so that it can be shared with parse_opt_passthru_argv() in
  the next patch.

* Introduced the OPT_PASSTHRU() macro and documented it in
  Documentation/technical/api-parse-options.txt. This macro relieves the
  user of having to specify OPTION_CALLBACK and parse_opt_passthru() for
  every option.

* The function used to be named parse_opt_pass_strbuf() to save
  horizontal space, but then again parse_opt_passthru() is probably a
  better name.

* Added comment to the docstring that the callback should only be used
  for options where the last one wins.

 Documentation/technical/api-parse-options.txt |  7 
 parse-options-cb.c| 49 +++
 parse-options.h   |  3 ++
 3 files changed, 59 insertions(+)

diff --git a/Documentation/technical/api-parse-options.txt 
b/Documentation/technical/api-parse-options.txt
index 1f2db31..85d10ab 100644
--- a/Documentation/technical/api-parse-options.txt
+++ b/Documentation/technical/api-parse-options.txt
@@ -212,6 +212,13 @@ There are some macros to easily define options:
Use it to hide deprecated options that are still to be recognized
and ignored silently.
 
+`OPT_PASSTHRU(short, long, &char_var, arg_str, description, flags)`::
+   Introduce an option that will be reconstructed into a char* string,
+   which must be initialized to NULL. This is useful when you need to
+   pass the command-line option to another command. Any previous value
+   will be overwritten, so this should only be used for options where
+   the last one specified on the command line wins.
+
 
 The last element of the array must be `OPT_END()`.
 
diff --git a/parse-options-cb.c b/parse-options-cb.c
index be8c413..68bc593 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -134,3 +134,52 @@ int parse_opt_noop_cb(const struct option *opt, const char 
*arg, int unset)
 {
return 0;
 }
+
+/**
+ * Recreates the command-line option in the strbuf.
+ */
+static int recreate_opt(struct strbuf *sb, const struct option *opt,
+   const char *arg, int unset)
+{
+   strbuf_reset(sb);
+
+   if (opt->long_name) {
+   strbuf_addstr(sb, unset ? "--no-" : "--");
+   strbuf_addstr(sb, opt->long_name);
+   if (arg) {
+   strbuf_addch(sb, '=');
+   strbuf_addstr(sb, arg);
+   }
+   } else if (opt->short_name && !unset) {
+   strbuf_addch(sb, '-');
+   strbuf_addch(sb, opt->short_name);
+   if (arg)
+   strbuf_addstr(sb, arg);
+   } else
+   return -1;
+
+   return 0;
+}
+
+/**
+ * For an option opt, recreates the command-line option in opt->value which
+ * must be an char* initialized to NULL. This is useful when we need to pass
+ * the command-line option to another command. Since any previous value will be
+ * overwritten, this callback should only be used for options where the last
+ * one wins.
+ */
+int parse_opt_passthru(const struct option *opt, const char *arg, int unset)
+{
+   static struct strbuf sb = STRBUF_INIT;
+   char **opt_value = opt->value;
+
+   if (recreate_opt(&sb, opt, arg, unset) < 0)
+   return -1;
+
+   if (*opt_value)
+   free(*opt_value);
+
+   *opt_value = strbuf_detach(&sb, NULL);
+
+   return 0;
+}
diff --git a/parse-options.h b/parse-options.h
index c71e9da..5b0f886 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -224,6 +224,7 @@ extern int parse_opt_with_commit(const struct option *, 
const char *, int);
 extern int parse_opt_tertiary(const struct option *, const char *, int);
 extern int parse_opt_string_list(const struct option *, const char *, int);
 extern int parse_opt_noop_cb(const st

[PATCH v3 09/19] pull: error on no merge candidates

2015-06-14 Thread Paul Tan
Commit a8c9bef (pull: improve advice for unconfigured error case,
2009-10-05) fully established the current advices given by git-pull for
the different cases where git-fetch will not have anything marked for
merge:

1. We fetched from a specific remote, and a refspec was given, but it
   ended up not fetching anything. This is usually because the user
   provided a wildcard refspec which had no matches on the remote end.

2. We fetched from a non-default remote, but didn't specify a branch to
   merge. We can't use the configured one because it applies to the
   default remote, and thus the user must specify the branches to merge.

3. We fetched from the branch's or repo's default remote, but:

   a. We are not on a branch, so there will never be a configured branch
  to merge with.

   b. We are on a branch, but there is no configured branch to merge
  with.

4. We fetched from the branch's or repo's default remote, but the
   configured branch to merge didn't get fetched (either it doesn't
   exist, or wasn't part of the configured fetch refspec)

Re-implement the above behavior by implementing get_merge_heads() to
parse the heads in FETCH_HEAD for merging, and implementing
die_no_merge_candidates(), which will be called when FETCH_HEAD has no
heads for merging.

Helped-by: Johannes Schindelin 
Helped-by: Junio C Hamano 
Signed-off-by: Paul Tan 
---

Notes:
v3

* Tightening up of FETCH_HEAD parsing code and style fixes.

 builtin/pull.c | 113 +
 1 file changed, 113 insertions(+)

diff --git a/builtin/pull.c b/builtin/pull.c
index 731e2a6..83691fc 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -10,6 +10,8 @@
 #include "parse-options.h"
 #include "exec_cmd.h"
 #include "run-command.h"
+#include "sha1-array.h"
+#include "remote.h"
 
 static const char * const pull_usage[] = {
N_("git pull [options] [ [...]]"),
@@ -165,6 +167,111 @@ static void argv_push_force(struct argv_array *arr)
 }
 
 /**
+ * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge
+ * into merge_heads.
+ */
+static void get_merge_heads(struct sha1_array *merge_heads)
+{
+   const char *filename = git_path("FETCH_HEAD");
+   FILE *fp;
+   struct strbuf sb = STRBUF_INIT;
+   unsigned char sha1[GIT_SHA1_RAWSZ];
+
+   if (!(fp = fopen(filename, "r")))
+   die_errno(_("could not open '%s' for reading"), filename);
+   while (strbuf_getline(&sb, fp, '\n') != EOF) {
+   if (get_sha1_hex(sb.buf, sha1))
+   continue;  /* invalid line: does not start with SHA1 */
+   if (starts_with(sb.buf + GIT_SHA1_HEXSZ, "\tnot-for-merge\t"))
+   continue;  /* ref is not-for-merge */
+   sha1_array_append(merge_heads, sha1);
+   }
+   fclose(fp);
+   strbuf_release(&sb);
+}
+
+/**
+ * Used by die_no_merge_candidates() as a for_each_remote() callback to
+ * retrieve the name of the remote if the repository only has one remote.
+ */
+static int get_only_remote(struct remote *remote, void *cb_data)
+{
+   const char **remote_name = cb_data;
+
+   if (*remote_name)
+   return -1;
+
+   *remote_name = remote->name;
+   return 0;
+}
+
+/**
+ * Dies with the appropriate reason for why there are no merge candidates:
+ *
+ * 1. We fetched from a specific remote, and a refspec was given, but it ended
+ *up not fetching anything. This is usually because the user provided a
+ *wildcard refspec which had no matches on the remote end.
+ *
+ * 2. We fetched from a non-default remote, but didn't specify a branch to
+ *merge. We can't use the configured one because it applies to the default
+ *remote, thus the user must specify the branches to merge.
+ *
+ * 3. We fetched from the branch's or repo's default remote, but:
+ *
+ *a. We are not on a branch, so there will never be a configured branch to
+ *   merge with.
+ *
+ *b. We are on a branch, but there is no configured branch to merge with.
+ *
+ * 4. We fetched from the branch's or repo's default remote, but the configured
+ *branch to merge didn't get fetched. (Either it doesn't exist, or wasn't
+ *part of the configured fetch refspec.)
+ */
+static void NORETURN die_no_merge_candidates(const char *repo, const char 
**refspecs)
+{
+   struct branch *curr_branch = branch_get("HEAD");
+   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."));
+   fprintf_ln(stderr, _("Generally this means that you provided a 

[PATCH v3 02/19] parse-options-cb: implement parse_opt_passthru_argv()

2015-06-14 Thread Paul Tan
Certain git commands, such as git-pull, are simply wrappers around other
git commands like git-fetch, git-merge and git-rebase. As such, these
wrapper commands will typically need to "pass through" command-line
options of the commands they wrap.

Implement the parse_opt_passthru_argv() parse-options callback, which
will reconstruct all the provided command-line options into an
argv_array, such that it can be passed to another git command. This is
useful for passing command-line options that can be specified multiple
times.

Helped-by: Stefan Beller 
Signed-off-by: Paul Tan 
---

Notes:
v3

* Renamed function to the more descriptive parse_opt_passthru_argv().

* Introduced and documented OPT_PASSTHRU_ARGV() macro, which saves the
  user from having to specify OPTION_CALLBACK and
  parse_opt_passthru_argv() for each option.

 Documentation/technical/api-parse-options.txt |  6 ++
 parse-options-cb.c| 20 
 parse-options.h   |  3 +++
 3 files changed, 29 insertions(+)

diff --git a/Documentation/technical/api-parse-options.txt 
b/Documentation/technical/api-parse-options.txt
index 85d10ab..0b0ab01 100644
--- a/Documentation/technical/api-parse-options.txt
+++ b/Documentation/technical/api-parse-options.txt
@@ -219,6 +219,12 @@ There are some macros to easily define options:
will be overwritten, so this should only be used for options where
the last one specified on the command line wins.
 
+`OPT_PASSTHRU_ARGV(short, long, &argv_array_var, arg_str, description, 
flags)`::
+   Introduce an option where all instances of it on the command-line will
+   be reconstructed into an argv_array. This is useful when you need to
+   pass the command-line option, which can be specified multiple times,
+   to another command.
+
 
 The last element of the array must be `OPT_END()`.
 
diff --git a/parse-options-cb.c b/parse-options-cb.c
index 68bc593..5ab6ed6 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -4,6 +4,7 @@
 #include "commit.h"
 #include "color.h"
 #include "string-list.h"
+#include "argv-array.h"
 
 /*- some often used options -*/
 
@@ -183,3 +184,22 @@ int parse_opt_passthru(const struct option *opt, const 
char *arg, int unset)
 
return 0;
 }
+
+/**
+ * For an option opt, recreate the command-line option, appending it to
+ * opt->value which must be a argv_array. This is useful when we need to pass
+ * the command-line option, which can be specified multiple times, to another
+ * command.
+ */
+int parse_opt_passthru_argv(const struct option *opt, const char *arg, int 
unset)
+{
+   static struct strbuf sb = STRBUF_INIT;
+   struct argv_array *opt_value = opt->value;
+
+   if (recreate_opt(&sb, opt, arg, unset) < 0)
+   return -1;
+
+   argv_array_push(opt_value, sb.buf);
+
+   return 0;
+}
diff --git a/parse-options.h b/parse-options.h
index 5b0f886..aba06688 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -225,6 +225,7 @@ extern int parse_opt_tertiary(const struct option *, const 
char *, int);
 extern int parse_opt_string_list(const struct option *, const char *, int);
 extern int parse_opt_noop_cb(const struct option *, const char *, int);
 extern int parse_opt_passthru(const struct option *, const char *, int);
+extern int parse_opt_passthru_argv(const struct option *, const char *, int);
 
 #define OPT__VERBOSE(var, h)  OPT_COUNTUP('v', "verbose", (var), (h))
 #define OPT__QUIET(var, h)OPT_COUNTUP('q', "quiet",   (var), (h))
@@ -245,5 +246,7 @@ extern int parse_opt_passthru(const struct option *, const 
char *, int);
{ OPTION_CALLBACK, (s), (l), (v), N_("style"), (h), PARSE_OPT_OPTARG, 
parseopt_column_callback }
 #define OPT_PASSTHRU(s, l, v, a, h, f) \
{ OPTION_CALLBACK, (s), (l), (v), (a), (h), (f), parse_opt_passthru }
+#define OPT_PASSTHRU_ARGV(s, l, v, a, h, f) \
+   { OPTION_CALLBACK, (s), (l), (v), (a), (h), (f), 
parse_opt_passthru_argv }
 
 #endif
-- 
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 v3 04/19] pull: implement skeletal builtin pull

2015-06-14 Thread Paul Tan
For the purpose of rewriting git-pull.sh into a C builtin, implement a
skeletal builtin/pull.c that redirects to $GIT_EXEC_PATH/git-pull.sh if
the environment variable _GIT_USE_BUILTIN_PULL is not defined. This
allows us to fall back on the functional git-pull.sh when running the
test suite for tests that depend on a working git-pull implementation.

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

Helped-by: Junio C Hamano 
Signed-off-by: Paul Tan 
---

Notes:
v3

* style fixes

 Makefile   |  1 +
 builtin.h  |  1 +
 builtin/pull.c | 33 +
 git.c  |  1 +
 4 files changed, 36 insertions(+)
 create mode 100644 builtin/pull.c

diff --git a/Makefile b/Makefile
index 54ec511..2057a9d 100644
--- a/Makefile
+++ b/Makefile
@@ -877,6 +877,7 @@ BUILTIN_OBJS += builtin/pack-refs.o
 BUILTIN_OBJS += builtin/patch-id.o
 BUILTIN_OBJS += builtin/prune-packed.o
 BUILTIN_OBJS += builtin/prune.o
+BUILTIN_OBJS += builtin/pull.o
 BUILTIN_OBJS += builtin/push.o
 BUILTIN_OBJS += builtin/read-tree.o
 BUILTIN_OBJS += builtin/receive-pack.o
diff --git a/builtin.h b/builtin.h
index b87df70..ea3c834 100644
--- a/builtin.h
+++ b/builtin.h
@@ -98,6 +98,7 @@ extern int cmd_pack_redundant(int argc, const char **argv, 
const char *prefix);
 extern int cmd_patch_id(int argc, const char **argv, const char *prefix);
 extern int cmd_prune(int argc, const char **argv, const char *prefix);
 extern int cmd_prune_packed(int argc, const char **argv, const char *prefix);
+extern int cmd_pull(int argc, const char **argv, const char *prefix);
 extern int cmd_push(int argc, const char **argv, const char *prefix);
 extern int cmd_read_tree(int argc, const char **argv, const char *prefix);
 extern int cmd_receive_pack(int argc, const char **argv, const char *prefix);
diff --git a/builtin/pull.c b/builtin/pull.c
new file mode 100644
index 000..cabeed4
--- /dev/null
+++ b/builtin/pull.c
@@ -0,0 +1,33 @@
+/*
+ * Builtin "git pull"
+ *
+ * Based on git-pull.sh by Junio C Hamano
+ *
+ * Fetch one or more remote refs and merge it/them into the current HEAD.
+ */
+#include "cache.h"
+#include "builtin.h"
+#include "parse-options.h"
+#include "exec_cmd.h"
+
+static const char * const pull_usage[] = {
+   NULL
+};
+
+static struct option pull_options[] = {
+   OPT_END()
+};
+
+int cmd_pull(int argc, const char **argv, const char *prefix)
+{
+   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);
+   }
+
+   argc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0);
+
+   return 0;
+}
diff --git a/git.c b/git.c
index 44374b1..e7a7713 100644
--- a/git.c
+++ b/git.c
@@ -445,6 +445,7 @@ static struct cmd_struct commands[] = {
{ "pickaxe", cmd_blame, RUN_SETUP },
{ "prune", cmd_prune, RUN_SETUP },
{ "prune-packed", cmd_prune_packed, RUN_SETUP },
+   { "pull", cmd_pull, RUN_SETUP | NEED_WORK_TREE },
{ "push", cmd_push, RUN_SETUP },
{ "read-tree", cmd_read_tree, RUN_SETUP },
{ "receive-pack", cmd_receive_pack },
-- 
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 v3 06/19] pull: pass verbosity, --progress flags to fetch and merge

2015-06-14 Thread Paul Tan
7f87aff (Teach/Fix pull/fetch -q/-v options, 2008-11-15) taught git-pull
to accept the verbosity -v and -q options and pass them to git-fetch and
git-merge.

Re-implement support for the verbosity flags by adding it to the options
list and introducing argv_push_verbosity() to push the flags into the
argv array used to execute git-fetch and git-merge.

9839018 (fetch and pull: learn --progress, 2010-02-24) and bebd2fd
(pull: propagate --progress to merge, 2011-02-20) taught git-pull to
accept the --progress option and pass it to git-fetch and git-merge.

Use OPT_PASSTHRU() implemented earlier to pass the "--[no-]progress"
command line options to git-fetch and git-merge.

Helped-by: Junio C Hamano 
Signed-off-by: Paul Tan 
---

Notes:
v3

* Re-worded commit message.

 builtin/pull.c | 36 
 1 file changed, 36 insertions(+)

diff --git a/builtin/pull.c b/builtin/pull.c
index 9157536..5d9f2b5 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -16,11 +16,35 @@ static const char * const pull_usage[] = {
NULL
 };
 
+/* Shared options */
+static int opt_verbosity;
+static char *opt_progress;
+
 static struct option pull_options[] = {
+   /* Shared options */
+   OPT__VERBOSITY(&opt_verbosity),
+   OPT_PASSTHRU(0, "progress", &opt_progress, NULL,
+   N_("force progress reporting"),
+   PARSE_OPT_NOARG),
+
OPT_END()
 };
 
 /**
+ * Pushes "-q" or "-v" switches into arr to match the opt_verbosity level.
+ */
+static void argv_push_verbosity(struct argv_array *arr)
+{
+   int verbosity;
+
+   for (verbosity = opt_verbosity; verbosity > 0; verbosity--)
+   argv_array_push(arr, "-v");
+
+   for (verbosity = opt_verbosity; verbosity < 0; verbosity++)
+   argv_array_push(arr, "-q");
+}
+
+/**
  * Parses argv into [ [...]], returning their values in `repo`
  * as a string and `refspecs` as a null-terminated array of strings. If `repo`
  * is not provided in argv, it is set to NULL.
@@ -46,6 +70,12 @@ static int run_fetch(const char *repo, const char **refspecs)
int ret;
 
argv_array_pushl(&args, "fetch", "--update-head-ok", NULL);
+
+   /* Shared options */
+   argv_push_verbosity(&args);
+   if (opt_progress)
+   argv_array_push(&args, opt_progress);
+
if (repo) {
argv_array_push(&args, repo);
argv_array_pushv(&args, refspecs);
@@ -65,6 +95,12 @@ static int run_merge(void)
struct argv_array args = ARGV_ARRAY_INIT;
 
argv_array_pushl(&args, "merge", NULL);
+
+   /* Shared options */
+   argv_push_verbosity(&args);
+   if (opt_progress)
+   argv_array_push(&args, opt_progress);
+
argv_array_push(&args, "FETCH_HEAD");
ret = run_command_v_opt(args.argv, RUN_GIT_CMD);
argv_array_clear(&args);
-- 
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 v3 00/19] Make git-pull a builtin

2015-06-14 Thread Paul Tan
This is a re-roll of [v2]. Thanks Junio, Stefan for the reviews last round.

Previous versions:

[v1] http://thread.gmane.org/gmane.comp.version-control.git/269258
[v2] http://thread.gmane.org/gmane.comp.version-control.git/270639

git-pull is a commonly executed command to check for new changes in the
upstream repository and, if there are, fetch and integrate them into the
current branch. Currently it is implemented by the shell script git-pull.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 series rewrites git-pull.sh into a C builtin, thus improving its
performance and portability. It is part of my GSoC project to rewrite git-pull
and git-am into builtins[1].

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


Paul Tan (19):
  parse-options-cb: implement parse_opt_passthru()
  parse-options-cb: implement parse_opt_passthru_argv()
  argv-array: implement argv_array_pushv()
  pull: implement skeletal builtin pull
  pull: implement fetch + merge
  pull: pass verbosity, --progress flags to fetch and merge
  pull: pass git-merge's options to git-merge
  pull: pass git-fetch's options to git-fetch
  pull: error on no merge candidates
  pull: support pull.ff config
  pull: check if in unresolved merge state
  pull: fast-forward working tree if head is updated
  pull: implement pulling into an unborn branch
  pull: set reflog message
  pull: teach git pull about --rebase
  pull: configure --rebase via branch..rebase or pull.rebase
  pull --rebase: exit early when the working directory is dirty
  pull --rebase: error on no merge candidate cases
  pull: remove redirection to git-pull.sh

 Documentation/technical/api-argv-array.txt|   3 +
 Documentation/technical/api-parse-options.txt |  13 +
 Makefile  |   2 +-
 advice.c  |   8 +
 advice.h  |   1 +
 argv-array.c  |   6 +
 argv-array.h  |   1 +
 builtin.h |   1 +
 builtin/pull.c| 881 ++
 git-pull.sh => contrib/examples/git-pull.sh   |   0
 git.c |   1 +
 parse-options-cb.c|  69 ++
 parse-options.h   |   6 +
 13 files changed, 991 insertions(+), 1 deletion(-)
 create mode 100644 builtin/pull.c
 rename git-pull.sh => contrib/examples/git-pull.sh (100%)

-- 
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 v3 11/19] pull: check if in unresolved merge state

2015-06-14 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 
---
 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 0ddb964..cb5898a 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] [ [...]]"),
@@ -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: [PATCH v2 11/19] pull: check if in unresolved merge state

2015-06-14 Thread Paul Tan
On Thu, Jun 11, 2015 at 1:14 AM, Junio C Hamano  wrote:
> I (or at least some part of me) actually view git_config_get_*() as
> "if you are only going to peek a few variables, you do not have to
> do the looping yourself" convenience, which leads me (or at least
> that part of me) to say "if you are doing the looping anyway, you
> may be better off picking up the variables you care about yourself
> in that loop".

Not just a convenience, but I personally find that callback functions
usually leads to code that is harder to understand because of the use
of static/global variables to preserve state and the fact that it is
harder to break it up into smaller functions to reason about. This is
just a matter of taste though, so I don't have strong feelings about
it.

Besides, there is a greater technical reason why we should not use
git_config(): It essentially performs a linear search[1], while the
git_config_get_*() functions do a constant-time lookup. Ideally, we
should remove uses of git_config() for users that have no need to
iterate over all the configuration entries.

[1] Since we do a strcmp() for every supported config setting, for
every config entry.

I should emphasize that the call to git_config(git_default_config,
NULL) is just a workaround to load the advice.* settings. In fact,
git_default_config() only peeks at a few config settings anyway, and
can be re-written to not iterate over all the user's config entries.
As such, I don't see why the builtin/pull.c code should be written to
support the git_config() way of doing things, even if we have to use
the workaround of calling git_config(). It's like saying: we have a
bad solution, now let's make it worse ;-)

> By calling git_config() before calling any git_config_get_*()
> functions, you would be priming the cache layer with the entire
> contents of the configuration files anyway, so later calls to
> git_config_get_*() will read from that cache, so there is no
> performance penalty in mixing the two methods to access
> configuration data.  That is why I felt less certain that the
> suggestion to stick to one method (instead of mixing the two) is a
> good thing to do (hence "less certain 'might'").

Right, although I think that the performance penalty due to using
git_config() is greater, and switching all the git_config_get_*()
calls to use it would just make it worse.

By the way, I got the idea that git development was moving towards
replacing use of git_config() from 5801d3b (builtin/gc.c: replace
`git_config()` with `git_config_get_*()` family, 2014-08-07).

Thanks,
Paul
--
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 v2 09/19] pull: error on no merge candidates

2015-06-12 Thread Paul Tan
On Wed, Jun 10, 2015 at 7:56 AM, Junio C Hamano  wrote:
> Paul Tan  writes:
>
>>  /**
>> + * Appends merge candidates from FETCH_HEAD that are not marked 
>> not-for-merge
>> + * into merge_heads.
>> + */
>
> Hmph, I vaguely recall doing that in C elsewhere already, even
> though I do not remember where offhand...

Right, handle_fetch_head() in builtin/merge.c. It looks up the commit
IDs into commit objects though, which is not required for git-pull. We
only need the list of hashes.

>> +static void get_merge_heads(struct sha1_array *merge_heads)
>> +{
>> + const char *filename = git_path("FETCH_HEAD");
>> + FILE *fp;
>> + struct strbuf sb = STRBUF_INIT;
>> + unsigned char sha1[GIT_SHA1_RAWSZ];
>> +
>> + if (!(fp = fopen(filename, "r")))
>> + die_errno(_("could not open '%s' for reading"), filename);
>> + while(strbuf_getline(&sb, fp, '\n') != EOF) {
>
> Missing SP after "while"

OK

>> + if (get_sha1_hex(sb.buf, sha1))
>> + continue;  /* invalid line: does not start with SHA1 */
>> + if (starts_with(sb.buf + GIT_SHA1_HEXSZ, "\tnot-for-merge"))
>
> Look for "\tnot-for-merge\t" instead?

Right, it's better to be stricter.

Thanks,
Paul
--
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] t0302: "unreadable" test needs SANITY prereq

2015-06-12 Thread Paul Tan
The test expects that "chmod -r ~/.git-credentials" would make it
unreadable to the user, and thus needs the SANITY prerequisite.

Reported-by: Jean-Yves LENHOF 
Signed-off-by: Paul Tan 
---
 t/t0302-credential-store.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh
index 0979df9..1d8d1f2 100755
--- a/t/t0302-credential-store.sh
+++ b/t/t0302-credential-store.sh
@@ -75,7 +75,7 @@ test_expect_success 'get: use xdg file if home file has no 
matches' '
EOF
 '
 
-test_expect_success POSIXPERM 'get: use xdg file if home file is unreadable' '
+test_expect_success POSIXPERM,SANITY 'get: use xdg file if home file is 
unreadable' '
echo "https://home-user:home-p...@example.com"; 
>"$HOME/.git-credentials" &&
chmod -r "$HOME/.git-credentials" &&
mkdir -p "$HOME/.config/git" &&
-- 
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: Bug when doing make test using root user

2015-06-12 Thread Paul Tan
On Fri, Jun 12, 2015 at 5:43 PM, Jean-Yves LENHOF
 wrote:
> Hi,
>
> I tried to compile git 2.4.3 using root on a server. It failed on test 41 of
> t0302-credential-store.sh
> In fact even if we remove read access on a directory, root still can acces
> this directory.
> Using a not privilegied user make the test work.
> Perhaps the test should be adapted to this corner case.
> Trace below.

Right, the test should have the SANITY prereq.

Thanks for reporting.

Regards,
Paul
--
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 v2 2/2] pull: allow dirty tree when rebase.autostash enabled

2015-06-11 Thread Paul Tan
On Sun, Jun 7, 2015 at 5:12 AM, Kevin Daudt  wrote:
> From: Kevin Daudt 
>
> rebase learned to stash changes when it encounters a dirty work tree, but
> git pull --rebase does not.
>
> Only verify if the working tree is dirty when rebase.autostash is not
> enabled.
>
> Signed-off-by: Kevin Daudt 

Ehh? The sign-off does not match the author of the patch.

> Helped-by: Paul Tan 
> ---
>  git-pull.sh |  5 -
>  t/t5520-pull.sh | 12 
>  2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/git-pull.sh b/git-pull.sh
> index 0917d0d..f0a3b6e 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -239,7 +239,10 @@ test true = "$rebase" && {
> die "$(gettext "updating an unborn branch with 
> changes added to the index")"
> fi
> else
> -   require_clean_work_tree "pull with rebase" "Please commit or 
> stash them."
> +   if [ $(git config --bool --get rebase.autostash || echo 
> false) = false ]
> +   then
> +   require_clean_work_tree "pull with rebase" "Please 
> commit or stash them."
> +   fi
> fi
> oldremoteref= &&
> test -n "$curr_branch" &&
> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
> index 925ad49..d06119f 100755
> --- a/t/t5520-pull.sh
> +++ b/t/t5520-pull.sh
> @@ -135,6 +135,18 @@ test_expect_success 'pull --rebase dies early with dirty 
> working directory' '
> test_cmp file expect
>  '
>
> +test_expect_success 'pull --rebase succeeds with dirty working directory and 
> rebase.autostash set' '
> +   test_config branch.to-rebase.rebase true &&

Ok, though I wonder why not just a git pull --rebase...

> +   test_config rebase.autostash true &&
> +   git checkout HEAD -- file &&

Why not git reset --hard before-rebase? If we don't reset HEAD, then
how would we know if we actually did a rebase?

> +   echo dirty > new_file &&

style: echo dirty >new_file &&

> +   git add new_file &&
> +   git pull . copy &&
> +   test $(git rev-parse HEAD^) = $(git rev-parse copy) &&

Okay, although it would be better to use "test_cmp_rev HEAD^ copy"
because it prints out the hashes if they are different.

> +   test $(cat new_file) = dirty &&

"$(cat new_file)" should be quoted to prevent field splitting.

> +   test "$(cat file)" = "modified again"
> +'
> +
>  test_expect_success 'pull.rebase' '
> git reset --hard before-rebase &&
> test_config pull.rebase true &&
> --
> 2.4.2

Thanks,
Paul
--
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 v2 1/2] t5520-pull: Simplify --rebase with dirty tree test

2015-06-11 Thread Paul Tan
On Sun, Jun 7, 2015 at 5:12 AM, Kevin Daudt  wrote:
> @@ -278,25 +291,6 @@ test_expect_success 'rebased upstream + fetch + pull 
> --rebase' '
>
>  '
>
> -test_expect_success 'pull --rebase dies early with dirty working directory' '
> -
> -   git checkout to-rebase &&
> -   git update-ref refs/remotes/me/copy copy^ &&
> -   COPY=$(git rev-parse --verify me/copy) &&
> -   git rebase --onto $COPY copy &&
> -   test_config branch.to-rebase.remote me &&
> -   test_config branch.to-rebase.merge refs/heads/copy &&
> -   test_config branch.to-rebase.rebase true &&
> -   echo dirty >> file &&
> -   git add file &&
> -   test_must_fail git pull &&
> -   test $COPY = $(git rev-parse --verify me/copy) &&
> -   git checkout HEAD -- file &&
> -   git pull &&
> -   test $COPY != $(git rev-parse --verify me/copy)
> -
> -'

Eh whoops, I don't think we should touch this test. It comes from
f9189cf, which states that:

When rebasing fails during "pull --rebase", you cannot just clean up
the working directory and call "pull --rebase" again, since the
remote branch was already fetched.

Which makes me believe that "die-ing early with dirty working
directory" has something to do with the rebased upstream handling
feature of git-pull, and so this test is correct in testing that, and
thus we should not touch it.

The location of the test in the other patch is fine though.

Thanks,
Paul
--
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 v2 18/19] cache-tree: introduce write_index_as_tree()

2015-06-11 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 
---
 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 v2 19/19] am: implement 3-way merge

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

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

diff --git a/builtin/am.c b/builtin/am.c
index 71fda16..8566d22 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -19,6 +19,7 @@
 #include "unpack-trees.h"
 #include "branch.h"
 #include "sequencer.h"
+#include "merge-recursive.h"
 
 /**
  * Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -76,6 +77,8 @@ struct am_state {
const char *resolvemsg;
 
int sign;
+
+   int threeway;
 };
 
 /**
@@ -659,16 +662,32 @@ static void NORETURN die_user_resolve(const struct 
am_state *state)
 }
 
 /*
- * Applies current patch with git-apply. Returns 0 on success, -1 otherwise.
+ * Applies current patch with git-apply. Returns 0 on success, -1 otherwise. If
+ * index_file is not NULL, the patch will be applied to that index.
  */
-static int run_apply(const struct am_state *state)
+static int run_apply(const struct am_state *state, const char *index_file)
 {
struct child_process cp = CHILD_PROCESS_INIT;
 
cp.git_cmd = 1;
 
+   if (index_file)
+   argv_array_pushf(&cp.env_array, "GIT_INDEX_FILE=%s", 
index_file);
+
+   /*
+* If we are allowed to fall back on 3-way merge, don't give false
+* errors during the initial attempt.
+*/
+   if (state->threeway && !index_file) {
+   cp.no_stdout = 1;
+   cp.no_stderr = 1;
+   }
+
argv_array_push(&cp.args, "apply");
-   argv_array_push(&cp.args, "--index");
+   if (index_file)
+   argv_array_push(&cp.args, "--cached");
+   else
+   argv_array_push(&cp.args, "--index");
argv_array_push(&cp.args, am_path(state, "patch"));
 
if (run_command(&cp))
@@ -676,8 +695,92 @@ static int run_apply(const struct am_state *state)
 
/* Reload index as git-apply will have modified it. */
discard_cache();
+   read_cache_from(index_file ? index_file : get_index_file());
+
+   return 0;
+}
+
+/*
+ * Builds a index that contains just the blobs needed for a 3way merge.
+ */
+static int build_fake_ancestor(const struct am_state *state, const char 
*index_file)
+{
+   struct child_process cp = CHILD_PROCESS_INIT;
+
+   cp.git_cmd = 1;
+   argv_array_push(&cp.args, "apply");
+   argv_array_pushf(&cp.args, "--build-fake-ancestor=%s", index_file);
+   argv_array_push(&cp.args, am_path(state, "patch"));
+
+   if (run_command(&cp))
+   return -1;
+
+   return 0;
+}
+
+/**
+ * Attempt a threeway merge, using index_path as the temporary index.
+ */
+static int fall_back_threeway(const struct am_state *state, const char 
*index_path)
+{
+   unsigned char orig_tree[20], his_tree[20], our_tree[20];
+   const unsigned char *bases[1] = {orig_tree};
+   struct merge_options o;
+   struct commit *result;
+
+   if (get_sha1("HEAD", our_tree) < 0)
+   hashcpy(our_tree, EMPTY_TREE_SHA1_BIN);
+
+   if (build_fake_ancestor(state, index_path))
+   return error("could not build fake ancestor");
+
+   discard_cache();
+   read_cache_from(index_path);
+
+   if (write_index_as_tree(orig_tree, &the_index, index_path, 0, NULL))
+   return error(_("Repository lacks necessary blobs to fall back 
on 3-way merge."));
+
+   say(state, stdout, _("Using index info to reconstruct a base tree..."));
+
+   if (!state->quiet) {
+   struct child_process cp = CHILD_PROCESS_INIT;
+   cp.git_cmd = 1;
+   argv_array_pushf(&cp.env_array, "GIT_INDEX_FILE=%s", 
index_path);
+   argv_array_pushl(&cp.args, "diff-index", "--cached",
+   "--diff-filter=AM", "--name-status", "HEAD", 
NULL);
+   run_command(&cp);
+   }
+
+   if (run_apply(state, index_path))
+   return error(_("Did you hand edit your patch?\n"
+   "It does not apply to blobs recorded in its 
index."));
+
+   if (write_index_as_tree(his_tree, &the_index, index_path, 0, NULL))
+   return error("could not write tree");
+
+   say(state, stdout, _("Falling back to patching base and 3-way 
merge..."));
+
+   discard_cache();
  

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

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

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

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

--
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 v2 16/19] am: exit with user friendly message on patch failure

2015-06-11 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 
---
 builtin/am.c | 26 +++---
 1 file changed, 23 insertions(+), 3 deletions(-)

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

--
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 v2 15/19] am: implement quiet option

2015-06-11 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 
---
 builtin/am.c | 36 +---
 1 file changed, 33 insertions(+), 3 deletions(-)

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

--
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 v2 10/19] am: refresh the index at start

2015-06-11 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 
---
 builtin/am.c | 20 
 1 file changed, 20 insertions(+)

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

--
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 v2 14/19] am: implement --abort

2015-06-11 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 
---
 builtin/am.c | 95 ++--
 1 file changed, 92 insertions(+), 3 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 1434e68..cdc07ab 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -409,6 +409,8 @@ static int split_patches(struct am_state *state, enum 
patch_format patch_format,
 static void am_setup(struct am_state *state, enum patch_format patch_format,
struct string_list *paths)
 {
+   unsigned char curr_head[20];
+
if (!patch_format)
patch_format = detect_patch_format(paths);
 
@@ -428,6 +430,14 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
write_file(am_path(state, "next"), 1, "%d", state->cur);
 
write_file(am_path(state, "last"), 1, "%d", state->last);
+
+   if (!get_sha1("HEAD", curr_head)) {
+   write_file(am_path(state, "abort-safety"), 1, "%s", 
sha1_to_hex(curr_head));
+   update_ref("am", "ORIG_HEAD", curr_head, NULL, 0, 
UPDATE_REFS_DIE_ON_ERR);
+   } else {
+   write_file(am_path(state, "abort-safety"), 1, "%s", "");
+   delete_ref("ORIG_HEAD", NULL, 0);
+   }
 }
 
 /**
@@ -436,6 +446,8 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
  */
 static void am_next(struct am_state *state)
 {
+   unsigned char head[GIT_SHA1_RAWSZ];
+
state->cur++;
write_file(am_path(state, "next"), 1, "%d", state->cur);
 
@@ -446,6 +458,11 @@ static void am_next(struct am_state *state)
 
strbuf_reset(&state->msg);
unlink(am_path(state, "final-commit"));
+
+   if (!get_sha1("HEAD", head))
+   write_file(am_path(state, "abort-safety"), 1, "%s", 
sha1_to_hex(head));
+   else
+   write_file(am_path(state, "abort-safety"), 1, "%s", "");
 }
 
 /**
@@ -655,10 +672,14 @@ static void am_run(struct am_state *state)
 {
struct strbuf sb = STRBUF_INIT;
 
+   unlink(am_path(state, "dirtyindex"));
+
refresh_and_write_cache();
 
-   if (index_has_changes(&sb))
+   if (index_has_changes(&sb)) {
+   write_file(am_path(state, "dirtyindex"), 1, "t");
die(_("Dirty index: cannot apply patches (dirty: %s)"), sb.buf);
+   }
 
strbuf_release(&sb);
 
@@ -834,6 +855,67 @@ static void am_skip(struct am_state *state)
am_run(state);
 }
 
+static int safe_to_abort(const struct am_state *state)
+{
+   struct strbuf sb = STRBUF_INIT;
+   unsigned char abort_safety[20], head[20];
+
+   if (file_exists(am_path(state, "dirtyindex")))
+   return 0;
+
+   if (read_state_file(&sb, am_path(state, "abort-safety"), 40, 1) > 0) {
+   if (get_sha1_hex(sb.buf, abort_safety))
+   die(_("could not parse %s"), am_path(state, 
"abort_safety"));
+   } else
+   hashclr(abort_safety);
+
+   if (get_sha1("HEAD", head))
+   hashclr(head);
+
+   if (!hashcmp(head, abort_safety))
+   return 1;
+
+   error(_("You seem to have moved HEAD since the last 'am' failure.\n"
+   "Not rewinding to ORIG_HEAD"));
+
+   return 0;
+}
+
+/**
+ * Aborts the current am session if it is safe to do so.
+ */
+static void am_abort(struct am_state *state)
+{
+   unsigned char curr_head[20], orig_head[20];
+   int has_curr_head, has_orig_head;
+   const char *curr_branch;
+
+   if (!safe_to_abort(state)) {
+   am_destroy(state);
+   return;
+   }
+
+   curr_branch = resolve_refdup("HEAD", 0, curr_head, NULL);
+   has_curr_head = !is_null_sha1(curr_head);
+   if (!has_curr_head)
+   hashcpy(curr_head, EMPTY_TREE_SHA1_BIN);
+
+   has_orig_head = !get_sha1("ORIG_HEAD", orig_head);
+   if (!has_orig_head)
+   hashcpy(orig_head, EMPTY_TREE_SHA1_BIN);
+
+   clean_index(curr_head, orig_head);
+
+   if (has_orig_head)
+   update_ref("am --abort", "HEAD", ori

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

2015-06-11 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 
---
 builtin/am.c | 13 +
 1 file changed, 13 insertions(+)

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

--
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 v2 11/19] am: refuse to apply patches if index is dirty

2015-06-11 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 
---
 builtin/am.c | 46 ++
 1 file changed, 46 insertions(+)

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

--
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 v2 12/19] am: implement --resolved/--continue

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

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

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

--
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 v2 13/19] am: implement --skip

2015-06-11 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 
---
 builtin/am.c | 121 ++-
 1 file changed, 119 insertions(+), 2 deletions(-)

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

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

2015-06-11 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 
---
 builtin/am.c | 50 ++
 1 file changed, 46 insertions(+), 4 deletions(-)

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

--
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 v2 03/19] am: implement skeletal builtin am

2015-06-11 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.

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

Signed-off-by: Paul Tan 
---

Notes:
v2

* Declare struct am_state state as static.

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

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

--
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 v2 06/19] am: detect mbox patches

2015-06-11 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 
Signed-off-by: Paul Tan 
---

Notes:
v2

* Various small code tweaks suggested by Eric.

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

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

--
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 v2 01/19] wrapper: implement xopen()

2015-06-11 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 
Helped-by: Jeff King 
Signed-off-by: Paul Tan 
---

Notes:
v2

* retry on EINTR

* mode is now an optional argument in xopen(). We use the mode argument
  only if O_CREAT is specified in oflag.

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

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

--
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 v2 04/19] am: implement patch queue mechanism

2015-06-11 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 
---

Notes:
v2

* Declare struct am_state as static

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

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

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

2015-06-11 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 
---

Notes:
v2

* Removed the error message distinction between reading and writing.

* Handle EINTR.

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

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

--
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 v2 07/19] am: extract patch, message and authorship with git-mailinfo

2015-06-11 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 
Helped-by: Jeff King 
Signed-off-by: Paul Tan 
---

Notes:
v2

* use die_errno()

* use '%*d' as the format specifier for msgnum()

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

diff --git a/builtin/am.c b/builtin/am.c
index 7379b97..a1db474 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -9,6 +9,23 @@
 #include "parse-options.h"
 #include "dir.h"
 #include "run-command.h"
+#include "quote.h"
+
+/**
+ * Returns 1 if the file is empty or does not exist, 0 otherwise.
+ */
+static int is_empty_file(const char *filename)
+{
+   struct stat st;
+
+   if (stat(filename, &st) < 0) {
+   if (errno == ENOENT)
+   return 1;
+   die_errno(_("could not stat %s"), filename);
+   }
+
+   return !st.st_size;
+}
 
 enum patch_format {
PATCH_FORMAT_UNKNOWN = 0,
@@ -23,6 +40,12 @@ struct am_state {
int cur;
int last;
 
+   /* commit message and metadata */
+   struct strbuf author_name;
+   struct strbuf author_email;
+   struct strbuf author_date;
+   struct strbuf msg;
+
/* number of digits in patch filename */
int prec;
 };
@@ -35,6 +58,10 @@ static void am_state_init(struct am_state *state)
memset(state, 0, sizeof(*state));
 
strbuf_init(&state->dir, 0);
+   strbuf_init(&state->author_name, 0);
+   strbuf_init(&state->author_email, 0);
+   strbuf_init(&state->author_date, 0);
+   strbuf_init(&state->msg, 0);
state->prec = 4;
 }
 
@@ -44,6 +71,10 @@ static void am_state_init(struct am_state *state)
 static void am_state_release(struct am_state *state)
 {
strbuf_release(&state->dir);
+   strbuf_release(&state->author_name);
+   strbuf_release(&state->author_email);
+   strbuf_release(&state->author_date);
+   strbuf_release(&state->msg);
 }
 
 /**
@@ -93,6 +124,95 @@ static int read_state_file(struct strbuf *sb, const char 
*file, size_t hint, int
 }
 
 /**
+ * Parses the "author script" `filename`, and sets state->author_name,
+ * state->author_email and state->author_date accordingly. We are strict with
+ * our parsing, as the author script is supposed to be eval'd, and loosely
+ * parsing it may not give the results the user expects.
+ *
+ * The author script is of the format:
+ *
+ * GIT_AUTHOR_NAME='$author_name'
+ * GIT_AUTHOR_EMAIL='$author_email'
+ * GIT_AUTHOR_DATE='$author_date'
+ *
+ * where $author_name, $author_email and $author_date are quoted.
+ */
+static int read_author_script(struct am_state *state)
+{
+   char *value;
+   struct strbuf sb = STRBUF_INIT;
+   const char *filename = am_path(state, "author-script");
+   FILE *fp = fopen(filename, "r");
+   if (!fp) {
+   if (errno == ENOENT)
+   return 0;
+   die_errno(_("could not open '%s' for reading"), filename);
+   }
+
+   if (strbuf_getline(&sb, fp, '\n'))
+   return -1;
+   if (!skip_prefix(sb.buf, "GIT_AUTHOR_NAME=", (const char**) &value))
+   return -1;
+   value = sq_dequote(value);
+   if (!value)
+   return -1;
+   strbuf_reset(&state->author_name);
+   strbuf_addstr(&state->author_name, value);
+
+   if (strbuf_getline(&sb, fp, '\n'))
+   return -1;
+   if (!skip_prefix(sb.buf, "GIT_AUTHOR_EMAIL=", (const char**) &value))
+   return -1;
+   value = sq_dequote(value);
+   if (!value)
+   return -1;
+   strbuf_reset(&state->author_email);
+   strbuf_addstr(&state->author_email, value);
+
+   if (strbuf_getline(&sb, fp, '\n'))
+   return -1;
+   if (!skip_prefix(sb.buf, "GIT_AUTHOR_DATE=", (const char**) &value))
+   return -1;
+   value = sq_dequote(value);
+   if (!value)
+   return -1;
+   strbuf_reset(&state->author_date);
+   strbuf_addstr(&state->author_date, value);
+
+   if (fgetc(fp) != EOF)
+   return -1;
+
+   fclose(fp);
+   strbuf_release(&sb);
+   return 0;
+}
+
+/**
+ * Saves state->author_name, state->author_email and state->author_date in
+ * `file

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

2015-06-11 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 
---

Notes:
v2

* Declare int opt_patchformat as static.

* Fix up indentation style for the switch()

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

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

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

2015-06-11 Thread Paul Tan
This is a re-roll of [v1]. Thanks Junio, Torsten, Jeff, Eric for the reviews
last round.

Previous versions:

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

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

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

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

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

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

-- 
2.1.4

--
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 00/19] Make git-am a builtin

2015-06-11 Thread Paul Tan
This is a re-roll of [v1]. Thanks Junio, Torsten, Jeff, Eric for the reviews
last round.

Previous versions:

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

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

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

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


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

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

-- 
2.1.4

--
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 v2 15/19] pull: teach git pull about --rebase

2015-06-10 Thread Paul Tan
On Wed, Jun 10, 2015 at 10:44 PM, Junio C Hamano  wrote:
> Paul Tan  writes:
>
>>> Hmph, it is somewhat surprising that we do not have such a helper
>>> already. Wouldn't we need this logic to implement $branch@{upstream}
>>> syntax?
>>
>> Right, the @{upstream} syntax is implemented by branch_get_upstream()
>> in remote.c. It, however, does not check to see if the branch's remote
>> matches what is provided on the command-line, so we still have to
>> implement this check ourselves, which means this helper function is
>> still required.
>>
>> I guess we could still use branch_get_upstream() in this function though.
>
> It is entirely expected that existing function may not do exactly
> what the new caller you introduce might want to do, or may do more
> than what it wants.  That is where refactoring of existing code
> comes in.
>
> It somewhat feels strange that you have to write more than "shim"
> code to glue existing helpers and API functions together to
> re-implement what a scripted Porcelain is already doing, though.
> It can't be that git-pull.sh implements this logic as shell script,
> and it must be asking existing code in Git to do what the callers
> you added for this function would want to do, right?

Not git-pull.sh, but get_remote_merge_branch() git-parse-remote.sh.
The shell code that get_upstream_branch() in this patch implements is:

0|1)
origin="$1"
default=$(get_default_remote)
test -z "$origin" && origin=$default
curr_branch=$(git symbolic-ref -q HEAD) &&
[ "$origin" = "$default" ] &&

^ This here is where it checks to see if the branch's configured
remote matches the remote provided on the command line.

echo $(git for-each-ref --format='%(upstream)' $curr_branch)
;;

^ While here it calls git to get the upstream branch, which is
implemented by branch_get_upstream() on the C side.

So yes, we can use branch_get_upstream(), but we still need to
implement some code on top.

Just to add on, the shell code that get_tracking_branch() in this
patch implements is:

*)
repo=$1
shift
ref=$1
# FIXME: It should return the tracking branch
#Currently only works with the default mapping
case "$ref" in
+*)
ref=$(expr "z$ref" : 'z+\(.*\)')
;;
esac
expr "z$ref" : 'z.*:' >/dev/null || ref="${ref}:"
remote=$(expr "z$ref" : 'z\([^:]*\):')
case "$remote" in
'' | HEAD ) remote=HEAD ;;
heads/*) remote=${remote#heads/} ;;
refs/heads/*) remote=${remote#refs/heads/} ;;
refs/* | tags/* | remotes/* ) remote=
esac
[ -n "$remote" ] && case "$repo" in
.)
echo "refs/heads/$remote"
;;
*)
echo "refs/remotes/$repo/$remote"
;;
esac

so it's more or less a direct translation of the shell script, and we
can be sure it will have the same behavior. I'm definitely in favor of
switching this to use remote_find_tracking(), the question is whether
we want to do it in this patch or in a future patch on top.

Thanks,
Paul
--
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 v2 11/19] pull: check if in unresolved merge state

2015-06-10 Thread Paul Tan
On Wed, Jun 10, 2015 at 10:38 PM, Junio C Hamano  wrote:
> Junio C Hamano  writes:
>
>> Paul Tan  writes:
>>
>>> @@ -422,6 +423,14 @@ int cmd_pull(int argc, const char **argv, const char 
>>> *prefix)
>>>
>>>  parse_repo_refspecs(argc, argv, &repo, &refspecs);
>>>
>>> +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 (!opt_ff.len)
>>>  config_get_ff(&opt_ff);
>>
>> Hmph.
>>
>> If you are going to do the git_config() call yourself, it might make
>> more sense to define git_pull_config() callback and parse the pull.ff
>> yourself, updating the use of the lazy git_config_get_value() API you
>> introduced in patch 10/19.
>>
>> The above "might" is stronger than my usual "might"; I am undecided
>> yet before reading the remainder of the series.
>
> Let me clarify the above with s/stronger/with much less certainty/;

Uh, I have no idea what a "strong might" or a "less certain might" is. :-/

Parsing all the config values with a git_config() callback function is
certainly possible, but I was under the impression that we are moving
towards migrating use of all the git_config() callback loops to the
git_config_get_X() API.

In this case though, we have to use git_config() to initialize the
advice.resolveConflict config setting, but I don't see why it must be
in conflict with the above goal.

Thanks,
Paul
--
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 v2 15/19] pull: teach git pull about --rebase

2015-06-10 Thread Paul Tan
On Wed, Jun 10, 2015 at 9:56 AM, Junio C Hamano  wrote:
> Paul Tan  writes:
>
>> +enum rebase_type {
>> + REBASE_INVALID = -1,
>> + REBASE_FALSE = 0,
>> + REBASE_TRUE,
>> + REBASE_PRESERVE
>> +};
>> +
>> +/**
>> + * Parses the value of --rebase, branch.*.rebase or pull.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. Otherwise,
>> + * returns -1 to signify an invalid value.
>> + */
>> +static enum rebase_type parse_config_rebase(const char *value)
>> +{
>> + int v = git_config_maybe_bool("pull.rebase", value);
>> + if (!v)
>> + return REBASE_FALSE;
>> + else if (v >= 0)
>> + return REBASE_TRUE;
>
> It is somewhat misleading to say "v >= 0" when you already use !v to
> signal something else.  Perhaps "else if (v > 0)" is better?

Ah, right.

>> +/**
>> + * 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 char *get_upstream_branch(const char *remote)
>> +{
>> + struct remote *rm;
>> + struct branch *curr_branch;
>> +
>> + rm = remote_get(remote);
>> + if (!rm)
>> + return NULL;
>> +
>> + curr_branch = branch_get("HEAD");
>> + if (!curr_branch)
>> + return NULL;
>> +
>> + if (curr_branch->remote != rm)
>> + return NULL;
>> +
>> + if (!curr_branch->merge_nr)
>> + return NULL;
>> +
>> + return xstrdup(curr_branch->merge[0]->dst);
>> +}
>
> Hmph, it is somewhat surprising that we do not have such a helper
> already. Wouldn't we need this logic to implement $branch@{upstream}
> syntax?

Right, the @{upstream} syntax is implemented by branch_get_upstream()
in remote.c. It, however, does not check to see if the branch's remote
matches what is provided on the command-line, so we still have to
implement this check ourselves, which means this helper function is
still required.

I guess we could still use branch_get_upstream() in this function though.

>> +/**
>> + * Derives the remote tracking branch from the remote and refspec.
>> + *
>> + * FIXME: The current implementation assumes the default mapping of
>> + * refs/heads/ to refs/remotes//.
>> + */
>> +static char *get_tracking_branch(const char *remote, const char *refspec)
>> +{
>
> This does smell like an incomplete reimplementation of what
> get_fetch_map() knows how to do.

Yeah, this is just a direct rewrite of get_remote_merge_branch() in
git-parse-remote.sh. Johannes pointed out[1] that
remote_find_tracking() in remote.c does the exact same thing without
the assumption of the default fetch refmap. However, this would be a
separate modification on its own, so it may be better to do it in a
separate patch with regression tests. (e.g. what should we do if the
refspec dst is provided?)

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

>> +/**
>> + * Given the repo and refspecs, sets fork_point to the point at which the
>> + * current branch forked from its remote tracking branch. Returns 0 on 
>> success,
>> + * -1 on failure.
>> + */
>> +static int get_rebase_fork_point(unsigned char fork_point[GIT_SHA1_RAWSZ],
>> + const char *repo, const char *refspec)
>> +{
>> +...
>> +}
>
> This function looks OK (the two get_*_branch() helpers it uses I am
> not sure about though).
>
> Same comment on "fork_point[]" parameter's type applies here,
> though.  While I do not mind if you used "struct object_id" to
> represent these object names, if you are sticking to the traditional
> "unsigned char [20]", then these should be "unsigned char *" to be
> consistent with others.

Okay.

Thanks,
Paul
--
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 v2 02/19] parse-options-cb: implement parse_opt_pass_argv_array()

2015-06-10 Thread Paul Tan
On Wed, Jun 10, 2015 at 7:16 AM, Junio C Hamano  wrote:
> Almost the same comment as 01/19 applies to this comment.
>
> I think it makes good sense to have two variants, one that lets the
> last one win and pass only that last one (i.e. 01/19) and the other
> that accumulates them into an argv_array (i.e. this one).  But it
> feels iffy, given that the "acculate" version essentially creates an
> array of (char *), to make "the last one wins, leaving a single
> string" to use strbuf.  I'd find it much more understandable if 01/19
> took (char **) as opt->value instead of a strbuf.

I don't see how it feels iffy. The purpose of using strbufs (and
argv_arrays) is to avoid error-prone manual memory management.

> In any case, these two need to be added as a related pair to the API
> documentation.

Okay, I guess I could also add their macro functions as well.

Thanks,
Paul
--
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 2/5] am: teach StGit patch parser how to read from stdin

2015-06-09 Thread Paul Tan
On Tue, Jun 9, 2015 at 3:57 AM, Junio C Hamano  wrote:
> Paul Tan  writes:
>
>> git-mailsplit, which splits mbox patches, will read the patch from stdin
>> when the filename is "-" or there are no files listed on the
>> command-line.
>>
>> To be consistent with this behavior, teach the StGit patch parser to
>> read from stdin if the filename is "-" or no files are listed on the
>> command-line.
>
> Hmm, doesn't
>
> perl -ne 'processing for each line'
>
> with or without a BEGIN {} block, read from the standard input (if
> no filename is given) or the given file (if given), and more
> importantly, doesn't it treat a lone "-" as STDIN anyway?
>
> That is, wouldn't it make more sense to do something like:
>
> test $# != 0 || set -- -
> for stgit
> do
> ...
> @@PERL@@ -ne 'BEGIN { $subject = 0 }
> ...
> ' "$stgit" >"$dotest/$msgnum" || clean_abort
> done
>
> Same for patch 5/5.

Ah yes, this makes more sense.

Thanks,
Paul
--
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 5/6] am --abort: support aborting to unborn branch

2015-06-09 Thread Paul Tan
On Tue, Jun 9, 2015 at 4:10 AM, Junio C Hamano  wrote:
> Paul Tan  writes:
>
>> When git-am is first run on an unborn branch, no ORIG_HEAD is created.
>> As such, any applied commits will remain even after a git am --abort.
>
> I think this answered my question on 4/6; that may indicate that
> these two are either done in a wrong order or perhaps should be a
> single change.

Ah right, patch 4/6 was too sneaky in that it tried to do the "support
unborn branch" thing as well, which should only be handled in this
patch.

I still think the patches should be separate though since they are
conceptually different. 4/6 modifies the "index clean up" function,
while 5/6 modifies the "reset HEAD" function.

Thanks,
Paul
--
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 6/6] am --abort: keep unrelated commits on unborn branch

2015-06-09 Thread Paul Tan
On Tue, Jun 9, 2015 at 4:13 AM, Junio C Hamano  wrote:
> Paul Tan  writes:
>
>> Since 7b3b7e3 (am --abort: keep unrelated commits since the last failure
>> and warn, 2010-12-21), git-am would refuse to rewind HEAD if commits
>> were made since the last git-am failure. This check was implemented in
>> safe_to_abort(), which checked to see if HEAD's hash matched the
>> abort-safety file.
>>
>> However, this check was skipped if the abort-safety file was empty,
>> which can happen if git-am failed while on an unborn branch.
>
> Shouldn't we then be checking that the HEAD is still unborn if this
> file is found and says that there was no history in the beginning,
> in order to give the "am on top of unborn" workflow the same degree
> of safety?

We do already check to see if the HEAD is still unborn:

abort_safety=$(cat "$dotest/abort-safety")
if test "z$(git rev-parse --verify -q HEAD)" = "z$abort_safety"
then
return 0
fi
gettextln "You seem to have moved HEAD since the last 'am' failure.
Not rewinding to ORIG_HEAD" >&2

If HEAD is unborn, then git rev-parse will not print anything, so we
would be comparing an empty string to an empty string.

Thanks,
Paul
--
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 5/5] am: teach mercurial patch parser how to read from stdin

2015-06-08 Thread Paul Tan
git-mailsplit, which splits mbox patches, will read the patch from stdin
when the filename is "-" or there are no files listed on the
command-line.

To be consistent with this behavior, teach the mercurial patch parser to
read from stdin if the filename is "-" or no files are listed on the
command-line.

Based-on-patch-by: Chris Packham 
Signed-off-by: Paul Tan 
---
 git-am.sh |  4 +++-
 t/t4150-am.sh | 10 ++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/git-am.sh b/git-am.sh
index d97da85..0a40d46 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -327,6 +327,7 @@ split_patches () {
;;
hg)
this=0
+   test 0 -eq "$#" && set -- -
for hg in "$@"
do
this=$(( $this + 1 ))
@@ -338,6 +339,7 @@ split_patches () {
# Since we cannot guarantee that the commit message is 
in
# git-friendly format, we put no Subject: line and just 
consume
# all of the message as the body
+   cat "$hg" |
LANG=C LC_ALL=C @@PERL@@ -M'POSIX qw(strftime)' -ne 
'BEGIN { $subject = 0 }
if ($subject) { print ; }
elsif (/^\# User /) { s/\# User/From:/ ; print 
; }
@@ -353,7 +355,7 @@ split_patches () {
print "\n", $_ ;
$subject = 1;
}
-   ' <"$hg" >"$dotest/$msgnum" || clean_abort
+   ' >"$dotest/$msgnum" || clean_abort
done
echo "$this" >"$dotest/last"
this=
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 4beb4b3..3ebafd9 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -259,6 +259,16 @@ test_expect_success 'am applies hg patch' '
test_cmp_rev second^ HEAD^
 '
 
+test_expect_success 'am --patch-format=hg applies hg patch' '
+   rm -fr .git/rebase-apply &&
+   git checkout -f first &&
+   git am --patch-format=hg http://vger.kernel.org/majordomo-info.html


[PATCH 4/5] am: use gmtime() to parse mercurial patch date

2015-06-08 Thread Paul Tan
An example of the line in a mercurial patch that specifies the date of
the commit would be:

# Date 1433753301 25200

where the first number is the number of seconds since the unix epoch (in
UTC), and the second number is the offset of the timezone, in second s
west of UTC (negative if the timezone is east of UTC).

git-am uses localtime() to break down the first number into its
components (year, month, day, hours, minutes, seconds etc.). However,
the returned components are relative to the user's time zone. As a
result, if the user's time zone does not match the time zone specified
in the patch, the resulting commit will have the wrong author date.

Fix this by using gmtime() instead, which uses UTC instead of the user's
time zone.

Signed-off-by: Paul Tan 
---
 git-am.sh |  6 +++---
 t/t4150-am.sh | 23 +++
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 83f2ea6..d97da85 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -343,11 +343,11 @@ split_patches () {
elsif (/^\# User /) { s/\# User/From:/ ; print 
; }
elsif (/^\# Date /) {
my ($hashsign, $str, $time, $tz) = 
split ;
-   $tz = sprintf "%+05d", (0-$tz)/36;
+   $tz_str = sprintf "%+05d", (0-$tz)/36;
print "Date: " .
  strftime("%a, %d %b %Y %H:%M:%S ",
-  localtime($time))
- . "$tz\n";
+  gmtime($time-$tz))
+ . "$tz_str\n";
} elsif (/^\# /) { next ; }
else {
print "\n", $_ ;
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 7aad8f8..4beb4b3 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -122,6 +122,19 @@ test_expect_success setup '
echo "# This series applies on GIT commit $(git rev-parse 
first)" &&
echo "patch"
} >stgit-series/series &&
+   {
+   echo "# HG changeset patch" &&
+   echo "# User $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>" &&
+   echo "# Date $test_tick 25200" &&
+   echo "#  $(git show --pretty="%aD" -s second)" &&
+   echo "# Node ID $_z40" &&
+   echo "# Parent  $_z40" &&
+   cat msg &&
+   echo &&
+   echo "Signed-off-by: $GIT_COMMITTER_NAME 
<$GIT_COMMITTER_EMAIL>" &&
+   echo &&
+   git diff-tree --no-commit-id -p second
+   } >patch1-hg.eml &&
 
 
sed -n -e "3,\$p" msg >file &&
@@ -236,6 +249,16 @@ test_expect_success 'am applies stgit series' '
test_cmp_rev second^ HEAD^
 '
 
+test_expect_success 'am applies hg patch' '
+   rm -fr .git/rebase-apply &&
+   git checkout -f first &&
+   git am patch1-hg.eml &&
+   test_path_is_missing .git/rebase-apply &&
+   git diff --exit-code second &&
+   test_cmp_rev second HEAD &&
+   test_cmp_rev second^ HEAD^
+'
+
 test_expect_success 'setup: new author and committer' '
GIT_AUTHOR_NAME="Another Thor" &&
GIT_AUTHOR_EMAIL="a.t...@example.com" &&
-- 
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 1/5] t4150: test applying StGit patch

2015-06-08 Thread Paul Tan
By default, an StGit patch separates the subject from the commit message
and headers as follows:

$subject

From: $author_name <$author_email>

$message
---
$diffstats

We test git-am's ability to detect such a patch as an StGit patch, and
its ability to be able to extract the commit author, date and message
from such a patch.

Based-on-patch-by: Chris Packham 
Signed-off-by: Paul Tan 
---
 t/t4150-am.sh | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 306e6f3..0ead529 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -104,6 +104,18 @@ test_expect_success setup '
echo "X-Fake-Field: Line Three" &&
git format-patch --stdout first | sed -e "1d"
} > patch1-ws.eml &&
+   {
+   sed -ne "1p" msg &&
+   echo &&
+   echo "From: $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>" &&
+   echo "Date: $GIT_AUTHOR_DATE" &&
+   echo &&
+   sed -e "1,2d" msg &&
+   echo &&
+   echo "Signed-off-by: $GIT_COMMITTER_NAME 
<$GIT_COMMITTER_EMAIL>" &&
+   echo "---" &&
+   git diff-tree --no-commit-id --stat -p second
+   } >patch1-stgit.eml &&
 
sed -n -e "3,\$p" msg >file &&
git add file &&
@@ -187,6 +199,16 @@ test_expect_success 'am applies patch e-mail with 
preceding whitespace' '
test "$(git rev-parse second^)" = "$(git rev-parse HEAD^)"
 '
 
+test_expect_success 'am applies stgit patch' '
+   rm -fr .git/rebase-apply &&
+   git checkout -f first &&
+   git am patch1-stgit.eml &&
+   test_path_is_missing .git/rebase-apply &&
+   git diff --exit-code second &&
+   test_cmp_rev second HEAD &&
+   test_cmp_rev second^ HEAD^
+'
+
 test_expect_success 'setup: new author and committer' '
GIT_AUTHOR_NAME="Another Thor" &&
GIT_AUTHOR_EMAIL="a.t...@example.com" &&
-- 
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 3/5] t4150: test applying StGit series

2015-06-08 Thread Paul Tan
A StGit series is a directory containing a "series" file which begins
with the line:

# This series applies on GIT commit X

where X is the commit ID that the patch series applies on. Every
following line names a patch in the directory to be applied.

Test that git-am, when given this "series" file, is able to detect it as
an StGit series and apply all the patches in the series.

Signed-off-by: Paul Tan 
---
 t/t4150-am.sh | 17 +
 1 file changed, 17 insertions(+)

diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 51962e4..7aad8f8 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -116,6 +116,13 @@ test_expect_success setup '
echo "---" &&
git diff-tree --no-commit-id --stat -p second
} >patch1-stgit.eml &&
+   mkdir stgit-series &&
+   cp patch1-stgit.eml stgit-series/patch &&
+   {
+   echo "# This series applies on GIT commit $(git rev-parse 
first)" &&
+   echo "patch"
+   } >stgit-series/series &&
+
 
sed -n -e "3,\$p" msg >file &&
git add file &&
@@ -219,6 +226,16 @@ test_expect_success 'am --patch-format=stgit applies stgit 
patch' '
test_cmp_rev second^ HEAD^
 '
 
+test_expect_success 'am applies stgit series' '
+   rm -fr .git/rebase-apply &&
+   git checkout -f first &&
+   git am stgit-series/series &&
+   test_path_is_missing .git/rebase-apply &&
+   git diff --exit-code second &&
+   test_cmp_rev second HEAD &&
+   test_cmp_rev second^ HEAD^
+'
+
 test_expect_success 'setup: new author and committer' '
GIT_AUTHOR_NAME="Another Thor" &&
GIT_AUTHOR_EMAIL="a.t...@example.com" &&
-- 
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 2/5] am: teach StGit patch parser how to read from stdin

2015-06-08 Thread Paul Tan
git-mailsplit, which splits mbox patches, will read the patch from stdin
when the filename is "-" or there are no files listed on the
command-line.

To be consistent with this behavior, teach the StGit patch parser to
read from stdin if the filename is "-" or no files are listed on the
command-line.

Based-on-patch-by: Chris Packham 
Signed-off-by: Paul Tan 
---
 git-am.sh |  5 +++--
 t/t4150-am.sh | 10 ++
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 761befb..83f2ea6 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -297,6 +297,7 @@ split_patches () {
;;
stgit)
this=0
+   test 0 -eq "$#" && set -- -
for stgit in "$@"
do
this=$(expr "$this" + 1)
@@ -305,7 +306,7 @@ split_patches () {
# not starting with Author, From or Date is the
# subject, and the body starts with the next nonempty
# line not starting with Author, From or Date
-   @@PERL@@ -ne 'BEGIN { $subject = 0 }
+   cat "$stgit" | @@PERL@@ -ne 'BEGIN { $subject = 0 }
if ($subject > 1) { print ; }
elsif (/^\s+$/) { next ; }
elsif (/^Author:/) { s/Author/From/ ; print ;}
@@ -318,7 +319,7 @@ split_patches () {
print "Subject: ", $_ ;
$subject = 1;
}
-   ' < "$stgit" > "$dotest/$msgnum" || clean_abort
+   ' >"$dotest/$msgnum" || clean_abort
done
echo "$this" > "$dotest/last"
this=
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 0ead529..51962e4 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -209,6 +209,16 @@ test_expect_success 'am applies stgit patch' '
test_cmp_rev second^ HEAD^
 '
 
+test_expect_success 'am --patch-format=stgit applies stgit patch' '
+   rm -fr .git/rebase-apply &&
+   git checkout -f first &&
+   git am --patch-format=stgit http://vger.kernel.org/majordomo-info.html


[PATCH 0/5] am: improve test coverage and touch up foreign patch parsing

2015-06-08 Thread Paul Tan
git-am is able to parse StGit and mercurial patches. However, there are no
regression tests in the test suite for these patch formats, and there are some
small bugs as well:

* the mercurial and stgit patch parsers does not support reading from stdin

* the mercurial patch parser parsed the patch date wrongly and git-am is thus
  unable to reconstruct the exact commit.

Most patches are based on Chris' patch series[1], which I've credited 
accordingly.

[1] http://thread.gmane.org/gmane.comp.version-control.git/256502


Paul Tan (5):
  t4150: test applying StGit patch
  am: teach StGit patch parser how to read from stdin
  t4150: test applying StGit series
  am: use gmtime() to parse mercurial patch date
  am: teach mercurial patch parser how to read from stdin

 git-am.sh | 15 ++-
 t/t4150-am.sh | 82 +++
 2 files changed, 91 insertions(+), 6 deletions(-)

-- 
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 2/6] am -3: support 3way merge on unborn branch

2015-06-06 Thread Paul Tan
While on an unborn branch, git am -3 will fail to do a threeway merge as
it references HEAD as "our tree", but HEAD does not point to a valid
tree.

Fix this by using an empty tree as "our tree" when we are on an unborn
branch.

Signed-off-by: Paul Tan 
---
 git-am.sh   | 3 ++-
 t/t4151-am-abort.sh | 9 +
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/git-am.sh b/git-am.sh
index df3c8f4..c847b58 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -179,7 +179,8 @@ It does not apply to blobs recorded in its index.")"
 then
GIT_MERGE_VERBOSITY=0 && export GIT_MERGE_VERBOSITY
 fi
-git-merge-recursive $orig_tree -- HEAD $his_tree || {
+our_tree=$(git rev-parse --verify -q HEAD || echo $empty_tree)
+git-merge-recursive $orig_tree -- $our_tree $his_tree || {
git rerere $allow_rerere_autoupdate
die "$(gettext "Failed to merge in the changes.")"
 }
diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh
index e15acc8..b618ee0 100755
--- a/t/t4151-am-abort.sh
+++ b/t/t4151-am-abort.sh
@@ -83,4 +83,13 @@ test_expect_success 'am --abort will keep the local commits 
intact' '
test_cmp expect actual
 '
 
+test_expect_success 'am -3 stops on conflict on unborn branch' '
+   git checkout -f --orphan orphan &&
+   git reset &&
+   rm -f otherfile-4 &&
+   test_must_fail git am -3 0003-*.patch &&
+   test 2 -eq $(git ls-files -u | wc -l) &&
+   test 4 = "$(cat otherfile-4)"
+'
+
 test_done
-- 
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 5/6] am --abort: support aborting to unborn branch

2015-06-06 Thread Paul Tan
When git-am is first run on an unborn branch, no ORIG_HEAD is created.
As such, any applied commits will remain even after a git am --abort.

To be consistent with the behavior of git am --abort when it is not run
from an unborn branch, we empty the index, and then destroy the branch
pointed to by HEAD if there is no ORIG_HEAD.

Signed-off-by: Paul Tan 
---
 git-am.sh   |  9 -
 t/t4151-am-abort.sh | 17 +
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/git-am.sh b/git-am.sh
index e4fe3ed..95f90a0 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -524,7 +524,14 @@ then
index_tree=$(git write-tree) &&
orig_head=$(git rev-parse --verify -q ORIG_HEAD || echo 
$empty_tree) &&
git read-tree -m -u $index_tree $orig_head
-   git reset ORIG_HEAD
+   if git rev-parse --verify -q ORIG_HEAD >/dev/null 2>&1
+   then
+   git reset ORIG_HEAD
+   else
+   git read-tree $empty_tree
+   curr_branch=$(git symbolic-ref HEAD 
2>/dev/null) &&
+   git update-ref -d $curr_branch
+   fi
fi
rm -fr "$dotest"
exit ;;
diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh
index 2366042..b2a7fc5 100755
--- a/t/t4151-am-abort.sh
+++ b/t/t4151-am-abort.sh
@@ -14,6 +14,7 @@ test_expect_success setup '
git add file-1 file-2 &&
git commit -m initial &&
git tag initial &&
+   git format-patch --stdout --root initial >initial.patch &&
for i in 2 3 4 5 6
do
echo $i >>file-1 &&
@@ -125,4 +126,20 @@ test_expect_success 'am -3 --abort removes otherfile-4 on 
unborn branch' '
test_path_is_missing otherfile-4
 '
 
+test_expect_success 'am -3 --abort on unborn branch removes applied commits' '
+   git checkout -f --orphan orphan &&
+   git reset &&
+   rm -f otherfile-4 otherfile-2 file-1 file-2 &&
+   test_must_fail git am -3 initial.patch 0003-*.patch &&
+   test 3 -eq $(git ls-files -u | wc -l) &&
+   test 4 = "$(cat otherfile-4)" &&
+   git am --abort &&
+   test -z "$(git ls-files -u)" &&
+   test_path_is_missing otherfile-4 &&
+   test_path_is_missing file-1 &&
+   test_path_is_missing file-2 &&
+   test 0 -eq $(git log --oneline 2>/dev/null | wc -l) &&
+   test refs/heads/orphan = "$(git symbolic-ref HEAD)"
+'
+
 test_done
-- 
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 6/6] am --abort: keep unrelated commits on unborn branch

2015-06-06 Thread Paul Tan
Since 7b3b7e3 (am --abort: keep unrelated commits since the last failure
and warn, 2010-12-21), git-am would refuse to rewind HEAD if commits
were made since the last git-am failure. This check was implemented in
safe_to_abort(), which checked to see if HEAD's hash matched the
abort-safety file.

However, this check was skipped if the abort-safety file was empty,
which can happen if git-am failed while on an unborn branch. As such, if
any commits were made since then, they would be discarded. Fix this by
carrying on the abort safety check even if the abort-safety file is
empty.

Signed-off-by: Paul Tan 
---
 git-am.sh   |  2 +-
 t/t4151-am-abort.sh | 11 +++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/git-am.sh b/git-am.sh
index 95f90a0..4324bb1 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -87,7 +87,7 @@ safe_to_abort () {
return 1
fi
 
-   if ! test -s "$dotest/abort-safety"
+   if ! test -f "$dotest/abort-safety"
then
return 0
fi
diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh
index b2a7fc5..833e7b2 100755
--- a/t/t4151-am-abort.sh
+++ b/t/t4151-am-abort.sh
@@ -142,4 +142,15 @@ test_expect_success 'am -3 --abort on unborn branch 
removes applied commits' '
test refs/heads/orphan = "$(git symbolic-ref HEAD)"
 '
 
+test_expect_success 'am --abort on unborn branch will keep local commits 
intact' '
+   git checkout -f --orphan orphan &&
+   git reset &&
+   test_must_fail git am 0004-*.patch &&
+   test_commit unrelated2 &&
+   git rev-parse HEAD >expect &&
+   git am --abort &&
+   git rev-parse HEAD >actual &&
+   test_cmp expect actual
+'
+
 test_done
-- 
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 4/6] am --abort: revert changes introduced by failed 3way merge

2015-06-06 Thread Paul Tan
Even when a merge conflict occurs with am --3way, the index will be
modified with the results of any successfully merged files. These
changes to the index will not be reverted with a
"git read-tree --reset -u HEAD ORIG_HEAD", as git read-tree will not be
aware of how the current index differs from HEAD or ORIG_HEAD.

To fix this, we first reset any conflicting entries in the index. The
resulting index will contain the results of successfully merged files
introduced by the failed merge. We write this index to a tree, and then
use git read-tree to fast-forward this "index tree" back to ORIG_HEAD,
thus undoing all the changes from the failed merge.

When we are on an unborn branch, HEAD and ORIG_HEAD will not point to
valid trees. In this case, use an empty tree.

Signed-off-by: Paul Tan 
---
 git-am.sh   |  6 +-
 t/t4151-am-abort.sh | 23 +++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/git-am.sh b/git-am.sh
index 67f4f25..e4fe3ed 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -519,7 +519,11 @@ then
git rerere clear
if safe_to_abort
then
-   git read-tree --reset -u HEAD ORIG_HEAD
+   head_tree=$(git rev-parse --verify -q HEAD || echo 
$empty_tree) &&
+   git read-tree --reset -u $head_tree $head_tree &&
+   index_tree=$(git write-tree) &&
+   orig_head=$(git rev-parse --verify -q ORIG_HEAD || echo 
$empty_tree) &&
+   git read-tree -m -u $index_tree $orig_head
git reset ORIG_HEAD
fi
rm -fr "$dotest"
diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh
index ea4a49e..2366042 100755
--- a/t/t4151-am-abort.sh
+++ b/t/t4151-am-abort.sh
@@ -70,6 +70,17 @@ test_expect_success 'am -3 --skip removes otherfile-4' '
test 4 = "$(cat otherfile-4)" &&
git am --skip &&
test_cmp_rev initial HEAD &&
+   test -z "$(git ls-files -u)" &&
+   test_path_is_missing otherfile-4
+'
+
+test_expect_success 'am -3 --abort removes otherfile-4' '
+   git reset --hard initial &&
+   test_must_fail git am -3 0003-*.patch &&
+   test 3 -eq $(git ls-files -u | wc -l) &&
+   test 4 = "$(cat otherfile-4)" &&
+   git am --abort &&
+   test_cmp_rev initial HEAD &&
test -z $(git ls-files -u) &&
test_path_is_missing otherfile-4
 '
@@ -102,4 +113,16 @@ test_expect_success 'am -3 --skip clears index on unborn 
branch' '
test_path_is_missing tmpfile
 '
 
+test_expect_success 'am -3 --abort removes otherfile-4 on unborn branch' '
+   git checkout -f --orphan orphan &&
+   git reset &&
+   rm -f otherfile-4 file-1 &&
+   test_must_fail git am -3 0003-*.patch &&
+   test 2 -eq $(git ls-files -u | wc -l) &&
+   test 4 = "$(cat otherfile-4)" &&
+   git am --abort &&
+   test -z "$(git ls-files -u)" &&
+   test_path_is_missing otherfile-4
+'
+
 test_done
-- 
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 3/6] am --skip: support skipping while on unborn branch

2015-06-06 Thread Paul Tan
When git am --skip is run, git am will copy HEAD's tree entries to the
index with "git reset HEAD". However, on an unborn branch, HEAD does not
point to a tree, so "git reset HEAD" will fail.

Fix this by treating HEAD as en empty tree when we are on an unborn
branch.

Signed-off-by: Paul Tan 
---
 git-am.sh   |  4 +---
 t/t4151-am-abort.sh | 10 ++
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index c847b58..67f4f25 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -509,9 +509,7 @@ then
git read-tree --reset -u $head_tree $head_tree &&
index_tree=$(git write-tree) &&
git read-tree -m -u $index_tree $head_tree
-   orig_head=$(cat "$GIT_DIR/ORIG_HEAD")
-   git reset HEAD
-   git update-ref ORIG_HEAD $orig_head
+   git read-tree $head_tree
;;
,t)
if test -f "$dotest/rebasing"
diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh
index b618ee0..ea4a49e 100755
--- a/t/t4151-am-abort.sh
+++ b/t/t4151-am-abort.sh
@@ -92,4 +92,14 @@ test_expect_success 'am -3 stops on conflict on unborn 
branch' '
test 4 = "$(cat otherfile-4)"
 '
 
+test_expect_success 'am -3 --skip clears index on unborn branch' '
+   test_path_is_dir .git/rebase-apply &&
+   echo tmpfile >tmpfile &&
+   git add tmpfile &&
+   git am --skip &&
+   test -z "$(git ls-files)" &&
+   test_path_is_missing otherfile-4 &&
+   test_path_is_missing tmpfile
+'
+
 test_done
-- 
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 1/6] am --skip: revert changes introduced by failed 3way merge

2015-06-06 Thread Paul Tan
Even when a merge conflict occurs with am --3way, the index will be
modified with the results of any succesfully merged files (such as a new
file). These changes to the index will not be reverted with a
"git read-tree --reset -u HEAD HEAD", as git read-tree will not be aware
of how the current index differs from HEAD.

To fix this, we first reset any conflicting entries from the index. The
resulting index will contain the results of successfully merged files.
We write the index to a tree, then use git read-tree -m to fast-forward
the "index tree" back to HEAD, thus undoing all the changes from the
failed merge.

Signed-off-by: Paul Tan 
---
 git-am.sh   |  7 ++-
 t/t4151-am-abort.sh | 11 +++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/git-am.sh b/git-am.sh
index 761befb..df3c8f4 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -69,6 +69,8 @@ then
cmdline="$cmdline -3"
 fi
 
+empty_tree=4b825dc642cb6eb9a060e54bf8d69288fbee4904
+
 sq () {
git rev-parse --sq-quote "$@"
 }
@@ -502,7 +504,10 @@ then
;;
t,)
git rerere clear
-   git read-tree --reset -u HEAD HEAD
+   head_tree=$(git rev-parse --verify -q HEAD || echo $empty_tree) 
&&
+   git read-tree --reset -u $head_tree $head_tree &&
+   index_tree=$(git write-tree) &&
+   git read-tree -m -u $index_tree $head_tree
orig_head=$(cat "$GIT_DIR/ORIG_HEAD")
git reset HEAD
git update-ref ORIG_HEAD $orig_head
diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh
index 8d90634..e15acc8 100755
--- a/t/t4151-am-abort.sh
+++ b/t/t4151-am-abort.sh
@@ -63,6 +63,17 @@ do
 
 done
 
+test_expect_success 'am -3 --skip removes otherfile-4' '
+   git reset --hard initial &&
+   test_must_fail git am -3 0003-*.patch &&
+   test 3 -eq $(git ls-files -u | wc -l) &&
+   test 4 = "$(cat otherfile-4)" &&
+   git am --skip &&
+   test_cmp_rev initial HEAD &&
+   test -z $(git ls-files -u) &&
+   test_path_is_missing otherfile-4
+'
+
 test_expect_success 'am --abort will keep the local commits intact' '
test_must_fail git am 0004-*.patch &&
test_commit unrelated &&
-- 
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 0/6] am --skip/--abort: improve index/worktree cleanup

2015-06-06 Thread Paul Tan
Currently git-am attempts to clean up the index/worktree when skipping or
aborting, but does not do it very well:

* While it discards conflicted index entries, it does not remove any other
  modifications made to the index due to a previous threeway merge.

* It expects HEAD/ORIG_HEAD to exist, and thus does not clean up the index when
  on an unborn branch.

This patch series addresses the above two general problems by making the calls
to git-read-tree aware of the differences between our current index and
HEAD/ORIG_HEAD, and by explictly defining what happens when we are on an unborn
branch.


Paul Tan (6):
  am --skip: revert changes introduced by failed 3way merge
  am -3: support 3way merge on unborn branch
  am --skip: support skipping while on unborn branch
  am --abort: revert changes introduced by failed 3way merge
  am --abort: support aborting to unborn branch
  am --abort: keep unrelated commits on unborn branch

 git-am.sh   | 31 ++--
 t/t4151-am-abort.sh | 81 +
 2 files changed, 104 insertions(+), 8 deletions(-)

-- 
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: [RFC] git-am: handling unborn branches

2015-06-05 Thread Paul Tan
On Fri, Jun 5, 2015 at 11:36 PM, Junio C Hamano  wrote:
> Paul Tan  writes:
>
>> Hmm, actually git-am.sh doesn't seem to do that even when we have a
>> history to apply patches to. This is okay in the non-3way case, as
>> git-apply will check to see if the patch applies before it modifies
>> the index, but if we fall back on 3-way merge, any new files the
>> failed merge added will not be deleted in the "git read-tree --reset
>> -u HEAD HEAD".
>>
>> Should we do that?
>
> That sounds like the right thing to do; I agree that fixing it may
> or may not be outside the scope of the immediate series.

Hmm, thinking about it, the equivalent C code would be greatly
affected by whatever behavior we go with, so I think we should try
fixing the behavior first.

This was done really quickly, but I think this may fix it:

diff --git a/git-am.sh b/git-am.sh
index 761befb..f7d54bf 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -502,7 +502,9 @@ then
 ;;
 t,)
 git rerere clear
-git read-tree --reset -u HEAD HEAD
+git read-tree --reset HEAD HEAD &&
+our_tree=$(git write-tree) &&
+git read-tree -m -u $our_tree HEAD
 orig_head=$(cat "$GIT_DIR/ORIG_HEAD")
 git reset HEAD
 git update-ref ORIG_HEAD $orig_head
diff --git a/t/t4153-am-clean-index.sh b/t/t4153-am-clean-index.sh
new file mode 100755
index 000..6d696db
--- /dev/null
+++ b/t/t4153-am-clean-index.sh
@@ -0,0 +1,30 @@
+#!/bin/sh
+
+test_description='test clean index with am'
+. ./test-lib.sh
+
+test_expect_success setup '
+test_commit initial file &&
+test_commit master-commit file &&
+git checkout -b conflict master^ &&
+echo conflict-commit >file &&
+echo newfile >newfile &&
+git add newfile &&
+test_tick &&
+git commit -a -m conflict-commit &&
+git format-patch --stdout initial >conflict.patch &&
+git checkout master
+'
+
+test_expect_success 'am -3 pauses on conflict' '
+test_must_fail git am -3 conflict.patch &&
+echo newfile >expected &&
+test_cmp newfile expected
+'
+
+test_expect_success 'am --skip removes newfile' '
+git am --skip &&
+test_path_is_missing newfile
+'
+
+test_done

However, it assumes that the contents of the index are from the failed
merge. If the user modified the index before running git-am --skip,
e.g. the user added a file, then that file would be deleted, which may
not be desired...

Thanks,
Paul
--
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: [RFC] git-am: handling unborn branches

2015-06-05 Thread Paul Tan
On Fri, Jun 5, 2015 at 1:27 AM, Stefan Beller  wrote:
> On Thu, Jun 4, 2015 at 3:34 AM, Paul Tan  wrote:
>> Or, the current behavior of git-am.sh will print some scary errors
>> about the missing HEAD, but will then continue on to the next patch.
>> If the index is not clean, it will then error out. Should we preserve
>> this behavior? (without the errors about the missing HEAD)
>>
>> 2. am --abort
>>
>> For git am --abort, git-am.sh does something similar. It does a
>> fast-forward checkout from HEAD to ORIG_HEAD, discarding unmerged
>> entries, and then resets the index to ORIG_HEAD so that local changes
>> will be unstaged.
>>
>> if safe_to_abort
>> then
>> git read-tree --reset -u HEAD ORIG_HEAD
>> git reset ORIG_HEAD
>> fi
>> rm -fr "$dotest"
>>
>> This, however, requires a valid HEAD and ORIG_HEAD. If we don't have a
>> HEAD or ORIG_HEAD (because we were on an unborn branch when we started
>> git am), what should we do? (Note: safe_to_abort returns true if we
>> git am with no HEAD because $dotest/abort-safety will not exist)
>> Should we discard all entires in the index as well? (Since we might
>> think of the "original HEAD" as an empty tree?)
>>
>> Or, the current behavior of git-am.sh will print some scary errors
>> about the missing HEAD and ORIG_HEAD, but will not touch the index at
>> all, and still delete the rebase-apply directory. Should we preserve
>> this behavior (without the errors)?
>
> I guess so, looking at the documentation
>--abort
>Restore the original branch and abort the patching operation.
>
> a user may want to not go to the unborn branch, but rather to the previous
> HEAD?

I think we need to be consistent with the non-unborn-branch behavior
of git-am. In normal use am --abort would reset the branch back to
ORIG_HEAD, which is the HEAD when am was first run, thus losing all
the applied commits. However, since f79d4c8 (git-am: teach git-am to
apply a patch to an unborn branch, 2009-04-10) if git-am is run
without a HEAD, no ORIG_HEAD would be created.

I guess if we are to follow this normal behavior, we need to destroy
the current branch as well.

So the abort logic would be something like:

1. If we are still on an unborn branch, we clear the index.

2. If we are not on an unborn branch, but we have no ORIG_HEAD because
we started from an unborn branch, then we destroy the current branch.

3. If we are not on an unborn branch, and we have an ORIG_HEAD, then
we do the normal behavior of fast-forwarding and resetting the index
to ORIG_HEAD.

We also need to consider safe_to_abort() as well though, which was
introduced in 7b3b7e3 (am --abort: keep unrelated commits since the
last failure and warn, 2010-12-21). Specifically,

safe_to_abort () {
...
if ! test -s "$dotest/abort-safety"
then
return 0
fi
...
}

where we are allowed to reset HEAD if the abort-safety file is empty
or does not exist. If patch application failed while we are on an
unborn branch, we will have no abort-safety file. However, the user
may have made commits since then, and we should not make the user lose
those commits. As such, we should probably not reset HEAD if there is
no abort-safety file.

Thanks,
Paul
--
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: [RFC] git-am: handling unborn branches

2015-06-04 Thread Paul Tan
On Fri, Jun 5, 2015 at 1:26 AM, Junio C Hamano  wrote:
> Paul Tan  writes:
>
>> git-am generally supports applying patches to unborn branches.
>> However, there are 2 cases where git-am does not handle unborn
>> branches which I would like to address before the git-am rewrite to C:
>
>> 1. am --skip
>>
>> For git am --skip, git-am.sh does a fast-forward checkout from HEAD to
>> HEAD, discarding unmerged entries, and then resets the index to HEAD
>> so that the index is not dirty.
>>
>> git read-tree --reset -u HEAD HEAD
>> orig_head=$(cat "$GIT_DIR/ORIG_HEAD")
>> git reset HEAD
>> git update-ref ORIG_HEAD $orig_head
>>
>> This requires a valid HEAD. Since git-am requires an empty index for
>> unborn branches in the patch application stage anyway, I think we
>> should discard all entires in the index if we are on an unborn branch?
>
> Yes, and it should also remove the new files the failed application
> brought in to the working tree, if any, to match the "--skip" done
> in the normal case (i.e. when we already have a history to apply
> patches to), I would think.

Hmm, actually git-am.sh doesn't seem to do that even when we have a
history to apply patches to. This is okay in the non-3way case, as
git-apply will check to see if the patch applies before it modifies
the index, but if we fall back on 3-way merge, any new files the
failed merge added will not be deleted in the "git read-tree --reset
-u HEAD HEAD".

Should we do that? I dunno, but if we want to introduce this feature,
I think a "git read-tree --reset HEAD HEAD && git read-tree -m -u
$(git write-tree) HEAD" will do the trick? (We discard all unmerged
entries, then fast-forward from the tree the failed merged left us
with back to HEAD).

Clearing the index in the unborn branch case seems to be the most
consistent thing to do for now (for the purpose of the git-am
rewrite).

Thanks,
Paul
--
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 1/8] wrapper: implement xopen()

2015-06-04 Thread Paul Tan
On Thu, May 28, 2015 at 3:03 AM, Torsten Bögershausen  wrote:
> On 2015-05-27 15.33, Paul Tan wrote:
>> +/**
>> + * xopen() is the same as open(), but it die()s if the open() fails.
>> + */
>> +int xopen(const char *path, int flags, mode_t mode)
>> +{
>> + int fd;
>> +
>> + assert(path);
>> + fd = open(path, flags, mode);
>> + if (fd < 0) {
>> + if ((flags & O_WRONLY) || (flags & O_RDWR))
>> + die_errno(_("could not open '%s' for writing"), path);
> This is only partly true:
> it could be either "writing" or "read write".

Ah right, I see now that the POSIX spec allows for, and encourages
O_RDONLY | O_WRONLY == O_RDWR.

> I don't know if the info "for reading" or "for writing" is needed/helpful at 
> all,
> or if a simple "could not open" would be enough.

Yeah, I agree that it may not be helpful, but I noticed that most
error messages in git are of the form "unable to open X for writing",
"unable to open X for reading", "could not create X" etc. Or rather I
thought I noticed, but it now seems to me that there are quite a lot
of uses of "could not open X" as well.

I guess I will remove the distinction.

Thanks,
Paul
--
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


[RFC] git-am: handling unborn branches

2015-06-04 Thread Paul Tan
Hi,

git-am generally supports applying patches to unborn branches.
However, there are 2 cases where git-am does not handle unborn
branches which I would like to address before the git-am rewrite to C:

1. am --skip

For git am --skip, git-am.sh does a fast-forward checkout from HEAD to
HEAD, discarding unmerged entries, and then resets the index to HEAD
so that the index is not dirty.

git read-tree --reset -u HEAD HEAD
orig_head=$(cat "$GIT_DIR/ORIG_HEAD")
git reset HEAD
git update-ref ORIG_HEAD $orig_head

This requires a valid HEAD. Since git-am requires an empty index for
unborn branches in the patch application stage anyway, I think we
should discard all entires in the index if we are on an unborn branch?

Or, the current behavior of git-am.sh will print some scary errors
about the missing HEAD, but will then continue on to the next patch.
If the index is not clean, it will then error out. Should we preserve
this behavior? (without the errors about the missing HEAD)

2. am --abort

For git am --abort, git-am.sh does something similar. It does a
fast-forward checkout from HEAD to ORIG_HEAD, discarding unmerged
entries, and then resets the index to ORIG_HEAD so that local changes
will be unstaged.

if safe_to_abort
then
git read-tree --reset -u HEAD ORIG_HEAD
git reset ORIG_HEAD
fi
rm -fr "$dotest"

This, however, requires a valid HEAD and ORIG_HEAD. If we don't have a
HEAD or ORIG_HEAD (because we were on an unborn branch when we started
git am), what should we do? (Note: safe_to_abort returns true if we
git am with no HEAD because $dotest/abort-safety will not exist)
Should we discard all entires in the index as well? (Since we might
think of the "original HEAD" as an empty tree?)

Or, the current behavior of git-am.sh will print some scary errors
about the missing HEAD and ORIG_HEAD, but will not touch the index at
all, and still delete the rebase-apply directory. Should we preserve
this behavior (without the errors)?

Thanks,
Paul
--
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 1/8] wrapper: implement xopen()

2015-06-03 Thread Paul Tan
On Thu, May 28, 2015 at 5:53 AM, Jeff King  wrote:
> On Wed, May 27, 2015 at 09:03:47PM +0200, Torsten Bögershausen wrote:
>
>> The original open can take 2 or 3 parameters, how about this:
>> int xopen(const char *path, int oflag, ... )
>> {
>> va_list params;
>> int mode;
>> int fd;
>>
>> va_start(params, oflag);
>> mode = va_arg(params, int);
>> va_end(params);
>>
>> fd = open(path, oflag, mode);
>
> Don't you need a conditional on pulling the mode arg off the stack
> (i.e., if O_CREAT is in the flags)?

Yeah, we do, as va_arg()'s behavior is undefined if we do not have the
next argument.

The POSIX spec[1] only mentions O_CREAT as requiring the extra
argument, so I guess we'll only need to check for that.

[1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html

Thanks,
Paul
--
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 6/8] am: extract patch, message and authorship with git-mailinfo

2015-06-03 Thread Paul Tan
On Thu, May 28, 2015 at 4:44 AM, Junio C Hamano  wrote:
> Paul Tan  writes:
>
>> @@ -17,6 +34,10 @@ struct am_state {
>>   struct strbuf dir;/* state directory path */
>>   int cur;  /* current patch number */
>>   int last; /* last patch number */
>> + struct strbuf msg;/* commit message */
>> + struct strbuf author_name;/* commit author's name */
>> + struct strbuf author_email;   /* commit author's email */
>> + struct strbuf author_date;/* commit author's date */
>>   int prec; /* number of digits in patch filename */
>>  };
>
> I always get suspicious when structure fields are overly commented,
> wondering if it is a sign of naming fields poorly.  All of the above
> fields look quite self-explanatory and I am not sure if it is worth
> having these comments, spending efforts to type SP many times to
> align them and all.

Okay, I'll take Jeff's suggestion to organize them into blocks.

>> +static int read_author_script(struct am_state *state)
>> +{
>> + char *value;
>> + struct strbuf sb = STRBUF_INIT;
>> + const char *filename = am_path(state, "author-script");
>> + FILE *fp = fopen(filename, "r");
>> + if (!fp) {
>> + if (errno == ENOENT)
>> + return 0;
>> + die(_("could not open '%s' for reading"), filename);
>
> Hmph, do we want to report with die_errno()?

Yes, we do.

>> + }
>> +
>> + if (strbuf_getline(&sb, fp, '\n'))
>> + return -1;
>> + if (!skip_prefix(sb.buf, "GIT_AUTHOR_NAME=", (const char**) &value))
>
> This cast is unfortunate; can't "value" be of "const char *" type?

We can't, because sq_dequote() modifies the string directly. I would
think casting from non-const to const is safer than the other way
around.

Thanks,
Paul
--
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 6/8] am: extract patch, message and authorship with git-mailinfo

2015-06-03 Thread Paul Tan
On Thu, May 28, 2015 at 6:13 AM, Junio C Hamano  wrote:
> Paul Tan  writes:
>
>> +static const char *msgnum(const struct am_state *state)
>> +{
>> + static struct strbuf fmt = STRBUF_INIT;
>> + static struct strbuf sb = STRBUF_INIT;
>> +
>> + strbuf_reset(&fmt);
>> + strbuf_addf(&fmt, "%%0%dd", state->prec);
>> +
>> + strbuf_reset(&sb);
>> + strbuf_addf(&sb, fmt.buf, state->cur);
>
> Hmph, wouldn't ("%*d", state->prec, state->cur) work or am I missing
> something?

Yeah, I think it would. I justs didn't know about the existence of '*'.

Thanks,
Paul
--
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 05/19] pull: implement fetch + merge

2015-06-02 Thread Paul Tan
Implement the fetch + merge functionality of git-pull, by first running
git-fetch with the repo and refspecs provided on the command line, then
running git-merge on FETCH_HEAD to merge the fetched refs into the
current branch.

Signed-off-by: Paul Tan 
---
 builtin/pull.c | 61 +-
 1 file changed, 60 insertions(+), 1 deletion(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index f8b79a2..0ca23a3 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -9,8 +9,10 @@
 #include "builtin.h"
 #include "parse-options.h"
 #include "exec_cmd.h"
+#include "run-command.h"
 
 static const char * const pull_usage[] = {
+   N_("git pull [options] [ [...]]"),
NULL
 };
 
@@ -18,8 +20,60 @@ static struct option pull_options[] = {
OPT_END()
 };
 
+/**
+ * Parses argv into [ [...]], returning their values in `repo`
+ * as a string and `refspecs` as a null-terminated array of strings. If `repo`
+ * is not provided in argv, it is set to NULL.
+ */
+static void parse_repo_refspecs(int argc, const char **argv, const char **repo,
+   const char ***refspecs)
+{
+   if (argc > 0) {
+   *repo = *argv++;
+   argc--;
+   } else
+   *repo = NULL;
+   *refspecs = argv;
+}
+
+/**
+ * Runs git-fetch, returning its exit status. `repo` and `refspecs` are the
+ * repository and refspecs to fetch, or NULL if they are not provided.
+ */
+static int run_fetch(const char *repo, const char **refspecs)
+{
+   struct argv_array args = ARGV_ARRAY_INIT;
+   int ret;
+
+   argv_array_pushl(&args, "fetch", "--update-head-ok", NULL);
+   if (repo)
+   argv_array_push(&args, repo);
+   while (*refspecs)
+   argv_array_push(&args, *refspecs++);
+   ret = run_command_v_opt(args.argv, RUN_GIT_CMD);
+   argv_array_clear(&args);
+   return ret;
+}
+
+/**
+ * Runs git-merge, returning its exit status.
+ */
+static int run_merge(void)
+{
+   int ret;
+   struct argv_array args = ARGV_ARRAY_INIT;
+
+   argv_array_pushl(&args, "merge", NULL);
+   argv_array_push(&args, "FETCH_HEAD");
+   ret = run_command_v_opt(args.argv, RUN_GIT_CMD);
+   argv_array_clear(&args);
+   return ret;
+}
+
 int cmd_pull(int argc, const char **argv, const char *prefix)
 {
+   const char *repo, **refspecs;
+
if (!getenv("_GIT_USE_BUILTIN_PULL")) {
const char *path = mkpath("%s/git-pull", git_exec_path());
 
@@ -29,5 +83,10 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 
argc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0);
 
-   return 0;
+   parse_repo_refspecs(argc, argv, &repo, &refspecs);
+
+   if (run_fetch(repo, refspecs))
+   return 1;
+
+   return run_merge();
 }
-- 
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 v2 04/19] pull: implement skeletal builtin pull

2015-06-02 Thread Paul Tan
For the purpose of rewriting git-pull.sh into a C builtin, implement a
skeletal builtin/pull.c that redirects to $GIT_EXEC_PATH/git-pull.sh if
the environment variable _GIT_USE_BUILTIN_PULL is not defined. This
allows us to fall back on the functional git-pull.sh when running the
test suite for tests that depend on a working git-pull implementation.

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

Helped-by: Junio C Hamano 
Signed-off-by: Paul Tan 
---
 Makefile   |  1 +
 builtin.h  |  1 +
 builtin/pull.c | 33 +
 git.c  |  1 +
 4 files changed, 36 insertions(+)
 create mode 100644 builtin/pull.c

diff --git a/Makefile b/Makefile
index 54ec511..2057a9d 100644
--- a/Makefile
+++ b/Makefile
@@ -877,6 +877,7 @@ BUILTIN_OBJS += builtin/pack-refs.o
 BUILTIN_OBJS += builtin/patch-id.o
 BUILTIN_OBJS += builtin/prune-packed.o
 BUILTIN_OBJS += builtin/prune.o
+BUILTIN_OBJS += builtin/pull.o
 BUILTIN_OBJS += builtin/push.o
 BUILTIN_OBJS += builtin/read-tree.o
 BUILTIN_OBJS += builtin/receive-pack.o
diff --git a/builtin.h b/builtin.h
index b87df70..ea3c834 100644
--- a/builtin.h
+++ b/builtin.h
@@ -98,6 +98,7 @@ extern int cmd_pack_redundant(int argc, const char **argv, 
const char *prefix);
 extern int cmd_patch_id(int argc, const char **argv, const char *prefix);
 extern int cmd_prune(int argc, const char **argv, const char *prefix);
 extern int cmd_prune_packed(int argc, const char **argv, const char *prefix);
+extern int cmd_pull(int argc, const char **argv, const char *prefix);
 extern int cmd_push(int argc, const char **argv, const char *prefix);
 extern int cmd_read_tree(int argc, const char **argv, const char *prefix);
 extern int cmd_receive_pack(int argc, const char **argv, const char *prefix);
diff --git a/builtin/pull.c b/builtin/pull.c
new file mode 100644
index 000..f8b79a2
--- /dev/null
+++ b/builtin/pull.c
@@ -0,0 +1,33 @@
+/*
+ * Builtin "git pull"
+ *
+ * Based on git-pull.sh by Junio C Hamano
+ *
+ * Fetch one or more remote refs and merge it/them into the current HEAD.
+ */
+#include "cache.h"
+#include "builtin.h"
+#include "parse-options.h"
+#include "exec_cmd.h"
+
+static const char * const pull_usage[] = {
+   NULL
+};
+
+static struct option pull_options[] = {
+   OPT_END()
+};
+
+int cmd_pull(int argc, const char **argv, const char *prefix)
+{
+   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);
+   }
+
+   argc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0);
+
+   return 0;
+}
diff --git a/git.c b/git.c
index 44374b1..e7a7713 100644
--- a/git.c
+++ b/git.c
@@ -445,6 +445,7 @@ static struct cmd_struct commands[] = {
{ "pickaxe", cmd_blame, RUN_SETUP },
{ "prune", cmd_prune, RUN_SETUP },
{ "prune-packed", cmd_prune_packed, RUN_SETUP },
+   { "pull", cmd_pull, RUN_SETUP | NEED_WORK_TREE },
{ "push", cmd_push, RUN_SETUP },
{ "read-tree", cmd_read_tree, RUN_SETUP },
{ "receive-pack", cmd_receive_pack },
-- 
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 v2 16/19] pull: configure --rebase via branch..rebase or pull.rebase

2015-06-02 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..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 
Signed-off-by: Paul Tan 
---

Notes:
v2

* Previously, config_get_rebase() attempted to do too many things. It:

  1. Checked if there was a configuration for
 branch.$curr_branch.rebase, and if not, then pull.rebase

  2. Parsed the configuration value and died if the value is invalid.

  These 2 functions have been split into config_get_rebase_default() and
  config_get_rebase() respectively.

 builtin/pull.c | 48 
 1 file changed, 48 insertions(+)

diff --git a/builtin/pull.c b/builtin/pull.c
index 7645937..9759720 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -258,6 +258,52 @@ static void config_get_ff(struct strbuf *sb)
 }
 
 /**
+ * Sets `value` to the REBASE_* value for the configuration setting `key`.
+ * Returns 0 is `key` exists, -1 if it does not. Dies if the value of `key` is
+ * invalid.
+ */
+static int config_get_rebase(const char *key, enum rebase_type *value)
+{
+   const char *str_value;
+
+   if (git_config_get_value(key, &str_value))
+   return -1;
+   *value = parse_config_rebase(str_value);
+   if (*value == REBASE_INVALID)
+   die(_("Invalid value for %s: %s"), key, str_value);
+   return 0;
+}
+
+/**
+ * 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_default(void)
+{
+   struct branch *curr_branch = branch_get("HEAD");
+   enum rebase_type rebase_type;
+
+   if (curr_branch) {
+   int ret;
+   struct strbuf sb = STRBUF_INIT;
+
+   strbuf_addf(&sb, "branch.%s.rebase", curr_branch->name);
+   ret = config_get_rebase(sb.buf, &rebase_type);
+   strbuf_release(&sb);
+   if (!ret)
+   return rebase_type;
+   }
+
+   if (!config_get_rebase("pull.rebase", &rebase_type))
+   return rebase_type;
+
+   return REBASE_FALSE;
+}
+
+/**
  * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge
  * into merge_heads.
  */
@@ -691,6 +737,8 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
if (!getenv("GIT_REFLOG_ACTION"))
set_reflog_message(argc, argv);
 
+   opt_rebase = config_get_rebase_default();
+
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 v2 13/19] pull: implement pulling into an unborn branch

2015-06-02 Thread Paul Tan
b4dc085 (pull: merge into unborn by fast-forwarding from empty
tree, 2013-06-20) established git-pull's current behavior of pulling
into an unborn branch by fast-forwarding the work tree from an empty
tree to the merge head, then setting HEAD to the merge head.

Re-implement this behavior by introducing pull_into_void() which will be
called instead of run_merge() if HEAD is invalid.

Helped-by: Stephen Robin 
Signed-off-by: Paul Tan 
---
 builtin/pull.c | 29 -
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 8db0db2..f0d4710 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -13,6 +13,7 @@
 #include "sha1-array.h"
 #include "remote.h"
 #include "dir.h"
+#include "refs.h"
 
 static const char * const pull_usage[] = {
N_("git pull [options] [ [...]]"),
@@ -367,6 +368,27 @@ static int run_fetch(const char *repo, const char 
**refspecs)
 }
 
 /**
+ * "Pulls into void" by branching off merge_head.
+ */
+static int pull_into_void(unsigned char merge_head[GIT_SHA1_RAWSZ],
+   unsigned char curr_head[GIT_SHA1_RAWSZ])
+{
+   /*
+* Two-way merge: we claim the index is based on an empty tree,
+* and try to fast-forward to HEAD. This ensures we will not lose
+* index/worktree changes that the user already made on the unborn
+* branch.
+*/
+   if (checkout_fast_forward(EMPTY_TREE_SHA1_BIN, merge_head, 0))
+   return 1;
+
+   if (update_ref("initial pull", "HEAD", merge_head, curr_head, 0, 
UPDATE_REFS_DIE_ON_ERR))
+   return 1;
+
+   return 0;
+}
+
+/**
  * Runs git-merge, returning its exit status.
  */
 static int run_merge(void)
@@ -475,5 +497,10 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
if (!merge_heads.nr)
die_no_merge_candidates(repo, refspecs);
 
-   return run_merge();
+   if (is_null_sha1(orig_head)) {
+   if (merge_heads.nr > 1)
+   die(_("Cannot merge multiple branches into empty 
head."));
+   return pull_into_void(*merge_heads.sha1, curr_head);
+   } else
+   return run_merge();
 }
-- 
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 v2 06/19] pull: pass verbosity, --progress flags to fetch and merge

2015-06-02 Thread Paul Tan
7f87aff (Teach/Fix pull/fetch -q/-v options, 2008-11-15) taught git-pull
to accept the verbosity -v and -q options and pass them to git-fetch and
git-merge.

Re-implement support for the verbosity flags by adding it to the options
list and introducing argv_push_verbosity() to push the flags into the
argv array used to execute git-fetch and git-merge.

9839018 (fetch and pull: learn --progress, 2010-02-24) and bebd2fd
(pull: propagate --progress to merge, 2011-02-20) taught git-pull to
accept the --progress option and pass it to git-fetch and git-merge.

Re-implement support for this flag by introducing the option callback
handler parse_opt_passthru(). This callback is used to pass the
"--progress" or "--no-progress" command-line switch to git-fetch and
git-merge.

Signed-off-by: Paul Tan 
---

Notes:
v2

* Use parse_opt_pass_strbuf().

 builtin/pull.c | 36 
 1 file changed, 36 insertions(+)

diff --git a/builtin/pull.c b/builtin/pull.c
index 0ca23a3..c9c2cc0 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -16,11 +16,35 @@ static const char * const pull_usage[] = {
NULL
 };
 
+/* Shared options */
+static int opt_verbosity;
+static struct strbuf opt_progress = STRBUF_INIT;
+
 static struct option pull_options[] = {
+   /* Shared options */
+   OPT__VERBOSITY(&opt_verbosity),
+   { OPTION_CALLBACK, 0, "progress", &opt_progress, NULL,
+ N_("force progress reporting"),
+ PARSE_OPT_NOARG, parse_opt_pass_strbuf},
+
OPT_END()
 };
 
 /**
+ * Pushes "-q" or "-v" switches into arr to match the opt_verbosity level.
+ */
+static void argv_push_verbosity(struct argv_array *arr)
+{
+   int verbosity;
+
+   for (verbosity = opt_verbosity; verbosity > 0; verbosity--)
+   argv_array_push(arr, "-v");
+
+   for (verbosity = opt_verbosity; verbosity < 0; verbosity++)
+   argv_array_push(arr, "-q");
+}
+
+/**
  * Parses argv into [ [...]], returning their values in `repo`
  * as a string and `refspecs` as a null-terminated array of strings. If `repo`
  * is not provided in argv, it is set to NULL.
@@ -46,6 +70,12 @@ static int run_fetch(const char *repo, const char **refspecs)
int ret;
 
argv_array_pushl(&args, "fetch", "--update-head-ok", NULL);
+
+   /* Shared options */
+   argv_push_verbosity(&args);
+   if (opt_progress.len)
+   argv_array_push(&args, opt_progress.buf);
+
if (repo)
argv_array_push(&args, repo);
while (*refspecs)
@@ -64,6 +94,12 @@ static int run_merge(void)
struct argv_array args = ARGV_ARRAY_INIT;
 
argv_array_pushl(&args, "merge", NULL);
+
+   /* Shared options */
+   argv_push_verbosity(&args);
+   if (opt_progress.len)
+   argv_array_push(&args, opt_progress.buf);
+
argv_array_push(&args, "FETCH_HEAD");
ret = run_command_v_opt(args.argv, RUN_GIT_CMD);
argv_array_clear(&args);
-- 
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 v2 14/19] pull: set reflog message

2015-06-02 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 
Signed-off-by: Paul Tan 
---

Notes:
v2

* Don't use strbuf_rtrim().

 builtin/pull.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/builtin/pull.c b/builtin/pull.c
index f0d4710..c44cc90 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 "true", sets sb to "--ff". If pull.ff is "false", sets sb to
  * "--no-ff". If pull.ff is "only", sets sb to "--ff-only". If pull.ff is
  * set to an invalid value, die with an error.
@@ -442,6 +461,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 v2 15/19] pull: teach git pull about --rebase

2015-06-02 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 
Helped-by: Johannes Schindelin 
Signed-off-by: Paul Tan 
---

Notes:
v2

* enum rebase_type gained a new value REBASE_INVALID = -1 to represent
  invalid values passed to --rebase. We can't immediately die() on
  invalid values because we need parse_options() to show the git-pull
  usage as well.

* parse_config_rebase() now returns an enum rebase_type to make it clear
  what kind of value it returns.

* parse_config_rebase() now does not depend on git_config_maybe_bool()
  returning 1 for true and 0 for false. If it returns 0, it is
  REBASE_FALSE. If it returns any true value >=0, it is interpreted as
  REBASE_TRUE.

* get_merge_branch_1() has been renamed to get_upstream_branch() to be
  consistent with the similar get_upstream_branch() in sha1_name.c.

* get_merge_branch_2() has been renamed to get_tracking_branch() to
  indicate what it is doing: deriving the remote tracking branch from
  the refspec.

* Various small variable re-names and docstring tweaks. Hopefully
  everything reads better now.

 builtin/pull.c | 239 -
 1 file changed, 237 insertions(+), 2 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index c44cc90..7645937 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -15,6 +15,45 @@
 #include "dir.h"
 #include "refs.h"
 
+enum rebase_type {
+   REBASE_INVALID = -1,
+   REBASE_FALSE = 0,
+   REBASE_TRUE,
+   REBASE_PRESERVE
+};
+
+/**
+ * Parses the value of --rebase, branch.*.rebase or pull.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. Otherwise,
+ * returns -1 to signify an invalid value.
+ */
+static enum rebase_type parse_config_rebase(const char *value)
+{
+   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;
+   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(arg);
+   else
+   *value = unset ? REBASE_FALSE : REBASE_TRUE;
+   return *value == REBASE_INVALID ? -1 : 0;
+}
+
 static const char * const pull_usage[] = {
N_("git pull [options] [ [...]]"),
NULL
@@ -24,7 +63,8 @@ static const char * const pull_usage[] = {
 static int opt_verbosity;
 static struct strbuf opt_progress = STRBUF_INIT;
 
-/* Options passed to git-merge */
+/* Options passed to git-merge or git-rebase */
+static enum rebase_type opt_rebase;
 static struct strbuf opt_diffstat = STRBUF_INIT;
 static struct strbuf opt_log = STRBUF_INIT;
 static struct strbuf opt_squash = STRBUF_INIT;
@@ -58,8 +98,12 @@ static struct option pull_options[] = {
  N_("force progress reporting"),
  PARSE_OPT_NOARG, parse_opt_pass_strbuf},
 
-   /* 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,
+   

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

2015-06-02 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 
---

Notes:
v2

* Decided to use fprintf_ln() for the sake of code consistency, and for
  the added trailing newline.

 builtin/pull.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index f5d437a..4e1ab5b 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -439,7 +439,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))) {
@@ -449,7 +452,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  ");
@@ -461,7 +467,10 @@ static void NORETURN die_no_merge_candidates(const char 
*repo, const char **refs
remote_name = "";
 
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  ");
-- 
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 v2 17/19] pull --rebase: exit early when the working directory is dirty

2015-06-02 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 
---
 builtin/pull.c | 77 +-
 1 file changed, 76 insertions(+), 1 deletion(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 9759720..f5d437a 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,
@@ -304,6 +306,73 @@ static enum rebase_type config_get_rebase_default(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.
  */
@@ -757,9 +826,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 v2 08/19] pull: pass git-fetch's options to git-fetch

2015-06-02 Thread Paul Tan
Since eb2a8d9 (pull: handle git-fetch's options as well, 2015-06-02),
git-pull knows about and handles git-fetch's options, passing them to
git-fetch. Re-implement this behavior.

Since 29609e6 (pull: do nothing on --dry-run, 2010-05-25) git-pull
supported the --dry-run option, exiting after git-fetch if --dry-run is
set. Re-implement this behavior.

Signed-off-by: Paul Tan 
---

Notes:
v2

* Use parse_opt_parse_strbuf()

 builtin/pull.c | 95 ++
 1 file changed, 95 insertions(+)

diff --git a/builtin/pull.c b/builtin/pull.c
index 5f08634..0b66b43 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -32,6 +32,21 @@ static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
 static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
 static struct strbuf opt_gpg_sign = STRBUF_INIT;
 
+/* Options passed to git-fetch */
+static struct strbuf opt_all = STRBUF_INIT;
+static struct strbuf opt_append = STRBUF_INIT;
+static struct strbuf opt_upload_pack = STRBUF_INIT;
+static int opt_force;
+static struct strbuf opt_tags = STRBUF_INIT;
+static struct strbuf opt_prune = STRBUF_INIT;
+static struct strbuf opt_recurse_submodules = STRBUF_INIT;
+static int opt_dry_run;
+static struct strbuf opt_keep = STRBUF_INIT;
+static struct strbuf opt_depth = STRBUF_INIT;
+static struct strbuf opt_unshallow = STRBUF_INIT;
+static struct strbuf opt_update_shallow = STRBUF_INIT;
+static struct strbuf opt_refmap = STRBUF_INIT;
+
 static struct option pull_options[] = {
/* Shared options */
OPT__VERBOSITY(&opt_verbosity),
@@ -82,6 +97,46 @@ static struct option pull_options[] = {
  N_("GPG sign commit"),
  PARSE_OPT_OPTARG, parse_opt_pass_strbuf },
 
+   /* Options passed to git-fetch */
+   OPT_GROUP(N_("Options related to fetching")),
+   { OPTION_CALLBACK, 0, "all", &opt_all, 0,
+ N_("fetch from all remotes"),
+ PARSE_OPT_NOARG, parse_opt_pass_strbuf },
+   { OPTION_CALLBACK, 'a', "append", &opt_append, 0,
+ N_("append to .git/FETCH_HEAD instead of overwriting"),
+ PARSE_OPT_NOARG, parse_opt_pass_strbuf },
+   { OPTION_CALLBACK, 0, "upload-pack", &opt_upload_pack, N_("path"),
+ N_("path to upload pack on remote end"),
+ 0, parse_opt_pass_strbuf },
+   OPT__FORCE(&opt_force, N_("force overwrite of local branch")),
+   { OPTION_CALLBACK, 't', "tags", &opt_tags, 0,
+ N_("fetch all tags and associated objects"),
+ PARSE_OPT_NOARG, parse_opt_pass_strbuf },
+   { OPTION_CALLBACK, 'p', "prune", &opt_prune, 0,
+ N_("prune remote-tracking branches no longer on remote"),
+ PARSE_OPT_NOARG, parse_opt_pass_strbuf },
+   { OPTION_CALLBACK, 0, "recurse-submodules", &opt_recurse_submodules,
+ N_("on-demand"),
+ N_("control recursive fetching of submodules"),
+ PARSE_OPT_OPTARG, parse_opt_pass_strbuf },
+   OPT_BOOL(0, "dry-run", &opt_dry_run,
+   N_("dry run")),
+   { OPTION_CALLBACK, 'k', "keep", &opt_keep, 0,
+ N_("keep downloaded pack"),
+ PARSE_OPT_NOARG, parse_opt_pass_strbuf },
+   { OPTION_CALLBACK, 0, "depth", &opt_depth, N_("depth"),
+ N_("deepen history of shallow clone"),
+ 0, parse_opt_pass_strbuf },
+   { OPTION_CALLBACK, 0, "unshallow", &opt_unshallow, 0,
+ N_("convert to a complete repository"),
+ PARSE_OPT_NONEG | PARSE_OPT_NOARG, parse_opt_pass_strbuf },
+   { OPTION_CALLBACK, 0, "update-shallow", &opt_update_shallow, 0,
+ N_("accept refs that update .git/shallow"),
+ PARSE_OPT_NOARG, parse_opt_pass_strbuf },
+   { OPTION_CALLBACK, 0, "refmap", &opt_refmap, N_("refmap"),
+ N_("specify fetch refmap"),
+ PARSE_OPT_NONEG, parse_opt_pass_strbuf },
+
OPT_END()
 };
 
@@ -100,6 +155,16 @@ static void argv_push_verbosity(struct argv_array *arr)
 }
 
 /**
+ * Pushes "-f" switches into arr to match the opt_force level.
+ */
+static void argv_push_force(struct argv_array *arr)
+{
+   int force = opt_force;
+   while (force-- > 0)
+   argv_array_push(arr, "-f");
+}
+
+/**
  * Parses argv into [ [...]], returning their values in `repo`
  * as a string and `refspecs` as a null-terminated array of strings. If `repo`
  * is not provided in argv, it is set to NULL.
@@ -131,6 +196,33 @@ static int run_fetch(const char *repo, const char 
**refspecs)
if (opt_progress.len)
argv_array_push(&args, opt_progress.

[PATCH v2 12/19] pull: fast-forward working tree if head is updated

2015-06-02 Thread Paul Tan
Since b10ac50 (Fix pulling into the same branch., 2005-08-25), git-pull,
upon detecting that git-fetch updated the current head, will
fast-forward the working tree to the updated head commit.

Re-implement this behavior.

Signed-off-by: Paul Tan 
---
 builtin/pull.c | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/builtin/pull.c b/builtin/pull.c
index 703fc1d..8db0db2 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -411,6 +411,7 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
 {
const char *repo, **refspecs;
struct sha1_array merge_heads = SHA1_ARRAY_INIT;
+   unsigned char orig_head[GIT_SHA1_RAWSZ], curr_head[GIT_SHA1_RAWSZ];
 
if (!getenv("_GIT_USE_BUILTIN_PULL")) {
const char *path = mkpath("%s/git-pull", git_exec_path());
@@ -434,12 +435,41 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
if (!opt_ff.len)
config_get_ff(&opt_ff);
 
+   if (get_sha1("HEAD", orig_head))
+   hashclr(orig_head);
+
if (run_fetch(repo, refspecs))
return 1;
 
if (opt_dry_run)
return 0;
 
+   if (get_sha1("HEAD", curr_head))
+   hashclr(curr_head);
+
+   if (!is_null_sha1(orig_head) && !is_null_sha1(curr_head) &&
+   hashcmp(orig_head, curr_head)) {
+   /*
+* The fetch involved updating the current branch.
+*
+* The working tree and the index file are still based on
+* orig_head commit, but we are merging into curr_head.
+* Update the working tree to match curr_head.
+*/
+
+   warning(_("fetch updated the current branch head.\n"
+   "fast-forwarding your working tree from\n"
+   "commit %s."), sha1_to_hex(orig_head));
+
+   if (checkout_fast_forward(orig_head, curr_head, 0))
+   die(_("Cannot fast-forward your working tree.\n"
+   "After making sure that you saved anything 
precious from\n"
+   "$ git diff %s\n"
+   "output, run\n"
+   "$ git reset --hard\n"
+   "to recover."), sha1_to_hex(orig_head));
+   }
+
get_merge_heads(&merge_heads);
 
if (!merge_heads.nr)
-- 
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 v2 11/19] pull: check if in unresolved merge state

2015-06-02 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 
---
 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 1c1883d..703fc1d 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] [ [...]]"),
@@ -422,6 +423,14 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
 
parse_repo_refspecs(argc, argv, &repo, &refspecs);
 
+   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 (!opt_ff.len)
config_get_ff(&opt_ff);
 
-- 
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 v2 03/19] argv-array: implement argv_array_pushv()

2015-06-02 Thread Paul Tan
When we have a null-terminated array, it would be useful to convert it
or append it to an argv_array for further manipulation.

Implement argv_array_pushv() which will push a null-terminated array of
strings on to an argv_array.

Signed-off-by: Paul Tan 
---
 Documentation/technical/api-argv-array.txt | 3 +++
 argv-array.c   | 6 ++
 argv-array.h   | 1 +
 3 files changed, 10 insertions(+)

diff --git a/Documentation/technical/api-argv-array.txt 
b/Documentation/technical/api-argv-array.txt
index 1a79781..8076172 100644
--- a/Documentation/technical/api-argv-array.txt
+++ b/Documentation/technical/api-argv-array.txt
@@ -46,6 +46,9 @@ Functions
Format a string and push it onto the end of the array. This is a
convenience wrapper combining `strbuf_addf` and `argv_array_push`.
 
+`argv_array_pushv`::
+   Push a null-terminated array of strings onto the end of the array.
+
 `argv_array_pop`::
Remove the final element from the array. If there are no
elements in the array, do nothing.
diff --git a/argv-array.c b/argv-array.c
index 256741d..eaed477 100644
--- a/argv-array.c
+++ b/argv-array.c
@@ -49,6 +49,12 @@ void argv_array_pushl(struct argv_array *array, ...)
va_end(ap);
 }
 
+void argv_array_pushv(struct argv_array *array, const char **argv)
+{
+   for (; *argv; argv++)
+   argv_array_push(array, *argv);
+}
+
 void argv_array_pop(struct argv_array *array)
 {
if (!array->argc)
diff --git a/argv-array.h b/argv-array.h
index c65e6e8..a2fa0aa 100644
--- a/argv-array.h
+++ b/argv-array.h
@@ -17,6 +17,7 @@ __attribute__((format (printf,2,3)))
 void argv_array_pushf(struct argv_array *, const char *fmt, ...);
 LAST_ARG_MUST_BE_NULL
 void argv_array_pushl(struct argv_array *, ...);
+void argv_array_pushv(struct argv_array *, const char **);
 void argv_array_pop(struct argv_array *);
 void argv_array_clear(struct argv_array *);
 
-- 
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 v2 07/19] pull: pass git-merge's options to git-merge

2015-06-02 Thread Paul Tan
Specify git-merge's options in the option list, and pass any specified
options to git-merge.

These options are:

* -n, --stat, --summary: since d8abe14 (merge, pull: introduce
  '--(no-)stat' option, 2008-04-06)

* --log: since efb779f (merge, pull: add '--(no-)log' command line
  option, 2008-04-06)

* --squash: since 7d0c688 (git-merge --squash, 2006-06-23)

* --commit: since 5072a32 (Teach git-pull about --[no-]ff, --no-squash
  and --commit, 2007-10-29)

* --edit: since 8580830 ("git pull" doesn't know "--edit", 2012-02-11)

* --ff, --ff-only: since 5072a32 (Teach git-pull about --[no-]ff,
  --no-squash and --commit, 2007-10-29)

* --verify-signatures: since efed002 (merge/pull: verify GPG signatures
  of commits being merged, 2013-03-31)

* -s, --strategy: since 60fb5b2 (Use git-merge in git-pull (second
  try)., 2005-09-25)

* -X, --strategy-option: since ee2c795 (Teach git-pull to pass
  -X to git-merge, 2009-11-25)

* -S, --gpg-sign: since ea230d8 (pull: add the --gpg-sign option.,
  2014-02-10)

Signed-off-by: Paul Tan 
---

Notes:
v2

* Use opt_parse_pass_strbuf(), opt_parse_pass_argv_array() and
  argv_array_pushv()

 builtin/pull.c | 75 ++
 1 file changed, 75 insertions(+)

diff --git a/builtin/pull.c b/builtin/pull.c
index c9c2cc0..5f08634 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -20,6 +20,18 @@ static const char * const pull_usage[] = {
 static int opt_verbosity;
 static struct strbuf opt_progress = STRBUF_INIT;
 
+/* Options passed to git-merge */
+static struct strbuf opt_diffstat = STRBUF_INIT;
+static struct strbuf opt_log = STRBUF_INIT;
+static struct strbuf opt_squash = STRBUF_INIT;
+static struct strbuf opt_commit = STRBUF_INIT;
+static struct strbuf opt_edit = STRBUF_INIT;
+static struct strbuf opt_ff = STRBUF_INIT;
+static struct strbuf opt_verify_signatures = STRBUF_INIT;
+static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
+static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
+static struct strbuf opt_gpg_sign = STRBUF_INIT;
+
 static struct option pull_options[] = {
/* Shared options */
OPT__VERBOSITY(&opt_verbosity),
@@ -27,6 +39,49 @@ static struct option pull_options[] = {
  N_("force progress reporting"),
  PARSE_OPT_NOARG, parse_opt_pass_strbuf},
 
+   /* Options passed to git-merge */
+   OPT_GROUP(N_("Options related to merging")),
+   { OPTION_CALLBACK, 'n', NULL, &opt_diffstat, NULL,
+ N_("do not show a diffstat at the end of the merge"),
+ PARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_pass_strbuf },
+   { OPTION_CALLBACK, 0, "stat", &opt_diffstat, NULL,
+ N_("show a diffstat at the end of the merge"),
+ PARSE_OPT_NOARG, parse_opt_pass_strbuf },
+   { OPTION_CALLBACK, 0, "summary", &opt_diffstat, NULL,
+ N_("(synonym to --stat)"),
+ PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, parse_opt_pass_strbuf },
+   { OPTION_CALLBACK, 0, "log", &opt_log, N_("n"),
+ N_("add (at most ) entries from shortlog to merge commit message"),
+ PARSE_OPT_OPTARG, parse_opt_pass_strbuf },
+   { OPTION_CALLBACK, 0, "squash", &opt_squash, NULL,
+ N_("create a single commit instead of doing a merge"),
+ PARSE_OPT_NOARG, parse_opt_pass_strbuf },
+   { OPTION_CALLBACK, 0, "commit", &opt_commit, NULL,
+ N_("perform a commit if the merge succeeds (default)"),
+ PARSE_OPT_NOARG, parse_opt_pass_strbuf },
+   { OPTION_CALLBACK, 0, "edit", &opt_edit, NULL,
+ N_("edit message before committing"),
+ PARSE_OPT_NOARG, parse_opt_pass_strbuf },
+   { OPTION_CALLBACK, 0, "ff", &opt_ff, NULL,
+ N_("allow fast-forward"),
+ PARSE_OPT_NOARG, parse_opt_pass_strbuf },
+   { OPTION_CALLBACK, 0, "ff-only", &opt_ff, NULL,
+ N_("abort if fast-forward is not possible"),
+ PARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_pass_strbuf },
+   { OPTION_CALLBACK, 0, "verify-signatures", &opt_verify_signatures, NULL,
+ N_("verify that the named commit has a valid GPG signature"),
+ PARSE_OPT_NOARG, parse_opt_pass_strbuf },
+   { OPTION_CALLBACK, 's', "strategy", &opt_strategies, N_("strategy"),
+ N_("merge strategy to use"),
+ 0, parse_opt_pass_argv_array },
+   { OPTION_CALLBACK, 'X', "strategy-option", &opt_strategy_opts,
+ N_("option=value"),
+ N_("option for selected merge strategy"),
+ 0, parse_opt_pass_argv_array },
+   { OPTION_CAL

[PATCH v2 09/19] pull: error on no merge candidates

2015-06-02 Thread Paul Tan
Commit a8c9bef (pull: improve advice for unconfigured error case,
2009-10-05) fully established the current advices given by git-pull for
the different cases where git-fetch will not have anything marked for
merge:

1. We fetched from a specific remote, and a refspec was given, but it
   ended up not fetching anything. This is usually because the user
   provided a wildcard refspec which had no matches on the remote end.

2. We fetched from a non-default remote, but didn't specify a branch to
   merge. We can't use the configured one because it applies to the
   default remote, and thus the user must specify the branches to merge.

3. We fetched from the branch's or repo's default remote, but:

   a. We are not on a branch, so there will never be a configured branch
  to merge with.

   b. We are on a branch, but there is no configured branch to merge
  with.

4. We fetched from the branch's or repo's default remote, but the
   configured branch to merge didn't get fetched (either it doesn't
   exist, or wasn't part of the configured fetch refspec)

Re-implement the above behavior by implementing get_merge_heads() to
parse the heads in FETCH_HEAD for merging, and implementing
die_no_merge_candidates(), which will be called when FETCH_HEAD has no
heads for merging.

Helped-by: Johannes Schindelin 
Signed-off-by: Paul Tan 
---

Notes:
v2

* Switched to using fprintf_ln() which will append the trailing newline
  at the end for us.

* The die_no_merge_candidates() code has been reorganized a bit to
  support the later patch which will tweak the messages to be aware of
  git-pull --rebase.

 builtin/pull.c | 113 +
 1 file changed, 113 insertions(+)

diff --git a/builtin/pull.c b/builtin/pull.c
index 0b66b43..42a061d 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -10,6 +10,8 @@
 #include "parse-options.h"
 #include "exec_cmd.h"
 #include "run-command.h"
+#include "sha1-array.h"
+#include "remote.h"
 
 static const char * const pull_usage[] = {
N_("git pull [options] [ [...]]"),
@@ -165,6 +167,111 @@ static void argv_push_force(struct argv_array *arr)
 }
 
 /**
+ * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge
+ * into merge_heads.
+ */
+static void get_merge_heads(struct sha1_array *merge_heads)
+{
+   const char *filename = git_path("FETCH_HEAD");
+   FILE *fp;
+   struct strbuf sb = STRBUF_INIT;
+   unsigned char sha1[GIT_SHA1_RAWSZ];
+
+   if (!(fp = fopen(filename, "r")))
+   die_errno(_("could not open '%s' for reading"), filename);
+   while(strbuf_getline(&sb, fp, '\n') != EOF) {
+   if (get_sha1_hex(sb.buf, sha1))
+   continue;  /* invalid line: does not start with SHA1 */
+   if (starts_with(sb.buf + GIT_SHA1_HEXSZ, "\tnot-for-merge"))
+   continue;  /* ref is not-for-merge */
+   sha1_array_append(merge_heads, sha1);
+   }
+   fclose(fp);
+   strbuf_release(&sb);
+}
+
+/**
+ * Used by die_no_merge_candidates() as a for_each_remote() callback to
+ * retrieve the name of the remote if the repository only has one remote.
+ */
+static int get_only_remote(struct remote *remote, void *cb_data)
+{
+   const char **remote_name = cb_data;
+
+   if (*remote_name)
+   return -1;
+
+   *remote_name = remote->name;
+   return 0;
+}
+
+/**
+ * Dies with the appropriate reason for why there are no merge candidates:
+ *
+ * 1. We fetched from a specific remote, and a refspec was given, but it ended
+ *up not fetching anything. This is usually because the user provided a
+ *wildcard refspec which had no matches on the remote end.
+ *
+ * 2. We fetched from a non-default remote, but didn't specify a branch to
+ *merge. We can't use the configured one because it applies to the default
+ *remote, thus the user must specify the branches to merge.
+ *
+ * 3. We fetched from the branch's or repo's default remote, but:
+ *
+ *a. We are not on a branch, so there will never be a configured branch to
+ *   merge with.
+ *
+ *b. We are on a branch, but there is no configured branch to merge with.
+ *
+ * 4. We fetched from the branch's or repo's default remote, but the configured
+ *branch to merge didn't get fetched. (Either it doesn't exist, or wasn't
+ *part of the configured fetch refspec.)
+ */
+static void NORETURN die_no_merge_candidates(const char *repo, const char 
**refspecs)
+{
+   struct branch *curr_branch = branch_get("HEAD");
+   const char *remote = curr_branch ? curr_branch->remote_name : NULL;
+
+   if (*refspe

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

2015-06-02 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 
---
 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 2057a9d..67cef1c 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 4e1ab5b..dad49cf 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -805,13 +805,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 v2 10/19] pull: support pull.ff config

2015-06-02 Thread Paul Tan
Since b814da8 (pull: add pull.ff configuration, 2014-01-15), git-pull.sh
would lookup the configuration value of "pull.ff", and set the flag
"--ff" if its value is "true", "--no-ff" if its value is "false" and
"--ff-only" if its value is "only".

Re-implement this behavior.

Signed-off-by: Paul Tan 
---

Notes:
v2

* Yup, I did mean strcmp(value, "only")

 builtin/pull.c | 29 +
 1 file changed, 29 insertions(+)

diff --git a/builtin/pull.c b/builtin/pull.c
index 42a061d..1c1883d 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -167,6 +167,32 @@ static void argv_push_force(struct argv_array *arr)
 }
 
 /**
+ * If pull.ff is "true", sets sb to "--ff". If pull.ff is "false", sets sb to
+ * "--no-ff". If pull.ff is "only", sets sb to "--ff-only". If pull.ff is
+ * set to an invalid value, die with an error.
+ */
+static void config_get_ff(struct strbuf *sb)
+{
+   const char *value;
+
+   if (git_config_get_value("pull.ff", &value))
+   return;
+   switch (git_config_maybe_bool("pull.ff", value)) {
+   case 0:
+   strbuf_addstr(sb, "--no-ff");
+   return;
+   case 1:
+   strbuf_addstr(sb, "--ff");
+   return;
+   }
+   if (!strcmp(value, "only")) {
+   strbuf_addstr(sb, "--ff-only");
+   return;
+   }
+   die(_("Invalid value for pull.ff: %s"), value);
+}
+
+/**
  * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge
  * into merge_heads.
  */
@@ -396,6 +422,9 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
 
parse_repo_refspecs(argc, argv, &repo, &refspecs);
 
+   if (!opt_ff.len)
+   config_get_ff(&opt_ff);
+
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 v2 01/19] parse-options-cb: implement parse_opt_pass_strbuf()

2015-06-02 Thread Paul Tan
Certain git commands, such as git-pull, are simply wrappers around other
git commands like git-fetch, git-merge and git-rebase. As such, these
wrapper commands will typically need to "pass through" command-line
options of the commands they wrap.

Implement the parse_opt_pass_strbuf() parse-options callback, which will
reconstruct the command-line option into an strbuf, such that it can be
passed to another git command.

Helped-by: Johannes Schindelin 
Signed-off-by: Paul Tan 
---

Notes:
v2

* Previously implemented as a static function in builtin/pull.c. It now
  uses an strbuf instead of returning newly-allocated strings, to make
  memory management easier.

* We don't use defval anymore. Instead, we use long_name and short_name.

 parse-options-cb.c | 29 +
 parse-options.h|  1 +
 2 files changed, 30 insertions(+)

diff --git a/parse-options-cb.c b/parse-options-cb.c
index be8c413..5b1dbcf 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -134,3 +134,32 @@ int parse_opt_noop_cb(const struct option *opt, const char 
*arg, int unset)
 {
return 0;
 }
+
+/**
+ * For an option opt, recreates the command-line option in opt->value which
+ * must be an strbuf. This is useful when we need to pass the command-line
+ * option to another command.
+ */
+int parse_opt_pass_strbuf(const struct option *opt, const char *arg, int unset)
+{
+   struct strbuf *sb = opt->value;
+
+   strbuf_reset(sb);
+
+   if (opt->long_name) {
+   strbuf_addstr(sb, unset ? "--no-" : "--");
+   strbuf_addstr(sb, opt->long_name);
+   if (arg) {
+   strbuf_addch(sb, '=');
+   strbuf_addstr(sb, arg);
+   }
+   } else if (opt->short_name && !unset) {
+   strbuf_addch(sb, '-');
+   strbuf_addch(sb, opt->short_name);
+   if (arg)
+   strbuf_addstr(sb, arg);
+   } else
+   return -1;
+
+   return 0;
+}
diff --git a/parse-options.h b/parse-options.h
index c71e9da..1d21398 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -224,6 +224,7 @@ extern int parse_opt_with_commit(const struct option *, 
const char *, int);
 extern int parse_opt_tertiary(const struct option *, const char *, int);
 extern int parse_opt_string_list(const struct option *, const char *, int);
 extern int parse_opt_noop_cb(const struct option *, const char *, int);
+extern int parse_opt_pass_strbuf(const struct option *, const char *, int);
 
 #define OPT__VERBOSE(var, h)  OPT_COUNTUP('v', "verbose", (var), (h))
 #define OPT__QUIET(var, h)OPT_COUNTUP('q', "quiet",   (var), (h))
-- 
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 v2 00/19] Make git-pull a builtin

2015-06-02 Thread Paul Tan
This is a re-roll of [v1]. Thanks Johannes, Stefan and Junio for the reviews
last round.

Previous versions:

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

git-pull is a commonly executed command to check for new changes in the
upstream repository and, if there are, fetch and integrate them into the
current branch. Currently it is implemented by the shell script git-pull.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 series rewrites git-pull.sh into a C builtin, thus improving its
performance and portability. It is part of my GSoC project to rewrite git-pull
and git-am into builtins[1].

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


Paul Tan (19):
  parse-options-cb: implement parse_opt_pass_strbuf()
  parse-options-cb: implement parse_opt_pass_argv_array()
  argv-array: implement argv_array_pushv()
  pull: implement skeletal builtin pull
  pull: implement fetch + merge
  pull: pass verbosity, --progress flags to fetch and merge
  pull: pass git-merge's options to git-merge
  pull: pass git-fetch's options to git-fetch
  pull: error on no merge candidates
  pull: support pull.ff config
  pull: check if in unresolved merge state
  pull: fast-forward working tree if head is updated
  pull: implement pulling into an unborn branch
  pull: set reflog message
  pull: teach git pull about --rebase
  pull: configure --rebase via branch..rebase or pull.rebase
  pull --rebase: exit early when the working directory is dirty
  pull --rebase: error on no merge candidate cases
  pull: remove redirection to git-pull.sh

 Documentation/technical/api-argv-array.txt  |   3 +
 Makefile|   2 +-
 advice.c|   8 +
 advice.h|   1 +
 argv-array.c|   6 +
 argv-array.h|   1 +
 builtin.h   |   1 +
 builtin/pull.c  | 888 
 git-pull.sh => contrib/examples/git-pull.sh |   0
 git.c   |   1 +
 parse-options-cb.c  |  61 ++
 parse-options.h |   2 +
 12 files changed, 973 insertions(+), 1 deletion(-)
 create mode 100644 builtin/pull.c
 rename git-pull.sh => contrib/examples/git-pull.sh (100%)

-- 
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 v2 02/19] parse-options-cb: implement parse_opt_pass_argv_array()

2015-06-02 Thread Paul Tan
Certain git commands, such as git-pull, are simply wrappers around other
git commands like git-fetch, git-merge and git-rebase. As such, these
wrapper commands will typically need to "pass through" command-line
options of the commands they wrap.

Implement the parse_opt_pass_argv_array() parse-options callback, which
will reconstruct all the provided command-line options into an
argv_array, such that it can be passed to another git command. This is
useful for passing command-line options that can be specified multiple
times.

Signed-off-by: Paul Tan 
---

Notes:
v2

* This function is a requirement for the rewrite of git-am to handle
  passing git-apply's options to git-apply. Since it would be
  implemented anyway I thought it would be good if git-pull could take
  advantage of it as well to handle --strategy and --strategy-option.

 parse-options-cb.c | 32 
 parse-options.h|  1 +
 2 files changed, 33 insertions(+)

diff --git a/parse-options-cb.c b/parse-options-cb.c
index 5b1dbcf..7330506 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -4,6 +4,7 @@
 #include "commit.h"
 #include "color.h"
 #include "string-list.h"
+#include "argv-array.h"
 
 /*- some often used options -*/
 
@@ -163,3 +164,34 @@ int parse_opt_pass_strbuf(const struct option *opt, const 
char *arg, int unset)
 
return 0;
 }
+
+/**
+ * For an option opt, recreate the command-line option, appending it to
+ * opt->value which must be a argv_array. This is useful when we need to pass
+ * the command-line option, which can be specified multiple times, to another
+ * command.
+ */
+int parse_opt_pass_argv_array(const struct option *opt, const char *arg, int 
unset)
+{
+   struct strbuf sb = STRBUF_INIT;
+   struct argv_array *opt_value = opt->value;
+
+   if (opt->long_name) {
+   strbuf_addstr(&sb, unset ? "--no-" : "--");
+   strbuf_addstr(&sb, opt->long_name);
+   if (arg) {
+   strbuf_addch(&sb, '=');
+   strbuf_addstr(&sb, arg);
+   }
+   } else if (opt->short_name && !unset) {
+   strbuf_addch(&sb, '-');
+   strbuf_addch(&sb, opt->short_name);
+   if (arg)
+   strbuf_addstr(&sb, arg);
+   } else
+   return -1;
+
+   argv_array_push(opt_value, sb.buf);
+   strbuf_release(&sb);
+   return 0;
+}
diff --git a/parse-options.h b/parse-options.h
index 1d21398..b663f87 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -225,6 +225,7 @@ extern int parse_opt_tertiary(const struct option *, const 
char *, int);
 extern int parse_opt_string_list(const struct option *, const char *, int);
 extern int parse_opt_noop_cb(const struct option *, const char *, int);
 extern int parse_opt_pass_strbuf(const struct option *, const char *, int);
+extern int parse_opt_pass_argv_array(const struct option *, const char *, int);
 
 #define OPT__VERBOSE(var, h)  OPT_COUNTUP('v', "verbose", (var), (h))
 #define OPT__QUIET(var, h)OPT_COUNTUP('q', "quiet",   (var), (h))
-- 
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] pull: allow dirty tree when rebase.autostash enabled

2015-06-02 Thread Paul Tan
Hi,

Some comments which may not necessarily be correct.

On Wed, Jun 3, 2015 at 5:55 AM, Kevin Daudt  wrote:
> rebase learned to stash changes when it encounters a dirty work tree, but
> git pull --rebase does not.
>
> Only verify if the working tree is dirty when rebase.autostash is not
> enabled.
> ---

Missing sign-off.

>  git-pull.sh |  5 -
>  t/t5520-pull.sh | 17 +
>  2 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/git-pull.sh b/git-pull.sh
> index 0917d0d..6b9e8a3 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -239,7 +239,10 @@ test true = "$rebase" && {
> die "$(gettext "updating an unborn branch with 
> changes added to the index")"
> fi
> else
> -   require_clean_work_tree "pull with rebase" "Please commit or 
> stash them."
> +   if [ $(git config --bool --get rebase.autostash || echo 
> false) = "false" ]

"false" doesn't need to be quoted.

> +   then
> +   require_clean_work_tree "pull with rebase" "Please 
> commit or stash them."
> +   fi
> fi
> oldremoteref= &&
> test -n "$curr_branch" &&
> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
> index 7efd45b..d849a19 100755
> --- a/t/t5520-pull.sh
> +++ b/t/t5520-pull.sh
> @@ -297,6 +297,23 @@ test_expect_success 'pull --rebase dies early with dirty 
> working directory' '
>
>  '
>
> +test_expect_success 'pull --rebase succeeds with dirty working directory and 
> rebase.autostash set' '
> +

I know the surrounding old tests use a newline, but I think that all
new tests should use the modern style of not having a newline, since
t5520 already consists of a mix of old and modern styles anyway.

> +   test_when_finished "git rm -f file4" &&

There is trailing whitespace here.

Furthermore, git rm -f will fail if "file4" does not exist in the
index. Perhaps it should be moved below the "git add" below.

> +   git checkout to-rebase &&
> +   git update-ref refs/remotes/me/copy copy^ &&
> +   COPY=$(git rev-parse --verify me/copy) &&

$COPY is not used anywhere in the test.

> +   git rebase --onto $COPY copy &&
> +   test_config branch.to-rebase.remote me &&
> +   test_config branch.to-rebase.merge refs/heads/copy &&
> +   test_config branch.to-rebase.rebase true &&
> +   test_config rebase.autostash true &&
> +   echo dirty >> file4 &&

file4 does not exist, so we don't need to append to it. I know the
above few tests do not adhere to it, but CodingGuidelines says that
redirection operators do not have a space after

> +   git add file4 &&
> +   git pull

I think we should check for file contents to ensure that
git-pull/git-stash/git-rebase is doing its job properly.

> +

Same as above, no need the newline.

> +'
> +

With all that said, I wonder if this test, and the test above ("pull
--rebase dies early with dirty working directory") could be vastly
simplified, since we are not testing if we can handle a rebased
upstream.

E.g., my simplified version for the above test would be something like:

git checkout -f to-rebase &&
git rebase --onto copy^ copy &&
test_config rebase.autostash true &&
echo dirty >file4 &&
git add file4 &&
test_when_finished "git rm -f file4" &&
git pull --rebase . me/copy &&
test "$(cat file4)" = dirty &&
test "$(cat file2)" = file

It's still confusing though, because we cannot take advantage of the
'before-rebase' tag introduced in the above tests. I would much prefer
if this test and the ("pull --rebase dies with dirty working
directory") test could be moved to the --rebase tests at lines 214+.
Also, this section in the t5520 test suite always gives me a headache
trying to decipher what it is trying to do ><

Thanks,
Paul
--
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 v2 2/2] git-am: add am.threeWay config variable

2015-06-02 Thread Paul Tan
Hi,

On Tue, Jun 2, 2015 at 9:37 PM, Matthieu Moy
 wrote:
> Remi Lespinet  writes:
>
>> +if test "$(git config --bool --get am.threeWay)" = true
>> +then
>> +threeway=t
>> +fi
>
> I think you missed Paul's remark on this:
>
> http://article.gmane.org/gmane.comp.version-control.git/270150
>
> Not terribly important since am will be rewritten soon, though.

As the person who had to do four preparatory patch series' to fix bugs
for the rewrite of git-pull, I respectfully disagree ;-)

Regards,
Paul
--
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 4/8] am: split out mbox/maildir patches with git-mailsplit

2015-06-02 Thread Paul Tan
On Fri, May 29, 2015 at 7:05 AM, Eric Sunshine  wrote:
> On Wed, May 27, 2015 at 9:33 AM, Paul Tan  wrote:
>> @@ -128,13 +190,32 @@ static void am_next(struct am_state *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;
>> +}
>> +
>>  struct am_state state;
>> +int opt_patch_format;
>
> Should these two variables be static?

Whoops. Yes, they should.

Thanks,
Paul
--
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] pull: handle git-fetch's options as well

2015-06-02 Thread Paul Tan
While parsing the command-line arguments, git-pull stops parsing at the
first unrecognized option, assuming that any subsequent options are for
git-fetch, and can thus be kept in the shell's positional parameters
list, so that it can be passed to git-fetch via the expansion of "$@".

However, certain functions in git-pull assume that the positional
parameters do not contain any options:

* error_on_no_merge_candidates() uses the number of positional
  parameters to determine which error message to print out, and will
  thus print the wrong message if git-fetch's options are passed in as
  well.

* the call to get_remote_merge_branch() assumes that the positional
  parameters only contains the optional repo and refspecs, and will
  thus silently fail if git-fetch's options are passed in as well.

* --dry-run is a valid git-fetch option, but if provided after any
  git-fetch options, it is not recognized by git-pull and thus git-pull
  will continue to run the merge or rebase.

Fix these bugs by teaching git-pull to parse git-fetch's options as
well. Add tests to prevent regressions.

This removes the limitation where git-fetch's options have to come after
git-merge's and git-rebase's options on the command line. Update the
documentation to reflect this.

Signed-off-by: Paul Tan 
---

Notes:
Improve git-pull's option parsing

Previous versions:

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

This patch series is based on pt/pull-tests.

While parsing the command-line arguments, git-pull stops parsing at the
first unrecognized option, assuming that any subsequent options are for
git-fetch, and can thus be kept in the shell's positional parameters
list, so that it can be passed to git-fetch via the expansion of "$ 
".

However, certain functions in git-pull assume that the positional
parameters do not contain any options. Fix this by making git-pull
handle git-fetch's options as well at the option parsing stage.

With this change in place, we can move on to migrate git-pull to use
git-rev-parse --parseopt such that its option parsing is consistent with
the other git commands.

I believe this is the last required behavior change for my rewrite of
git-pull.sh to C.

v2

* Initialize variables to prevent them from being set in the command
  line.

* Update the documentation as well.

 Documentation/git-pull.txt |  3 ---
 git-pull.sh| 37 ++---
 t/t5520-pull.sh| 20 
 t/t5521-pull-options.sh| 14 ++
 4 files changed, 68 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 712ab4b..93c72a2 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -74,9 +74,6 @@ pulling or stash them away with linkgit:git-stash[1].
 OPTIONS
 ---
 
-Options meant for 'git pull' itself and the underlying 'git merge'
-must be given before the options meant for 'git fetch'.
-
 -q::
 --quiet::
This is passed to both underlying git-fetch to squelch reporting of
diff --git a/git-pull.sh b/git-pull.sh
index 0917d0d..623ba7a 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -44,7 +44,8 @@ bool_or_string_config () {
 
 strategy_args= diffstat= no_commit= squash= no_ff= ff_only=
 log_arg= verbosity= progress= recurse_submodules= verify_signatures=
-merge_args= edit= rebase_args=
+merge_args= edit= rebase_args= all= append= upload_pack= force= tags= prune=
+keep= depth= unshallow= update_shallow= refmap=
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short="${curr_branch#refs/heads/}"
 rebase=$(bool_or_string_config branch.$curr_branch_short.rebase)
@@ -166,11 +167,39 @@ do
--d|--dr|--dry|--dry-|--dry-r|--dry-ru|--dry-run)
dry_run=--dry-run
;;
+   --all|--no-all)
+   all=$1 ;;
+   -a|--append|--no-append)
+   append=$1 ;;
+   --upload-pack=*|--no-upload-pack)
+   upload_pack=$1 ;;
+   -f|--force|--no-force)
+   force="$force $1" ;;
+   -t|--tags|--no-tags)
+   tags=$1 ;;
+   -p|--prune|--no-prune)
+   prune=$1 ;;
+   -k|--keep|--no-keep)
+   keep=$1 ;;
+   --depth=*|--no-depth)
+   depth=$1 ;;
+   --unshallow|--no-unshallow)
+   unshallow=$1 ;;
+   --update-shallow|--no-update-shallow)
+   update_shallow=$1 ;;
+   --refmap=*|--no-refmap)
+   refmap=$1 ;;
-h|--help-all)
usage
;;
+   --)
+   shift
+   break
+   ;;
+   -*)
+   usage
+   ;;
*)
-   # Pass thru anything tha

[PATCH v2 0/2] Improve git-pull's option parsing

2015-06-02 Thread Paul Tan
This is a re-roll of [v1]. Thanks Johannes for the reviews last round.

Previous versions:

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

This patch series is based on pt/pull-tests.

While parsing the command-line arguments, git-pull stops parsing at the
first unrecognized option, assuming that any subsequent options are for
git-fetch, and can thus be kept in the shell's positional parameters
list, so that it can be passed to git-fetch via the expansion of "$@".

However, certain functions in git-pull assume that the positional
parameters do not contain any options. Fix this by making git-pull
handle git-fetch's options as well at the option parsing stage.

With this change in place, we can move on to migrate git-pull to use
git-rev-parse --parseopt such that its option parsing is consistent with
the other git commands.

I believe this is the last required behavior change for my rewrite of
git-pull.sh to C.

Paul Tan (2):
  pull: handle git-fetch's options as well
  pull: use git-rev-parse --parseopt for option parsing

 Documentation/git-pull.txt |   3 --
 git-pull.sh| 128 +++--
 t/t5520-pull.sh|  20 +++
 t/t5521-pull-options.sh|  14 +
 4 files changed, 122 insertions(+), 43 deletions(-)

-- 
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 v2 2/2] pull: use git-rev-parse --parseopt for option parsing

2015-06-02 Thread Paul Tan
To enable unambiguous parsing of abbreviated options, bundled short
options, separate form options and to provide consistent usage help, use
git-rev-parse --parseopt for option parsing. With this, simplify the
option parsing code.

Signed-off-by: Paul Tan 
---

Notes:
v2

* Don't use OPTIONS_KEEPDASHDASH

 git-pull.sh | 97 -
 1 file changed, 57 insertions(+), 40 deletions(-)

diff --git a/git-pull.sh b/git-pull.sh
index 623ba7a..a814bf6 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -4,13 +4,53 @@
 #
 # Fetch one or more remote refs and merge it/them into the current HEAD.
 
-USAGE='[-n | --no-stat] [--[no-]commit] [--[no-]squash] [--[no-]ff|--ff-only] 
[--[no-]rebase|--rebase=preserve] [-s strategy]... []  
...'
-LONG_USAGE='Fetch one or more remote refs and integrate it/them with the 
current HEAD.'
 SUBDIRECTORY_OK=Yes
-OPTIONS_SPEC=
+OPTIONS_KEEPDASHDASH=
+OPTIONS_STUCKLONG=Yes
+OPTIONS_SPEC="\
+git pull [options] [ [...]]
+
+Fetch one or more remote refs and integrate it/them with the current HEAD.
+--
+v,verbose  be more verbose
+q,quietbe more quiet
+progress   force progress reporting
+
+  Options related to merging
+r,rebase?false|true|preserve incorporate changes by rebasing rather than 
merging
+n! do not show a diffstat at the end of the merge
+stat   show a diffstat at the end of the merge
+summary(synonym to --stat)
+log?n  add (at most ) entries from shortlog to merge 
commit message
+squash create a single commit instead of doing a merge
+commit perform a commit if the merge succeeds (default)
+e,edit   edit message before committing
+ff allow fast-forward
+ff-only!   abort if fast-forward is not possible
+verify-signatures  verify that the named commit has a valid GPG 
signature
+s,strategy=strategymerge strategy to use
+X,strategy-option=option   option for selected merge strategy
+S,gpg-sign?key-id  GPG sign commit
+
+  Options related to fetching
+allfetch from all remotes
+a,append   append to .git/FETCH_HEAD instead of overwriting
+upload-pack=path   path to upload pack on remote end
+f,forceforce overwrite of local branch
+t,tags fetch all tags and associated objects
+p,pruneprune remote-tracking branches no longer on remote
+recurse-submodules?on-demand control recursive fetching of submodules
+dry-rundry run
+k,keep keep downloaded pack
+depth=depthdeepen history of shallow clone
+unshallow  convert to a complete repository
+update-shallow accept refs that update .git/shallow
+refmap=refmap  specify fetch refmap
+"
+test $# -gt 0 && args="$*"
 . git-sh-setup
 . git-sh-i18n
-set_reflog_action "pull${1+ $*}"
+set_reflog_action "pull${args+ $args}"
 require_work_tree_exists
 cd_to_toplevel
 
@@ -87,17 +127,17 @@ do
diffstat=--stat ;;
--log|--log=*|--no-log)
log_arg="$1" ;;
-   --no-c|--no-co|--no-com|--no-comm|--no-commi|--no-commit)
+   --no-commit)
no_commit=--no-commit ;;
-   --c|--co|--com|--comm|--commi|--commit)
+   --commit)
no_commit=--commit ;;
-e|--edit)
edit=--edit ;;
--no-edit)
edit=--no-edit ;;
-   --sq|--squ|--squa|--squas|--squash)
+   --squash)
squash=--squash ;;
-   --no-sq|--no-squ|--no-squa|--no-squas|--no-squash)
+   --no-squash)
squash=--no-squash ;;
--ff)
no_ff=--ff ;;
@@ -105,39 +145,19 @@ do
no_ff=--no-ff ;;
--ff-only)
ff_only=--ff-only ;;
-   -s=*|--s=*|--st=*|--str=*|--stra=*|--strat=*|--strate=*|\
-   --strateg=*|--strategy=*|\
-   -s|--s|--st|--str|--stra|--strat|--strate|--strateg|--strategy)
-   case "$#,$1" in
-   *,*=*)
-   strategy=$(expr "z$1" : 'z-[^=]*=\(.*\)') ;;
-   1,*)
-   usage ;;
-   *)
-   strategy="$2"
-   shift ;;
-   esac
-   strategy_args="${strategy_args}-s $strategy "
+   -s*|--strategy=*)
+   strategy_args="$strategy_args $1"
;;
-   -X*)
-   case "$#,$1" in
-   1,-X)
-   usage ;;
-   *,-X)
-   xx="-X $(git rev-par

Re: [PATCH 11/14] pull: teach git pull about --rebase

2015-06-02 Thread Paul Tan
On Sun, May 31, 2015 at 4:18 PM, Paul Tan  wrote:
> On Tue, May 19, 2015 at 9:04 PM, Johannes Schindelin
>  wrote:
>> Also, I wonder if something like this would do the job:
>> spec = parse_fetch_refspec(1, &refspec);
>> if (spec->dst)
>> return spec->dst;
>
> Hmm, I notice that get_remote_merge_branch() only looks at the src
> part of the refspec. However, I guess it is true that if the dst part
> is provided, the user may be wishing for that to be interpreted as the
> "remote tracking branch", so we should be looking at it to calculate
> the fork point.
>
>> if (!(remote = get_remote(remote_name)))
>> return NULL;
>> if (remote_find_tracking(remote, spec))
>> return spec->dst;
>
> ... and if the dst part of the refspec is not provided, we fall back
> to see if there is any remote tracking branch in the repo for the src
> part of the ref, which matches the intention of
> get_remote_merge_branch() I think, while being better because
> remote_find_tracking() takes into account the actual configured fetch
> refspecs for the remote.
>
> However, we also need to consider if the user provided a wildcard
> refspec, as it will not make sense in this case. From my reading,
> remote_find_tracking(), which calls query_refspecs(), would just match
> the src part literally, so I guess we should explicitly detect and
> error out in this case.

With all that said, after thinking about it I feel that this patch
series should focus solely on rewriting git-pull.sh 1:1. While I do
agree with the above suggested improvements, I think they should be
implemented as separated patch(es) on top of this series since we
would be technically changing git-pull's behavior, even if we are
improving it.

As such, the issue that I think should be focused on for this patch
is: does get_merge_branch_1() and get_merge_branch_2() in this patch
implement the same behavior as get_remote_merge_branch() in
git-parse.remote.sh? If it does, then its purpose is fulfilled.

So, I'll keep the overall logic of get_merge_branch_2() the same for
the next re-roll. (Other than renaming the function and fixing code
style issues). Once this series is okay, I'll look into doing a
separate patch on top that changes the function to use
remote_find_tracking() so that we fix the assumption that the default
fetch mapping is used.

The other possibility is that we fix this in git-parse-remote.sh, but
I'm personally getting a bit tired from having to re-implement the
same thing in shell script and C. Furthermore, the only script using
get_remote_merge_branch() is git-pull.sh.

> [...]
> Since this is just a 1:1 rewrite I just tried to keep as close to the
> original as possible. However, thinking about it, since we *are* just
> using the first refspec for fork point calculation, I do agree that we
> should probably give an warning() here as well if the user provided
> more than one refspec, like "Cannot calculate rebase fork point as you
> provided more than one refspec. git-pull will not be able to handle a
> rebased upstream". I do not think it is fatal enough that we should
> error() or die(), as e.g. the first refspec may be a wildcard refspec
> that matches nothing, and the second refspec that matches one merge
> head for rebasing. This is pretty contrived though, but still
> technically legitimate. I dunno.
>[...]
>> We should probably `return error(_"No tracking branch found for %s@, refspec 
>> ? refspec : "HEAD");` so that the user has a chance to understand that there 
>> has been a problem and how to solve it.
>
> Just like the above, I don't think this is serious enough to be
> considered an error() though. Sure, this means that we cannot properly
> handle the case where the upstream has been rebased, but this is not
> always the case. We could probably have a warning() here, but I think
> the message should be improved to tell the user what exactly she is
> losing out on. e.g. "No tracking branch found for %s. git-pull will
> not be able to handle a rebased upstream."

Likewise, I won't introduce the error()s or warning()s for now.

Other than that, all other code style related issues have been/will be
fixed. Thanks for the reviews.

Regards,
Paul
--
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 11/14] pull: teach git pull about --rebase

2015-05-31 Thread Paul Tan
Hi Johannes,

On Tue, May 19, 2015 at 9:04 PM, Johannes Schindelin
 wrote:
> On 2015-05-18 17:06, Paul Tan wrote:
>> +/**
>> + * Given a refspec, returns the merge branch. Returns NULL if the refspec 
>> src
>> + * does not refer to a branch.
>> + *
>> + * FIXME: It should return the tracking branch. Currently only works with 
>> the
>> + * default mapping.
>> + */
>> +static char *get_merge_branch_2(const char *repo, const char *refspec)
>> +{
>> + struct refspec *spec;
>> + const char *remote;
>> + char *merge_branch;
>> +
>> + spec = parse_fetch_refspec(1, &refspec);
>> + remote = spec->src;
>> + if (!*remote || !strcmp(remote, "HEAD"))
>> + remote = "HEAD";
>> + else if (skip_prefix(remote, "heads/", &remote))
>> + ;
>> + else if (skip_prefix(remote, "refs/heads/", &remote))
>> + ;
>> + else if (starts_with(remote, "refs/") ||
>> + starts_with(remote, "tags/") ||
>> + starts_with(remote, "remotes/"))
>> + remote = "";
>> +
>> + if (*remote) {
>> + if (!strcmp(repo, "."))
>> + merge_branch = mkpathdup("refs/heads/%s", remote);
>> + else
>> + merge_branch = mkpathdup("refs/remotes/%s/%s", repo, 
>> remote);
>> + } else
>> + merge_branch = NULL;
>> +
>> + free_refspec(1, spec);
>> + return merge_branch;
>> +}
>
> I have to admit that it took me a substantial amount of time to deduce from 
> the code what `get_merge_branch_2()` really does (judging from the 
> description, I thought that it would be essentially the same as 
> `get_merge_branch_1()` without the hard-coded "HEAD"). If the comment above 
> the function would have said something like:
>
> /**
>  * Given a refspec, returns the name of the local tracking ref.
>  */
>
> I would have had an easier time. Also, I wonder if something like this would 
> do the job:

Yeah whoops, this came from a confusion over the difference over
"merge branch" and "remote tracking branch". A merge branch would be a
remote tracking branch, but a remote tracking branch is not
necessarily a merge branch.

> spec = parse_fetch_refspec(1, &refspec);
> if (spec->dst)
> return spec->dst;

Hmm, I notice that get_remote_merge_branch() only looks at the src
part of the refspec. However, I guess it is true that if the dst part
is provided, the user may be wishing for that to be interpreted as the
"remote tracking branch", so we should be looking at it to calculate
the fork point.

> if (!(remote = get_remote(remote_name)))
> return NULL;
> if (remote_find_tracking(remote, spec))
> return spec->dst;

... and if the dst part of the refspec is not provided, we fall back
to see if there is any remote tracking branch in the repo for the src
part of the ref, which matches the intention of
get_remote_merge_branch() I think, while being better because
remote_find_tracking() takes into account the actual configured fetch
refspecs for the remote.

However, we also need to consider if the user provided a wildcard
refspec, as it will not make sense in this case. From my reading,
remote_find_tracking(), which calls query_refspecs(), would just match
the src part literally, so I guess we should explicitly detect and
error out in this case.

> return NULL;
>
> (I guess we'd have to `xstrdup()` the return values because the return value 
> of `get_merge_branch_1()` needs to be `free()`d, but maybe we can avoid so 
> much `malloc()/free()`ing.) Again, the function name should probably be 
> changed to something clearer, maybe to `get_tracking_branch()`.

Yeah, it should be changed to get_tracking_branch(), since it is
different from get_merge_branch().

> One thing that is not clear at all to me is whether
>
> git pull --rebase origin master next
>
> would error out as expected, or simply rebase to `origin/master`.

In git-pull.sh, for the rebase fork point calculation it just used the
first refspec. However, when running git-rebase it checked to see if
there was only one merge base, which is replicated here:

@@ -573,6 +792,10 @@ int cmd_pull(int argc, const char **argv,
const char *prefix)
if (merge_heads.nr > 1)
die(_("Cannot merge multiple branches into
empty head."));
return pull_into_void(*merge_heads.s

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

2015-05-30 Thread Paul Tan
On Sat, May 30, 2015 at 3:29 PM, Paul Tan  wrote:
> Hi,
> Okay, I'm trying this out in the next re-roll. I do agree that this
> patch series should not touch anything in t/ at all.
>
> One problem(?) is that putting builtins/pull.o in the BUILTIN_OBJS and
> leaving git-pull.sh in SCRIPT_SH in the Makefile will generate 2
> targets to ./git-pull (they will clobber each other). For GNU Make,
> the last defined target will win, so in this case it just happens that
> git-pull.sh will win because the build targets for the shell scripts
> are defined after the build targets for the builtins, so this works in
> our favor I guess.
>
> Regards,
> Paul

Just to add on, I just discovered that test-lib.sh unsets all GIT_*
environment variables "for repeatability", so the name
"GIT_USE_BUILTIN_PULL" cannot be used. I'm tempted to just add a
underscore just before the name ("_GIT_USE_BUILTIN_PULL") to work
around this, since it's just a temporary thing.

Thanks,
Paul
--
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 00/14] Make git-pull a builtin

2015-05-30 Thread Paul Tan
Hi,

On Tue, May 19, 2015 at 3:21 AM, Junio C Hamano  wrote:
> Paul Tan  writes:
>
>> This series rewrites git-pull.sh into a C builtin, thus improving its
>> performance and portability. It is part of my GSoC project to rewrite 
>> git-pull
>> and git-am into builtins[2].
>
> Earlier you were worried about 'git pull' being used in many tests
> for the purpose of testing other parts of the system and not testing
> 'pull' itself.  For a program as complex as 'git pull', you may want
> to take the 'competing implementations' approach.
>
> (1) write an empty cmd_pull() that relaunches "git pull" scripted
> porcelain from the $GIT_EXEC_PATH with given parameters, and
> wire all the necessary bits to git.c.
>
> (2) enhance cmd_pull() a bit by bit, but keep something like this
>
> if (getenv(GIT_USE_BUILTIN_PULL)) {
> /* original run_command("git-pull.sh") code */
> exit $?
> }
>
> ... your "C" version ...
>
> (3) add "GIT_USE_BUILTIN_PULL=Yes; export GIT_USE_BUILTIN_PULL" at
> the beginning of "t55??" test scripts (but not others that rely
> on working pull and that are not interested in catching bugs in
> pull).
>
> (4) once cmd_pull() becomes fully operational, drop (3) and also the
> conditional one you added in (2), and retire the environment
> variable.  Retire the git-pull.sh script to contrib/examples/
> boneyard.
>
> That way, you will always have a reference you can use throughout
> the development.
>
> Just a suggestion, not a requirement.

Okay, I'm trying this out in the next re-roll. I do agree that this
patch series should not touch anything in t/ at all.

One problem(?) is that putting builtins/pull.o in the BUILTIN_OBJS and
leaving git-pull.sh in SCRIPT_SH in the Makefile will generate 2
targets to ./git-pull (they will clobber each other). For GNU Make,
the last defined target will win, so in this case it just happens that
git-pull.sh will win because the build targets for the shell scripts
are defined after the build targets for the builtins, so this works in
our favor I guess.

Regards,
Paul
--
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 7/8] t5521: test --dry-run does not make any changes

2015-05-29 Thread Paul Tan
Test that when --dry-run is provided to git-pull, it does not make any
changes, namely:

* --dry-run gets passed to git-fetch, so no FETCH_HEAD will be created
  and no refs will be fetched.

* The index and work tree will not be modified.

Signed-off-by: Paul Tan 
---

 t/t5521-pull-options.sh | 13 +
 1 file changed, 13 insertions(+)

diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
index 453aba5..56e7377 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -117,4 +117,17 @@ test_expect_success 'git pull --all' '
)
 '
 
+test_expect_success 'git pull --dry-run' '
+   test_when_finished "rm -rf clonedry" &&
+   git init clonedry &&
+   (
+   cd clonedry &&
+   git pull --dry-run ../parent &&
+   test_path_is_missing .git/FETCH_HEAD &&
+   test_path_is_missing .git/refs/heads/master &&
+   test_path_is_missing .git/index &&
+   test_path_is_missing file
+   )
+'
+
 test_done
-- 
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


<    1   2   3   4   5   6   7   >