{"thread":{"id":"39509","subject":"[PATCH v2 01/19] parse-options-cb: implement parse_opt_pass_strbuf()","startedAt":"2015-06-03T06:48:44Z","lastAt":"2015-06-14T07:44:59Z","messageCount":48,"participants":["Paul Tan","Kevin Daudt","Stefan Beller","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":19},"messages":[{"id":"262820","messageId":"1433314143-4478-1-git-send-email-pyokagan@gmail.com","threadId":"39509","inReplyTo":null,"subject":"[PATCH v2 00/19] Make git-pull a builtin","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-03T06:48:44Z","receivedAt":"2015-06-03T06:48:44Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"This is a re-roll of [v1]. Thanks Johannes, Stefan and Junio for the reviews\nlast round.\n\nPrevious versions:\n\n[v1] http://thread.gmane.org/gmane.comp.version-control.git/269258\n\ngit-pull is a commonly executed command to check for new changes in the\nupstream repository and, if there are, fetch and integrate them into the\ncurrent branch. Currently it is implemented by the shell script git-pull.sh.\nHowever, compared to C, shell scripts have certain deficiencies -- they need to\nspawn a lot of processes, introduce a lot of dependencies and cannot take\nadvantage of git's internal caches.\n\nThis series rewrites git-pull.sh into a C builtin, thus improving its\nperformance and portability. It is part of my GSoC project to rewrite git-pull\nand git-am into builtins[1].\n\n[1] https://gist.github.com/pyokagan/1b7b0d1f4dab6ba3cef1\n\n\nPaul Tan (19):\n  parse-options-cb: implement parse_opt_pass_strbuf()\n  parse-options-cb: implement parse_opt_pass_argv_array()\n  argv-array: implement argv_array_pushv()\n  pull: implement skeletal builtin pull\n  pull: implement fetch + merge\n  pull: pass verbosity, --progress flags to fetch and merge\n  pull: pass git-merge's options to git-merge\n  pull: pass git-fetch's options to git-fetch\n  pull: error on no merge candidates\n  pull: support pull.ff config\n  pull: check if in unresolved merge state\n  pull: fast-forward working tree if head is updated\n  pull: implement pulling into an unborn branch\n  pull: set reflog message\n  pull: teach git pull about --rebase\n  pull: configure --rebase via branch.<name>.rebase or pull.rebase\n  pull --rebase: exit early when the working directory is dirty\n  pull --rebase: error on no merge candidate cases\n  pull: remove redirection to git-pull.sh\n\n Documentation/technical/api-argv-array.txt  |   3 +\n Makefile                                    |   2 +-\n advice.c                                    |   8 +\n advice.h                                    |   1 +\n argv-array.c                                |   6 +\n argv-array.h                                |   1 +\n builtin.h                                   |   1 +\n builtin/pull.c                              | 888 ++++++++++++++++++++++++++++\n git-pull.sh => contrib/examples/git-pull.sh |   0\n git.c                                       |   1 +\n parse-options-cb.c                          |  61 ++\n parse-options.h                             |   2 +\n 12 files changed, 973 insertions(+), 1 deletion(-)\n create mode 100644 builtin/pull.c\n rename git-pull.sh => contrib/examples/git-pull.sh (100%)\n\n-- \n2.1.4\n"},{"id":"262819","messageId":"1433314143-4478-2-git-send-email-pyokagan@gmail.com","threadId":"39509","inReplyTo":"1433314143-4478-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 01/19] parse-options-cb: implement parse_opt_pass_strbuf()","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-03T06:48:45Z","receivedAt":"2015-06-03T06:48:45Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Certain git commands, such as git-pull, are simply wrappers around other\ngit commands like git-fetch, git-merge and git-rebase. As such, these\nwrapper commands will typically need to \"pass through\" command-line\noptions of the commands they wrap.\n\nImplement the parse_opt_pass_strbuf() parse-options callback, which will\nreconstruct the command-line option into an strbuf, such that it can be\npassed to another git command.\n\nHelped-by: Johannes Schindelin <johannes.schindelin@gmx.de>\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n\nNotes:\n    v2\n    \n    * Previously implemented as a static function in builtin/pull.c. It now\n      uses an strbuf instead of returning newly-allocated strings, to make\n      memory management easier.\n    \n    * We don't use defval anymore. Instead, we use long_name and short_name.\n\n parse-options-cb.c | 29 +++++++++++++++++++++++++++++\n parse-options.h    |  1 +\n 2 files changed, 30 insertions(+)\n\ndiff --git a/parse-options-cb.c b/parse-options-cb.c\nindex be8c413..5b1dbcf 100644\n--- a/parse-options-cb.c\n+++ b/parse-options-cb.c\n@@ -134,3 +134,32 @@ int parse_opt_noop_cb(const struct option *opt, const char *arg, int unset)\n {\n \treturn 0;\n }\n+\n+/**\n+ * For an option opt, recreates the command-line option in opt->value which\n+ * must be an strbuf. This is useful when we need to pass the command-line\n+ * option to another command.\n+ */\n+int parse_opt_pass_strbuf(const struct option *opt, const char *arg, int unset)\n+{\n+\tstruct strbuf *sb = opt->value;\n+\n+\tstrbuf_reset(sb);\n+\n+\tif (opt->long_name) {\n+\t\tstrbuf_addstr(sb, unset ? \"--no-\" : \"--\");\n+\t\tstrbuf_addstr(sb, opt->long_name);\n+\t\tif (arg) {\n+\t\t\tstrbuf_addch(sb, '=');\n+\t\t\tstrbuf_addstr(sb, arg);\n+\t\t}\n+\t} else if (opt->short_name && !unset) {\n+\t\tstrbuf_addch(sb, '-');\n+\t\tstrbuf_addch(sb, opt->short_name);\n+\t\tif (arg)\n+\t\t\tstrbuf_addstr(sb, arg);\n+\t} else\n+\t\treturn -1;\n+\n+\treturn 0;\n+}\ndiff --git a/parse-options.h b/parse-options.h\nindex c71e9da..1d21398 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -224,6 +224,7 @@ extern int parse_opt_with_commit(const struct option *, const char *, int);\n extern int parse_opt_tertiary(const struct option *, const char *, int);\n extern int parse_opt_string_list(const struct option *, const char *, int);\n extern int parse_opt_noop_cb(const struct option *, const char *, int);\n+extern int parse_opt_pass_strbuf(const struct option *, const char *, int);\n \n #define OPT__VERBOSE(var, h)  OPT_COUNTUP('v', \"verbose\", (var), (h))\n #define OPT__QUIET(var, h)    OPT_COUNTUP('q', \"quiet\",   (var), (h))\n-- \n2.1.4\n"},{"id":"262822","messageId":"1433314143-4478-3-git-send-email-pyokagan@gmail.com","threadId":"39509","inReplyTo":"1433314143-4478-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 02/19] parse-options-cb: implement parse_opt_pass_argv_array()","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-03T06:48:46Z","receivedAt":"2015-06-03T06:48:46Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Certain git commands, such as git-pull, are simply wrappers around other\ngit commands like git-fetch, git-merge and git-rebase. As such, these\nwrapper commands will typically need to \"pass through\" command-line\noptions of the commands they wrap.\n\nImplement the parse_opt_pass_argv_array() parse-options callback, which\nwill reconstruct all the provided command-line options into an\nargv_array, such that it can be passed to another git command. This is\nuseful for passing command-line options that can be specified multiple\ntimes.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n\nNotes:\n    v2\n    \n    * This function is a requirement for the rewrite of git-am to handle\n      passing git-apply's options to git-apply. Since it would be\n      implemented anyway I thought it would be good if git-pull could take\n      advantage of it as well to handle --strategy and --strategy-option.\n\n parse-options-cb.c | 32 ++++++++++++++++++++++++++++++++\n parse-options.h    |  1 +\n 2 files changed, 33 insertions(+)\n\ndiff --git a/parse-options-cb.c b/parse-options-cb.c\nindex 5b1dbcf..7330506 100644\n--- a/parse-options-cb.c\n+++ b/parse-options-cb.c\n@@ -4,6 +4,7 @@\n #include \"commit.h\"\n #include \"color.h\"\n #include \"string-list.h\"\n+#include \"argv-array.h\"\n \n /*----- some often used options -----*/\n \n@@ -163,3 +164,34 @@ int parse_opt_pass_strbuf(const struct option *opt, const char *arg, int unset)\n \n \treturn 0;\n }\n+\n+/**\n+ * For an option opt, recreate the command-line option, appending it to\n+ * opt->value which must be a argv_array. This is useful when we need to pass\n+ * the command-line option, which can be specified multiple times, to another\n+ * command.\n+ */\n+int parse_opt_pass_argv_array(const struct option *opt, const char *arg, int unset)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tstruct argv_array *opt_value = opt->value;\n+\n+\tif (opt->long_name) {\n+\t\tstrbuf_addstr(&sb, unset ? \"--no-\" : \"--\");\n+\t\tstrbuf_addstr(&sb, opt->long_name);\n+\t\tif (arg) {\n+\t\t\tstrbuf_addch(&sb, '=');\n+\t\t\tstrbuf_addstr(&sb, arg);\n+\t\t}\n+\t} else if (opt->short_name && !unset) {\n+\t\tstrbuf_addch(&sb, '-');\n+\t\tstrbuf_addch(&sb, opt->short_name);\n+\t\tif (arg)\n+\t\t\tstrbuf_addstr(&sb, arg);\n+\t} else\n+\t\treturn -1;\n+\n+\targv_array_push(opt_value, sb.buf);\n+\tstrbuf_release(&sb);\n+\treturn 0;\n+}\ndiff --git a/parse-options.h b/parse-options.h\nindex 1d21398..b663f87 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -225,6 +225,7 @@ extern int parse_opt_tertiary(const struct option *, const char *, int);\n extern int parse_opt_string_list(const struct option *, const char *, int);\n extern int parse_opt_noop_cb(const struct option *, const char *, int);\n extern int parse_opt_pass_strbuf(const struct option *, const char *, int);\n+extern int parse_opt_pass_argv_array(const struct option *, const char *, int);\n \n #define OPT__VERBOSE(var, h)  OPT_COUNTUP('v', \"verbose\", (var), (h))\n #define OPT__QUIET(var, h)    OPT_COUNTUP('q', \"quiet\",   (var), (h))\n-- \n2.1.4\n"},{"id":"262821","messageId":"1433314143-4478-4-git-send-email-pyokagan@gmail.com","threadId":"39509","inReplyTo":"1433314143-4478-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 03/19] argv-array: implement argv_array_pushv()","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-03T06:48:47Z","receivedAt":"2015-06-03T06:48:47Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"When we have a null-terminated array, it would be useful to convert it\nor append it to an argv_array for further manipulation.\n\nImplement argv_array_pushv() which will push a null-terminated array of\nstrings on to an argv_array.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n Documentation/technical/api-argv-array.txt | 3 +++\n argv-array.c                               | 6 ++++++\n argv-array.h                               | 1 +\n 3 files changed, 10 insertions(+)\n\ndiff --git a/Documentation/technical/api-argv-array.txt b/Documentation/technical/api-argv-array.txt\nindex 1a79781..8076172 100644\n--- a/Documentation/technical/api-argv-array.txt\n+++ b/Documentation/technical/api-argv-array.txt\n@@ -46,6 +46,9 @@ Functions\n \tFormat a string and push it onto the end of the array. This is a\n \tconvenience wrapper combining `strbuf_addf` and `argv_array_push`.\n \n+`argv_array_pushv`::\n+\tPush a null-terminated array of strings onto the end of the array.\n+\n `argv_array_pop`::\n \tRemove the final element from the array. If there are no\n \telements in the array, do nothing.\ndiff --git a/argv-array.c b/argv-array.c\nindex 256741d..eaed477 100644\n--- a/argv-array.c\n+++ b/argv-array.c\n@@ -49,6 +49,12 @@ void argv_array_pushl(struct argv_array *array, ...)\n \tva_end(ap);\n }\n \n+void argv_array_pushv(struct argv_array *array, const char **argv)\n+{\n+\tfor (; *argv; argv++)\n+\t\targv_array_push(array, *argv);\n+}\n+\n void argv_array_pop(struct argv_array *array)\n {\n \tif (!array->argc)\ndiff --git a/argv-array.h b/argv-array.h\nindex c65e6e8..a2fa0aa 100644\n--- a/argv-array.h\n+++ b/argv-array.h\n@@ -17,6 +17,7 @@ __attribute__((format (printf,2,3)))\n void argv_array_pushf(struct argv_array *, const char *fmt, ...);\n LAST_ARG_MUST_BE_NULL\n void argv_array_pushl(struct argv_array *, ...);\n+void argv_array_pushv(struct argv_array *, const char **);\n void argv_array_pop(struct argv_array *);\n void argv_array_clear(struct argv_array *);\n \n-- \n2.1.4\n"},{"id":"262823","messageId":"1433314143-4478-5-git-send-email-pyokagan@gmail.com","threadId":"39509","inReplyTo":"1433314143-4478-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 04/19] pull: implement skeletal builtin pull","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-03T06:48:48Z","receivedAt":"2015-06-03T06:48:48Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"For the purpose of rewriting git-pull.sh into a C builtin, implement a\nskeletal builtin/pull.c that redirects to $GIT_EXEC_PATH/git-pull.sh if\nthe environment variable _GIT_USE_BUILTIN_PULL is not defined. This\nallows us to fall back on the functional git-pull.sh when running the\ntest suite for tests that depend on a working git-pull implementation.\n\nThis redirection should be removed when all the features of git-pull.sh\nhave been re-implemented in builtin/pull.c.\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n Makefile       |  1 +\n builtin.h      |  1 +\n builtin/pull.c | 33 +++++++++++++++++++++++++++++++++\n git.c          |  1 +\n 4 files changed, 36 insertions(+)\n create mode 100644 builtin/pull.c\n\ndiff --git a/Makefile b/Makefile\nindex 54ec511..2057a9d 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -877,6 +877,7 @@ BUILTIN_OBJS += builtin/pack-refs.o\n BUILTIN_OBJS += builtin/patch-id.o\n BUILTIN_OBJS += builtin/prune-packed.o\n BUILTIN_OBJS += builtin/prune.o\n+BUILTIN_OBJS += builtin/pull.o\n BUILTIN_OBJS += builtin/push.o\n BUILTIN_OBJS += builtin/read-tree.o\n BUILTIN_OBJS += builtin/receive-pack.o\ndiff --git a/builtin.h b/builtin.h\nindex b87df70..ea3c834 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -98,6 +98,7 @@ extern int cmd_pack_redundant(int argc, const char **argv, const char *prefix);\n extern int cmd_patch_id(int argc, const char **argv, const char *prefix);\n extern int cmd_prune(int argc, const char **argv, const char *prefix);\n extern int cmd_prune_packed(int argc, const char **argv, const char *prefix);\n+extern int cmd_pull(int argc, const char **argv, const char *prefix);\n extern int cmd_push(int argc, const char **argv, const char *prefix);\n extern int cmd_read_tree(int argc, const char **argv, const char *prefix);\n extern int cmd_receive_pack(int argc, const char **argv, const char *prefix);\ndiff --git a/builtin/pull.c b/builtin/pull.c\nnew file mode 100644\nindex 0000000..f8b79a2\n--- /dev/null\n+++ b/builtin/pull.c\n@@ -0,0 +1,33 @@\n+/*\n+ * Builtin \"git pull\"\n+ *\n+ * Based on git-pull.sh by Junio C Hamano\n+ *\n+ * Fetch one or more remote refs and merge it/them into the current HEAD.\n+ */\n+#include \"cache.h\"\n+#include \"builtin.h\"\n+#include \"parse-options.h\"\n+#include \"exec_cmd.h\"\n+\n+static const char * const pull_usage[] = {\n+\tNULL\n+};\n+\n+static struct option pull_options[] = {\n+\tOPT_END()\n+};\n+\n+int cmd_pull(int argc, const char **argv, const char *prefix)\n+{\n+\tif (!getenv(\"_GIT_USE_BUILTIN_PULL\")) {\n+\t\tconst char *path = mkpath(\"%s/git-pull\", git_exec_path());\n+\n+\t\tif (sane_execvp(path, (char**) argv) < 0)\n+\t\t\tdie_errno(\"could not exec %s\", path);\n+\t}\n+\n+\targc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0);\n+\n+\treturn 0;\n+}\ndiff --git a/git.c b/git.c\nindex 44374b1..e7a7713 100644\n--- a/git.c\n+++ b/git.c\n@@ -445,6 +445,7 @@ static struct cmd_struct commands[] = {\n \t{ \"pickaxe\", cmd_blame, RUN_SETUP },\n \t{ \"prune\", cmd_prune, RUN_SETUP },\n \t{ \"prune-packed\", cmd_prune_packed, RUN_SETUP },\n+\t{ \"pull\", cmd_pull, RUN_SETUP | NEED_WORK_TREE },\n \t{ \"push\", cmd_push, RUN_SETUP },\n \t{ \"read-tree\", cmd_read_tree, RUN_SETUP },\n \t{ \"receive-pack\", cmd_receive_pack },\n-- \n2.1.4\n"},{"id":"262825","messageId":"1433314143-4478-6-git-send-email-pyokagan@gmail.com","threadId":"39509","inReplyTo":"1433314143-4478-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 05/19] pull: implement fetch + merge","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-03T06:48:49Z","receivedAt":"2015-06-03T06:48:49Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Implement the fetch + merge functionality of git-pull, by first running\ngit-fetch with the repo and refspecs provided on the command line, then\nrunning git-merge on FETCH_HEAD to merge the fetched refs into the\ncurrent branch.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n builtin/pull.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 60 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex f8b79a2..0ca23a3 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -9,8 +9,10 @@\n #include \"builtin.h\"\n #include \"parse-options.h\"\n #include \"exec_cmd.h\"\n+#include \"run-command.h\"\n \n static const char * const pull_usage[] = {\n+\tN_(\"git pull [options] [<repository> [<refspec>...]]\"),\n \tNULL\n };\n \n@@ -18,8 +20,60 @@ static struct option pull_options[] = {\n \tOPT_END()\n };\n \n+/**\n+ * Parses argv into [<repo> [<refspecs>...]], returning their values in `repo`\n+ * as a string and `refspecs` as a null-terminated array of strings. If `repo`\n+ * is not provided in argv, it is set to NULL.\n+ */\n+static void parse_repo_refspecs(int argc, const char **argv, const char **repo,\n+\t\tconst char ***refspecs)\n+{\n+\tif (argc > 0) {\n+\t\t*repo = *argv++;\n+\t\targc--;\n+\t} else\n+\t\t*repo = NULL;\n+\t*refspecs = argv;\n+}\n+\n+/**\n+ * Runs git-fetch, returning its exit status. `repo` and `refspecs` are the\n+ * repository and refspecs to fetch, or NULL if they are not provided.\n+ */\n+static int run_fetch(const char *repo, const char **refspecs)\n+{\n+\tstruct argv_array args = ARGV_ARRAY_INIT;\n+\tint ret;\n+\n+\targv_array_pushl(&args, \"fetch\", \"--update-head-ok\", NULL);\n+\tif (repo)\n+\t\targv_array_push(&args, repo);\n+\twhile (*refspecs)\n+\t\targv_array_push(&args, *refspecs++);\n+\tret = run_command_v_opt(args.argv, RUN_GIT_CMD);\n+\targv_array_clear(&args);\n+\treturn ret;\n+}\n+\n+/**\n+ * Runs git-merge, returning its exit status.\n+ */\n+static int run_merge(void)\n+{\n+\tint ret;\n+\tstruct argv_array args = ARGV_ARRAY_INIT;\n+\n+\targv_array_pushl(&args, \"merge\", NULL);\n+\targv_array_push(&args, \"FETCH_HEAD\");\n+\tret = run_command_v_opt(args.argv, RUN_GIT_CMD);\n+\targv_array_clear(&args);\n+\treturn ret;\n+}\n+\n int cmd_pull(int argc, const char **argv, const char *prefix)\n {\n+\tconst char *repo, **refspecs;\n+\n \tif (!getenv(\"_GIT_USE_BUILTIN_PULL\")) {\n \t\tconst char *path = mkpath(\"%s/git-pull\", git_exec_path());\n \n@@ -29,5 +83,10 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \n \targc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0);\n \n-\treturn 0;\n+\tparse_repo_refspecs(argc, argv, &repo, &refspecs);\n+\n+\tif (run_fetch(repo, refspecs))\n+\t\treturn 1;\n+\n+\treturn run_merge();\n }\n-- \n2.1.4\n"},{"id":"262826","messageId":"1433314143-4478-7-git-send-email-pyokagan@gmail.com","threadId":"39509","inReplyTo":"1433314143-4478-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 06/19] pull: pass verbosity, --progress flags to fetch and merge","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-03T06:48:50Z","receivedAt":"2015-06-03T06:48:50Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"7f87aff (Teach/Fix pull/fetch -q/-v options, 2008-11-15) taught git-pull\nto accept the verbosity -v and -q options and pass them to git-fetch and\ngit-merge.\n\nRe-implement support for the verbosity flags by adding it to the options\nlist and introducing argv_push_verbosity() to push the flags into the\nargv array used to execute git-fetch and git-merge.\n\n9839018 (fetch and pull: learn --progress, 2010-02-24) and bebd2fd\n(pull: propagate --progress to merge, 2011-02-20) taught git-pull to\naccept the --progress option and pass it to git-fetch and git-merge.\n\nRe-implement support for this flag by introducing the option callback\nhandler parse_opt_passthru(). This callback is used to pass the\n\"--progress\" or \"--no-progress\" command-line switch to git-fetch and\ngit-merge.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n\nNotes:\n    v2\n    \n    * Use parse_opt_pass_strbuf().\n\n builtin/pull.c | 36 ++++++++++++++++++++++++++++++++++++\n 1 file changed, 36 insertions(+)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 0ca23a3..c9c2cc0 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -16,11 +16,35 @@ static const char * const pull_usage[] = {\n \tNULL\n };\n \n+/* Shared options */\n+static int opt_verbosity;\n+static struct strbuf opt_progress = STRBUF_INIT;\n+\n static struct option pull_options[] = {\n+\t/* Shared options */\n+\tOPT__VERBOSITY(&opt_verbosity),\n+\t{ OPTION_CALLBACK, 0, \"progress\", &opt_progress, NULL,\n+\t  N_(\"force progress reporting\"),\n+\t  PARSE_OPT_NOARG, parse_opt_pass_strbuf},\n+\n \tOPT_END()\n };\n \n /**\n+ * Pushes \"-q\" or \"-v\" switches into arr to match the opt_verbosity level.\n+ */\n+static void argv_push_verbosity(struct argv_array *arr)\n+{\n+\tint verbosity;\n+\n+\tfor (verbosity = opt_verbosity; verbosity > 0; verbosity--)\n+\t\targv_array_push(arr, \"-v\");\n+\n+\tfor (verbosity = opt_verbosity; verbosity < 0; verbosity++)\n+\t\targv_array_push(arr, \"-q\");\n+}\n+\n+/**\n  * Parses argv into [<repo> [<refspecs>...]], returning their values in `repo`\n  * as a string and `refspecs` as a null-terminated array of strings. If `repo`\n  * is not provided in argv, it is set to NULL.\n@@ -46,6 +70,12 @@ static int run_fetch(const char *repo, const char **refspecs)\n \tint ret;\n \n \targv_array_pushl(&args, \"fetch\", \"--update-head-ok\", NULL);\n+\n+\t/* Shared options */\n+\targv_push_verbosity(&args);\n+\tif (opt_progress.len)\n+\t\targv_array_push(&args, opt_progress.buf);\n+\n \tif (repo)\n \t\targv_array_push(&args, repo);\n \twhile (*refspecs)\n@@ -64,6 +94,12 @@ static int run_merge(void)\n \tstruct argv_array args = ARGV_ARRAY_INIT;\n \n \targv_array_pushl(&args, \"merge\", NULL);\n+\n+\t/* Shared options */\n+\targv_push_verbosity(&args);\n+\tif (opt_progress.len)\n+\t\targv_array_push(&args, opt_progress.buf);\n+\n \targv_array_push(&args, \"FETCH_HEAD\");\n \tret = run_command_v_opt(args.argv, RUN_GIT_CMD);\n \targv_array_clear(&args);\n-- \n2.1.4\n"},{"id":"262824","messageId":"1433314143-4478-8-git-send-email-pyokagan@gmail.com","threadId":"39509","inReplyTo":"1433314143-4478-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 07/19] pull: pass git-merge's options to git-merge","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-03T06:48:51Z","receivedAt":"2015-06-03T06:48:51Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Specify git-merge's options in the option list, and pass any specified\noptions to git-merge.\n\nThese options are:\n\n* -n, --stat, --summary: since d8abe14 (merge, pull: introduce\n  '--(no-)stat' option, 2008-04-06)\n\n* --log: since efb779f (merge, pull: add '--(no-)log' command line\n  option, 2008-04-06)\n\n* --squash: since 7d0c688 (git-merge --squash, 2006-06-23)\n\n* --commit: since 5072a32 (Teach git-pull about --[no-]ff, --no-squash\n  and --commit, 2007-10-29)\n\n* --edit: since 8580830 (\"git pull\" doesn't know \"--edit\", 2012-02-11)\n\n* --ff, --ff-only: since 5072a32 (Teach git-pull about --[no-]ff,\n  --no-squash and --commit, 2007-10-29)\n\n* --verify-signatures: since efed002 (merge/pull: verify GPG signatures\n  of commits being merged, 2013-03-31)\n\n* -s, --strategy: since 60fb5b2 (Use git-merge in git-pull (second\n  try)., 2005-09-25)\n\n* -X, --strategy-option: since ee2c795 (Teach git-pull to pass\n  -X<option> to git-merge, 2009-11-25)\n\n* -S, --gpg-sign: since ea230d8 (pull: add the --gpg-sign option.,\n  2014-02-10)\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n\nNotes:\n    v2\n    \n    * Use opt_parse_pass_strbuf(), opt_parse_pass_argv_array() and\n      argv_array_pushv()\n\n builtin/pull.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 75 insertions(+)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex c9c2cc0..5f08634 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -20,6 +20,18 @@ static const char * const pull_usage[] = {\n static int opt_verbosity;\n static struct strbuf opt_progress = STRBUF_INIT;\n \n+/* Options passed to git-merge */\n+static struct strbuf opt_diffstat = STRBUF_INIT;\n+static struct strbuf opt_log = STRBUF_INIT;\n+static struct strbuf opt_squash = STRBUF_INIT;\n+static struct strbuf opt_commit = STRBUF_INIT;\n+static struct strbuf opt_edit = STRBUF_INIT;\n+static struct strbuf opt_ff = STRBUF_INIT;\n+static struct strbuf opt_verify_signatures = STRBUF_INIT;\n+static struct argv_array opt_strategies = ARGV_ARRAY_INIT;\n+static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;\n+static struct strbuf opt_gpg_sign = STRBUF_INIT;\n+\n static struct option pull_options[] = {\n \t/* Shared options */\n \tOPT__VERBOSITY(&opt_verbosity),\n@@ -27,6 +39,49 @@ static struct option pull_options[] = {\n \t  N_(\"force progress reporting\"),\n \t  PARSE_OPT_NOARG, parse_opt_pass_strbuf},\n \n+\t/* Options passed to git-merge */\n+\tOPT_GROUP(N_(\"Options related to merging\")),\n+\t{ OPTION_CALLBACK, 'n', NULL, &opt_diffstat, NULL,\n+\t  N_(\"do not show a diffstat at the end of the merge\"),\n+\t  PARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_pass_strbuf },\n+\t{ OPTION_CALLBACK, 0, \"stat\", &opt_diffstat, NULL,\n+\t  N_(\"show a diffstat at the end of the merge\"),\n+\t  PARSE_OPT_NOARG, parse_opt_pass_strbuf },\n+\t{ OPTION_CALLBACK, 0, \"summary\", &opt_diffstat, NULL,\n+\t  N_(\"(synonym to --stat)\"),\n+\t  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, parse_opt_pass_strbuf },\n+\t{ OPTION_CALLBACK, 0, \"log\", &opt_log, N_(\"n\"),\n+\t  N_(\"add (at most <n>) entries from shortlog to merge commit message\"),\n+\t  PARSE_OPT_OPTARG, parse_opt_pass_strbuf },\n+\t{ OPTION_CALLBACK, 0, \"squash\", &opt_squash, NULL,\n+\t  N_(\"create a single commit instead of doing a merge\"),\n+\t  PARSE_OPT_NOARG, parse_opt_pass_strbuf },\n+\t{ OPTION_CALLBACK, 0, \"commit\", &opt_commit, NULL,\n+\t  N_(\"perform a commit if the merge succeeds (default)\"),\n+\t  PARSE_OPT_NOARG, parse_opt_pass_strbuf },\n+\t{ OPTION_CALLBACK, 0, \"edit\", &opt_edit, NULL,\n+\t  N_(\"edit message before committing\"),\n+\t  PARSE_OPT_NOARG, parse_opt_pass_strbuf },\n+\t{ OPTION_CALLBACK, 0, \"ff\", &opt_ff, NULL,\n+\t  N_(\"allow fast-forward\"),\n+\t  PARSE_OPT_NOARG, parse_opt_pass_strbuf },\n+\t{ OPTION_CALLBACK, 0, \"ff-only\", &opt_ff, NULL,\n+\t  N_(\"abort if fast-forward is not possible\"),\n+\t  PARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_pass_strbuf },\n+\t{ OPTION_CALLBACK, 0, \"verify-signatures\", &opt_verify_signatures, NULL,\n+\t  N_(\"verify that the named commit has a valid GPG signature\"),\n+\t  PARSE_OPT_NOARG, parse_opt_pass_strbuf },\n+\t{ OPTION_CALLBACK, 's', \"strategy\", &opt_strategies, N_(\"strategy\"),\n+\t  N_(\"merge strategy to use\"),\n+\t  0, parse_opt_pass_argv_array },\n+\t{ OPTION_CALLBACK, 'X', \"strategy-option\", &opt_strategy_opts,\n+\t  N_(\"option=value\"),\n+\t  N_(\"option for selected merge strategy\"),\n+\t  0, parse_opt_pass_argv_array },\n+\t{ OPTION_CALLBACK, 'S', \"gpg-sign\", &opt_gpg_sign, N_(\"key-id\"),\n+\t  N_(\"GPG sign commit\"),\n+\t  PARSE_OPT_OPTARG, parse_opt_pass_strbuf },\n+\n \tOPT_END()\n };\n \n@@ -100,6 +155,26 @@ static int run_merge(void)\n \tif (opt_progress.len)\n \t\targv_array_push(&args, opt_progress.buf);\n \n+\t/* Options passed to git-merge */\n+\tif (opt_diffstat.len)\n+\t\targv_array_push(&args, opt_diffstat.buf);\n+\tif (opt_log.len)\n+\t\targv_array_push(&args, opt_log.buf);\n+\tif (opt_squash.len)\n+\t\targv_array_push(&args, opt_squash.buf);\n+\tif (opt_commit.len)\n+\t\targv_array_push(&args, opt_commit.buf);\n+\tif (opt_edit.len)\n+\t\targv_array_push(&args, opt_edit.buf);\n+\tif (opt_ff.len)\n+\t\targv_array_push(&args, opt_ff.buf);\n+\tif (opt_verify_signatures.len)\n+\t\targv_array_push(&args, opt_verify_signatures.buf);\n+\targv_array_pushv(&args, opt_strategies.argv);\n+\targv_array_pushv(&args, opt_strategy_opts.argv);\n+\tif (opt_gpg_sign.len)\n+\t\targv_array_push(&args, opt_gpg_sign.buf);\n+\n \targv_array_push(&args, \"FETCH_HEAD\");\n \tret = run_command_v_opt(args.argv, RUN_GIT_CMD);\n \targv_array_clear(&args);\n-- \n2.1.4\n"},{"id":"262828","messageId":"1433314143-4478-9-git-send-email-pyokagan@gmail.com","threadId":"39509","inReplyTo":"1433314143-4478-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 08/19] pull: pass git-fetch's options to git-fetch","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-03T06:48:52Z","receivedAt":"2015-06-03T06:48:52Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Since eb2a8d9 (pull: handle git-fetch's options as well, 2015-06-02),\ngit-pull knows about and handles git-fetch's options, passing them to\ngit-fetch. Re-implement this behavior.\n\nSince 29609e6 (pull: do nothing on --dry-run, 2010-05-25) git-pull\nsupported the --dry-run option, exiting after git-fetch if --dry-run is\nset. Re-implement this behavior.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n\nNotes:\n    v2\n    \n    * Use parse_opt_parse_strbuf()\n\n builtin/pull.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 95 insertions(+)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 5f08634..0b66b43 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -32,6 +32,21 @@ static struct argv_array opt_strategies = ARGV_ARRAY_INIT;\n static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;\n static struct strbuf opt_gpg_sign = STRBUF_INIT;\n \n+/* Options passed to git-fetch */\n+static struct strbuf opt_all = STRBUF_INIT;\n+static struct strbuf opt_append = STRBUF_INIT;\n+static struct strbuf opt_upload_pack = STRBUF_INIT;\n+static int opt_force;\n+static struct strbuf opt_tags = STRBUF_INIT;\n+static struct strbuf opt_prune = STRBUF_INIT;\n+static struct strbuf opt_recurse_submodules = STRBUF_INIT;\n+static int opt_dry_run;\n+static struct strbuf opt_keep = STRBUF_INIT;\n+static struct strbuf opt_depth = STRBUF_INIT;\n+static struct strbuf opt_unshallow = STRBUF_INIT;\n+static struct strbuf opt_update_shallow = STRBUF_INIT;\n+static struct strbuf opt_refmap = STRBUF_INIT;\n+\n static struct option pull_options[] = {\n \t/* Shared options */\n \tOPT__VERBOSITY(&opt_verbosity),\n@@ -82,6 +97,46 @@ static struct option pull_options[] = {\n \t  N_(\"GPG sign commit\"),\n \t  PARSE_OPT_OPTARG, parse_opt_pass_strbuf },\n \n+\t/* Options passed to git-fetch */\n+\tOPT_GROUP(N_(\"Options related to fetching\")),\n+\t{ OPTION_CALLBACK, 0, \"all\", &opt_all, 0,\n+\t  N_(\"fetch from all remotes\"),\n+\t  PARSE_OPT_NOARG, parse_opt_pass_strbuf },\n+\t{ OPTION_CALLBACK, 'a', \"append\", &opt_append, 0,\n+\t  N_(\"append to .git/FETCH_HEAD instead of overwriting\"),\n+\t  PARSE_OPT_NOARG, parse_opt_pass_strbuf },\n+\t{ OPTION_CALLBACK, 0, \"upload-pack\", &opt_upload_pack, N_(\"path\"),\n+\t  N_(\"path to upload pack on remote end\"),\n+\t  0, parse_opt_pass_strbuf },\n+\tOPT__FORCE(&opt_force, N_(\"force overwrite of local branch\")),\n+\t{ OPTION_CALLBACK, 't', \"tags\", &opt_tags, 0,\n+\t  N_(\"fetch all tags and associated objects\"),\n+\t  PARSE_OPT_NOARG, parse_opt_pass_strbuf },\n+\t{ OPTION_CALLBACK, 'p', \"prune\", &opt_prune, 0,\n+\t  N_(\"prune remote-tracking branches no longer on remote\"),\n+\t  PARSE_OPT_NOARG, parse_opt_pass_strbuf },\n+\t{ OPTION_CALLBACK, 0, \"recurse-submodules\", &opt_recurse_submodules,\n+\t  N_(\"on-demand\"),\n+\t  N_(\"control recursive fetching of submodules\"),\n+\t  PARSE_OPT_OPTARG, parse_opt_pass_strbuf },\n+\tOPT_BOOL(0, \"dry-run\", &opt_dry_run,\n+\t\tN_(\"dry run\")),\n+\t{ OPTION_CALLBACK, 'k', \"keep\", &opt_keep, 0,\n+\t  N_(\"keep downloaded pack\"),\n+\t  PARSE_OPT_NOARG, parse_opt_pass_strbuf },\n+\t{ OPTION_CALLBACK, 0, \"depth\", &opt_depth, N_(\"depth\"),\n+\t  N_(\"deepen history of shallow clone\"),\n+\t  0, parse_opt_pass_strbuf },\n+\t{ OPTION_CALLBACK, 0, \"unshallow\", &opt_unshallow, 0,\n+\t  N_(\"convert to a complete repository\"),\n+\t  PARSE_OPT_NONEG | PARSE_OPT_NOARG, parse_opt_pass_strbuf },\n+\t{ OPTION_CALLBACK, 0, \"update-shallow\", &opt_update_shallow, 0,\n+\t  N_(\"accept refs that update .git/shallow\"),\n+\t  PARSE_OPT_NOARG, parse_opt_pass_strbuf },\n+\t{ OPTION_CALLBACK, 0, \"refmap\", &opt_refmap, N_(\"refmap\"),\n+\t  N_(\"specify fetch refmap\"),\n+\t  PARSE_OPT_NONEG, parse_opt_pass_strbuf },\n+\n \tOPT_END()\n };\n \n@@ -100,6 +155,16 @@ static void argv_push_verbosity(struct argv_array *arr)\n }\n \n /**\n+ * Pushes \"-f\" switches into arr to match the opt_force level.\n+ */\n+static void argv_push_force(struct argv_array *arr)\n+{\n+\tint force = opt_force;\n+\twhile (force-- > 0)\n+\t\targv_array_push(arr, \"-f\");\n+}\n+\n+/**\n  * Parses argv into [<repo> [<refspecs>...]], returning their values in `repo`\n  * as a string and `refspecs` as a null-terminated array of strings. If `repo`\n  * is not provided in argv, it is set to NULL.\n@@ -131,6 +196,33 @@ static int run_fetch(const char *repo, const char **refspecs)\n \tif (opt_progress.len)\n \t\targv_array_push(&args, opt_progress.buf);\n \n+\t/* Options passed to git-fetch */\n+\tif (opt_all.len)\n+\t\targv_array_push(&args, opt_all.buf);\n+\tif (opt_append.len)\n+\t\targv_array_push(&args, opt_append.buf);\n+\tif (opt_upload_pack.len)\n+\t\targv_array_push(&args, opt_upload_pack.buf);\n+\targv_push_force(&args);\n+\tif (opt_tags.len)\n+\t\targv_array_push(&args, opt_tags.buf);\n+\tif (opt_prune.len)\n+\t\targv_array_push(&args, opt_prune.buf);\n+\tif (opt_recurse_submodules.len)\n+\t\targv_array_push(&args, opt_recurse_submodules.buf);\n+\tif (opt_dry_run)\n+\t\targv_array_push(&args, \"--dry-run\");\n+\tif (opt_keep.len)\n+\t\targv_array_push(&args, opt_keep.buf);\n+\tif (opt_depth.len)\n+\t\targv_array_push(&args, opt_depth.buf);\n+\tif (opt_unshallow.len)\n+\t\targv_array_push(&args, opt_unshallow.buf);\n+\tif (opt_update_shallow.len)\n+\t\targv_array_push(&args, opt_update_shallow.buf);\n+\tif (opt_refmap.len)\n+\t\targv_array_push(&args, opt_refmap.buf);\n+\n \tif (repo)\n \t\targv_array_push(&args, repo);\n \twhile (*refspecs)\n@@ -199,5 +291,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \tif (run_fetch(repo, refspecs))\n \t\treturn 1;\n \n+\tif (opt_dry_run)\n+\t\treturn 0;\n+\n \treturn run_merge();\n }\n-- \n2.1.4\n"},{"id":"262832","messageId":"1433314143-4478-10-git-send-email-pyokagan@gmail.com","threadId":"39509","inReplyTo":"1433314143-4478-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 09/19] pull: error on no merge candidates","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-03T06:48:53Z","receivedAt":"2015-06-03T06:48:53Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Commit a8c9bef (pull: improve advice for unconfigured error case,\n2009-10-05) fully established the current advices given by git-pull for\nthe different cases where git-fetch will not have anything marked for\nmerge:\n\n1. We fetched from a specific remote, and a refspec was given, but it\n   ended up not fetching anything. This is usually because the user\n   provided a wildcard refspec which had no matches on the remote end.\n\n2. We fetched from a non-default remote, but didn't specify a branch to\n   merge. We can't use the configured one because it applies to the\n   default remote, and thus the user must specify the branches to merge.\n\n3. We fetched from the branch's or repo's default remote, but:\n\n   a. We are not on a branch, so there will never be a configured branch\n      to merge with.\n\n   b. We are on a branch, but there is no configured branch to merge\n      with.\n\n4. We fetched from the branch's or repo's default remote, but the\n   configured branch to merge didn't get fetched (either it doesn't\n   exist, or wasn't part of the configured fetch refspec)\n\nRe-implement the above behavior by implementing get_merge_heads() to\nparse the heads in FETCH_HEAD for merging, and implementing\ndie_no_merge_candidates(), which will be called when FETCH_HEAD has no\nheads for merging.\n\nHelped-by: Johannes Schindelin <johannes.schindelin@gmx.de>\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n\nNotes:\n    v2\n    \n    * Switched to using fprintf_ln() which will append the trailing newline\n      at the end for us.\n    \n    * The die_no_merge_candidates() code has been reorganized a bit to\n      support the later patch which will tweak the messages to be aware of\n      git-pull --rebase.\n\n builtin/pull.c | 113 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 113 insertions(+)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 0b66b43..42a061d 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -10,6 +10,8 @@\n #include \"parse-options.h\"\n #include \"exec_cmd.h\"\n #include \"run-command.h\"\n+#include \"sha1-array.h\"\n+#include \"remote.h\"\n \n static const char * const pull_usage[] = {\n \tN_(\"git pull [options] [<repository> [<refspec>...]]\"),\n@@ -165,6 +167,111 @@ static void argv_push_force(struct argv_array *arr)\n }\n \n /**\n+ * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge\n+ * into merge_heads.\n+ */\n+static void get_merge_heads(struct sha1_array *merge_heads)\n+{\n+\tconst char *filename = git_path(\"FETCH_HEAD\");\n+\tFILE *fp;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tunsigned char sha1[GIT_SHA1_RAWSZ];\n+\n+\tif (!(fp = fopen(filename, \"r\")))\n+\t\tdie_errno(_(\"could not open '%s' for reading\"), filename);\n+\twhile(strbuf_getline(&sb, fp, '\\n') != EOF) {\n+\t\tif (get_sha1_hex(sb.buf, sha1))\n+\t\t\tcontinue;  /* invalid line: does not start with SHA1 */\n+\t\tif (starts_with(sb.buf + GIT_SHA1_HEXSZ, \"\\tnot-for-merge\"))\n+\t\t\tcontinue;  /* ref is not-for-merge */\n+\t\tsha1_array_append(merge_heads, sha1);\n+\t}\n+\tfclose(fp);\n+\tstrbuf_release(&sb);\n+}\n+\n+/**\n+ * Used by die_no_merge_candidates() as a for_each_remote() callback to\n+ * retrieve the name of the remote if the repository only has one remote.\n+ */\n+static int get_only_remote(struct remote *remote, void *cb_data)\n+{\n+\tconst char **remote_name = cb_data;\n+\n+\tif (*remote_name)\n+\t\treturn -1;\n+\n+\t*remote_name = remote->name;\n+\treturn 0;\n+}\n+\n+/**\n+ * Dies with the appropriate reason for why there are no merge candidates:\n+ *\n+ * 1. We fetched from a specific remote, and a refspec was given, but it ended\n+ *    up not fetching anything. This is usually because the user provided a\n+ *    wildcard refspec which had no matches on the remote end.\n+ *\n+ * 2. We fetched from a non-default remote, but didn't specify a branch to\n+ *    merge. We can't use the configured one because it applies to the default\n+ *    remote, thus the user must specify the branches to merge.\n+ *\n+ * 3. We fetched from the branch's or repo's default remote, but:\n+ *\n+ *    a. We are not on a branch, so there will never be a configured branch to\n+ *       merge with.\n+ *\n+ *    b. We are on a branch, but there is no configured branch to merge with.\n+ *\n+ * 4. We fetched from the branch's or repo's default remote, but the configured\n+ *    branch to merge didn't get fetched. (Either it doesn't exist, or wasn't\n+ *    part of the configured fetch refspec.)\n+ */\n+static void NORETURN die_no_merge_candidates(const char *repo, const char **refspecs)\n+{\n+\tstruct branch *curr_branch = branch_get(\"HEAD\");\n+\tconst char *remote = curr_branch ? curr_branch->remote_name : NULL;\n+\n+\tif (*refspecs) {\n+\t\tfprintf_ln(stderr, _(\"There are no candidates for merging among the refs that you just fetched.\"));\n+\t\tfprintf_ln(stderr, _(\"Generally this means that you provided a wildcard refspec which had no\\n\"\n+\t\t\t\t\t\"matches on the remote end.\"));\n+\t} else if (repo && curr_branch && (!remote || strcmp(repo, remote))) {\n+\t\tfprintf_ln(stderr, _(\"You asked to pull from the remote '%s', but did not specify\\n\"\n+\t\t\t\"a branch. Because this is not the default configured remote\\n\"\n+\t\t\t\"for your current branch, you must specify a branch on the command line.\"),\n+\t\t\trepo);\n+\t} else if (!curr_branch) {\n+\t\tfprintf_ln(stderr, _(\"You are not currently on a branch.\"));\n+\t\tfprintf_ln(stderr, _(\"Please specify which branch you want to merge with.\"));\n+\t\tfprintf_ln(stderr, _(\"See git-pull(1) for details.\"));\n+\t\tfprintf(stderr, \"\\n\");\n+\t\tfprintf_ln(stderr, \"    git pull <remote> <branch>\");\n+\t\tfprintf(stderr, \"\\n\");\n+\t} else if (!curr_branch->merge_nr) {\n+\t\tconst char *remote_name = NULL;\n+\n+\t\tif (for_each_remote(get_only_remote, &remote_name) || !remote_name)\n+\t\t\tremote_name = \"<remote>\";\n+\n+\t\tfprintf_ln(stderr, _(\"There is no tracking information for the current branch.\"));\n+\t\tfprintf_ln(stderr, _(\"Please specify which branch you want to merge with.\"));\n+\t\tfprintf_ln(stderr, _(\"See git-pull(1) for details.\"));\n+\t\tfprintf(stderr, \"\\n\");\n+\t\tfprintf_ln(stderr, \"    git pull <remote> <branch>\");\n+\t\tfprintf(stderr, \"\\n\");\n+\t\tfprintf_ln(stderr, _(\"If you wish to set tracking information for this branch you can do so with:\\n\"\n+\t\t\t\t\"\\n\"\n+\t\t\t\t\"    git branch --set-upstream-to=%s/<branch> %s\\n\"),\n+\t\t\t\tremote_name, curr_branch->name);\n+\t} else\n+\t\tfprintf_ln(stderr, _(\"Your configuration specifies to merge with the ref '%s'\\n\"\n+\t\t\t\"from the remote, but no such ref was fetched.\"),\n+\t\t\t*curr_branch->merge_name);\n+\texit(1);\n+}\n+\n+/**\n  * Parses argv into [<repo> [<refspecs>...]], returning their values in `repo`\n  * as a string and `refspecs` as a null-terminated array of strings. If `repo`\n  * is not provided in argv, it is set to NULL.\n@@ -276,6 +383,7 @@ static int run_merge(void)\n int cmd_pull(int argc, const char **argv, const char *prefix)\n {\n \tconst char *repo, **refspecs;\n+\tstruct sha1_array merge_heads = SHA1_ARRAY_INIT;\n \n \tif (!getenv(\"_GIT_USE_BUILTIN_PULL\")) {\n \t\tconst char *path = mkpath(\"%s/git-pull\", git_exec_path());\n@@ -294,5 +402,10 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \tif (opt_dry_run)\n \t\treturn 0;\n \n+\tget_merge_heads(&merge_heads);\n+\n+\tif (!merge_heads.nr)\n+\t\tdie_no_merge_candidates(repo, refspecs);\n+\n \treturn run_merge();\n }\n-- \n2.1.4\n"},{"id":"262827","messageId":"1433314143-4478-11-git-send-email-pyokagan@gmail.com","threadId":"39509","inReplyTo":"1433314143-4478-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 10/19] pull: support pull.ff config","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-03T06:48:54Z","receivedAt":"2015-06-03T06:48:54Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Since b814da8 (pull: add pull.ff configuration, 2014-01-15), git-pull.sh\nwould lookup the configuration value of \"pull.ff\", and set the flag\n\"--ff\" if its value is \"true\", \"--no-ff\" if its value is \"false\" and\n\"--ff-only\" if its value is \"only\".\n\nRe-implement this behavior.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n\nNotes:\n    v2\n    \n    * Yup, I did mean strcmp(value, \"only\")\n\n builtin/pull.c | 29 +++++++++++++++++++++++++++++\n 1 file changed, 29 insertions(+)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 42a061d..1c1883d 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -167,6 +167,32 @@ static void argv_push_force(struct argv_array *arr)\n }\n \n /**\n+ * If pull.ff is \"true\", sets sb to \"--ff\". If pull.ff is \"false\", sets sb to\n+ * \"--no-ff\". If pull.ff is \"only\", sets sb to \"--ff-only\". If pull.ff is\n+ * set to an invalid value, die with an error.\n+ */\n+static void config_get_ff(struct strbuf *sb)\n+{\n+\tconst char *value;\n+\n+\tif (git_config_get_value(\"pull.ff\", &value))\n+\t\treturn;\n+\tswitch (git_config_maybe_bool(\"pull.ff\", value)) {\n+\t\tcase 0:\n+\t\t\tstrbuf_addstr(sb, \"--no-ff\");\n+\t\t\treturn;\n+\t\tcase 1:\n+\t\t\tstrbuf_addstr(sb, \"--ff\");\n+\t\t\treturn;\n+\t}\n+\tif (!strcmp(value, \"only\")) {\n+\t\tstrbuf_addstr(sb, \"--ff-only\");\n+\t\treturn;\n+\t}\n+\tdie(_(\"Invalid value for pull.ff: %s\"), value);\n+}\n+\n+/**\n  * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge\n  * into merge_heads.\n  */\n@@ -396,6 +422,9 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \n \tparse_repo_refspecs(argc, argv, &repo, &refspecs);\n \n+\tif (!opt_ff.len)\n+\t\tconfig_get_ff(&opt_ff);\n+\n \tif (run_fetch(repo, refspecs))\n \t\treturn 1;\n \n-- \n2.1.4\n"},{"id":"262831","messageId":"1433314143-4478-12-git-send-email-pyokagan@gmail.com","threadId":"39509","inReplyTo":"1433314143-4478-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 11/19] pull: check if in unresolved merge state","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-03T06:48:55Z","receivedAt":"2015-06-03T06:48:55Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Since d38a30d (Be more user-friendly when refusing to do something\nbecause of conflict., 2010-01-12), git-pull will error out with\nuser-friendly advices if the user is in the middle of a merge or has\nunmerged files.\n\nRe-implement this behavior. While the \"has unmerged files\" case can be\nhandled by die_resolve_conflict(), we introduce a new function\ndie_conclude_merge() for printing a different error message for when\nthere are no unmerged files but the merge has not been finished.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n advice.c       | 8 ++++++++\n advice.h       | 1 +\n builtin/pull.c | 9 +++++++++\n 3 files changed, 18 insertions(+)\n\ndiff --git a/advice.c b/advice.c\nindex 575bec2..4965686 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -96,6 +96,14 @@ void NORETURN die_resolve_conflict(const char *me)\n \tdie(\"Exiting because of an unresolved conflict.\");\n }\n \n+void NORETURN die_conclude_merge(void)\n+{\n+\terror(_(\"You have not concluded your merge (MERGE_HEAD exists).\"));\n+\tif (advice_resolve_conflict)\n+\t\tadvise(_(\"Please, commit your changes before you can merge.\"));\n+\tdie(_(\"Exiting because of unfinished merge.\"));\n+}\n+\n void detach_advice(const char *new_name)\n {\n \tconst char fmt[] =\ndiff --git a/advice.h b/advice.h\nindex 5ecc6c1..b341a55 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -24,6 +24,7 @@ __attribute__((format (printf, 1, 2)))\n void advise(const char *advice, ...);\n int error_resolve_conflict(const char *me);\n extern void NORETURN die_resolve_conflict(const char *me);\n+void NORETURN die_conclude_merge(void);\n void detach_advice(const char *new_name);\n \n #endif /* ADVICE_H */\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 1c1883d..703fc1d 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -12,6 +12,7 @@\n #include \"run-command.h\"\n #include \"sha1-array.h\"\n #include \"remote.h\"\n+#include \"dir.h\"\n \n static const char * const pull_usage[] = {\n \tN_(\"git pull [options] [<repository> [<refspec>...]]\"),\n@@ -422,6 +423,14 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \n \tparse_repo_refspecs(argc, argv, &repo, &refspecs);\n \n+\tgit_config(git_default_config, NULL);\n+\n+\tif (read_cache_unmerged())\n+\t\tdie_resolve_conflict(\"Pull\");\n+\n+\tif (file_exists(git_path(\"MERGE_HEAD\")))\n+\t\tdie_conclude_merge();\n+\n \tif (!opt_ff.len)\n \t\tconfig_get_ff(&opt_ff);\n \n-- \n2.1.4\n"},{"id":"262829","messageId":"1433314143-4478-13-git-send-email-pyokagan@gmail.com","threadId":"39509","inReplyTo":"1433314143-4478-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 12/19] pull: fast-forward working tree if head is updated","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-03T06:48:56Z","receivedAt":"2015-06-03T06:48:56Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Since b10ac50 (Fix pulling into the same branch., 2005-08-25), git-pull,\nupon detecting that git-fetch updated the current head, will\nfast-forward the working tree to the updated head commit.\n\nRe-implement this behavior.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n builtin/pull.c | 30 ++++++++++++++++++++++++++++++\n 1 file changed, 30 insertions(+)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 703fc1d..8db0db2 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -411,6 +411,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n {\n \tconst char *repo, **refspecs;\n \tstruct sha1_array merge_heads = SHA1_ARRAY_INIT;\n+\tunsigned char orig_head[GIT_SHA1_RAWSZ], curr_head[GIT_SHA1_RAWSZ];\n \n \tif (!getenv(\"_GIT_USE_BUILTIN_PULL\")) {\n \t\tconst char *path = mkpath(\"%s/git-pull\", git_exec_path());\n@@ -434,12 +435,41 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \tif (!opt_ff.len)\n \t\tconfig_get_ff(&opt_ff);\n \n+\tif (get_sha1(\"HEAD\", orig_head))\n+\t\thashclr(orig_head);\n+\n \tif (run_fetch(repo, refspecs))\n \t\treturn 1;\n \n \tif (opt_dry_run)\n \t\treturn 0;\n \n+\tif (get_sha1(\"HEAD\", curr_head))\n+\t\thashclr(curr_head);\n+\n+\tif (!is_null_sha1(orig_head) && !is_null_sha1(curr_head) &&\n+\t\t\thashcmp(orig_head, curr_head)) {\n+\t\t/*\n+\t\t * The fetch involved updating the current branch.\n+\t\t *\n+\t\t * The working tree and the index file are still based on\n+\t\t * orig_head commit, but we are merging into curr_head.\n+\t\t * Update the working tree to match curr_head.\n+\t\t */\n+\n+\t\twarning(_(\"fetch updated the current branch head.\\n\"\n+\t\t\t\"fast-forwarding your working tree from\\n\"\n+\t\t\t\"commit %s.\"), sha1_to_hex(orig_head));\n+\n+\t\tif (checkout_fast_forward(orig_head, curr_head, 0))\n+\t\t\tdie(_(\"Cannot fast-forward your working tree.\\n\"\n+\t\t\t\t\"After making sure that you saved anything precious from\\n\"\n+\t\t\t\t\"$ git diff %s\\n\"\n+\t\t\t\t\"output, run\\n\"\n+\t\t\t\t\"$ git reset --hard\\n\"\n+\t\t\t\t\"to recover.\"), sha1_to_hex(orig_head));\n+\t}\n+\n \tget_merge_heads(&merge_heads);\n \n \tif (!merge_heads.nr)\n-- \n2.1.4\n"},{"id":"262830","messageId":"1433314143-4478-14-git-send-email-pyokagan@gmail.com","threadId":"39509","inReplyTo":"1433314143-4478-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 13/19] pull: implement pulling into an unborn branch","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-03T06:48:57Z","receivedAt":"2015-06-03T06:48:57Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"b4dc085 (pull: merge into unborn by fast-forwarding from empty\ntree, 2013-06-20) established git-pull's current behavior of pulling\ninto an unborn branch by fast-forwarding the work tree from an empty\ntree to the merge head, then setting HEAD to the merge head.\n\nRe-implement this behavior by introducing pull_into_void() which will be\ncalled instead of run_merge() if HEAD is invalid.\n\nHelped-by: Stephen Robin <stephen.robin@gmail.com>\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n builtin/pull.c | 29 ++++++++++++++++++++++++++++-\n 1 file changed, 28 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 8db0db2..f0d4710 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -13,6 +13,7 @@\n #include \"sha1-array.h\"\n #include \"remote.h\"\n #include \"dir.h\"\n+#include \"refs.h\"\n \n static const char * const pull_usage[] = {\n \tN_(\"git pull [options] [<repository> [<refspec>...]]\"),\n@@ -367,6 +368,27 @@ static int run_fetch(const char *repo, const char **refspecs)\n }\n \n /**\n+ * \"Pulls into void\" by branching off merge_head.\n+ */\n+static int pull_into_void(unsigned char merge_head[GIT_SHA1_RAWSZ],\n+\t\tunsigned char curr_head[GIT_SHA1_RAWSZ])\n+{\n+\t/*\n+\t * Two-way merge: we claim the index is based on an empty tree,\n+\t * and try to fast-forward to HEAD. This ensures we will not lose\n+\t * index/worktree changes that the user already made on the unborn\n+\t * branch.\n+\t */\n+\tif (checkout_fast_forward(EMPTY_TREE_SHA1_BIN, merge_head, 0))\n+\t\treturn 1;\n+\n+\tif (update_ref(\"initial pull\", \"HEAD\", merge_head, curr_head, 0, UPDATE_REFS_DIE_ON_ERR))\n+\t\treturn 1;\n+\n+\treturn 0;\n+}\n+\n+/**\n  * Runs git-merge, returning its exit status.\n  */\n static int run_merge(void)\n@@ -475,5 +497,10 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \tif (!merge_heads.nr)\n \t\tdie_no_merge_candidates(repo, refspecs);\n \n-\treturn run_merge();\n+\tif (is_null_sha1(orig_head)) {\n+\t\tif (merge_heads.nr > 1)\n+\t\t\tdie(_(\"Cannot merge multiple branches into empty head.\"));\n+\t\treturn pull_into_void(*merge_heads.sha1, curr_head);\n+\t} else\n+\t\treturn run_merge();\n }\n-- \n2.1.4\n"},{"id":"262833","messageId":"1433314143-4478-15-git-send-email-pyokagan@gmail.com","threadId":"39509","inReplyTo":"1433314143-4478-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 14/19] pull: set reflog message","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-03T06:48:58Z","receivedAt":"2015-06-03T06:48:58Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"f947413 (Use GIT_REFLOG_ACTION environment variable instead.,\n2006-12-28) established git-pull's method for setting the reflog\nmessage, which is to set the environment variable GIT_REFLOG_ACTION to\nthe evaluation of \"pull${1+ $*}\" if it has not already been set.\n\nRe-implement this behavior.\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n\nNotes:\n    v2\n    \n    * Don't use strbuf_rtrim().\n\n builtin/pull.c | 22 ++++++++++++++++++++++\n 1 file changed, 22 insertions(+)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex f0d4710..c44cc90 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -169,6 +169,25 @@ static void argv_push_force(struct argv_array *arr)\n }\n \n /**\n+ * Sets the GIT_REFLOG_ACTION environment variable to the concatenation of argv\n+ */\n+static void set_reflog_message(int argc, const char **argv)\n+{\n+\tint i;\n+\tstruct strbuf msg = STRBUF_INIT;\n+\n+\tfor (i = 0; i < argc; i++) {\n+\t\tif (i)\n+\t\t\tstrbuf_addch(&msg, ' ');\n+\t\tstrbuf_addstr(&msg, argv[i]);\n+\t}\n+\n+\tsetenv(\"GIT_REFLOG_ACTION\", msg.buf, 0);\n+\n+\tstrbuf_release(&msg);\n+}\n+\n+/*\n  * If pull.ff is \"true\", sets sb to \"--ff\". If pull.ff is \"false\", sets sb to\n  * \"--no-ff\". If pull.ff is \"only\", sets sb to \"--ff-only\". If pull.ff is\n  * set to an invalid value, die with an error.\n@@ -442,6 +461,9 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \t\t\tdie_errno(\"could not exec %s\", path);\n \t}\n \n+\tif (!getenv(\"GIT_REFLOG_ACTION\"))\n+\t\tset_reflog_message(argc, argv);\n+\n \targc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0);\n \n \tparse_repo_refspecs(argc, argv, &repo, &refspecs);\n-- \n2.1.4\n"},{"id":"262837","messageId":"1433314143-4478-16-git-send-email-pyokagan@gmail.com","threadId":"39509","inReplyTo":"1433314143-4478-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 15/19] pull: teach git pull about --rebase","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-03T06:48:59Z","receivedAt":"2015-06-03T06:48:59Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Since cd67e4d (Teach 'git pull' about --rebase, 2007-11-28), if the\n--rebase option is set, git-rebase is run instead of git-merge.\n\nRe-implement this by introducing run_rebase(), which is called instead\nof run_merge() if opt_rebase is a true value.\n\nSince c85c792 (pull --rebase: be cleverer with rebased upstream\nbranches, 2008-01-26), git-pull handles the case where the upstream\nbranch was rebased since it was last fetched. The fork point (old remote\nref) of the branch from the upstream branch is calculated before fetch,\nand then rebased from onto the new remote head (merge_head) after fetch.\n\nRe-implement this by introducing get_merge_branch_2() and\nget_merge_branch_1() to find the upstream branch for the\nspecified/current branch, and get_rebase_fork_point() which will find\nthe fork point between the upstream branch and current branch.\n\nHowever, the above change created a problem where git-rebase cannot\ndetect commits that are already upstream, and thus may result in\nunnecessary conflicts. cf65426 (pull --rebase: Avoid spurious conflicts\nand reapplying unnecessary patches, 2010-08-12) fixes this by ignoring\nthe above old remote ref if it is contained within the merge base of the\nmerge head and the current branch.\n\nThis is re-implemented in run_rebase() where fork_point is not used if\nit is the merge base returned by get_octopus_merge_base().\n\nHelped-by: Stefan Beller <sbeller@google.com>\nHelped-by: Johannes Schindelin <johannes.schindelin@gmx.de>\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n\nNotes:\n    v2\n    \n    * enum rebase_type gained a new value REBASE_INVALID = -1 to represent\n      invalid values passed to --rebase. We can't immediately die() on\n      invalid values because we need parse_options() to show the git-pull\n      usage as well.\n    \n    * parse_config_rebase() now returns an enum rebase_type to make it clear\n      what kind of value it returns.\n    \n    * parse_config_rebase() now does not depend on git_config_maybe_bool()\n      returning 1 for true and 0 for false. If it returns 0, it is\n      REBASE_FALSE. If it returns any true value >=0, it is interpreted as\n      REBASE_TRUE.\n    \n    * get_merge_branch_1() has been renamed to get_upstream_branch() to be\n      consistent with the similar get_upstream_branch() in sha1_name.c.\n    \n    * get_merge_branch_2() has been renamed to get_tracking_branch() to\n      indicate what it is doing: deriving the remote tracking branch from\n      the refspec.\n    \n    * Various small variable re-names and docstring tweaks. Hopefully\n      everything reads better now.\n\n builtin/pull.c | 239 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 237 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex c44cc90..7645937 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -15,6 +15,45 @@\n #include \"dir.h\"\n #include \"refs.h\"\n \n+enum rebase_type {\n+\tREBASE_INVALID = -1,\n+\tREBASE_FALSE = 0,\n+\tREBASE_TRUE,\n+\tREBASE_PRESERVE\n+};\n+\n+/**\n+ * Parses the value of --rebase, branch.*.rebase or pull.rebase. If value is a\n+ * false value, returns REBASE_FALSE. If value is a true value, returns\n+ * REBASE_TRUE. If value is \"preserve\", returns REBASE_PRESERVE. Otherwise,\n+ * returns -1 to signify an invalid value.\n+ */\n+static enum rebase_type parse_config_rebase(const char *value)\n+{\n+\tint v = git_config_maybe_bool(\"pull.rebase\", value);\n+\tif (!v)\n+\t\treturn REBASE_FALSE;\n+\telse if (v >= 0)\n+\t\treturn REBASE_TRUE;\n+\telse if (!strcmp(value, \"preserve\"))\n+\t\treturn REBASE_PRESERVE;\n+\treturn REBASE_INVALID;\n+}\n+\n+/**\n+ * Callback for --rebase, which parses arg with parse_config_rebase().\n+ */\n+static int parse_opt_rebase(const struct option *opt, const char *arg, int unset)\n+{\n+\tenum rebase_type *value = opt->value;\n+\n+\tif (arg)\n+\t\t*value = parse_config_rebase(arg);\n+\telse\n+\t\t*value = unset ? REBASE_FALSE : REBASE_TRUE;\n+\treturn *value == REBASE_INVALID ? -1 : 0;\n+}\n+\n static const char * const pull_usage[] = {\n \tN_(\"git pull [options] [<repository> [<refspec>...]]\"),\n \tNULL\n@@ -24,7 +63,8 @@ static const char * const pull_usage[] = {\n static int opt_verbosity;\n static struct strbuf opt_progress = STRBUF_INIT;\n \n-/* Options passed to git-merge */\n+/* Options passed to git-merge or git-rebase */\n+static enum rebase_type opt_rebase;\n static struct strbuf opt_diffstat = STRBUF_INIT;\n static struct strbuf opt_log = STRBUF_INIT;\n static struct strbuf opt_squash = STRBUF_INIT;\n@@ -58,8 +98,12 @@ static struct option pull_options[] = {\n \t  N_(\"force progress reporting\"),\n \t  PARSE_OPT_NOARG, parse_opt_pass_strbuf},\n \n-\t/* Options passed to git-merge */\n+\t/* Options passed to git-merge or git-rebase */\n \tOPT_GROUP(N_(\"Options related to merging\")),\n+\t{ OPTION_CALLBACK, 'r', \"rebase\", &opt_rebase,\n+\t  N_(\"false|true|preserve\"),\n+\t  N_(\"incorporate changes by rebasing rather than merging\"),\n+\t  PARSE_OPT_OPTARG, parse_opt_rebase },\n \t{ OPTION_CALLBACK, 'n', NULL, &opt_diffstat, NULL,\n \t  N_(\"do not show a diffstat at the end of the merge\"),\n \t  PARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_pass_strbuf },\n@@ -448,11 +492,194 @@ static int run_merge(void)\n \treturn ret;\n }\n \n+/**\n+ * Returns remote's upstream branch for the current branch. If remote is NULL,\n+ * the current branch's configured default remote is used. Returns NULL if\n+ * `remote` does not name a valid remote, HEAD does not point to a branch,\n+ * remote is not the branch's configured remote or the branch does not have any\n+ * configured upstream branch.\n+ */\n+static char *get_upstream_branch(const char *remote)\n+{\n+\tstruct remote *rm;\n+\tstruct branch *curr_branch;\n+\n+\trm = remote_get(remote);\n+\tif (!rm)\n+\t\treturn NULL;\n+\n+\tcurr_branch = branch_get(\"HEAD\");\n+\tif (!curr_branch)\n+\t\treturn NULL;\n+\n+\tif (curr_branch->remote != rm)\n+\t\treturn NULL;\n+\n+\tif (!curr_branch->merge_nr)\n+\t\treturn NULL;\n+\n+\treturn xstrdup(curr_branch->merge[0]->dst);\n+}\n+\n+/**\n+ * Derives the remote tracking branch from the remote and refspec.\n+ *\n+ * FIXME: The current implementation assumes the default mapping of\n+ * refs/heads/<branch_name> to refs/remotes/<remote_name>/<branch_name>.\n+ */\n+static char *get_tracking_branch(const char *remote, const char *refspec)\n+{\n+\tstruct refspec *spec;\n+\tconst char *spec_src;\n+\tchar *merge_branch;\n+\n+\tspec = parse_fetch_refspec(1, &refspec);\n+\tspec_src = spec->src;\n+\tif (!*spec_src || !strcmp(spec_src, \"HEAD\"))\n+\t\tspec_src = \"HEAD\";\n+\telse if (skip_prefix(spec_src, \"heads/\", &spec_src))\n+\t\t;\n+\telse if (skip_prefix(spec_src, \"refs/heads/\", &spec_src))\n+\t\t;\n+\telse if (starts_with(spec_src, \"refs/\") ||\n+\t\tstarts_with(spec_src, \"tags/\") ||\n+\t\tstarts_with(spec_src, \"remotes/\"))\n+\t\tspec_src = \"\";\n+\n+\tif (*spec_src) {\n+\t\tif (!strcmp(remote, \".\"))\n+\t\t\tmerge_branch = mkpathdup(\"refs/heads/%s\", spec_src);\n+\t\telse\n+\t\t\tmerge_branch = mkpathdup(\"refs/remotes/%s/%s\", remote, spec_src);\n+\t} else\n+\t\tmerge_branch = NULL;\n+\n+\tfree_refspec(1, spec);\n+\treturn merge_branch;\n+}\n+\n+/**\n+ * Given the repo and refspecs, sets fork_point to the point at which the\n+ * current branch forked from its remote tracking branch. Returns 0 on success,\n+ * -1 on failure.\n+ */\n+static int get_rebase_fork_point(unsigned char fork_point[GIT_SHA1_RAWSZ],\n+\t\tconst char *repo, const char *refspec)\n+{\n+\tint ret;\n+\tstruct branch *curr_branch;\n+\tchar *remote_branch;\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\n+\tcurr_branch = branch_get(\"HEAD\");\n+\tif (!curr_branch)\n+\t\treturn -1;\n+\n+\tif (refspec)\n+\t\tremote_branch = get_tracking_branch(repo, refspec);\n+\telse\n+\t\tremote_branch = get_upstream_branch(repo);\n+\n+\tif (!remote_branch)\n+\t\treturn -1;\n+\n+\targv_array_pushl(&cp.args, \"merge-base\", \"--fork-point\",\n+\t\t\tremote_branch, curr_branch->name, NULL);\n+\tcp.no_stdin = 1;\n+\tcp.no_stderr = 1;\n+\tcp.git_cmd = 1;\n+\n+\tret = capture_command(&cp, &sb, GIT_SHA1_HEXSZ);\n+\tif (ret)\n+\t\tgoto cleanup;\n+\n+\tret = get_sha1_hex(sb.buf, fork_point);\n+\tif (ret)\n+\t\tgoto cleanup;\n+\n+cleanup:\n+\tfree(remote_branch);\n+\tstrbuf_release(&sb);\n+\treturn ret ? -1 : 0;\n+}\n+\n+/**\n+ * Sets merge_base to the octopus merge base of curr_head, merge_head and\n+ * fork_point. Returns 0 if a merge base is found, 1 otherwise.\n+ */\n+static int get_octopus_merge_base(unsigned char merge_base[GIT_SHA1_HEXSZ],\n+\t\tunsigned char curr_head[GIT_SHA1_RAWSZ],\n+\t\tunsigned char merge_head[GIT_SHA1_RAWSZ],\n+\t\tunsigned char fork_point[GIT_SHA1_RAWSZ])\n+{\n+\tstruct commit_list *revs = NULL, *result;\n+\n+\tcommit_list_insert(lookup_commit_reference(curr_head), &revs);\n+\tcommit_list_insert(lookup_commit_reference(merge_head), &revs);\n+\tif (!is_null_sha1(fork_point))\n+\t\tcommit_list_insert(lookup_commit_reference(fork_point), &revs);\n+\n+\tresult = reduce_heads(get_octopus_merge_bases(revs));\n+\tfree_commit_list(revs);\n+\tif (!result)\n+\t\treturn 1;\n+\n+\thashcpy(merge_base, result->item->object.sha1);\n+\treturn 0;\n+}\n+\n+/**\n+ * Given the current HEAD SHA1, the merge head returned from git-fetch and the\n+ * fork point calculated by get_rebase_fork_point(), runs git-rebase with the\n+ * appropriate arguments and returns its exit status.\n+ */\n+static int run_rebase(unsigned char curr_head[GIT_SHA1_RAWSZ],\n+\t\tunsigned char merge_head[GIT_SHA1_RAWSZ],\n+\t\tunsigned char fork_point[GIT_SHA1_RAWSZ])\n+{\n+\tint ret;\n+\tunsigned char oct_merge_base[GIT_SHA1_RAWSZ];\n+\tstruct argv_array args = ARGV_ARRAY_INIT;\n+\n+\tif (!get_octopus_merge_base(oct_merge_base, curr_head, merge_head, fork_point))\n+\t\tif (!is_null_sha1(fork_point) && !hashcmp(oct_merge_base, fork_point))\n+\t\t\thashclr(fork_point);\n+\n+\targv_array_push(&args, \"rebase\");\n+\n+\t/* Shared options */\n+\targv_push_verbosity(&args);\n+\n+\t/* Options passed to git-rebase */\n+\tif (opt_rebase == REBASE_PRESERVE)\n+\t\targv_array_push(&args, \"--preserve-merges\");\n+\tif (opt_diffstat.len)\n+\t\targv_array_push(&args, opt_diffstat.buf);\n+\targv_array_pushv(&args, opt_strategies.argv);\n+\targv_array_pushv(&args, opt_strategy_opts.argv);\n+\tif (opt_gpg_sign.len)\n+\t\targv_array_push(&args, opt_gpg_sign.buf);\n+\n+\targv_array_push(&args, \"--onto\");\n+\targv_array_push(&args, sha1_to_hex(merge_head));\n+\n+\tif (!is_null_sha1(fork_point))\n+\t\targv_array_push(&args, sha1_to_hex(fork_point));\n+\telse\n+\t\targv_array_push(&args, sha1_to_hex(merge_head));\n+\n+\tret = run_command_v_opt(args.argv, RUN_GIT_CMD);\n+\targv_array_clear(&args);\n+\treturn ret;\n+}\n+\n int cmd_pull(int argc, const char **argv, const char *prefix)\n {\n \tconst char *repo, **refspecs;\n \tstruct sha1_array merge_heads = SHA1_ARRAY_INIT;\n \tunsigned char orig_head[GIT_SHA1_RAWSZ], curr_head[GIT_SHA1_RAWSZ];\n+\tunsigned char rebase_fork_point[GIT_SHA1_RAWSZ];\n \n \tif (!getenv(\"_GIT_USE_BUILTIN_PULL\")) {\n \t\tconst char *path = mkpath(\"%s/git-pull\", git_exec_path());\n@@ -482,6 +709,10 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \tif (get_sha1(\"HEAD\", orig_head))\n \t\thashclr(orig_head);\n \n+\tif (opt_rebase)\n+\t\tif (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))\n+\t\t\thashclr(rebase_fork_point);\n+\n \tif (run_fetch(repo, refspecs))\n \t\treturn 1;\n \n@@ -523,6 +754,10 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \t\tif (merge_heads.nr > 1)\n \t\t\tdie(_(\"Cannot merge multiple branches into empty head.\"));\n \t\treturn pull_into_void(*merge_heads.sha1, curr_head);\n+\t} else if (opt_rebase) {\n+\t\tif (merge_heads.nr > 1)\n+\t\t\tdie(_(\"Cannot rebase onto multiple branches.\"));\n+\t\treturn run_rebase(curr_head, *merge_heads.sha1, rebase_fork_point);\n \t} else\n \t\treturn run_merge();\n }\n-- \n2.1.4\n"},{"id":"262834","messageId":"1433314143-4478-17-git-send-email-pyokagan@gmail.com","threadId":"39509","inReplyTo":"1433314143-4478-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 16/19] pull: configure --rebase via branch.<name>.rebase or pull.rebase","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-03T06:49:00Z","receivedAt":"2015-06-03T06:49:00Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Since cd67e4d (Teach 'git pull' about --rebase, 2007-11-28),\nfetch+rebase could be set by default by defining the config variable\nbranch.<name>.rebase. This setting can be overriden on the command line\nby --rebase and --no-rebase.\n\nSince 6b37dff (pull: introduce a pull.rebase option to enable --rebase,\n2011-11-06), git-pull --rebase can also be configured via the\npull.rebase configuration option.\n\nRe-implement support for these two configuration settings by introducing\nconfig_get_rebase() which is called before parse_options() to set the\ndefault value of opt_rebase.\n\nHelped-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n\nNotes:\n    v2\n    \n    * Previously, config_get_rebase() attempted to do too many things. It:\n    \n      1. Checked if there was a configuration for\n         branch.$curr_branch.rebase, and if not, then pull.rebase\n    \n      2. Parsed the configuration value and died if the value is invalid.\n    \n      These 2 functions have been split into config_get_rebase_default() and\n      config_get_rebase() respectively.\n\n builtin/pull.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 48 insertions(+)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 7645937..9759720 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -258,6 +258,52 @@ static void config_get_ff(struct strbuf *sb)\n }\n \n /**\n+ * Sets `value` to the REBASE_* value for the configuration setting `key`.\n+ * Returns 0 is `key` exists, -1 if it does not. Dies if the value of `key` is\n+ * invalid.\n+ */\n+static int config_get_rebase(const char *key, enum rebase_type *value)\n+{\n+\tconst char *str_value;\n+\n+\tif (git_config_get_value(key, &str_value))\n+\t\treturn -1;\n+\t*value = parse_config_rebase(str_value);\n+\tif (*value == REBASE_INVALID)\n+\t\tdie(_(\"Invalid value for %s: %s\"), key, str_value);\n+\treturn 0;\n+}\n+\n+/**\n+ * Returns the default configured value for --rebase. It first looks for the\n+ * value of \"branch.$curr_branch.rebase\", where $curr_branch is the current\n+ * branch, and if HEAD is detached or the configuration key does not exist,\n+ * looks for the value of \"pull.rebase\". If both configuration keys do not\n+ * exist, returns REBASE_FALSE.\n+ */\n+static enum rebase_type config_get_rebase_default(void)\n+{\n+\tstruct branch *curr_branch = branch_get(\"HEAD\");\n+\tenum rebase_type rebase_type;\n+\n+\tif (curr_branch) {\n+\t\tint ret;\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\n+\t\tstrbuf_addf(&sb, \"branch.%s.rebase\", curr_branch->name);\n+\t\tret = config_get_rebase(sb.buf, &rebase_type);\n+\t\tstrbuf_release(&sb);\n+\t\tif (!ret)\n+\t\t\treturn rebase_type;\n+\t}\n+\n+\tif (!config_get_rebase(\"pull.rebase\", &rebase_type))\n+\t\treturn rebase_type;\n+\n+\treturn REBASE_FALSE;\n+}\n+\n+/**\n  * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge\n  * into merge_heads.\n  */\n@@ -691,6 +737,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \tif (!getenv(\"GIT_REFLOG_ACTION\"))\n \t\tset_reflog_message(argc, argv);\n \n+\topt_rebase = config_get_rebase_default();\n+\n \targc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0);\n \n \tparse_repo_refspecs(argc, argv, &repo, &refspecs);\n-- \n2.1.4\n"},{"id":"262835","messageId":"1433314143-4478-18-git-send-email-pyokagan@gmail.com","threadId":"39509","inReplyTo":"1433314143-4478-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 17/19] pull --rebase: exit early when the working directory is dirty","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-03T06:49:01Z","receivedAt":"2015-06-03T06:49:01Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Re-implement the behavior introduced by f9189cf (pull --rebase: exit\nearly when the working directory is dirty, 2008-05-21).\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n builtin/pull.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 76 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 9759720..f5d437a 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -14,6 +14,8 @@\n #include \"remote.h\"\n #include \"dir.h\"\n #include \"refs.h\"\n+#include \"revision.h\"\n+#include \"lockfile.h\"\n \n enum rebase_type {\n \tREBASE_INVALID = -1,\n@@ -304,6 +306,73 @@ static enum rebase_type config_get_rebase_default(void)\n }\n \n /**\n+ * Returns 1 if there are unstaged changes, 0 otherwise.\n+ */\n+static int has_unstaged_changes(const char *prefix)\n+{\n+\tstruct rev_info rev_info;\n+\tint result;\n+\n+\tinit_revisions(&rev_info, prefix);\n+\tDIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);\n+\tDIFF_OPT_SET(&rev_info.diffopt, QUICK);\n+\tdiff_setup_done(&rev_info.diffopt);\n+\tresult = run_diff_files(&rev_info, 0);\n+\treturn diff_result_code(&rev_info.diffopt, result);\n+}\n+\n+/**\n+ * Returns 1 if there are uncommitted changes, 0 otherwise.\n+ */\n+static int has_uncommitted_changes(const char *prefix)\n+{\n+\tstruct rev_info rev_info;\n+\tint result;\n+\n+\tif (is_cache_unborn())\n+\t\treturn 0;\n+\n+\tinit_revisions(&rev_info, prefix);\n+\tDIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);\n+\tDIFF_OPT_SET(&rev_info.diffopt, QUICK);\n+\tadd_head_to_pending(&rev_info);\n+\tdiff_setup_done(&rev_info.diffopt);\n+\tresult = run_diff_index(&rev_info, 1);\n+\treturn diff_result_code(&rev_info.diffopt, result);\n+}\n+\n+/**\n+ * If the work tree has unstaged or uncommitted changes, dies with the\n+ * appropriate message.\n+ */\n+static void die_on_unclean_work_tree(const char *prefix)\n+{\n+\tstruct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));\n+\tint do_die = 0;\n+\n+\thold_locked_index(lock_file, 0);\n+\trefresh_cache(REFRESH_QUIET);\n+\tupdate_index_if_able(&the_index, lock_file);\n+\trollback_lock_file(lock_file);\n+\n+\tif (has_unstaged_changes(prefix)) {\n+\t\terror(_(\"Cannot pull with rebase: You have unstaged changes.\"));\n+\t\tdo_die = 1;\n+\t}\n+\n+\tif (has_uncommitted_changes(prefix)) {\n+\t\tif (do_die)\n+\t\t\terror(_(\"Additionally, your index contains uncommitted changes.\"));\n+\t\telse\n+\t\t\terror(_(\"Cannot pull with rebase: Your index contains uncommitted changes.\"));\n+\t\tdo_die = 1;\n+\t}\n+\n+\tif (do_die)\n+\t\texit(1);\n+}\n+\n+/**\n  * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge\n  * into merge_heads.\n  */\n@@ -757,9 +826,15 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \tif (get_sha1(\"HEAD\", orig_head))\n \t\thashclr(orig_head);\n \n-\tif (opt_rebase)\n+\tif (opt_rebase) {\n+\t\tif (is_null_sha1(orig_head) && !is_cache_unborn())\n+\t\t\tdie(_(\"Updating an unborn branch with changes added to the index.\"));\n+\n+\t\tdie_on_unclean_work_tree(prefix);\n+\n \t\tif (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))\n \t\t\thashclr(rebase_fork_point);\n+\t}\n \n \tif (run_fetch(repo, refspecs))\n \t\treturn 1;\n-- \n2.1.4\n"},{"id":"262838","messageId":"1433314143-4478-19-git-send-email-pyokagan@gmail.com","threadId":"39509","inReplyTo":"1433314143-4478-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 18/19] pull --rebase: error on no merge candidate cases","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-03T06:49:02Z","receivedAt":"2015-06-03T06:49:02Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Tweak the error messages printed by die_no_merge_candidates() to take\ninto account that we may be \"rebasing against\" rather than \"merging\nwith\".\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n\nNotes:\n    v2\n    \n    * Decided to use fprintf_ln() for the sake of code consistency, and for\n      the added trailing newline.\n\n builtin/pull.c | 15 ++++++++++++---\n 1 file changed, 12 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex f5d437a..4e1ab5b 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -439,7 +439,10 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs\n \tconst char *remote = curr_branch ? curr_branch->remote_name : NULL;\n \n \tif (*refspecs) {\n-\t\tfprintf_ln(stderr, _(\"There are no candidates for merging among the refs that you just fetched.\"));\n+\t\tif (opt_rebase)\n+\t\t\tfprintf_ln(stderr, _(\"There is no candidate for rebasing against among the refs that you just fetched.\"));\n+\t\telse\n+\t\t\tfprintf_ln(stderr, _(\"There are no candidates for merging among the refs that you just fetched.\"));\n \t\tfprintf_ln(stderr, _(\"Generally this means that you provided a wildcard refspec which had no\\n\"\n \t\t\t\t\t\"matches on the remote end.\"));\n \t} else if (repo && curr_branch && (!remote || strcmp(repo, remote))) {\n@@ -449,7 +452,10 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs\n \t\t\trepo);\n \t} else if (!curr_branch) {\n \t\tfprintf_ln(stderr, _(\"You are not currently on a branch.\"));\n-\t\tfprintf_ln(stderr, _(\"Please specify which branch you want to merge with.\"));\n+\t\tif (opt_rebase)\n+\t\t\tfprintf_ln(stderr, _(\"Please specify which branch you want to rebase against.\"));\n+\t\telse\n+\t\t\tfprintf_ln(stderr, _(\"Please specify which branch you want to merge with.\"));\n \t\tfprintf_ln(stderr, _(\"See git-pull(1) for details.\"));\n \t\tfprintf(stderr, \"\\n\");\n \t\tfprintf_ln(stderr, \"    git pull <remote> <branch>\");\n@@ -461,7 +467,10 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs\n \t\t\tremote_name = \"<remote>\";\n \n \t\tfprintf_ln(stderr, _(\"There is no tracking information for the current branch.\"));\n-\t\tfprintf_ln(stderr, _(\"Please specify which branch you want to merge with.\"));\n+\t\tif (opt_rebase)\n+\t\t\tfprintf_ln(stderr, _(\"Please specify which branch you want to rebase against.\"));\n+\t\telse\n+\t\t\tfprintf_ln(stderr, _(\"Please specify which branch you want to merge with.\"));\n \t\tfprintf_ln(stderr, _(\"See git-pull(1) for details.\"));\n \t\tfprintf(stderr, \"\\n\");\n \t\tfprintf_ln(stderr, \"    git pull <remote> <branch>\");\n-- \n2.1.4\n"},{"id":"262839","messageId":"1433314143-4478-20-git-send-email-pyokagan@gmail.com","threadId":"39509","inReplyTo":"1433314143-4478-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 19/19] pull: remove redirection to git-pull.sh","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-03T06:49:03Z","receivedAt":"2015-06-03T06:49:03Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"At the beginning of the rewrite of git-pull.sh to C, we introduced a\nredirection to git-pull.sh if the environment variable\n_GIT_USE_BUILTIN_PULL was not defined in order to not break test scripts\nthat relied on a functional git-pull.\n\nNow that all of git-pull's functionality has been re-implemented in\nbuiltin/pull.c, remove this redirection, and retire the old git-pull.sh\ninto contrib/examples/.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n Makefile                                    | 1 -\n builtin/pull.c                              | 7 -------\n git-pull.sh => contrib/examples/git-pull.sh | 0\n 3 files changed, 8 deletions(-)\n rename git-pull.sh => contrib/examples/git-pull.sh (100%)\n\ndiff --git a/Makefile b/Makefile\nindex 2057a9d..67cef1c 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -474,7 +474,6 @@ SCRIPT_SH += git-merge-octopus.sh\n SCRIPT_SH += git-merge-one-file.sh\n SCRIPT_SH += git-merge-resolve.sh\n SCRIPT_SH += git-mergetool.sh\n-SCRIPT_SH += git-pull.sh\n SCRIPT_SH += git-quiltimport.sh\n SCRIPT_SH += git-rebase.sh\n SCRIPT_SH += git-remote-testgit.sh\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 4e1ab5b..dad49cf 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -805,13 +805,6 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \tunsigned char orig_head[GIT_SHA1_RAWSZ], curr_head[GIT_SHA1_RAWSZ];\n \tunsigned char rebase_fork_point[GIT_SHA1_RAWSZ];\n \n-\tif (!getenv(\"_GIT_USE_BUILTIN_PULL\")) {\n-\t\tconst char *path = mkpath(\"%s/git-pull\", git_exec_path());\n-\n-\t\tif (sane_execvp(path, (char**) argv) < 0)\n-\t\t\tdie_errno(\"could not exec %s\", path);\n-\t}\n-\n \tif (!getenv(\"GIT_REFLOG_ACTION\"))\n \t\tset_reflog_message(argc, argv);\n \ndiff --git a/git-pull.sh b/contrib/examples/git-pull.sh\nsimilarity index 100%\nrename from git-pull.sh\nrename to contrib/examples/git-pull.sh\n-- \n2.1.4\n"},{"id":"262858","messageId":"20150603102716.GA29730@vps892.directvps.nl","threadId":"39509","inReplyTo":"1433314143-4478-18-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 17/19] pull --rebase: exit early when the working directory is dirty","fromName":"Kevin Daudt","fromEmail":"me@ikke.info","sentAt":"2015-06-03T10:27:16Z","receivedAt":"2015-06-03T10:27:16Z","isPatch":true,"sender":{"key":"me@ikke.info","avatar":"https://avatars.githubusercontent.com/u/135698?v=4"},"body":"On Wed, Jun 03, 2015 at 02:49:01PM +0800, Paul Tan wrote:\n> Re-implement the behavior introduced by f9189cf (pull --rebase: exit\n> early when the working directory is dirty, 2008-05-21).\n\nWhen the option rebase.autoStash is set, it should not be necessary to\ndie in this case. See also this[1] patch I'm working on.\n\n[1]: http://article.gmane.org/gmane.comp.version-control.git/270612\n"},{"id":"262880","messageId":"CAGZ79kbGN6fxCPsARp3fAioi73vsddqLAKTOi9Kmd2DCcWhizA@mail.gmail.com","threadId":"39509","inReplyTo":"1433314143-4478-3-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 02/19] parse-options-cb: implement parse_opt_pass_argv_array()","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-03T16:56:39Z","receivedAt":"2015-06-03T16:56:39Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Jun 2, 2015 at 11:48 PM, Paul Tan <pyokagan@gmail.com> wrote:\n> Certain git commands, such as git-pull, are simply wrappers around other\n> git commands like git-fetch, git-merge and git-rebase. As such, these\n> wrapper commands will typically need to \"pass through\" command-line\n> options of the commands they wrap.\n>\n> Implement the parse_opt_pass_argv_array() parse-options callback, which\n> will reconstruct all the provided command-line options into an\n> argv_array, such that it can be passed to another git command. This is\n> useful for passing command-line options that can be specified multiple\n> times.\n>\n> Signed-off-by: Paul Tan <pyokagan@gmail.com>\n> ---\n>\n> Notes:\n>     v2\n>\n>     * This function is a requirement for the rewrite of git-am to handle\n>       passing git-apply's options to git-apply. Since it would be\n>       implemented anyway I thought it would be good if git-pull could take\n>       advantage of it as well to handle --strategy and --strategy-option.\n>\n>  parse-options-cb.c | 32 ++++++++++++++++++++++++++++++++\n>  parse-options.h    |  1 +\n>  2 files changed, 33 insertions(+)\n>\n> diff --git a/parse-options-cb.c b/parse-options-cb.c\n> index 5b1dbcf..7330506 100644\n> --- a/parse-options-cb.c\n> +++ b/parse-options-cb.c\n> @@ -4,6 +4,7 @@\n>  #include \"commit.h\"\n>  #include \"color.h\"\n>  #include \"string-list.h\"\n> +#include \"argv-array.h\"\n>\n>  /*----- some often used options -----*/\n>\n> @@ -163,3 +164,34 @@ int parse_opt_pass_strbuf(const struct option *opt, const char *arg, int unset)\n>\n>         return 0;\n>  }\n> +\n> +/**\n> + * For an option opt, recreate the command-line option, appending it to\n> + * opt->value which must be a argv_array. This is useful when we need to pass\n> + * the command-line option, which can be specified multiple times, to another\n> + * command.\n> + */\n> +int parse_opt_pass_argv_array(const struct option *opt, const char *arg, int unset)\n> +{\n> +       struct strbuf sb = STRBUF_INIT;\n> +       struct argv_array *opt_value = opt->value;\n> +\n> +       if (opt->long_name) {\n> +               strbuf_addstr(&sb, unset ? \"--no-\" : \"--\");\n> +               strbuf_addstr(&sb, opt->long_name);\n> +               if (arg) {\n> +                       strbuf_addch(&sb, '=');\n> +                       strbuf_addstr(&sb, arg);\n> +               }\n> +       } else if (opt->short_name && !unset) {\n> +               strbuf_addch(&sb, '-');\n> +               strbuf_addch(&sb, opt->short_name);\n> +               if (arg)\n> +                       strbuf_addstr(&sb, arg);\n> +       } else\n> +               return -1;\n> +\n> +       argv_array_push(opt_value, sb.buf);\n> +       strbuf_release(&sb);\n> +       return 0;\n> +}\n> diff --git a/parse-options.h b/parse-options.h\n> index 1d21398..b663f87 100644\n> --- a/parse-options.h\n> +++ b/parse-options.h\n> @@ -225,6 +225,7 @@ extern int parse_opt_tertiary(const struct option *, const char *, int);\n>  extern int parse_opt_string_list(const struct option *, const char *, int);\n>  extern int parse_opt_noop_cb(const struct option *, const char *, int);\n>  extern int parse_opt_pass_strbuf(const struct option *, const char *, int);\n> +extern int parse_opt_pass_argv_array(const struct option *, const char *, int);\n>\n>  #define OPT__VERBOSE(var, h)  OPT_COUNTUP('v', \"verbose\", (var), (h))\n>  #define OPT__QUIET(var, h)    OPT_COUNTUP('q', \"quiet\",   (var), (h))\n> --\n> 2.1.4\n>\n\nThis patch looks very similar to the first one, so I wonder if the\ncombination of these\ncould be written as:\n\nstatic int parse_opt_pass_strbuf_internal(const struct option *opt,\nconst char *arg, int unset, struct strbuf *sb)\n{\n       if (opt->long_name) {\n               strbuf_addstr(sb, unset ? \"--no-\" : \"--\");\n               strbuf_addstr(sb, opt->long_name);\n               if (arg) {\n                       strbuf_addch(sb, '=');\n                       strbuf_addstr(sb, arg);\n               }\n       } else if (opt->short_name && !unset) {\n               strbuf_addch(sb, '-');\n               strbuf_addch(sb, opt->short_name);\n               if (arg)\n                       strbuf_addstr(sb, arg);\n       } else\n               return -1;\n      return 0;\n}\n\n/**\n * For an option opt, recreates the command-line option in opt->value which\n * must be an strbuf. This is useful when we need to pass the command-line\n * option to another command.\n */\nint parse_opt_pass_strbuf(const struct option *opt, const char *arg, int unset)\n{\n       struct strbuf *sb = opt->value;\n       strbuf_reset(sb);\n       return parse_opt_pass_strbuf_internal(...);\n}\n\n/**\n * For an option opt, recreate the command-line option, appending it to\n * opt->value which must be a argv_array. This is useful when we need to pass\n * the command-line option, which can be specified multiple times, to another\n * command.\n */\nint parse_opt_pass_argv_array(const struct option *opt, const char\n*arg, int unset)\n{\n       struct strbuf sb = STRBUF_INIT;\n       struct argv_array *opt_value = opt->value;\n\n       int ret = parse_opt_pass_strbuf_internal(...)\n\n       argv_array_push(opt_value, sb.buf);\n       strbuf_release(&sb);\n       return 0;\n}\n"},{"id":"262882","messageId":"CAGZ79kbZEyz1w4ATBB+nZObLmyTNPi+m_b5pj_Kbp+o-36HnHw@mail.gmail.com","threadId":"39509","inReplyTo":"1433314143-4478-9-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 08/19] pull: pass git-fetch's options to git-fetch","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-03T17:16:19Z","receivedAt":"2015-06-03T17:16:19Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Jun 2, 2015 at 11:48 PM, Paul Tan <pyokagan@gmail.com> wrote:\n> Since eb2a8d9 (pull: handle git-fetch's options as well, 2015-06-02),\n> git-pull knows about and handles git-fetch's options, passing them to\n> git-fetch. Re-implement this behavior.\n>\n> Since 29609e6 (pull: do nothing on --dry-run, 2010-05-25) git-pull\n> supported the --dry-run option, exiting after git-fetch if --dry-run is\n> set. Re-implement this behavior.\n>\n> Signed-off-by: Paul Tan <pyokagan@gmail.com>\n> ---\n>\n> Notes:\n>     v2\n>\n>     * Use parse_opt_parse_strbuf()\n>\n>  builtin/pull.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n>  1 file changed, 95 insertions(+)\n>\n> diff --git a/builtin/pull.c b/builtin/pull.c\n> index 5f08634..0b66b43 100644\n> --- a/builtin/pull.c\n> +++ b/builtin/pull.c\n> @@ -32,6 +32,21 @@ static struct argv_array opt_strategies = ARGV_ARRAY_INIT;\n>  static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;\n>  static struct strbuf opt_gpg_sign = STRBUF_INIT;\n>\n> +/* Options passed to git-fetch */\n> +static struct strbuf opt_all = STRBUF_INIT;\n> +static struct strbuf opt_append = STRBUF_INIT;\n> +static struct strbuf opt_upload_pack = STRBUF_INIT;\n> +static int opt_force;\n> +static struct strbuf opt_tags = STRBUF_INIT;\n> +static struct strbuf opt_prune = STRBUF_INIT;\n> +static struct strbuf opt_recurse_submodules = STRBUF_INIT;\n> +static int opt_dry_run;\n> +static struct strbuf opt_keep = STRBUF_INIT;\n> +static struct strbuf opt_depth = STRBUF_INIT;\n> +static struct strbuf opt_unshallow = STRBUF_INIT;\n> +static struct strbuf opt_update_shallow = STRBUF_INIT;\n> +static struct strbuf opt_refmap = STRBUF_INIT;\n> +\n>  static struct option pull_options[] = {\n>         /* Shared options */\n>         OPT__VERBOSITY(&opt_verbosity),\n> @@ -82,6 +97,46 @@ static struct option pull_options[] = {\n>           N_(\"GPG sign commit\"),\n>           PARSE_OPT_OPTARG, parse_opt_pass_strbuf },\n>\n> +       /* Options passed to git-fetch */\n> +       OPT_GROUP(N_(\"Options related to fetching\")),\n> +       { OPTION_CALLBACK, 0, \"all\", &opt_all, 0,\n> +         N_(\"fetch from all remotes\"),\n> +         PARSE_OPT_NOARG, parse_opt_pass_strbuf },\n> +       { OPTION_CALLBACK, 'a', \"append\", &opt_append, 0,\n> +         N_(\"append to .git/FETCH_HEAD instead of overwriting\"),\n> +         PARSE_OPT_NOARG, parse_opt_pass_strbuf },\n> +       { OPTION_CALLBACK, 0, \"upload-pack\", &opt_upload_pack, N_(\"path\"),\n> +         N_(\"path to upload pack on remote end\"),\n> +         0, parse_opt_pass_strbuf },\n> +       OPT__FORCE(&opt_force, N_(\"force overwrite of local branch\")),\n> +       { OPTION_CALLBACK, 't', \"tags\", &opt_tags, 0,\n> +         N_(\"fetch all tags and associated objects\"),\n> +         PARSE_OPT_NOARG, parse_opt_pass_strbuf },\n> +       { OPTION_CALLBACK, 'p', \"prune\", &opt_prune, 0,\n> +         N_(\"prune remote-tracking branches no longer on remote\"),\n> +         PARSE_OPT_NOARG, parse_opt_pass_strbuf },\n> +       { OPTION_CALLBACK, 0, \"recurse-submodules\", &opt_recurse_submodules,\n> +         N_(\"on-demand\"),\n> +         N_(\"control recursive fetching of submodules\"),\n> +         PARSE_OPT_OPTARG, parse_opt_pass_strbuf },\n> +       OPT_BOOL(0, \"dry-run\", &opt_dry_run,\n> +               N_(\"dry run\")),\n> +       { OPTION_CALLBACK, 'k', \"keep\", &opt_keep, 0,\n> +         N_(\"keep downloaded pack\"),\n> +         PARSE_OPT_NOARG, parse_opt_pass_strbuf },\n> +       { OPTION_CALLBACK, 0, \"depth\", &opt_depth, N_(\"depth\"),\n> +         N_(\"deepen history of shallow clone\"),\n> +         0, parse_opt_pass_strbuf },\n> +       { OPTION_CALLBACK, 0, \"unshallow\", &opt_unshallow, 0,\n> +         N_(\"convert to a complete repository\"),\n> +         PARSE_OPT_NONEG | PARSE_OPT_NOARG, parse_opt_pass_strbuf },\n> +       { OPTION_CALLBACK, 0, \"update-shallow\", &opt_update_shallow, 0,\n> +         N_(\"accept refs that update .git/shallow\"),\n> +         PARSE_OPT_NOARG, parse_opt_pass_strbuf },\n> +       { OPTION_CALLBACK, 0, \"refmap\", &opt_refmap, N_(\"refmap\"),\n> +         N_(\"specify fetch refmap\"),\n> +         PARSE_OPT_NONEG, parse_opt_pass_strbuf },\n> +\n>         OPT_END()\n>  };\n>\n> @@ -100,6 +155,16 @@ static void argv_push_verbosity(struct argv_array *arr)\n>  }\n>\n>  /**\n> + * Pushes \"-f\" switches into arr to match the opt_force level.\n> + */\n> +static void argv_push_force(struct argv_array *arr)\n> +{\n> +       int force = opt_force;\n> +       while (force-- > 0)\n\nThis made me chuckle despite the formatting,\nas we referred to it as the limes operator in school for fun\n\n    #define limes while\n    limes (n --> 0) { // n goes towards zero\n        ...\n    }\n\nA quick\n    grep -r \"--\" -- *.c |grep while\nshows we actually use this quite a lot, though the \"> 0\" is\nomitted quite often.\n\n> +               argv_array_push(arr, \"-f\");\n> +}\n> +\n> +/**\n>   * Parses argv into [<repo> [<refspecs>...]], returning their values in `repo`\n>   * as a string and `refspecs` as a null-terminated array of strings. If `repo`\n>   * is not provided in argv, it is set to NULL.\n> @@ -131,6 +196,33 @@ static int run_fetch(const char *repo, const char **refspecs)\n>         if (opt_progress.len)\n>                 argv_array_push(&args, opt_progress.buf);\n>\n> +       /* Options passed to git-fetch */\n> +       if (opt_all.len)\n> +               argv_array_push(&args, opt_all.buf);\n> +       if (opt_append.len)\n> +               argv_array_push(&args, opt_append.buf);\n> +       if (opt_upload_pack.len)\n> +               argv_array_push(&args, opt_upload_pack.buf);\n> +       argv_push_force(&args);\n> +       if (opt_tags.len)\n> +               argv_array_push(&args, opt_tags.buf);\n> +       if (opt_prune.len)\n> +               argv_array_push(&args, opt_prune.buf);\n> +       if (opt_recurse_submodules.len)\n> +               argv_array_push(&args, opt_recurse_submodules.buf);\n> +       if (opt_dry_run)\n> +               argv_array_push(&args, \"--dry-run\");\n> +       if (opt_keep.len)\n> +               argv_array_push(&args, opt_keep.buf);\n> +       if (opt_depth.len)\n> +               argv_array_push(&args, opt_depth.buf);\n> +       if (opt_unshallow.len)\n> +               argv_array_push(&args, opt_unshallow.buf);\n> +       if (opt_update_shallow.len)\n> +               argv_array_push(&args, opt_update_shallow.buf);\n> +       if (opt_refmap.len)\n> +               argv_array_push(&args, opt_refmap.buf);\n> +\n>         if (repo)\n>                 argv_array_push(&args, repo);\n>         while (*refspecs)\n> @@ -199,5 +291,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n>         if (run_fetch(repo, refspecs))\n>                 return 1;\n>\n> +       if (opt_dry_run)\n> +               return 0;\n> +\n>         return run_merge();\n>  }\n> --\n> 2.1.4\n>\n"},{"id":"262888","messageId":"CAGZ79kZN=1fzjSv-dSgq38CtEKTDxLMoPY8ToNfiEA8QDh3Xpg@mail.gmail.com","threadId":"39509","inReplyTo":"1433314143-4478-19-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 18/19] pull --rebase: error on no merge candidate cases","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-03T17:38:13Z","receivedAt":"2015-06-03T17:38:13Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Jun 2, 2015 at 11:49 PM, Paul Tan <pyokagan@gmail.com> wrote:\n> Tweak the error messages printed by die_no_merge_candidates() to take\n> into account that we may be \"rebasing against\" rather than \"merging\n> with\".\n>\n> Signed-off-by: Paul Tan <pyokagan@gmail.com>\n> ---\n>\n> Notes:\n>     v2\n>\n>     * Decided to use fprintf_ln() for the sake of code consistency, and for\n>       the added trailing newline.\n>\n>  builtin/pull.c | 15 ++++++++++++---\n>  1 file changed, 12 insertions(+), 3 deletions(-)\n>\n> diff --git a/builtin/pull.c b/builtin/pull.c\n> index f5d437a..4e1ab5b 100644\n> --- a/builtin/pull.c\n> +++ b/builtin/pull.c\n> @@ -439,7 +439,10 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs\n>         const char *remote = curr_branch ? curr_branch->remote_name : NULL;\n>\n>         if (*refspecs) {\n> -               fprintf_ln(stderr, _(\"There are no candidates for merging among the refs that you just fetched.\"));\n> +               if (opt_rebase)\n> +                       fprintf_ln(stderr, _(\"There is no candidate for rebasing against among the refs that you just fetched.\"));\n> +               else\n> +                       fprintf_ln(stderr, _(\"There are no candidates for merging among the refs that you just fetched.\"));\n>                 fprintf_ln(stderr, _(\"Generally this means that you provided a wildcard refspec which had no\\n\"\n>                                         \"matches on the remote end.\"));\n>         } else if (repo && curr_branch && (!remote || strcmp(repo, remote))) {\n> @@ -449,7 +452,10 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs\n>                         repo);\n>         } else if (!curr_branch) {\n>                 fprintf_ln(stderr, _(\"You are not currently on a branch.\"));\n> -               fprintf_ln(stderr, _(\"Please specify which branch you want to merge with.\"));\n> +               if (opt_rebase)\n> +                       fprintf_ln(stderr, _(\"Please specify which branch you want to rebase against.\"));\n> +               else\n> +                       fprintf_ln(stderr, _(\"Please specify which branch you want to merge with.\"));\n\n\nNow that you're using fprintf you could make use of its formatting\ncapabilities, but then it occurred to me\nit's translated strings, so it's most likely better to not make it\nconcise but rather easier for the translators\nby having each sentence written out in full.\n\n>                 fprintf_ln(stderr, _(\"See git-pull(1) for details.\"));\n>                 fprintf(stderr, \"\\n\");\n>                 fprintf_ln(stderr, \"    git pull <remote> <branch>\");\n> @@ -461,7 +467,10 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs\n>                         remote_name = \"<remote>\";\n>\n>                 fprintf_ln(stderr, _(\"There is no tracking information for the current branch.\"));\n> -               fprintf_ln(stderr, _(\"Please specify which branch you want to merge with.\"));\n> +               if (opt_rebase)\n> +                       fprintf_ln(stderr, _(\"Please specify which branch you want to rebase against.\"));\n> +               else\n> +                       fprintf_ln(stderr, _(\"Please specify which branch you want to merge with.\"));\n>                 fprintf_ln(stderr, _(\"See git-pull(1) for details.\"));\n>                 fprintf(stderr, \"\\n\");\n>                 fprintf_ln(stderr, \"    git pull <remote> <branch>\");\n> --\n> 2.1.4\n>\n"},{"id":"262892","messageId":"CAGZ79kaR_MFgbo6F9=1s9h5OtXSDu6526iKT_hVcUCq9vpf8-A@mail.gmail.com","threadId":"39509","inReplyTo":"1433314143-4478-20-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 19/19] pull: remove redirection to git-pull.sh","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-03T17:49:54Z","receivedAt":"2015-06-03T17:49:54Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Jun 2, 2015 at 11:49 PM, Paul Tan <pyokagan@gmail.com> wrote:\n> At the beginning of the rewrite of git-pull.sh to C, we introduced a\n> redirection to git-pull.sh if the environment variable\n> _GIT_USE_BUILTIN_PULL was not defined in order to not break test scripts\n> that relied on a functional git-pull.\n>\n> Now that all of git-pull's functionality has been re-implemented in\n> builtin/pull.c, remove this redirection, and retire the old git-pull.sh\n> into contrib/examples/.\n>\n> Signed-off-by: Paul Tan <pyokagan@gmail.com>\n\nThe whole series was a pleasant read.\n\nThanks,\nStefan\n\n> ---\n>  Makefile                                    | 1 -\n>  builtin/pull.c                              | 7 -------\n>  git-pull.sh => contrib/examples/git-pull.sh | 0\n>  3 files changed, 8 deletions(-)\n>  rename git-pull.sh => contrib/examples/git-pull.sh (100%)\n>\n> diff --git a/Makefile b/Makefile\n> index 2057a9d..67cef1c 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -474,7 +474,6 @@ SCRIPT_SH += git-merge-octopus.sh\n>  SCRIPT_SH += git-merge-one-file.sh\n>  SCRIPT_SH += git-merge-resolve.sh\n>  SCRIPT_SH += git-mergetool.sh\n> -SCRIPT_SH += git-pull.sh\n>  SCRIPT_SH += git-quiltimport.sh\n>  SCRIPT_SH += git-rebase.sh\n>  SCRIPT_SH += git-remote-testgit.sh\n> diff --git a/builtin/pull.c b/builtin/pull.c\n> index 4e1ab5b..dad49cf 100644\n> --- a/builtin/pull.c\n> +++ b/builtin/pull.c\n> @@ -805,13 +805,6 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n>         unsigned char orig_head[GIT_SHA1_RAWSZ], curr_head[GIT_SHA1_RAWSZ];\n>         unsigned char rebase_fork_point[GIT_SHA1_RAWSZ];\n>\n> -       if (!getenv(\"_GIT_USE_BUILTIN_PULL\")) {\n> -               const char *path = mkpath(\"%s/git-pull\", git_exec_path());\n> -\n> -               if (sane_execvp(path, (char**) argv) < 0)\n> -                       die_errno(\"could not exec %s\", path);\n> -       }\n> -\n>         if (!getenv(\"GIT_REFLOG_ACTION\"))\n>                 set_reflog_message(argc, argv);\n>\n> diff --git a/git-pull.sh b/contrib/examples/git-pull.sh\n> similarity index 100%\n> rename from git-pull.sh\n> rename to contrib/examples/git-pull.sh\n> --\n> 2.1.4\n>\n"},{"id":"263415","messageId":"xmqqvbewwk7o.fsf@gitster.dls.corp.google.com","threadId":"39509","inReplyTo":"1433314143-4478-2-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 01/19] parse-options-cb: implement parse_opt_pass_strbuf()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-09T23:11:39Z","receivedAt":"2015-06-09T23:11:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> diff --git a/parse-options-cb.c b/parse-options-cb.c\n> index be8c413..5b1dbcf 100644\n> --- a/parse-options-cb.c\n> +++ b/parse-options-cb.c\n> @@ -134,3 +134,32 @@ int parse_opt_noop_cb(const struct option *opt, const char *arg, int unset)\n>  {\n>  \treturn 0;\n>  }\n> +\n> +/**\n> + * For an option opt, recreates the command-line option in opt->value which\n> + * must be an strbuf. This is useful when we need to pass the command-line\n> + * option to another command.\n> + */\n\nThis is to be used to create a single argument, not an entire\ncommand line to be executed via run_command() or a shell, right?\nIt wasn't very clear from the above command.\n\nIf the same option is given more than once, the earlier ones are\ndiscarded and the last one wins.  That may be OK for your expected\ncallers, and I do not think this needs to have an option to instead\naccumulate them, but it needs to be made clear in the comment and\nthe API documentation for future developers who want to use the\nparse-options API.\n\n> +int parse_opt_pass_strbuf(const struct option *opt, const char *arg, int unset)\n> +{\n> +\tstruct strbuf *sb = opt->value;\n> +\n> +\tstrbuf_reset(sb);\n> +\n> +\tif (opt->long_name) {\n> +\t\tstrbuf_addstr(sb, unset ? \"--no-\" : \"--\");\n> +\t\tstrbuf_addstr(sb, opt->long_name);\n> +\t\tif (arg) {\n> +\t\t\tstrbuf_addch(sb, '=');\n> +\t\t\tstrbuf_addstr(sb, arg);\n> +\t\t}\n> +\t} else if (opt->short_name && !unset) {\n> +\t\tstrbuf_addch(sb, '-');\n> +\t\tstrbuf_addch(sb, opt->short_name);\n> +\t\tif (arg)\n> +\t\t\tstrbuf_addstr(sb, arg);\n> +\t} else\n> +\t\treturn -1;\n> +\n> +\treturn 0;\n> +}\n> diff --git a/parse-options.h b/parse-options.h\n> index c71e9da..1d21398 100644\n> --- a/parse-options.h\n> +++ b/parse-options.h\n> @@ -224,6 +224,7 @@ extern int parse_opt_with_commit(const struct option *, const char *, int);\n>  extern int parse_opt_tertiary(const struct option *, const char *, int);\n>  extern int parse_opt_string_list(const struct option *, const char *, int);\n>  extern int parse_opt_noop_cb(const struct option *, const char *, int);\n> +extern int parse_opt_pass_strbuf(const struct option *, const char *, int);\n>  \n>  #define OPT__VERBOSE(var, h)  OPT_COUNTUP('v', \"verbose\", (var), (h))\n>  #define OPT__QUIET(var, h)    OPT_COUNTUP('q', \"quiet\",   (var), (h))\n"},{"id":"263417","messageId":"xmqqr3pkwjz6.fsf@gitster.dls.corp.google.com","threadId":"39509","inReplyTo":"1433314143-4478-3-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 02/19] parse-options-cb: implement parse_opt_pass_argv_array()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-09T23:16:45Z","receivedAt":"2015-06-09T23:16:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> Certain git commands, such as git-pull, are simply wrappers around other\n> git commands like git-fetch, git-merge and git-rebase. As such, these\n> wrapper commands will typically need to \"pass through\" command-line\n> options of the commands they wrap.\n>\n> Implement the parse_opt_pass_argv_array() parse-options callback, which\n> will reconstruct all the provided command-line options into an\n> argv_array, such that it can be passed to another git command. This is\n> useful for passing command-line options that can be specified multiple\n> times.\n>\n> Signed-off-by: Paul Tan <pyokagan@gmail.com>\n> ---\n>\n> Notes:\n>     v2\n>     \n>     * This function is a requirement for the rewrite of git-am to handle\n>       passing git-apply's options to git-apply. Since it would be\n>       implemented anyway I thought it would be good if git-pull could take\n>       advantage of it as well to handle --strategy and --strategy-option.\n>\n>  parse-options-cb.c | 32 ++++++++++++++++++++++++++++++++\n>  parse-options.h    |  1 +\n>  2 files changed, 33 insertions(+)\n>\n> diff --git a/parse-options-cb.c b/parse-options-cb.c\n> index 5b1dbcf..7330506 100644\n> --- a/parse-options-cb.c\n> +++ b/parse-options-cb.c\n> @@ -4,6 +4,7 @@\n>  #include \"commit.h\"\n>  #include \"color.h\"\n>  #include \"string-list.h\"\n> +#include \"argv-array.h\"\n>  \n>  /*----- some often used options -----*/\n>  \n> @@ -163,3 +164,34 @@ int parse_opt_pass_strbuf(const struct option *opt, const char *arg, int unset)\n>  \n>  \treturn 0;\n>  }\n> +\n> +/**\n> + * For an option opt, recreate the command-line option, appending it to\n> + * opt->value which must be a argv_array. This is useful when we need to pass\n> + * the command-line option, which can be specified multiple times, to another\n> + * command.\n> + */\n\nAlmost the same comment as 01/19 applies to this comment.\n\nI think it makes good sense to have two variants, one that lets the\nlast one win and pass only that last one (i.e. 01/19) and the other\nthat accumulates them into an argv_array (i.e. this one).  But it\nfeels iffy, given that the \"acculate\" version essentially creates an\narray of (char *), to make \"the last one wins, leaving a single\nstring\" to use strbuf.  I'd find it much more understandable if 01/19\ntook (char **) as opt->value instead of a strbuf.\n\nIn any case, these two need to be added as a related pair to the API\ndocumentation.\n\nThanks.\n\n> +int parse_opt_pass_argv_array(const struct option *opt, const char *arg, int unset)\n> +{\n> +\tstruct strbuf sb = STRBUF_INIT;\n> +\tstruct argv_array *opt_value = opt->value;\n> +\n> +\tif (opt->long_name) {\n> +\t\tstrbuf_addstr(&sb, unset ? \"--no-\" : \"--\");\n> +\t\tstrbuf_addstr(&sb, opt->long_name);\n> +\t\tif (arg) {\n> +\t\t\tstrbuf_addch(&sb, '=');\n> +\t\t\tstrbuf_addstr(&sb, arg);\n> +\t\t}\n> +\t} else if (opt->short_name && !unset) {\n> +\t\tstrbuf_addch(&sb, '-');\n> +\t\tstrbuf_addch(&sb, opt->short_name);\n> +\t\tif (arg)\n> +\t\t\tstrbuf_addstr(&sb, arg);\n> +\t} else\n> +\t\treturn -1;\n> +\n> +\targv_array_push(opt_value, sb.buf);\n> +\tstrbuf_release(&sb);\n> +\treturn 0;\n> +}\n> diff --git a/parse-options.h b/parse-options.h\n> index 1d21398..b663f87 100644\n> --- a/parse-options.h\n> +++ b/parse-options.h\n> @@ -225,6 +225,7 @@ extern int parse_opt_tertiary(const struct option *, const char *, int);\n>  extern int parse_opt_string_list(const struct option *, const char *, int);\n>  extern int parse_opt_noop_cb(const struct option *, const char *, int);\n>  extern int parse_opt_pass_strbuf(const struct option *, const char *, int);\n> +extern int parse_opt_pass_argv_array(const struct option *, const char *, int);\n>  \n>  #define OPT__VERBOSE(var, h)  OPT_COUNTUP('v', \"verbose\", (var), (h))\n>  #define OPT__QUIET(var, h)    OPT_COUNTUP('q', \"quiet\",   (var), (h))\n"},{"id":"263419","messageId":"xmqqmw08wjgp.fsf@gitster.dls.corp.google.com","threadId":"39509","inReplyTo":"1433314143-4478-6-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 05/19] pull: implement fetch + merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-09T23:27:50Z","receivedAt":"2015-06-09T23:27:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> +/**\n> + * Parses argv into [<repo> [<refspecs>...]], returning their values in `repo`\n> + * as a string and `refspecs` as a null-terminated array of strings. If `repo`\n> + * is not provided in argv, it is set to NULL.\n> + */\n> +static void parse_repo_refspecs(int argc, const char **argv, const char **repo,\n> +\t\tconst char ***refspecs)\n> +{\n> +\tif (argc > 0) {\n> +\t\t*repo = *argv++;\n> +\t\targc--;\n> +\t} else\n> +\t\t*repo = NULL;\n> +\t*refspecs = argv;\n> +}\n> +\n> +/**\n> + * Runs git-fetch, returning its exit status. `repo` and `refspecs` are the\n> + * repository and refspecs to fetch, or NULL if they are not provided.\n> + */\n> +static int run_fetch(const char *repo, const char **refspecs)\n> +{\n> +\tstruct argv_array args = ARGV_ARRAY_INIT;\n> +\tint ret;\n> +\n> +\targv_array_pushl(&args, \"fetch\", \"--update-head-ok\", NULL);\n> +\tif (repo)\n> +\t\targv_array_push(&args, repo);\n> +\twhile (*refspecs)\n> +\t\targv_array_push(&args, *refspecs++);\n\nAs you cannot say \"git pull refspecs...\", the above might be more\nclear if you spelled it like this instead:\n\n\tif (repo) {\n\t\targv_array_push(&args, repo);\n\t\targv_array_pushv(&args, refspecs);\n\t} else if (*refspecs) {\n        \tdie(\"BUG: refspec without repo?\");\n\t}\n\n> +/**\n> + * Runs git-merge, returning its exit status.\n> + */\n> +static int run_merge(void)\n> +{\n> +\tint ret;\n> +\tstruct argv_array args = ARGV_ARRAY_INIT;\n> +\n> +\targv_array_pushl(&args, \"merge\", NULL);\n> +\targv_array_push(&args, \"FETCH_HEAD\");\n> +\tret = run_command_v_opt(args.argv, RUN_GIT_CMD);\n> +\targv_array_clear(&args);\n> +\treturn ret;\n> +}\n\nNo frills yet, which is a good way to start with and show the\noverall structure.\n"},{"id":"263420","messageId":"xmqqioawwj1u.fsf@gitster.dls.corp.google.com","threadId":"39509","inReplyTo":"1433314143-4478-7-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 06/19] pull: pass verbosity, --progress flags to fetch and merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-09T23:36:45Z","receivedAt":"2015-06-09T23:36:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> Re-implement support for this flag by introducing the option callback\n> handler parse_opt_passthru(). This callback is used to pass the\n> \"--progress\" or \"--no-progress\" command-line switch to git-fetch and\n> git-merge.\n\nForgot to rephrase?  parse-opt-passthru() is a good name for \"pass\nthe single string, last one wins\", but is implemented in another\npatch.\n\n\tUse parse_opt_passthru() implemented earlier to pass the\n\t\"--[no-]progress\" command line options to git-fetch and\n\tgit-merge.\n\nor something like that.\n\n> diff --git a/builtin/pull.c b/builtin/pull.c\n> index 0ca23a3..c9c2cc0 100644\n> --- a/builtin/pull.c\n> +++ b/builtin/pull.c\n> @@ -16,11 +16,35 @@ static const char * const pull_usage[] = {\n>  \tNULL\n>  };\n>  \n> +/* Shared options */\n> +static int opt_verbosity;\n> +static struct strbuf opt_progress = STRBUF_INIT;\n> +\n>  static struct option pull_options[] = {\n> +\t/* Shared options */\n> +\tOPT__VERBOSITY(&opt_verbosity),\n> +\t{ OPTION_CALLBACK, 0, \"progress\", &opt_progress, NULL,\n> +\t  N_(\"force progress reporting\"),\n> +\t  PARSE_OPT_NOARG, parse_opt_pass_strbuf},\n> +\n>  \tOPT_END()\n>  };\n\nSeeing this use case convinces me that the new parse-opt callback\nparse-opt-pass-single-string-last-one-wins() in 01/19 should not be\nstrbuf based interface but should be (const char *) based one.\n\nOther than that the change looks nicely done.  So far, the series\nhas been a very pleasant read up to this step.\n"},{"id":"263421","messageId":"xmqqeglkwi4u.fsf@gitster.dls.corp.google.com","threadId":"39509","inReplyTo":"1433314143-4478-10-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 09/19] pull: error on no merge candidates","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-09T23:56:33Z","receivedAt":"2015-06-09T23:56:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n>  /**\n> + * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge\n> + * into merge_heads.\n> + */\n\nHmph, I vaguely recall doing that in C elsewhere already, even\nthough I do not remember where offhand...\n\n> +static void get_merge_heads(struct sha1_array *merge_heads)\n> +{\n> +\tconst char *filename = git_path(\"FETCH_HEAD\");\n> +\tFILE *fp;\n> +\tstruct strbuf sb = STRBUF_INIT;\n> +\tunsigned char sha1[GIT_SHA1_RAWSZ];\n> +\n> +\tif (!(fp = fopen(filename, \"r\")))\n> +\t\tdie_errno(_(\"could not open '%s' for reading\"), filename);\n> +\twhile(strbuf_getline(&sb, fp, '\\n') != EOF) {\n\nMissing SP after \"while\"\n\n> +\t\tif (get_sha1_hex(sb.buf, sha1))\n> +\t\t\tcontinue;  /* invalid line: does not start with SHA1 */\n> +\t\tif (starts_with(sb.buf + GIT_SHA1_HEXSZ, \"\\tnot-for-merge\"))\n\nLook for \"\\tnot-for-merge\\t\" instead?\n\nThe patch overall looks good.  Thanks.\n"},{"id":"263422","messageId":"xmqqa8w8whzz.fsf@gitster.dls.corp.google.com","threadId":"39509","inReplyTo":"1433314143-4478-11-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 10/19] pull: support pull.ff config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-09T23:59:28Z","receivedAt":"2015-06-09T23:59:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n>     \n>     * Yup, I did mean strcmp(value, \"only\")\n\nI may be missing some backstory behind this comment; in the patch, I\ninstead see !strcmp(value, \"only\") and I think it makes sense.\n\n> +\tif (git_config_get_value(\"pull.ff\", &value))\n> +\t\treturn;\n> +\tswitch (git_config_maybe_bool(\"pull.ff\", value)) {\n> +\t\tcase 0:\n> +\t\t\tstrbuf_addstr(sb, \"--no-ff\");\n> +\t\t\treturn;\n\nAlign switch and case at the same indent level; the body of switch\nis overindented.\n\n> +\t\tcase 1:\n> +\t\t\tstrbuf_addstr(sb, \"--ff\");\n> +\t\t\treturn;\n> +\t}\n> +\tif (!strcmp(value, \"only\")) {\n> +\t\tstrbuf_addstr(sb, \"--ff-only\");\n> +\t\treturn;\n> +\t}\n"},{"id":"263423","messageId":"xmqq616wwgvz.fsf@gitster.dls.corp.google.com","threadId":"39509","inReplyTo":"1433314143-4478-5-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 04/19] pull: implement skeletal builtin pull","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-10T00:23:28Z","receivedAt":"2015-06-10T00:23:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> +int cmd_pull(int argc, const char **argv, const char *prefix)\n> +{\n> +\tif (!getenv(\"_GIT_USE_BUILTIN_PULL\")) {\n> +\t\tconst char *path = mkpath(\"%s/git-pull\", git_exec_path());\n> +\n> +\t\tif (sane_execvp(path, (char**) argv) < 0)\n\nStyle: \"(char **)argv\".\n"},{"id":"263426","messageId":"xmqqsia0uzaa.fsf@gitster.dls.corp.google.com","threadId":"39509","inReplyTo":"1433314143-4478-12-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 11/19] pull: check if in unresolved merge state","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-10T01:29:01Z","receivedAt":"2015-06-10T01:29:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> @@ -422,6 +423,14 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n>  \n>  \tparse_repo_refspecs(argc, argv, &repo, &refspecs);\n>  \n> +\tgit_config(git_default_config, NULL);\n> +\n> +\tif (read_cache_unmerged())\n> +\t\tdie_resolve_conflict(\"Pull\");\n> +\n> +\tif (file_exists(git_path(\"MERGE_HEAD\")))\n> +\t\tdie_conclude_merge();\n> +\n>  \tif (!opt_ff.len)\n>  \t\tconfig_get_ff(&opt_ff);\n\nHmph.\n\nIf you are going to do the git_config() call yourself, it might make\nmore sense to define git_pull_config() callback and parse the pull.ff\nyourself, updating the use of the lazy git_config_get_value() API you\nintroduced in patch 10/19.\n\nThe above \"might\" is stronger than my usual \"might\"; I am undecided\nyet before reading the remainder of the series.\n"},{"id":"263427","messageId":"xmqqoakouz5k.fsf@gitster.dls.corp.google.com","threadId":"39509","inReplyTo":"1433314143-4478-14-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 13/19] pull: implement pulling into an unborn branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-10T01:31:51Z","receivedAt":"2015-06-10T01:31:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n>  /**\n> + * \"Pulls into void\" by branching off merge_head.\n> + */\n> +static int pull_into_void(unsigned char merge_head[GIT_SHA1_RAWSZ],\n> +\t\tunsigned char curr_head[GIT_SHA1_RAWSZ])\n> +{\n\nIt is not wrong per-se, but is rather unusual (and misleading) to\nspecify the array-ness and array-size of parameters.  Our codebase\ntends to prefer spelling it like so instead:\n\n\tstatic int pull_into_void(unsigned char *merge_head,\n\t\t\t          unsigned char *curr_head)\n\t{\n\n> +\t/*\n> +\t * Two-way merge: we claim the index is based on an empty tree,\n\ns/claim/treat/ perhaps?\n\n> +\t * and try to fast-forward to HEAD. This ensures we will not lose\n> +\t * index/worktree changes that the user already made on the unborn\n> +\t * branch.\n> +\t */\n> +\tif (checkout_fast_forward(EMPTY_TREE_SHA1_BIN, merge_head, 0))\n> +\t\treturn 1;\n> +\n> +\tif (update_ref(\"initial pull\", \"HEAD\", merge_head, curr_head, 0, UPDATE_REFS_DIE_ON_ERR))\n> +\t\treturn 1;\n> +\n> +\treturn 0;\n> +}\n> +\n> +/**\n>   * Runs git-merge, returning its exit status.\n>   */\n>  static int run_merge(void)\n> @@ -475,5 +497,10 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n>  \tif (!merge_heads.nr)\n>  \t\tdie_no_merge_candidates(repo, refspecs);\n>  \n> -\treturn run_merge();\n> +\tif (is_null_sha1(orig_head)) {\n> +\t\tif (merge_heads.nr > 1)\n> +\t\t\tdie(_(\"Cannot merge multiple branches into empty head.\"));\n> +\t\treturn pull_into_void(*merge_heads.sha1, curr_head);\n> +\t} else\n> +\t\treturn run_merge();\n>  }\n\nSensible.\n"},{"id":"263428","messageId":"xmqqk2vcuy10.fsf@gitster.dls.corp.google.com","threadId":"39509","inReplyTo":"1433314143-4478-16-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 15/19] pull: teach git pull about --rebase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-10T01:56:11Z","receivedAt":"2015-06-10T01:56:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> +enum rebase_type {\n> +\tREBASE_INVALID = -1,\n> +\tREBASE_FALSE = 0,\n> +\tREBASE_TRUE,\n> +\tREBASE_PRESERVE\n> +};\n> +\n> +/**\n> + * Parses the value of --rebase, branch.*.rebase or pull.rebase. If value is a\n> + * false value, returns REBASE_FALSE. If value is a true value, returns\n> + * REBASE_TRUE. If value is \"preserve\", returns REBASE_PRESERVE. Otherwise,\n> + * returns -1 to signify an invalid value.\n> + */\n> +static enum rebase_type parse_config_rebase(const char *value)\n> +{\n> +\tint v = git_config_maybe_bool(\"pull.rebase\", value);\n> +\tif (!v)\n> +\t\treturn REBASE_FALSE;\n> +\telse if (v >= 0)\n> +\t\treturn REBASE_TRUE;\n\nIt is somewhat misleading to say \"v >= 0\" when you already use !v to\nsignal something else.  Perhaps \"else if (v > 0)\" is better?\n\n> +/**\n> + * Returns remote's upstream branch for the current branch. If remote is NULL,\n> + * the current branch's configured default remote is used. Returns NULL if\n> + * `remote` does not name a valid remote, HEAD does not point to a branch,\n> + * remote is not the branch's configured remote or the branch does not have any\n> + * configured upstream branch.\n> + */\n> +static char *get_upstream_branch(const char *remote)\n> +{\n> +\tstruct remote *rm;\n> +\tstruct branch *curr_branch;\n> +\n> +\trm = remote_get(remote);\n> +\tif (!rm)\n> +\t\treturn NULL;\n> +\n> +\tcurr_branch = branch_get(\"HEAD\");\n> +\tif (!curr_branch)\n> +\t\treturn NULL;\n> +\n> +\tif (curr_branch->remote != rm)\n> +\t\treturn NULL;\n> +\n> +\tif (!curr_branch->merge_nr)\n> +\t\treturn NULL;\n> +\n> +\treturn xstrdup(curr_branch->merge[0]->dst);\n> +}\n\nHmph, it is somewhat surprising that we do not have such a helper\nalready. Wouldn't we need this logic to implement $branch@{upstream}\nsyntax?\n\n> +/**\n> + * Derives the remote tracking branch from the remote and refspec.\n> + *\n> + * FIXME: The current implementation assumes the default mapping of\n> + * refs/heads/<branch_name> to refs/remotes/<remote_name>/<branch_name>.\n> + */\n> +static char *get_tracking_branch(const char *remote, const char *refspec)\n> +{\n\nThis does smell like an incomplete reimplementation of what\nget_fetch_map() knows how to do.\n\n> +/**\n> + * Given the repo and refspecs, sets fork_point to the point at which the\n> + * current branch forked from its remote tracking branch. Returns 0 on success,\n> + * -1 on failure.\n> + */\n> +static int get_rebase_fork_point(unsigned char fork_point[GIT_SHA1_RAWSZ],\n> +\t\tconst char *repo, const char *refspec)\n> +{\n> +...\n> +}\n\nThis function looks OK (the two get_*_branch() helpers it uses I am\nnot sure about though).\n\nSame comment on \"fork_point[]\" parameter's type applies here,\nthough.  While I do not mind if you used \"struct object_id\" to\nrepresent these object names, if you are sticking to the traditional\n\"unsigned char [20]\", then these should be \"unsigned char *\" to be\nconsistent with others.\n\n> +/**\n> + * Sets merge_base to the octopus merge base of curr_head, merge_head and\n> + * fork_point. Returns 0 if a merge base is found, 1 otherwise.\n> + */\n> +static int get_octopus_merge_base(unsigned char merge_base[GIT_SHA1_HEXSZ],\n> +\t\tunsigned char curr_head[GIT_SHA1_RAWSZ],\n> +\t\tunsigned char merge_head[GIT_SHA1_RAWSZ],\n> +\t\tunsigned char fork_point[GIT_SHA1_RAWSZ])\n> +{\n> +...\n> +}\n\nOK (and everything after this point looks good).\n"},{"id":"263432","messageId":"20150610055340.GA15753@vps892.directvps.nl","threadId":"39509","inReplyTo":"20150603102716.GA29730@vps892.directvps.nl","subject":"Re: [PATCH v2 17/19] pull --rebase: exit early when the working directory is dirty","fromName":"Kevin Daudt","fromEmail":"me@ikke.info","sentAt":"2015-06-10T05:53:40Z","receivedAt":"2015-06-10T05:53:40Z","isPatch":true,"sender":{"key":"me@ikke.info","avatar":"https://avatars.githubusercontent.com/u/135698?v=4"},"body":"On Wed, Jun 03, 2015 at 12:27:16PM +0200, Kevin Daudt wrote:\n> On Wed, Jun 03, 2015 at 02:49:01PM +0800, Paul Tan wrote:\n> > Re-implement the behavior introduced by f9189cf (pull --rebase: exit\n> > early when the working directory is dirty, 2008-05-21).\n> \n> When the option rebase.autoStash is set, it should not be necessary to\n> die in this case. See also this[1] patch I'm working on.\n> \n\nSorry, didn't notice it was you who reviewed my patch.\n"},{"id":"263436","messageId":"CACRoPnRVjNseXPOxKe4_3rnKXmAYeA80iCHY3o8euWA=EUX6hA@mail.gmail.com","threadId":"39509","inReplyTo":"xmqqr3pkwjz6.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 02/19] parse-options-cb: implement parse_opt_pass_argv_array()","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-10T07:11:01Z","receivedAt":"2015-06-10T07:11:01Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"On Wed, Jun 10, 2015 at 7:16 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Almost the same comment as 01/19 applies to this comment.\n>\n> I think it makes good sense to have two variants, one that lets the\n> last one win and pass only that last one (i.e. 01/19) and the other\n> that accumulates them into an argv_array (i.e. this one).  But it\n> feels iffy, given that the \"acculate\" version essentially creates an\n> array of (char *), to make \"the last one wins, leaving a single\n> string\" to use strbuf.  I'd find it much more understandable if 01/19\n> took (char **) as opt->value instead of a strbuf.\n\nI don't see how it feels iffy. The purpose of using strbufs (and\nargv_arrays) is to avoid error-prone manual memory management.\n\n> In any case, these two need to be added as a related pair to the API\n> documentation.\n\nOkay, I guess I could also add their macro functions as well.\n\nThanks,\nPaul\n"},{"id":"263446","messageId":"CACRoPnR5shi800KZ_Do5V469ZgZYAzUZKy9mNYNNRPF6HFqs_w@mail.gmail.com","threadId":"39509","inReplyTo":"xmqqk2vcuy10.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 15/19] pull: teach git pull about --rebase","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-10T07:55:26Z","receivedAt":"2015-06-10T07:55:26Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"On Wed, Jun 10, 2015 at 9:56 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Paul Tan <pyokagan@gmail.com> writes:\n>\n>> +enum rebase_type {\n>> +     REBASE_INVALID = -1,\n>> +     REBASE_FALSE = 0,\n>> +     REBASE_TRUE,\n>> +     REBASE_PRESERVE\n>> +};\n>> +\n>> +/**\n>> + * Parses the value of --rebase, branch.*.rebase or pull.rebase. If value is a\n>> + * false value, returns REBASE_FALSE. If value is a true value, returns\n>> + * REBASE_TRUE. If value is \"preserve\", returns REBASE_PRESERVE. Otherwise,\n>> + * returns -1 to signify an invalid value.\n>> + */\n>> +static enum rebase_type parse_config_rebase(const char *value)\n>> +{\n>> +     int v = git_config_maybe_bool(\"pull.rebase\", value);\n>> +     if (!v)\n>> +             return REBASE_FALSE;\n>> +     else if (v >= 0)\n>> +             return REBASE_TRUE;\n>\n> It is somewhat misleading to say \"v >= 0\" when you already use !v to\n> signal something else.  Perhaps \"else if (v > 0)\" is better?\n\nAh, right.\n\n>> +/**\n>> + * Returns remote's upstream branch for the current branch. If remote is NULL,\n>> + * the current branch's configured default remote is used. Returns NULL if\n>> + * `remote` does not name a valid remote, HEAD does not point to a branch,\n>> + * remote is not the branch's configured remote or the branch does not have any\n>> + * configured upstream branch.\n>> + */\n>> +static char *get_upstream_branch(const char *remote)\n>> +{\n>> +     struct remote *rm;\n>> +     struct branch *curr_branch;\n>> +\n>> +     rm = remote_get(remote);\n>> +     if (!rm)\n>> +             return NULL;\n>> +\n>> +     curr_branch = branch_get(\"HEAD\");\n>> +     if (!curr_branch)\n>> +             return NULL;\n>> +\n>> +     if (curr_branch->remote != rm)\n>> +             return NULL;\n>> +\n>> +     if (!curr_branch->merge_nr)\n>> +             return NULL;\n>> +\n>> +     return xstrdup(curr_branch->merge[0]->dst);\n>> +}\n>\n> Hmph, it is somewhat surprising that we do not have such a helper\n> already. Wouldn't we need this logic to implement $branch@{upstream}\n> syntax?\n\nRight, the @{upstream} syntax is implemented by branch_get_upstream()\nin remote.c. It, however, does not check to see if the branch's remote\nmatches what is provided on the command-line, so we still have to\nimplement this check ourselves, which means this helper function is\nstill required.\n\nI guess we could still use branch_get_upstream() in this function though.\n\n>> +/**\n>> + * Derives the remote tracking branch from the remote and refspec.\n>> + *\n>> + * FIXME: The current implementation assumes the default mapping of\n>> + * refs/heads/<branch_name> to refs/remotes/<remote_name>/<branch_name>.\n>> + */\n>> +static char *get_tracking_branch(const char *remote, const char *refspec)\n>> +{\n>\n> This does smell like an incomplete reimplementation of what\n> get_fetch_map() knows how to do.\n\nYeah, this is just a direct rewrite of get_remote_merge_branch() in\ngit-parse-remote.sh. Johannes pointed out[1] that\nremote_find_tracking() in remote.c does the exact same thing without\nthe assumption of the default fetch refmap. However, this would be a\nseparate modification on its own, so it may be better to do it in a\nseparate patch with regression tests. (e.g. what should we do if the\nrefspec dst is provided?)\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/269258/focus=269350\n\n>> +/**\n>> + * Given the repo and refspecs, sets fork_point to the point at which the\n>> + * current branch forked from its remote tracking branch. Returns 0 on success,\n>> + * -1 on failure.\n>> + */\n>> +static int get_rebase_fork_point(unsigned char fork_point[GIT_SHA1_RAWSZ],\n>> +             const char *repo, const char *refspec)\n>> +{\n>> +...\n>> +}\n>\n> This function looks OK (the two get_*_branch() helpers it uses I am\n> not sure about though).\n>\n> Same comment on \"fork_point[]\" parameter's type applies here,\n> though.  While I do not mind if you used \"struct object_id\" to\n> represent these object names, if you are sticking to the traditional\n> \"unsigned char [20]\", then these should be \"unsigned char *\" to be\n> consistent with others.\n\nOkay.\n\nThanks,\nPaul\n"},{"id":"263449","messageId":"xmqqy4jst2fp.fsf@gitster.dls.corp.google.com","threadId":"39509","inReplyTo":"CACRoPnRVjNseXPOxKe4_3rnKXmAYeA80iCHY3o8euWA=EUX6hA@mail.gmail.com","subject":"Re: [PATCH v2 02/19] parse-options-cb: implement parse_opt_pass_argv_array()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-10T08:03:54Z","receivedAt":"2015-06-10T08:03:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> I don't see how it feels iffy.\n\nThat is largely a taste thing.  And a good taste matters.\n\nWhat is iffy is to use strbuf as an external interface between the\nimplementation of the parse_opt_pass() API function and its users.\n\nI would expect that no users of the parse_opt_pass() would ever take\nadvantage of the fact that the value they are using is implemented\nas a strbuf, allowing them to use the family of functions in the\nstrbuf API to further manipulate it.  All users you added wants to\nuse a plain vanilla string, and only because you used strbuf as the\ninterface element, they have to say \"var.buf\" to get to the value\nthey want to use.\n\n> The purpose of using strbufs (and\n> argv_arrays) is to avoid error-prone manual memory management.\n\nIt is a good idea to use strbuf as an implementation detail of\nparse_opt_pass() function.  After all, safer, easier and efficient\nmanipulation of strings is why we added the strbuf API to the system\nin the first place.\n\nBut it is a different matter to expose that as an API element when\nthe recipient is not going to benefit.\n\nIn other words, callers of the API designed with a better taste\nwould look more like this:\n\n\tstatic const char *opt_diffstat;\n\n\tstatic struct option pull_options[] = {\n        \t...\n                { OPTION_CALLBACK, 'n', NULL, &opt_diffstat, NULL,\n\t\t  N_(\"do not show a diffstat at the end of the merge\"),\n\t\t  PARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_pass\n        \t},\n\t\t...\n\t};\n\n\tstatic int run_merge(void)\n        {\n        \t...\n                if (opt_diffstat)\n                \targv_array_push(&args, opt_diffstat);\n\t\t...\n\t}\n\nThat way, they do not have to define strbuf variables and\ndereference the value as opt_diffstat.buf.\n\nAnd the implementation of the API element is not all that different\nfrom what you wrote:\n\n\tint parse_opt_pass(const struct options *opt, const char *arg, int unset)\n\t{\n\t\tstruct strbuf sb = STRBUF_INIT;\n\t\tchar **value = opt->value;\n\n\t\tif (*value)\n                \tfree(*value);\n\t\tif (opt->long_name) {\n                \tstrbuf_addf(&sb, \"--%s%s\",\n                        \t    unset ? \"no-\" : \"\",\n                                    opt->long_name);\n\t\t\t...\n                }\n                ...\n                *value = strbuf_detach(&sb, NULL);\n\t\treturn 0;\n\t}\n"},{"id":"263467","messageId":"xmqqr3pjocgm.fsf@gitster.dls.corp.google.com","threadId":"39509","inReplyTo":"xmqqsia0uzaa.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 11/19] pull: check if in unresolved merge state","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-10T14:38:33Z","receivedAt":"2015-06-10T14:38:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Paul Tan <pyokagan@gmail.com> writes:\n>\n>> @@ -422,6 +423,14 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n>>  \n>>  \tparse_repo_refspecs(argc, argv, &repo, &refspecs);\n>>  \n>> +\tgit_config(git_default_config, NULL);\n>> +\n>> +\tif (read_cache_unmerged())\n>> +\t\tdie_resolve_conflict(\"Pull\");\n>> +\n>> +\tif (file_exists(git_path(\"MERGE_HEAD\")))\n>> +\t\tdie_conclude_merge();\n>> +\n>>  \tif (!opt_ff.len)\n>>  \t\tconfig_get_ff(&opt_ff);\n>\n> Hmph.\n>\n> If you are going to do the git_config() call yourself, it might make\n> more sense to define git_pull_config() callback and parse the pull.ff\n> yourself, updating the use of the lazy git_config_get_value() API you\n> introduced in patch 10/19.\n>\n> The above \"might\" is stronger than my usual \"might\"; I am undecided\n> yet before reading the remainder of the series.\n\nLet me clarify the above with s/stronger/with much less certainty/;\n"},{"id":"263468","messageId":"xmqqmw07oc72.fsf@gitster.dls.corp.google.com","threadId":"39509","inReplyTo":"CACRoPnR5shi800KZ_Do5V469ZgZYAzUZKy9mNYNNRPF6HFqs_w@mail.gmail.com","subject":"Re: [PATCH v2 15/19] pull: teach git pull about --rebase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-10T14:44:17Z","receivedAt":"2015-06-10T14:44:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n>> Hmph, it is somewhat surprising that we do not have such a helper\n>> already. Wouldn't we need this logic to implement $branch@{upstream}\n>> syntax?\n>\n> Right, the @{upstream} syntax is implemented by branch_get_upstream()\n> in remote.c. It, however, does not check to see if the branch's remote\n> matches what is provided on the command-line, so we still have to\n> implement this check ourselves, which means this helper function is\n> still required.\n>\n> I guess we could still use branch_get_upstream() in this function though.\n\nIt is entirely expected that existing function may not do exactly\nwhat the new caller you introduce might want to do, or may do more\nthan what it wants.  That is where refactoring of existing code\ncomes in.\n\nIt somewhat feels strange that you have to write more than \"shim\"\ncode to glue existing helpers and API functions together to\nre-implement what a scripted Porcelain is already doing, though.\nIt can't be that git-pull.sh implements this logic as shell script,\nand it must be asking existing code in Git to do what the callers\nyou added for this function would want to do, right?  That suggests\nthat we must have enough logic already in C.\n"},{"id":"263474","messageId":"CACRoPnSQd_YEp4mNFbqDZtBJGztkHLXWbDojcAJRNvpynFRCew@mail.gmail.com","threadId":"39509","inReplyTo":"xmqqr3pjocgm.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 11/19] pull: check if in unresolved merge state","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-10T15:12:06Z","receivedAt":"2015-06-10T15:12:06Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"On Wed, Jun 10, 2015 at 10:38 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Paul Tan <pyokagan@gmail.com> writes:\n>>\n>>> @@ -422,6 +423,14 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n>>>\n>>>      parse_repo_refspecs(argc, argv, &repo, &refspecs);\n>>>\n>>> +    git_config(git_default_config, NULL);\n>>> +\n>>> +    if (read_cache_unmerged())\n>>> +            die_resolve_conflict(\"Pull\");\n>>> +\n>>> +    if (file_exists(git_path(\"MERGE_HEAD\")))\n>>> +            die_conclude_merge();\n>>> +\n>>>      if (!opt_ff.len)\n>>>              config_get_ff(&opt_ff);\n>>\n>> Hmph.\n>>\n>> If you are going to do the git_config() call yourself, it might make\n>> more sense to define git_pull_config() callback and parse the pull.ff\n>> yourself, updating the use of the lazy git_config_get_value() API you\n>> introduced in patch 10/19.\n>>\n>> The above \"might\" is stronger than my usual \"might\"; I am undecided\n>> yet before reading the remainder of the series.\n>\n> Let me clarify the above with s/stronger/with much less certainty/;\n\nUh, I have no idea what a \"strong might\" or a \"less certain might\" is. :-/\n\nParsing all the config values with a git_config() callback function is\ncertainly possible, but I was under the impression that we are moving\ntowards migrating use of all the git_config() callback loops to the\ngit_config_get_X() API.\n\nIn this case though, we have to use git_config() to initialize the\nadvice.resolveConflict config setting, but I don't see why it must be\nin conflict with the above goal.\n\nThanks,\nPaul\n"},{"id":"263484","messageId":"CACRoPnSgVPVDTC=nY6FQHoVRPF1HZhRYXaKoQ1oJrJCoG1W8xg@mail.gmail.com","threadId":"39509","inReplyTo":"xmqqmw07oc72.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 15/19] pull: teach git pull about --rebase","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-10T15:35:18Z","receivedAt":"2015-06-10T15:35:18Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"On Wed, Jun 10, 2015 at 10:44 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Paul Tan <pyokagan@gmail.com> writes:\n>\n>>> Hmph, it is somewhat surprising that we do not have such a helper\n>>> already. Wouldn't we need this logic to implement $branch@{upstream}\n>>> syntax?\n>>\n>> Right, the @{upstream} syntax is implemented by branch_get_upstream()\n>> in remote.c. It, however, does not check to see if the branch's remote\n>> matches what is provided on the command-line, so we still have to\n>> implement this check ourselves, which means this helper function is\n>> still required.\n>>\n>> I guess we could still use branch_get_upstream() in this function though.\n>\n> It is entirely expected that existing function may not do exactly\n> what the new caller you introduce might want to do, or may do more\n> than what it wants.  That is where refactoring of existing code\n> comes in.\n>\n> It somewhat feels strange that you have to write more than \"shim\"\n> code to glue existing helpers and API functions together to\n> re-implement what a scripted Porcelain is already doing, though.\n> It can't be that git-pull.sh implements this logic as shell script,\n> and it must be asking existing code in Git to do what the callers\n> you added for this function would want to do, right?\n\nNot git-pull.sh, but get_remote_merge_branch() git-parse-remote.sh.\nThe shell code that get_upstream_branch() in this patch implements is:\n\n    0|1)\n        origin=\"$1\"\n        default=$(get_default_remote)\n        test -z \"$origin\" && origin=$default\n        curr_branch=$(git symbolic-ref -q HEAD) &&\n        [ \"$origin\" = \"$default\" ] &&\n\n^ This here is where it checks to see if the branch's configured\nremote matches the remote provided on the command line.\n\n        echo $(git for-each-ref --format='%(upstream)' $curr_branch)\n        ;;\n\n^ While here it calls git to get the upstream branch, which is\nimplemented by branch_get_upstream() on the C side.\n\nSo yes, we can use branch_get_upstream(), but we still need to\nimplement some code on top.\n\nJust to add on, the shell code that get_tracking_branch() in this\npatch implements is:\n\n    *)\n        repo=$1\n        shift\n        ref=$1\n        # FIXME: It should return the tracking branch\n        #        Currently only works with the default mapping\n        case \"$ref\" in\n        +*)\n        ref=$(expr \"z$ref\" : 'z+\\(.*\\)')\n        ;;\n        esac\n        expr \"z$ref\" : 'z.*:' >/dev/null || ref=\"${ref}:\"\n        remote=$(expr \"z$ref\" : 'z\\([^:]*\\):')\n        case \"$remote\" in\n        '' | HEAD ) remote=HEAD ;;\n        heads/*) remote=${remote#heads/} ;;\n        refs/heads/*) remote=${remote#refs/heads/} ;;\n        refs/* | tags/* | remotes/* ) remote=\n        esac\n        [ -n \"$remote\" ] && case \"$repo\" in\n        .)\n            echo \"refs/heads/$remote\"\n            ;;\n        *)\n            echo \"refs/remotes/$repo/$remote\"\n            ;;\n        esac\n\nso it's more or less a direct translation of the shell script, and we\ncan be sure it will have the same behavior. I'm definitely in favor of\nswitching this to use remote_find_tracking(), the question is whether\nwe want to do it in this patch or in a future patch on top.\n\nThanks,\nPaul\n"},{"id":"263495","messageId":"xmqqk2vbmth7.fsf@gitster.dls.corp.google.com","threadId":"39509","inReplyTo":"CACRoPnSgVPVDTC=nY6FQHoVRPF1HZhRYXaKoQ1oJrJCoG1W8xg@mail.gmail.com","subject":"Re: [PATCH v2 15/19] pull: teach git pull about --rebase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-10T16:13:56Z","receivedAt":"2015-06-10T16:13:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> so it's more or less a direct translation of the shell script, and we\n> can be sure it will have the same behavior. I'm definitely in favor of\n> switching this to use remote_find_tracking(), the question is whether\n> we want to do it in this patch or in a future patch on top.\n\nAh, OK.\n\nI then agree that the topic should be a straight-forward literal\ntranslation of the current scripted Porcelain.  Switching\nimplementation detail to share more code can and should come on top\nwith more careful consideration, as that step can change the\nsemantics by mistake.\n\nThanks.\n"},{"id":"263507","messageId":"xmqq381zmqon.fsf@gitster.dls.corp.google.com","threadId":"39509","inReplyTo":"CACRoPnSQd_YEp4mNFbqDZtBJGztkHLXWbDojcAJRNvpynFRCew@mail.gmail.com","subject":"Re: [PATCH v2 11/19] pull: check if in unresolved merge state","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-10T17:14:16Z","receivedAt":"2015-06-10T17:14:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> On Wed, Jun 10, 2015 at 10:38 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>> If you are going to do the git_config() call yourself, it might make\n>>> more sense to define git_pull_config() callback and parse the pull.ff\n>>> yourself, updating the use of the lazy git_config_get_value() API you\n>>> introduced in patch 10/19.\n>>>\n>>> The above \"might\" is stronger than my usual \"might\"; I am undecided\n>>> yet before reading the remainder of the series.\n>>\n>> Let me clarify the above with s/stronger/with much less certainty/;\n>\n> Uh, I have no idea what a \"strong might\" or a \"less certain might\" is. :-/\n>\n> Parsing all the config values with a git_config() callback function is\n> certainly possible, but I was under the impression that we are moving\n> towards migrating use of all the git_config() callback loops to the\n> git_config_get_X() API.\n>\n> In this case though, we have to use git_config() to initialize the\n> advice.resolveConflict config setting, but I don't see why it must be\n> in conflict with the above goal.\n\nI (or at least some part of me) actually view git_config_get_*() as\n\"if you are only going to peek a few variables, you do not have to\ndo the looping yourself\" convenience, which leads me (or at least\nthat part of me) to say \"if you are doing the looping anyway, you\nmay be better off picking up the variables you care about yourself\nin that loop\".\n\nBy calling git_config() before calling any git_config_get_*()\nfunctions, you would be priming the cache layer with the entire\ncontents of the configuration files anyway, so later calls to\ngit_config_get_*() will read from that cache, so there is no\nperformance penalty in mixing the two methods to access\nconfiguration data.  That is why I felt less certain that the\nsuggestion to stick to one method (instead of mixing the two) is a\ngood thing to do (hence \"less certain 'might'\").\n"},{"id":"263542","messageId":"xmqqy4jrgob3.fsf@gitster.dls.corp.google.com","threadId":"39509","inReplyTo":"1433314143-4478-16-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 15/19] pull: teach git pull about --rebase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-10T23:02:08Z","receivedAt":"2015-06-10T23:02:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> +/**\n> + * Returns remote's upstream branch for the current branch. If remote is NULL,\n> + * the current branch's configured default remote is used. Returns NULL if\n> + * `remote` does not name a valid remote, HEAD does not point to a branch,\n> + * remote is not the branch's configured remote or the branch does not have any\n> + * configured upstream branch.\n> + */\n> +static char *get_upstream_branch(const char *remote)\n> +{\n> +\tstruct remote *rm;\n> +\tstruct branch *curr_branch;\n> +\n> +\trm = remote_get(remote);\n> +\tif (!rm)\n> +\t\treturn NULL;\n> +\n> +\tcurr_branch = branch_get(\"HEAD\");\n> +\tif (!curr_branch)\n> +\t\treturn NULL;\n> +\n> +\tif (curr_branch->remote != rm)\n> +\t\treturn NULL;\n> +\n> +\tif (!curr_branch->merge_nr)\n> +\t\treturn NULL;\n> +\n> +\treturn xstrdup(curr_branch->merge[0]->dst);\n> +}\n\nThis part needs to be rebased, as branch->remote no longer exists in\nthe recent world order.\n"},{"id":"263719","messageId":"CACRoPnR6+xBr5T+KGe5fFoi9GEtG5LsjZ9nYVqiTz1=R+7cZ6w@mail.gmail.com","threadId":"39509","inReplyTo":"xmqqeglkwi4u.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 09/19] pull: error on no merge candidates","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-13T05:52:57Z","receivedAt":"2015-06-13T05:52:57Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"On Wed, Jun 10, 2015 at 7:56 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Paul Tan <pyokagan@gmail.com> writes:\n>\n>>  /**\n>> + * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge\n>> + * into merge_heads.\n>> + */\n>\n> Hmph, I vaguely recall doing that in C elsewhere already, even\n> though I do not remember where offhand...\n\nRight, handle_fetch_head() in builtin/merge.c. It looks up the commit\nIDs into commit objects though, which is not required for git-pull. We\nonly need the list of hashes.\n\n>> +static void get_merge_heads(struct sha1_array *merge_heads)\n>> +{\n>> +     const char *filename = git_path(\"FETCH_HEAD\");\n>> +     FILE *fp;\n>> +     struct strbuf sb = STRBUF_INIT;\n>> +     unsigned char sha1[GIT_SHA1_RAWSZ];\n>> +\n>> +     if (!(fp = fopen(filename, \"r\")))\n>> +             die_errno(_(\"could not open '%s' for reading\"), filename);\n>> +     while(strbuf_getline(&sb, fp, '\\n') != EOF) {\n>\n> Missing SP after \"while\"\n\nOK\n\n>> +             if (get_sha1_hex(sb.buf, sha1))\n>> +                     continue;  /* invalid line: does not start with SHA1 */\n>> +             if (starts_with(sb.buf + GIT_SHA1_HEXSZ, \"\\tnot-for-merge\"))\n>\n> Look for \"\\tnot-for-merge\\t\" instead?\n\nRight, it's better to be stricter.\n\nThanks,\nPaul\n"},{"id":"263784","messageId":"CACRoPnTVXCGs6gwYxPLZfD-jK3qfjVzXhNSArsO-JA+a406SHg@mail.gmail.com","threadId":"39509","inReplyTo":"xmqq381zmqon.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 11/19] pull: check if in unresolved merge state","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-14T07:44:59Z","receivedAt":"2015-06-14T07:44:59Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"On Thu, Jun 11, 2015 at 1:14 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> I (or at least some part of me) actually view git_config_get_*() as\n> \"if you are only going to peek a few variables, you do not have to\n> do the looping yourself\" convenience, which leads me (or at least\n> that part of me) to say \"if you are doing the looping anyway, you\n> may be better off picking up the variables you care about yourself\n> in that loop\".\n\nNot just a convenience, but I personally find that callback functions\nusually leads to code that is harder to understand because of the use\nof static/global variables to preserve state and the fact that it is\nharder to break it up into smaller functions to reason about. This is\njust a matter of taste though, so I don't have strong feelings about\nit.\n\nBesides, there is a greater technical reason why we should not use\ngit_config(): It essentially performs a linear search[1], while the\ngit_config_get_*() functions do a constant-time lookup. Ideally, we\nshould remove uses of git_config() for users that have no need to\niterate over all the configuration entries.\n\n[1] Since we do a strcmp() for every supported config setting, for\nevery config entry.\n\nI should emphasize that the call to git_config(git_default_config,\nNULL) is just a workaround to load the advice.* settings. In fact,\ngit_default_config() only peeks at a few config settings anyway, and\ncan be re-written to not iterate over all the user's config entries.\nAs such, I don't see why the builtin/pull.c code should be written to\nsupport the git_config() way of doing things, even if we have to use\nthe workaround of calling git_config(). It's like saying: we have a\nbad solution, now let's make it worse ;-)\n\n> By calling git_config() before calling any git_config_get_*()\n> functions, you would be priming the cache layer with the entire\n> contents of the configuration files anyway, so later calls to\n> git_config_get_*() will read from that cache, so there is no\n> performance penalty in mixing the two methods to access\n> configuration data.  That is why I felt less certain that the\n> suggestion to stick to one method (instead of mixing the two) is a\n> good thing to do (hence \"less certain 'might'\").\n\nRight, although I think that the performance penalty due to using\ngit_config() is greater, and switching all the git_config_get_*()\ncalls to use it would just make it worse.\n\nBy the way, I got the idea that git development was moving towards\nreplacing use of git_config() from 5801d3b (builtin/gc.c: replace\n`git_config()` with `git_config_get_*()` family, 2014-08-07).\n\nThanks,\nPaul\n"}]}