[PATCH v3 16/19] pull: configure --rebase via branch..rebase or pull.rebase
Since cd67e4d (Teach 'git pull' about --rebase, 2007-11-28), fetch+rebase could be set by default by defining the config variable branch..rebase. This setting can be overriden on the command line by --rebase and --no-rebase. Since 6b37dff (pull: introduce a pull.rebase option to enable --rebase, 2011-11-06), git-pull --rebase can also be configured via the pull.rebase configuration option. Re-implement support for these two configuration settings by introducing config_get_rebase() which is called before parse_options() to set the default value of opt_rebase. Helped-by: Stefan Beller Signed-off-by: Paul Tan --- Notes: v3 * Now that parse_config_rebase() takes care of the die()-ing and error()-ing, we only need one function again. Yay! * The free()s is ugly though. Ideally, I would like to have a xstrfmt() function that returns a static buffer. * We now don't lookup the pull.rebase config if the --rebase option is provided on the command-line. builtin/pull.c | 34 +- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/builtin/pull.c b/builtin/pull.c index 8915947..e4098d0 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -72,7 +72,7 @@ static int opt_verbosity; static char *opt_progress; /* Options passed to git-merge or git-rebase */ -static enum rebase_type opt_rebase; +static enum rebase_type opt_rebase = -1; static char *opt_diffstat; static char *opt_log; static char *opt_squash; @@ -266,6 +266,35 @@ static const char *config_get_ff(void) } /** + * Returns the default configured value for --rebase. It first looks for the + * value of "branch.$curr_branch.rebase", where $curr_branch is the current + * branch, and if HEAD is detached or the configuration key does not exist, + * looks for the value of "pull.rebase". If both configuration keys do not + * exist, returns REBASE_FALSE. + */ +static enum rebase_type config_get_rebase(void) +{ + struct branch *curr_branch = branch_get("HEAD"); + const char *value; + + if (curr_branch) { + char *key = xstrfmt("branch.%s.rebase", curr_branch->name); + + if (!git_config_get_value(key, &value)) { + free(key); + return parse_config_rebase(key, value, 1); + } + + free(key); + } + + if (!git_config_get_value("pull.rebase", &value)) + return parse_config_rebase("pull.rebase", value, 1); + + return REBASE_FALSE; +} + +/** * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge * into merge_heads. */ @@ -707,6 +736,9 @@ int cmd_pull(int argc, const char **argv, const char *prefix) if (!opt_ff) opt_ff = xstrdup_or_null(config_get_ff()); + if (opt_rebase < 0) + opt_rebase = config_get_rebase(); + git_config(git_default_config, NULL); if (read_cache_unmerged()) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 14/19] pull: set reflog message
f947413 (Use GIT_REFLOG_ACTION environment variable instead., 2006-12-28) established git-pull's method for setting the reflog message, which is to set the environment variable GIT_REFLOG_ACTION to the evaluation of "pull${1+ $*}" if it has not already been set. Re-implement this behavior. Helped-by: Junio C Hamano Signed-off-by: Paul Tan --- builtin/pull.c | 22 ++ 1 file changed, 22 insertions(+) diff --git a/builtin/pull.c b/builtin/pull.c index a2dd0ba..a2c900e 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -169,6 +169,25 @@ static void argv_push_force(struct argv_array *arr) } /** + * Sets the GIT_REFLOG_ACTION environment variable to the concatenation of argv + */ +static void set_reflog_message(int argc, const char **argv) +{ + int i; + struct strbuf msg = STRBUF_INIT; + + for (i = 0; i < argc; i++) { + if (i) + strbuf_addch(&msg, ' '); + strbuf_addstr(&msg, argv[i]); + } + + setenv("GIT_REFLOG_ACTION", msg.buf, 0); + + strbuf_release(&msg); +} + +/** * If pull.ff is unset, returns NULL. If pull.ff is "true", returns "--ff". If * pull.ff is "false", returns "--no-ff". If pull.ff is "only", returns * "--ff-only". Otherwise, if pull.ff is set to an invalid value, die with an @@ -443,6 +462,9 @@ int cmd_pull(int argc, const char **argv, const char *prefix) die_errno("could not exec %s", path); } + if (!getenv("GIT_REFLOG_ACTION")) + set_reflog_message(argc, argv); + argc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0); parse_repo_refspecs(argc, argv, &repo, &refspecs); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 18/19] pull --rebase: error on no merge candidate cases
Tweak the error messages printed by die_no_merge_candidates() to take into account that we may be "rebasing against" rather than "merging with". Signed-off-by: Paul Tan --- builtin/pull.c | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/builtin/pull.c b/builtin/pull.c index a379c1f..b78c67c 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -430,7 +430,10 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs const char *remote = curr_branch ? curr_branch->remote_name : NULL; if (*refspecs) { - fprintf_ln(stderr, _("There are no candidates for merging among the refs that you just fetched.")); + if (opt_rebase) + fprintf_ln(stderr, _("There is no candidate for rebasing against among the refs that you just fetched.")); + else + fprintf_ln(stderr, _("There are no candidates for merging among the refs that you just fetched.")); fprintf_ln(stderr, _("Generally this means that you provided a wildcard refspec which had no\n" "matches on the remote end.")); } else if (repo && curr_branch && (!remote || strcmp(repo, remote))) { @@ -440,7 +443,10 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs repo); } else if (!curr_branch) { fprintf_ln(stderr, _("You are not currently on a branch.")); - fprintf_ln(stderr, _("Please specify which branch you want to merge with.")); + if (opt_rebase) + fprintf_ln(stderr, _("Please specify which branch you want to rebase against.")); + else + fprintf_ln(stderr, _("Please specify which branch you want to merge with.")); fprintf_ln(stderr, _("See git-pull(1) for details.")); fprintf(stderr, "\n"); fprintf_ln(stderr, "git pull "); @@ -452,7 +458,10 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs remote_name = ""; fprintf_ln(stderr, _("There is no tracking information for the current branch.")); - fprintf_ln(stderr, _("Please specify which branch you want to merge with.")); + if (opt_rebase) + fprintf_ln(stderr, _("Please specify which branch you want to rebase against.")); + else + fprintf_ln(stderr, _("Please specify which branch you want to merge with.")); fprintf_ln(stderr, _("See git-pull(1) for details.")); fprintf(stderr, "\n"); fprintf_ln(stderr, "git pull "); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 08/19] pull: pass git-fetch's options to git-fetch
Since eb2a8d9 (pull: handle git-fetch's options as well, 2015-06-02), git-pull knows about and handles git-fetch's options, passing them to git-fetch. Re-implement this behavior. Since 29609e6 (pull: do nothing on --dry-run, 2010-05-25) git-pull supported the --dry-run option, exiting after git-fetch if --dry-run is set. Re-implement this behavior. Signed-off-by: Paul Tan --- builtin/pull.c | 95 ++ 1 file changed, 95 insertions(+) diff --git a/builtin/pull.c b/builtin/pull.c index 0442da9..731e2a6 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -32,6 +32,21 @@ static struct argv_array opt_strategies = ARGV_ARRAY_INIT; static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT; static char *opt_gpg_sign; +/* Options passed to git-fetch */ +static char *opt_all; +static char *opt_append; +static char *opt_upload_pack; +static int opt_force; +static char *opt_tags; +static char *opt_prune; +static char *opt_recurse_submodules; +static int opt_dry_run; +static char *opt_keep; +static char *opt_depth; +static char *opt_unshallow; +static char *opt_update_shallow; +static char *opt_refmap; + static struct option pull_options[] = { /* Shared options */ OPT__VERBOSITY(&opt_verbosity), @@ -82,6 +97,46 @@ static struct option pull_options[] = { N_("GPG sign commit"), PARSE_OPT_OPTARG), + /* Options passed to git-fetch */ + OPT_GROUP(N_("Options related to fetching")), + OPT_PASSTHRU(0, "all", &opt_all, 0, + N_("fetch from all remotes"), + PARSE_OPT_NOARG), + OPT_PASSTHRU('a', "append", &opt_append, 0, + N_("append to .git/FETCH_HEAD instead of overwriting"), + PARSE_OPT_NOARG), + OPT_PASSTHRU(0, "upload-pack", &opt_upload_pack, N_("path"), + N_("path to upload pack on remote end"), + 0), + OPT__FORCE(&opt_force, N_("force overwrite of local branch")), + OPT_PASSTHRU('t', "tags", &opt_tags, 0, + N_("fetch all tags and associated objects"), + PARSE_OPT_NOARG), + OPT_PASSTHRU('p', "prune", &opt_prune, 0, + N_("prune remote-tracking branches no longer on remote"), + PARSE_OPT_NOARG), + OPT_PASSTHRU(0, "recurse-submodules", &opt_recurse_submodules, + N_("on-demand"), + N_("control recursive fetching of submodules"), + PARSE_OPT_OPTARG), + OPT_BOOL(0, "dry-run", &opt_dry_run, + N_("dry run")), + OPT_PASSTHRU('k', "keep", &opt_keep, 0, + N_("keep downloaded pack"), + PARSE_OPT_NOARG), + OPT_PASSTHRU(0, "depth", &opt_depth, N_("depth"), + N_("deepen history of shallow clone"), + 0), + OPT_PASSTHRU(0, "unshallow", &opt_unshallow, 0, + N_("convert to a complete repository"), + PARSE_OPT_NONEG | PARSE_OPT_NOARG), + OPT_PASSTHRU(0, "update-shallow", &opt_update_shallow, 0, + N_("accept refs that update .git/shallow"), + PARSE_OPT_NOARG), + OPT_PASSTHRU(0, "refmap", &opt_refmap, N_("refmap"), + N_("specify fetch refmap"), + PARSE_OPT_NONEG), + OPT_END() }; @@ -100,6 +155,16 @@ static void argv_push_verbosity(struct argv_array *arr) } /** + * Pushes "-f" switches into arr to match the opt_force level. + */ +static void argv_push_force(struct argv_array *arr) +{ + int force = opt_force; + while (force-- > 0) + argv_array_push(arr, "-f"); +} + +/** * Parses argv into [ [...]], returning their values in `repo` * as a string and `refspecs` as a null-terminated array of strings. If `repo` * is not provided in argv, it is set to NULL. @@ -131,6 +196,33 @@ static int run_fetch(const char *repo, const char **refspecs) if (opt_progress) argv_array_push(&args, opt_progress); + /* Options passed to git-fetch */ + if (opt_all) + argv_array_push(&args, opt_all); + if (opt_append) + argv_array_push(&args, opt_append); + if (opt_upload_pack) + argv_array_push(&args, opt_upload_pack); + argv_push_force(&args); + if (opt_tags) + argv_array_push(&args, opt_tags); + if (opt_prune) + argv_array_push(&args, opt_prune); + if (opt_recurse_submodules) +
[PATCH v3 10/19] pull: support pull.ff config
Since b814da8 (pull: add pull.ff configuration, 2014-01-15), git-pull.sh would lookup the configuration value of "pull.ff", and set the flag "--ff" if its value is "true", "--no-ff" if its value is "false" and "--ff-only" if its value is "only". Re-implement this behavior. Signed-off-by: Paul Tan --- Notes: v3 * style fixes builtin/pull.c | 29 + 1 file changed, 29 insertions(+) diff --git a/builtin/pull.c b/builtin/pull.c index 83691fc..0ddb964 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -167,6 +167,32 @@ static void argv_push_force(struct argv_array *arr) } /** + * If pull.ff is unset, returns NULL. If pull.ff is "true", returns "--ff". If + * pull.ff is "false", returns "--no-ff". If pull.ff is "only", returns + * "--ff-only". Otherwise, if pull.ff is set to an invalid value, die with an + * error. + */ +static const char *config_get_ff(void) +{ + const char *value; + + if (git_config_get_value("pull.ff", &value)) + return NULL; + + switch (git_config_maybe_bool("pull.ff", value)) { + case 0: + return "--no-ff"; + case 1: + return "--ff"; + } + + if (!strcmp(value, "only")) + return "--ff-only"; + + die(_("Invalid value for pull.ff: %s"), value); +} + +/** * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge * into merge_heads. */ @@ -397,6 +423,9 @@ int cmd_pull(int argc, const char **argv, const char *prefix) parse_repo_refspecs(argc, argv, &repo, &refspecs); + if (!opt_ff) + opt_ff = xstrdup_or_null(config_get_ff()); + if (run_fetch(repo, refspecs)) return 1; -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 05/19] pull: implement fetch + merge
Implement the fetch + merge functionality of git-pull, by first running git-fetch with the repo and refspecs provided on the command line, then running git-merge on FETCH_HEAD to merge the fetched refs into the current branch. Helped-by: Junio C Hamano Signed-off-by: Paul Tan --- Notes: v3 * Catch bug where there is are refspecs but no repo. builtin/pull.c | 62 +- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/builtin/pull.c b/builtin/pull.c index cabeed4..9157536 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -9,8 +9,10 @@ #include "builtin.h" #include "parse-options.h" #include "exec_cmd.h" +#include "run-command.h" static const char * const pull_usage[] = { + N_("git pull [options] [ [...]]"), NULL }; @@ -18,8 +20,61 @@ static struct option pull_options[] = { OPT_END() }; +/** + * Parses argv into [ [...]], returning their values in `repo` + * as a string and `refspecs` as a null-terminated array of strings. If `repo` + * is not provided in argv, it is set to NULL. + */ +static void parse_repo_refspecs(int argc, const char **argv, const char **repo, + const char ***refspecs) +{ + if (argc > 0) { + *repo = *argv++; + argc--; + } else + *repo = NULL; + *refspecs = argv; +} + +/** + * Runs git-fetch, returning its exit status. `repo` and `refspecs` are the + * repository and refspecs to fetch, or NULL if they are not provided. + */ +static int run_fetch(const char *repo, const char **refspecs) +{ + struct argv_array args = ARGV_ARRAY_INIT; + int ret; + + argv_array_pushl(&args, "fetch", "--update-head-ok", NULL); + if (repo) { + argv_array_push(&args, repo); + argv_array_pushv(&args, refspecs); + } else if (*refspecs) + die("BUG: refspecs without repo?"); + ret = run_command_v_opt(args.argv, RUN_GIT_CMD); + argv_array_clear(&args); + return ret; +} + +/** + * Runs git-merge, returning its exit status. + */ +static int run_merge(void) +{ + int ret; + struct argv_array args = ARGV_ARRAY_INIT; + + argv_array_pushl(&args, "merge", NULL); + argv_array_push(&args, "FETCH_HEAD"); + ret = run_command_v_opt(args.argv, RUN_GIT_CMD); + argv_array_clear(&args); + return ret; +} + int cmd_pull(int argc, const char **argv, const char *prefix) { + const char *repo, **refspecs; + if (!getenv("_GIT_USE_BUILTIN_PULL")) { const char *path = mkpath("%s/git-pull", git_exec_path()); @@ -29,5 +84,10 @@ int cmd_pull(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0); - return 0; + parse_repo_refspecs(argc, argv, &repo, &refspecs); + + if (run_fetch(repo, refspecs)) + return 1; + + return run_merge(); } -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 03/19] argv-array: implement argv_array_pushv()
When we have a null-terminated array, it would be useful to convert it or append it to an argv_array for further manipulation. Implement argv_array_pushv() which will push a null-terminated array of strings on to an argv_array. Signed-off-by: Paul Tan --- Documentation/technical/api-argv-array.txt | 3 +++ argv-array.c | 6 ++ argv-array.h | 1 + 3 files changed, 10 insertions(+) diff --git a/Documentation/technical/api-argv-array.txt b/Documentation/technical/api-argv-array.txt index 1a79781..8076172 100644 --- a/Documentation/technical/api-argv-array.txt +++ b/Documentation/technical/api-argv-array.txt @@ -46,6 +46,9 @@ Functions Format a string and push it onto the end of the array. This is a convenience wrapper combining `strbuf_addf` and `argv_array_push`. +`argv_array_pushv`:: + Push a null-terminated array of strings onto the end of the array. + `argv_array_pop`:: Remove the final element from the array. If there are no elements in the array, do nothing. diff --git a/argv-array.c b/argv-array.c index 256741d..eaed477 100644 --- a/argv-array.c +++ b/argv-array.c @@ -49,6 +49,12 @@ void argv_array_pushl(struct argv_array *array, ...) va_end(ap); } +void argv_array_pushv(struct argv_array *array, const char **argv) +{ + for (; *argv; argv++) + argv_array_push(array, *argv); +} + void argv_array_pop(struct argv_array *array) { if (!array->argc) diff --git a/argv-array.h b/argv-array.h index c65e6e8..a2fa0aa 100644 --- a/argv-array.h +++ b/argv-array.h @@ -17,6 +17,7 @@ __attribute__((format (printf,2,3))) void argv_array_pushf(struct argv_array *, const char *fmt, ...); LAST_ARG_MUST_BE_NULL void argv_array_pushl(struct argv_array *, ...); +void argv_array_pushv(struct argv_array *, const char **); void argv_array_pop(struct argv_array *); void argv_array_clear(struct argv_array *); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 01/19] parse-options-cb: implement parse_opt_passthru()
Certain git commands, such as git-pull, are simply wrappers around other git commands like git-fetch, git-merge and git-rebase. As such, these wrapper commands will typically need to "pass through" command-line options of the commands they wrap. Implement the parse_opt_passthru() parse-options callback, which will reconstruct the command-line option into an char* string, such that it can be passed to another git command. Helped-by: Johannes Schindelin Helped-by: Junio C Hamano Helped-by: Stefan Beller Signed-off-by: Paul Tan --- Notes: v3 * Reverted back to returning newly-allocated strings. Junio raised the concern that it may not be such a good idea to burden the users of the API to always use X.buf to access the string. Personally I believe that memory management safety is usually more important, but I don't have strong feelings about this. * Extracted out the "option reconstruction" logic into a private function so that it can be shared with parse_opt_passthru_argv() in the next patch. * Introduced the OPT_PASSTHRU() macro and documented it in Documentation/technical/api-parse-options.txt. This macro relieves the user of having to specify OPTION_CALLBACK and parse_opt_passthru() for every option. * The function used to be named parse_opt_pass_strbuf() to save horizontal space, but then again parse_opt_passthru() is probably a better name. * Added comment to the docstring that the callback should only be used for options where the last one wins. Documentation/technical/api-parse-options.txt | 7 parse-options-cb.c| 49 +++ parse-options.h | 3 ++ 3 files changed, 59 insertions(+) diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt index 1f2db31..85d10ab 100644 --- a/Documentation/technical/api-parse-options.txt +++ b/Documentation/technical/api-parse-options.txt @@ -212,6 +212,13 @@ There are some macros to easily define options: Use it to hide deprecated options that are still to be recognized and ignored silently. +`OPT_PASSTHRU(short, long, &char_var, arg_str, description, flags)`:: + Introduce an option that will be reconstructed into a char* string, + which must be initialized to NULL. This is useful when you need to + pass the command-line option to another command. Any previous value + will be overwritten, so this should only be used for options where + the last one specified on the command line wins. + The last element of the array must be `OPT_END()`. diff --git a/parse-options-cb.c b/parse-options-cb.c index be8c413..68bc593 100644 --- a/parse-options-cb.c +++ b/parse-options-cb.c @@ -134,3 +134,52 @@ int parse_opt_noop_cb(const struct option *opt, const char *arg, int unset) { return 0; } + +/** + * Recreates the command-line option in the strbuf. + */ +static int recreate_opt(struct strbuf *sb, const struct option *opt, + const char *arg, int unset) +{ + strbuf_reset(sb); + + if (opt->long_name) { + strbuf_addstr(sb, unset ? "--no-" : "--"); + strbuf_addstr(sb, opt->long_name); + if (arg) { + strbuf_addch(sb, '='); + strbuf_addstr(sb, arg); + } + } else if (opt->short_name && !unset) { + strbuf_addch(sb, '-'); + strbuf_addch(sb, opt->short_name); + if (arg) + strbuf_addstr(sb, arg); + } else + return -1; + + return 0; +} + +/** + * For an option opt, recreates the command-line option in opt->value which + * must be an char* initialized to NULL. This is useful when we need to pass + * the command-line option to another command. Since any previous value will be + * overwritten, this callback should only be used for options where the last + * one wins. + */ +int parse_opt_passthru(const struct option *opt, const char *arg, int unset) +{ + static struct strbuf sb = STRBUF_INIT; + char **opt_value = opt->value; + + if (recreate_opt(&sb, opt, arg, unset) < 0) + return -1; + + if (*opt_value) + free(*opt_value); + + *opt_value = strbuf_detach(&sb, NULL); + + return 0; +} diff --git a/parse-options.h b/parse-options.h index c71e9da..5b0f886 100644 --- a/parse-options.h +++ b/parse-options.h @@ -224,6 +224,7 @@ extern int parse_opt_with_commit(const struct option *, const char *, int); extern int parse_opt_tertiary(const struct option *, const char *, int); extern int parse_opt_string_list(const struct option *, const char *, int); extern int parse_opt_noop_cb(const st
[PATCH v3 09/19] pull: error on no merge candidates
Commit a8c9bef (pull: improve advice for unconfigured error case, 2009-10-05) fully established the current advices given by git-pull for the different cases where git-fetch will not have anything marked for merge: 1. We fetched from a specific remote, and a refspec was given, but it ended up not fetching anything. This is usually because the user provided a wildcard refspec which had no matches on the remote end. 2. We fetched from a non-default remote, but didn't specify a branch to merge. We can't use the configured one because it applies to the default remote, and thus the user must specify the branches to merge. 3. We fetched from the branch's or repo's default remote, but: a. We are not on a branch, so there will never be a configured branch to merge with. b. We are on a branch, but there is no configured branch to merge with. 4. We fetched from the branch's or repo's default remote, but the configured branch to merge didn't get fetched (either it doesn't exist, or wasn't part of the configured fetch refspec) Re-implement the above behavior by implementing get_merge_heads() to parse the heads in FETCH_HEAD for merging, and implementing die_no_merge_candidates(), which will be called when FETCH_HEAD has no heads for merging. Helped-by: Johannes Schindelin Helped-by: Junio C Hamano Signed-off-by: Paul Tan --- Notes: v3 * Tightening up of FETCH_HEAD parsing code and style fixes. builtin/pull.c | 113 + 1 file changed, 113 insertions(+) diff --git a/builtin/pull.c b/builtin/pull.c index 731e2a6..83691fc 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -10,6 +10,8 @@ #include "parse-options.h" #include "exec_cmd.h" #include "run-command.h" +#include "sha1-array.h" +#include "remote.h" static const char * const pull_usage[] = { N_("git pull [options] [ [...]]"), @@ -165,6 +167,111 @@ static void argv_push_force(struct argv_array *arr) } /** + * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge + * into merge_heads. + */ +static void get_merge_heads(struct sha1_array *merge_heads) +{ + const char *filename = git_path("FETCH_HEAD"); + FILE *fp; + struct strbuf sb = STRBUF_INIT; + unsigned char sha1[GIT_SHA1_RAWSZ]; + + if (!(fp = fopen(filename, "r"))) + die_errno(_("could not open '%s' for reading"), filename); + while (strbuf_getline(&sb, fp, '\n') != EOF) { + if (get_sha1_hex(sb.buf, sha1)) + continue; /* invalid line: does not start with SHA1 */ + if (starts_with(sb.buf + GIT_SHA1_HEXSZ, "\tnot-for-merge\t")) + continue; /* ref is not-for-merge */ + sha1_array_append(merge_heads, sha1); + } + fclose(fp); + strbuf_release(&sb); +} + +/** + * Used by die_no_merge_candidates() as a for_each_remote() callback to + * retrieve the name of the remote if the repository only has one remote. + */ +static int get_only_remote(struct remote *remote, void *cb_data) +{ + const char **remote_name = cb_data; + + if (*remote_name) + return -1; + + *remote_name = remote->name; + return 0; +} + +/** + * Dies with the appropriate reason for why there are no merge candidates: + * + * 1. We fetched from a specific remote, and a refspec was given, but it ended + *up not fetching anything. This is usually because the user provided a + *wildcard refspec which had no matches on the remote end. + * + * 2. We fetched from a non-default remote, but didn't specify a branch to + *merge. We can't use the configured one because it applies to the default + *remote, thus the user must specify the branches to merge. + * + * 3. We fetched from the branch's or repo's default remote, but: + * + *a. We are not on a branch, so there will never be a configured branch to + * merge with. + * + *b. We are on a branch, but there is no configured branch to merge with. + * + * 4. We fetched from the branch's or repo's default remote, but the configured + *branch to merge didn't get fetched. (Either it doesn't exist, or wasn't + *part of the configured fetch refspec.) + */ +static void NORETURN die_no_merge_candidates(const char *repo, const char **refspecs) +{ + struct branch *curr_branch = branch_get("HEAD"); + const char *remote = curr_branch ? curr_branch->remote_name : NULL; + + if (*refspecs) { + fprintf_ln(stderr, _("There are no candidates for merging among the refs that you just fetched.")); + fprintf_ln(stderr, _("Generally this means that you provided a
[PATCH v3 02/19] parse-options-cb: implement parse_opt_passthru_argv()
Certain git commands, such as git-pull, are simply wrappers around other git commands like git-fetch, git-merge and git-rebase. As such, these wrapper commands will typically need to "pass through" command-line options of the commands they wrap. Implement the parse_opt_passthru_argv() parse-options callback, which will reconstruct all the provided command-line options into an argv_array, such that it can be passed to another git command. This is useful for passing command-line options that can be specified multiple times. Helped-by: Stefan Beller Signed-off-by: Paul Tan --- Notes: v3 * Renamed function to the more descriptive parse_opt_passthru_argv(). * Introduced and documented OPT_PASSTHRU_ARGV() macro, which saves the user from having to specify OPTION_CALLBACK and parse_opt_passthru_argv() for each option. Documentation/technical/api-parse-options.txt | 6 ++ parse-options-cb.c| 20 parse-options.h | 3 +++ 3 files changed, 29 insertions(+) diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt index 85d10ab..0b0ab01 100644 --- a/Documentation/technical/api-parse-options.txt +++ b/Documentation/technical/api-parse-options.txt @@ -219,6 +219,12 @@ There are some macros to easily define options: will be overwritten, so this should only be used for options where the last one specified on the command line wins. +`OPT_PASSTHRU_ARGV(short, long, &argv_array_var, arg_str, description, flags)`:: + Introduce an option where all instances of it on the command-line will + be reconstructed into an argv_array. This is useful when you need to + pass the command-line option, which can be specified multiple times, + to another command. + The last element of the array must be `OPT_END()`. diff --git a/parse-options-cb.c b/parse-options-cb.c index 68bc593..5ab6ed6 100644 --- a/parse-options-cb.c +++ b/parse-options-cb.c @@ -4,6 +4,7 @@ #include "commit.h" #include "color.h" #include "string-list.h" +#include "argv-array.h" /*- some often used options -*/ @@ -183,3 +184,22 @@ int parse_opt_passthru(const struct option *opt, const char *arg, int unset) return 0; } + +/** + * For an option opt, recreate the command-line option, appending it to + * opt->value which must be a argv_array. This is useful when we need to pass + * the command-line option, which can be specified multiple times, to another + * command. + */ +int parse_opt_passthru_argv(const struct option *opt, const char *arg, int unset) +{ + static struct strbuf sb = STRBUF_INIT; + struct argv_array *opt_value = opt->value; + + if (recreate_opt(&sb, opt, arg, unset) < 0) + return -1; + + argv_array_push(opt_value, sb.buf); + + return 0; +} diff --git a/parse-options.h b/parse-options.h index 5b0f886..aba06688 100644 --- a/parse-options.h +++ b/parse-options.h @@ -225,6 +225,7 @@ extern int parse_opt_tertiary(const struct option *, const char *, int); extern int parse_opt_string_list(const struct option *, const char *, int); extern int parse_opt_noop_cb(const struct option *, const char *, int); extern int parse_opt_passthru(const struct option *, const char *, int); +extern int parse_opt_passthru_argv(const struct option *, const char *, int); #define OPT__VERBOSE(var, h) OPT_COUNTUP('v', "verbose", (var), (h)) #define OPT__QUIET(var, h)OPT_COUNTUP('q', "quiet", (var), (h)) @@ -245,5 +246,7 @@ extern int parse_opt_passthru(const struct option *, const char *, int); { OPTION_CALLBACK, (s), (l), (v), N_("style"), (h), PARSE_OPT_OPTARG, parseopt_column_callback } #define OPT_PASSTHRU(s, l, v, a, h, f) \ { OPTION_CALLBACK, (s), (l), (v), (a), (h), (f), parse_opt_passthru } +#define OPT_PASSTHRU_ARGV(s, l, v, a, h, f) \ + { OPTION_CALLBACK, (s), (l), (v), (a), (h), (f), parse_opt_passthru_argv } #endif -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 04/19] pull: implement skeletal builtin pull
For the purpose of rewriting git-pull.sh into a C builtin, implement a skeletal builtin/pull.c that redirects to $GIT_EXEC_PATH/git-pull.sh if the environment variable _GIT_USE_BUILTIN_PULL is not defined. This allows us to fall back on the functional git-pull.sh when running the test suite for tests that depend on a working git-pull implementation. This redirection should be removed when all the features of git-pull.sh have been re-implemented in builtin/pull.c. Helped-by: Junio C Hamano Signed-off-by: Paul Tan --- Notes: v3 * style fixes Makefile | 1 + builtin.h | 1 + builtin/pull.c | 33 + git.c | 1 + 4 files changed, 36 insertions(+) create mode 100644 builtin/pull.c diff --git a/Makefile b/Makefile index 54ec511..2057a9d 100644 --- a/Makefile +++ b/Makefile @@ -877,6 +877,7 @@ BUILTIN_OBJS += builtin/pack-refs.o BUILTIN_OBJS += builtin/patch-id.o BUILTIN_OBJS += builtin/prune-packed.o BUILTIN_OBJS += builtin/prune.o +BUILTIN_OBJS += builtin/pull.o BUILTIN_OBJS += builtin/push.o BUILTIN_OBJS += builtin/read-tree.o BUILTIN_OBJS += builtin/receive-pack.o diff --git a/builtin.h b/builtin.h index b87df70..ea3c834 100644 --- a/builtin.h +++ b/builtin.h @@ -98,6 +98,7 @@ extern int cmd_pack_redundant(int argc, const char **argv, const char *prefix); extern int cmd_patch_id(int argc, const char **argv, const char *prefix); extern int cmd_prune(int argc, const char **argv, const char *prefix); extern int cmd_prune_packed(int argc, const char **argv, const char *prefix); +extern int cmd_pull(int argc, const char **argv, const char *prefix); extern int cmd_push(int argc, const char **argv, const char *prefix); extern int cmd_read_tree(int argc, const char **argv, const char *prefix); extern int cmd_receive_pack(int argc, const char **argv, const char *prefix); diff --git a/builtin/pull.c b/builtin/pull.c new file mode 100644 index 000..cabeed4 --- /dev/null +++ b/builtin/pull.c @@ -0,0 +1,33 @@ +/* + * Builtin "git pull" + * + * Based on git-pull.sh by Junio C Hamano + * + * Fetch one or more remote refs and merge it/them into the current HEAD. + */ +#include "cache.h" +#include "builtin.h" +#include "parse-options.h" +#include "exec_cmd.h" + +static const char * const pull_usage[] = { + NULL +}; + +static struct option pull_options[] = { + OPT_END() +}; + +int cmd_pull(int argc, const char **argv, const char *prefix) +{ + if (!getenv("_GIT_USE_BUILTIN_PULL")) { + const char *path = mkpath("%s/git-pull", git_exec_path()); + + if (sane_execvp(path, (char **)argv) < 0) + die_errno("could not exec %s", path); + } + + argc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0); + + return 0; +} diff --git a/git.c b/git.c index 44374b1..e7a7713 100644 --- a/git.c +++ b/git.c @@ -445,6 +445,7 @@ static struct cmd_struct commands[] = { { "pickaxe", cmd_blame, RUN_SETUP }, { "prune", cmd_prune, RUN_SETUP }, { "prune-packed", cmd_prune_packed, RUN_SETUP }, + { "pull", cmd_pull, RUN_SETUP | NEED_WORK_TREE }, { "push", cmd_push, RUN_SETUP }, { "read-tree", cmd_read_tree, RUN_SETUP }, { "receive-pack", cmd_receive_pack }, -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 06/19] pull: pass verbosity, --progress flags to fetch and merge
7f87aff (Teach/Fix pull/fetch -q/-v options, 2008-11-15) taught git-pull to accept the verbosity -v and -q options and pass them to git-fetch and git-merge. Re-implement support for the verbosity flags by adding it to the options list and introducing argv_push_verbosity() to push the flags into the argv array used to execute git-fetch and git-merge. 9839018 (fetch and pull: learn --progress, 2010-02-24) and bebd2fd (pull: propagate --progress to merge, 2011-02-20) taught git-pull to accept the --progress option and pass it to git-fetch and git-merge. Use OPT_PASSTHRU() implemented earlier to pass the "--[no-]progress" command line options to git-fetch and git-merge. Helped-by: Junio C Hamano Signed-off-by: Paul Tan --- Notes: v3 * Re-worded commit message. builtin/pull.c | 36 1 file changed, 36 insertions(+) diff --git a/builtin/pull.c b/builtin/pull.c index 9157536..5d9f2b5 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -16,11 +16,35 @@ static const char * const pull_usage[] = { NULL }; +/* Shared options */ +static int opt_verbosity; +static char *opt_progress; + static struct option pull_options[] = { + /* Shared options */ + OPT__VERBOSITY(&opt_verbosity), + OPT_PASSTHRU(0, "progress", &opt_progress, NULL, + N_("force progress reporting"), + PARSE_OPT_NOARG), + OPT_END() }; /** + * Pushes "-q" or "-v" switches into arr to match the opt_verbosity level. + */ +static void argv_push_verbosity(struct argv_array *arr) +{ + int verbosity; + + for (verbosity = opt_verbosity; verbosity > 0; verbosity--) + argv_array_push(arr, "-v"); + + for (verbosity = opt_verbosity; verbosity < 0; verbosity++) + argv_array_push(arr, "-q"); +} + +/** * Parses argv into [ [...]], returning their values in `repo` * as a string and `refspecs` as a null-terminated array of strings. If `repo` * is not provided in argv, it is set to NULL. @@ -46,6 +70,12 @@ static int run_fetch(const char *repo, const char **refspecs) int ret; argv_array_pushl(&args, "fetch", "--update-head-ok", NULL); + + /* Shared options */ + argv_push_verbosity(&args); + if (opt_progress) + argv_array_push(&args, opt_progress); + if (repo) { argv_array_push(&args, repo); argv_array_pushv(&args, refspecs); @@ -65,6 +95,12 @@ static int run_merge(void) struct argv_array args = ARGV_ARRAY_INIT; argv_array_pushl(&args, "merge", NULL); + + /* Shared options */ + argv_push_verbosity(&args); + if (opt_progress) + argv_array_push(&args, opt_progress); + argv_array_push(&args, "FETCH_HEAD"); ret = run_command_v_opt(args.argv, RUN_GIT_CMD); argv_array_clear(&args); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 00/19] Make git-pull a builtin
This is a re-roll of [v2]. Thanks Junio, Stefan for the reviews last round. Previous versions: [v1] http://thread.gmane.org/gmane.comp.version-control.git/269258 [v2] http://thread.gmane.org/gmane.comp.version-control.git/270639 git-pull is a commonly executed command to check for new changes in the upstream repository and, if there are, fetch and integrate them into the current branch. Currently it is implemented by the shell script git-pull.sh. However, compared to C, shell scripts have certain deficiencies -- they need to spawn a lot of processes, introduce a lot of dependencies and cannot take advantage of git's internal caches. This series rewrites git-pull.sh into a C builtin, thus improving its performance and portability. It is part of my GSoC project to rewrite git-pull and git-am into builtins[1]. [1] https://gist.github.com/pyokagan/1b7b0d1f4dab6ba3cef1 Paul Tan (19): parse-options-cb: implement parse_opt_passthru() parse-options-cb: implement parse_opt_passthru_argv() argv-array: implement argv_array_pushv() pull: implement skeletal builtin pull pull: implement fetch + merge pull: pass verbosity, --progress flags to fetch and merge pull: pass git-merge's options to git-merge pull: pass git-fetch's options to git-fetch pull: error on no merge candidates pull: support pull.ff config pull: check if in unresolved merge state pull: fast-forward working tree if head is updated pull: implement pulling into an unborn branch pull: set reflog message pull: teach git pull about --rebase pull: configure --rebase via branch..rebase or pull.rebase pull --rebase: exit early when the working directory is dirty pull --rebase: error on no merge candidate cases pull: remove redirection to git-pull.sh Documentation/technical/api-argv-array.txt| 3 + Documentation/technical/api-parse-options.txt | 13 + Makefile | 2 +- advice.c | 8 + advice.h | 1 + argv-array.c | 6 + argv-array.h | 1 + builtin.h | 1 + builtin/pull.c| 881 ++ git-pull.sh => contrib/examples/git-pull.sh | 0 git.c | 1 + parse-options-cb.c| 69 ++ parse-options.h | 6 + 13 files changed, 991 insertions(+), 1 deletion(-) create mode 100644 builtin/pull.c rename git-pull.sh => contrib/examples/git-pull.sh (100%) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 11/19] pull: check if in unresolved merge state
Since d38a30d (Be more user-friendly when refusing to do something because of conflict., 2010-01-12), git-pull will error out with user-friendly advices if the user is in the middle of a merge or has unmerged files. Re-implement this behavior. While the "has unmerged files" case can be handled by die_resolve_conflict(), we introduce a new function die_conclude_merge() for printing a different error message for when there are no unmerged files but the merge has not been finished. Signed-off-by: Paul Tan --- advice.c | 8 advice.h | 1 + builtin/pull.c | 9 + 3 files changed, 18 insertions(+) diff --git a/advice.c b/advice.c index 575bec2..4965686 100644 --- a/advice.c +++ b/advice.c @@ -96,6 +96,14 @@ void NORETURN die_resolve_conflict(const char *me) die("Exiting because of an unresolved conflict."); } +void NORETURN die_conclude_merge(void) +{ + error(_("You have not concluded your merge (MERGE_HEAD exists).")); + if (advice_resolve_conflict) + advise(_("Please, commit your changes before you can merge.")); + die(_("Exiting because of unfinished merge.")); +} + void detach_advice(const char *new_name) { const char fmt[] = diff --git a/advice.h b/advice.h index 5ecc6c1..b341a55 100644 --- a/advice.h +++ b/advice.h @@ -24,6 +24,7 @@ __attribute__((format (printf, 1, 2))) void advise(const char *advice, ...); int error_resolve_conflict(const char *me); extern void NORETURN die_resolve_conflict(const char *me); +void NORETURN die_conclude_merge(void); void detach_advice(const char *new_name); #endif /* ADVICE_H */ diff --git a/builtin/pull.c b/builtin/pull.c index 0ddb964..cb5898a 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -12,6 +12,7 @@ #include "run-command.h" #include "sha1-array.h" #include "remote.h" +#include "dir.h" static const char * const pull_usage[] = { N_("git pull [options] [ [...]]"), @@ -426,6 +427,14 @@ int cmd_pull(int argc, const char **argv, const char *prefix) if (!opt_ff) opt_ff = xstrdup_or_null(config_get_ff()); + git_config(git_default_config, NULL); + + if (read_cache_unmerged()) + die_resolve_conflict("Pull"); + + if (file_exists(git_path("MERGE_HEAD"))) + die_conclude_merge(); + if (run_fetch(repo, refspecs)) return 1; -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 11/19] pull: check if in unresolved merge state
On Thu, Jun 11, 2015 at 1:14 AM, Junio C Hamano wrote: > I (or at least some part of me) actually view git_config_get_*() as > "if you are only going to peek a few variables, you do not have to > do the looping yourself" convenience, which leads me (or at least > that part of me) to say "if you are doing the looping anyway, you > may be better off picking up the variables you care about yourself > in that loop". Not just a convenience, but I personally find that callback functions usually leads to code that is harder to understand because of the use of static/global variables to preserve state and the fact that it is harder to break it up into smaller functions to reason about. This is just a matter of taste though, so I don't have strong feelings about it. Besides, there is a greater technical reason why we should not use git_config(): It essentially performs a linear search[1], while the git_config_get_*() functions do a constant-time lookup. Ideally, we should remove uses of git_config() for users that have no need to iterate over all the configuration entries. [1] Since we do a strcmp() for every supported config setting, for every config entry. I should emphasize that the call to git_config(git_default_config, NULL) is just a workaround to load the advice.* settings. In fact, git_default_config() only peeks at a few config settings anyway, and can be re-written to not iterate over all the user's config entries. As such, I don't see why the builtin/pull.c code should be written to support the git_config() way of doing things, even if we have to use the workaround of calling git_config(). It's like saying: we have a bad solution, now let's make it worse ;-) > By calling git_config() before calling any git_config_get_*() > functions, you would be priming the cache layer with the entire > contents of the configuration files anyway, so later calls to > git_config_get_*() will read from that cache, so there is no > performance penalty in mixing the two methods to access > configuration data. That is why I felt less certain that the > suggestion to stick to one method (instead of mixing the two) is a > good thing to do (hence "less certain 'might'"). Right, although I think that the performance penalty due to using git_config() is greater, and switching all the git_config_get_*() calls to use it would just make it worse. By the way, I got the idea that git development was moving towards replacing use of git_config() from 5801d3b (builtin/gc.c: replace `git_config()` with `git_config_get_*()` family, 2014-08-07). Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 09/19] pull: error on no merge candidates
On Wed, Jun 10, 2015 at 7:56 AM, Junio C Hamano wrote: > Paul Tan writes: > >> /** >> + * Appends merge candidates from FETCH_HEAD that are not marked >> not-for-merge >> + * into merge_heads. >> + */ > > Hmph, I vaguely recall doing that in C elsewhere already, even > though I do not remember where offhand... Right, handle_fetch_head() in builtin/merge.c. It looks up the commit IDs into commit objects though, which is not required for git-pull. We only need the list of hashes. >> +static void get_merge_heads(struct sha1_array *merge_heads) >> +{ >> + const char *filename = git_path("FETCH_HEAD"); >> + FILE *fp; >> + struct strbuf sb = STRBUF_INIT; >> + unsigned char sha1[GIT_SHA1_RAWSZ]; >> + >> + if (!(fp = fopen(filename, "r"))) >> + die_errno(_("could not open '%s' for reading"), filename); >> + while(strbuf_getline(&sb, fp, '\n') != EOF) { > > Missing SP after "while" OK >> + if (get_sha1_hex(sb.buf, sha1)) >> + continue; /* invalid line: does not start with SHA1 */ >> + if (starts_with(sb.buf + GIT_SHA1_HEXSZ, "\tnot-for-merge")) > > Look for "\tnot-for-merge\t" instead? Right, it's better to be stricter. Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] t0302: "unreadable" test needs SANITY prereq
The test expects that "chmod -r ~/.git-credentials" would make it unreadable to the user, and thus needs the SANITY prerequisite. Reported-by: Jean-Yves LENHOF Signed-off-by: Paul Tan --- t/t0302-credential-store.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh index 0979df9..1d8d1f2 100755 --- a/t/t0302-credential-store.sh +++ b/t/t0302-credential-store.sh @@ -75,7 +75,7 @@ test_expect_success 'get: use xdg file if home file has no matches' ' EOF ' -test_expect_success POSIXPERM 'get: use xdg file if home file is unreadable' ' +test_expect_success POSIXPERM,SANITY 'get: use xdg file if home file is unreadable' ' echo "https://home-user:home-p...@example.com"; >"$HOME/.git-credentials" && chmod -r "$HOME/.git-credentials" && mkdir -p "$HOME/.config/git" && -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug when doing make test using root user
On Fri, Jun 12, 2015 at 5:43 PM, Jean-Yves LENHOF wrote: > Hi, > > I tried to compile git 2.4.3 using root on a server. It failed on test 41 of > t0302-credential-store.sh > In fact even if we remove read access on a directory, root still can acces > this directory. > Using a not privilegied user make the test work. > Perhaps the test should be adapted to this corner case. > Trace below. Right, the test should have the SANITY prereq. Thanks for reporting. Regards, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] pull: allow dirty tree when rebase.autostash enabled
On Sun, Jun 7, 2015 at 5:12 AM, Kevin Daudt wrote: > From: Kevin Daudt > > rebase learned to stash changes when it encounters a dirty work tree, but > git pull --rebase does not. > > Only verify if the working tree is dirty when rebase.autostash is not > enabled. > > Signed-off-by: Kevin Daudt Ehh? The sign-off does not match the author of the patch. > Helped-by: Paul Tan > --- > git-pull.sh | 5 - > t/t5520-pull.sh | 12 > 2 files changed, 16 insertions(+), 1 deletion(-) > > diff --git a/git-pull.sh b/git-pull.sh > index 0917d0d..f0a3b6e 100755 > --- a/git-pull.sh > +++ b/git-pull.sh > @@ -239,7 +239,10 @@ test true = "$rebase" && { > die "$(gettext "updating an unborn branch with > changes added to the index")" > fi > else > - require_clean_work_tree "pull with rebase" "Please commit or > stash them." > + if [ $(git config --bool --get rebase.autostash || echo > false) = false ] > + then > + require_clean_work_tree "pull with rebase" "Please > commit or stash them." > + fi > fi > oldremoteref= && > test -n "$curr_branch" && > diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh > index 925ad49..d06119f 100755 > --- a/t/t5520-pull.sh > +++ b/t/t5520-pull.sh > @@ -135,6 +135,18 @@ test_expect_success 'pull --rebase dies early with dirty > working directory' ' > test_cmp file expect > ' > > +test_expect_success 'pull --rebase succeeds with dirty working directory and > rebase.autostash set' ' > + test_config branch.to-rebase.rebase true && Ok, though I wonder why not just a git pull --rebase... > + test_config rebase.autostash true && > + git checkout HEAD -- file && Why not git reset --hard before-rebase? If we don't reset HEAD, then how would we know if we actually did a rebase? > + echo dirty > new_file && style: echo dirty >new_file && > + git add new_file && > + git pull . copy && > + test $(git rev-parse HEAD^) = $(git rev-parse copy) && Okay, although it would be better to use "test_cmp_rev HEAD^ copy" because it prints out the hashes if they are different. > + test $(cat new_file) = dirty && "$(cat new_file)" should be quoted to prevent field splitting. > + test "$(cat file)" = "modified again" > +' > + > test_expect_success 'pull.rebase' ' > git reset --hard before-rebase && > test_config pull.rebase true && > -- > 2.4.2 Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] t5520-pull: Simplify --rebase with dirty tree test
On Sun, Jun 7, 2015 at 5:12 AM, Kevin Daudt wrote: > @@ -278,25 +291,6 @@ test_expect_success 'rebased upstream + fetch + pull > --rebase' ' > > ' > > -test_expect_success 'pull --rebase dies early with dirty working directory' ' > - > - git checkout to-rebase && > - git update-ref refs/remotes/me/copy copy^ && > - COPY=$(git rev-parse --verify me/copy) && > - git rebase --onto $COPY copy && > - test_config branch.to-rebase.remote me && > - test_config branch.to-rebase.merge refs/heads/copy && > - test_config branch.to-rebase.rebase true && > - echo dirty >> file && > - git add file && > - test_must_fail git pull && > - test $COPY = $(git rev-parse --verify me/copy) && > - git checkout HEAD -- file && > - git pull && > - test $COPY != $(git rev-parse --verify me/copy) > - > -' Eh whoops, I don't think we should touch this test. It comes from f9189cf, which states that: When rebasing fails during "pull --rebase", you cannot just clean up the working directory and call "pull --rebase" again, since the remote branch was already fetched. Which makes me believe that "die-ing early with dirty working directory" has something to do with the rebased upstream handling feature of git-pull, and so this test is correct in testing that, and thus we should not touch it. The location of the test in the other patch is fine though. Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v2 18/19] cache-tree: introduce write_index_as_tree()
A caller may wish to write a temporary index as a tree. However, write_cache_as_tree() assumes that the index was read from, and will write to, the default index file path. Introduce write_index_as_tree() which removes this limitation by allowing the caller to specify its own index_state and index file path. Signed-off-by: Paul Tan --- cache-tree.c | 29 + cache-tree.h | 1 + 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/cache-tree.c b/cache-tree.c index 32772b9..feace8b 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -592,7 +592,7 @@ static struct cache_tree *cache_tree_find(struct cache_tree *it, const char *pat return it; } -int write_cache_as_tree(unsigned char *sha1, int flags, const char *prefix) +int write_index_as_tree(unsigned char *sha1, struct index_state *index_state, const char *index_path, int flags, const char *prefix) { int entries, was_valid, newfd; struct lock_file *lock_file; @@ -603,23 +603,23 @@ int write_cache_as_tree(unsigned char *sha1, int flags, const char *prefix) */ lock_file = xcalloc(1, sizeof(struct lock_file)); - newfd = hold_locked_index(lock_file, 1); + newfd = hold_lock_file_for_update(lock_file, index_path, LOCK_DIE_ON_ERROR); - entries = read_cache(); + entries = read_index_from(index_state, index_path); if (entries < 0) return WRITE_TREE_UNREADABLE_INDEX; if (flags & WRITE_TREE_IGNORE_CACHE_TREE) - cache_tree_free(&(active_cache_tree)); + cache_tree_free(&index_state->cache_tree); - if (!active_cache_tree) - active_cache_tree = cache_tree(); + if (!index_state->cache_tree) + index_state->cache_tree = cache_tree(); - was_valid = cache_tree_fully_valid(active_cache_tree); + was_valid = cache_tree_fully_valid(index_state->cache_tree); if (!was_valid) { - if (cache_tree_update(&the_index, flags) < 0) + if (cache_tree_update(index_state, flags) < 0) return WRITE_TREE_UNMERGED_INDEX; if (0 <= newfd) { - if (!write_locked_index(&the_index, lock_file, COMMIT_LOCK)) + if (!write_locked_index(index_state, lock_file, COMMIT_LOCK)) newfd = -1; } /* Not being able to write is fine -- we are only interested @@ -631,14 +631,14 @@ int write_cache_as_tree(unsigned char *sha1, int flags, const char *prefix) } if (prefix) { - struct cache_tree *subtree = - cache_tree_find(active_cache_tree, prefix); + struct cache_tree *subtree; + subtree = cache_tree_find(index_state->cache_tree, prefix); if (!subtree) return WRITE_TREE_PREFIX_ERROR; hashcpy(sha1, subtree->sha1); } else - hashcpy(sha1, active_cache_tree->sha1); + hashcpy(sha1, index_state->cache_tree->sha1); if (0 <= newfd) rollback_lock_file(lock_file); @@ -646,6 +646,11 @@ int write_cache_as_tree(unsigned char *sha1, int flags, const char *prefix) return 0; } +int write_cache_as_tree(unsigned char *sha1, int flags, const char *prefix) +{ + return write_index_as_tree(sha1, &the_index, get_index_file(), flags, prefix); +} + static void prime_cache_tree_rec(struct cache_tree *it, struct tree *tree) { struct tree_desc desc; diff --git a/cache-tree.h b/cache-tree.h index aa7b3e4..41c5746 100644 --- a/cache-tree.h +++ b/cache-tree.h @@ -46,6 +46,7 @@ int update_main_cache_tree(int); #define WRITE_TREE_UNMERGED_INDEX (-2) #define WRITE_TREE_PREFIX_ERROR (-3) +int write_index_as_tree(unsigned char *sha1, struct index_state *index_state, const char *index_path, int flags, const char *prefix); int write_cache_as_tree(unsigned char *sha1, int flags, const char *prefix); void prime_cache_tree(struct index_state *, struct tree *); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v2 19/19] am: implement 3-way merge
Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07), git-am supported the --3way option, and if set, would attempt to do a 3-way merge if the initial patch application fails. Re-implement this feature through the fall_back_threeway() function. Signed-off-by: Paul Tan --- builtin/am.c | 133 +-- 1 file changed, 129 insertions(+), 4 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 71fda16..8566d22 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -19,6 +19,7 @@ #include "unpack-trees.h" #include "branch.h" #include "sequencer.h" +#include "merge-recursive.h" /** * Returns 1 if the file is empty or does not exist, 0 otherwise. @@ -76,6 +77,8 @@ struct am_state { const char *resolvemsg; int sign; + + int threeway; }; /** @@ -659,16 +662,32 @@ static void NORETURN die_user_resolve(const struct am_state *state) } /* - * Applies current patch with git-apply. Returns 0 on success, -1 otherwise. + * Applies current patch with git-apply. Returns 0 on success, -1 otherwise. If + * index_file is not NULL, the patch will be applied to that index. */ -static int run_apply(const struct am_state *state) +static int run_apply(const struct am_state *state, const char *index_file) { struct child_process cp = CHILD_PROCESS_INIT; cp.git_cmd = 1; + if (index_file) + argv_array_pushf(&cp.env_array, "GIT_INDEX_FILE=%s", index_file); + + /* +* If we are allowed to fall back on 3-way merge, don't give false +* errors during the initial attempt. +*/ + if (state->threeway && !index_file) { + cp.no_stdout = 1; + cp.no_stderr = 1; + } + argv_array_push(&cp.args, "apply"); - argv_array_push(&cp.args, "--index"); + if (index_file) + argv_array_push(&cp.args, "--cached"); + else + argv_array_push(&cp.args, "--index"); argv_array_push(&cp.args, am_path(state, "patch")); if (run_command(&cp)) @@ -676,8 +695,92 @@ static int run_apply(const struct am_state *state) /* Reload index as git-apply will have modified it. */ discard_cache(); + read_cache_from(index_file ? index_file : get_index_file()); + + return 0; +} + +/* + * Builds a index that contains just the blobs needed for a 3way merge. + */ +static int build_fake_ancestor(const struct am_state *state, const char *index_file) +{ + struct child_process cp = CHILD_PROCESS_INIT; + + cp.git_cmd = 1; + argv_array_push(&cp.args, "apply"); + argv_array_pushf(&cp.args, "--build-fake-ancestor=%s", index_file); + argv_array_push(&cp.args, am_path(state, "patch")); + + if (run_command(&cp)) + return -1; + + return 0; +} + +/** + * Attempt a threeway merge, using index_path as the temporary index. + */ +static int fall_back_threeway(const struct am_state *state, const char *index_path) +{ + unsigned char orig_tree[20], his_tree[20], our_tree[20]; + const unsigned char *bases[1] = {orig_tree}; + struct merge_options o; + struct commit *result; + + if (get_sha1("HEAD", our_tree) < 0) + hashcpy(our_tree, EMPTY_TREE_SHA1_BIN); + + if (build_fake_ancestor(state, index_path)) + return error("could not build fake ancestor"); + + discard_cache(); + read_cache_from(index_path); + + if (write_index_as_tree(orig_tree, &the_index, index_path, 0, NULL)) + return error(_("Repository lacks necessary blobs to fall back on 3-way merge.")); + + say(state, stdout, _("Using index info to reconstruct a base tree...")); + + if (!state->quiet) { + struct child_process cp = CHILD_PROCESS_INIT; + cp.git_cmd = 1; + argv_array_pushf(&cp.env_array, "GIT_INDEX_FILE=%s", index_path); + argv_array_pushl(&cp.args, "diff-index", "--cached", + "--diff-filter=AM", "--name-status", "HEAD", NULL); + run_command(&cp); + } + + if (run_apply(state, index_path)) + return error(_("Did you hand edit your patch?\n" + "It does not apply to blobs recorded in its index.")); + + if (write_index_as_tree(his_tree, &the_index, index_path, 0, NULL)) + return error("could not write tree"); + + say(state, stdout, _("Falling back to patching base and 3-way merge...")); + + discard_cache();
[PATCH/WIP v2 08/19] am: apply patch with git-apply
Implement applying the patch to the index using git-apply. Signed-off-by: Paul Tan --- builtin/am.c | 55 ++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/builtin/am.c b/builtin/am.c index a1db474..b725a74 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -27,6 +27,18 @@ static int is_empty_file(const char *filename) return !st.st_size; } +/** + * Returns the first line of msg + */ +static const char *firstline(const char *msg) +{ + static struct strbuf sb = STRBUF_INIT; + + strbuf_reset(&sb); + strbuf_add(&sb, msg, strchrnul(msg, '\n') - msg); + return sb.buf; +} + enum patch_format { PATCH_FORMAT_UNKNOWN = 0, PATCH_FORMAT_MBOX @@ -512,6 +524,29 @@ static int parse_patch(struct am_state *state, const char *patch) return 0; } +/* + * Applies current patch with git-apply. Returns 0 on success, -1 otherwise. + */ +static int run_apply(const struct am_state *state) +{ + struct child_process cp = CHILD_PROCESS_INIT; + + cp.git_cmd = 1; + + argv_array_push(&cp.args, "apply"); + argv_array_push(&cp.args, "--index"); + argv_array_push(&cp.args, am_path(state, "patch")); + + if (run_command(&cp)) + return -1; + + /* Reload index as git-apply will have modified it. */ + discard_cache(); + read_cache(); + + return 0; +} + /** * Applies all queued patches. */ @@ -529,7 +564,25 @@ static void am_run(struct am_state *state) write_author_script(state); write_file(am_path(state, "final-commit"), 1, "%s", state->msg.buf); - /* TODO: Patch application not implemented yet */ + printf_ln(_("Applying: %s"), firstline(state->msg.buf)); + + if (run_apply(state) < 0) { + int value; + + printf_ln(_("Patch failed at %s %s"), msgnum(state), + firstline(state->msg.buf)); + + if (!git_config_get_bool("advice.amworkdir", &value) && !value) + printf_ln(_("The copy of the patch that failed is found in: %s"), + am_path(state, "patch")); + + exit(128); + } + + /* +* TODO: After the patch has been applied to the index with +* git-apply, we need to make commit as well. +*/ next: am_next(state); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v2 16/19] am: exit with user friendly message on patch failure
Since ced9456 (Give the user a hint for how to continue in the case that git-am fails because it requires user intervention, 2006-05-02), git-am prints additional information on how the user can re-invoke git-am to resume patch application after resolving the failure. Re-implement this through the die_user_resolve() function. Since cc12005 (Make git rebase interactive help match documentation., 2006-05-13), git-am supports the --resolvemsg option which is used by git-rebase to override the message printed out when git-am fails. Re-implement this option. Signed-off-by: Paul Tan --- builtin/am.c | 26 +++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 795b672..4cd21b8 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -70,6 +70,9 @@ struct am_state { int prec; int quiet; + + /* override error message when patch failure occurs */ + const char *resolvemsg; }; /** @@ -629,6 +632,21 @@ static int parse_patch(struct am_state *state, const char *patch) return 0; } +/** + * Dies with a user-friendly message on how to proceed after resolving the + * problem. This message can be overridden with state->resolvemsg. + */ +static void NORETURN die_user_resolve(const struct am_state *state) +{ + if (state->resolvemsg) + printf_ln("%s", state->resolvemsg); + else + printf_ln(_("When you have resolved this problem, run \"git am --continue\".\n" + "If you prefer to skip this patch, run \"git am --skip\" instead.\n" + "To restore the original branch and stop patching, run \"git am --abort\".")); + exit(128); +} + /* * Applies current patch with git-apply. Returns 0 on success, -1 otherwise. */ @@ -736,7 +754,7 @@ static void am_run(struct am_state *state) printf_ln(_("The copy of the patch that failed is found in: %s"), am_path(state, "patch")); - exit(128); + die_user_resolve(state); } do_commit(state); @@ -761,13 +779,13 @@ static void am_resolve(struct am_state *state) printf_ln(_("No changes - did you forget to use 'git add'?\n" "If there is nothing left to stage, chances are that something else\n" "already introduced the same changes; you might want to skip this patch.")); - exit(128); + die_user_resolve(state); } if (unmerged_cache()) { printf_ln(_("You still have unmerged paths in your index.\n" "Did you forget to use 'git add'?")); - exit(128); + die_user_resolve(state); } do_commit(state); @@ -981,6 +999,8 @@ static struct option am_options[] = { OPT__QUIET(&state.quiet, N_("be quiet")), OPT_CALLBACK(0, "patch-format", &opt_patch_format, N_("format"), N_("format the patch(es) are in"), parse_opt_patchformat), + OPT_STRING(0, "resolvemsg", &state.resolvemsg, NULL, + N_("override error message when patch failure occurs")), OPT_CMDMODE(0, "continue", &opt_resume, N_("continue applying patches after resolving a conflict"), RESUME_RESOLVED), -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v2 15/19] am: implement quiet option
Since 0e987a1 (am, rebase: teach quiet option, 2009-06-16), git-am supported the --quiet option and GIT_QUIET environment variable, and when told to be quiet, would only speak on failure. Re-implement this by introducing the say() function, which works like fprintf_ln(), but would only write to the stream when state->quiet is false. Signed-off-by: Paul Tan --- builtin/am.c | 36 +--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index cdc07ab..795b672 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -68,6 +68,8 @@ struct am_state { /* number of digits in patch filename */ int prec; + + int quiet; }; /** @@ -75,6 +77,8 @@ struct am_state { */ static void am_state_init(struct am_state *state) { + const char *quiet; + memset(state, 0, sizeof(*state)); strbuf_init(&state->dir, 0); @@ -83,6 +87,10 @@ static void am_state_init(struct am_state *state) strbuf_init(&state->author_date, 0); strbuf_init(&state->msg, 0); state->prec = 4; + + quiet = getenv("GIT_QUIET"); + if (quiet && *quiet) + state->quiet = 1; } /** @@ -106,6 +114,22 @@ static inline const char *am_path(const struct am_state *state, const char *path } /** + * If state->quiet is false, calls fprintf(fp, fmt, ...), and appends a newline + * at the end. + */ +static void say(const struct am_state *state, FILE *fp, const char *fmt, ...) +{ + va_list ap; + + va_start(ap, fmt); + if (!state->quiet) { + vfprintf(fp, fmt, ap); + putc('\n', fp); + } + va_end(ap); +} + +/** * Returns 1 if there is an am session in progress, 0 otherwise. */ static int am_in_progress(const struct am_state *state) @@ -250,6 +274,9 @@ static void am_load(struct am_state *state) read_state_file(&state->msg, am_path(state, "final-commit"), 0, 0); + read_state_file(&sb, am_path(state, "quiet"), 2, 1); + state->quiet = !strcmp(sb.buf, "t"); + strbuf_release(&sb); } @@ -431,6 +458,8 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, write_file(am_path(state, "last"), 1, "%d", state->last); + write_file(am_path(state, "quiet"), 1, state->quiet ? "t" : "f"); + if (!get_sha1("HEAD", curr_head)) { write_file(am_path(state, "abort-safety"), 1, "%s", sha1_to_hex(curr_head)); update_ref("am", "ORIG_HEAD", curr_head, NULL, 0, UPDATE_REFS_DIE_ON_ERR); @@ -644,7 +673,7 @@ static void do_commit(const struct am_state *state) commit_list_insert(lookup_commit(parent), &parents); } else { ptr = NULL; - fprintf_ln(stderr, _("applying to an empty history")); + say(state, stderr, _("applying to an empty history")); } author = fmt_ident(state->author_name.buf, state->author_email.buf, @@ -695,7 +724,7 @@ static void am_run(struct am_state *state) write_author_script(state); write_file(am_path(state, "final-commit"), 1, "%s", state->msg.buf); - printf_ln(_("Applying: %s"), firstline(state->msg.buf)); + say(state, stdout, _("Applying: %s"), firstline(state->msg.buf)); if (run_apply(state) < 0) { int value; @@ -726,7 +755,7 @@ next: */ static void am_resolve(struct am_state *state) { - printf_ln(_("Applying: %s"), firstline(state->msg.buf)); + say(state, stdout, _("Applying: %s"), firstline(state->msg.buf)); if (!index_has_changes(NULL)) { printf_ln(_("No changes - did you forget to use 'git add'?\n" @@ -949,6 +978,7 @@ static const char * const am_usage[] = { }; static struct option am_options[] = { + OPT__QUIET(&state.quiet, N_("be quiet")), OPT_CALLBACK(0, "patch-format", &opt_patch_format, N_("format"), N_("format the patch(es) are in"), parse_opt_patchformat), OPT_CMDMODE(0, "continue", &opt_resume, -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v2 10/19] am: refresh the index at start
If a file is unchanged but stat-dirty, git-apply may erroneously fail to apply patches, thinking that they conflict with a dirty working tree. As such, since 2a6f08a (am: refresh the index at start and --resolved, 2011-08-15), git-am will refresh the index before applying patches. Re-implement this behavior. Signed-off-by: Paul Tan --- builtin/am.c | 20 1 file changed, 20 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index ecc6d29..417bfde 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -13,6 +13,7 @@ #include "cache-tree.h" #include "refs.h" #include "commit.h" +#include "lockfile.h" /** * Returns 1 if the file is empty or does not exist, 0 otherwise. @@ -457,6 +458,20 @@ static const char *msgnum(const struct am_state *state) } /** + * Refresh and write index. + */ +static void refresh_and_write_cache(void) +{ + static struct lock_file lock_file; + + hold_locked_index(&lock_file, 1); + refresh_cache(REFRESH_QUIET); + if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK)) + die(_("unable to write index file")); + rollback_lock_file(&lock_file); +} + +/** * Parses `patch` using git-mailinfo. state->msg will be set to the patch * message. state->author_name, state->author_email, state->author_date will be * set to the patch author's name, email and date respectively. The patch's @@ -597,6 +612,8 @@ static void do_commit(const struct am_state *state) */ static void am_run(struct am_state *state) { + refresh_and_write_cache(); + while (state->cur <= state->last) { const char *patch = am_path(state, msgnum(state)); @@ -678,6 +695,9 @@ int cmd_am(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, am_options, am_usage, 0); + if (read_index_preload(&the_index, NULL) < 0) + die(_("failed to read the index")); + if (am_in_progress(&state)) am_load(&state); else { -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v2 14/19] am: implement --abort
Since 3e5057a (git am --abort, 2008-07-16), git-am supported the --abort option that will rewind HEAD back to the original commit. Re-implement this feature through am_abort(). Since 7b3b7e3 (am --abort: keep unrelated commits since the last failure and warn, 2010-12-21), to prevent commits made since the last failure from being lost, git-am will not rewind HEAD back to the original commit if HEAD moved since the last failure. Re-implement this through safe_to_abort(). Signed-off-by: Paul Tan --- builtin/am.c | 95 ++-- 1 file changed, 92 insertions(+), 3 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 1434e68..cdc07ab 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -409,6 +409,8 @@ static int split_patches(struct am_state *state, enum patch_format patch_format, static void am_setup(struct am_state *state, enum patch_format patch_format, struct string_list *paths) { + unsigned char curr_head[20]; + if (!patch_format) patch_format = detect_patch_format(paths); @@ -428,6 +430,14 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, write_file(am_path(state, "next"), 1, "%d", state->cur); write_file(am_path(state, "last"), 1, "%d", state->last); + + if (!get_sha1("HEAD", curr_head)) { + write_file(am_path(state, "abort-safety"), 1, "%s", sha1_to_hex(curr_head)); + update_ref("am", "ORIG_HEAD", curr_head, NULL, 0, UPDATE_REFS_DIE_ON_ERR); + } else { + write_file(am_path(state, "abort-safety"), 1, "%s", ""); + delete_ref("ORIG_HEAD", NULL, 0); + } } /** @@ -436,6 +446,8 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, */ static void am_next(struct am_state *state) { + unsigned char head[GIT_SHA1_RAWSZ]; + state->cur++; write_file(am_path(state, "next"), 1, "%d", state->cur); @@ -446,6 +458,11 @@ static void am_next(struct am_state *state) strbuf_reset(&state->msg); unlink(am_path(state, "final-commit")); + + if (!get_sha1("HEAD", head)) + write_file(am_path(state, "abort-safety"), 1, "%s", sha1_to_hex(head)); + else + write_file(am_path(state, "abort-safety"), 1, "%s", ""); } /** @@ -655,10 +672,14 @@ static void am_run(struct am_state *state) { struct strbuf sb = STRBUF_INIT; + unlink(am_path(state, "dirtyindex")); + refresh_and_write_cache(); - if (index_has_changes(&sb)) + if (index_has_changes(&sb)) { + write_file(am_path(state, "dirtyindex"), 1, "t"); die(_("Dirty index: cannot apply patches (dirty: %s)"), sb.buf); + } strbuf_release(&sb); @@ -834,6 +855,67 @@ static void am_skip(struct am_state *state) am_run(state); } +static int safe_to_abort(const struct am_state *state) +{ + struct strbuf sb = STRBUF_INIT; + unsigned char abort_safety[20], head[20]; + + if (file_exists(am_path(state, "dirtyindex"))) + return 0; + + if (read_state_file(&sb, am_path(state, "abort-safety"), 40, 1) > 0) { + if (get_sha1_hex(sb.buf, abort_safety)) + die(_("could not parse %s"), am_path(state, "abort_safety")); + } else + hashclr(abort_safety); + + if (get_sha1("HEAD", head)) + hashclr(head); + + if (!hashcmp(head, abort_safety)) + return 1; + + error(_("You seem to have moved HEAD since the last 'am' failure.\n" + "Not rewinding to ORIG_HEAD")); + + return 0; +} + +/** + * Aborts the current am session if it is safe to do so. + */ +static void am_abort(struct am_state *state) +{ + unsigned char curr_head[20], orig_head[20]; + int has_curr_head, has_orig_head; + const char *curr_branch; + + if (!safe_to_abort(state)) { + am_destroy(state); + return; + } + + curr_branch = resolve_refdup("HEAD", 0, curr_head, NULL); + has_curr_head = !is_null_sha1(curr_head); + if (!has_curr_head) + hashcpy(curr_head, EMPTY_TREE_SHA1_BIN); + + has_orig_head = !get_sha1("ORIG_HEAD", orig_head); + if (!has_orig_head) + hashcpy(orig_head, EMPTY_TREE_SHA1_BIN); + + clean_index(curr_head, orig_head); + + if (has_orig_head) + update_ref("am --abort", "HEAD", ori
[PATCH/WIP v2 17/19] am: implement am --signoff
Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07), git-am supported the --signoff option which will append a signoff at the end of the commit messsage. Re-implement this feature by calling append_signoff() if the option is set. Signed-off-by: Paul Tan --- builtin/am.c | 13 + 1 file changed, 13 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 4cd21b8..71fda16 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -18,6 +18,7 @@ #include "diffcore.h" #include "unpack-trees.h" #include "branch.h" +#include "sequencer.h" /** * Returns 1 if the file is empty or does not exist, 0 otherwise. @@ -73,6 +74,8 @@ struct am_state { /* override error message when patch failure occurs */ const char *resolvemsg; + + int sign; }; /** @@ -280,6 +283,9 @@ static void am_load(struct am_state *state) read_state_file(&sb, am_path(state, "quiet"), 2, 1); state->quiet = !strcmp(sb.buf, "t"); + read_state_file(&sb, am_path(state, "sign"), 2, 1); + state->sign = !strcmp(sb.buf, "t"); + strbuf_release(&sb); } @@ -463,6 +469,8 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, write_file(am_path(state, "quiet"), 1, state->quiet ? "t" : "f"); + write_file(am_path(state, "sign"), 1, state->sign ? "t" : "f"); + if (!get_sha1("HEAD", curr_head)) { write_file(am_path(state, "abort-safety"), 1, "%s", sha1_to_hex(curr_head)); update_ref("am", "ORIG_HEAD", curr_head, NULL, 0, UPDATE_REFS_DIE_ON_ERR); @@ -629,6 +637,9 @@ static int parse_patch(struct am_state *state, const char *patch) die_errno(_("could not read '%s'"), am_path(state, "msg")); stripspace(&state->msg, 0); + if (state->sign) + append_signoff(&state->msg, 0, 0); + return 0; } @@ -997,6 +1008,8 @@ static const char * const am_usage[] = { static struct option am_options[] = { OPT__QUIET(&state.quiet, N_("be quiet")), + OPT_BOOL('s', "signoff", &state.sign, + N_("add a Signed-off-by line to the commit message")), OPT_CALLBACK(0, "patch-format", &opt_patch_format, N_("format"), N_("format the patch(es) are in"), parse_opt_patchformat), OPT_STRING(0, "resolvemsg", &state.resolvemsg, NULL, -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v2 11/19] am: refuse to apply patches if index is dirty
Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07), git-am will refuse to apply patches if the index is dirty. Re-implement this behavior. Signed-off-by: Paul Tan --- builtin/am.c | 46 ++ 1 file changed, 46 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 417bfde..234762c 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -14,6 +14,8 @@ #include "refs.h" #include "commit.h" #include "lockfile.h" +#include "diff.h" +#include "diffcore.h" /** * Returns 1 if the file is empty or does not exist, 0 otherwise. @@ -472,6 +474,43 @@ static void refresh_and_write_cache(void) } /** + * Returns 1 if the index differs from HEAD, 0 otherwise. When on an unborn + * branch, returns 1 if there are entries in the index, 0 otherwise. If an + * strbuf is provided, the space-separated list of files that differ will be + * appended to it. + */ +static int index_has_changes(struct strbuf *sb) +{ + unsigned char head[GIT_SHA1_RAWSZ]; + int i; + + if (!get_sha1_tree("HEAD", head)) { + struct diff_options opt; + + diff_setup(&opt); + DIFF_OPT_SET(&opt, EXIT_WITH_STATUS); + if (!sb) + DIFF_OPT_SET(&opt, QUICK); + do_diff_cache(head, &opt); + diffcore_std(&opt); + for (i = 0; sb && i < diff_queued_diff.nr; i++) { + if (i) + strbuf_addch(sb, ' '); + strbuf_addstr(sb, diff_queued_diff.queue[i]->two->path); + } + diff_flush(&opt); + return DIFF_OPT_TST(&opt, HAS_CHANGES) != 0; + } else { + for (i = 0; sb && i < active_nr; i++) { + if (i) + strbuf_addch(sb, ' '); + strbuf_addstr(sb, active_cache[i]->name); + } + return !!active_nr; + } +} + +/** * Parses `patch` using git-mailinfo. state->msg will be set to the patch * message. state->author_name, state->author_email, state->author_date will be * set to the patch author's name, email and date respectively. The patch's @@ -612,8 +651,15 @@ static void do_commit(const struct am_state *state) */ static void am_run(struct am_state *state) { + struct strbuf sb = STRBUF_INIT; + refresh_and_write_cache(); + if (index_has_changes(&sb)) + die(_("Dirty index: cannot apply patches (dirty: %s)"), sb.buf); + + strbuf_release(&sb); + while (state->cur <= state->last) { const char *patch = am_path(state, msgnum(state)); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v2 12/19] am: implement --resolved/--continue
Since 0c15cc9 (git-am: --resolved., 2005-11-16), git-am supported resuming from a failed patch application. The user will manually apply the patch, and the run git am --resolved which will then commit the resulting index. Re-implement this feature by introducing am_resolve(). Signed-off-by: Paul Tan --- builtin/am.c | 52 +++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/builtin/am.c b/builtin/am.c index 234762c..935ffcb 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -697,6 +697,34 @@ next: } /** + * Resume the current am session after patch application failure. The user did + * all the hard work, and we do not have to do any patch application. Just + * trust and commit what the user has in the index and working tree. + */ +static void am_resolve(struct am_state *state) +{ + printf_ln(_("Applying: %s"), firstline(state->msg.buf)); + + if (!index_has_changes(NULL)) { + printf_ln(_("No changes - did you forget to use 'git add'?\n" + "If there is nothing left to stage, chances are that something else\n" + "already introduced the same changes; you might want to skip this patch.")); + exit(128); + } + + if (unmerged_cache()) { + printf_ln(_("You still have unmerged paths in your index.\n" + "Did you forget to use 'git add'?")); + exit(128); + } + + do_commit(state); + + am_next(state); + am_run(state); +} + +/** * parse_options() callback that validates and sets opt->value to the * PATCH_FORMAT_* enum value corresponding to `arg`. */ @@ -711,17 +739,30 @@ static int parse_opt_patchformat(const struct option *opt, const char *arg, int return 0; } +enum resume_mode { + RESUME_FALSE = 0, + RESUME_RESOLVED +}; + static struct am_state state; static int opt_patch_format; +static enum resume_mode opt_resume; static const char * const am_usage[] = { N_("git am [options] [(|)...]"), + N_("git am [options] --continue"), NULL }; static struct option am_options[] = { OPT_CALLBACK(0, "patch-format", &opt_patch_format, N_("format"), N_("format the patch(es) are in"), parse_opt_patchformat), + OPT_CMDMODE(0, "continue", &opt_resume, + N_("continue applying patches after resolving a conflict"), + RESUME_RESOLVED), + OPT_CMDMODE('r', "resolved", &opt_resume, + N_("synonyms for --continue"), + RESUME_RESOLVED), OPT_END() }; @@ -762,7 +803,16 @@ int cmd_am(int argc, const char **argv, const char *prefix) string_list_clear(&paths, 0); } - am_run(&state); + switch (opt_resume) { + case RESUME_FALSE: + am_run(&state); + break; + case RESUME_RESOLVED: + am_resolve(&state); + break; + default: + die("BUG: invalid resume value"); + } am_state_release(&state); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v2 13/19] am: implement --skip
Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07), git-am supported resuming from a failed patch application by skipping the current patch. Re-implement this feature by introducing am_skip(). Signed-off-by: Paul Tan --- builtin/am.c | 121 ++- 1 file changed, 119 insertions(+), 2 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 935ffcb..1434e68 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -16,6 +16,8 @@ #include "lockfile.h" #include "diff.h" #include "diffcore.h" +#include "unpack-trees.h" +#include "branch.h" /** * Returns 1 if the file is empty or does not exist, 0 otherwise. @@ -725,6 +727,114 @@ static void am_resolve(struct am_state *state) } /** + * Performs a checkout fast-forward from `head` to `remote`. If `reset` is + * true, any unmerged entries will be discarded. Returns 0 on success, -1 on + * failure. + */ +static int fast_forward_to(struct tree *head, struct tree *remote, int reset) +{ + struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file)); + struct unpack_trees_options opts; + struct tree_desc t[2]; + + if (parse_tree(head) || parse_tree(remote)) + return -1; + + hold_locked_index(lock_file, 1); + + refresh_cache(REFRESH_QUIET); + + memset(&opts, 0, sizeof(opts)); + opts.head_idx = 1; + opts.src_index = &the_index; + opts.dst_index = &the_index; + opts.update = 1; + opts.merge = 1; + opts.reset = reset; + opts.fn = twoway_merge; + init_tree_desc(&t[0], head->buffer, head->size); + init_tree_desc(&t[1], remote->buffer, remote->size); + + if (unpack_trees(2, t, &opts)) { + rollback_lock_file(lock_file); + return -1; + } + + if (write_locked_index(&the_index, lock_file, COMMIT_LOCK)) + die(_("unable to write new index file")); + + return 0; +} + +/** + * Clean the index without touching entries that are not modified between + * `head` and `remote`. + */ +static int clean_index(const unsigned char *head, const unsigned char *remote) +{ + struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file)); + struct tree *head_tree, *remote_tree, *index_tree; + unsigned char index[GIT_SHA1_RAWSZ]; + struct pathspec pathspec; + + head_tree = parse_tree_indirect(head); + if (!head_tree) + return error(_("Could not parse object '%s'."), sha1_to_hex(head)); + + remote_tree = parse_tree_indirect(remote); + if (!remote_tree) + return error(_("Could not parse object '%s'."), sha1_to_hex(remote)); + + read_cache_unmerged(); + + if (fast_forward_to(head_tree, head_tree, 1)) + return -1; + + if (write_cache_as_tree(index, 0, NULL)) + return -1; + + index_tree = parse_tree_indirect(index); + if (!index_tree) + return error(_("Could not parse object '%s'."), sha1_to_hex(index)); + + if (fast_forward_to(index_tree, remote_tree, 0)) + return -1; + + memset(&pathspec, 0, sizeof(pathspec)); + + hold_locked_index(lock_file, 1); + + if (read_tree(remote_tree, 0, &pathspec)) { + rollback_lock_file(lock_file); + return -1; + } + + if (write_locked_index(&the_index, lock_file, COMMIT_LOCK)) + die(_("unable to write new index file")); + + remove_branch_state(); + + return 0; +} + +/** + * Resume the current am session by skipping the current patch. + */ +static void am_skip(struct am_state *state) +{ + unsigned char head[GIT_SHA1_RAWSZ]; + + if (get_sha1("HEAD", head)) + hashcpy(head, EMPTY_TREE_SHA1_BIN); + + if (clean_index(head, head)) + die(_("failed to clean index")); + + am_next(state); + am_run(state); +} + +/** * parse_options() callback that validates and sets opt->value to the * PATCH_FORMAT_* enum value corresponding to `arg`. */ @@ -741,7 +851,8 @@ static int parse_opt_patchformat(const struct option *opt, const char *arg, int enum resume_mode { RESUME_FALSE = 0, - RESUME_RESOLVED + RESUME_RESOLVED, + RESUME_SKIP }; static struct am_state state; @@ -750,7 +861,7 @@ static enum resume_mode opt_resume; static const char * const am_usage[] = { N_("git am [options] [(|)...]"), - N_("git am [options] --continue"), + N_("git am [options] (--continue | --skip)"), NULL }; @@ -763,6 +874,9 @@ static struct option am_options[] = { OPT_CMDMODE('r', "resolved", &opt_resume, N
[PATCH/WIP v2 09/19] am: commit applied patch
Implement do_commit(), which commits the index which contains the results of applying the patch, along with the extracted commit message and authorship information. Signed-off-by: Paul Tan --- builtin/am.c | 50 ++ 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index b725a74..ecc6d29 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -10,6 +10,9 @@ #include "dir.h" #include "run-command.h" #include "quote.h" +#include "cache-tree.h" +#include "refs.h" +#include "commit.h" /** * Returns 1 if the file is empty or does not exist, 0 otherwise. @@ -548,6 +551,48 @@ static int run_apply(const struct am_state *state) } /** + * Commits the current index with state->msg as the commit message and + * state->author_name, state->author_email and state->author_date as the author + * information. + */ +static void do_commit(const struct am_state *state) +{ + unsigned char tree[20], parent[20], commit[20]; + unsigned char *ptr; + struct commit_list *parents = NULL; + const char *reflog_msg, *author; + struct strbuf sb = STRBUF_INIT; + + if (write_cache_as_tree(tree, 0, NULL)) + die(_("git write-tree failed to write a tree")); + + if (!get_sha1_commit("HEAD", parent)) { + ptr = parent; + commit_list_insert(lookup_commit(parent), &parents); + } else { + ptr = NULL; + fprintf_ln(stderr, _("applying to an empty history")); + } + + author = fmt_ident(state->author_name.buf, state->author_email.buf, + state->author_date.buf, IDENT_STRICT); + + if (commit_tree(state->msg.buf, state->msg.len, tree, parents, commit, + author, NULL)) + die(_("failed to write commit object")); + + reflog_msg = getenv("GIT_REFLOG_ACTION"); + if (!reflog_msg) + reflog_msg = "am"; + + strbuf_addf(&sb, "%s: %s", reflog_msg, firstline(state->msg.buf)); + + update_ref(sb.buf, "HEAD", commit, ptr, 0, UPDATE_REFS_DIE_ON_ERR); + + strbuf_release(&sb); +} + +/** * Applies all queued patches. */ static void am_run(struct am_state *state) @@ -579,10 +624,7 @@ static void am_run(struct am_state *state) exit(128); } - /* -* TODO: After the patch has been applied to the index with -* git-apply, we need to make commit as well. -*/ + do_commit(state); next: am_next(state); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v2 03/19] am: implement skeletal builtin am
For the purpose of rewriting git-am.sh into a C builtin, implement a skeletal builtin/am.c that redirects to $GIT_EXEC_PATH/git-am if the environment variable _GIT_USE_BUILTIN_AM is not defined. Since in the Makefile git-am.sh takes precedence over builtin/am.c, $GIT_EXEC_PATH/git-am will contain the shell script git-am.sh, and thus this allows us to fall back on the functional git-am.sh when running the test suite for tests that depend on a working git-am implementation. This redirection will be removed when all the features of git-am.sh have been re-implemented in builtin/am.c. Signed-off-by: Paul Tan --- Notes: v2 * Declare struct am_state state as static. Makefile | 1 + builtin.h| 1 + builtin/am.c | 20 git.c| 1 + 4 files changed, 23 insertions(+) create mode 100644 builtin/am.c diff --git a/Makefile b/Makefile index 54ec511..b82ba0e 100644 --- a/Makefile +++ b/Makefile @@ -812,6 +812,7 @@ LIB_OBJS += xdiff-interface.o LIB_OBJS += zlib.o BUILTIN_OBJS += builtin/add.o +BUILTIN_OBJS += builtin/am.o BUILTIN_OBJS += builtin/annotate.o BUILTIN_OBJS += builtin/apply.o BUILTIN_OBJS += builtin/archive.o diff --git a/builtin.h b/builtin.h index b87df70..d50c9d1 100644 --- a/builtin.h +++ b/builtin.h @@ -30,6 +30,7 @@ extern int textconv_object(const char *path, unsigned mode, const unsigned char extern int is_builtin(const char *s); extern int cmd_add(int argc, const char **argv, const char *prefix); +extern int cmd_am(int argc, const char **argv, const char *prefix); extern int cmd_annotate(int argc, const char **argv, const char *prefix); extern int cmd_apply(int argc, const char **argv, const char *prefix); extern int cmd_archive(int argc, const char **argv, const char *prefix); diff --git a/builtin/am.c b/builtin/am.c new file mode 100644 index 000..0ccbe33 --- /dev/null +++ b/builtin/am.c @@ -0,0 +1,20 @@ +/* + * Builtin "git am" + * + * Based on git-am.sh by Junio C Hamano. + */ +#include "cache.h" +#include "builtin.h" +#include "exec_cmd.h" + +int cmd_am(int argc, const char **argv, const char *prefix) +{ + if (!getenv("_GIT_USE_BUILTIN_AM")) { + const char *path = mkpath("%s/git-am", git_exec_path()); + + if (sane_execvp(path, (char**) argv) < 0) + die_errno("could not exec %s", path); + } + + return 0; +} diff --git a/git.c b/git.c index 44374b1..42328ed 100644 --- a/git.c +++ b/git.c @@ -370,6 +370,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv) static struct cmd_struct commands[] = { { "add", cmd_add, RUN_SETUP | NEED_WORK_TREE }, + { "am", cmd_am, RUN_SETUP | NEED_WORK_TREE }, { "annotate", cmd_annotate, RUN_SETUP }, { "apply", cmd_apply, RUN_SETUP_GENTLY }, { "archive", cmd_archive }, -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v2 06/19] am: detect mbox patches
Since 15ced75 (git-am foreign patch support: autodetect some patch formats, 2009-05-27), git-am.sh is able to autodetect mbox, stgit and mercurial patches through heuristics. Re-implement support for autodetecting mbox/maildir files. Helped-by: Eric Sunshine Signed-off-by: Paul Tan --- Notes: v2 * Various small code tweaks suggested by Eric. builtin/am.c | 98 1 file changed, 98 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 5198a8e..7379b97 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -120,6 +120,96 @@ static void am_destroy(const struct am_state *state) strbuf_release(&sb); } +/* + * Returns 1 if the file looks like a piece of email a-la RFC2822, 0 otherwise. + * We check this by grabbing all the non-indented lines and seeing if they look + * like they begin with valid header field names. + */ +static int is_email(const char *filename) +{ + struct strbuf sb = STRBUF_INIT; + FILE *fp = xfopen(filename, "r"); + int ret = 1; + + while (!strbuf_getline(&sb, fp, '\n')) { + const char *x; + + strbuf_rtrim(&sb); + + if (!sb.len) + break; /* End of header */ + + /* Ignore indented folded lines */ + if (*sb.buf == '\t' || *sb.buf == ' ') + continue; + + /* It's a header if it matches the regexp "^[!-9;-~]+:" */ + for (x = sb.buf; *x; x++) { + if (('!' <= *x && *x <= '9') || (';' <= *x && *x <= '~')) + continue; + if (*x == ':' && x != sb.buf) + break; + ret = 0; + goto done; + } + } + +done: + fclose(fp); + strbuf_release(&sb); + return ret; +} + +/** + * Attempts to detect the patch_format of the patches contained in `paths`, + * returning the PATCH_FORMAT_* enum value. Returns PATCH_FORMAT_UNKNOWN if + * detection fails. + */ +static int detect_patch_format(struct string_list *paths) +{ + enum patch_format ret = PATCH_FORMAT_UNKNOWN; + struct strbuf l1 = STRBUF_INIT; + struct strbuf l2 = STRBUF_INIT; + struct strbuf l3 = STRBUF_INIT; + FILE *fp; + + /* +* We default to mbox format if input is from stdin and for directories +*/ + if (!paths->nr || !strcmp(paths->items->string, "-") || + is_directory(paths->items->string)) { + ret = PATCH_FORMAT_MBOX; + goto done; + } + + /* +* Otherwise, check the first 3 lines of the first patch, starting +* from the first non-blank line, to try to detect its format. +*/ + fp = xfopen(paths->items->string, "r"); + while (!strbuf_getline(&l1, fp, '\n')) { + strbuf_trim(&l1); + if (l1.len) + break; + } + strbuf_getline(&l2, fp, '\n'); + strbuf_trim(&l2); + strbuf_getline(&l3, fp, '\n'); + strbuf_trim(&l3); + fclose(fp); + + if (starts_with(l1.buf, "From ") || starts_with(l1.buf, "From: ")) + ret = PATCH_FORMAT_MBOX; + else if (l1.len && l2.len && l3.len && is_email(paths->items->string)) + ret = PATCH_FORMAT_MBOX; + +done: + strbuf_release(&l1); + strbuf_release(&l2); + strbuf_release(&l3); + return ret; +} + /** * Splits out individual patches from `paths`, where each path is either a mbox * file or a Maildir. Return 0 on success, -1 on failure. @@ -174,6 +264,14 @@ static int split_patches(struct am_state *state, enum patch_format patch_format, static void am_setup(struct am_state *state, enum patch_format patch_format, struct string_list *paths) { + if (!patch_format) + patch_format = detect_patch_format(paths); + + if (!patch_format) { + fprintf_ln(stderr, _("Patch format detection failed.")); + exit(128); + } + if (mkdir(state->dir.buf, 0777) < 0 && errno != EEXIST) die_errno(_("failed to create directory '%s'"), state->dir.buf); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v2 01/19] wrapper: implement xopen()
A common usage pattern of open() is to check if it was successful, and die() if it was not: int fd = open(path, O_WRONLY | O_CREAT, 0777); if (fd < 0) die_errno(_("Could not open '%s' for writing."), path); Implement a wrapper function xopen() that does the above so that we can save a few lines of code, and make the die() messages consistent. Helped-by: Torsten Bögershausen Helped-by: Jeff King Signed-off-by: Paul Tan --- Notes: v2 * retry on EINTR * mode is now an optional argument in xopen(). We use the mode argument only if O_CREAT is specified in oflag. git-compat-util.h | 1 + wrapper.c | 25 + 2 files changed, 26 insertions(+) diff --git a/git-compat-util.h b/git-compat-util.h index 17584ad..95cc278 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -718,6 +718,7 @@ extern char *xstrndup(const char *str, size_t len); extern void *xrealloc(void *ptr, size_t size); extern void *xcalloc(size_t nmemb, size_t size); extern void *xmmap(void *start, size_t length, int prot, int flags, int fd, off_t offset); +extern int xopen(const char *path, int flags, ...); extern ssize_t xread(int fd, void *buf, size_t len); extern ssize_t xwrite(int fd, const void *buf, size_t len); extern ssize_t xpread(int fd, void *buf, size_t len, off_t offset); diff --git a/wrapper.c b/wrapper.c index c1a663f..82658b3 100644 --- a/wrapper.c +++ b/wrapper.c @@ -189,6 +189,31 @@ void *xcalloc(size_t nmemb, size_t size) # endif #endif +/** + * xopen() is the same as open(), but it die()s if the open() fails. + */ +int xopen(const char *path, int oflag, ...) +{ + mode_t mode = 0; + va_list ap; + + va_start(ap, oflag); + if (oflag & O_CREAT) + mode = va_arg(ap, mode_t); + va_end(ap); + + assert(path); + + for (;;) { + int fd = open(path, oflag, mode); + if (fd >= 0) + return fd; + if (errno == EINTR) + continue; + die_errno(_("could not open '%s'"), path); + } +} + /* * xread() is the same a read(), but it automatically restarts read() * operations with a recoverable error (EAGAIN and EINTR). xread() -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v2 04/19] am: implement patch queue mechanism
git-am applies a series of patches. If the process terminates abnormally, we want to be able to resume applying the series of patches. This requires the session state to be saved in a persistent location. Implement the mechanism of a "patch queue", represented by 2 integers -- the index of the current patch we are applying and the index of the last patch, as well as its lifecycle through the following functions: * am_setup(), which will set up the state directory $GIT_DIR/rebase-apply. As such, even if the process exits abnormally, the last-known state will still persist. * am_load(), which is called if there is an am session in progress, to load the last known state from the state directory so we can resume applying patches. * am_run(), which will do the actual patch application. After applying a patch, it calls am_next() to increment the current patch index. The logic for applying and committing a patch is not implemented yet. * am_destroy(), which is finally called when we successfully applied all the patches in the queue, to clean up by removing the state directory and its contents. Signed-off-by: Paul Tan --- Notes: v2 * Declare struct am_state as static builtin/am.c | 164 +++ 1 file changed, 164 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 0ccbe33..f061d21 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -6,6 +6,154 @@ #include "cache.h" #include "builtin.h" #include "exec_cmd.h" +#include "parse-options.h" +#include "dir.h" + +struct am_state { + /* state directory path */ + struct strbuf dir; + + /* current and last patch numbers, 1-indexed */ + int cur; + int last; +}; + +/** + * Initializes am_state with the default values. + */ +static void am_state_init(struct am_state *state) +{ + memset(state, 0, sizeof(*state)); + + strbuf_init(&state->dir, 0); +} + +/** + * Release memory allocated by an am_state. + */ +static void am_state_release(struct am_state *state) +{ + strbuf_release(&state->dir); +} + +/** + * Returns path relative to the am_state directory. + */ +static inline const char *am_path(const struct am_state *state, const char *path) +{ + return mkpath("%s/%s", state->dir.buf, path); +} + +/** + * Returns 1 if there is an am session in progress, 0 otherwise. + */ +static int am_in_progress(const struct am_state *state) +{ + struct stat st; + + if (lstat(state->dir.buf, &st) < 0 || !S_ISDIR(st.st_mode)) + return 0; + if (lstat(am_path(state, "last"), &st) || !S_ISREG(st.st_mode)) + return 0; + if (lstat(am_path(state, "next"), &st) || !S_ISREG(st.st_mode)) + return 0; + return 1; +} + +/** + * Reads the contents of `file`. The third argument can be used to give a hint + * about the file size, to avoid reallocs. Returns number of bytes read on + * success, -1 if the file does not exist. If trim is set, trailing whitespace + * will be removed from the file contents. + */ +static int read_state_file(struct strbuf *sb, const char *file, size_t hint, int trim) +{ + strbuf_reset(sb); + if (strbuf_read_file(sb, file, hint) >= 0) { + if (trim) + strbuf_rtrim(sb); + + return sb->len; + } + + if (errno == ENOENT) + return -1; + + die_errno(_("could not read '%s'"), file); +} + +/** + * Loads state from disk. + */ +static void am_load(struct am_state *state) +{ + struct strbuf sb = STRBUF_INIT; + + read_state_file(&sb, am_path(state, "next"), 8, 1); + state->cur = strtol(sb.buf, NULL, 10); + + read_state_file(&sb, am_path(state, "last"), 8, 1); + state->last = strtol(sb.buf, NULL, 10); + + strbuf_release(&sb); +} + +/** + * Remove the am_state directory. + */ +static void am_destroy(const struct am_state *state) +{ + struct strbuf sb = STRBUF_INIT; + + strbuf_addstr(&sb, state->dir.buf); + remove_dir_recursively(&sb, 0); + strbuf_release(&sb); +} + +/** + * Setup a new am session for applying patches + */ +static void am_setup(struct am_state *state) +{ + if (mkdir(state->dir.buf, 0777) < 0 && errno != EEXIST) + die_errno(_("failed to create directory '%s'"), state->dir.buf); + + write_file(am_path(state, "next"), 1, "%d", state->cur); + + write_file(am_path(state, "last"), 1, "%d", state->last); +} + +/** + * Increments the patch pointer, and cleans am_state for the application of the + * next patch. + */ +static void am_next(struct am_state *state) +{ + state->cur++; + write_fil
[PATCH/WIP v2 02/19] wrapper: implement xfopen()
A common usage pattern of fopen() is to check if it succeeded, and die() if it failed: FILE *fp = fopen(path, "w"); if (!fp) die_errno(_("could not open '%s' for writing"), path); Implement a wrapper function xfopen() for the above, so that we can save a few lines of code and make the die() messages consistent. Signed-off-by: Paul Tan --- Notes: v2 * Removed the error message distinction between reading and writing. * Handle EINTR. git-compat-util.h | 1 + wrapper.c | 18 ++ 2 files changed, 19 insertions(+) diff --git a/git-compat-util.h b/git-compat-util.h index 95cc278..03ea3a2 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -723,6 +723,7 @@ extern ssize_t xread(int fd, void *buf, size_t len); extern ssize_t xwrite(int fd, const void *buf, size_t len); extern ssize_t xpread(int fd, void *buf, size_t len, off_t offset); extern int xdup(int fd); +extern FILE *xfopen(const char *path, const char *mode); extern FILE *xfdopen(int fd, const char *mode); extern int xmkstemp(char *template); extern int xmkstemp_mode(char *template, int mode); diff --git a/wrapper.c b/wrapper.c index 82658b3..9692460 100644 --- a/wrapper.c +++ b/wrapper.c @@ -336,6 +336,24 @@ int xdup(int fd) return ret; } +/** + * xfopen() is the same as fopen(), but it die()s if the fopen() fails. + */ +FILE *xfopen(const char *path, const char *mode) +{ + assert(path); + assert(mode); + + for (;;) { + FILE *fp = fopen(path, mode); + if (fp) + return fp; + if (errno == EINTR) + continue; + die_errno(_("could not open '%s'"), path); + } +} + FILE *xfdopen(int fd, const char *mode) { FILE *stream = fdopen(fd, mode); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v2 07/19] am: extract patch, message and authorship with git-mailinfo
For the purpose of applying the patch and committing the results, implement extracting the patch data, commit message and authorship from an e-mail message using git-mailinfo. git-mailinfo is run as a separate process, but ideally in the future, we should be be able to access its functionality directly without spawning a new process. Helped-by: Junio C Hamano Helped-by: Jeff King Signed-off-by: Paul Tan --- Notes: v2 * use die_errno() * use '%*d' as the format specifier for msgnum() builtin/am.c | 228 +++ 1 file changed, 228 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 7379b97..a1db474 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -9,6 +9,23 @@ #include "parse-options.h" #include "dir.h" #include "run-command.h" +#include "quote.h" + +/** + * Returns 1 if the file is empty or does not exist, 0 otherwise. + */ +static int is_empty_file(const char *filename) +{ + struct stat st; + + if (stat(filename, &st) < 0) { + if (errno == ENOENT) + return 1; + die_errno(_("could not stat %s"), filename); + } + + return !st.st_size; +} enum patch_format { PATCH_FORMAT_UNKNOWN = 0, @@ -23,6 +40,12 @@ struct am_state { int cur; int last; + /* commit message and metadata */ + struct strbuf author_name; + struct strbuf author_email; + struct strbuf author_date; + struct strbuf msg; + /* number of digits in patch filename */ int prec; }; @@ -35,6 +58,10 @@ static void am_state_init(struct am_state *state) memset(state, 0, sizeof(*state)); strbuf_init(&state->dir, 0); + strbuf_init(&state->author_name, 0); + strbuf_init(&state->author_email, 0); + strbuf_init(&state->author_date, 0); + strbuf_init(&state->msg, 0); state->prec = 4; } @@ -44,6 +71,10 @@ static void am_state_init(struct am_state *state) static void am_state_release(struct am_state *state) { strbuf_release(&state->dir); + strbuf_release(&state->author_name); + strbuf_release(&state->author_email); + strbuf_release(&state->author_date); + strbuf_release(&state->msg); } /** @@ -93,6 +124,95 @@ static int read_state_file(struct strbuf *sb, const char *file, size_t hint, int } /** + * Parses the "author script" `filename`, and sets state->author_name, + * state->author_email and state->author_date accordingly. We are strict with + * our parsing, as the author script is supposed to be eval'd, and loosely + * parsing it may not give the results the user expects. + * + * The author script is of the format: + * + * GIT_AUTHOR_NAME='$author_name' + * GIT_AUTHOR_EMAIL='$author_email' + * GIT_AUTHOR_DATE='$author_date' + * + * where $author_name, $author_email and $author_date are quoted. + */ +static int read_author_script(struct am_state *state) +{ + char *value; + struct strbuf sb = STRBUF_INIT; + const char *filename = am_path(state, "author-script"); + FILE *fp = fopen(filename, "r"); + if (!fp) { + if (errno == ENOENT) + return 0; + die_errno(_("could not open '%s' for reading"), filename); + } + + if (strbuf_getline(&sb, fp, '\n')) + return -1; + if (!skip_prefix(sb.buf, "GIT_AUTHOR_NAME=", (const char**) &value)) + return -1; + value = sq_dequote(value); + if (!value) + return -1; + strbuf_reset(&state->author_name); + strbuf_addstr(&state->author_name, value); + + if (strbuf_getline(&sb, fp, '\n')) + return -1; + if (!skip_prefix(sb.buf, "GIT_AUTHOR_EMAIL=", (const char**) &value)) + return -1; + value = sq_dequote(value); + if (!value) + return -1; + strbuf_reset(&state->author_email); + strbuf_addstr(&state->author_email, value); + + if (strbuf_getline(&sb, fp, '\n')) + return -1; + if (!skip_prefix(sb.buf, "GIT_AUTHOR_DATE=", (const char**) &value)) + return -1; + value = sq_dequote(value); + if (!value) + return -1; + strbuf_reset(&state->author_date); + strbuf_addstr(&state->author_date, value); + + if (fgetc(fp) != EOF) + return -1; + + fclose(fp); + strbuf_release(&sb); + return 0; +} + +/** + * Saves state->author_name, state->author_email and state->author_date in + * `file
[PATCH/WIP v2 05/19] am: split out mbox/maildir patches with git-mailsplit
git-am.sh supports mbox, stgit and mercurial patches. Re-implement support for splitting out mbox/maildirs using git-mailsplit, while also implementing the framework required to support other patch formats in the future. Re-implement support for the --patch-format option (since a5a6755 (git-am foreign patch support: introduce patch_format, 2009-05-27)) to allow the user to choose between the different patch formats. Signed-off-by: Paul Tan --- Notes: v2 * Declare int opt_patchformat as static. * Fix up indentation style for the switch() builtin/am.c | 107 --- 1 file changed, 103 insertions(+), 4 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index f061d21..5198a8e 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -8,6 +8,12 @@ #include "exec_cmd.h" #include "parse-options.h" #include "dir.h" +#include "run-command.h" + +enum patch_format { + PATCH_FORMAT_UNKNOWN = 0, + PATCH_FORMAT_MBOX +}; struct am_state { /* state directory path */ @@ -16,6 +22,9 @@ struct am_state { /* current and last patch numbers, 1-indexed */ int cur; int last; + + /* number of digits in patch filename */ + int prec; }; /** @@ -26,6 +35,7 @@ static void am_state_init(struct am_state *state) memset(state, 0, sizeof(*state)); strbuf_init(&state->dir, 0); + state->prec = 4; } /** @@ -111,13 +121,67 @@ static void am_destroy(const struct am_state *state) } /** + * Splits out individual patches from `paths`, where each path is either a mbox + * file or a Maildir. Return 0 on success, -1 on failure. + */ +static int split_patches_mbox(struct am_state *state, struct string_list *paths) +{ + struct child_process cp = CHILD_PROCESS_INIT; + struct string_list_item *item; + struct strbuf last = STRBUF_INIT; + + cp.git_cmd = 1; + argv_array_push(&cp.args, "mailsplit"); + argv_array_pushf(&cp.args, "-d%d", state->prec); + argv_array_pushf(&cp.args, "-o%s", state->dir.buf); + argv_array_push(&cp.args, "-b"); + argv_array_push(&cp.args, "--"); + + for_each_string_list_item(item, paths) + argv_array_push(&cp.args, item->string); + + if (capture_command(&cp, &last, 8)) + return -1; + + state->cur = 1; + state->last = strtol(last.buf, NULL, 10); + + return 0; +} + +/** + * Splits out individual patches, of patch_format, contained within paths. + * These patches will be stored in the state directory, with each patch's + * filename being its index, padded to state->prec digits. state->cur will be + * set to the index of the first patch, and state->last will be set to the + * index of the last patch. Returns 0 on success, -1 on failure. + */ +static int split_patches(struct am_state *state, enum patch_format patch_format, + struct string_list *paths) +{ + switch (patch_format) { + case PATCH_FORMAT_MBOX: + return split_patches_mbox(state, paths); + default: + die("BUG: invalid patch_format"); + } + return -1; +} + +/** * Setup a new am session for applying patches */ -static void am_setup(struct am_state *state) +static void am_setup(struct am_state *state, enum patch_format patch_format, + struct string_list *paths) { if (mkdir(state->dir.buf, 0777) < 0 && errno != EEXIST) die_errno(_("failed to create directory '%s'"), state->dir.buf); + if (split_patches(state, patch_format, paths) < 0) { + am_destroy(state); + die(_("Failed to split patches.")); + } + write_file(am_path(state, "next"), 1, "%d", state->cur); write_file(am_path(state, "last"), 1, "%d", state->last); @@ -138,13 +202,33 @@ static void am_next(struct am_state *state) */ static void am_run(struct am_state *state) { - while (state->cur <= state->last) + while (state->cur <= state->last) { + + /* TODO: Patch application not implemented yet */ + am_next(state); + } am_destroy(state); } +/** + * parse_options() callback that validates and sets opt->value to the + * PATCH_FORMAT_* enum value corresponding to `arg`. + */ +static int parse_opt_patchformat(const struct option *opt, const char *arg, int unset) +{ + int *opt_value = opt->value; + + if (!strcmp(arg, "mbox")) + *opt_value = PATCH_FORMAT_MBOX; + else + return -1; + return 0; +} + static struct am_state state; +static int opt_patch_format; static
[PATCH/WIP v2 00/19] Make git-am a builtin
This is a re-roll of [v1]. Thanks Junio, Torsten, Jeff, Eric for the reviews last round. Previous versions: [v1] http://thread.gmane.org/gmane.comp.version-control.git/270048 git-am is a commonly used command for applying a series of patches from a mailbox to the current branch. Currently, it is implemented by the shell script git-am.sh. However, compared to C, shell scripts have certain deficiencies: they need to spawn a lot of processes, introduce a lot of dependencies and cannot take advantage of git's internal caches. This WIP patch series rewrites git-am.sh into optimized C builtin/am.c, and is part of my GSoC project to rewrite git-pull and git-am into C builtins[1]. [1] https://gist.github.com/pyokagan/1b7b0d1f4dab6ba3cef1 Paul Tan (19): wrapper: implement xopen() wrapper: implement xfopen() am: implement skeletal builtin am am: implement patch queue mechanism am: split out mbox/maildir patches with git-mailsplit am: detect mbox patches am: extract patch, message and authorship with git-mailinfo am: apply patch with git-apply am: commit applied patch am: refresh the index at start am: refuse to apply patches if index is dirty am: implement --resolved/--continue am: implement --skip am: implement --abort am: implement quiet option am: exit with user friendly message on patch failure am: implement am --signoff cache-tree: introduce write_index_as_tree() am: implement 3-way merge Makefile |1 + builtin.h |1 + builtin/am.c | 1214 + cache-tree.c | 29 +- cache-tree.h |1 + git-compat-util.h |2 + git.c |1 + wrapper.c | 43 ++ 8 files changed, 1280 insertions(+), 12 deletions(-) create mode 100644 builtin/am.c -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 00/19] Make git-am a builtin
This is a re-roll of [v1]. Thanks Junio, Torsten, Jeff, Eric for the reviews last round. Previous versions: [v1] http://thread.gmane.org/gmane.comp.version-control.git/270048 git-am is a commonly used command for applying a series of patches from a mailbox to the current branch. Currently, it is implemented by the shell script git-am.sh. However, compared to C, shell scripts have certain deficiencies: they need to spawn a lot of processes, introduce a lot of dependencies and cannot take advantage of git's internal caches. This WIP patch series rewrites git-am.sh into optimized C builtin/am.c, and is part of my GSoC project to rewrite git-pull and git-am into C builtins[1]. [1] https://gist.github.com/pyokagan/1b7b0d1f4dab6ba3cef1 Paul Tan (19): wrapper: implement xopen() wrapper: implement xfopen() am: implement skeletal builtin am am: implement patch queue mechanism am: split out mbox/maildir patches with git-mailsplit am: detect mbox patches am: extract patch, message and authorship with git-mailinfo am: apply patch with git-apply am: commit applied patch am: refresh the index at start am: refuse to apply patches if index is dirty am: implement --resolved/--continue am: implement --skip am: implement --abort am: implement quiet option am: exit with user friendly message on patch failure am: implement am --signoff cache-tree: introduce write_index_as_tree() am: implement 3-way merge Makefile |1 + builtin.h |1 + builtin/am.c | 1214 + cache-tree.c | 29 +- cache-tree.h |1 + git-compat-util.h |2 + git.c |1 + wrapper.c | 43 ++ 8 files changed, 1280 insertions(+), 12 deletions(-) create mode 100644 builtin/am.c -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 15/19] pull: teach git pull about --rebase
On Wed, Jun 10, 2015 at 10:44 PM, Junio C Hamano wrote: > Paul Tan writes: > >>> Hmph, it is somewhat surprising that we do not have such a helper >>> already. Wouldn't we need this logic to implement $branch@{upstream} >>> syntax? >> >> Right, the @{upstream} syntax is implemented by branch_get_upstream() >> in remote.c. It, however, does not check to see if the branch's remote >> matches what is provided on the command-line, so we still have to >> implement this check ourselves, which means this helper function is >> still required. >> >> I guess we could still use branch_get_upstream() in this function though. > > It is entirely expected that existing function may not do exactly > what the new caller you introduce might want to do, or may do more > than what it wants. That is where refactoring of existing code > comes in. > > It somewhat feels strange that you have to write more than "shim" > code to glue existing helpers and API functions together to > re-implement what a scripted Porcelain is already doing, though. > It can't be that git-pull.sh implements this logic as shell script, > and it must be asking existing code in Git to do what the callers > you added for this function would want to do, right? Not git-pull.sh, but get_remote_merge_branch() git-parse-remote.sh. The shell code that get_upstream_branch() in this patch implements is: 0|1) origin="$1" default=$(get_default_remote) test -z "$origin" && origin=$default curr_branch=$(git symbolic-ref -q HEAD) && [ "$origin" = "$default" ] && ^ This here is where it checks to see if the branch's configured remote matches the remote provided on the command line. echo $(git for-each-ref --format='%(upstream)' $curr_branch) ;; ^ While here it calls git to get the upstream branch, which is implemented by branch_get_upstream() on the C side. So yes, we can use branch_get_upstream(), but we still need to implement some code on top. Just to add on, the shell code that get_tracking_branch() in this patch implements is: *) repo=$1 shift ref=$1 # FIXME: It should return the tracking branch #Currently only works with the default mapping case "$ref" in +*) ref=$(expr "z$ref" : 'z+\(.*\)') ;; esac expr "z$ref" : 'z.*:' >/dev/null || ref="${ref}:" remote=$(expr "z$ref" : 'z\([^:]*\):') case "$remote" in '' | HEAD ) remote=HEAD ;; heads/*) remote=${remote#heads/} ;; refs/heads/*) remote=${remote#refs/heads/} ;; refs/* | tags/* | remotes/* ) remote= esac [ -n "$remote" ] && case "$repo" in .) echo "refs/heads/$remote" ;; *) echo "refs/remotes/$repo/$remote" ;; esac so it's more or less a direct translation of the shell script, and we can be sure it will have the same behavior. I'm definitely in favor of switching this to use remote_find_tracking(), the question is whether we want to do it in this patch or in a future patch on top. Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 11/19] pull: check if in unresolved merge state
On Wed, Jun 10, 2015 at 10:38 PM, Junio C Hamano wrote: > Junio C Hamano writes: > >> Paul Tan writes: >> >>> @@ -422,6 +423,14 @@ int cmd_pull(int argc, const char **argv, const char >>> *prefix) >>> >>> parse_repo_refspecs(argc, argv, &repo, &refspecs); >>> >>> +git_config(git_default_config, NULL); >>> + >>> +if (read_cache_unmerged()) >>> +die_resolve_conflict("Pull"); >>> + >>> +if (file_exists(git_path("MERGE_HEAD"))) >>> +die_conclude_merge(); >>> + >>> if (!opt_ff.len) >>> config_get_ff(&opt_ff); >> >> Hmph. >> >> If you are going to do the git_config() call yourself, it might make >> more sense to define git_pull_config() callback and parse the pull.ff >> yourself, updating the use of the lazy git_config_get_value() API you >> introduced in patch 10/19. >> >> The above "might" is stronger than my usual "might"; I am undecided >> yet before reading the remainder of the series. > > Let me clarify the above with s/stronger/with much less certainty/; Uh, I have no idea what a "strong might" or a "less certain might" is. :-/ Parsing all the config values with a git_config() callback function is certainly possible, but I was under the impression that we are moving towards migrating use of all the git_config() callback loops to the git_config_get_X() API. In this case though, we have to use git_config() to initialize the advice.resolveConflict config setting, but I don't see why it must be in conflict with the above goal. Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 15/19] pull: teach git pull about --rebase
On Wed, Jun 10, 2015 at 9:56 AM, Junio C Hamano wrote: > Paul Tan writes: > >> +enum rebase_type { >> + REBASE_INVALID = -1, >> + REBASE_FALSE = 0, >> + REBASE_TRUE, >> + REBASE_PRESERVE >> +}; >> + >> +/** >> + * Parses the value of --rebase, branch.*.rebase or pull.rebase. If value >> is a >> + * false value, returns REBASE_FALSE. If value is a true value, returns >> + * REBASE_TRUE. If value is "preserve", returns REBASE_PRESERVE. Otherwise, >> + * returns -1 to signify an invalid value. >> + */ >> +static enum rebase_type parse_config_rebase(const char *value) >> +{ >> + int v = git_config_maybe_bool("pull.rebase", value); >> + if (!v) >> + return REBASE_FALSE; >> + else if (v >= 0) >> + return REBASE_TRUE; > > It is somewhat misleading to say "v >= 0" when you already use !v to > signal something else. Perhaps "else if (v > 0)" is better? Ah, right. >> +/** >> + * Returns remote's upstream branch for the current branch. If remote is >> NULL, >> + * the current branch's configured default remote is used. Returns NULL if >> + * `remote` does not name a valid remote, HEAD does not point to a branch, >> + * remote is not the branch's configured remote or the branch does not have >> any >> + * configured upstream branch. >> + */ >> +static char *get_upstream_branch(const char *remote) >> +{ >> + struct remote *rm; >> + struct branch *curr_branch; >> + >> + rm = remote_get(remote); >> + if (!rm) >> + return NULL; >> + >> + curr_branch = branch_get("HEAD"); >> + if (!curr_branch) >> + return NULL; >> + >> + if (curr_branch->remote != rm) >> + return NULL; >> + >> + if (!curr_branch->merge_nr) >> + return NULL; >> + >> + return xstrdup(curr_branch->merge[0]->dst); >> +} > > Hmph, it is somewhat surprising that we do not have such a helper > already. Wouldn't we need this logic to implement $branch@{upstream} > syntax? Right, the @{upstream} syntax is implemented by branch_get_upstream() in remote.c. It, however, does not check to see if the branch's remote matches what is provided on the command-line, so we still have to implement this check ourselves, which means this helper function is still required. I guess we could still use branch_get_upstream() in this function though. >> +/** >> + * Derives the remote tracking branch from the remote and refspec. >> + * >> + * FIXME: The current implementation assumes the default mapping of >> + * refs/heads/ to refs/remotes//. >> + */ >> +static char *get_tracking_branch(const char *remote, const char *refspec) >> +{ > > This does smell like an incomplete reimplementation of what > get_fetch_map() knows how to do. Yeah, this is just a direct rewrite of get_remote_merge_branch() in git-parse-remote.sh. Johannes pointed out[1] that remote_find_tracking() in remote.c does the exact same thing without the assumption of the default fetch refmap. However, this would be a separate modification on its own, so it may be better to do it in a separate patch with regression tests. (e.g. what should we do if the refspec dst is provided?) [1] http://thread.gmane.org/gmane.comp.version-control.git/269258/focus=269350 >> +/** >> + * Given the repo and refspecs, sets fork_point to the point at which the >> + * current branch forked from its remote tracking branch. Returns 0 on >> success, >> + * -1 on failure. >> + */ >> +static int get_rebase_fork_point(unsigned char fork_point[GIT_SHA1_RAWSZ], >> + const char *repo, const char *refspec) >> +{ >> +... >> +} > > This function looks OK (the two get_*_branch() helpers it uses I am > not sure about though). > > Same comment on "fork_point[]" parameter's type applies here, > though. While I do not mind if you used "struct object_id" to > represent these object names, if you are sticking to the traditional > "unsigned char [20]", then these should be "unsigned char *" to be > consistent with others. Okay. Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 02/19] parse-options-cb: implement parse_opt_pass_argv_array()
On Wed, Jun 10, 2015 at 7:16 AM, Junio C Hamano wrote: > Almost the same comment as 01/19 applies to this comment. > > I think it makes good sense to have two variants, one that lets the > last one win and pass only that last one (i.e. 01/19) and the other > that accumulates them into an argv_array (i.e. this one). But it > feels iffy, given that the "acculate" version essentially creates an > array of (char *), to make "the last one wins, leaving a single > string" to use strbuf. I'd find it much more understandable if 01/19 > took (char **) as opt->value instead of a strbuf. I don't see how it feels iffy. The purpose of using strbufs (and argv_arrays) is to avoid error-prone manual memory management. > In any case, these two need to be added as a related pair to the API > documentation. Okay, I guess I could also add their macro functions as well. Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] am: teach StGit patch parser how to read from stdin
On Tue, Jun 9, 2015 at 3:57 AM, Junio C Hamano wrote: > Paul Tan writes: > >> git-mailsplit, which splits mbox patches, will read the patch from stdin >> when the filename is "-" or there are no files listed on the >> command-line. >> >> To be consistent with this behavior, teach the StGit patch parser to >> read from stdin if the filename is "-" or no files are listed on the >> command-line. > > Hmm, doesn't > > perl -ne 'processing for each line' > > with or without a BEGIN {} block, read from the standard input (if > no filename is given) or the given file (if given), and more > importantly, doesn't it treat a lone "-" as STDIN anyway? > > That is, wouldn't it make more sense to do something like: > > test $# != 0 || set -- - > for stgit > do > ... > @@PERL@@ -ne 'BEGIN { $subject = 0 } > ... > ' "$stgit" >"$dotest/$msgnum" || clean_abort > done > > Same for patch 5/5. Ah yes, this makes more sense. Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6] am --abort: support aborting to unborn branch
On Tue, Jun 9, 2015 at 4:10 AM, Junio C Hamano wrote: > Paul Tan writes: > >> When git-am is first run on an unborn branch, no ORIG_HEAD is created. >> As such, any applied commits will remain even after a git am --abort. > > I think this answered my question on 4/6; that may indicate that > these two are either done in a wrong order or perhaps should be a > single change. Ah right, patch 4/6 was too sneaky in that it tried to do the "support unborn branch" thing as well, which should only be handled in this patch. I still think the patches should be separate though since they are conceptually different. 4/6 modifies the "index clean up" function, while 5/6 modifies the "reset HEAD" function. Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] am --abort: keep unrelated commits on unborn branch
On Tue, Jun 9, 2015 at 4:13 AM, Junio C Hamano wrote: > Paul Tan writes: > >> Since 7b3b7e3 (am --abort: keep unrelated commits since the last failure >> and warn, 2010-12-21), git-am would refuse to rewind HEAD if commits >> were made since the last git-am failure. This check was implemented in >> safe_to_abort(), which checked to see if HEAD's hash matched the >> abort-safety file. >> >> However, this check was skipped if the abort-safety file was empty, >> which can happen if git-am failed while on an unborn branch. > > Shouldn't we then be checking that the HEAD is still unborn if this > file is found and says that there was no history in the beginning, > in order to give the "am on top of unborn" workflow the same degree > of safety? We do already check to see if the HEAD is still unborn: abort_safety=$(cat "$dotest/abort-safety") if test "z$(git rev-parse --verify -q HEAD)" = "z$abort_safety" then return 0 fi gettextln "You seem to have moved HEAD since the last 'am' failure. Not rewinding to ORIG_HEAD" >&2 If HEAD is unborn, then git rev-parse will not print anything, so we would be comparing an empty string to an empty string. Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] am: teach mercurial patch parser how to read from stdin
git-mailsplit, which splits mbox patches, will read the patch from stdin when the filename is "-" or there are no files listed on the command-line. To be consistent with this behavior, teach the mercurial patch parser to read from stdin if the filename is "-" or no files are listed on the command-line. Based-on-patch-by: Chris Packham Signed-off-by: Paul Tan --- git-am.sh | 4 +++- t/t4150-am.sh | 10 ++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/git-am.sh b/git-am.sh index d97da85..0a40d46 100755 --- a/git-am.sh +++ b/git-am.sh @@ -327,6 +327,7 @@ split_patches () { ;; hg) this=0 + test 0 -eq "$#" && set -- - for hg in "$@" do this=$(( $this + 1 )) @@ -338,6 +339,7 @@ split_patches () { # Since we cannot guarantee that the commit message is in # git-friendly format, we put no Subject: line and just consume # all of the message as the body + cat "$hg" | LANG=C LC_ALL=C @@PERL@@ -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 } if ($subject) { print ; } elsif (/^\# User /) { s/\# User/From:/ ; print ; } @@ -353,7 +355,7 @@ split_patches () { print "\n", $_ ; $subject = 1; } - ' <"$hg" >"$dotest/$msgnum" || clean_abort + ' >"$dotest/$msgnum" || clean_abort done echo "$this" >"$dotest/last" this= diff --git a/t/t4150-am.sh b/t/t4150-am.sh index 4beb4b3..3ebafd9 100755 --- a/t/t4150-am.sh +++ b/t/t4150-am.sh @@ -259,6 +259,16 @@ test_expect_success 'am applies hg patch' ' test_cmp_rev second^ HEAD^ ' +test_expect_success 'am --patch-format=hg applies hg patch' ' + rm -fr .git/rebase-apply && + git checkout -f first && + git am --patch-format=hg http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] am: use gmtime() to parse mercurial patch date
An example of the line in a mercurial patch that specifies the date of the commit would be: # Date 1433753301 25200 where the first number is the number of seconds since the unix epoch (in UTC), and the second number is the offset of the timezone, in second s west of UTC (negative if the timezone is east of UTC). git-am uses localtime() to break down the first number into its components (year, month, day, hours, minutes, seconds etc.). However, the returned components are relative to the user's time zone. As a result, if the user's time zone does not match the time zone specified in the patch, the resulting commit will have the wrong author date. Fix this by using gmtime() instead, which uses UTC instead of the user's time zone. Signed-off-by: Paul Tan --- git-am.sh | 6 +++--- t/t4150-am.sh | 23 +++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/git-am.sh b/git-am.sh index 83f2ea6..d97da85 100755 --- a/git-am.sh +++ b/git-am.sh @@ -343,11 +343,11 @@ split_patches () { elsif (/^\# User /) { s/\# User/From:/ ; print ; } elsif (/^\# Date /) { my ($hashsign, $str, $time, $tz) = split ; - $tz = sprintf "%+05d", (0-$tz)/36; + $tz_str = sprintf "%+05d", (0-$tz)/36; print "Date: " . strftime("%a, %d %b %Y %H:%M:%S ", - localtime($time)) - . "$tz\n"; + gmtime($time-$tz)) + . "$tz_str\n"; } elsif (/^\# /) { next ; } else { print "\n", $_ ; diff --git a/t/t4150-am.sh b/t/t4150-am.sh index 7aad8f8..4beb4b3 100755 --- a/t/t4150-am.sh +++ b/t/t4150-am.sh @@ -122,6 +122,19 @@ test_expect_success setup ' echo "# This series applies on GIT commit $(git rev-parse first)" && echo "patch" } >stgit-series/series && + { + echo "# HG changeset patch" && + echo "# User $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>" && + echo "# Date $test_tick 25200" && + echo "# $(git show --pretty="%aD" -s second)" && + echo "# Node ID $_z40" && + echo "# Parent $_z40" && + cat msg && + echo && + echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" && + echo && + git diff-tree --no-commit-id -p second + } >patch1-hg.eml && sed -n -e "3,\$p" msg >file && @@ -236,6 +249,16 @@ test_expect_success 'am applies stgit series' ' test_cmp_rev second^ HEAD^ ' +test_expect_success 'am applies hg patch' ' + rm -fr .git/rebase-apply && + git checkout -f first && + git am patch1-hg.eml && + test_path_is_missing .git/rebase-apply && + git diff --exit-code second && + test_cmp_rev second HEAD && + test_cmp_rev second^ HEAD^ +' + test_expect_success 'setup: new author and committer' ' GIT_AUTHOR_NAME="Another Thor" && GIT_AUTHOR_EMAIL="a.t...@example.com" && -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] t4150: test applying StGit patch
By default, an StGit patch separates the subject from the commit message and headers as follows: $subject From: $author_name <$author_email> $message --- $diffstats We test git-am's ability to detect such a patch as an StGit patch, and its ability to be able to extract the commit author, date and message from such a patch. Based-on-patch-by: Chris Packham Signed-off-by: Paul Tan --- t/t4150-am.sh | 22 ++ 1 file changed, 22 insertions(+) diff --git a/t/t4150-am.sh b/t/t4150-am.sh index 306e6f3..0ead529 100755 --- a/t/t4150-am.sh +++ b/t/t4150-am.sh @@ -104,6 +104,18 @@ test_expect_success setup ' echo "X-Fake-Field: Line Three" && git format-patch --stdout first | sed -e "1d" } > patch1-ws.eml && + { + sed -ne "1p" msg && + echo && + echo "From: $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>" && + echo "Date: $GIT_AUTHOR_DATE" && + echo && + sed -e "1,2d" msg && + echo && + echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" && + echo "---" && + git diff-tree --no-commit-id --stat -p second + } >patch1-stgit.eml && sed -n -e "3,\$p" msg >file && git add file && @@ -187,6 +199,16 @@ test_expect_success 'am applies patch e-mail with preceding whitespace' ' test "$(git rev-parse second^)" = "$(git rev-parse HEAD^)" ' +test_expect_success 'am applies stgit patch' ' + rm -fr .git/rebase-apply && + git checkout -f first && + git am patch1-stgit.eml && + test_path_is_missing .git/rebase-apply && + git diff --exit-code second && + test_cmp_rev second HEAD && + test_cmp_rev second^ HEAD^ +' + test_expect_success 'setup: new author and committer' ' GIT_AUTHOR_NAME="Another Thor" && GIT_AUTHOR_EMAIL="a.t...@example.com" && -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] t4150: test applying StGit series
A StGit series is a directory containing a "series" file which begins with the line: # This series applies on GIT commit X where X is the commit ID that the patch series applies on. Every following line names a patch in the directory to be applied. Test that git-am, when given this "series" file, is able to detect it as an StGit series and apply all the patches in the series. Signed-off-by: Paul Tan --- t/t4150-am.sh | 17 + 1 file changed, 17 insertions(+) diff --git a/t/t4150-am.sh b/t/t4150-am.sh index 51962e4..7aad8f8 100755 --- a/t/t4150-am.sh +++ b/t/t4150-am.sh @@ -116,6 +116,13 @@ test_expect_success setup ' echo "---" && git diff-tree --no-commit-id --stat -p second } >patch1-stgit.eml && + mkdir stgit-series && + cp patch1-stgit.eml stgit-series/patch && + { + echo "# This series applies on GIT commit $(git rev-parse first)" && + echo "patch" + } >stgit-series/series && + sed -n -e "3,\$p" msg >file && git add file && @@ -219,6 +226,16 @@ test_expect_success 'am --patch-format=stgit applies stgit patch' ' test_cmp_rev second^ HEAD^ ' +test_expect_success 'am applies stgit series' ' + rm -fr .git/rebase-apply && + git checkout -f first && + git am stgit-series/series && + test_path_is_missing .git/rebase-apply && + git diff --exit-code second && + test_cmp_rev second HEAD && + test_cmp_rev second^ HEAD^ +' + test_expect_success 'setup: new author and committer' ' GIT_AUTHOR_NAME="Another Thor" && GIT_AUTHOR_EMAIL="a.t...@example.com" && -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] am: teach StGit patch parser how to read from stdin
git-mailsplit, which splits mbox patches, will read the patch from stdin when the filename is "-" or there are no files listed on the command-line. To be consistent with this behavior, teach the StGit patch parser to read from stdin if the filename is "-" or no files are listed on the command-line. Based-on-patch-by: Chris Packham Signed-off-by: Paul Tan --- git-am.sh | 5 +++-- t/t4150-am.sh | 10 ++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/git-am.sh b/git-am.sh index 761befb..83f2ea6 100755 --- a/git-am.sh +++ b/git-am.sh @@ -297,6 +297,7 @@ split_patches () { ;; stgit) this=0 + test 0 -eq "$#" && set -- - for stgit in "$@" do this=$(expr "$this" + 1) @@ -305,7 +306,7 @@ split_patches () { # not starting with Author, From or Date is the # subject, and the body starts with the next nonempty # line not starting with Author, From or Date - @@PERL@@ -ne 'BEGIN { $subject = 0 } + cat "$stgit" | @@PERL@@ -ne 'BEGIN { $subject = 0 } if ($subject > 1) { print ; } elsif (/^\s+$/) { next ; } elsif (/^Author:/) { s/Author/From/ ; print ;} @@ -318,7 +319,7 @@ split_patches () { print "Subject: ", $_ ; $subject = 1; } - ' < "$stgit" > "$dotest/$msgnum" || clean_abort + ' >"$dotest/$msgnum" || clean_abort done echo "$this" > "$dotest/last" this= diff --git a/t/t4150-am.sh b/t/t4150-am.sh index 0ead529..51962e4 100755 --- a/t/t4150-am.sh +++ b/t/t4150-am.sh @@ -209,6 +209,16 @@ test_expect_success 'am applies stgit patch' ' test_cmp_rev second^ HEAD^ ' +test_expect_success 'am --patch-format=stgit applies stgit patch' ' + rm -fr .git/rebase-apply && + git checkout -f first && + git am --patch-format=stgit http://vger.kernel.org/majordomo-info.html
[PATCH 0/5] am: improve test coverage and touch up foreign patch parsing
git-am is able to parse StGit and mercurial patches. However, there are no regression tests in the test suite for these patch formats, and there are some small bugs as well: * the mercurial and stgit patch parsers does not support reading from stdin * the mercurial patch parser parsed the patch date wrongly and git-am is thus unable to reconstruct the exact commit. Most patches are based on Chris' patch series[1], which I've credited accordingly. [1] http://thread.gmane.org/gmane.comp.version-control.git/256502 Paul Tan (5): t4150: test applying StGit patch am: teach StGit patch parser how to read from stdin t4150: test applying StGit series am: use gmtime() to parse mercurial patch date am: teach mercurial patch parser how to read from stdin git-am.sh | 15 ++- t/t4150-am.sh | 82 +++ 2 files changed, 91 insertions(+), 6 deletions(-) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/6] am -3: support 3way merge on unborn branch
While on an unborn branch, git am -3 will fail to do a threeway merge as it references HEAD as "our tree", but HEAD does not point to a valid tree. Fix this by using an empty tree as "our tree" when we are on an unborn branch. Signed-off-by: Paul Tan --- git-am.sh | 3 ++- t/t4151-am-abort.sh | 9 + 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/git-am.sh b/git-am.sh index df3c8f4..c847b58 100755 --- a/git-am.sh +++ b/git-am.sh @@ -179,7 +179,8 @@ It does not apply to blobs recorded in its index.")" then GIT_MERGE_VERBOSITY=0 && export GIT_MERGE_VERBOSITY fi -git-merge-recursive $orig_tree -- HEAD $his_tree || { +our_tree=$(git rev-parse --verify -q HEAD || echo $empty_tree) +git-merge-recursive $orig_tree -- $our_tree $his_tree || { git rerere $allow_rerere_autoupdate die "$(gettext "Failed to merge in the changes.")" } diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh index e15acc8..b618ee0 100755 --- a/t/t4151-am-abort.sh +++ b/t/t4151-am-abort.sh @@ -83,4 +83,13 @@ test_expect_success 'am --abort will keep the local commits intact' ' test_cmp expect actual ' +test_expect_success 'am -3 stops on conflict on unborn branch' ' + git checkout -f --orphan orphan && + git reset && + rm -f otherfile-4 && + test_must_fail git am -3 0003-*.patch && + test 2 -eq $(git ls-files -u | wc -l) && + test 4 = "$(cat otherfile-4)" +' + test_done -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/6] am --abort: support aborting to unborn branch
When git-am is first run on an unborn branch, no ORIG_HEAD is created. As such, any applied commits will remain even after a git am --abort. To be consistent with the behavior of git am --abort when it is not run from an unborn branch, we empty the index, and then destroy the branch pointed to by HEAD if there is no ORIG_HEAD. Signed-off-by: Paul Tan --- git-am.sh | 9 - t/t4151-am-abort.sh | 17 + 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/git-am.sh b/git-am.sh index e4fe3ed..95f90a0 100755 --- a/git-am.sh +++ b/git-am.sh @@ -524,7 +524,14 @@ then index_tree=$(git write-tree) && orig_head=$(git rev-parse --verify -q ORIG_HEAD || echo $empty_tree) && git read-tree -m -u $index_tree $orig_head - git reset ORIG_HEAD + if git rev-parse --verify -q ORIG_HEAD >/dev/null 2>&1 + then + git reset ORIG_HEAD + else + git read-tree $empty_tree + curr_branch=$(git symbolic-ref HEAD 2>/dev/null) && + git update-ref -d $curr_branch + fi fi rm -fr "$dotest" exit ;; diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh index 2366042..b2a7fc5 100755 --- a/t/t4151-am-abort.sh +++ b/t/t4151-am-abort.sh @@ -14,6 +14,7 @@ test_expect_success setup ' git add file-1 file-2 && git commit -m initial && git tag initial && + git format-patch --stdout --root initial >initial.patch && for i in 2 3 4 5 6 do echo $i >>file-1 && @@ -125,4 +126,20 @@ test_expect_success 'am -3 --abort removes otherfile-4 on unborn branch' ' test_path_is_missing otherfile-4 ' +test_expect_success 'am -3 --abort on unborn branch removes applied commits' ' + git checkout -f --orphan orphan && + git reset && + rm -f otherfile-4 otherfile-2 file-1 file-2 && + test_must_fail git am -3 initial.patch 0003-*.patch && + test 3 -eq $(git ls-files -u | wc -l) && + test 4 = "$(cat otherfile-4)" && + git am --abort && + test -z "$(git ls-files -u)" && + test_path_is_missing otherfile-4 && + test_path_is_missing file-1 && + test_path_is_missing file-2 && + test 0 -eq $(git log --oneline 2>/dev/null | wc -l) && + test refs/heads/orphan = "$(git symbolic-ref HEAD)" +' + test_done -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/6] am --abort: keep unrelated commits on unborn branch
Since 7b3b7e3 (am --abort: keep unrelated commits since the last failure and warn, 2010-12-21), git-am would refuse to rewind HEAD if commits were made since the last git-am failure. This check was implemented in safe_to_abort(), which checked to see if HEAD's hash matched the abort-safety file. However, this check was skipped if the abort-safety file was empty, which can happen if git-am failed while on an unborn branch. As such, if any commits were made since then, they would be discarded. Fix this by carrying on the abort safety check even if the abort-safety file is empty. Signed-off-by: Paul Tan --- git-am.sh | 2 +- t/t4151-am-abort.sh | 11 +++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/git-am.sh b/git-am.sh index 95f90a0..4324bb1 100755 --- a/git-am.sh +++ b/git-am.sh @@ -87,7 +87,7 @@ safe_to_abort () { return 1 fi - if ! test -s "$dotest/abort-safety" + if ! test -f "$dotest/abort-safety" then return 0 fi diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh index b2a7fc5..833e7b2 100755 --- a/t/t4151-am-abort.sh +++ b/t/t4151-am-abort.sh @@ -142,4 +142,15 @@ test_expect_success 'am -3 --abort on unborn branch removes applied commits' ' test refs/heads/orphan = "$(git symbolic-ref HEAD)" ' +test_expect_success 'am --abort on unborn branch will keep local commits intact' ' + git checkout -f --orphan orphan && + git reset && + test_must_fail git am 0004-*.patch && + test_commit unrelated2 && + git rev-parse HEAD >expect && + git am --abort && + git rev-parse HEAD >actual && + test_cmp expect actual +' + test_done -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/6] am --abort: revert changes introduced by failed 3way merge
Even when a merge conflict occurs with am --3way, the index will be modified with the results of any successfully merged files. These changes to the index will not be reverted with a "git read-tree --reset -u HEAD ORIG_HEAD", as git read-tree will not be aware of how the current index differs from HEAD or ORIG_HEAD. To fix this, we first reset any conflicting entries in the index. The resulting index will contain the results of successfully merged files introduced by the failed merge. We write this index to a tree, and then use git read-tree to fast-forward this "index tree" back to ORIG_HEAD, thus undoing all the changes from the failed merge. When we are on an unborn branch, HEAD and ORIG_HEAD will not point to valid trees. In this case, use an empty tree. Signed-off-by: Paul Tan --- git-am.sh | 6 +- t/t4151-am-abort.sh | 23 +++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/git-am.sh b/git-am.sh index 67f4f25..e4fe3ed 100755 --- a/git-am.sh +++ b/git-am.sh @@ -519,7 +519,11 @@ then git rerere clear if safe_to_abort then - git read-tree --reset -u HEAD ORIG_HEAD + head_tree=$(git rev-parse --verify -q HEAD || echo $empty_tree) && + git read-tree --reset -u $head_tree $head_tree && + index_tree=$(git write-tree) && + orig_head=$(git rev-parse --verify -q ORIG_HEAD || echo $empty_tree) && + git read-tree -m -u $index_tree $orig_head git reset ORIG_HEAD fi rm -fr "$dotest" diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh index ea4a49e..2366042 100755 --- a/t/t4151-am-abort.sh +++ b/t/t4151-am-abort.sh @@ -70,6 +70,17 @@ test_expect_success 'am -3 --skip removes otherfile-4' ' test 4 = "$(cat otherfile-4)" && git am --skip && test_cmp_rev initial HEAD && + test -z "$(git ls-files -u)" && + test_path_is_missing otherfile-4 +' + +test_expect_success 'am -3 --abort removes otherfile-4' ' + git reset --hard initial && + test_must_fail git am -3 0003-*.patch && + test 3 -eq $(git ls-files -u | wc -l) && + test 4 = "$(cat otherfile-4)" && + git am --abort && + test_cmp_rev initial HEAD && test -z $(git ls-files -u) && test_path_is_missing otherfile-4 ' @@ -102,4 +113,16 @@ test_expect_success 'am -3 --skip clears index on unborn branch' ' test_path_is_missing tmpfile ' +test_expect_success 'am -3 --abort removes otherfile-4 on unborn branch' ' + git checkout -f --orphan orphan && + git reset && + rm -f otherfile-4 file-1 && + test_must_fail git am -3 0003-*.patch && + test 2 -eq $(git ls-files -u | wc -l) && + test 4 = "$(cat otherfile-4)" && + git am --abort && + test -z "$(git ls-files -u)" && + test_path_is_missing otherfile-4 +' + test_done -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/6] am --skip: support skipping while on unborn branch
When git am --skip is run, git am will copy HEAD's tree entries to the index with "git reset HEAD". However, on an unborn branch, HEAD does not point to a tree, so "git reset HEAD" will fail. Fix this by treating HEAD as en empty tree when we are on an unborn branch. Signed-off-by: Paul Tan --- git-am.sh | 4 +--- t/t4151-am-abort.sh | 10 ++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/git-am.sh b/git-am.sh index c847b58..67f4f25 100755 --- a/git-am.sh +++ b/git-am.sh @@ -509,9 +509,7 @@ then git read-tree --reset -u $head_tree $head_tree && index_tree=$(git write-tree) && git read-tree -m -u $index_tree $head_tree - orig_head=$(cat "$GIT_DIR/ORIG_HEAD") - git reset HEAD - git update-ref ORIG_HEAD $orig_head + git read-tree $head_tree ;; ,t) if test -f "$dotest/rebasing" diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh index b618ee0..ea4a49e 100755 --- a/t/t4151-am-abort.sh +++ b/t/t4151-am-abort.sh @@ -92,4 +92,14 @@ test_expect_success 'am -3 stops on conflict on unborn branch' ' test 4 = "$(cat otherfile-4)" ' +test_expect_success 'am -3 --skip clears index on unborn branch' ' + test_path_is_dir .git/rebase-apply && + echo tmpfile >tmpfile && + git add tmpfile && + git am --skip && + test -z "$(git ls-files)" && + test_path_is_missing otherfile-4 && + test_path_is_missing tmpfile +' + test_done -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/6] am --skip: revert changes introduced by failed 3way merge
Even when a merge conflict occurs with am --3way, the index will be modified with the results of any succesfully merged files (such as a new file). These changes to the index will not be reverted with a "git read-tree --reset -u HEAD HEAD", as git read-tree will not be aware of how the current index differs from HEAD. To fix this, we first reset any conflicting entries from the index. The resulting index will contain the results of successfully merged files. We write the index to a tree, then use git read-tree -m to fast-forward the "index tree" back to HEAD, thus undoing all the changes from the failed merge. Signed-off-by: Paul Tan --- git-am.sh | 7 ++- t/t4151-am-abort.sh | 11 +++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/git-am.sh b/git-am.sh index 761befb..df3c8f4 100755 --- a/git-am.sh +++ b/git-am.sh @@ -69,6 +69,8 @@ then cmdline="$cmdline -3" fi +empty_tree=4b825dc642cb6eb9a060e54bf8d69288fbee4904 + sq () { git rev-parse --sq-quote "$@" } @@ -502,7 +504,10 @@ then ;; t,) git rerere clear - git read-tree --reset -u HEAD HEAD + head_tree=$(git rev-parse --verify -q HEAD || echo $empty_tree) && + git read-tree --reset -u $head_tree $head_tree && + index_tree=$(git write-tree) && + git read-tree -m -u $index_tree $head_tree orig_head=$(cat "$GIT_DIR/ORIG_HEAD") git reset HEAD git update-ref ORIG_HEAD $orig_head diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh index 8d90634..e15acc8 100755 --- a/t/t4151-am-abort.sh +++ b/t/t4151-am-abort.sh @@ -63,6 +63,17 @@ do done +test_expect_success 'am -3 --skip removes otherfile-4' ' + git reset --hard initial && + test_must_fail git am -3 0003-*.patch && + test 3 -eq $(git ls-files -u | wc -l) && + test 4 = "$(cat otherfile-4)" && + git am --skip && + test_cmp_rev initial HEAD && + test -z $(git ls-files -u) && + test_path_is_missing otherfile-4 +' + test_expect_success 'am --abort will keep the local commits intact' ' test_must_fail git am 0004-*.patch && test_commit unrelated && -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/6] am --skip/--abort: improve index/worktree cleanup
Currently git-am attempts to clean up the index/worktree when skipping or aborting, but does not do it very well: * While it discards conflicted index entries, it does not remove any other modifications made to the index due to a previous threeway merge. * It expects HEAD/ORIG_HEAD to exist, and thus does not clean up the index when on an unborn branch. This patch series addresses the above two general problems by making the calls to git-read-tree aware of the differences between our current index and HEAD/ORIG_HEAD, and by explictly defining what happens when we are on an unborn branch. Paul Tan (6): am --skip: revert changes introduced by failed 3way merge am -3: support 3way merge on unborn branch am --skip: support skipping while on unborn branch am --abort: revert changes introduced by failed 3way merge am --abort: support aborting to unborn branch am --abort: keep unrelated commits on unborn branch git-am.sh | 31 ++-- t/t4151-am-abort.sh | 81 + 2 files changed, 104 insertions(+), 8 deletions(-) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] git-am: handling unborn branches
On Fri, Jun 5, 2015 at 11:36 PM, Junio C Hamano wrote: > Paul Tan writes: > >> Hmm, actually git-am.sh doesn't seem to do that even when we have a >> history to apply patches to. This is okay in the non-3way case, as >> git-apply will check to see if the patch applies before it modifies >> the index, but if we fall back on 3-way merge, any new files the >> failed merge added will not be deleted in the "git read-tree --reset >> -u HEAD HEAD". >> >> Should we do that? > > That sounds like the right thing to do; I agree that fixing it may > or may not be outside the scope of the immediate series. Hmm, thinking about it, the equivalent C code would be greatly affected by whatever behavior we go with, so I think we should try fixing the behavior first. This was done really quickly, but I think this may fix it: diff --git a/git-am.sh b/git-am.sh index 761befb..f7d54bf 100755 --- a/git-am.sh +++ b/git-am.sh @@ -502,7 +502,9 @@ then ;; t,) git rerere clear -git read-tree --reset -u HEAD HEAD +git read-tree --reset HEAD HEAD && +our_tree=$(git write-tree) && +git read-tree -m -u $our_tree HEAD orig_head=$(cat "$GIT_DIR/ORIG_HEAD") git reset HEAD git update-ref ORIG_HEAD $orig_head diff --git a/t/t4153-am-clean-index.sh b/t/t4153-am-clean-index.sh new file mode 100755 index 000..6d696db --- /dev/null +++ b/t/t4153-am-clean-index.sh @@ -0,0 +1,30 @@ +#!/bin/sh + +test_description='test clean index with am' +. ./test-lib.sh + +test_expect_success setup ' +test_commit initial file && +test_commit master-commit file && +git checkout -b conflict master^ && +echo conflict-commit >file && +echo newfile >newfile && +git add newfile && +test_tick && +git commit -a -m conflict-commit && +git format-patch --stdout initial >conflict.patch && +git checkout master +' + +test_expect_success 'am -3 pauses on conflict' ' +test_must_fail git am -3 conflict.patch && +echo newfile >expected && +test_cmp newfile expected +' + +test_expect_success 'am --skip removes newfile' ' +git am --skip && +test_path_is_missing newfile +' + +test_done However, it assumes that the contents of the index are from the failed merge. If the user modified the index before running git-am --skip, e.g. the user added a file, then that file would be deleted, which may not be desired... Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] git-am: handling unborn branches
On Fri, Jun 5, 2015 at 1:27 AM, Stefan Beller wrote: > On Thu, Jun 4, 2015 at 3:34 AM, Paul Tan wrote: >> Or, the current behavior of git-am.sh will print some scary errors >> about the missing HEAD, but will then continue on to the next patch. >> If the index is not clean, it will then error out. Should we preserve >> this behavior? (without the errors about the missing HEAD) >> >> 2. am --abort >> >> For git am --abort, git-am.sh does something similar. It does a >> fast-forward checkout from HEAD to ORIG_HEAD, discarding unmerged >> entries, and then resets the index to ORIG_HEAD so that local changes >> will be unstaged. >> >> if safe_to_abort >> then >> git read-tree --reset -u HEAD ORIG_HEAD >> git reset ORIG_HEAD >> fi >> rm -fr "$dotest" >> >> This, however, requires a valid HEAD and ORIG_HEAD. If we don't have a >> HEAD or ORIG_HEAD (because we were on an unborn branch when we started >> git am), what should we do? (Note: safe_to_abort returns true if we >> git am with no HEAD because $dotest/abort-safety will not exist) >> Should we discard all entires in the index as well? (Since we might >> think of the "original HEAD" as an empty tree?) >> >> Or, the current behavior of git-am.sh will print some scary errors >> about the missing HEAD and ORIG_HEAD, but will not touch the index at >> all, and still delete the rebase-apply directory. Should we preserve >> this behavior (without the errors)? > > I guess so, looking at the documentation >--abort >Restore the original branch and abort the patching operation. > > a user may want to not go to the unborn branch, but rather to the previous > HEAD? I think we need to be consistent with the non-unborn-branch behavior of git-am. In normal use am --abort would reset the branch back to ORIG_HEAD, which is the HEAD when am was first run, thus losing all the applied commits. However, since f79d4c8 (git-am: teach git-am to apply a patch to an unborn branch, 2009-04-10) if git-am is run without a HEAD, no ORIG_HEAD would be created. I guess if we are to follow this normal behavior, we need to destroy the current branch as well. So the abort logic would be something like: 1. If we are still on an unborn branch, we clear the index. 2. If we are not on an unborn branch, but we have no ORIG_HEAD because we started from an unborn branch, then we destroy the current branch. 3. If we are not on an unborn branch, and we have an ORIG_HEAD, then we do the normal behavior of fast-forwarding and resetting the index to ORIG_HEAD. We also need to consider safe_to_abort() as well though, which was introduced in 7b3b7e3 (am --abort: keep unrelated commits since the last failure and warn, 2010-12-21). Specifically, safe_to_abort () { ... if ! test -s "$dotest/abort-safety" then return 0 fi ... } where we are allowed to reset HEAD if the abort-safety file is empty or does not exist. If patch application failed while we are on an unborn branch, we will have no abort-safety file. However, the user may have made commits since then, and we should not make the user lose those commits. As such, we should probably not reset HEAD if there is no abort-safety file. Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] git-am: handling unborn branches
On Fri, Jun 5, 2015 at 1:26 AM, Junio C Hamano wrote: > Paul Tan writes: > >> git-am generally supports applying patches to unborn branches. >> However, there are 2 cases where git-am does not handle unborn >> branches which I would like to address before the git-am rewrite to C: > >> 1. am --skip >> >> For git am --skip, git-am.sh does a fast-forward checkout from HEAD to >> HEAD, discarding unmerged entries, and then resets the index to HEAD >> so that the index is not dirty. >> >> git read-tree --reset -u HEAD HEAD >> orig_head=$(cat "$GIT_DIR/ORIG_HEAD") >> git reset HEAD >> git update-ref ORIG_HEAD $orig_head >> >> This requires a valid HEAD. Since git-am requires an empty index for >> unborn branches in the patch application stage anyway, I think we >> should discard all entires in the index if we are on an unborn branch? > > Yes, and it should also remove the new files the failed application > brought in to the working tree, if any, to match the "--skip" done > in the normal case (i.e. when we already have a history to apply > patches to), I would think. Hmm, actually git-am.sh doesn't seem to do that even when we have a history to apply patches to. This is okay in the non-3way case, as git-apply will check to see if the patch applies before it modifies the index, but if we fall back on 3-way merge, any new files the failed merge added will not be deleted in the "git read-tree --reset -u HEAD HEAD". Should we do that? I dunno, but if we want to introduce this feature, I think a "git read-tree --reset HEAD HEAD && git read-tree -m -u $(git write-tree) HEAD" will do the trick? (We discard all unmerged entries, then fast-forward from the tree the failed merged left us with back to HEAD). Clearing the index in the unborn branch case seems to be the most consistent thing to do for now (for the purpose of the git-am rewrite). Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/WIP 1/8] wrapper: implement xopen()
On Thu, May 28, 2015 at 3:03 AM, Torsten Bögershausen wrote: > On 2015-05-27 15.33, Paul Tan wrote: >> +/** >> + * xopen() is the same as open(), but it die()s if the open() fails. >> + */ >> +int xopen(const char *path, int flags, mode_t mode) >> +{ >> + int fd; >> + >> + assert(path); >> + fd = open(path, flags, mode); >> + if (fd < 0) { >> + if ((flags & O_WRONLY) || (flags & O_RDWR)) >> + die_errno(_("could not open '%s' for writing"), path); > This is only partly true: > it could be either "writing" or "read write". Ah right, I see now that the POSIX spec allows for, and encourages O_RDONLY | O_WRONLY == O_RDWR. > I don't know if the info "for reading" or "for writing" is needed/helpful at > all, > or if a simple "could not open" would be enough. Yeah, I agree that it may not be helpful, but I noticed that most error messages in git are of the form "unable to open X for writing", "unable to open X for reading", "could not create X" etc. Or rather I thought I noticed, but it now seems to me that there are quite a lot of uses of "could not open X" as well. I guess I will remove the distinction. Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] git-am: handling unborn branches
Hi, git-am generally supports applying patches to unborn branches. However, there are 2 cases where git-am does not handle unborn branches which I would like to address before the git-am rewrite to C: 1. am --skip For git am --skip, git-am.sh does a fast-forward checkout from HEAD to HEAD, discarding unmerged entries, and then resets the index to HEAD so that the index is not dirty. git read-tree --reset -u HEAD HEAD orig_head=$(cat "$GIT_DIR/ORIG_HEAD") git reset HEAD git update-ref ORIG_HEAD $orig_head This requires a valid HEAD. Since git-am requires an empty index for unborn branches in the patch application stage anyway, I think we should discard all entires in the index if we are on an unborn branch? Or, the current behavior of git-am.sh will print some scary errors about the missing HEAD, but will then continue on to the next patch. If the index is not clean, it will then error out. Should we preserve this behavior? (without the errors about the missing HEAD) 2. am --abort For git am --abort, git-am.sh does something similar. It does a fast-forward checkout from HEAD to ORIG_HEAD, discarding unmerged entries, and then resets the index to ORIG_HEAD so that local changes will be unstaged. if safe_to_abort then git read-tree --reset -u HEAD ORIG_HEAD git reset ORIG_HEAD fi rm -fr "$dotest" This, however, requires a valid HEAD and ORIG_HEAD. If we don't have a HEAD or ORIG_HEAD (because we were on an unborn branch when we started git am), what should we do? (Note: safe_to_abort returns true if we git am with no HEAD because $dotest/abort-safety will not exist) Should we discard all entires in the index as well? (Since we might think of the "original HEAD" as an empty tree?) Or, the current behavior of git-am.sh will print some scary errors about the missing HEAD and ORIG_HEAD, but will not touch the index at all, and still delete the rebase-apply directory. Should we preserve this behavior (without the errors)? Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/WIP 1/8] wrapper: implement xopen()
On Thu, May 28, 2015 at 5:53 AM, Jeff King wrote: > On Wed, May 27, 2015 at 09:03:47PM +0200, Torsten Bögershausen wrote: > >> The original open can take 2 or 3 parameters, how about this: >> int xopen(const char *path, int oflag, ... ) >> { >> va_list params; >> int mode; >> int fd; >> >> va_start(params, oflag); >> mode = va_arg(params, int); >> va_end(params); >> >> fd = open(path, oflag, mode); > > Don't you need a conditional on pulling the mode arg off the stack > (i.e., if O_CREAT is in the flags)? Yeah, we do, as va_arg()'s behavior is undefined if we do not have the next argument. The POSIX spec[1] only mentions O_CREAT as requiring the extra argument, so I guess we'll only need to check for that. [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/WIP 6/8] am: extract patch, message and authorship with git-mailinfo
On Thu, May 28, 2015 at 4:44 AM, Junio C Hamano wrote: > Paul Tan writes: > >> @@ -17,6 +34,10 @@ struct am_state { >> struct strbuf dir;/* state directory path */ >> int cur; /* current patch number */ >> int last; /* last patch number */ >> + struct strbuf msg;/* commit message */ >> + struct strbuf author_name;/* commit author's name */ >> + struct strbuf author_email; /* commit author's email */ >> + struct strbuf author_date;/* commit author's date */ >> int prec; /* number of digits in patch filename */ >> }; > > I always get suspicious when structure fields are overly commented, > wondering if it is a sign of naming fields poorly. All of the above > fields look quite self-explanatory and I am not sure if it is worth > having these comments, spending efforts to type SP many times to > align them and all. Okay, I'll take Jeff's suggestion to organize them into blocks. >> +static int read_author_script(struct am_state *state) >> +{ >> + char *value; >> + struct strbuf sb = STRBUF_INIT; >> + const char *filename = am_path(state, "author-script"); >> + FILE *fp = fopen(filename, "r"); >> + if (!fp) { >> + if (errno == ENOENT) >> + return 0; >> + die(_("could not open '%s' for reading"), filename); > > Hmph, do we want to report with die_errno()? Yes, we do. >> + } >> + >> + if (strbuf_getline(&sb, fp, '\n')) >> + return -1; >> + if (!skip_prefix(sb.buf, "GIT_AUTHOR_NAME=", (const char**) &value)) > > This cast is unfortunate; can't "value" be of "const char *" type? We can't, because sq_dequote() modifies the string directly. I would think casting from non-const to const is safer than the other way around. Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/WIP 6/8] am: extract patch, message and authorship with git-mailinfo
On Thu, May 28, 2015 at 6:13 AM, Junio C Hamano wrote: > Paul Tan writes: > >> +static const char *msgnum(const struct am_state *state) >> +{ >> + static struct strbuf fmt = STRBUF_INIT; >> + static struct strbuf sb = STRBUF_INIT; >> + >> + strbuf_reset(&fmt); >> + strbuf_addf(&fmt, "%%0%dd", state->prec); >> + >> + strbuf_reset(&sb); >> + strbuf_addf(&sb, fmt.buf, state->cur); > > Hmph, wouldn't ("%*d", state->prec, state->cur) work or am I missing > something? Yeah, I think it would. I justs didn't know about the existence of '*'. Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 05/19] pull: implement fetch + merge
Implement the fetch + merge functionality of git-pull, by first running git-fetch with the repo and refspecs provided on the command line, then running git-merge on FETCH_HEAD to merge the fetched refs into the current branch. Signed-off-by: Paul Tan --- builtin/pull.c | 61 +- 1 file changed, 60 insertions(+), 1 deletion(-) diff --git a/builtin/pull.c b/builtin/pull.c index f8b79a2..0ca23a3 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -9,8 +9,10 @@ #include "builtin.h" #include "parse-options.h" #include "exec_cmd.h" +#include "run-command.h" static const char * const pull_usage[] = { + N_("git pull [options] [ [...]]"), NULL }; @@ -18,8 +20,60 @@ static struct option pull_options[] = { OPT_END() }; +/** + * Parses argv into [ [...]], returning their values in `repo` + * as a string and `refspecs` as a null-terminated array of strings. If `repo` + * is not provided in argv, it is set to NULL. + */ +static void parse_repo_refspecs(int argc, const char **argv, const char **repo, + const char ***refspecs) +{ + if (argc > 0) { + *repo = *argv++; + argc--; + } else + *repo = NULL; + *refspecs = argv; +} + +/** + * Runs git-fetch, returning its exit status. `repo` and `refspecs` are the + * repository and refspecs to fetch, or NULL if they are not provided. + */ +static int run_fetch(const char *repo, const char **refspecs) +{ + struct argv_array args = ARGV_ARRAY_INIT; + int ret; + + argv_array_pushl(&args, "fetch", "--update-head-ok", NULL); + if (repo) + argv_array_push(&args, repo); + while (*refspecs) + argv_array_push(&args, *refspecs++); + ret = run_command_v_opt(args.argv, RUN_GIT_CMD); + argv_array_clear(&args); + return ret; +} + +/** + * Runs git-merge, returning its exit status. + */ +static int run_merge(void) +{ + int ret; + struct argv_array args = ARGV_ARRAY_INIT; + + argv_array_pushl(&args, "merge", NULL); + argv_array_push(&args, "FETCH_HEAD"); + ret = run_command_v_opt(args.argv, RUN_GIT_CMD); + argv_array_clear(&args); + return ret; +} + int cmd_pull(int argc, const char **argv, const char *prefix) { + const char *repo, **refspecs; + if (!getenv("_GIT_USE_BUILTIN_PULL")) { const char *path = mkpath("%s/git-pull", git_exec_path()); @@ -29,5 +83,10 @@ int cmd_pull(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0); - return 0; + parse_repo_refspecs(argc, argv, &repo, &refspecs); + + if (run_fetch(repo, refspecs)) + return 1; + + return run_merge(); } -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 04/19] pull: implement skeletal builtin pull
For the purpose of rewriting git-pull.sh into a C builtin, implement a skeletal builtin/pull.c that redirects to $GIT_EXEC_PATH/git-pull.sh if the environment variable _GIT_USE_BUILTIN_PULL is not defined. This allows us to fall back on the functional git-pull.sh when running the test suite for tests that depend on a working git-pull implementation. This redirection should be removed when all the features of git-pull.sh have been re-implemented in builtin/pull.c. Helped-by: Junio C Hamano Signed-off-by: Paul Tan --- Makefile | 1 + builtin.h | 1 + builtin/pull.c | 33 + git.c | 1 + 4 files changed, 36 insertions(+) create mode 100644 builtin/pull.c diff --git a/Makefile b/Makefile index 54ec511..2057a9d 100644 --- a/Makefile +++ b/Makefile @@ -877,6 +877,7 @@ BUILTIN_OBJS += builtin/pack-refs.o BUILTIN_OBJS += builtin/patch-id.o BUILTIN_OBJS += builtin/prune-packed.o BUILTIN_OBJS += builtin/prune.o +BUILTIN_OBJS += builtin/pull.o BUILTIN_OBJS += builtin/push.o BUILTIN_OBJS += builtin/read-tree.o BUILTIN_OBJS += builtin/receive-pack.o diff --git a/builtin.h b/builtin.h index b87df70..ea3c834 100644 --- a/builtin.h +++ b/builtin.h @@ -98,6 +98,7 @@ extern int cmd_pack_redundant(int argc, const char **argv, const char *prefix); extern int cmd_patch_id(int argc, const char **argv, const char *prefix); extern int cmd_prune(int argc, const char **argv, const char *prefix); extern int cmd_prune_packed(int argc, const char **argv, const char *prefix); +extern int cmd_pull(int argc, const char **argv, const char *prefix); extern int cmd_push(int argc, const char **argv, const char *prefix); extern int cmd_read_tree(int argc, const char **argv, const char *prefix); extern int cmd_receive_pack(int argc, const char **argv, const char *prefix); diff --git a/builtin/pull.c b/builtin/pull.c new file mode 100644 index 000..f8b79a2 --- /dev/null +++ b/builtin/pull.c @@ -0,0 +1,33 @@ +/* + * Builtin "git pull" + * + * Based on git-pull.sh by Junio C Hamano + * + * Fetch one or more remote refs and merge it/them into the current HEAD. + */ +#include "cache.h" +#include "builtin.h" +#include "parse-options.h" +#include "exec_cmd.h" + +static const char * const pull_usage[] = { + NULL +}; + +static struct option pull_options[] = { + OPT_END() +}; + +int cmd_pull(int argc, const char **argv, const char *prefix) +{ + if (!getenv("_GIT_USE_BUILTIN_PULL")) { + const char *path = mkpath("%s/git-pull", git_exec_path()); + + if (sane_execvp(path, (char**) argv) < 0) + die_errno("could not exec %s", path); + } + + argc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0); + + return 0; +} diff --git a/git.c b/git.c index 44374b1..e7a7713 100644 --- a/git.c +++ b/git.c @@ -445,6 +445,7 @@ static struct cmd_struct commands[] = { { "pickaxe", cmd_blame, RUN_SETUP }, { "prune", cmd_prune, RUN_SETUP }, { "prune-packed", cmd_prune_packed, RUN_SETUP }, + { "pull", cmd_pull, RUN_SETUP | NEED_WORK_TREE }, { "push", cmd_push, RUN_SETUP }, { "read-tree", cmd_read_tree, RUN_SETUP }, { "receive-pack", cmd_receive_pack }, -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 16/19] pull: configure --rebase via branch..rebase or pull.rebase
Since cd67e4d (Teach 'git pull' about --rebase, 2007-11-28), fetch+rebase could be set by default by defining the config variable branch..rebase. This setting can be overriden on the command line by --rebase and --no-rebase. Since 6b37dff (pull: introduce a pull.rebase option to enable --rebase, 2011-11-06), git-pull --rebase can also be configured via the pull.rebase configuration option. Re-implement support for these two configuration settings by introducing config_get_rebase() which is called before parse_options() to set the default value of opt_rebase. Helped-by: Stefan Beller Signed-off-by: Paul Tan --- Notes: v2 * Previously, config_get_rebase() attempted to do too many things. It: 1. Checked if there was a configuration for branch.$curr_branch.rebase, and if not, then pull.rebase 2. Parsed the configuration value and died if the value is invalid. These 2 functions have been split into config_get_rebase_default() and config_get_rebase() respectively. builtin/pull.c | 48 1 file changed, 48 insertions(+) diff --git a/builtin/pull.c b/builtin/pull.c index 7645937..9759720 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -258,6 +258,52 @@ static void config_get_ff(struct strbuf *sb) } /** + * Sets `value` to the REBASE_* value for the configuration setting `key`. + * Returns 0 is `key` exists, -1 if it does not. Dies if the value of `key` is + * invalid. + */ +static int config_get_rebase(const char *key, enum rebase_type *value) +{ + const char *str_value; + + if (git_config_get_value(key, &str_value)) + return -1; + *value = parse_config_rebase(str_value); + if (*value == REBASE_INVALID) + die(_("Invalid value for %s: %s"), key, str_value); + return 0; +} + +/** + * Returns the default configured value for --rebase. It first looks for the + * value of "branch.$curr_branch.rebase", where $curr_branch is the current + * branch, and if HEAD is detached or the configuration key does not exist, + * looks for the value of "pull.rebase". If both configuration keys do not + * exist, returns REBASE_FALSE. + */ +static enum rebase_type config_get_rebase_default(void) +{ + struct branch *curr_branch = branch_get("HEAD"); + enum rebase_type rebase_type; + + if (curr_branch) { + int ret; + struct strbuf sb = STRBUF_INIT; + + strbuf_addf(&sb, "branch.%s.rebase", curr_branch->name); + ret = config_get_rebase(sb.buf, &rebase_type); + strbuf_release(&sb); + if (!ret) + return rebase_type; + } + + if (!config_get_rebase("pull.rebase", &rebase_type)) + return rebase_type; + + return REBASE_FALSE; +} + +/** * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge * into merge_heads. */ @@ -691,6 +737,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix) if (!getenv("GIT_REFLOG_ACTION")) set_reflog_message(argc, argv); + opt_rebase = config_get_rebase_default(); + argc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0); parse_repo_refspecs(argc, argv, &repo, &refspecs); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 13/19] pull: implement pulling into an unborn branch
b4dc085 (pull: merge into unborn by fast-forwarding from empty tree, 2013-06-20) established git-pull's current behavior of pulling into an unborn branch by fast-forwarding the work tree from an empty tree to the merge head, then setting HEAD to the merge head. Re-implement this behavior by introducing pull_into_void() which will be called instead of run_merge() if HEAD is invalid. Helped-by: Stephen Robin Signed-off-by: Paul Tan --- builtin/pull.c | 29 - 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/builtin/pull.c b/builtin/pull.c index 8db0db2..f0d4710 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -13,6 +13,7 @@ #include "sha1-array.h" #include "remote.h" #include "dir.h" +#include "refs.h" static const char * const pull_usage[] = { N_("git pull [options] [ [...]]"), @@ -367,6 +368,27 @@ static int run_fetch(const char *repo, const char **refspecs) } /** + * "Pulls into void" by branching off merge_head. + */ +static int pull_into_void(unsigned char merge_head[GIT_SHA1_RAWSZ], + unsigned char curr_head[GIT_SHA1_RAWSZ]) +{ + /* +* Two-way merge: we claim the index is based on an empty tree, +* and try to fast-forward to HEAD. This ensures we will not lose +* index/worktree changes that the user already made on the unborn +* branch. +*/ + if (checkout_fast_forward(EMPTY_TREE_SHA1_BIN, merge_head, 0)) + return 1; + + if (update_ref("initial pull", "HEAD", merge_head, curr_head, 0, UPDATE_REFS_DIE_ON_ERR)) + return 1; + + return 0; +} + +/** * Runs git-merge, returning its exit status. */ static int run_merge(void) @@ -475,5 +497,10 @@ int cmd_pull(int argc, const char **argv, const char *prefix) if (!merge_heads.nr) die_no_merge_candidates(repo, refspecs); - return run_merge(); + if (is_null_sha1(orig_head)) { + if (merge_heads.nr > 1) + die(_("Cannot merge multiple branches into empty head.")); + return pull_into_void(*merge_heads.sha1, curr_head); + } else + return run_merge(); } -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 06/19] pull: pass verbosity, --progress flags to fetch and merge
7f87aff (Teach/Fix pull/fetch -q/-v options, 2008-11-15) taught git-pull to accept the verbosity -v and -q options and pass them to git-fetch and git-merge. Re-implement support for the verbosity flags by adding it to the options list and introducing argv_push_verbosity() to push the flags into the argv array used to execute git-fetch and git-merge. 9839018 (fetch and pull: learn --progress, 2010-02-24) and bebd2fd (pull: propagate --progress to merge, 2011-02-20) taught git-pull to accept the --progress option and pass it to git-fetch and git-merge. Re-implement support for this flag by introducing the option callback handler parse_opt_passthru(). This callback is used to pass the "--progress" or "--no-progress" command-line switch to git-fetch and git-merge. Signed-off-by: Paul Tan --- Notes: v2 * Use parse_opt_pass_strbuf(). builtin/pull.c | 36 1 file changed, 36 insertions(+) diff --git a/builtin/pull.c b/builtin/pull.c index 0ca23a3..c9c2cc0 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -16,11 +16,35 @@ static const char * const pull_usage[] = { NULL }; +/* Shared options */ +static int opt_verbosity; +static struct strbuf opt_progress = STRBUF_INIT; + static struct option pull_options[] = { + /* Shared options */ + OPT__VERBOSITY(&opt_verbosity), + { OPTION_CALLBACK, 0, "progress", &opt_progress, NULL, + N_("force progress reporting"), + PARSE_OPT_NOARG, parse_opt_pass_strbuf}, + OPT_END() }; /** + * Pushes "-q" or "-v" switches into arr to match the opt_verbosity level. + */ +static void argv_push_verbosity(struct argv_array *arr) +{ + int verbosity; + + for (verbosity = opt_verbosity; verbosity > 0; verbosity--) + argv_array_push(arr, "-v"); + + for (verbosity = opt_verbosity; verbosity < 0; verbosity++) + argv_array_push(arr, "-q"); +} + +/** * Parses argv into [ [...]], returning their values in `repo` * as a string and `refspecs` as a null-terminated array of strings. If `repo` * is not provided in argv, it is set to NULL. @@ -46,6 +70,12 @@ static int run_fetch(const char *repo, const char **refspecs) int ret; argv_array_pushl(&args, "fetch", "--update-head-ok", NULL); + + /* Shared options */ + argv_push_verbosity(&args); + if (opt_progress.len) + argv_array_push(&args, opt_progress.buf); + if (repo) argv_array_push(&args, repo); while (*refspecs) @@ -64,6 +94,12 @@ static int run_merge(void) struct argv_array args = ARGV_ARRAY_INIT; argv_array_pushl(&args, "merge", NULL); + + /* Shared options */ + argv_push_verbosity(&args); + if (opt_progress.len) + argv_array_push(&args, opt_progress.buf); + argv_array_push(&args, "FETCH_HEAD"); ret = run_command_v_opt(args.argv, RUN_GIT_CMD); argv_array_clear(&args); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 14/19] pull: set reflog message
f947413 (Use GIT_REFLOG_ACTION environment variable instead., 2006-12-28) established git-pull's method for setting the reflog message, which is to set the environment variable GIT_REFLOG_ACTION to the evaluation of "pull${1+ $*}" if it has not already been set. Re-implement this behavior. Helped-by: Junio C Hamano Signed-off-by: Paul Tan --- Notes: v2 * Don't use strbuf_rtrim(). builtin/pull.c | 22 ++ 1 file changed, 22 insertions(+) diff --git a/builtin/pull.c b/builtin/pull.c index f0d4710..c44cc90 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -169,6 +169,25 @@ static void argv_push_force(struct argv_array *arr) } /** + * Sets the GIT_REFLOG_ACTION environment variable to the concatenation of argv + */ +static void set_reflog_message(int argc, const char **argv) +{ + int i; + struct strbuf msg = STRBUF_INIT; + + for (i = 0; i < argc; i++) { + if (i) + strbuf_addch(&msg, ' '); + strbuf_addstr(&msg, argv[i]); + } + + setenv("GIT_REFLOG_ACTION", msg.buf, 0); + + strbuf_release(&msg); +} + +/* * If pull.ff is "true", sets sb to "--ff". If pull.ff is "false", sets sb to * "--no-ff". If pull.ff is "only", sets sb to "--ff-only". If pull.ff is * set to an invalid value, die with an error. @@ -442,6 +461,9 @@ int cmd_pull(int argc, const char **argv, const char *prefix) die_errno("could not exec %s", path); } + if (!getenv("GIT_REFLOG_ACTION")) + set_reflog_message(argc, argv); + argc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0); parse_repo_refspecs(argc, argv, &repo, &refspecs); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 15/19] pull: teach git pull about --rebase
Since cd67e4d (Teach 'git pull' about --rebase, 2007-11-28), if the --rebase option is set, git-rebase is run instead of git-merge. Re-implement this by introducing run_rebase(), which is called instead of run_merge() if opt_rebase is a true value. Since c85c792 (pull --rebase: be cleverer with rebased upstream branches, 2008-01-26), git-pull handles the case where the upstream branch was rebased since it was last fetched. The fork point (old remote ref) of the branch from the upstream branch is calculated before fetch, and then rebased from onto the new remote head (merge_head) after fetch. Re-implement this by introducing get_merge_branch_2() and get_merge_branch_1() to find the upstream branch for the specified/current branch, and get_rebase_fork_point() which will find the fork point between the upstream branch and current branch. However, the above change created a problem where git-rebase cannot detect commits that are already upstream, and thus may result in unnecessary conflicts. cf65426 (pull --rebase: Avoid spurious conflicts and reapplying unnecessary patches, 2010-08-12) fixes this by ignoring the above old remote ref if it is contained within the merge base of the merge head and the current branch. This is re-implemented in run_rebase() where fork_point is not used if it is the merge base returned by get_octopus_merge_base(). Helped-by: Stefan Beller Helped-by: Johannes Schindelin Signed-off-by: Paul Tan --- Notes: v2 * enum rebase_type gained a new value REBASE_INVALID = -1 to represent invalid values passed to --rebase. We can't immediately die() on invalid values because we need parse_options() to show the git-pull usage as well. * parse_config_rebase() now returns an enum rebase_type to make it clear what kind of value it returns. * parse_config_rebase() now does not depend on git_config_maybe_bool() returning 1 for true and 0 for false. If it returns 0, it is REBASE_FALSE. If it returns any true value >=0, it is interpreted as REBASE_TRUE. * get_merge_branch_1() has been renamed to get_upstream_branch() to be consistent with the similar get_upstream_branch() in sha1_name.c. * get_merge_branch_2() has been renamed to get_tracking_branch() to indicate what it is doing: deriving the remote tracking branch from the refspec. * Various small variable re-names and docstring tweaks. Hopefully everything reads better now. builtin/pull.c | 239 - 1 file changed, 237 insertions(+), 2 deletions(-) diff --git a/builtin/pull.c b/builtin/pull.c index c44cc90..7645937 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -15,6 +15,45 @@ #include "dir.h" #include "refs.h" +enum rebase_type { + REBASE_INVALID = -1, + REBASE_FALSE = 0, + REBASE_TRUE, + REBASE_PRESERVE +}; + +/** + * Parses the value of --rebase, branch.*.rebase or pull.rebase. If value is a + * false value, returns REBASE_FALSE. If value is a true value, returns + * REBASE_TRUE. If value is "preserve", returns REBASE_PRESERVE. Otherwise, + * returns -1 to signify an invalid value. + */ +static enum rebase_type parse_config_rebase(const char *value) +{ + int v = git_config_maybe_bool("pull.rebase", value); + if (!v) + return REBASE_FALSE; + else if (v >= 0) + return REBASE_TRUE; + else if (!strcmp(value, "preserve")) + return REBASE_PRESERVE; + return REBASE_INVALID; +} + +/** + * Callback for --rebase, which parses arg with parse_config_rebase(). + */ +static int parse_opt_rebase(const struct option *opt, const char *arg, int unset) +{ + enum rebase_type *value = opt->value; + + if (arg) + *value = parse_config_rebase(arg); + else + *value = unset ? REBASE_FALSE : REBASE_TRUE; + return *value == REBASE_INVALID ? -1 : 0; +} + static const char * const pull_usage[] = { N_("git pull [options] [ [...]]"), NULL @@ -24,7 +63,8 @@ static const char * const pull_usage[] = { static int opt_verbosity; static struct strbuf opt_progress = STRBUF_INIT; -/* Options passed to git-merge */ +/* Options passed to git-merge or git-rebase */ +static enum rebase_type opt_rebase; static struct strbuf opt_diffstat = STRBUF_INIT; static struct strbuf opt_log = STRBUF_INIT; static struct strbuf opt_squash = STRBUF_INIT; @@ -58,8 +98,12 @@ static struct option pull_options[] = { N_("force progress reporting"), PARSE_OPT_NOARG, parse_opt_pass_strbuf}, - /* Options passed to git-merge */ + /* Options passed to git-merge or git-rebase */ OPT_GROUP(N_("Options related to merging")), + { OPTION_CALLBACK, 'r', "rebase", &opt_rebase, +
[PATCH v2 18/19] pull --rebase: error on no merge candidate cases
Tweak the error messages printed by die_no_merge_candidates() to take into account that we may be "rebasing against" rather than "merging with". Signed-off-by: Paul Tan --- Notes: v2 * Decided to use fprintf_ln() for the sake of code consistency, and for the added trailing newline. builtin/pull.c | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/builtin/pull.c b/builtin/pull.c index f5d437a..4e1ab5b 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -439,7 +439,10 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs const char *remote = curr_branch ? curr_branch->remote_name : NULL; if (*refspecs) { - fprintf_ln(stderr, _("There are no candidates for merging among the refs that you just fetched.")); + if (opt_rebase) + fprintf_ln(stderr, _("There is no candidate for rebasing against among the refs that you just fetched.")); + else + fprintf_ln(stderr, _("There are no candidates for merging among the refs that you just fetched.")); fprintf_ln(stderr, _("Generally this means that you provided a wildcard refspec which had no\n" "matches on the remote end.")); } else if (repo && curr_branch && (!remote || strcmp(repo, remote))) { @@ -449,7 +452,10 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs repo); } else if (!curr_branch) { fprintf_ln(stderr, _("You are not currently on a branch.")); - fprintf_ln(stderr, _("Please specify which branch you want to merge with.")); + if (opt_rebase) + fprintf_ln(stderr, _("Please specify which branch you want to rebase against.")); + else + fprintf_ln(stderr, _("Please specify which branch you want to merge with.")); fprintf_ln(stderr, _("See git-pull(1) for details.")); fprintf(stderr, "\n"); fprintf_ln(stderr, "git pull "); @@ -461,7 +467,10 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs remote_name = ""; fprintf_ln(stderr, _("There is no tracking information for the current branch.")); - fprintf_ln(stderr, _("Please specify which branch you want to merge with.")); + if (opt_rebase) + fprintf_ln(stderr, _("Please specify which branch you want to rebase against.")); + else + fprintf_ln(stderr, _("Please specify which branch you want to merge with.")); fprintf_ln(stderr, _("See git-pull(1) for details.")); fprintf(stderr, "\n"); fprintf_ln(stderr, "git pull "); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 17/19] pull --rebase: exit early when the working directory is dirty
Re-implement the behavior introduced by f9189cf (pull --rebase: exit early when the working directory is dirty, 2008-05-21). Signed-off-by: Paul Tan --- builtin/pull.c | 77 +- 1 file changed, 76 insertions(+), 1 deletion(-) diff --git a/builtin/pull.c b/builtin/pull.c index 9759720..f5d437a 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -14,6 +14,8 @@ #include "remote.h" #include "dir.h" #include "refs.h" +#include "revision.h" +#include "lockfile.h" enum rebase_type { REBASE_INVALID = -1, @@ -304,6 +306,73 @@ static enum rebase_type config_get_rebase_default(void) } /** + * Returns 1 if there are unstaged changes, 0 otherwise. + */ +static int has_unstaged_changes(const char *prefix) +{ + struct rev_info rev_info; + int result; + + init_revisions(&rev_info, prefix); + DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES); + DIFF_OPT_SET(&rev_info.diffopt, QUICK); + diff_setup_done(&rev_info.diffopt); + result = run_diff_files(&rev_info, 0); + return diff_result_code(&rev_info.diffopt, result); +} + +/** + * Returns 1 if there are uncommitted changes, 0 otherwise. + */ +static int has_uncommitted_changes(const char *prefix) +{ + struct rev_info rev_info; + int result; + + if (is_cache_unborn()) + return 0; + + init_revisions(&rev_info, prefix); + DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES); + DIFF_OPT_SET(&rev_info.diffopt, QUICK); + add_head_to_pending(&rev_info); + diff_setup_done(&rev_info.diffopt); + result = run_diff_index(&rev_info, 1); + return diff_result_code(&rev_info.diffopt, result); +} + +/** + * If the work tree has unstaged or uncommitted changes, dies with the + * appropriate message. + */ +static void die_on_unclean_work_tree(const char *prefix) +{ + struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file)); + int do_die = 0; + + hold_locked_index(lock_file, 0); + refresh_cache(REFRESH_QUIET); + update_index_if_able(&the_index, lock_file); + rollback_lock_file(lock_file); + + if (has_unstaged_changes(prefix)) { + error(_("Cannot pull with rebase: You have unstaged changes.")); + do_die = 1; + } + + if (has_uncommitted_changes(prefix)) { + if (do_die) + error(_("Additionally, your index contains uncommitted changes.")); + else + error(_("Cannot pull with rebase: Your index contains uncommitted changes.")); + do_die = 1; + } + + if (do_die) + exit(1); +} + +/** * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge * into merge_heads. */ @@ -757,9 +826,15 @@ int cmd_pull(int argc, const char **argv, const char *prefix) if (get_sha1("HEAD", orig_head)) hashclr(orig_head); - if (opt_rebase) + if (opt_rebase) { + if (is_null_sha1(orig_head) && !is_cache_unborn()) + die(_("Updating an unborn branch with changes added to the index.")); + + die_on_unclean_work_tree(prefix); + if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs)) hashclr(rebase_fork_point); + } if (run_fetch(repo, refspecs)) return 1; -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 08/19] pull: pass git-fetch's options to git-fetch
Since eb2a8d9 (pull: handle git-fetch's options as well, 2015-06-02), git-pull knows about and handles git-fetch's options, passing them to git-fetch. Re-implement this behavior. Since 29609e6 (pull: do nothing on --dry-run, 2010-05-25) git-pull supported the --dry-run option, exiting after git-fetch if --dry-run is set. Re-implement this behavior. Signed-off-by: Paul Tan --- Notes: v2 * Use parse_opt_parse_strbuf() builtin/pull.c | 95 ++ 1 file changed, 95 insertions(+) diff --git a/builtin/pull.c b/builtin/pull.c index 5f08634..0b66b43 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -32,6 +32,21 @@ static struct argv_array opt_strategies = ARGV_ARRAY_INIT; static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT; static struct strbuf opt_gpg_sign = STRBUF_INIT; +/* Options passed to git-fetch */ +static struct strbuf opt_all = STRBUF_INIT; +static struct strbuf opt_append = STRBUF_INIT; +static struct strbuf opt_upload_pack = STRBUF_INIT; +static int opt_force; +static struct strbuf opt_tags = STRBUF_INIT; +static struct strbuf opt_prune = STRBUF_INIT; +static struct strbuf opt_recurse_submodules = STRBUF_INIT; +static int opt_dry_run; +static struct strbuf opt_keep = STRBUF_INIT; +static struct strbuf opt_depth = STRBUF_INIT; +static struct strbuf opt_unshallow = STRBUF_INIT; +static struct strbuf opt_update_shallow = STRBUF_INIT; +static struct strbuf opt_refmap = STRBUF_INIT; + static struct option pull_options[] = { /* Shared options */ OPT__VERBOSITY(&opt_verbosity), @@ -82,6 +97,46 @@ static struct option pull_options[] = { N_("GPG sign commit"), PARSE_OPT_OPTARG, parse_opt_pass_strbuf }, + /* Options passed to git-fetch */ + OPT_GROUP(N_("Options related to fetching")), + { OPTION_CALLBACK, 0, "all", &opt_all, 0, + N_("fetch from all remotes"), + PARSE_OPT_NOARG, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 'a', "append", &opt_append, 0, + N_("append to .git/FETCH_HEAD instead of overwriting"), + PARSE_OPT_NOARG, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 0, "upload-pack", &opt_upload_pack, N_("path"), + N_("path to upload pack on remote end"), + 0, parse_opt_pass_strbuf }, + OPT__FORCE(&opt_force, N_("force overwrite of local branch")), + { OPTION_CALLBACK, 't', "tags", &opt_tags, 0, + N_("fetch all tags and associated objects"), + PARSE_OPT_NOARG, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 'p', "prune", &opt_prune, 0, + N_("prune remote-tracking branches no longer on remote"), + PARSE_OPT_NOARG, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 0, "recurse-submodules", &opt_recurse_submodules, + N_("on-demand"), + N_("control recursive fetching of submodules"), + PARSE_OPT_OPTARG, parse_opt_pass_strbuf }, + OPT_BOOL(0, "dry-run", &opt_dry_run, + N_("dry run")), + { OPTION_CALLBACK, 'k', "keep", &opt_keep, 0, + N_("keep downloaded pack"), + PARSE_OPT_NOARG, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 0, "depth", &opt_depth, N_("depth"), + N_("deepen history of shallow clone"), + 0, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 0, "unshallow", &opt_unshallow, 0, + N_("convert to a complete repository"), + PARSE_OPT_NONEG | PARSE_OPT_NOARG, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 0, "update-shallow", &opt_update_shallow, 0, + N_("accept refs that update .git/shallow"), + PARSE_OPT_NOARG, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 0, "refmap", &opt_refmap, N_("refmap"), + N_("specify fetch refmap"), + PARSE_OPT_NONEG, parse_opt_pass_strbuf }, + OPT_END() }; @@ -100,6 +155,16 @@ static void argv_push_verbosity(struct argv_array *arr) } /** + * Pushes "-f" switches into arr to match the opt_force level. + */ +static void argv_push_force(struct argv_array *arr) +{ + int force = opt_force; + while (force-- > 0) + argv_array_push(arr, "-f"); +} + +/** * Parses argv into [ [...]], returning their values in `repo` * as a string and `refspecs` as a null-terminated array of strings. If `repo` * is not provided in argv, it is set to NULL. @@ -131,6 +196,33 @@ static int run_fetch(const char *repo, const char **refspecs) if (opt_progress.len) argv_array_push(&args, opt_progress.
[PATCH v2 12/19] pull: fast-forward working tree if head is updated
Since b10ac50 (Fix pulling into the same branch., 2005-08-25), git-pull, upon detecting that git-fetch updated the current head, will fast-forward the working tree to the updated head commit. Re-implement this behavior. Signed-off-by: Paul Tan --- builtin/pull.c | 30 ++ 1 file changed, 30 insertions(+) diff --git a/builtin/pull.c b/builtin/pull.c index 703fc1d..8db0db2 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -411,6 +411,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix) { const char *repo, **refspecs; struct sha1_array merge_heads = SHA1_ARRAY_INIT; + unsigned char orig_head[GIT_SHA1_RAWSZ], curr_head[GIT_SHA1_RAWSZ]; if (!getenv("_GIT_USE_BUILTIN_PULL")) { const char *path = mkpath("%s/git-pull", git_exec_path()); @@ -434,12 +435,41 @@ int cmd_pull(int argc, const char **argv, const char *prefix) if (!opt_ff.len) config_get_ff(&opt_ff); + if (get_sha1("HEAD", orig_head)) + hashclr(orig_head); + if (run_fetch(repo, refspecs)) return 1; if (opt_dry_run) return 0; + if (get_sha1("HEAD", curr_head)) + hashclr(curr_head); + + if (!is_null_sha1(orig_head) && !is_null_sha1(curr_head) && + hashcmp(orig_head, curr_head)) { + /* +* The fetch involved updating the current branch. +* +* The working tree and the index file are still based on +* orig_head commit, but we are merging into curr_head. +* Update the working tree to match curr_head. +*/ + + warning(_("fetch updated the current branch head.\n" + "fast-forwarding your working tree from\n" + "commit %s."), sha1_to_hex(orig_head)); + + if (checkout_fast_forward(orig_head, curr_head, 0)) + die(_("Cannot fast-forward your working tree.\n" + "After making sure that you saved anything precious from\n" + "$ git diff %s\n" + "output, run\n" + "$ git reset --hard\n" + "to recover."), sha1_to_hex(orig_head)); + } + get_merge_heads(&merge_heads); if (!merge_heads.nr) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 11/19] pull: check if in unresolved merge state
Since d38a30d (Be more user-friendly when refusing to do something because of conflict., 2010-01-12), git-pull will error out with user-friendly advices if the user is in the middle of a merge or has unmerged files. Re-implement this behavior. While the "has unmerged files" case can be handled by die_resolve_conflict(), we introduce a new function die_conclude_merge() for printing a different error message for when there are no unmerged files but the merge has not been finished. Signed-off-by: Paul Tan --- advice.c | 8 advice.h | 1 + builtin/pull.c | 9 + 3 files changed, 18 insertions(+) diff --git a/advice.c b/advice.c index 575bec2..4965686 100644 --- a/advice.c +++ b/advice.c @@ -96,6 +96,14 @@ void NORETURN die_resolve_conflict(const char *me) die("Exiting because of an unresolved conflict."); } +void NORETURN die_conclude_merge(void) +{ + error(_("You have not concluded your merge (MERGE_HEAD exists).")); + if (advice_resolve_conflict) + advise(_("Please, commit your changes before you can merge.")); + die(_("Exiting because of unfinished merge.")); +} + void detach_advice(const char *new_name) { const char fmt[] = diff --git a/advice.h b/advice.h index 5ecc6c1..b341a55 100644 --- a/advice.h +++ b/advice.h @@ -24,6 +24,7 @@ __attribute__((format (printf, 1, 2))) void advise(const char *advice, ...); int error_resolve_conflict(const char *me); extern void NORETURN die_resolve_conflict(const char *me); +void NORETURN die_conclude_merge(void); void detach_advice(const char *new_name); #endif /* ADVICE_H */ diff --git a/builtin/pull.c b/builtin/pull.c index 1c1883d..703fc1d 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -12,6 +12,7 @@ #include "run-command.h" #include "sha1-array.h" #include "remote.h" +#include "dir.h" static const char * const pull_usage[] = { N_("git pull [options] [ [...]]"), @@ -422,6 +423,14 @@ int cmd_pull(int argc, const char **argv, const char *prefix) parse_repo_refspecs(argc, argv, &repo, &refspecs); + git_config(git_default_config, NULL); + + if (read_cache_unmerged()) + die_resolve_conflict("Pull"); + + if (file_exists(git_path("MERGE_HEAD"))) + die_conclude_merge(); + if (!opt_ff.len) config_get_ff(&opt_ff); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 03/19] argv-array: implement argv_array_pushv()
When we have a null-terminated array, it would be useful to convert it or append it to an argv_array for further manipulation. Implement argv_array_pushv() which will push a null-terminated array of strings on to an argv_array. Signed-off-by: Paul Tan --- Documentation/technical/api-argv-array.txt | 3 +++ argv-array.c | 6 ++ argv-array.h | 1 + 3 files changed, 10 insertions(+) diff --git a/Documentation/technical/api-argv-array.txt b/Documentation/technical/api-argv-array.txt index 1a79781..8076172 100644 --- a/Documentation/technical/api-argv-array.txt +++ b/Documentation/technical/api-argv-array.txt @@ -46,6 +46,9 @@ Functions Format a string and push it onto the end of the array. This is a convenience wrapper combining `strbuf_addf` and `argv_array_push`. +`argv_array_pushv`:: + Push a null-terminated array of strings onto the end of the array. + `argv_array_pop`:: Remove the final element from the array. If there are no elements in the array, do nothing. diff --git a/argv-array.c b/argv-array.c index 256741d..eaed477 100644 --- a/argv-array.c +++ b/argv-array.c @@ -49,6 +49,12 @@ void argv_array_pushl(struct argv_array *array, ...) va_end(ap); } +void argv_array_pushv(struct argv_array *array, const char **argv) +{ + for (; *argv; argv++) + argv_array_push(array, *argv); +} + void argv_array_pop(struct argv_array *array) { if (!array->argc) diff --git a/argv-array.h b/argv-array.h index c65e6e8..a2fa0aa 100644 --- a/argv-array.h +++ b/argv-array.h @@ -17,6 +17,7 @@ __attribute__((format (printf,2,3))) void argv_array_pushf(struct argv_array *, const char *fmt, ...); LAST_ARG_MUST_BE_NULL void argv_array_pushl(struct argv_array *, ...); +void argv_array_pushv(struct argv_array *, const char **); void argv_array_pop(struct argv_array *); void argv_array_clear(struct argv_array *); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 07/19] pull: pass git-merge's options to git-merge
Specify git-merge's options in the option list, and pass any specified options to git-merge. These options are: * -n, --stat, --summary: since d8abe14 (merge, pull: introduce '--(no-)stat' option, 2008-04-06) * --log: since efb779f (merge, pull: add '--(no-)log' command line option, 2008-04-06) * --squash: since 7d0c688 (git-merge --squash, 2006-06-23) * --commit: since 5072a32 (Teach git-pull about --[no-]ff, --no-squash and --commit, 2007-10-29) * --edit: since 8580830 ("git pull" doesn't know "--edit", 2012-02-11) * --ff, --ff-only: since 5072a32 (Teach git-pull about --[no-]ff, --no-squash and --commit, 2007-10-29) * --verify-signatures: since efed002 (merge/pull: verify GPG signatures of commits being merged, 2013-03-31) * -s, --strategy: since 60fb5b2 (Use git-merge in git-pull (second try)., 2005-09-25) * -X, --strategy-option: since ee2c795 (Teach git-pull to pass -X to git-merge, 2009-11-25) * -S, --gpg-sign: since ea230d8 (pull: add the --gpg-sign option., 2014-02-10) Signed-off-by: Paul Tan --- Notes: v2 * Use opt_parse_pass_strbuf(), opt_parse_pass_argv_array() and argv_array_pushv() builtin/pull.c | 75 ++ 1 file changed, 75 insertions(+) diff --git a/builtin/pull.c b/builtin/pull.c index c9c2cc0..5f08634 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -20,6 +20,18 @@ static const char * const pull_usage[] = { static int opt_verbosity; static struct strbuf opt_progress = STRBUF_INIT; +/* Options passed to git-merge */ +static struct strbuf opt_diffstat = STRBUF_INIT; +static struct strbuf opt_log = STRBUF_INIT; +static struct strbuf opt_squash = STRBUF_INIT; +static struct strbuf opt_commit = STRBUF_INIT; +static struct strbuf opt_edit = STRBUF_INIT; +static struct strbuf opt_ff = STRBUF_INIT; +static struct strbuf opt_verify_signatures = STRBUF_INIT; +static struct argv_array opt_strategies = ARGV_ARRAY_INIT; +static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT; +static struct strbuf opt_gpg_sign = STRBUF_INIT; + static struct option pull_options[] = { /* Shared options */ OPT__VERBOSITY(&opt_verbosity), @@ -27,6 +39,49 @@ static struct option pull_options[] = { N_("force progress reporting"), PARSE_OPT_NOARG, parse_opt_pass_strbuf}, + /* Options passed to git-merge */ + OPT_GROUP(N_("Options related to merging")), + { OPTION_CALLBACK, 'n', NULL, &opt_diffstat, NULL, + N_("do not show a diffstat at the end of the merge"), + PARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 0, "stat", &opt_diffstat, NULL, + N_("show a diffstat at the end of the merge"), + PARSE_OPT_NOARG, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 0, "summary", &opt_diffstat, NULL, + N_("(synonym to --stat)"), + PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 0, "log", &opt_log, N_("n"), + N_("add (at most ) entries from shortlog to merge commit message"), + PARSE_OPT_OPTARG, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 0, "squash", &opt_squash, NULL, + N_("create a single commit instead of doing a merge"), + PARSE_OPT_NOARG, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 0, "commit", &opt_commit, NULL, + N_("perform a commit if the merge succeeds (default)"), + PARSE_OPT_NOARG, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 0, "edit", &opt_edit, NULL, + N_("edit message before committing"), + PARSE_OPT_NOARG, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 0, "ff", &opt_ff, NULL, + N_("allow fast-forward"), + PARSE_OPT_NOARG, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 0, "ff-only", &opt_ff, NULL, + N_("abort if fast-forward is not possible"), + PARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 0, "verify-signatures", &opt_verify_signatures, NULL, + N_("verify that the named commit has a valid GPG signature"), + PARSE_OPT_NOARG, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 's', "strategy", &opt_strategies, N_("strategy"), + N_("merge strategy to use"), + 0, parse_opt_pass_argv_array }, + { OPTION_CALLBACK, 'X', "strategy-option", &opt_strategy_opts, + N_("option=value"), + N_("option for selected merge strategy"), + 0, parse_opt_pass_argv_array }, + { OPTION_CAL
[PATCH v2 09/19] pull: error on no merge candidates
Commit a8c9bef (pull: improve advice for unconfigured error case, 2009-10-05) fully established the current advices given by git-pull for the different cases where git-fetch will not have anything marked for merge: 1. We fetched from a specific remote, and a refspec was given, but it ended up not fetching anything. This is usually because the user provided a wildcard refspec which had no matches on the remote end. 2. We fetched from a non-default remote, but didn't specify a branch to merge. We can't use the configured one because it applies to the default remote, and thus the user must specify the branches to merge. 3. We fetched from the branch's or repo's default remote, but: a. We are not on a branch, so there will never be a configured branch to merge with. b. We are on a branch, but there is no configured branch to merge with. 4. We fetched from the branch's or repo's default remote, but the configured branch to merge didn't get fetched (either it doesn't exist, or wasn't part of the configured fetch refspec) Re-implement the above behavior by implementing get_merge_heads() to parse the heads in FETCH_HEAD for merging, and implementing die_no_merge_candidates(), which will be called when FETCH_HEAD has no heads for merging. Helped-by: Johannes Schindelin Signed-off-by: Paul Tan --- Notes: v2 * Switched to using fprintf_ln() which will append the trailing newline at the end for us. * The die_no_merge_candidates() code has been reorganized a bit to support the later patch which will tweak the messages to be aware of git-pull --rebase. builtin/pull.c | 113 + 1 file changed, 113 insertions(+) diff --git a/builtin/pull.c b/builtin/pull.c index 0b66b43..42a061d 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -10,6 +10,8 @@ #include "parse-options.h" #include "exec_cmd.h" #include "run-command.h" +#include "sha1-array.h" +#include "remote.h" static const char * const pull_usage[] = { N_("git pull [options] [ [...]]"), @@ -165,6 +167,111 @@ static void argv_push_force(struct argv_array *arr) } /** + * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge + * into merge_heads. + */ +static void get_merge_heads(struct sha1_array *merge_heads) +{ + const char *filename = git_path("FETCH_HEAD"); + FILE *fp; + struct strbuf sb = STRBUF_INIT; + unsigned char sha1[GIT_SHA1_RAWSZ]; + + if (!(fp = fopen(filename, "r"))) + die_errno(_("could not open '%s' for reading"), filename); + while(strbuf_getline(&sb, fp, '\n') != EOF) { + if (get_sha1_hex(sb.buf, sha1)) + continue; /* invalid line: does not start with SHA1 */ + if (starts_with(sb.buf + GIT_SHA1_HEXSZ, "\tnot-for-merge")) + continue; /* ref is not-for-merge */ + sha1_array_append(merge_heads, sha1); + } + fclose(fp); + strbuf_release(&sb); +} + +/** + * Used by die_no_merge_candidates() as a for_each_remote() callback to + * retrieve the name of the remote if the repository only has one remote. + */ +static int get_only_remote(struct remote *remote, void *cb_data) +{ + const char **remote_name = cb_data; + + if (*remote_name) + return -1; + + *remote_name = remote->name; + return 0; +} + +/** + * Dies with the appropriate reason for why there are no merge candidates: + * + * 1. We fetched from a specific remote, and a refspec was given, but it ended + *up not fetching anything. This is usually because the user provided a + *wildcard refspec which had no matches on the remote end. + * + * 2. We fetched from a non-default remote, but didn't specify a branch to + *merge. We can't use the configured one because it applies to the default + *remote, thus the user must specify the branches to merge. + * + * 3. We fetched from the branch's or repo's default remote, but: + * + *a. We are not on a branch, so there will never be a configured branch to + * merge with. + * + *b. We are on a branch, but there is no configured branch to merge with. + * + * 4. We fetched from the branch's or repo's default remote, but the configured + *branch to merge didn't get fetched. (Either it doesn't exist, or wasn't + *part of the configured fetch refspec.) + */ +static void NORETURN die_no_merge_candidates(const char *repo, const char **refspecs) +{ + struct branch *curr_branch = branch_get("HEAD"); + const char *remote = curr_branch ? curr_branch->remote_name : NULL; + + if (*refspe
[PATCH v2 19/19] pull: remove redirection to git-pull.sh
At the beginning of the rewrite of git-pull.sh to C, we introduced a redirection to git-pull.sh if the environment variable _GIT_USE_BUILTIN_PULL was not defined in order to not break test scripts that relied on a functional git-pull. Now that all of git-pull's functionality has been re-implemented in builtin/pull.c, remove this redirection, and retire the old git-pull.sh into contrib/examples/. Signed-off-by: Paul Tan --- Makefile| 1 - builtin/pull.c | 7 --- git-pull.sh => contrib/examples/git-pull.sh | 0 3 files changed, 8 deletions(-) rename git-pull.sh => contrib/examples/git-pull.sh (100%) diff --git a/Makefile b/Makefile index 2057a9d..67cef1c 100644 --- a/Makefile +++ b/Makefile @@ -474,7 +474,6 @@ SCRIPT_SH += git-merge-octopus.sh SCRIPT_SH += git-merge-one-file.sh SCRIPT_SH += git-merge-resolve.sh SCRIPT_SH += git-mergetool.sh -SCRIPT_SH += git-pull.sh SCRIPT_SH += git-quiltimport.sh SCRIPT_SH += git-rebase.sh SCRIPT_SH += git-remote-testgit.sh diff --git a/builtin/pull.c b/builtin/pull.c index 4e1ab5b..dad49cf 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -805,13 +805,6 @@ int cmd_pull(int argc, const char **argv, const char *prefix) unsigned char orig_head[GIT_SHA1_RAWSZ], curr_head[GIT_SHA1_RAWSZ]; unsigned char rebase_fork_point[GIT_SHA1_RAWSZ]; - if (!getenv("_GIT_USE_BUILTIN_PULL")) { - const char *path = mkpath("%s/git-pull", git_exec_path()); - - if (sane_execvp(path, (char**) argv) < 0) - die_errno("could not exec %s", path); - } - if (!getenv("GIT_REFLOG_ACTION")) set_reflog_message(argc, argv); diff --git a/git-pull.sh b/contrib/examples/git-pull.sh similarity index 100% rename from git-pull.sh rename to contrib/examples/git-pull.sh -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 10/19] pull: support pull.ff config
Since b814da8 (pull: add pull.ff configuration, 2014-01-15), git-pull.sh would lookup the configuration value of "pull.ff", and set the flag "--ff" if its value is "true", "--no-ff" if its value is "false" and "--ff-only" if its value is "only". Re-implement this behavior. Signed-off-by: Paul Tan --- Notes: v2 * Yup, I did mean strcmp(value, "only") builtin/pull.c | 29 + 1 file changed, 29 insertions(+) diff --git a/builtin/pull.c b/builtin/pull.c index 42a061d..1c1883d 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -167,6 +167,32 @@ static void argv_push_force(struct argv_array *arr) } /** + * If pull.ff is "true", sets sb to "--ff". If pull.ff is "false", sets sb to + * "--no-ff". If pull.ff is "only", sets sb to "--ff-only". If pull.ff is + * set to an invalid value, die with an error. + */ +static void config_get_ff(struct strbuf *sb) +{ + const char *value; + + if (git_config_get_value("pull.ff", &value)) + return; + switch (git_config_maybe_bool("pull.ff", value)) { + case 0: + strbuf_addstr(sb, "--no-ff"); + return; + case 1: + strbuf_addstr(sb, "--ff"); + return; + } + if (!strcmp(value, "only")) { + strbuf_addstr(sb, "--ff-only"); + return; + } + die(_("Invalid value for pull.ff: %s"), value); +} + +/** * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge * into merge_heads. */ @@ -396,6 +422,9 @@ int cmd_pull(int argc, const char **argv, const char *prefix) parse_repo_refspecs(argc, argv, &repo, &refspecs); + if (!opt_ff.len) + config_get_ff(&opt_ff); + if (run_fetch(repo, refspecs)) return 1; -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 01/19] parse-options-cb: implement parse_opt_pass_strbuf()
Certain git commands, such as git-pull, are simply wrappers around other git commands like git-fetch, git-merge and git-rebase. As such, these wrapper commands will typically need to "pass through" command-line options of the commands they wrap. Implement the parse_opt_pass_strbuf() parse-options callback, which will reconstruct the command-line option into an strbuf, such that it can be passed to another git command. Helped-by: Johannes Schindelin Signed-off-by: Paul Tan --- Notes: v2 * Previously implemented as a static function in builtin/pull.c. It now uses an strbuf instead of returning newly-allocated strings, to make memory management easier. * We don't use defval anymore. Instead, we use long_name and short_name. parse-options-cb.c | 29 + parse-options.h| 1 + 2 files changed, 30 insertions(+) diff --git a/parse-options-cb.c b/parse-options-cb.c index be8c413..5b1dbcf 100644 --- a/parse-options-cb.c +++ b/parse-options-cb.c @@ -134,3 +134,32 @@ int parse_opt_noop_cb(const struct option *opt, const char *arg, int unset) { return 0; } + +/** + * For an option opt, recreates the command-line option in opt->value which + * must be an strbuf. This is useful when we need to pass the command-line + * option to another command. + */ +int parse_opt_pass_strbuf(const struct option *opt, const char *arg, int unset) +{ + struct strbuf *sb = opt->value; + + strbuf_reset(sb); + + if (opt->long_name) { + strbuf_addstr(sb, unset ? "--no-" : "--"); + strbuf_addstr(sb, opt->long_name); + if (arg) { + strbuf_addch(sb, '='); + strbuf_addstr(sb, arg); + } + } else if (opt->short_name && !unset) { + strbuf_addch(sb, '-'); + strbuf_addch(sb, opt->short_name); + if (arg) + strbuf_addstr(sb, arg); + } else + return -1; + + return 0; +} diff --git a/parse-options.h b/parse-options.h index c71e9da..1d21398 100644 --- a/parse-options.h +++ b/parse-options.h @@ -224,6 +224,7 @@ extern int parse_opt_with_commit(const struct option *, const char *, int); extern int parse_opt_tertiary(const struct option *, const char *, int); extern int parse_opt_string_list(const struct option *, const char *, int); extern int parse_opt_noop_cb(const struct option *, const char *, int); +extern int parse_opt_pass_strbuf(const struct option *, const char *, int); #define OPT__VERBOSE(var, h) OPT_COUNTUP('v', "verbose", (var), (h)) #define OPT__QUIET(var, h)OPT_COUNTUP('q', "quiet", (var), (h)) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 00/19] Make git-pull a builtin
This is a re-roll of [v1]. Thanks Johannes, Stefan and Junio for the reviews last round. Previous versions: [v1] http://thread.gmane.org/gmane.comp.version-control.git/269258 git-pull is a commonly executed command to check for new changes in the upstream repository and, if there are, fetch and integrate them into the current branch. Currently it is implemented by the shell script git-pull.sh. However, compared to C, shell scripts have certain deficiencies -- they need to spawn a lot of processes, introduce a lot of dependencies and cannot take advantage of git's internal caches. This series rewrites git-pull.sh into a C builtin, thus improving its performance and portability. It is part of my GSoC project to rewrite git-pull and git-am into builtins[1]. [1] https://gist.github.com/pyokagan/1b7b0d1f4dab6ba3cef1 Paul Tan (19): parse-options-cb: implement parse_opt_pass_strbuf() parse-options-cb: implement parse_opt_pass_argv_array() argv-array: implement argv_array_pushv() pull: implement skeletal builtin pull pull: implement fetch + merge pull: pass verbosity, --progress flags to fetch and merge pull: pass git-merge's options to git-merge pull: pass git-fetch's options to git-fetch pull: error on no merge candidates pull: support pull.ff config pull: check if in unresolved merge state pull: fast-forward working tree if head is updated pull: implement pulling into an unborn branch pull: set reflog message pull: teach git pull about --rebase pull: configure --rebase via branch..rebase or pull.rebase pull --rebase: exit early when the working directory is dirty pull --rebase: error on no merge candidate cases pull: remove redirection to git-pull.sh Documentation/technical/api-argv-array.txt | 3 + Makefile| 2 +- advice.c| 8 + advice.h| 1 + argv-array.c| 6 + argv-array.h| 1 + builtin.h | 1 + builtin/pull.c | 888 git-pull.sh => contrib/examples/git-pull.sh | 0 git.c | 1 + parse-options-cb.c | 61 ++ parse-options.h | 2 + 12 files changed, 973 insertions(+), 1 deletion(-) create mode 100644 builtin/pull.c rename git-pull.sh => contrib/examples/git-pull.sh (100%) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 02/19] parse-options-cb: implement parse_opt_pass_argv_array()
Certain git commands, such as git-pull, are simply wrappers around other git commands like git-fetch, git-merge and git-rebase. As such, these wrapper commands will typically need to "pass through" command-line options of the commands they wrap. Implement the parse_opt_pass_argv_array() parse-options callback, which will reconstruct all the provided command-line options into an argv_array, such that it can be passed to another git command. This is useful for passing command-line options that can be specified multiple times. Signed-off-by: Paul Tan --- Notes: v2 * This function is a requirement for the rewrite of git-am to handle passing git-apply's options to git-apply. Since it would be implemented anyway I thought it would be good if git-pull could take advantage of it as well to handle --strategy and --strategy-option. parse-options-cb.c | 32 parse-options.h| 1 + 2 files changed, 33 insertions(+) diff --git a/parse-options-cb.c b/parse-options-cb.c index 5b1dbcf..7330506 100644 --- a/parse-options-cb.c +++ b/parse-options-cb.c @@ -4,6 +4,7 @@ #include "commit.h" #include "color.h" #include "string-list.h" +#include "argv-array.h" /*- some often used options -*/ @@ -163,3 +164,34 @@ int parse_opt_pass_strbuf(const struct option *opt, const char *arg, int unset) return 0; } + +/** + * For an option opt, recreate the command-line option, appending it to + * opt->value which must be a argv_array. This is useful when we need to pass + * the command-line option, which can be specified multiple times, to another + * command. + */ +int parse_opt_pass_argv_array(const struct option *opt, const char *arg, int unset) +{ + struct strbuf sb = STRBUF_INIT; + struct argv_array *opt_value = opt->value; + + if (opt->long_name) { + strbuf_addstr(&sb, unset ? "--no-" : "--"); + strbuf_addstr(&sb, opt->long_name); + if (arg) { + strbuf_addch(&sb, '='); + strbuf_addstr(&sb, arg); + } + } else if (opt->short_name && !unset) { + strbuf_addch(&sb, '-'); + strbuf_addch(&sb, opt->short_name); + if (arg) + strbuf_addstr(&sb, arg); + } else + return -1; + + argv_array_push(opt_value, sb.buf); + strbuf_release(&sb); + return 0; +} diff --git a/parse-options.h b/parse-options.h index 1d21398..b663f87 100644 --- a/parse-options.h +++ b/parse-options.h @@ -225,6 +225,7 @@ extern int parse_opt_tertiary(const struct option *, const char *, int); extern int parse_opt_string_list(const struct option *, const char *, int); extern int parse_opt_noop_cb(const struct option *, const char *, int); extern int parse_opt_pass_strbuf(const struct option *, const char *, int); +extern int parse_opt_pass_argv_array(const struct option *, const char *, int); #define OPT__VERBOSE(var, h) OPT_COUNTUP('v', "verbose", (var), (h)) #define OPT__QUIET(var, h)OPT_COUNTUP('q', "quiet", (var), (h)) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pull: allow dirty tree when rebase.autostash enabled
Hi, Some comments which may not necessarily be correct. On Wed, Jun 3, 2015 at 5:55 AM, Kevin Daudt wrote: > rebase learned to stash changes when it encounters a dirty work tree, but > git pull --rebase does not. > > Only verify if the working tree is dirty when rebase.autostash is not > enabled. > --- Missing sign-off. > git-pull.sh | 5 - > t/t5520-pull.sh | 17 + > 2 files changed, 21 insertions(+), 1 deletion(-) > > diff --git a/git-pull.sh b/git-pull.sh > index 0917d0d..6b9e8a3 100755 > --- a/git-pull.sh > +++ b/git-pull.sh > @@ -239,7 +239,10 @@ test true = "$rebase" && { > die "$(gettext "updating an unborn branch with > changes added to the index")" > fi > else > - require_clean_work_tree "pull with rebase" "Please commit or > stash them." > + if [ $(git config --bool --get rebase.autostash || echo > false) = "false" ] "false" doesn't need to be quoted. > + then > + require_clean_work_tree "pull with rebase" "Please > commit or stash them." > + fi > fi > oldremoteref= && > test -n "$curr_branch" && > diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh > index 7efd45b..d849a19 100755 > --- a/t/t5520-pull.sh > +++ b/t/t5520-pull.sh > @@ -297,6 +297,23 @@ test_expect_success 'pull --rebase dies early with dirty > working directory' ' > > ' > > +test_expect_success 'pull --rebase succeeds with dirty working directory and > rebase.autostash set' ' > + I know the surrounding old tests use a newline, but I think that all new tests should use the modern style of not having a newline, since t5520 already consists of a mix of old and modern styles anyway. > + test_when_finished "git rm -f file4" && There is trailing whitespace here. Furthermore, git rm -f will fail if "file4" does not exist in the index. Perhaps it should be moved below the "git add" below. > + git checkout to-rebase && > + git update-ref refs/remotes/me/copy copy^ && > + COPY=$(git rev-parse --verify me/copy) && $COPY is not used anywhere in the test. > + git rebase --onto $COPY copy && > + test_config branch.to-rebase.remote me && > + test_config branch.to-rebase.merge refs/heads/copy && > + test_config branch.to-rebase.rebase true && > + test_config rebase.autostash true && > + echo dirty >> file4 && file4 does not exist, so we don't need to append to it. I know the above few tests do not adhere to it, but CodingGuidelines says that redirection operators do not have a space after > + git add file4 && > + git pull I think we should check for file contents to ensure that git-pull/git-stash/git-rebase is doing its job properly. > + Same as above, no need the newline. > +' > + With all that said, I wonder if this test, and the test above ("pull --rebase dies early with dirty working directory") could be vastly simplified, since we are not testing if we can handle a rebased upstream. E.g., my simplified version for the above test would be something like: git checkout -f to-rebase && git rebase --onto copy^ copy && test_config rebase.autostash true && echo dirty >file4 && git add file4 && test_when_finished "git rm -f file4" && git pull --rebase . me/copy && test "$(cat file4)" = dirty && test "$(cat file2)" = file It's still confusing though, because we cannot take advantage of the 'before-rebase' tag introduced in the above tests. I would much prefer if this test and the ("pull --rebase dies with dirty working directory") test could be moved to the --rebase tests at lines 214+. Also, this section in the t5520 test suite always gives me a headache trying to decipher what it is trying to do >< Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v2 2/2] git-am: add am.threeWay config variable
Hi, On Tue, Jun 2, 2015 at 9:37 PM, Matthieu Moy wrote: > Remi Lespinet writes: > >> +if test "$(git config --bool --get am.threeWay)" = true >> +then >> +threeway=t >> +fi > > I think you missed Paul's remark on this: > > http://article.gmane.org/gmane.comp.version-control.git/270150 > > Not terribly important since am will be rewritten soon, though. As the person who had to do four preparatory patch series' to fix bugs for the rewrite of git-pull, I respectfully disagree ;-) Regards, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/WIP 4/8] am: split out mbox/maildir patches with git-mailsplit
On Fri, May 29, 2015 at 7:05 AM, Eric Sunshine wrote: > On Wed, May 27, 2015 at 9:33 AM, Paul Tan wrote: >> @@ -128,13 +190,32 @@ static void am_next(struct am_state *state) >> */ >> +/** >> + * parse_options() callback that validates and sets opt->value to the >> + * PATCH_FORMAT_* enum value corresponding to `arg`. >> + */ >> +static int parse_opt_patchformat(const struct option *opt, const char *arg, >> int unset) >> +{ >> + int *opt_value = opt->value; >> + >> + if (!strcmp(arg, "mbox")) >> + *opt_value = PATCH_FORMAT_MBOX; >> + else >> + return -1; >> + return 0; >> +} >> + >> struct am_state state; >> +int opt_patch_format; > > Should these two variables be static? Whoops. Yes, they should. Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] pull: handle git-fetch's options as well
While parsing the command-line arguments, git-pull stops parsing at the first unrecognized option, assuming that any subsequent options are for git-fetch, and can thus be kept in the shell's positional parameters list, so that it can be passed to git-fetch via the expansion of "$@". However, certain functions in git-pull assume that the positional parameters do not contain any options: * error_on_no_merge_candidates() uses the number of positional parameters to determine which error message to print out, and will thus print the wrong message if git-fetch's options are passed in as well. * the call to get_remote_merge_branch() assumes that the positional parameters only contains the optional repo and refspecs, and will thus silently fail if git-fetch's options are passed in as well. * --dry-run is a valid git-fetch option, but if provided after any git-fetch options, it is not recognized by git-pull and thus git-pull will continue to run the merge or rebase. Fix these bugs by teaching git-pull to parse git-fetch's options as well. Add tests to prevent regressions. This removes the limitation where git-fetch's options have to come after git-merge's and git-rebase's options on the command line. Update the documentation to reflect this. Signed-off-by: Paul Tan --- Notes: Improve git-pull's option parsing Previous versions: [v1] http://thread.gmane.org/gmane.comp.version-control.git/269249 This patch series is based on pt/pull-tests. While parsing the command-line arguments, git-pull stops parsing at the first unrecognized option, assuming that any subsequent options are for git-fetch, and can thus be kept in the shell's positional parameters list, so that it can be passed to git-fetch via the expansion of "$ ". However, certain functions in git-pull assume that the positional parameters do not contain any options. Fix this by making git-pull handle git-fetch's options as well at the option parsing stage. With this change in place, we can move on to migrate git-pull to use git-rev-parse --parseopt such that its option parsing is consistent with the other git commands. I believe this is the last required behavior change for my rewrite of git-pull.sh to C. v2 * Initialize variables to prevent them from being set in the command line. * Update the documentation as well. Documentation/git-pull.txt | 3 --- git-pull.sh| 37 ++--- t/t5520-pull.sh| 20 t/t5521-pull-options.sh| 14 ++ 4 files changed, 68 insertions(+), 6 deletions(-) diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt index 712ab4b..93c72a2 100644 --- a/Documentation/git-pull.txt +++ b/Documentation/git-pull.txt @@ -74,9 +74,6 @@ pulling or stash them away with linkgit:git-stash[1]. OPTIONS --- -Options meant for 'git pull' itself and the underlying 'git merge' -must be given before the options meant for 'git fetch'. - -q:: --quiet:: This is passed to both underlying git-fetch to squelch reporting of diff --git a/git-pull.sh b/git-pull.sh index 0917d0d..623ba7a 100755 --- a/git-pull.sh +++ b/git-pull.sh @@ -44,7 +44,8 @@ bool_or_string_config () { strategy_args= diffstat= no_commit= squash= no_ff= ff_only= log_arg= verbosity= progress= recurse_submodules= verify_signatures= -merge_args= edit= rebase_args= +merge_args= edit= rebase_args= all= append= upload_pack= force= tags= prune= +keep= depth= unshallow= update_shallow= refmap= curr_branch=$(git symbolic-ref -q HEAD) curr_branch_short="${curr_branch#refs/heads/}" rebase=$(bool_or_string_config branch.$curr_branch_short.rebase) @@ -166,11 +167,39 @@ do --d|--dr|--dry|--dry-|--dry-r|--dry-ru|--dry-run) dry_run=--dry-run ;; + --all|--no-all) + all=$1 ;; + -a|--append|--no-append) + append=$1 ;; + --upload-pack=*|--no-upload-pack) + upload_pack=$1 ;; + -f|--force|--no-force) + force="$force $1" ;; + -t|--tags|--no-tags) + tags=$1 ;; + -p|--prune|--no-prune) + prune=$1 ;; + -k|--keep|--no-keep) + keep=$1 ;; + --depth=*|--no-depth) + depth=$1 ;; + --unshallow|--no-unshallow) + unshallow=$1 ;; + --update-shallow|--no-update-shallow) + update_shallow=$1 ;; + --refmap=*|--no-refmap) + refmap=$1 ;; -h|--help-all) usage ;; + --) + shift + break + ;; + -*) + usage + ;; *) - # Pass thru anything tha
[PATCH v2 0/2] Improve git-pull's option parsing
This is a re-roll of [v1]. Thanks Johannes for the reviews last round. Previous versions: [v1] http://thread.gmane.org/gmane.comp.version-control.git/269249 This patch series is based on pt/pull-tests. While parsing the command-line arguments, git-pull stops parsing at the first unrecognized option, assuming that any subsequent options are for git-fetch, and can thus be kept in the shell's positional parameters list, so that it can be passed to git-fetch via the expansion of "$@". However, certain functions in git-pull assume that the positional parameters do not contain any options. Fix this by making git-pull handle git-fetch's options as well at the option parsing stage. With this change in place, we can move on to migrate git-pull to use git-rev-parse --parseopt such that its option parsing is consistent with the other git commands. I believe this is the last required behavior change for my rewrite of git-pull.sh to C. Paul Tan (2): pull: handle git-fetch's options as well pull: use git-rev-parse --parseopt for option parsing Documentation/git-pull.txt | 3 -- git-pull.sh| 128 +++-- t/t5520-pull.sh| 20 +++ t/t5521-pull-options.sh| 14 + 4 files changed, 122 insertions(+), 43 deletions(-) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] pull: use git-rev-parse --parseopt for option parsing
To enable unambiguous parsing of abbreviated options, bundled short options, separate form options and to provide consistent usage help, use git-rev-parse --parseopt for option parsing. With this, simplify the option parsing code. Signed-off-by: Paul Tan --- Notes: v2 * Don't use OPTIONS_KEEPDASHDASH git-pull.sh | 97 - 1 file changed, 57 insertions(+), 40 deletions(-) diff --git a/git-pull.sh b/git-pull.sh index 623ba7a..a814bf6 100755 --- a/git-pull.sh +++ b/git-pull.sh @@ -4,13 +4,53 @@ # # Fetch one or more remote refs and merge it/them into the current HEAD. -USAGE='[-n | --no-stat] [--[no-]commit] [--[no-]squash] [--[no-]ff|--ff-only] [--[no-]rebase|--rebase=preserve] [-s strategy]... [] ...' -LONG_USAGE='Fetch one or more remote refs and integrate it/them with the current HEAD.' SUBDIRECTORY_OK=Yes -OPTIONS_SPEC= +OPTIONS_KEEPDASHDASH= +OPTIONS_STUCKLONG=Yes +OPTIONS_SPEC="\ +git pull [options] [ [...]] + +Fetch one or more remote refs and integrate it/them with the current HEAD. +-- +v,verbose be more verbose +q,quietbe more quiet +progress force progress reporting + + Options related to merging +r,rebase?false|true|preserve incorporate changes by rebasing rather than merging +n! do not show a diffstat at the end of the merge +stat show a diffstat at the end of the merge +summary(synonym to --stat) +log?n add (at most ) entries from shortlog to merge commit message +squash create a single commit instead of doing a merge +commit perform a commit if the merge succeeds (default) +e,edit edit message before committing +ff allow fast-forward +ff-only! abort if fast-forward is not possible +verify-signatures verify that the named commit has a valid GPG signature +s,strategy=strategymerge strategy to use +X,strategy-option=option option for selected merge strategy +S,gpg-sign?key-id GPG sign commit + + Options related to fetching +allfetch from all remotes +a,append append to .git/FETCH_HEAD instead of overwriting +upload-pack=path path to upload pack on remote end +f,forceforce overwrite of local branch +t,tags fetch all tags and associated objects +p,pruneprune remote-tracking branches no longer on remote +recurse-submodules?on-demand control recursive fetching of submodules +dry-rundry run +k,keep keep downloaded pack +depth=depthdeepen history of shallow clone +unshallow convert to a complete repository +update-shallow accept refs that update .git/shallow +refmap=refmap specify fetch refmap +" +test $# -gt 0 && args="$*" . git-sh-setup . git-sh-i18n -set_reflog_action "pull${1+ $*}" +set_reflog_action "pull${args+ $args}" require_work_tree_exists cd_to_toplevel @@ -87,17 +127,17 @@ do diffstat=--stat ;; --log|--log=*|--no-log) log_arg="$1" ;; - --no-c|--no-co|--no-com|--no-comm|--no-commi|--no-commit) + --no-commit) no_commit=--no-commit ;; - --c|--co|--com|--comm|--commi|--commit) + --commit) no_commit=--commit ;; -e|--edit) edit=--edit ;; --no-edit) edit=--no-edit ;; - --sq|--squ|--squa|--squas|--squash) + --squash) squash=--squash ;; - --no-sq|--no-squ|--no-squa|--no-squas|--no-squash) + --no-squash) squash=--no-squash ;; --ff) no_ff=--ff ;; @@ -105,39 +145,19 @@ do no_ff=--no-ff ;; --ff-only) ff_only=--ff-only ;; - -s=*|--s=*|--st=*|--str=*|--stra=*|--strat=*|--strate=*|\ - --strateg=*|--strategy=*|\ - -s|--s|--st|--str|--stra|--strat|--strate|--strateg|--strategy) - case "$#,$1" in - *,*=*) - strategy=$(expr "z$1" : 'z-[^=]*=\(.*\)') ;; - 1,*) - usage ;; - *) - strategy="$2" - shift ;; - esac - strategy_args="${strategy_args}-s $strategy " + -s*|--strategy=*) + strategy_args="$strategy_args $1" ;; - -X*) - case "$#,$1" in - 1,-X) - usage ;; - *,-X) - xx="-X $(git rev-par
Re: [PATCH 11/14] pull: teach git pull about --rebase
On Sun, May 31, 2015 at 4:18 PM, Paul Tan wrote: > On Tue, May 19, 2015 at 9:04 PM, Johannes Schindelin > wrote: >> Also, I wonder if something like this would do the job: >> spec = parse_fetch_refspec(1, &refspec); >> if (spec->dst) >> return spec->dst; > > Hmm, I notice that get_remote_merge_branch() only looks at the src > part of the refspec. However, I guess it is true that if the dst part > is provided, the user may be wishing for that to be interpreted as the > "remote tracking branch", so we should be looking at it to calculate > the fork point. > >> if (!(remote = get_remote(remote_name))) >> return NULL; >> if (remote_find_tracking(remote, spec)) >> return spec->dst; > > ... and if the dst part of the refspec is not provided, we fall back > to see if there is any remote tracking branch in the repo for the src > part of the ref, which matches the intention of > get_remote_merge_branch() I think, while being better because > remote_find_tracking() takes into account the actual configured fetch > refspecs for the remote. > > However, we also need to consider if the user provided a wildcard > refspec, as it will not make sense in this case. From my reading, > remote_find_tracking(), which calls query_refspecs(), would just match > the src part literally, so I guess we should explicitly detect and > error out in this case. With all that said, after thinking about it I feel that this patch series should focus solely on rewriting git-pull.sh 1:1. While I do agree with the above suggested improvements, I think they should be implemented as separated patch(es) on top of this series since we would be technically changing git-pull's behavior, even if we are improving it. As such, the issue that I think should be focused on for this patch is: does get_merge_branch_1() and get_merge_branch_2() in this patch implement the same behavior as get_remote_merge_branch() in git-parse.remote.sh? If it does, then its purpose is fulfilled. So, I'll keep the overall logic of get_merge_branch_2() the same for the next re-roll. (Other than renaming the function and fixing code style issues). Once this series is okay, I'll look into doing a separate patch on top that changes the function to use remote_find_tracking() so that we fix the assumption that the default fetch mapping is used. The other possibility is that we fix this in git-parse-remote.sh, but I'm personally getting a bit tired from having to re-implement the same thing in shell script and C. Furthermore, the only script using get_remote_merge_branch() is git-pull.sh. > [...] > Since this is just a 1:1 rewrite I just tried to keep as close to the > original as possible. However, thinking about it, since we *are* just > using the first refspec for fork point calculation, I do agree that we > should probably give an warning() here as well if the user provided > more than one refspec, like "Cannot calculate rebase fork point as you > provided more than one refspec. git-pull will not be able to handle a > rebased upstream". I do not think it is fatal enough that we should > error() or die(), as e.g. the first refspec may be a wildcard refspec > that matches nothing, and the second refspec that matches one merge > head for rebasing. This is pretty contrived though, but still > technically legitimate. I dunno. >[...] >> We should probably `return error(_"No tracking branch found for %s@, refspec >> ? refspec : "HEAD");` so that the user has a chance to understand that there >> has been a problem and how to solve it. > > Just like the above, I don't think this is serious enough to be > considered an error() though. Sure, this means that we cannot properly > handle the case where the upstream has been rebased, but this is not > always the case. We could probably have a warning() here, but I think > the message should be improved to tell the user what exactly she is > losing out on. e.g. "No tracking branch found for %s. git-pull will > not be able to handle a rebased upstream." Likewise, I won't introduce the error()s or warning()s for now. Other than that, all other code style related issues have been/will be fixed. Thanks for the reviews. Regards, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/14] pull: teach git pull about --rebase
Hi Johannes, On Tue, May 19, 2015 at 9:04 PM, Johannes Schindelin wrote: > On 2015-05-18 17:06, Paul Tan wrote: >> +/** >> + * Given a refspec, returns the merge branch. Returns NULL if the refspec >> src >> + * does not refer to a branch. >> + * >> + * FIXME: It should return the tracking branch. Currently only works with >> the >> + * default mapping. >> + */ >> +static char *get_merge_branch_2(const char *repo, const char *refspec) >> +{ >> + struct refspec *spec; >> + const char *remote; >> + char *merge_branch; >> + >> + spec = parse_fetch_refspec(1, &refspec); >> + remote = spec->src; >> + if (!*remote || !strcmp(remote, "HEAD")) >> + remote = "HEAD"; >> + else if (skip_prefix(remote, "heads/", &remote)) >> + ; >> + else if (skip_prefix(remote, "refs/heads/", &remote)) >> + ; >> + else if (starts_with(remote, "refs/") || >> + starts_with(remote, "tags/") || >> + starts_with(remote, "remotes/")) >> + remote = ""; >> + >> + if (*remote) { >> + if (!strcmp(repo, ".")) >> + merge_branch = mkpathdup("refs/heads/%s", remote); >> + else >> + merge_branch = mkpathdup("refs/remotes/%s/%s", repo, >> remote); >> + } else >> + merge_branch = NULL; >> + >> + free_refspec(1, spec); >> + return merge_branch; >> +} > > I have to admit that it took me a substantial amount of time to deduce from > the code what `get_merge_branch_2()` really does (judging from the > description, I thought that it would be essentially the same as > `get_merge_branch_1()` without the hard-coded "HEAD"). If the comment above > the function would have said something like: > > /** > * Given a refspec, returns the name of the local tracking ref. > */ > > I would have had an easier time. Also, I wonder if something like this would > do the job: Yeah whoops, this came from a confusion over the difference over "merge branch" and "remote tracking branch". A merge branch would be a remote tracking branch, but a remote tracking branch is not necessarily a merge branch. > spec = parse_fetch_refspec(1, &refspec); > if (spec->dst) > return spec->dst; Hmm, I notice that get_remote_merge_branch() only looks at the src part of the refspec. However, I guess it is true that if the dst part is provided, the user may be wishing for that to be interpreted as the "remote tracking branch", so we should be looking at it to calculate the fork point. > if (!(remote = get_remote(remote_name))) > return NULL; > if (remote_find_tracking(remote, spec)) > return spec->dst; ... and if the dst part of the refspec is not provided, we fall back to see if there is any remote tracking branch in the repo for the src part of the ref, which matches the intention of get_remote_merge_branch() I think, while being better because remote_find_tracking() takes into account the actual configured fetch refspecs for the remote. However, we also need to consider if the user provided a wildcard refspec, as it will not make sense in this case. From my reading, remote_find_tracking(), which calls query_refspecs(), would just match the src part literally, so I guess we should explicitly detect and error out in this case. > return NULL; > > (I guess we'd have to `xstrdup()` the return values because the return value > of `get_merge_branch_1()` needs to be `free()`d, but maybe we can avoid so > much `malloc()/free()`ing.) Again, the function name should probably be > changed to something clearer, maybe to `get_tracking_branch()`. Yeah, it should be changed to get_tracking_branch(), since it is different from get_merge_branch(). > One thing that is not clear at all to me is whether > > git pull --rebase origin master next > > would error out as expected, or simply rebase to `origin/master`. In git-pull.sh, for the rebase fork point calculation it just used the first refspec. However, when running git-rebase it checked to see if there was only one merge base, which is replicated here: @@ -573,6 +792,10 @@ int cmd_pull(int argc, const char **argv, const char *prefix) if (merge_heads.nr > 1) die(_("Cannot merge multiple branches into empty head.")); return pull_into_void(*merge_heads.s
Re: [PATCH 00/14] Make git-pull a builtin
On Sat, May 30, 2015 at 3:29 PM, Paul Tan wrote: > Hi, > Okay, I'm trying this out in the next re-roll. I do agree that this > patch series should not touch anything in t/ at all. > > One problem(?) is that putting builtins/pull.o in the BUILTIN_OBJS and > leaving git-pull.sh in SCRIPT_SH in the Makefile will generate 2 > targets to ./git-pull (they will clobber each other). For GNU Make, > the last defined target will win, so in this case it just happens that > git-pull.sh will win because the build targets for the shell scripts > are defined after the build targets for the builtins, so this works in > our favor I guess. > > Regards, > Paul Just to add on, I just discovered that test-lib.sh unsets all GIT_* environment variables "for repeatability", so the name "GIT_USE_BUILTIN_PULL" cannot be used. I'm tempted to just add a underscore just before the name ("_GIT_USE_BUILTIN_PULL") to work around this, since it's just a temporary thing. Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/14] Make git-pull a builtin
Hi, On Tue, May 19, 2015 at 3:21 AM, Junio C Hamano wrote: > Paul Tan writes: > >> This series rewrites git-pull.sh into a C builtin, thus improving its >> performance and portability. It is part of my GSoC project to rewrite >> git-pull >> and git-am into builtins[2]. > > Earlier you were worried about 'git pull' being used in many tests > for the purpose of testing other parts of the system and not testing > 'pull' itself. For a program as complex as 'git pull', you may want > to take the 'competing implementations' approach. > > (1) write an empty cmd_pull() that relaunches "git pull" scripted > porcelain from the $GIT_EXEC_PATH with given parameters, and > wire all the necessary bits to git.c. > > (2) enhance cmd_pull() a bit by bit, but keep something like this > > if (getenv(GIT_USE_BUILTIN_PULL)) { > /* original run_command("git-pull.sh") code */ > exit $? > } > > ... your "C" version ... > > (3) add "GIT_USE_BUILTIN_PULL=Yes; export GIT_USE_BUILTIN_PULL" at > the beginning of "t55??" test scripts (but not others that rely > on working pull and that are not interested in catching bugs in > pull). > > (4) once cmd_pull() becomes fully operational, drop (3) and also the > conditional one you added in (2), and retire the environment > variable. Retire the git-pull.sh script to contrib/examples/ > boneyard. > > That way, you will always have a reference you can use throughout > the development. > > Just a suggestion, not a requirement. Okay, I'm trying this out in the next re-roll. I do agree that this patch series should not touch anything in t/ at all. One problem(?) is that putting builtins/pull.o in the BUILTIN_OBJS and leaving git-pull.sh in SCRIPT_SH in the Makefile will generate 2 targets to ./git-pull (they will clobber each other). For GNU Make, the last defined target will win, so in this case it just happens that git-pull.sh will win because the build targets for the shell scripts are defined after the build targets for the builtins, so this works in our favor I guess. Regards, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 7/8] t5521: test --dry-run does not make any changes
Test that when --dry-run is provided to git-pull, it does not make any changes, namely: * --dry-run gets passed to git-fetch, so no FETCH_HEAD will be created and no refs will be fetched. * The index and work tree will not be modified. Signed-off-by: Paul Tan --- t/t5521-pull-options.sh | 13 + 1 file changed, 13 insertions(+) diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh index 453aba5..56e7377 100755 --- a/t/t5521-pull-options.sh +++ b/t/t5521-pull-options.sh @@ -117,4 +117,17 @@ test_expect_success 'git pull --all' ' ) ' +test_expect_success 'git pull --dry-run' ' + test_when_finished "rm -rf clonedry" && + git init clonedry && + ( + cd clonedry && + git pull --dry-run ../parent && + test_path_is_missing .git/FETCH_HEAD && + test_path_is_missing .git/refs/heads/master && + test_path_is_missing .git/index && + test_path_is_missing file + ) +' + test_done -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html