[PATCH v4 12/44] builtin-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 f21565b..5087094 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -16,6 +16,8 @@ #include "commit.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. @@ -872,6 +874,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`. */ @@ -888,7 +998,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 }; int cmd_am(int argc, const char **argv, const char *prefix) @@ -899,7 +1010,7 @@ int cmd_am(int argc, const char **argv, const char *prefix) const char * const usage[] = { N_("git am [options] [(|)...]"), - N_("git am [options] --continue"), + N_("git am [options] (--continue | --skip)"), NULL }; @@ -913,6 +1024,9 @@ int cmd_am(int argc, c
[PATCH v4 13/44] builtin-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 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 | 102 +-- 1 file changed, 99 insertions(+), 3 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 5087094..1db95f2 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -507,6 +507,8 @@ static int split_mail(struct am_state *state, enum patch_format patch_format, static void am_setup(struct am_state *state, enum patch_format patch_format, const char **paths) { + unsigned char curr_head[GIT_SHA1_RAWSZ]; + if (!patch_format) patch_format = detect_patch_format(paths); @@ -523,6 +525,14 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, die(_("Failed to split patches.")); } + 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); + } + /* * NOTE: Since the "next" and "last" files determine if an am_state * session is in progress, they should be written last. @@ -539,6 +549,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]; + if (state->author_name) free(state->author_name); state->author_name = NULL; @@ -559,6 +571,11 @@ static void am_next(struct am_state *state) unlink(am_path(state, "author-script")); 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", ""); + state->cur++; write_file(am_path(state, "next"), 1, "%d", state->cur); } @@ -791,10 +808,14 @@ static void am_run(struct am_state *state) const char *argv_gc_auto[] = {"gc", "--auto", NULL}; 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); @@ -982,6 +1003,74 @@ static void am_skip(struct am_state *state) } /** + * Returns true if it is safe to reset HEAD to the ORIG_HEAD, false otherwise. + * + * It is not safe to reset HEAD when: + * 1. git-am previously failed because the index was dirty. + * 2. HEAD has moved since git-am previously failed. + */ +static int safe_to_abort(const struct am_state *state) +{ + struct strbuf sb = STRBUF_INIT; + unsigned char abort_safety[GIT_SHA1_RAWSZ], head[GIT_SHA1_RAWSZ]; + + if (file_exists(am_path(state, "dirtyindex"))) + return 0; + + if (read_state_file(&sb, state, "abort-safety", 1) > 0) { + if (get_sha1_hex(sb.buf, abort_safety)) + die(_("could not parse %s"), am_path(state, "abort_safety")); + } else + hashclr(abort_safety); + + if (get_sha1("HEAD", head)) + hashclr(head); + + if (!hashcmp(head, abort_safety)) + return 1; + + error(_("You seem to have moved HEAD since the last 'am' failure.\n" + "Not rewinding to ORIG_HEAD")); + + return 0; +} + +/** + * Aborts the current am session if it is safe to do so. + */ +static void am_abort(struct am_state *state) +{ + unsigned char curr_head[GIT_SHA1_RAWSZ], orig_head[GIT_SHA1_RAWSZ]; + int has_curr_head, has_orig_head; + const char *curr_branch; + + if (!safe_to_abort(state)) { + am_destroy(state); + return; +
[PATCH v4 16/44] builtin-am: exit with user friendly message on 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 | 32 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 2643b04..7832ecf 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -82,6 +82,9 @@ struct am_state { int prec; int quiet; + + /* override error message when patch failure occurs */ + const char *resolvemsg; }; /** @@ -674,6 +677,25 @@ static int index_has_changes(struct strbuf *sb) } /** + * 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 { + const char *cmdline = "git am"; + + printf_ln(_("When you have resolved this problem, run \"%s --continue\"."), cmdline); + printf_ln(_("If you prefer to skip this patch, run \"%s --skip\" instead."), cmdline); + printf_ln(_("To restore the original branch and stop patching, run \"%s --abort\"."), cmdline); + } + + exit(128); +} + +/** * Parses `mail` using git-mailinfo, extracting its patch and authorship info. * state->msg will be set to the patch message. state->author_name, * state->author_email and state->author_date will be set to the patch author's @@ -733,7 +755,7 @@ static int parse_mail(struct am_state *state, const char *mail) if (is_empty_file(am_path(state, "patch"))) { printf_ln(_("Patch is empty. Was it split wrong?")); - exit(128); + die_user_resolve(state); } strbuf_addstr(&msg, "\n\n"); @@ -874,7 +896,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); @@ -908,13 +930,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); @@ -1138,6 +1160,8 @@ int cmd_am(int argc, const char **argv, const char *prefix) OPT_CALLBACK(0, "patch-format", &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", &resume, N_("continue applying patches after resolving a conflict"), RESUME_RESOLVED), -- 2.5.0.rc0.76.gb2c6e93 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 23/44] builtin-am: implement -u/--utf8
Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07), git-am.sh supported the -u,--utf8 option. If set, the -u option will be passed to git-mailinfo to re-code the commit log message and authorship in the charset specified by i18n.commitencoding. If unset, the -n option will be passed to git-mailinfo, which disables the re-encoding. Since d84029b (--utf8 is now default for 'git-am', 2007-01-08), --utf8 is specified by default in git-am.sh. Re-implement the above in builtin/am.c. Signed-off-by: Paul Tan --- builtin/am.c | 12 1 file changed, 12 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 49a6840..bc44225 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -92,6 +92,8 @@ struct am_state { int append_signoff; + int utf8; + /* override error message when patch failure occurs */ const char *resolvemsg; @@ -118,6 +120,8 @@ static void am_state_init(struct am_state *state, const char *dir) quiet = getenv("GIT_QUIET"); if (quiet && *quiet) state->quiet = 1; + + state->utf8 = 1; } /** @@ -396,6 +400,9 @@ static void am_load(struct am_state *state) read_state_file(&sb, state, "sign", 1); state->append_signoff = !strcmp(sb.buf, "t"); + read_state_file(&sb, state, "utf8", 1); + state->utf8 = !strcmp(sb.buf, "t"); + state->rebasing = !!file_exists(am_path(state, "rebasing")); strbuf_release(&sb); @@ -585,6 +592,8 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, write_file(am_path(state, "sign"), 1, state->append_signoff ? "t" : "f"); + write_file(am_path(state, "utf8"), 1, state->utf8 ? "t" : "f"); + if (state->rebasing) write_file(am_path(state, "rebasing"), 1, "%s", ""); else @@ -756,6 +765,7 @@ static int parse_mail(struct am_state *state, const char *mail) cp.out = xopen(am_path(state, "info"), O_WRONLY | O_CREAT, 0777); argv_array_push(&cp.args, "mailinfo"); + argv_array_push(&cp.args, state->utf8 ? "-u" : "-n"); argv_array_push(&cp.args, am_path(state, "msg")); argv_array_push(&cp.args, am_path(state, "patch")); @@ -1470,6 +1480,8 @@ int cmd_am(int argc, const char **argv, const char *prefix) OPT__QUIET(&state.quiet, N_("be quiet")), OPT_BOOL('s', "signoff", &state.append_signoff, N_("add a Signed-off-by line to the commit message")), + OPT_BOOL('u', "utf8", &state.utf8, + N_("recode into utf8 (default)")), OPT_CALLBACK(0, "patch-format", &patch_format, N_("format"), N_("format the patch(es) are in"), parse_opt_patchformat), -- 2.5.0.rc0.76.gb2c6e93 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 14/44] builtin-am: reject patches when there's a session in progress
Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07), git-am would error out if the user gave it mbox(s) on the command-line, but there was a session in progress. Since c95b138 (Fix git-am safety checks, 2006-09-15), git-am would detect if the user attempted to feed it a mbox via stdin, by checking if stdin is not a tty and there is no resume command given. Re-implement the above two safety checks. Signed-off-by: Paul Tan --- Notes: NOTE: there's no test for this builtin/am.c | 19 +-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 1db95f2..e066a97 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1147,9 +1147,24 @@ int cmd_am(int argc, const char **argv, const char *prefix) if (read_index_preload(&the_index, NULL) < 0) die(_("failed to read the index")); - if (am_in_progress(&state)) + if (am_in_progress(&state)) { + /* +* Catch user error to feed us patches when there is a session +* in progress: +* +* 1. mbox path(s) are provided on the command-line. +* 2. stdin is not a tty: the user is trying to feed us a patch +*from standard input. This is somewhat unreliable -- stdin +*could be /dev/null for example and the caller did not +*intend to feed us a patch but wanted to continue +*unattended. +*/ + if (argc || (resume == RESUME_FALSE && !isatty(0))) + die(_("previous rebase directory %s still exists but mbox given."), + state.dir); + am_load(&state); - else { + } else { struct argv_array paths = ARGV_ARRAY_INIT; int i; -- 2.5.0.rc0.76.gb2c6e93 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 08/44] builtin-am: apply patch with git-apply
Implement applying the patch to the index using git-apply. 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. Helped-by: Junio C Hamano Signed-off-by: Paul Tan --- Notes: v4 * Switch firstline() to linelen(), use .%s format specifier. * Remove blank lines. builtin/am.c | 73 +++- 1 file changed, 72 insertions(+), 1 deletion(-) diff --git a/builtin/am.c b/builtin/am.c index 2fbad5b..0762c70 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -10,6 +10,7 @@ #include "dir.h" #include "run-command.h" #include "quote.h" +#include "lockfile.h" /** * Returns 1 if the file is empty or does not exist, 0 otherwise. @@ -42,6 +43,14 @@ static int strbuf_getline_crlf(struct strbuf *sb, FILE *fp) return 0; } +/** + * Returns the length of the first line of msg. + */ +static int linelen(const char *msg) +{ + return strchrnul(msg, '\n') - msg; +} + enum patch_format { PATCH_FORMAT_UNKNOWN = 0, PATCH_FORMAT_MBOX @@ -561,6 +570,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 `mail` using git-mailinfo, extracting its patch and authorship info. * state->msg will be set to the patch message. state->author_name, * state->author_email and state->author_date will be set to the patch author's @@ -650,10 +673,35 @@ finish: } /** + * 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 mail. */ static void am_run(struct am_state *state) { + refresh_and_write_cache(); + while (state->cur <= state->last) { const char *mail = am_path(state, msgnum(state)); @@ -666,7 +714,27 @@ static void am_run(struct am_state *state) write_author_script(state); write_commit_msg(state); - /* TODO: Patch application not implemented yet */ + printf_ln(_("Applying: %.*s"), linelen(state->msg), state->msg); + + if (run_apply(state) < 0) { + int advice_amworkdir = 1; + + printf_ln(_("Patch failed at %s %.*s"), msgnum(state), + linelen(state->msg), state->msg); + + git_config_get_bool("advice.amworkdir", &advice_amworkdir); + + if (advice_amworkdir) + 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); @@ -728,6 +796,9 @@ int cmd_am(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, options, 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.5.0.rc0.76.gb2c6e93 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 03/44] builtin-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. Since git-am.sh cannot handle any environment modifications by setup_git_directory(), "am" is declared with no setup flags in git.c. On the other hand, to re-implement git-am.sh in builtin/am.c, we need to run all the git dir and work tree setup logic that git.c typically does for us. As such, we work around this temporarily by copying the logic in git.c's run_builtin(), which is roughly: prefix = setup_git_directory(); trace_repo_setup(prefix); setup_work_tree(); This redirection should be removed when all the features of git-am.sh have been re-implemented in builtin/am.c. Helped-by: Junio C Hamano Signed-off-by: Paul Tan --- Notes: v4 * Don't use NO_SETUP * Comment wording changes Makefile | 1 + builtin.h| 1 + builtin/am.c | 29 + git.c| 6 ++ 4 files changed, 37 insertions(+) create mode 100644 builtin/am.c diff --git a/Makefile b/Makefile index 93e4fa2..ff9bdc0 100644 --- a/Makefile +++ b/Makefile @@ -811,6 +811,7 @@ LIB_OBJS += xdiff-interface.o LIB_OBJS += zlib.o BUILTIN_OBJS += builtin/add.o +BUILTIN_OBJS += builtin/am.o BUILTIN_OBJS += builtin/annotate.o BUILTIN_OBJS += builtin/apply.o BUILTIN_OBJS += builtin/archive.o diff --git a/builtin.h b/builtin.h index ea3c834..f30cf00 100644 --- a/builtin.h +++ b/builtin.h @@ -30,6 +30,7 @@ extern int textconv_object(const char *path, unsigned mode, const unsigned char extern int is_builtin(const char *s); extern int cmd_add(int argc, const char **argv, const char *prefix); +extern int cmd_am(int argc, const char **argv, const char *prefix); extern int cmd_annotate(int argc, const char **argv, const char *prefix); extern int cmd_apply(int argc, const char **argv, const char *prefix); extern int cmd_archive(int argc, const char **argv, const char *prefix); diff --git a/builtin/am.c b/builtin/am.c new file mode 100644 index 000..fd32caf --- /dev/null +++ b/builtin/am.c @@ -0,0 +1,29 @@ +/* + * 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) +{ + + /* +* NEEDSWORK: Once all the features of git-am.sh have been +* re-implemented in builtin/am.c, this preamble can be removed. +*/ + if (!getenv("_GIT_USE_BUILTIN_AM")) { + const char *path = mkpath("%s/git-am", git_exec_path()); + + if (sane_execvp(path, (char **)argv) < 0) + die_errno("could not exec %s", path); + } else { + prefix = setup_git_directory(); + trace_repo_setup(prefix); + setup_work_tree(); + } + + return 0; +} diff --git a/git.c b/git.c index e7a7713..e84e1a1 100644 --- a/git.c +++ b/git.c @@ -370,6 +370,12 @@ 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 }, + /* +* NEEDSWORK: Once the redirection to git-am.sh in builtin/am.c has +* been removed, this entry should be changed to +* RUN_SETUP | NEED_WORK_TREE +*/ + { "am", cmd_am }, { "annotate", cmd_annotate, RUN_SETUP }, { "apply", cmd_apply, RUN_SETUP_GENTLY }, { "archive", cmd_archive }, -- 2.5.0.rc0.76.gb2c6e93 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 01/44] 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 --- 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 c6d391f..e168dfd 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -717,6 +717,7 @@ extern void *xrealloc(void *ptr, size_t size); extern void *xcalloc(size_t nmemb, size_t size); extern void *xmmap(void *start, size_t length, int prot, int flags, int fd, off_t offset); extern void *xmmap_gently(void *start, size_t length, int prot, int flags, int fd, off_t offset); +extern int xopen(const char *path, int flags, ...); extern ssize_t xread(int fd, void *buf, size_t len); extern ssize_t xwrite(int fd, const void *buf, size_t len); extern ssize_t xpread(int fd, void *buf, size_t len, off_t offset); diff --git a/wrapper.c b/wrapper.c index ff49807..7e13ae0 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.5.0.rc0.76.gb2c6e93 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 05/44] builtin-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. Helped-by: Junio C Hamano Signed-off-by: Paul Tan --- Notes: v4 * The term "patch" was overloaded to mean "the RFC2822 mail" and the "patch diff". To avoid confusion, use "mail" to refer to the RFC2822 mail, and thus split_patches() has been renamed to split_mail(). * An argv_array is used for `paths` instead of a string_list to avoid clunkiness with string_list_item in later patches. builtin/am.c | 107 +-- 1 file changed, 104 insertions(+), 3 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 5b4e9af..136ccc6 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; }; /** @@ -28,6 +37,8 @@ static void am_state_init(struct am_state *state, const char *dir) assert(dir); state->dir = xstrdup(dir); + + state->prec = 4; } /** @@ -120,13 +131,71 @@ static void am_destroy(const struct am_state *state) } /** + * Splits out individual email patches from `paths`, where each path is either + * a mbox file or a Maildir. Returns 0 on success, -1 on failure. + */ +static int split_mail_mbox(struct am_state *state, const char **paths) +{ + struct child_process cp = CHILD_PROCESS_INIT; + 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); + argv_array_push(&cp.args, "-b"); + argv_array_push(&cp.args, "--"); + argv_array_pushv(&cp.args, paths); + + if (capture_command(&cp, &last, 8)) + return -1; + + state->cur = 1; + state->last = strtol(last.buf, NULL, 10); + + return 0; +} + +/** + * Splits a list of files/directories into individual email patches. Each path + * in `paths` must be a file/directory that is formatted according to + * `patch_format`. + * + * Once split out, the individual email 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 mail, and state->last will + * be set to the index of the last mail. + * + * Returns 0 on success, -1 on failure. + */ +static int split_mail(struct am_state *state, enum patch_format patch_format, + const char **paths) +{ + switch (patch_format) { + case PATCH_FORMAT_MBOX: + return split_mail_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, + const char **paths) { if (mkdir(state->dir, 0777) < 0 && errno != EEXIST) die_errno(_("failed to create directory '%s'"), state->dir); + if (split_mail(state, patch_format, paths) < 0) { + am_destroy(state); + die(_("Failed to split patches.")); + } + /* * NOTE: Since the "next" and "last" files determine if an am_state * session is in progress, they should be written last. @@ -162,9 +231,25 @@ static void am_run(struct am_state *state) am_destroy(state); } +/** + * parse_options() callback that validates and sets opt->value to the + * PATCH_FORMAT_* enum value corresponding to `arg`. + */ +static int parse_opt_patchformat(const struct option *opt, const char *arg, int unset) +{ + int *opt_value = opt->value; + + if (!strcmp(arg, "mbox")) + *opt_value = PATCH_FORMAT_MBOX; + else + return error(_("Invalid value for --patch-forma
[PATCH v4 07/44] builtin-am: extract patch and commit info 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 Helped-by: Johannes Schindelin Signed-off-by: Paul Tan --- Notes: v4 * The word "patch" was overloaded to mean "the RFC2822 mail" and the "patch diff". To avoid confusion, we use "mail" to mean "the RFC2822 mail", and thus parse_patch() has been renamed to parse_mail(). * To discourage people touching the code from modifying the contents of the author_name, author_email, author_date and msg fields, as well as to make any modifications to them stick out to reviewers, they have been changed to char*. * Besides parse_mail(), the msg field can also be modified in other ways such as from am_load(), and in interactive mode when implemented in a later patch. At these points, users may sneak in NUL bytes in the commit message. As such, we need to store the "msg_len" as well so as to properly error out should the user sneak in any NUL bytes, rather than ignore the rest of the buffer after the NUL byte. * To aid in this, reading and writing to the "final-commit" file has been factored out into read_commit_msg() and write_commit_msg() functions that will properly set the msg_len field as well. * Restructuring of write_author_script() so only one strbuf needs to be used. builtin/am.c | 335 +++ 1 file changed, 335 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 31d85eb..2fbad5b 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; +} /** * Like strbuf_getline(), but treats both '\n' and "\r\n" as line terminators. @@ -38,6 +55,13 @@ struct am_state { int cur; int last; + /* commit metadata and message */ + char *author_name; + char *author_email; + char *author_date; + char *msg; + size_t msg_len; + /* number of digits in patch filename */ int prec; }; @@ -63,6 +87,18 @@ static void am_state_release(struct am_state *state) { if (state->dir) free(state->dir); + + if (state->author_name) + free(state->author_name); + + if (state->author_email) + free(state->author_email); + + if (state->author_date) + free(state->author_date); + + if (state->msg) + free(state->msg); } /** @@ -115,6 +151,167 @@ static int read_state_file(struct strbuf *sb, const struct am_state *state, } /** + * Reads a KEY=VALUE shell variable assignment from `fp`, returning the VALUE + * as a newly-allocated string. VALUE must be a quoted string, and the KEY must + * match `key`. Returns NULL on failure. + * + * This is used by read_author_script() to read the GIT_AUTHOR_* variables from + * the author-script. + */ +static char *read_shell_var(FILE *fp, const char *key) +{ + struct strbuf sb = STRBUF_INIT; + const char *str; + + if (strbuf_getline(&sb, fp, '\n')) + goto fail; + + if (!skip_prefix(sb.buf, key, &str)) + goto fail; + + if (!skip_prefix(str, "=", &str)) + goto fail; + + strbuf_remove(&sb, 0, str - sb.buf); + + str = sq_dequote(sb.buf); + if (!str) + goto fail; + + return strbuf_detach(&sb, NULL); + +fail: + strbuf_release(&sb); + return NULL; +} + +/** + * Reads and parses the state directory's "author-script" file, and sets + * state->author_name, state->author_email and state->author_date accordingly. + * Returns 0 on success, -1 if the file could not be parsed. + * + * 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. We are strict + * with
[PATCH v4 06/44] builtin-am: auto-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 in builtin/am.c. RFC 2822 requires that lines are terminated by "\r\n". To support this, implement strbuf_getline_crlf(), which will remove both '\n' and "\r\n" from the end of the line. Helped-by: Junio C Hamano Helped-by: Eric Sunshine Helped-by: Johannes Schindelin Signed-off-by: Paul Tan --- Notes: v4 * Using strbuf_trim() to remove the \r from CRLF lines is obviously wrong. Instead, implement strbuf_getline_crlf() to do it correctly for us. * Use a regex * Instead of re-opening the file again in is_email(), rewind the already-opened data stream. builtin/am.c | 109 +++ 1 file changed, 109 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 136ccc6..31d85eb 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -10,6 +10,21 @@ #include "dir.h" #include "run-command.h" +/** + * Like strbuf_getline(), but treats both '\n' and "\r\n" as line terminators. + */ +static int strbuf_getline_crlf(struct strbuf *sb, FILE *fp) +{ + if (strbuf_getwholeline(sb, fp, '\n')) + return EOF; + if (sb->buf[sb->len - 1] == '\n') { + strbuf_setlen(sb, sb->len - 1); + if (sb->len > 0 && sb->buf[sb->len - 1] == '\r') + strbuf_setlen(sb, sb->len - 1); + } + return 0; +} + enum patch_format { PATCH_FORMAT_UNKNOWN = 0, PATCH_FORMAT_MBOX @@ -131,6 +146,92 @@ static void am_destroy(const struct am_state *state) } /** + * Determines if the file looks like a piece of RFC2822 mail by grabbing all + * non-indented lines and checking if they look like they begin with valid + * header field names. + * + * Returns 1 if the file looks like a piece of mail, 0 otherwise. + */ +static int is_mail(FILE *fp) +{ + const char *header_regex = "^[!-9;-~]+:"; + struct strbuf sb = STRBUF_INIT; + regex_t regex; + int ret = 1; + + if (fseek(fp, 0L, SEEK_SET)) + die_errno(_("fseek failed")); + + if (regcomp(®ex, header_regex, REG_NOSUB | REG_EXTENDED)) + die("invalid pattern: %s", header_regex); + + while (!strbuf_getline_crlf(&sb, fp)) { + 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 header_regex */ + if (regexec(®ex, sb.buf, 0, NULL, 0)) { + ret = 0; + goto done; + } + } + +done: + regfree(®ex); + 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(const char **paths) +{ + enum patch_format ret = PATCH_FORMAT_UNKNOWN; + struct strbuf l1 = STRBUF_INIT; + FILE *fp; + + /* +* We default to mbox format if input is from stdin and for directories +*/ + if (!*paths || !strcmp(*paths, "-") || is_directory(*paths)) + return PATCH_FORMAT_MBOX; + + /* +* Otherwise, check the first few lines of the first patch, starting +* from the first non-blank line, to try to detect its format. +*/ + + fp = xfopen(*paths, "r"); + + while (!strbuf_getline_crlf(&l1, fp)) { + if (l1.len) + break; + } + + if (starts_with(l1.buf, "From ") || starts_with(l1.buf, "From: ")) { + ret = PATCH_FORMAT_MBOX; + goto done; + } + + if (l1.len && is_mail(fp)) { + ret = PATCH_FORMAT_MBOX; + goto done; + } + +done: + fclose(fp); + strbuf_release(&l1); + return ret; +} + +/** * Splits out individual email patches from `paths`, where each path is either * a mbox file or a Maildir. Returns 0 on success, -1 on failure. */ @@ -188,6 +289,14 @@ static int split_mail(struct am_state *state, enum patch_format patch_format, static void am_setup(struct am_state *state, enum patch_format patch_format, const char **paths) { + if (!patch_format) + patch_format = detect_patch_format(paths); + + if (!patch_format) { +
[PATCH v4 04/44] builtin-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. Helped-by: Junio C Hamano Helped-by: Stefan Beller Helped-by: Johannes Schindelin Signed-off-by: Paul Tan --- Notes: v4 * Corrected docstring of read_state_file() * Corrected docstring of am_state_release() * am_state's "dir" field is now a char*. To help API users, am_state_init() takes an additional const char *dir argument. * The opt_* option variables, am_options[] and am_usage[] have been moved into cmd_am()'s scope. * signature of read_state_file() has been changed to read_state_file(strbuf, state, file, trim) builtin/am.c | 180 +++ 1 file changed, 180 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index fd32caf..5b4e9af 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -6,9 +6,174 @@ #include "cache.h" #include "builtin.h" #include "exec_cmd.h" +#include "parse-options.h" +#include "dir.h" + +struct am_state { + /* state directory path */ + char *dir; + + /* current and last patch numbers, 1-indexed */ + int cur; + int last; +}; + +/** + * Initializes am_state with the default values. The state directory is set to + * dir. + */ +static void am_state_init(struct am_state *state, const char *dir) +{ + memset(state, 0, sizeof(*state)); + + assert(dir); + state->dir = xstrdup(dir); +} + +/** + * Releases memory allocated by an am_state. + */ +static void am_state_release(struct am_state *state) +{ + if (state->dir) + free(state->dir); +} + +/** + * Returns path relative to the am_state directory. + */ +static inline const char *am_path(const struct am_state *state, const char *path) +{ + assert(state->dir); + assert(path); + return mkpath("%s/%s", state->dir, 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, &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` in the `state` directory into `sb`. Returns the + * number of bytes read on success, -1 if the file does not exist. If `trim` is + * set, trailing whitespace will be removed. + */ +static int read_state_file(struct strbuf *sb, const struct am_state *state, + const char *file, int trim) +{ + strbuf_reset(sb); + + if (strbuf_read_file(sb, am_path(state, file), 0) >= 0) { + if (trim) + strbuf_trim(sb); + + return sb->len; + } + + if (errno == ENOENT) + return -1; + + die_errno(_("could not read '%s'"), am_path(state, file)); +} + +/** + * Loads state from disk. + */ +static void am_load(struct am_state *state) +{ + struct strbuf sb = STRBUF_INIT; + + if (read_state_file(&sb, state, "next", 1) < 0) + die("BUG: state file 'next' does not exist"); + state->cur = strtol(sb.buf, NULL, 10); + + if (read_state_file(&sb, state, "last", 1) < 0) + die("BUG: state file 'last' does not exist"); + state->last = strtol(sb.buf, NULL, 10); + + strbuf_release(&sb); +} + +/** + * Removes the am_state directory, forcefully terminating the current am + * session. + */ +static void am_destroy(const struct am
[PATCH v4 02/44] 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 --- 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 e168dfd..392da79 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -722,6 +722,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 7e13ae0..f127eb3 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.5.0.rc0.76.gb2c6e93 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 00/44] Make git-am a builtin
This patch series depends on pt/pull-builtin. This is a re-roll of [WIP v3]. This patch series is now out of WIP, as the git-am feature set is complete. I've marked out some features which lack tests though, which I'm working on fixing. Thanks Junio, Stefan, Johannes for the reviews last round. Also, v4, with 44 patches ;-) Previous versions: [WIP v1] http://thread.gmane.org/gmane.comp.version-control.git/270048 [WIP v2] http://thread.gmane.org/gmane.comp.version-control.git/271381 [WIP v3] http://thread.gmane.org/gmane.comp.version-control.git/271967 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 (44): wrapper: implement xopen() wrapper: implement xfopen() builtin-am: implement skeletal builtin am builtin-am: implement patch queue mechanism builtin-am: split out mbox/maildir patches with git-mailsplit builtin-am: auto-detect mbox patches builtin-am: extract patch and commit info with git-mailinfo builtin-am: apply patch with git-apply builtin-am: implement committing applied patch builtin-am: refuse to apply patches if index is dirty builtin-am: implement --resolved/--continue builtin-am: implement --skip builtin-am: implement --abort builtin-am: reject patches when there's a session in progress builtin-am: implement -q/--quiet, GIT_QUIET builtin-am: exit with user friendly message on failure builtin-am: implement -s/--signoff cache-tree: introduce write_index_as_tree() builtin-am: implement --3way, am.threeway builtin-am: implement --rebasing mode builtin-am: bypass git-mailinfo when --rebasing builtin-am: handle stray state directory builtin-am: implement -u/--utf8 builtin-am: implement -k/--keep, --keep-non-patch builtin-am: implement --[no-]message-id, am.messageid builtin-am: support --keep-cr, am.keepcr builtin-am: implement --[no-]scissors builtin-am: pass git-apply's options to git-apply builtin-am: implement --ignore-date builtin-am: implement --committer-date-is-author-date builtin-am: implement -S/--gpg-sign, commit.gpgsign builtin-am: invoke post-rewrite hook builtin-am: support automatic notes copying builtin-am: invoke applypatch-msg hook builtin-am: invoke pre-applypatch hook builtin-am: invoke post-applypatch hook builtin-am: rerere support builtin-am: support and auto-detect StGit patches builtin-am: support and auto-detect StGit series files builtin-am: support and auto-detect mercurial patches builtin-am: implement -i/--interactive builtin-am: implement legacy -b/--binary option builtin-am: check for valid committer ident builtin-am: remove redirection to git-am.sh Makefile|2 +- builtin.h |1 + builtin/am.c| 2335 +++ cache-tree.c| 29 +- cache-tree.h|1 + git-am.sh => contrib/examples/git-am.sh |0 git-compat-util.h |2 + git.c |1 + wrapper.c | 43 + 9 files changed, 2401 insertions(+), 13 deletions(-) create mode 100644 builtin/am.c rename git-am.sh => contrib/examples/git-am.sh (100%) -- 2.5.0.rc0.76.gb2c6e93 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/WIP v3 07/31] am: extract patch, message and authorship with git-mailinfo
On Thu, Jun 25, 2015 at 12:36 AM, Johannes Schindelin wrote: > On 2015-06-18 13:25, Paul Tan wrote: >> diff --git a/builtin/am.c b/builtin/am.c >> index 7b97ea8..d6434e4 100644 >> --- a/builtin/am.c >> +++ b/builtin/am.c >> @@ -94,6 +126,105 @@ static int read_state_file(struct strbuf *sb, >> const char *file, size_t hint, int >> } >> >> /** >> + * Reads a KEY=VALUE shell variable assignment from fp, and returns the >> VALUE >> + * in `value`. VALUE must be a quoted string, and the KEY must match `key`. >> + * Returns 0 on success, -1 on failure. >> + * >> + * This is used by read_author_script() to read the GIT_AUTHOR_* variables >> from >> + * the author-script. >> + */ >> +static int read_shell_var(struct strbuf *value, FILE *fp, const char *key) >> +{ >> + struct strbuf sb = STRBUF_INIT; >> + char *str; >> + >> + if (strbuf_getline(&sb, fp, '\n')) >> + return -1; >> + >> + if (!skip_prefix(sb.buf, key, (const char **)&str)) >> + return -1; >> + >> + if (!skip_prefix(str, "=", (const char **)&str)) >> + return -1; >> + >> + str = sq_dequote(str); >> + if (!str) >> + return -1; >> + >> + strbuf_reset(value); >> + strbuf_addstr(value, str); >> + >> + strbuf_release(&sb); >> + >> + return 0; >> +} > > How about using `strbuf_remove()` and keeping `str` as `const char *`? OK, I'll try that out. Looks like this now: static char *read_shell_var(FILE *fp, const char *key) { struct strbuf sb = STRBUF_INIT; const char *str; if (strbuf_getline(&sb, fp, '\n')) return NULL; if (!skip_prefix(sb.buf, key, &str)) return NULL; if (!skip_prefix(str, "=", &str)) return NULL; strbuf_remove(&sb, 0, str - sb.buf); str = sq_dequote(sb.buf); if (!str) return NULL; return strbuf_detach(&sb, NULL); } > I also think we can fold it into the `read_author_script()` function and make > it more resilient with regards to the order of the variables. Something like > this: > [...] Hmm, I think we should be very strict about parsing the author-script file though. As explained in read_author_script(), git-am.sh evals the author-script, which we can't in C. I would much rather we barf at the first sign that the author-script is not what we expect, rather than attempt to parse it as much as possible, but end up with the wrong results as compared to git-am.sh. Besides, currently git-am.sh will always write the author-script with the order of GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL and GIT_AUTHOR_DATE. If the order is wrong, I would think it means that something is messing with the author-script, and it would be better if we bail out immediately, instead of potentially doing something wrong. >> +/** >> + * Saves state->author_name, state->author_email and state->author_date in >> + * `filename` as an "author script", which is the format used by git-am.sh. >> + */ >> +static void write_author_script(const struct am_state *state) >> +{ >> + static const char fmt[] = "GIT_AUTHOR_NAME=%s\n" >> + "GIT_AUTHOR_EMAIL=%s\n" >> + "GIT_AUTHOR_DATE=%s\n"; >> + struct strbuf author_name = STRBUF_INIT; >> + struct strbuf author_email = STRBUF_INIT; >> + struct strbuf author_date = STRBUF_INIT; >> + >> + sq_quote_buf(&author_name, state->author_name.buf); >> + sq_quote_buf(&author_email, state->author_email.buf); >> + sq_quote_buf(&author_date, state->author_date.buf); > > The `sq_quote_buf()` function does not call `strbuf_reset()`. Therefore you > could just use a single strbuf to construct the entire three lines and then > write that out. Again, if you follow my suggestion to keep a "scratch pad" > strbuf in am_state, you could reuse that. Right, makes sense. I've implemented it on my end. 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 v3 06/31] am: detect mbox patches
On Thu, Jun 25, 2015 at 9:40 PM, Paul Tan wrote: > On Wed, Jun 24, 2015 at 11:10 PM, Johannes Schindelin > wrote: >>> + else if (l1.len && l2.len && l3.len && is_email(paths->items->string)) >>> + ret = PATCH_FORMAT_MBOX; >> >> Maybe we can do better than this by folding the `is_email() function into >> this here function, reusing the same strbuf to read the lines and keeping >> track of the email header lines we saw... I would really like to avoid >> opening the same file twice just to figure out whether it is in email format. > > Okay, how about every time we call a strbuf_getline(), we save the > line to a string_list as well? Like string_list_getline_crlf() below: > [...] Hmm, on second thought, I don't think it's worth the code complexity. While I agree it's desirable to not open the file twice, I don't think detecting the patch format is so IO intensive that it needs to be optimized to that extent. Instead, we should probably just modify is_email() to take a FILE*, and then fseek(fp, 0L, SEEK_SET) to the beginning. I think the logic of is_email() is complex and so it should not be folded into the detect_patch_format() function, especially since we may add detection of other patch formats in the future, and may need more complex heuristics. 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 v3 04/31] am: implement patch queue mechanism
On Wed, Jun 24, 2015 at 10:59 PM, Johannes Schindelin wrote: >> diff --git a/builtin/am.c b/builtin/am.c >> index dbc8836..af68c51 100644 >> --- a/builtin/am.c >> +++ b/builtin/am.c >> @@ -6,6 +6,158 @@ >> #include "cache.h" >> #include "builtin.h" >> #include "exec_cmd.h" >> +#include "parse-options.h" >> +#include "dir.h" >> + >> +struct am_state { >> + /* state directory path */ >> + struct strbuf dir; >> + >> + /* current and last patch numbers, 1-indexed */ >> + int cur; >> + int last; >> +}; >> + >> +/** >> + * Initializes am_state with the default values. >> + */ >> +static void am_state_init(struct am_state *state) >> +{ >> + memset(state, 0, sizeof(*state)); >> + >> + strbuf_init(&state->dir, 0); >> +} > > With strbufs, we use the initializer STRBUF_INIT. How about using > > #define AM_STATE_INIT { STRBUF_INIT, 0, 0 } > > here? Later in the patch series am_state_init() will also take into account config settings when filling up the default values. e.g. see patches 25/31[1] or 31/31[2]. I think that is reasonable: the purpose of am_state_init() is to initialize the am_state struct with the default values, and the default values can be set by the user through the config settings. This means, though, that we can't use initializers without introducing global variables. [1] http://thread.gmane.org/gmane.comp.version-control.git/271967/focus=271972 [2] http://thread.gmane.org/gmane.comp.version-control.git/271967/focus=271972 >> +/** >> + * Reads the contents of `file`. The third argument can be used to give a >> hint >> + * about the file size, to avoid reallocs. Returns number of bytes read on >> + * success, -1 if the file does not exist. If trim is set, trailing >> whitespace >> + * will be removed from the file contents. >> + */ >> +static int read_state_file(struct strbuf *sb, const char *file, >> size_t hint, int trim) >> +{ >> + strbuf_reset(sb); >> + if (strbuf_read_file(sb, file, hint) >= 0) { >> + if (trim) >> + strbuf_trim(sb); >> + >> + return sb->len; >> + } >> + >> + if (errno == ENOENT) >> + return -1; >> + >> + die_errno(_("could not read '%s'"), file); >> +} > > A couple of thoughts: > > - why not reuse the strbuf by making it a part of the am_state()? That way, > you can allocate, say, 1024 bytes (should be plenty enough for most of our > operations) and just reuse them in all of the functions. We will not make any > of this multi-threaded anyway, I don't think. But too much usage of this temporary strbuf may lead to a situation where one function calls another, and both functions write to the strbuf and clobber its contents. Besides, if we are talking about read_state_file(), it takes an external strbuf, so it gives the caller the freedom to choose which strbuf it uses (e.g. if it is stack allocated or in the am_state struct). I think it's more flexible this way. > - Given that we only read short files all the time, why not skip the hint > parameter? Especially if we reuse the strbuf, it should be good enough to > allocate a reasonable buffer first and then just assume that we do not have > to reallocate it all that often anyway. Doh! Right, the hint parameter is quite useless, since in am_load() we use the same strbuf anyway. (And strbuf_init() can set a hint as well) I've removed it on my side. > - Since we only read files from the state directory, why not pass the > basename as parameter? That way we can avoid calling `am_path()` explicitly > over and over again (and yours truly cannot forget to call `am_path()` in > future patches). Makes sense. After all, this function is called read_STATE_file() ;-) > - If you agree with these suggestions, the signature would become something > like > > static void read_state_file(struct am_state *state, const char *basename, int > trim); So for now, my function signature is static void read_state_file(struct strbuf *sb, const struct am_state *state, const char *basename, int trim); >> +/** >> + * 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); >> +} > > Given that `remove_dir_recursively()` has to reset the strbuf with the > directory's path to the original value before it returns (because it recurses > into itself, therefore the value *has* to be reset when returning), we can > just call > > remove_dir_recursively(&state->dir, 0); > > and do not need another temporary strbuf. Ah right. Although, state->dir is not an strbuf anymore on my side. As Junio[3] rightfully noted, state->dir is not modified by the am_*() API, so it's been changed to a char*. Which means an strbuf is required to be passed to remove_dir_recursively(); >> +/** >> + * Increments the patch pointer, and cleans am_state f
Re: [PATCH/WIP v3 06/31] am: detect mbox patches
On Wed, Jun 24, 2015 at 11:10 PM, Johannes Schindelin wrote: > Hi Paul, > > On 2015-06-18 13:25, Paul Tan wrote: > >> diff --git a/builtin/am.c b/builtin/am.c >> index e9a3687..7b97ea8 100644 >> --- a/builtin/am.c >> +++ b/builtin/am.c >> @@ -121,6 +121,96 @@ static void am_destroy(const struct am_state *state) >> strbuf_release(&sb); >> } >> >> +/* >> + * Returns 1 if the file looks like a piece of email a-la RFC2822, 0 >> otherwise. >> + * We check this by grabbing all the non-indented lines and seeing if they >> look >> + * like they begin with valid header field names. >> + */ >> +static int is_email(const char *filename) >> +{ >> + struct strbuf sb = STRBUF_INIT; >> + FILE *fp = xfopen(filename, "r"); >> + int ret = 1; >> + >> + while (!strbuf_getline(&sb, fp, '\n')) { >> + const char *x; >> + >> + strbuf_rtrim(&sb); >> + >> + if (!sb.len) >> + break; /* End of header */ >> + >> + /* Ignore indented folded lines */ >> + if (*sb.buf == '\t' || *sb.buf == ' ') >> + continue; >> + >> + /* It's a header if it matches the regexp "^[!-9;-~]+:" */ > > Why not just compile a regex and use it here? We use regexes elsewhere > anyway... Ah, you're right. A regular expression would definitely be clearer. I've fixed it on my end. >> +/** >> + * 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'); > > We should test the return value of `strbuf_getline()`; if EOF was reached > already, `strbuf_getwholeline()` does not touch the strbuf. I know, the > strbuf is still initialized empty here, but it is too easy to forget when > changing this code. Ah OK. I'll vote for doing a strbuf_reset() just before the strbuf_getline() though. >> + 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; > > Hmm. We can test that earlier and return without reading from the file any > further, I think. The "reading 3 lines at the beginning" logic is meant to support a later patch where support for StGit and mercurial patches is added. That said, it's true that we don't need to read 3 lines in this patch, so I think I will remove it in this patch. >> + else if (l1.len && l2.len && l3.len && is_email(paths->items->string)) >> + ret = PATCH_FORMAT_MBOX; > > Maybe we can do better than this by folding the `is_email() function into > this here function, reusing the same strbuf to read the lines and keeping > track of the email header lines we saw... I would really like to avoid > opening the same file twice just to figure out whether it is in email format. Okay, how about every time we call a strbuf_getline(), we save the line to a string_list as well? Like string_list_getline_crlf() below: /** * Like strbuf_getline(), but supports both '\n' and "\r\n" as line * terminators. */ static int strbuf_getline_crlf(struct
Re: [PATCH/WIP v3 07/31] am: extract patch, message and authorship with git-mailinfo
On Wed, Jun 24, 2015 at 11:59 PM, Junio C Hamano wrote: > Paul Tan writes: > >> 3. I'm over-thinking this and you just want the "struct strbufs" in the >>struct am_state to be switched to "char*"s? > > Yes, everybody interacts with am_state, and these fields are > supposed to be constant during the processing of each patch input > message; they should be simple strings, not strbufs, to make sure if > anybody _does_ muck with them in-place, that would be very visible. > The helpers to initialize them are free to use strbuf API to prepare > these simple string fields, of course. OK. I've implemented it on my end. >> (On a somewhat related thought, currently we do write_file() all over >> the place, which is really ugly. I'm leaning heavily on introducing an >> am_save() function, for "I don't care how it is done but just update the >> contents of the am state directory so that it matches the contents of >> the struct am_state". > > Sure; the scripted Porcelain may have done "echo here, echo there" > instead of "concatenate into a $var and then 'echo $var' at end" as > that is more natural way to program in that environment. You are > doing this in C and "prepare the thing in-core and write it all at > the point to snapshot" may well be the more natural way to program. > > As long as a process that stops in the middle does not leave on-disk > state inconsistent, batching would be fine. For example, you may > apply and commit two (or more) patches without updating the on-disk > state as you do not see need to give control back to the user > (i.e. they applied cleanly) and then write out the on-disk state > with .next incremented by two (or more) before giving the control > back could be a valid optimization (take this example with a grain > of salt, though; I haven't thought too deeply about what should > happen if you Ctrl-C the process in the middle). Right, I briefly wondered if we could hold off writing the am_state to disk as much as possible, and only write to disk when we are about to terminate. This would involve installing an atexit() and SIGTERM signal handler, but I haven't thought too deeply about that. Anyway, moving all the "writing am_state to disk" logic to a central function am_save() would be a good immediate step, I think, so I've implemented it on my end. 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] Revert "git am/mailinfo: Don't look at in-body headers when rebasing"
Hi Junio, On Tue, Jun 16, 2015 at 5:03 PM, Paul Tan wrote: > This reverts commit d25e51596be9271ad833805a3d6f9012dc24ee79, removing > git-mailsplit's --no-inbody-headers option. > > While --no-inbody-headers was introduced to prevent commit messages from > being munged by git-mailinfo while rebasing, the need for this option > disappeared since 5e835ca (rebase: do not munge commit log message, > 2008-04-16), as git-am bypasses git-mailinfo and gets the commit message > directly from the commit ID in the patch. > > git-am is the only user of --no-inbody-headers, and this option is not > documented. As such, it should be removed. > > Signed-off-by: Paul Tan What do you think about applying this patch? Either way, the no-inbody-headers code in git-am.sh is dead code so I don't think I will be implementing it in 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 v3 06/31] am: detect mbox patches
On Fri, Jun 19, 2015 at 5:02 AM, Junio C Hamano wrote: > Paul Tan writes: > >> +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); > > Is this a good thing? strbuf_getline() already has stripped the LF > at the end, so you'd be treating a line with only whitespaces as if > it is a truly empty line. > > I know the series is about literal translation and the script may > lose the distinction between the two, but I do not think you need > (or want) to be literally same for things like this. > > Same comment applies to other uses of "trim" in this patch. No, the uses of strbuf_trim() are not good at all. Will remove them. 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 v3 07/31] am: extract patch, message and authorship with git-mailinfo
Okay, let's focus only on the API design issues. For the record, I'm not completely satisfied with the current code organization and API, however I don't have really good ideas at hand to improve it, so any ideas with examples would be appreciated. On Fri, Jun 19, 2015 at 09:13:00AM -0700, Junio C Hamano wrote: > Paul Tan writes: > With the above fields, it is clear that the above fields are > per-message thing. So the loop to process multiple messages is > conceptually: > > > set up the entire am_state (for things like cur=1, last=N) > for each message { > update per-message fields like cur, author, msg, etc. > process the single message > clean up per-message fileds like cur++, free(msg), etc. > } > tear down the entire am_state > > Reusing the same strbuf and rely on them to clean up after > themselves is simply a bad taste. Hmm, reading "reusing the same strbuf" makes me wonder if I'm misunderstanding something. Do you mean that: 1. We should release the memory in state->msg, state->author_* after processing each message? Because that is what am_next() already does. Granted, it does strbuf_reset(), but it could just as well be strbuf_release(). or 2. The per-message fields (state->msg, state->author_*) should become local variables in am_run()? or 3. I'm over-thinking this and you just want the "struct strbufs" in the struct am_state to be switched to "char*"s? If that is so, I've attached a sample patch below. For (2), my idea is that am_state represents the contents of the state directory at any point in time -- a design feature carried over from git-am.sh. This is why parse_patch() modifies the am_state struct directly -- the "msg" and "author_*" fields would be written to the "final-commit" and "author-script" files directly after. I think it would be confusing if we don't update the am_state struct directly, as we will never know if the "am_state struct" contains the actual current state of the patch application session. (On a somewhat related thought, currently we do write_file() all over the place, which is really ugly. I'm leaning heavily on introducing an am_save() function, for "I don't care how it is done but just update the contents of the am state directory so that it matches the contents of the struct am_state". To save pointless writes, we can compare the am_state with the last-written/loaded am_state, so we only write the files that have changed. What do others think?) > More importantly, we'd want to make sure that people who will be > touching the "process the single message" part in the above loop to > think twice before mucking with read-only fields like author. Okay, I understand and agree with your reasoning. However, note that this "process the single message" part consists solely of run_apply() and do_commit(). Both of them take a "const struct am_state*", which means that the compiler will warn/fail if anybody tries to touch any of the fields in the am_state. This extends to strbufs as well. Isn't this already safe enough? Or do you want this safety to extend throughout am_run() as well? > If they are "char *", they would need to allocate new storage > themselves, format a new value into there, free the original, and > replace the field with the new value. It takes a conscious effort > and very much visible code and would be clear to reviewers what is > going on, and gives them a chance to question if it is a good idea > to update things in-place in the first place. Okay, although personally I think reviewers may tend to miss out that a single line like: state->msg = strbuf_detach(&sb, NULL); introduces a memory leak. > If you left it in strbuf, that invites "let's extend it temporarily > with strbuf_add() and then return to the original once I am done > with this single step", which is an entry to a slippery slope to > "let's extend it with strbuf_add() for my purpose, and I do not even > bother to clean up because I know I am the last person to touch > this". > > So, no, please don't leave a field that won't change during the bulk > of the processing in strbuf, unless you have a compelling reason to > do so (and "compelling reasons" are pretty much limited to "after > all, that field we originally thought it won't change is something > we need to change very often"). Anyway, assuming that you just want the strbufs in the am_state struct to be switched to "char*"s, here's a quick diff of the result. Can I confirm that this is the direction that you w
Re: [PATCH/WIP v3 07/31] am: extract patch, message and authorship with git-mailinfo
On Fri, Jun 19, 2015 at 11:09 PM, Junio C Hamano wrote: > You do realize that strbuf internally does alloc/free so as a solution to > fragmentation issue you are at the mercy of the same alloc/free, don't you? Yes, of course, but it has the "alloc" variable to keep track of the size of the allocated buffer, and provided that ALLOC_GROW() uses a sensible factor to grow the buffer, it won't be calling malloc/free that much to be a problem. Of course, we could do the same and just add a "alloc" variable as well and then use it with strbuf_attach()/strbuf_detach(), but at this point why not just store it in an strbuf then and avoid the extra "alloc" struct member? > The primary thing I care about is to discourage callers of the API element > am_state from touching these fields with strbuf functions. If it is char * > then > the would think twice before modifying (which would involve allocating the > new buffer, forming the new value and assigning into it). With the fields left > as strbuf after they are read or formed by the API (by reading by the API > implementation from $GIT_DIR/rebase-apply/), it is too tempting to do > strbuf_add(), strbuf_trim(), etc., without restoring the value to the original > saying "I'm the last user of it anyway"--that is the sloppiness we can prevent > by not leaving it as strbuf. I don't think this is a good deterrent. If the code wanted to, it could just use strbuf_attach()/strbuf_detach() as well, no? I believe this kind of issue would be better solved with a big WARNING comment and code review. Also, there may be use cases where the user may wish to modify the commit message/author fields. E.g., user may wish to modify the message of the commit that results from am --continue to take into account the conflicts that caused the am to fail. Regards, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in
Re: [PATCH/WIP v3 04/31] am: implement patch queue mechanism
On Fri, Jun 19, 2015 at 4:43 AM, Junio C Hamano wrote: > Paul Tan writes: > >> diff --git a/builtin/am.c b/builtin/am.c >> index dbc8836..af68c51 100644 >> --- a/builtin/am.c >> +++ b/builtin/am.c >> @@ -6,6 +6,158 @@ >> #include "cache.h" >> #include "builtin.h" >> #include "exec_cmd.h" >> +#include "parse-options.h" >> +#include "dir.h" >> + >> +struct am_state { >> + /* state directory path */ >> + struct strbuf dir; > > Is this a temporary variable you will append "/patch", etc. to form > a different string to use for fopen() etc., or do you use separate > strbufs for things like that and this is only used to initialize > them? > > - If the former then "dir" is a misnomer. > > - If the latter, then it probably does not have to be a strbuf; >rather, it should probably be a "const char *". Unless you pass >this directly to functions that take a strbuf, such as >remove_dir_recursively(), that is. It's the latter, and yes it does not need to be an strbuf since it will not usually change. However, I don't think it should be a const char*, as it means that the API user has to keep track of the lifetime of the "dir" string. Note that we will have to git_pathdup() as git_path() returns a static buffer: char *dir = git_pathdup("rebase-apply"); struct am_state state; am_state_init(&state); state.dir = dir; ...stuff... if (foo) { ... separate code path ... am_state_release(&state); free(dir); return 0; } ... separate code path ... am_state_release(&state); free(dir); return 0; Note the additional memory bookkeeping. Instead, I would rather the am_state struct take ownership of the "dir" string and free it in am_state_release(). So dir will be a char*: struct am_state state; am_state_init(&state, git_path("rebase-apply")); ... stuff ... if (foo) { ... separate code path ... am_state_release(&state); return 0; } ... separate code path ... am_state_release(&state); return 0; >> +/** >> + * Release memory allocated by an am_state. >> + */ > > Everybody else in this file seems to say "Initializes", "Returns", > "Reads", etc. While I personally prefer to use imperative > (i.e. give command to this function to "release memory allocated"), > you would want to be consistent throughout the file; "Releases > memory" would make it so. OK >> +/** >> + * 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); > > These two lines are closely related pair; drop the blank in between. > > I am tno sure if write_file() is an appropriate thing to use, > though. What happens when you get interrupted after opening the > file but before you manage to write and close? Shouldn't we be > doing the usual "write to temp, close and then rename to final" > dance? This comment applies to all the other use of write_file(). We could, although I don't think it would help us much. The state directory is made up of many different files, so even if we made modifications to single files atomic, there's no point if we terminate unexpectedly in the middle of writing multiple files to the state directory as the state, as a whole, is corrupted anyway. So, we must be able to make updates to the entire state directory as a single transaction, which is a more difficult problem... Furthermore, while applying patches, we may face abnormal termination between e.g. the patch application and commit, so I think it is safe to say that if abnormal termination occurs, we will not be able to reliably --resume or --skip anyway, and the user should just run --abort to go back to the last known state. However, it would be an improvement if we wrote the "next" and "last" files last, as we use their presence to determine if we have an am session in progress or not, so if we terminate in am_setup() we will just have a stray directory. I will make this change in the next iteration. Regards, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in
Re: [PATCH v4 00/19] Make git-pull a builtin
On Fri, Jun 19, 2015 at 4:13 AM, Junio C Hamano wrote: > I didn't look carefully, but does that mean 04/19 has the "what if > you start from a subdirectory and are still using the scripted one?" > issue we discussed recently for "am"? It does, but git-pull.sh does not care about the original working directory, no? 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 v3 03/31] am: implement skeletal builtin am
On Fri, Jun 19, 2015 at 4:26 AM, Junio C Hamano wrote: > Paul Tan writes: >> @@ -0,0 +1,28 @@ >> +/* >> + * Builtin "git am" >> + * >> + * Based on git-am.sh by Junio C Hamano. >> + */ >> +#include "cache.h" >> +#include "builtin.h" >> +#include "exec_cmd.h" >> + >> +int cmd_am(int argc, const char **argv, const char *prefix) >> +{ >> + /* >> + * FIXME: Once all the features of git-am.sh have been re-implemented >> + * in builtin/am.c, this preamble can be removed. >> + */ > > It's not broken, so "FIXME" is not quite appropriate (and that is > why I sent you "NEEDSWORK"). OK. > Also mention that the entry in the > commands[] array needs "RUN_SETUP | NEED_WORK_TREE" added, I think. OK. >> + if (!getenv("_GIT_USE_BUILTIN_AM")) { >> + const char *path = mkpath("%s/git-am", git_exec_path()); >> + >> + if (sane_execvp(path, (char **)argv) < 0) >> + die_errno("could not exec %s", path); >> + } else { >> + prefix = setup_git_directory(); >> + trace_repo_setup(prefix); >> + setup_work_tree(); >> + } >> + >> + return 0; >> +} >> diff --git a/git.c b/git.c >> index e7a7713..a671535 100644 >> --- a/git.c >> +++ b/git.c >> @@ -370,6 +370,7 @@ static int run_builtin(struct cmd_struct *p, int argc, >> const char **argv) >> >> static struct cmd_struct commands[] = { >> { "add", cmd_add, RUN_SETUP | NEED_WORK_TREE }, >> + { "am", cmd_am, NO_SETUP }, > > NO_SETUP is for things like init and clone that start without a > repository and then work in the one that they create. I think > imitating "archive" or "diff" is more appropriate. Ah OK. I thought that handle_alias() in git.c would chdir() and set GIT_DIR because it called setup_git_directory_gently(), and thus requires NO_SETUP to restore the initial environment, but turns out it chdir()s back to the original directory, and sets GIT_DIR appropriately, so we're OK. 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 v3 07/31] am: extract patch, message and authorship with git-mailinfo
On Fri, Jun 19, 2015 at 5:10 AM, Junio C Hamano wrote: > Paul Tan writes: > >> + /* commit message and metadata */ >> + struct strbuf author_name; >> + struct strbuf author_email; >> + struct strbuf author_date; >> + struct strbuf msg; > > Same comment as "dir" in the earlier patch applies to these. If the > fields are read or computed and then kept constant, use a temporary > variable that is a strbuf to read/compute the final value, and then > detach to a "const char *" field. If they are constantly changing > and in-place updates are vital, then they can and should be strbufs, > but I do not think that is the case for these. I do think it is the case here. The commit message and metadata fields change for every patch we process, and we could be processing a large volume of patches, so we must be careful to not leak any memory. We could use a char* field, but then to prevent memory leaks we would then have to free() it and malloc() a new string for every patch we process, which will lead to lots of malloc()s and free()s over a large volume of patches that can lead to memory fragmentation. 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 v3 10/31] am: refresh the index at start
On Fri, Jun 19, 2015 at 5:28 AM, Junio C Hamano wrote: > Paul Tan writes: > >> 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. > > Good. > > I would actually have expected to see this as part of 08/31, though. Ah right, makes sense since this patch is primarily about git-apply. I've squashed this patch into 08/31. 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/WIP v3 18/31] 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 1807d12..d45dd41 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -18,6 +18,7 @@ #include "diffcore.h" #include "unpack-trees.h" #include "branch.h" +#include "sequencer.h" /** * Returns 1 if the file is empty or does not exist, 0 otherwise. @@ -71,6 +72,8 @@ struct am_state { int quiet; + int sign; + /* override error message when patch failure occurs */ const char *resolvemsg; }; @@ -292,6 +295,9 @@ static void am_load(struct am_state *state) read_state_file(&sb, am_path(state, "quiet"), 2, 1); state->quiet = !strcmp(sb.buf, "t"); + read_state_file(&sb, am_path(state, "sign"), 2, 1); + state->sign = !strcmp(sb.buf, "t"); + strbuf_release(&sb); } @@ -477,6 +483,8 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, write_file(am_path(state, "quiet"), 1, state->quiet ? "t" : "f"); + write_file(am_path(state, "sign"), 1, state->sign ? "t" : "f"); + if (!get_sha1("HEAD", curr_head)) { write_file(am_path(state, "abort-safety"), 1, "%s", sha1_to_hex(curr_head)); update_ref("am", "ORIG_HEAD", curr_head, NULL, 0, UPDATE_REFS_DIE_ON_ERR); @@ -636,6 +644,9 @@ static int parse_patch(struct am_state *state, const char *patch) die_errno(_("could not read '%s'"), am_path(state, "msg")); stripspace(&state->msg, 0); + if (state->sign) + append_signoff(&state->msg, 0, 0); + return 0; } @@ -1007,6 +1018,8 @@ static const char * const am_usage[] = { static struct option am_options[] = { OPT__QUIET(&state.quiet, N_("be quiet")), + OPT_BOOL('s', "signoff", &state.sign, + N_("add a Signed-off-by line to the commit message")), OPT_CALLBACK(0, "patch-format", &opt_patch_format, N_("format"), N_("format the patch(es) are in"), parse_opt_patchformat), OPT_STRING(0, "resolvemsg", &state.resolvemsg, NULL, -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v3 11/31] 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 --- Notes: Note: no tests for this builtin/am.c | 46 ++ 1 file changed, 46 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index a7efe85..9d6ab2a 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -14,6 +14,8 @@ #include "refs.h" #include "commit.h" #include "lockfile.h" +#include "diff.h" +#include "diffcore.h" /** * Returns 1 if the file is empty or does not exist, 0 otherwise. @@ -486,6 +488,43 @@ static void refresh_and_write_cache(void) } /** + * Returns 1 if the index differs from HEAD, 0 otherwise. When on an unborn + * branch, returns 1 if there are entries in the index, 0 otherwise. If an + * strbuf is provided, the space-separated list of files that differ will be + * appended to it. + */ +static int index_has_changes(struct strbuf *sb) +{ + unsigned char head[GIT_SHA1_RAWSZ]; + int i; + + if (!get_sha1_tree("HEAD", head)) { + struct diff_options opt; + + diff_setup(&opt); + DIFF_OPT_SET(&opt, EXIT_WITH_STATUS); + if (!sb) + DIFF_OPT_SET(&opt, QUICK); + do_diff_cache(head, &opt); + diffcore_std(&opt); + for (i = 0; sb && i < diff_queued_diff.nr; i++) { + if (i) + strbuf_addch(sb, ' '); + strbuf_addstr(sb, diff_queued_diff.queue[i]->two->path); + } + diff_flush(&opt); + return DIFF_OPT_TST(&opt, HAS_CHANGES) != 0; + } else { + for (i = 0; sb && i < active_nr; i++) { + if (i) + strbuf_addch(sb, ' '); + strbuf_addstr(sb, active_cache[i]->name); + } + return !!active_nr; + } +} + +/** * Parses `patch` using git-mailinfo. state->msg will be set to the patch * message. state->author_name, state->author_email, state->author_date will be * set to the patch author's name, email and date respectively. The patch's @@ -622,8 +661,15 @@ static void do_commit(const struct am_state *state) */ static void am_run(struct am_state *state) { + struct strbuf sb = STRBUF_INIT; + refresh_and_write_cache(); + if (index_has_changes(&sb)) + die(_("Dirty index: cannot apply patches (dirty: %s)"), sb.buf); + + strbuf_release(&sb); + while (state->cur <= state->last) { const char *patch = am_path(state, msgnum(state)); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v3 23/31] am: handle stray state directory
Should git-am terminate unexpectedly between the point where the state directory is created, but the "next" and "last" files are not written yet, a stray state directory will be left behind. As such, since b141f3c (am: handle stray $dotest directory, 2013-06-15), git-am.sh explicitly recognizes such a stray directory, and allows the user to remove it with am --abort. Re-implement this feature in builtin/am.c. Signed-off-by: Paul Tan --- builtin/am.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 0d7e37c..bbef91f 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1395,6 +1395,21 @@ int cmd_am(int argc, const char **argv, const char *prefix) struct string_list paths = STRING_LIST_INIT_DUP; int i; + /* +* Possible stray dotest directory in the independent-run case. +*/ + if (file_exists(state.dir.buf) && !state.rebasing) { + if (opt_resume == RESUME_ABORT) { + am_destroy(&state); + am_state_release(&state); + return 0; + } + + die(_("Stray %s directory found.\n" + "Use \"git am --abort\" to remove it."), + state.dir.buf); + } + if (opt_resume) die(_("Resolve operation not in progress, we are not resuming.")); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v3 17/31] am: exit with user friendly message on patch failure
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 5f38264..1807d12 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -70,6 +70,9 @@ struct am_state { int prec; int quiet; + + /* override error message when patch failure occurs */ + const char *resolvemsg; }; /** @@ -636,6 +639,21 @@ static int parse_patch(struct am_state *state, const char *patch) return 0; } +/** + * Dies with a user-friendly message on how to proceed after resolving the + * problem. This message can be overridden with state->resolvemsg. + */ +static void NORETURN die_user_resolve(const struct am_state *state) +{ + if (state->resolvemsg) + printf_ln("%s", state->resolvemsg); + else + printf_ln(_("When you have resolved this problem, run \"git am --continue\".\n" + "If you prefer to skip this patch, run \"git am --skip\" instead.\n" + "To restore the original branch and stop patching, run \"git am --abort\".")); + exit(128); +} + /* * Applies current patch with git-apply. Returns 0 on success, -1 otherwise. */ @@ -746,7 +764,7 @@ static void am_run(struct am_state *state) printf_ln(_("The copy of the patch that failed is found in: %s"), am_path(state, "patch")); - exit(128); + die_user_resolve(state); } do_commit(state); @@ -771,13 +789,13 @@ static void am_resolve(struct am_state *state) printf_ln(_("No changes - did you forget to use 'git add'?\n" "If there is nothing left to stage, chances are that something else\n" "already introduced the same changes; you might want to skip this patch.")); - exit(128); + die_user_resolve(state); } if (unmerged_cache()) { printf_ln(_("You still have unmerged paths in your index.\n" "Did you forget to use 'git add'?")); - exit(128); + die_user_resolve(state); } do_commit(state); @@ -991,6 +1009,8 @@ static struct option am_options[] = { OPT__QUIET(&state.quiet, N_("be quiet")), OPT_CALLBACK(0, "patch-format", &opt_patch_format, N_("format"), N_("format the patch(es) are in"), parse_opt_patchformat), + OPT_STRING(0, "resolvemsg", &state.resolvemsg, NULL, + N_("override error message when patch failure occurs")), OPT_CMDMODE(0, "continue", &opt_resume, N_("continue applying patches after resolving a conflict"), RESUME_RESOLVED), -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v3 16/31] 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 4adc487..5f38264 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -68,6 +68,8 @@ struct am_state { /* number of digits in patch filename */ int prec; + + int quiet; }; /** @@ -75,6 +77,8 @@ struct am_state { */ static void am_state_init(struct am_state *state) { + const char *quiet; + memset(state, 0, sizeof(*state)); strbuf_init(&state->dir, 0); @@ -85,6 +89,10 @@ static void am_state_init(struct am_state *state) strbuf_init(&state->msg, 0); state->prec = 4; + + quiet = getenv("GIT_QUIET"); + if (quiet && *quiet) + state->quiet = 1; } /** @@ -108,6 +116,22 @@ static inline const char *am_path(const struct am_state *state, const char *path } /** + * If state->quiet is false, calls fprintf(fp, fmt, ...), and appends a newline + * at the end. + */ +static void say(const struct am_state *state, FILE *fp, const char *fmt, ...) +{ + va_list ap; + + va_start(ap, fmt); + if (!state->quiet) { + vfprintf(fp, fmt, ap); + putc('\n', fp); + } + va_end(ap); +} + +/** * Returns 1 if there is an am session in progress, 0 otherwise. */ static int am_in_progress(const struct am_state *state) @@ -262,6 +286,9 @@ static void am_load(struct am_state *state) read_state_file(&state->msg, am_path(state, "final-commit"), 0, 0); + read_state_file(&sb, am_path(state, "quiet"), 2, 1); + state->quiet = !strcmp(sb.buf, "t"); + strbuf_release(&sb); } @@ -445,6 +472,8 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, write_file(am_path(state, "last"), 1, "%d", state->last); + write_file(am_path(state, "quiet"), 1, state->quiet ? "t" : "f"); + if (!get_sha1("HEAD", curr_head)) { write_file(am_path(state, "abort-safety"), 1, "%s", sha1_to_hex(curr_head)); update_ref("am", "ORIG_HEAD", curr_head, NULL, 0, UPDATE_REFS_DIE_ON_ERR); @@ -654,7 +683,7 @@ static void do_commit(const struct am_state *state) commit_list_insert(lookup_commit(parent), &parents); } else { ptr = NULL; - fprintf_ln(stderr, _("applying to an empty history")); + say(state, stderr, _("applying to an empty history")); } author = fmt_ident(state->author_name.buf, state->author_email.buf, @@ -705,7 +734,7 @@ static void am_run(struct am_state *state) write_author_script(state); write_file(am_path(state, "final-commit"), 1, "%s", state->msg.buf); - printf_ln(_("Applying: %s"), firstline(state->msg.buf)); + say(state, stdout, _("Applying: %s"), firstline(state->msg.buf)); if (run_apply(state) < 0) { int value; @@ -736,7 +765,7 @@ next: */ static void am_resolve(struct am_state *state) { - printf_ln(_("Applying: %s"), firstline(state->msg.buf)); + say(state, stdout, _("Applying: %s"), firstline(state->msg.buf)); if (!index_has_changes(NULL)) { printf_ln(_("No changes - did you forget to use 'git add'?\n" @@ -959,6 +988,7 @@ static const char * const am_usage[] = { }; static struct option am_options[] = { + OPT__QUIET(&state.quiet, N_("be quiet")), OPT_CALLBACK(0, "patch-format", &opt_patch_format, N_("format"), N_("format the patch(es) are in"), parse_opt_patchformat), OPT_CMDMODE(0, "continue", &opt_resume, -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v3 12/31] am: implement --resolved/--continue
Since 0c15cc9 (git-am: --resolved., 2005-11-16), git-am supported resuming from a failed patch application. The user will manually apply the patch, and the run git am --resolved which will then commit the resulting index. Re-implement this feature by introducing am_resolve(). Since it makes no sense for the user to run am --resolved when there is no session in progress, we error out in this case. Signed-off-by: Paul Tan --- builtin/am.c | 55 ++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/builtin/am.c b/builtin/am.c index 9d6ab2a..4381164 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -707,6 +707,34 @@ next: } /** + * Resume the current am session after patch application failure. The user did + * all the hard work, and we do not have to do any patch application. Just + * trust and commit what the user has in the index and working tree. + */ +static void am_resolve(struct am_state *state) +{ + printf_ln(_("Applying: %s"), firstline(state->msg.buf)); + + if (!index_has_changes(NULL)) { + printf_ln(_("No changes - did you forget to use 'git add'?\n" + "If there is nothing left to stage, chances are that something else\n" + "already introduced the same changes; you might want to skip this patch.")); + exit(128); + } + + if (unmerged_cache()) { + printf_ln(_("You still have unmerged paths in your index.\n" + "Did you forget to use 'git add'?")); + exit(128); + } + + do_commit(state); + + am_next(state); + am_run(state); +} + +/** * parse_options() callback that validates and sets opt->value to the * PATCH_FORMAT_* enum value corresponding to `arg`. */ @@ -721,17 +749,30 @@ static int parse_opt_patchformat(const struct option *opt, const char *arg, int return 0; } +enum resume_mode { + RESUME_FALSE = 0, + RESUME_RESOLVED +}; + static struct am_state state; static int opt_patch_format; +static enum resume_mode opt_resume; static const char * const am_usage[] = { N_("git am [options] [(|)...]"), + N_("git am [options] --continue"), NULL }; static struct option am_options[] = { OPT_CALLBACK(0, "patch-format", &opt_patch_format, N_("format"), N_("format the patch(es) are in"), parse_opt_patchformat), + OPT_CMDMODE(0, "continue", &opt_resume, + N_("continue applying patches after resolving a conflict"), + RESUME_RESOLVED), + OPT_CMDMODE('r', "resolved", &opt_resume, + N_("synonyms for --continue"), + RESUME_RESOLVED), OPT_END() }; @@ -768,6 +809,9 @@ int cmd_am(int argc, const char **argv, const char *prefix) struct string_list paths = STRING_LIST_INIT_DUP; int i; + if (opt_resume) + die(_("Resolve operation not in progress, we are not resuming.")); + for (i = 0; i < argc; i++) { if (is_absolute_path(argv[i]) || !prefix) string_list_append(&paths, argv[i]); @@ -780,7 +824,16 @@ int cmd_am(int argc, const char **argv, const char *prefix) string_list_clear(&paths, 0); } - am_run(&state); + switch (opt_resume) { + case RESUME_FALSE: + am_run(&state); + break; + case RESUME_RESOLVED: + am_resolve(&state); + break; + default: + die("BUG: invalid resume value"); + } am_state_release(&state); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v3 22/31] am: don't use git-mailinfo if --rebasing
Since 5e835ca (rebase: do not munge commit log message, 2008-04-16), git am --rebasing no longer gets the commit log message from the patch, but reads it directly from the commit identified by the "From " header line. Since 43c2325 (am: use get_author_ident_from_commit instead of mailinfo when rebasing, 2010-06-16), git am --rebasing also gets the author name, email and date directly from the commit. Since 0fbb95d (am: don't call mailinfo if $rebasing, 2012-06-26), git am --rebasing does not use git-mailinfo to get the patch body, but rather generates it directly from the commit itself. The above 3 commits introduced a separate parse_patch() code path in git-am.sh's --rebasing mode that bypasses git-mailinfo. Re-implement this code path in builtin/am.c as parse_patch_rebase(). Signed-off-by: Paul Tan --- builtin/am.c | 155 ++- 1 file changed, 153 insertions(+), 2 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 9afa3bb..0d7e37c 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -21,6 +21,8 @@ #include "sequencer.h" #include "revision.h" #include "merge-recursive.h" +#include "revision.h" +#include "log-tree.h" /** * Returns 1 if the file is empty or does not exist, 0 otherwise. @@ -50,6 +52,30 @@ static const char *firstline(const char *msg) return sb.buf; } +/** + * Iterate over lines, each terminated by c, in a string, without modifying the + * string itself. begin will be set to the beginning of the line, while end + * will be set to the line termination or null character at the end of the line + * or string. On the first call, begin and end must be initialized to the + * string to be iterated over. E.g.: + * + * const char *begin, *end, *str = "1\n2\n3"; + * begin = end = str; + * while (!getbufline(&begin, &end, '\n')) + * ... + */ +static int getbufline(const char **begin, const char **end, int c) +{ + if (!**end) + return EOF; + else if (**end == c) + *begin = *end + 1; + else + *begin = *end; + *end = strchrnul(*begin, c); + return 0; +} + enum patch_format { PATCH_FORMAT_UNKNOWN = 0, PATCH_FORMAT_MBOX @@ -675,6 +701,126 @@ static int parse_patch(struct am_state *state, const char *patch) } /** + * Sets commit_id to the commit hash where the patch was generated from. + * Returns 0 on success, -1 on failure. + */ +static int get_patch_commit_sha1(unsigned char *commit_id, const char *patch) +{ + struct strbuf sb = STRBUF_INIT; + FILE *fp = xfopen(patch, "r"); + const char *x; + + if (strbuf_getline(&sb, fp, '\n')) + return -1; + + if (!skip_prefix(sb.buf, "From ", &x)) + return -1; + + if (get_sha1_hex(x, commit_id) < 0) + return -1; + + strbuf_release(&sb); + fclose(fp); + return 0; +} + +/** + * Sets state->msg, state->author_name, state->author_email, state->author_date + * to the commit's respective info. + */ +static void get_commit_info(struct am_state *state, struct commit *commit) +{ + const char *buf, *begin, *end; + + buf = logmsg_reencode(commit, NULL, get_commit_output_encoding()); + begin = end = buf; + + while (!getbufline(&begin, &end, '\n')) { + const char *x; + + if (skip_prefix(begin, "author ", &x)) { + struct ident_split split; + const char *date; + + if (split_ident_line(&split, x, end - x) < 0) { + struct strbuf sb = STRBUF_INIT; + + strbuf_add(&sb, x, end - x); + die(_("invalid ident line: %s"), sb.buf); + } + + if (split.name_begin) + strbuf_add(&state->author_name, + split.name_begin, + split.name_end - split.name_begin); + + if (split.mail_begin) + strbuf_add(&state->author_email, + split.mail_begin, + split.mail_end - split.mail_begin); + + date = show_ident_date(&split, DATE_NORMAL); + strbuf_addstr(&state->author_date, date); + } else if (begin == end) { + end++; + break; + } + } + + strbuf_addstr(&state->msg, end); +} + +/** + * Writes commit as a patch to $state_dir/patch. + */ +static void write_commit_patch(const struct
[PATCH/WIP v3 29/31] am: implement --ignore-date
Since a79ec62 (git-am: Add --ignore-date option, 2009-01-24), git-am.sh supported the --ignore-date option, and would use the current timestamp instead of the one provided in the patch if the option was set. Re-implement this option in builtin/am.c. Signed-off-by: Paul Tan --- builtin/am.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/builtin/am.c b/builtin/am.c index 3556765..6623b49 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -130,6 +130,8 @@ struct am_state { /* override error message when patch failure occurs */ const char *resolvemsg; + int ignore_date; + int rebasing; }; @@ -1131,7 +1133,8 @@ static void do_commit(const struct am_state *state) } author = fmt_ident(state->author_name.buf, state->author_email.buf, - state->author_date.buf, IDENT_STRICT); + state->ignore_date ? NULL : state->author_date.buf, + IDENT_STRICT); if (commit_tree(state->msg.buf, state->msg.len, tree, parents, commit, author, NULL)) @@ -1521,6 +1524,8 @@ static struct option am_options[] = { OPT_CMDMODE(0, "abort", &opt_resume, N_("restore the original branch and abort the patching operation."), RESUME_ABORT), + OPT_BOOL(0, "ignore-date", &state.ignore_date, + N_("use current timestamp for author date")), OPT_HIDDEN_BOOL(0, "rebasing", &state.rebasing, N_("(internal use for git-rebase)")), OPT_END() -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v3 25/31] am: implement --[no-]message-id, am.messageid
Since a078f73 (git-am: add --message-id/--no-message-id, 2014-11-25), git-am.sh supported the --[no-]message-id options, and the "am.messageid" setting which specifies the default option. --[no-]message-id tells git-am whether or not the -m option should be passed to git-mailinfo. Re-implement this option in builtin/am.c. Signed-off-by: Paul Tan --- Notes: No test for am.messageid builtin/am.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index b73549f..4cec380 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -113,6 +113,9 @@ struct am_state { /* one of the enum keep_type values */ int keep; + /* pass -m flag to git-mailinfo */ + int message_id; + /* override error message when patch failure occurs */ const char *resolvemsg; @@ -140,6 +143,8 @@ static void am_state_init(struct am_state *state) quiet = getenv("GIT_QUIET"); if (quiet && *quiet) state->quiet = 1; + + git_config_get_bool("am.messageid", &state->message_id); } /** @@ -350,6 +355,9 @@ static void am_load(struct am_state *state) else state->keep = KEEP_FALSE; + read_state_file(&sb, am_path(state, "messageid"), 2, 1); + state->message_id = !strcmp(sb.buf, "t"); + state->rebasing = !!file_exists(am_path(state, "rebasing")); strbuf_release(&sb); @@ -560,6 +568,8 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, } write_file(am_path(state, "keep"), 1, "%s", str); + write_file(am_path(state, "messageid"), 1, state->message_id ? "t" : "f"); + if (state->rebasing) write_file(am_path(state, "rebasing"), 1, "%s", ""); else @@ -700,6 +710,9 @@ static int parse_patch(struct am_state *state, const char *patch) die("BUG: invalid value for state->keep"); } + if (state->message_id) + argv_array_push(&cp.args, "-m"); + argv_array_push(&cp.args, am_path(state, "msg")); argv_array_push(&cp.args, am_path(state, "patch")); @@ -1377,6 +1390,8 @@ static struct option am_options[] = { N_("pass -k flag to git-mailinfo"), KEEP_TRUE), OPT_SET_INT(0, "keep-non-patch", &state.keep, N_("pass -b flag to git-mailinfo"), KEEP_NON_PATCH), + OPT_BOOL('m', "message-id", &state.message_id, + N_("pass -m flag to git-mailinfo")), OPT_CALLBACK(0, "patch-format", &opt_patch_format, N_("format"), N_("format the patch(es) are in"), parse_opt_patchformat), OPT_STRING(0, "resolvemsg", &state.resolvemsg, NULL, -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v3 13/31] am: implement --skip
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 4381164..0d7af19 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -16,6 +16,8 @@ #include "lockfile.h" #include "diff.h" #include "diffcore.h" +#include "unpack-trees.h" +#include "branch.h" /** * Returns 1 if the file is empty or does not exist, 0 otherwise. @@ -735,6 +737,114 @@ static void am_resolve(struct am_state *state) } /** + * Performs a checkout fast-forward from `head` to `remote`. If `reset` is + * true, any unmerged entries will be discarded. Returns 0 on success, -1 on + * failure. + */ +static int fast_forward_to(struct tree *head, struct tree *remote, int reset) +{ + struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file)); + struct unpack_trees_options opts; + struct tree_desc t[2]; + + if (parse_tree(head) || parse_tree(remote)) + return -1; + + hold_locked_index(lock_file, 1); + + refresh_cache(REFRESH_QUIET); + + memset(&opts, 0, sizeof(opts)); + opts.head_idx = 1; + opts.src_index = &the_index; + opts.dst_index = &the_index; + opts.update = 1; + opts.merge = 1; + opts.reset = reset; + opts.fn = twoway_merge; + init_tree_desc(&t[0], head->buffer, head->size); + init_tree_desc(&t[1], remote->buffer, remote->size); + + if (unpack_trees(2, t, &opts)) { + rollback_lock_file(lock_file); + return -1; + } + + if (write_locked_index(&the_index, lock_file, COMMIT_LOCK)) + die(_("unable to write new index file")); + + return 0; +} + +/** + * Clean the index without touching entries that are not modified between + * `head` and `remote`. + */ +static int clean_index(const unsigned char *head, const unsigned char *remote) +{ + struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file)); + struct tree *head_tree, *remote_tree, *index_tree; + unsigned char index[GIT_SHA1_RAWSZ]; + struct pathspec pathspec; + + head_tree = parse_tree_indirect(head); + if (!head_tree) + return error(_("Could not parse object '%s'."), sha1_to_hex(head)); + + remote_tree = parse_tree_indirect(remote); + if (!remote_tree) + return error(_("Could not parse object '%s'."), sha1_to_hex(remote)); + + read_cache_unmerged(); + + if (fast_forward_to(head_tree, head_tree, 1)) + return -1; + + if (write_cache_as_tree(index, 0, NULL)) + return -1; + + index_tree = parse_tree_indirect(index); + if (!index_tree) + return error(_("Could not parse object '%s'."), sha1_to_hex(index)); + + if (fast_forward_to(index_tree, remote_tree, 0)) + return -1; + + memset(&pathspec, 0, sizeof(pathspec)); + + hold_locked_index(lock_file, 1); + + if (read_tree(remote_tree, 0, &pathspec)) { + rollback_lock_file(lock_file); + return -1; + } + + if (write_locked_index(&the_index, lock_file, COMMIT_LOCK)) + die(_("unable to write new index file")); + + remove_branch_state(); + + return 0; +} + +/** + * Resume the current am session by skipping the current patch. + */ +static void am_skip(struct am_state *state) +{ + unsigned char head[GIT_SHA1_RAWSZ]; + + if (get_sha1("HEAD", head)) + hashcpy(head, EMPTY_TREE_SHA1_BIN); + + if (clean_index(head, head)) + die(_("failed to clean index")); + + am_next(state); + am_run(state); +} + +/** * parse_options() callback that validates and sets opt->value to the * PATCH_FORMAT_* enum value corresponding to `arg`. */ @@ -751,7 +861,8 @@ static int parse_opt_patchformat(const struct option *opt, const char *arg, int enum resume_mode { RESUME_FALSE = 0, - RESUME_RESOLVED + RESUME_RESOLVED, + RESUME_SKIP }; static struct am_state state; @@ -760,7 +871,7 @@ static enum resume_mode opt_resume; static const char * const am_usage[] = { N_("git am [options] [(|)...]"), - N_("git am [options] --continue"), + N_("git am [options] (--continue | --skip)"), NULL }; @@ -773,6 +884,9 @@ static struct option am_options[] = { OPT_CMDMODE('r', "resolved", &opt_resume, N
[PATCH/WIP v3 31/31] am: implement -S/--gpg-sign, commit.gpgsign
Since 3b4e395 (am: add the --gpg-sign option, 2014-02-01), git-am.sh supported the --gpg-sign option, and would pass it to git-commit-tree, thus GPG-signing the commit object. Re-implement this option in builtin/am.c. git-commit-tree would also sign the commit by default if the commit.gpgsign setting is true. Since we do not run commit-tree, we re-implement this behavior by handling the commit.gpgsign setting ourselves. Signed-off-by: Paul Tan --- builtin/am.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/builtin/am.c b/builtin/am.c index 608a2da..91dae92 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -134,6 +134,8 @@ struct am_state { int ignore_date; + const char *sign_commit; + int rebasing; }; @@ -143,6 +145,7 @@ struct am_state { static void am_state_init(struct am_state *state) { const char *quiet; + int sign_commit; memset(state, 0, sizeof(*state)); @@ -164,6 +167,9 @@ static void am_state_init(struct am_state *state) state->scissors = SCISSORS_UNSET; argv_array_init(&state->git_apply_opts); + + if (!git_config_get_bool("commit.gpgsign", &sign_commit)) + state->sign_commit = sign_commit ? "" : NULL; } /** @@ -1143,7 +1149,7 @@ static void do_commit(const struct am_state *state) state->ignore_date ? "" : state->author_date.buf, 1); if (commit_tree(state->msg.buf, state->msg.len, tree, parents, commit, - author, NULL)) + author, state->sign_commit)) die(_("failed to write commit object")); reflog_msg = getenv("GIT_REFLOG_ACTION"); @@ -1535,6 +1541,9 @@ static struct option am_options[] = { N_("lie about committer date")), OPT_BOOL(0, "ignore-date", &state.ignore_date, N_("use current timestamp for author date")), + { OPTION_STRING, 'S', "gpg-sign", &state.sign_commit, N_("key-id"), + N_("GPG-sign commits"), + PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, OPT_HIDDEN_BOOL(0, "rebasing", &state.rebasing, N_("(internal use for git-rebase)")), OPT_END() -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v3 26/31] am: support --keep-cr, am.keepcr
Since ad2c928 (git-am: Add command line parameter `--keep-cr` passing it to git-mailsplit, 2010-02-27), git-am.sh supported the --keep-cr option and would pass it to git-mailsplit. Since e80d4cb (git-am: Add am.keepcr and --no-keep-cr to override it, 2010-02-27), git-am.sh supported the am.keepcr config setting, which controls whether --keep-cr is on by default. Re-implement the above in builtin/am.c. Signed-off-by: Paul Tan --- builtin/am.c | 30 -- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 4cec380..1991f36 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -469,7 +469,8 @@ done: * Splits out individual patches from `paths`, where each path is either a mbox * file or a Maildir. Return 0 on success, -1 on failure. */ -static int split_patches_mbox(struct am_state *state, struct string_list *paths) +static int split_patches_mbox(struct am_state *state, struct string_list *paths, + int keep_cr) { struct child_process cp = CHILD_PROCESS_INIT; struct string_list_item *item; @@ -480,6 +481,8 @@ static int split_patches_mbox(struct am_state *state, struct string_list *paths) argv_array_pushf(&cp.args, "-d%d", state->prec); argv_array_pushf(&cp.args, "-o%s", state->dir.buf); argv_array_push(&cp.args, "-b"); + if (keep_cr) + argv_array_push(&cp.args, "--keep-cr"); argv_array_push(&cp.args, "--"); for_each_string_list_item(item, paths) @@ -501,14 +504,22 @@ static int split_patches_mbox(struct am_state *state, struct string_list *paths) * set to the index of the first patch, and state->last will be set to the * index of the last patch. * + * Set keep_cr to 0 to convert all lines ending with \r\n to end with \n, 1 + * to disable this behavior, -1 to use the default configured setting. + * * Returns 0 on success, -1 on failure. */ static int split_patches(struct am_state *state, enum patch_format patch_format, - struct string_list *paths) + struct string_list *paths, int keep_cr) { + if (keep_cr < 0) { + keep_cr = 0; + git_config_get_bool("am.keepcr", &keep_cr); + } + switch (patch_format) { case PATCH_FORMAT_MBOX: - return split_patches_mbox(state, paths); + return split_patches_mbox(state, paths, keep_cr); default: die("BUG: invalid patch_format"); } @@ -519,7 +530,7 @@ static int split_patches(struct am_state *state, enum patch_format patch_format, * Setup a new am session for applying patches */ static void am_setup(struct am_state *state, enum patch_format patch_format, - struct string_list *paths) + struct string_list *paths, int keep_cr) { unsigned char curr_head[GIT_SHA1_RAWSZ]; const char *str; @@ -535,7 +546,7 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, if (mkdir(state->dir.buf, 0777) < 0 && errno != EEXIST) die_errno(_("failed to create directory '%s'"), state->dir.buf); - if (split_patches(state, patch_format, paths) < 0) { + if (split_patches(state, patch_format, paths, keep_cr) < 0) { am_destroy(state); die(_("Failed to split patches.")); } @@ -1371,6 +1382,7 @@ enum resume_mode { }; static struct am_state state; +static int opt_keep_cr = -1; static int opt_patch_format; static enum resume_mode opt_resume; @@ -1392,6 +1404,12 @@ static struct option am_options[] = { N_("pass -b flag to git-mailinfo"), KEEP_NON_PATCH), OPT_BOOL('m', "message-id", &state.message_id, N_("pass -m flag to git-mailinfo")), + { OPTION_SET_INT, 0, "keep-cr", &opt_keep_cr, NULL, + N_("pass --keep-cr flag to git-mailsplit for mbox format"), + PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1}, + { OPTION_SET_INT, 0, "no-keep-cr", &opt_keep_cr, NULL, + N_("do not pass --keep-cr flag to git-mailsplit independent of am.keepcr"), + PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 0}, OPT_CALLBACK(0, "patch-format", &opt_patch_format, N_("format"), N_("format the patch(es) are in"), parse_opt_patchformat), OPT_STRING(0, "resolvemsg", &state.resolvemsg, NULL, @@ -1486,7 +1504,7 @@ int cmd_am(int argc, const char **argv, const char *prefix) string_list_append(&paths, mkpath("%s/%s", prefix, argv[i])); } - am_setup(&state, opt_patch_format, &paths
[PATCH/WIP v3 24/31] am: implement -k/--keep, --keep-non-patch
Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07), git-am.sh supported the -k/--keep option to pass the -k option to git-mailsplit. Since f7e5ea1 (am: learn passing -b to mailinfo, 2012-01-16), git-am.sh supported the --keep-non-patch option to pass the -b option to git-mailsplit. Re-implement these two options in builtin/am.c. Signed-off-by: Paul Tan --- builtin/am.c | 51 +++ 1 file changed, 51 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index bbef91f..b73549f 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -81,6 +81,12 @@ enum patch_format { PATCH_FORMAT_MBOX }; +enum keep_type { + KEEP_FALSE = 0, + KEEP_TRUE, /* pass -k flag to git-mailinfo */ + KEEP_NON_PATCH /* pass -b flag to git-mailinfo */ +}; + struct am_state { /* state directory path */ struct strbuf dir; @@ -104,6 +110,9 @@ struct am_state { int sign; + /* one of the enum keep_type values */ + int keep; + /* override error message when patch failure occurs */ const char *resolvemsg; @@ -333,6 +342,14 @@ static void am_load(struct am_state *state) read_state_file(&sb, am_path(state, "sign"), 2, 1); state->sign = !strcmp(sb.buf, "t"); + read_state_file(&sb, am_path(state, "keep"), 2, 1); + if (!strcmp(sb.buf, "t")) + state->keep = KEEP_TRUE; + else if (!strcmp(sb.buf, "b")) + state->keep = KEEP_NON_PATCH; + else + state->keep = KEEP_FALSE; + state->rebasing = !!file_exists(am_path(state, "rebasing")); strbuf_release(&sb); @@ -497,6 +514,7 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, struct string_list *paths) { unsigned char curr_head[GIT_SHA1_RAWSZ]; + const char *str; if (!patch_format) patch_format = detect_patch_format(paths); @@ -527,6 +545,21 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, write_file(am_path(state, "sign"), 1, state->sign ? "t" : "f"); + switch (state->keep) { + case KEEP_FALSE: + str = "f"; + break; + case KEEP_TRUE: + str = "t"; + break; + case KEEP_NON_PATCH: + str = "b"; + break; + default: + die("BUG: invalid value for state->keep"); + } + write_file(am_path(state, "keep"), 1, "%s", str); + if (state->rebasing) write_file(am_path(state, "rebasing"), 1, "%s", ""); else @@ -653,6 +686,20 @@ static int parse_patch(struct am_state *state, const char *patch) cp.out = xopen(am_path(state, "info"), O_WRONLY | O_CREAT, 0777); argv_array_push(&cp.args, "mailinfo"); + + switch (state->keep) { + case KEEP_FALSE: + break; + case KEEP_TRUE: + argv_array_push(&cp.args, "-k"); + break; + case KEEP_NON_PATCH: + argv_array_push(&cp.args, "-b"); + break; + default: + die("BUG: invalid value for state->keep"); + } + argv_array_push(&cp.args, am_path(state, "msg")); argv_array_push(&cp.args, am_path(state, "patch")); @@ -1326,6 +1373,10 @@ static struct option am_options[] = { OPT__QUIET(&state.quiet, N_("be quiet")), OPT_BOOL('s', "signoff", &state.sign, N_("add a Signed-off-by line to the commit message")), + OPT_SET_INT('k', "keep", &state.keep, + N_("pass -k flag to git-mailinfo"), KEEP_TRUE), + OPT_SET_INT(0, "keep-non-patch", &state.keep, + N_("pass -b flag to git-mailinfo"), KEEP_NON_PATCH), OPT_CALLBACK(0, "patch-format", &opt_patch_format, N_("format"), N_("format the patch(es) are in"), parse_opt_patchformat), OPT_STRING(0, "resolvemsg", &state.resolvemsg, NULL, -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v3 10/31] 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 dfb6f7e..a7efe85 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -13,6 +13,7 @@ #include "cache-tree.h" #include "refs.h" #include "commit.h" +#include "lockfile.h" /** * Returns 1 if the file is empty or does not exist, 0 otherwise. @@ -471,6 +472,20 @@ static const char *msgnum(const struct am_state *state) } /** + * Refresh and write index. + */ +static void refresh_and_write_cache(void) +{ + static struct lock_file lock_file; + + hold_locked_index(&lock_file, 1); + refresh_cache(REFRESH_QUIET); + if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK)) + die(_("unable to write index file")); + rollback_lock_file(&lock_file); +} + +/** * Parses `patch` using git-mailinfo. state->msg will be set to the patch * message. state->author_name, state->author_email, state->author_date will be * set to the patch author's name, email and date respectively. The patch's @@ -607,6 +622,8 @@ static void do_commit(const struct am_state *state) */ static void am_run(struct am_state *state) { + refresh_and_write_cache(); + while (state->cur <= state->last) { const char *patch = am_path(state, msgnum(state)); @@ -696,6 +713,9 @@ int cmd_am(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, am_options, am_usage, 0); + if (read_index_preload(&the_index, NULL) < 0) + die(_("failed to read the index")); + if (am_in_progress(&state)) am_load(&state); else { -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v3 19/31] 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 v3 20/31] am: implement 3-way merge
Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07), git-am.sh supported the --3way option, and if set, would attempt to do a 3-way merge if the initial patch application fails. Since 5d86861 (am -3: list the paths that needed 3-way fallback, 2012-03-28), in a 3-way merge git-am.sh would list the paths that needed 3-way fallback, so that the user can review them more carefully to spot mismerges. Re-implement the above in builtin/am.c. Signed-off-by: Paul Tan --- builtin/am.c | 147 +-- 1 file changed, 143 insertions(+), 4 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index d45dd41..e154c87 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -19,6 +19,8 @@ #include "unpack-trees.h" #include "branch.h" #include "sequencer.h" +#include "revision.h" +#include "merge-recursive.h" /** * Returns 1 if the file is empty or does not exist, 0 otherwise. @@ -70,6 +72,8 @@ struct am_state { /* number of digits in patch filename */ int prec; + int threeway; + int quiet; int sign; @@ -292,6 +296,9 @@ static void am_load(struct am_state *state) read_state_file(&state->msg, am_path(state, "final-commit"), 0, 0); + read_state_file(&sb, am_path(state, "threeway"), 2, 1); + state->threeway = !strcmp(sb.buf, "t"); + read_state_file(&sb, am_path(state, "quiet"), 2, 1); state->quiet = !strcmp(sb.buf, "t"); @@ -481,6 +488,8 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, write_file(am_path(state, "last"), 1, "%d", state->last); + write_file(am_path(state, "threeway"), 1, state->threeway ? "t" : "f"); + write_file(am_path(state, "quiet"), 1, state->quiet ? "t" : "f"); write_file(am_path(state, "sign"), 1, state->sign ? "t" : "f"); @@ -666,17 +675,33 @@ static void NORETURN die_user_resolve(const struct am_state *state) } /* - * Applies current patch with git-apply. Returns 0 on success, -1 otherwise. + * Applies current patch with git-apply. Returns 0 on success, -1 otherwise. If + * index_file is not NULL, the patch will be applied to that index. */ -static int run_apply(const struct am_state *state) +static int run_apply(const struct am_state *state, const char *index_file) { struct child_process cp = CHILD_PROCESS_INIT; cp.git_cmd = 1; + if (index_file) + argv_array_pushf(&cp.env_array, "GIT_INDEX_FILE=%s", index_file); + + /* +* If we are allowed to fall back on 3-way merge, don't give false +* errors during the initial attempt. +*/ + if (state->threeway && !index_file) { + cp.no_stdout = 1; + cp.no_stderr = 1; + } + argv_array_push(&cp.args, "apply"); - argv_array_push(&cp.args, "--index"); + if (index_file) + argv_array_push(&cp.args, "--cached"); + else + argv_array_push(&cp.args, "--index"); argv_array_push(&cp.args, am_path(state, "patch")); @@ -685,8 +710,100 @@ static int run_apply(const struct am_state *state) /* Reload index as git-apply will have modified it. */ discard_cache(); + read_cache_from(index_file ? index_file : get_index_file()); + + return 0; +} + +/** + * Builds a index that contains just the blobs needed for a 3way merge. + */ +static int build_fake_ancestor(const struct am_state *state, const char *index_file) +{ + struct child_process cp = CHILD_PROCESS_INIT; + + cp.git_cmd = 1; + argv_array_push(&cp.args, "apply"); + argv_array_pushf(&cp.args, "--build-fake-ancestor=%s", index_file); + argv_array_push(&cp.args, am_path(state, "patch")); + + if (run_command(&cp)) + return -1; + + return 0; +} + +/** + * Attempt a threeway merge, using index_path as the temporary index. + */ +static int fall_back_threeway(const struct am_state *state, const char *index_path) +{ + unsigned char orig_tree[GIT_SHA1_RAWSZ], his_tree[GIT_SHA1_RAWSZ], + our_tree[GIT_SHA1_RAWSZ]; + const unsigned char *bases[1] = {orig_tree}; + struct merge_options o; + struct commit *result; + + if (get_sha1("HEAD", our_tree) < 0) + hashcpy(our_tree, EMPTY_TREE_SHA1_BIN); + + if (build_fake_ancestor(state, index_path)) + return error("could not build fake ancestor"); + + discard_cache(); + read_cache_from(index_path); + + if (write
[PATCH/WIP v3 27/31] am: implement --[no-]scissors
Since 017678b (am/mailinfo: Disable scissors processing by default, 2009-08-26), git-am supported the --[no-]scissors option, passing it to git-mailinfo. Re-implement support for this option. Signed-off-by: Paul Tan --- Notes: There are tests for mailinfo --scissors, but not am --scissors, or mailinfo.scissors. builtin/am.c | 49 + 1 file changed, 49 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 1991f36..42efce1 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -87,6 +87,12 @@ enum keep_type { KEEP_NON_PATCH /* pass -b flag to git-mailinfo */ }; +enum scissors_type { + SCISSORS_UNSET = -1, + SCISSORS_TRUE, /* pass --scissors to git-mailinfo */ + SCISSORS_FALSE /* pass --no-scissors to git-mailinfo */ +}; + struct am_state { /* state directory path */ struct strbuf dir; @@ -116,6 +122,9 @@ struct am_state { /* pass -m flag to git-mailinfo */ int message_id; + /* one of the enum scissors_type values */ + int scissors; + /* override error message when patch failure occurs */ const char *resolvemsg; @@ -145,6 +154,8 @@ static void am_state_init(struct am_state *state) state->quiet = 1; git_config_get_bool("am.messageid", &state->message_id); + + state->scissors = SCISSORS_UNSET; } /** @@ -358,6 +369,14 @@ static void am_load(struct am_state *state) read_state_file(&sb, am_path(state, "messageid"), 2, 1); state->message_id = !strcmp(sb.buf, "t"); + read_state_file(&sb, am_path(state, "scissors"), 2, 1); + if (!strcmp(sb.buf, "t")) + state->scissors = SCISSORS_TRUE; + else if (!strcmp(sb.buf, "f")) + state->scissors = SCISSORS_FALSE; + else + state->scissors = SCISSORS_UNSET; + state->rebasing = !!file_exists(am_path(state, "rebasing")); strbuf_release(&sb); @@ -581,6 +600,21 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, write_file(am_path(state, "messageid"), 1, state->message_id ? "t" : "f"); + switch (state->scissors) { + case SCISSORS_UNSET: + str = ""; + break; + case SCISSORS_FALSE: + str = "f"; + break; + case SCISSORS_TRUE: + str = "t"; + break; + default: + die("BUG: invalid value for state->scissors"); + } + write_file(am_path(state, "scissors"), 1, "%s", str); + if (state->rebasing) write_file(am_path(state, "rebasing"), 1, "%s", ""); else @@ -724,6 +758,19 @@ static int parse_patch(struct am_state *state, const char *patch) if (state->message_id) argv_array_push(&cp.args, "-m"); + switch (state->scissors) { + case SCISSORS_UNSET: + break; + case SCISSORS_FALSE: + argv_array_push(&cp.args, "--no-scissors"); + break; + case SCISSORS_TRUE: + argv_array_push(&cp.args, "--scissors"); + break; + default: + die("BUG: invalid value for state->scissors"); + } + argv_array_push(&cp.args, am_path(state, "msg")); argv_array_push(&cp.args, am_path(state, "patch")); @@ -1410,6 +1457,8 @@ static struct option am_options[] = { { OPTION_SET_INT, 0, "no-keep-cr", &opt_keep_cr, NULL, N_("do not pass --keep-cr flag to git-mailsplit independent of am.keepcr"), PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 0}, + OPT_BOOL('c', "scissors", &state.scissors, + N_("strip everything before a scissors line")), OPT_CALLBACK(0, "patch-format", &opt_patch_format, N_("format"), N_("format the patch(es) are in"), parse_opt_patchformat), OPT_STRING(0, "resolvemsg", &state.resolvemsg, NULL, -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v3 14/31] am: implement --abort
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 0d7af19..7053b8f 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -423,6 +423,8 @@ static int split_patches(struct am_state *state, enum patch_format patch_format, static void am_setup(struct am_state *state, enum patch_format patch_format, struct string_list *paths) { + unsigned char curr_head[GIT_SHA1_RAWSZ]; + if (!patch_format) patch_format = detect_patch_format(paths); @@ -442,6 +444,14 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, write_file(am_path(state, "next"), 1, "%d", state->cur); write_file(am_path(state, "last"), 1, "%d", state->last); + + if (!get_sha1("HEAD", curr_head)) { + write_file(am_path(state, "abort-safety"), 1, "%s", sha1_to_hex(curr_head)); + update_ref("am", "ORIG_HEAD", curr_head, NULL, 0, UPDATE_REFS_DIE_ON_ERR); + } else { + write_file(am_path(state, "abort-safety"), 1, "%s", ""); + delete_ref("ORIG_HEAD", NULL, 0); + } } /** @@ -450,6 +460,8 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, */ static void am_next(struct am_state *state) { + unsigned char head[GIT_SHA1_RAWSZ]; + state->cur++; write_file(am_path(state, "next"), 1, "%d", state->cur); @@ -460,6 +472,11 @@ static void am_next(struct am_state *state) strbuf_reset(&state->msg); unlink(am_path(state, "final-commit")); + + if (!get_sha1("HEAD", head)) + write_file(am_path(state, "abort-safety"), 1, "%s", sha1_to_hex(head)); + else + write_file(am_path(state, "abort-safety"), 1, "%s", ""); } /** @@ -665,10 +682,14 @@ static void am_run(struct am_state *state) { struct strbuf sb = STRBUF_INIT; + unlink(am_path(state, "dirtyindex")); + refresh_and_write_cache(); - if (index_has_changes(&sb)) + if (index_has_changes(&sb)) { + write_file(am_path(state, "dirtyindex"), 1, "t"); die(_("Dirty index: cannot apply patches (dirty: %s)"), sb.buf); + } strbuf_release(&sb); @@ -844,6 +865,67 @@ static void am_skip(struct am_state *state) am_run(state); } +static int safe_to_abort(const struct am_state *state) +{ + struct strbuf sb = STRBUF_INIT; + unsigned char abort_safety[GIT_SHA1_RAWSZ], head[GIT_SHA1_RAWSZ]; + + if (file_exists(am_path(state, "dirtyindex"))) + return 0; + + if (read_state_file(&sb, am_path(state, "abort-safety"), 40, 1) > 0) { + if (get_sha1_hex(sb.buf, abort_safety)) + die(_("could not parse %s"), am_path(state, "abort_safety")); + } else + hashclr(abort_safety); + + if (get_sha1("HEAD", head)) + hashclr(head); + + if (!hashcmp(head, abort_safety)) + return 1; + + error(_("You seem to have moved HEAD since the last 'am' failure.\n" + "Not rewinding to ORIG_HEAD")); + + return 0; +} + +/** + * Aborts the current am session if it is safe to do so. + */ +static void am_abort(struct am_state *state) +{ + unsigned char curr_head[GIT_SHA1_RAWSZ], orig_head[GIT_SHA1_RAWSZ]; + int has_curr_head, has_orig_head; + const char *curr_branch; + + if (!safe_to_abort(state)) { + am_destroy(state); + return; + } + + curr_branch = resolve_refdup("HEAD", 0, curr_head, NULL); + has_curr_head = !is_null_sha1(curr_head); + if (!has_curr_head) + hashcpy(curr_head, EMPTY_TREE_SHA1_BIN); + + has_orig_head = !get_sha1("ORIG_HEAD", orig_head); + if (!has_orig_head) + hashcpy(orig_head, EMPTY_TREE_SHA1_BIN); + + clean_index(curr_head, orig_head); + + if (has_orig_head) + u
[PATCH/WIP v3 15/31] am: don't accept patches when there's a session in progress
Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07), git-am would error out if the user gave it mbox(s) on the command-line, but there was a session in progress. Since c95b138 (Fix git-am safety checks, 2006-09-15), git-am would detect if the user attempted to feed it a mbox via stdin, by checking if stdin is not a tty and there is no resume command given. Re-implement the above two safety checks. Signed-off-by: Paul Tan --- Notes: NOTE: there's no test for this builtin/am.c | 19 +-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 7053b8f..4adc487 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1003,9 +1003,24 @@ int cmd_am(int argc, const char **argv, const char *prefix) if (read_index_preload(&the_index, NULL) < 0) die(_("failed to read the index")); - if (am_in_progress(&state)) + if (am_in_progress(&state)) { + /* +* Catch user error to feed us patches when there is a session +* in progress: +* +* 1. mbox path(s) are provided on the command-line. +* 2. stdin is not a tty: the user is trying to feed us a patch +*from standard input. This is somewhat unreliable -- stdin +*could be /dev/null for example and the caller did not +*intend to feed us a patch but wanted to continue +*unattended. +*/ + if (argc || (!opt_resume && !isatty(0))) + die(_("previous rebase directory %s still exists but mbox given."), + state.dir.buf); + am_load(&state); - else { + } else { struct string_list paths = STRING_LIST_INIT_DUP; int i; -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v3 21/31] am: --rebasing
Since 3041c32 (am: --rebasing, 2008-03-04), git-am.sh supported the --rebasing option, which is used internally by git-rebase to tell git-am that it is being used for its purpose. It would create the empty file $state_dir/rebasing to help "completion" scripts tell if the ongoing operation is am or rebase. As of 0fbb95d (am: don't call mailinfo if $rebasing, 2012-06-26), --rebasing also implies --3way as well. Since a1549e1 (am: return control to caller, for housekeeping, 2013-05-12), git-am.sh would only clean up the state directory when it is not --rebasing, instead deferring cleanup to git-rebase.sh. Re-implement the above in builtin/am.c. Signed-off-by: Paul Tan --- builtin/am.c | 24 +--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index e154c87..9afa3bb 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -80,6 +80,8 @@ struct am_state { /* override error message when patch failure occurs */ const char *resolvemsg; + + int rebasing; }; /** @@ -305,6 +307,8 @@ static void am_load(struct am_state *state) read_state_file(&sb, am_path(state, "sign"), 2, 1); state->sign = !strcmp(sb.buf, "t"); + state->rebasing = !!file_exists(am_path(state, "rebasing")); + strbuf_release(&sb); } @@ -484,6 +488,9 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, die(_("Failed to split patches.")); } + if (state->rebasing) + state->threeway = 1; + write_file(am_path(state, "next"), 1, "%d", state->cur); write_file(am_path(state, "last"), 1, "%d", state->last); @@ -494,12 +501,20 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, write_file(am_path(state, "sign"), 1, state->sign ? "t" : "f"); + if (state->rebasing) + write_file(am_path(state, "rebasing"), 1, "%s", ""); + else + write_file(am_path(state, "applying"), 1, "%s", ""); + if (!get_sha1("HEAD", curr_head)) { write_file(am_path(state, "abort-safety"), 1, "%s", sha1_to_hex(curr_head)); - update_ref("am", "ORIG_HEAD", curr_head, NULL, 0, UPDATE_REFS_DIE_ON_ERR); + if (!state->rebasing) + update_ref("am", "ORIG_HEAD", curr_head, NULL, 0, + UPDATE_REFS_DIE_ON_ERR); } else { write_file(am_path(state, "abort-safety"), 1, "%s", ""); - delete_ref("ORIG_HEAD", NULL, 0); + if (!state->rebasing) + delete_ref("ORIG_HEAD", NULL, 0); } } @@ -921,7 +936,8 @@ next: am_next(state); } - am_destroy(state); + if (!state->rebasing) + am_destroy(state); } /** @@ -1175,6 +1191,8 @@ static struct option am_options[] = { OPT_CMDMODE(0, "abort", &opt_resume, N_("restore the original branch and abort the patching operation."), RESUME_ABORT), + OPT_HIDDEN_BOOL(0, "rebasing", &state.rebasing, + N_("(internal use for git-rebase)")), OPT_END() }; -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v3 30/31] am: implement --committer-date-is-author-date
Since 3f01ad6 (am: Add --committer-date-is-author-date option, 2009-01-22), git-am.sh implemented the --committer-date-is-author-date option, which tells git-am to use the timestamp recorded in the email message as both author and committer date. Re-implement this option in builtin/am.c. Signed-off-by: Paul Tan --- builtin/am.c | 9 + 1 file changed, 9 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 6623b49..608a2da 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -130,6 +130,8 @@ struct am_state { /* override error message when patch failure occurs */ const char *resolvemsg; + int committer_date_is_author_date; + int ignore_date; int rebasing; @@ -1136,6 +1138,10 @@ static void do_commit(const struct am_state *state) state->ignore_date ? NULL : state->author_date.buf, IDENT_STRICT); + if (state->committer_date_is_author_date) + setenv("GIT_COMMITTER_DATE", + state->ignore_date ? "" : state->author_date.buf, 1); + if (commit_tree(state->msg.buf, state->msg.len, tree, parents, commit, author, NULL)) die(_("failed to write commit object")); @@ -1524,6 +1530,9 @@ static struct option am_options[] = { OPT_CMDMODE(0, "abort", &opt_resume, N_("restore the original branch and abort the patching operation."), RESUME_ABORT), + OPT_BOOL(0, "committer-date-is-author-date", + &state.committer_date_is_author_date, + N_("lie about committer date")), OPT_BOOL(0, "ignore-date", &state.ignore_date, N_("use current timestamp for author date")), OPT_HIDDEN_BOOL(0, "rebasing", &state.rebasing, -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v3 28/31] am: pass git-apply's options to git-apply
git-am.sh recognizes some of git-apply's options, and would pass them to git-apply: * --whitespace, since 8c31cb8 (git-am: --whitespace=x option., 2006-02-28) * -C, since 67dad68 (add -C[NUM] to git-am, 2007-02-08) * -p, since 2092a1f (Teach git-am to pass -p option down to git-apply, 2007-02-11) * --directory, since b47dfe9 (git-am: add --directory= option, 2009-01-11) * --reject, since b80da42 (git-am: implement --reject option passed to git-apply, 2009-01-23) * --ignore-space-change, --ignore-whitespace, since 86c91f9 (git apply: option to ignore whitespace differences, 2009-08-04) * --exclude, since 77e9e49 (am: pass exclude down to apply, 2011-08-03) * --include, since 58725ef (am: support --include option, 2012-03-28) Re-implement support for these options in builtin/am.c. Signed-off-by: Paul Tan --- builtin/am.c | 46 ++ 1 file changed, 46 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 42efce1..3556765 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -125,6 +125,8 @@ struct am_state { /* one of the enum scissors_type values */ int scissors; + struct argv_array git_apply_opts; + /* override error message when patch failure occurs */ const char *resolvemsg; @@ -156,6 +158,8 @@ static void am_state_init(struct am_state *state) git_config_get_bool("am.messageid", &state->message_id); state->scissors = SCISSORS_UNSET; + + argv_array_init(&state->git_apply_opts); } /** @@ -168,6 +172,8 @@ static void am_state_release(struct am_state *state) strbuf_release(&state->author_email); strbuf_release(&state->author_date); strbuf_release(&state->msg); + + argv_array_clear(&state->git_apply_opts); } /** @@ -377,6 +383,11 @@ static void am_load(struct am_state *state) else state->scissors = SCISSORS_UNSET; + read_state_file(&sb, am_path(state, "apply-opt"), 128, 1); + argv_array_clear(&state->git_apply_opts); + if (sq_dequote_to_argv_array(sb.buf, &state->git_apply_opts) < 0) + die(_("could not parse %s"), am_path(state, "apply-opt")); + state->rebasing = !!file_exists(am_path(state, "rebasing")); strbuf_release(&sb); @@ -553,6 +564,7 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, { unsigned char curr_head[GIT_SHA1_RAWSZ]; const char *str; + struct strbuf sb = STRBUF_INIT; if (!patch_format) patch_format = detect_patch_format(paths); @@ -615,6 +627,10 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, } write_file(am_path(state, "scissors"), 1, "%s", str); + sq_quote_argv(&sb, state->git_apply_opts.argv, 0); + write_file(am_path(state, "apply-opt"), 1, "%s", sb.buf); + strbuf_release(&sb); + if (state->rebasing) write_file(am_path(state, "rebasing"), 1, "%s", ""); else @@ -977,6 +993,8 @@ static int run_apply(const struct am_state *state, const char *index_file) argv_array_push(&cp.args, "apply"); + argv_array_pushv(&cp.args, state->git_apply_opts.argv); + if (index_file) argv_array_push(&cp.args, "--cached"); else @@ -1003,6 +1021,7 @@ static int build_fake_ancestor(const struct am_state *state, const char *index_f cp.git_cmd = 1; argv_array_push(&cp.args, "apply"); + argv_array_pushv(&cp.args, state->git_apply_opts.argv); argv_array_pushf(&cp.args, "--build-fake-ancestor=%s", index_file); argv_array_push(&cp.args, am_path(state, "patch")); @@ -1459,8 +1478,35 @@ static struct option am_options[] = { PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 0}, OPT_BOOL('c', "scissors", &state.scissors, N_("strip everything before a scissors line")), + OPT_PASSTHRU_ARGV(0, "whitespace", &state.git_apply_opts, N_("action"), + N_("pass it through git-apply"), + 0), + OPT_PASSTHRU_ARGV(0, "ignore-space-change", &state.git_apply_opts, NULL, + N_("pass it through git-apply"), + PARSE_OPT_NOARG), + OPT_PASSTHRU_ARGV(0, "ignore-whitespace", &state.git_apply_opts, NULL, + N_("pass it through git-apply"), + PARSE_OPT_NOARG), + OPT_PASSTHRU_ARGV(0, "directory", &state.git_apply_opts, N_("root"), + N_("pass it
[PATCH/WIP v3 07/31] 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: v3 * Style fixes builtin/am.c | 232 +++ 1 file changed, 232 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 7b97ea8..d6434e4 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -9,6 +9,23 @@ #include "parse-options.h" #include "dir.h" #include "run-command.h" +#include "quote.h" + +/** + * Returns 1 if the file is empty or does not exist, 0 otherwise. + */ +static int is_empty_file(const char *filename) +{ + struct stat st; + + if (stat(filename, &st) < 0) { + if (errno == ENOENT) + return 1; + die_errno(_("could not stat %s"), filename); + } + + return !st.st_size; +} enum patch_format { PATCH_FORMAT_UNKNOWN = 0, @@ -23,6 +40,12 @@ struct am_state { int cur; int last; + /* commit message and metadata */ + struct strbuf author_name; + struct strbuf author_email; + struct strbuf author_date; + struct strbuf msg; + /* number of digits in patch filename */ int prec; }; @@ -36,6 +59,11 @@ static void am_state_init(struct am_state *state) strbuf_init(&state->dir, 0); + strbuf_init(&state->author_name, 0); + strbuf_init(&state->author_email, 0); + strbuf_init(&state->author_date, 0); + strbuf_init(&state->msg, 0); + state->prec = 4; } @@ -45,6 +73,10 @@ static void am_state_init(struct am_state *state) static void am_state_release(struct am_state *state) { strbuf_release(&state->dir); + strbuf_release(&state->author_name); + strbuf_release(&state->author_email); + strbuf_release(&state->author_date); + strbuf_release(&state->msg); } /** @@ -94,6 +126,105 @@ static int read_state_file(struct strbuf *sb, const char *file, size_t hint, int } /** + * Reads a KEY=VALUE shell variable assignment from fp, and returns the VALUE + * in `value`. VALUE must be a quoted string, and the KEY must match `key`. + * Returns 0 on success, -1 on failure. + * + * This is used by read_author_script() to read the GIT_AUTHOR_* variables from + * the author-script. + */ +static int read_shell_var(struct strbuf *value, FILE *fp, const char *key) +{ + struct strbuf sb = STRBUF_INIT; + char *str; + + if (strbuf_getline(&sb, fp, '\n')) + return -1; + + if (!skip_prefix(sb.buf, key, (const char **)&str)) + return -1; + + if (!skip_prefix(str, "=", (const char **)&str)) + return -1; + + str = sq_dequote(str); + if (!str) + return -1; + + strbuf_reset(value); + strbuf_addstr(value, str); + + strbuf_release(&sb); + + return 0; +} + +/** + * Parses the "author script" `filename`, and sets state->author_name, + * state->author_email and state->author_date accordingly. We are strict with + * our parsing, as the author script is supposed to be eval'd, and loosely + * parsing it may not give the results the user expects. + * + * The author script is of the format: + * + * GIT_AUTHOR_NAME='$author_name' + * GIT_AUTHOR_EMAIL='$author_email' + * GIT_AUTHOR_DATE='$author_date' + * + * where $author_name, $author_email and $author_date are quoted. + */ +static int read_author_script(struct am_state *state) +{ + const char *filename = am_path(state, "author-script"); + FILE *fp = fopen(filename, "r"); + if (!fp) { + if (errno == ENOENT) + return 0; + die_errno(_("could not open '%s' for reading"), filename); + } + + if (read_shell_var(&state->author_name, fp, "GIT_AUTHOR_NAME")) + return -1; + + if (read_shell_var(&state->author_email, fp, "GIT_AUTHOR_EMAIL")) + return -1; + + if (read_shell_var(&state->author_date, fp, "GIT_AUTHOR_DATE")) + return -1; + + if (fgetc(fp) != EOF) + return -1; + + fclose(fp); + return 0; +} + +/** + * Saves state->author_name, state->author_email and state->author_date in + * `filename` as an "author script", which is the format used by git-am.sh. + */ +static void write_
[PATCH/WIP v3 00/31] Make git-am a builtin
This patch series depends on pt/pull-builtin for OPT_PASSTHRU_ARGV() and argv_array_pushv(). This is a re-roll of [WIP v3]. Thanks Junio and Stefan for the reviews last round. The biggest addition this round would be the support for git-rebase. Here's a small unscientific benchmark that rebases 50 patches: git init && echo initial >file && git add file && git commit -m initial && git tag initial && for x in $(seq 50) do echo $x >>file && git commit -a -m $x done && git checkout -b onto-rebase initial && git commit --allow-empty -mempty && time git rebase -q --onto onto-rebase initial master With master: 1.53s, 1.55s, 1.17s, 1.52s, 1.22s. Avg: ~1.40s With master + this patch series: 0.22s, 0.22s, 0.18s, 0.21s, 0.18s. Avg: ~0.20s So this is around a 6-7x speedup. Previous versions: [WIP v1] http://thread.gmane.org/gmane.comp.version-control.git/270048 [WIP v2] http://thread.gmane.org/gmane.comp.version-control.git/271381 git-am is a commonly used command for applying a series of patches from a mailbox to the current branch. Currently, it is implemented by the shell script git-am.sh. However, compared to C, shell scripts have certain deficiencies: they need to spawn a lot of processes, introduce a lot of dependencies and cannot take advantage of git's internal caches. This WIP patch series rewrites git-am.sh into optimized C builtin/am.c, and is part of my GSoC project to rewrite git-pull and git-am into C builtins[1]. [1] https://gist.github.com/pyokagan/1b7b0d1f4dab6ba3cef1 Paul Tan (31): wrapper: implement xopen() wrapper: implement xfopen() am: implement skeletal builtin am am: implement patch queue mechanism am: split out mbox/maildir patches with git-mailsplit am: detect mbox patches am: extract patch, message and authorship with git-mailinfo am: apply patch with git-apply am: commit applied patch am: refresh the index at start am: refuse to apply patches if index is dirty am: implement --resolved/--continue am: implement --skip am: implement --abort am: don't accept patches when there's a session in progress am: implement quiet option am: exit with user friendly message on patch failure am: implement am --signoff cache-tree: introduce write_index_as_tree() am: implement 3-way merge am: --rebasing am: don't use git-mailinfo if --rebasing am: handle stray state directory am: implement -k/--keep, --keep-non-patch am: implement --[no-]message-id, am.messageid am: support --keep-cr, am.keepcr am: implement --[no-]scissors am: pass git-apply's options to git-apply am: implement --ignore-date am: implement --committer-date-is-author-date am: implement -S/--gpg-sign, commit.gpgsign Makefile |1 + builtin.h |1 + builtin/am.c | 1650 + cache-tree.c | 29 +- cache-tree.h |1 + git-compat-util.h |2 + git.c |1 + wrapper.c | 43 ++ 8 files changed, 1716 insertions(+), 12 deletions(-) create mode 100644 builtin/am.c -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v3 09/31] 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 | 51 +++ 1 file changed, 47 insertions(+), 4 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 296a5fc..dfb6f7e 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -10,6 +10,9 @@ #include "dir.h" #include "run-command.h" #include "quote.h" +#include "cache-tree.h" +#include "refs.h" +#include "commit.h" /** * Returns 1 if the file is empty or does not exist, 0 otherwise. @@ -557,6 +560,49 @@ static int run_apply(const struct am_state *state) } /** + * Commits the current index with state->msg as the commit message and + * state->author_name, state->author_email and state->author_date as the author + * information. + */ +static void do_commit(const struct am_state *state) +{ + unsigned char tree[GIT_SHA1_RAWSZ], parent[GIT_SHA1_RAWSZ], + commit[GIT_SHA1_RAWSZ]; + unsigned char *ptr; + struct commit_list *parents = NULL; + const char *reflog_msg, *author; + struct strbuf sb = STRBUF_INIT; + + if (write_cache_as_tree(tree, 0, NULL)) + die(_("git write-tree failed to write a tree")); + + if (!get_sha1_commit("HEAD", parent)) { + ptr = parent; + commit_list_insert(lookup_commit(parent), &parents); + } else { + ptr = NULL; + fprintf_ln(stderr, _("applying to an empty history")); + } + + author = fmt_ident(state->author_name.buf, state->author_email.buf, + state->author_date.buf, IDENT_STRICT); + + if (commit_tree(state->msg.buf, state->msg.len, tree, parents, commit, + author, NULL)) + die(_("failed to write commit object")); + + reflog_msg = getenv("GIT_REFLOG_ACTION"); + if (!reflog_msg) + reflog_msg = "am"; + + strbuf_addf(&sb, "%s: %s", reflog_msg, firstline(state->msg.buf)); + + update_ref(sb.buf, "HEAD", commit, ptr, 0, UPDATE_REFS_DIE_ON_ERR); + + strbuf_release(&sb); +} + +/** * Applies all queued patches. */ static void am_run(struct am_state *state) @@ -588,10 +634,7 @@ static void am_run(struct am_state *state) exit(128); } - /* -* TODO: After the patch has been applied to the index with -* git-apply, we need to make commit as well. -*/ + do_commit(state); next: am_next(state); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v3 08/31] am: apply patch with git-apply
Implement applying the patch to the index using git-apply. Signed-off-by: Paul Tan --- builtin/am.c | 57 - 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/builtin/am.c b/builtin/am.c index d6434e4..296a5fc 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -27,6 +27,18 @@ static int is_empty_file(const char *filename) return !st.st_size; } +/** + * Returns the first line of msg + */ +static const char *firstline(const char *msg) +{ + static struct strbuf sb = STRBUF_INIT; + + strbuf_reset(&sb); + strbuf_add(&sb, msg, strchrnul(msg, '\n') - msg); + return sb.buf; +} + enum patch_format { PATCH_FORMAT_UNKNOWN = 0, PATCH_FORMAT_MBOX @@ -519,6 +531,31 @@ static int parse_patch(struct am_state *state, const char *patch) return 0; } +/* + * Applies current patch with git-apply. Returns 0 on success, -1 otherwise. + */ +static int run_apply(const struct am_state *state) +{ + struct child_process cp = CHILD_PROCESS_INIT; + + cp.git_cmd = 1; + + argv_array_push(&cp.args, "apply"); + + argv_array_push(&cp.args, "--index"); + + argv_array_push(&cp.args, am_path(state, "patch")); + + if (run_command(&cp)) + return -1; + + /* Reload index as git-apply will have modified it. */ + discard_cache(); + read_cache(); + + return 0; +} + /** * Applies all queued patches. */ @@ -536,7 +573,25 @@ static void am_run(struct am_state *state) write_author_script(state); write_file(am_path(state, "final-commit"), 1, "%s", state->msg.buf); - /* TODO: Patch application not implemented yet */ + printf_ln(_("Applying: %s"), firstline(state->msg.buf)); + + if (run_apply(state) < 0) { + int value; + + printf_ln(_("Patch failed at %s %s"), msgnum(state), + firstline(state->msg.buf)); + + if (!git_config_get_bool("advice.amworkdir", &value) && !value) + printf_ln(_("The copy of the patch that failed is found in: %s"), + am_path(state, "patch")); + + exit(128); + } + + /* +* TODO: After the patch has been applied to the index with +* git-apply, we need to make commit as well. +*/ next: am_next(state); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v3 02/31] wrapper: implement xfopen()
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 --- git-compat-util.h | 1 + wrapper.c | 18 ++ 2 files changed, 19 insertions(+) diff --git a/git-compat-util.h b/git-compat-util.h index bc77d77..4e69110 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -724,6 +724,7 @@ extern ssize_t xread(int fd, void *buf, size_t len); extern ssize_t xwrite(int fd, const void *buf, size_t len); extern ssize_t xpread(int fd, void *buf, size_t len, off_t offset); extern int xdup(int fd); +extern FILE *xfopen(const char *path, const char *mode); extern FILE *xfdopen(int fd, const char *mode); extern int xmkstemp(char *template); extern int xmkstemp_mode(char *template, int mode); diff --git a/wrapper.c b/wrapper.c index 82658b3..9692460 100644 --- a/wrapper.c +++ b/wrapper.c @@ -336,6 +336,24 @@ int xdup(int fd) return ret; } +/** + * xfopen() is the same as fopen(), but it die()s if the fopen() fails. + */ +FILE *xfopen(const char *path, const char *mode) +{ + assert(path); + assert(mode); + + for (;;) { + FILE *fp = fopen(path, mode); + if (fp) + return fp; + if (errno == EINTR) + continue; + die_errno(_("could not open '%s'"), path); + } +} + FILE *xfdopen(int fd, const char *mode) { FILE *stream = fdopen(fd, mode); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v3 03/31] am: implement skeletal builtin am
For the purpose of rewriting git-am.sh into a C builtin, implement a skeletal builtin/am.c that redirects to $GIT_EXEC_PATH/git-am if the environment variable _GIT_USE_BUILTIN_AM is not defined. Since in the Makefile git-am.sh takes precedence over builtin/am.c, $GIT_EXEC_PATH/git-am will contain the shell script git-am.sh, and thus this allows us to fall back on the functional git-am.sh when running the test suite for tests that depend on a working git-am implementation. Since git-am.sh cannot handle any environment modifications by setup_git_directory(), "am" has to be declared as NO_SETUP in git.c. On the other hand, to re-implement git-am.sh in builtin/am.c, we do need to run all the git dir and work tree setup logic that git.c does for us. As such, we work around this temporarily by copying the logic in git.c's run_builtin(), which amounts to: prefix = setup_git_directory(); trace_repo_setup(prefix); setup_work_tree(); This redirection should be removed when all the features of git-am.sh have been re-implemented in builtin/am.c. Helped-by: Junio C Hamano Signed-off-by: Paul Tan --- Notes: v3 * Style fixes * git-am.sh cannot handle the chdir() and GIT_DIR envionment variable that setup_git_directory() sets, so we work around it by copying the logic of git.c's run_builtin(), and running it only when we are using the builtin am. Makefile | 1 + builtin.h| 1 + builtin/am.c | 28 git.c| 1 + 4 files changed, 31 insertions(+) create mode 100644 builtin/am.c diff --git a/Makefile b/Makefile index 93e4fa2..ff9bdc0 100644 --- a/Makefile +++ b/Makefile @@ -811,6 +811,7 @@ LIB_OBJS += xdiff-interface.o LIB_OBJS += zlib.o BUILTIN_OBJS += builtin/add.o +BUILTIN_OBJS += builtin/am.o BUILTIN_OBJS += builtin/annotate.o BUILTIN_OBJS += builtin/apply.o BUILTIN_OBJS += builtin/archive.o diff --git a/builtin.h b/builtin.h index ea3c834..f30cf00 100644 --- a/builtin.h +++ b/builtin.h @@ -30,6 +30,7 @@ extern int textconv_object(const char *path, unsigned mode, const unsigned char extern int is_builtin(const char *s); extern int cmd_add(int argc, const char **argv, const char *prefix); +extern int cmd_am(int argc, const char **argv, const char *prefix); extern int cmd_annotate(int argc, const char **argv, const char *prefix); extern int cmd_apply(int argc, const char **argv, const char *prefix); extern int cmd_archive(int argc, const char **argv, const char *prefix); diff --git a/builtin/am.c b/builtin/am.c new file mode 100644 index 000..dbc8836 --- /dev/null +++ b/builtin/am.c @@ -0,0 +1,28 @@ +/* + * Builtin "git am" + * + * Based on git-am.sh by Junio C Hamano. + */ +#include "cache.h" +#include "builtin.h" +#include "exec_cmd.h" + +int cmd_am(int argc, const char **argv, const char *prefix) +{ + /* +* FIXME: Once all the features of git-am.sh have been re-implemented +* in builtin/am.c, this preamble can be removed. +*/ + if (!getenv("_GIT_USE_BUILTIN_AM")) { + const char *path = mkpath("%s/git-am", git_exec_path()); + + if (sane_execvp(path, (char **)argv) < 0) + die_errno("could not exec %s", path); + } else { + prefix = setup_git_directory(); + trace_repo_setup(prefix); + setup_work_tree(); + } + + return 0; +} diff --git a/git.c b/git.c index e7a7713..a671535 100644 --- a/git.c +++ b/git.c @@ -370,6 +370,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv) static struct cmd_struct commands[] = { { "add", cmd_add, RUN_SETUP | NEED_WORK_TREE }, + { "am", cmd_am, NO_SETUP }, { "annotate", cmd_annotate, RUN_SETUP }, { "apply", cmd_apply, RUN_SETUP_GENTLY }, { "archive", cmd_archive }, -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v3 01/31] wrapper: implement xopen()
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 --- git-compat-util.h | 1 + wrapper.c | 25 + 2 files changed, 26 insertions(+) diff --git a/git-compat-util.h b/git-compat-util.h index 0cc7ae8..bc77d77 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -719,6 +719,7 @@ extern void *xrealloc(void *ptr, size_t size); extern void *xcalloc(size_t nmemb, size_t size); extern void *xmmap(void *start, size_t length, int prot, int flags, int fd, off_t offset); extern void *xmmap_gently(void *start, size_t length, int prot, int flags, int fd, off_t offset); +extern int xopen(const char *path, int flags, ...); extern ssize_t xread(int fd, void *buf, size_t len); extern ssize_t xwrite(int fd, const void *buf, size_t len); extern ssize_t xpread(int fd, void *buf, size_t len, off_t offset); diff --git a/wrapper.c b/wrapper.c index c1a663f..82658b3 100644 --- a/wrapper.c +++ b/wrapper.c @@ -189,6 +189,31 @@ void *xcalloc(size_t nmemb, size_t size) # endif #endif +/** + * xopen() is the same as open(), but it die()s if the open() fails. + */ +int xopen(const char *path, int oflag, ...) +{ + mode_t mode = 0; + va_list ap; + + va_start(ap, oflag); + if (oflag & O_CREAT) + mode = va_arg(ap, mode_t); + va_end(ap); + + assert(path); + + for (;;) { + int fd = open(path, oflag, mode); + if (fd >= 0) + return fd; + if (errno == EINTR) + continue; + die_errno(_("could not open '%s'"), path); + } +} + /* * xread() is the same a read(), but it automatically restarts read() * operations with a recoverable error (EAGAIN and EINTR). xread() -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v3 05/31] am: split out mbox/maildir patches with git-mailsplit
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: v3 * Moved the TODO comment to the previous patch builtin/am.c | 104 +-- 1 file changed, 101 insertions(+), 3 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index af68c51..e9a3687 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -8,6 +8,12 @@ #include "exec_cmd.h" #include "parse-options.h" #include "dir.h" +#include "run-command.h" + +enum patch_format { + PATCH_FORMAT_UNKNOWN = 0, + PATCH_FORMAT_MBOX +}; struct am_state { /* state directory path */ @@ -16,6 +22,9 @@ struct am_state { /* current and last patch numbers, 1-indexed */ int cur; int last; + + /* number of digits in patch filename */ + int prec; }; /** @@ -26,6 +35,8 @@ static void am_state_init(struct am_state *state) memset(state, 0, sizeof(*state)); strbuf_init(&state->dir, 0); + + state->prec = 4; } /** @@ -111,13 +122,69 @@ static void am_destroy(const struct am_state *state) } /** + * Splits out individual patches from `paths`, where each path is either a mbox + * file or a Maildir. Return 0 on success, -1 on failure. + */ +static int split_patches_mbox(struct am_state *state, struct string_list *paths) +{ + struct child_process cp = CHILD_PROCESS_INIT; + struct string_list_item *item; + struct strbuf last = STRBUF_INIT; + + cp.git_cmd = 1; + argv_array_push(&cp.args, "mailsplit"); + argv_array_pushf(&cp.args, "-d%d", state->prec); + argv_array_pushf(&cp.args, "-o%s", state->dir.buf); + argv_array_push(&cp.args, "-b"); + argv_array_push(&cp.args, "--"); + + for_each_string_list_item(item, paths) + argv_array_push(&cp.args, item->string); + + if (capture_command(&cp, &last, 8)) + return -1; + + state->cur = 1; + state->last = strtol(last.buf, NULL, 10); + + return 0; +} + +/** + * Splits out individual patches, of patch_format, contained within paths. + * These patches will be stored in the state directory, with each patch's + * filename being its index, padded to state->prec digits. state->cur will be + * set to the index of the first patch, and state->last will be set to the + * index of the last patch. + * + * Returns 0 on success, -1 on failure. + */ +static int split_patches(struct am_state *state, enum patch_format patch_format, + struct string_list *paths) +{ + switch (patch_format) { + case PATCH_FORMAT_MBOX: + return split_patches_mbox(state, paths); + default: + die("BUG: invalid patch_format"); + } + return -1; +} + +/** * Setup a new am session for applying patches */ -static void am_setup(struct am_state *state) +static void am_setup(struct am_state *state, enum patch_format patch_format, + struct string_list *paths) { if (mkdir(state->dir.buf, 0777) < 0 && errno != EEXIST) die_errno(_("failed to create directory '%s'"), state->dir.buf); + if (split_patches(state, patch_format, paths) < 0) { + am_destroy(state); + die(_("Failed to split patches.")); + } + write_file(am_path(state, "next"), 1, "%d", state->cur); write_file(am_path(state, "last"), 1, "%d", state->last); @@ -148,7 +215,23 @@ static void am_run(struct am_state *state) am_destroy(state); } +/** + * parse_options() callback that validates and sets opt->value to the + * PATCH_FORMAT_* enum value corresponding to `arg`. + */ +static int parse_opt_patchformat(const struct option *opt, const char *arg, int unset) +{ + int *opt_value = opt->value; + + if (!strcmp(arg, "mbox")) + *opt_value = PATCH_FORMAT_MBOX; + else + return -1; + return 0; +} + static struct am_state state; +static int opt_patch_format; static const char * const am_usage[] = { N_("git am [options] [(|)...]"), @@ -156,6 +239,8 @@ static const char * const am_usage[] = { }; static struct option am_options[] = { + OPT_CALLBACK(0, "patch-format", &opt_patch_format, N_("format"), + N_
[PATCH/WIP v3 06/31] 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 --- builtin/am.c | 98 1 file changed, 98 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index e9a3687..7b97ea8 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -121,6 +121,96 @@ static void am_destroy(const struct am_state *state) strbuf_release(&sb); } +/* + * Returns 1 if the file looks like a piece of email a-la RFC2822, 0 otherwise. + * We check this by grabbing all the non-indented lines and seeing if they look + * like they begin with valid header field names. + */ +static int is_email(const char *filename) +{ + struct strbuf sb = STRBUF_INIT; + FILE *fp = xfopen(filename, "r"); + int ret = 1; + + while (!strbuf_getline(&sb, fp, '\n')) { + const char *x; + + strbuf_rtrim(&sb); + + if (!sb.len) + break; /* End of header */ + + /* Ignore indented folded lines */ + if (*sb.buf == '\t' || *sb.buf == ' ') + continue; + + /* It's a header if it matches the regexp "^[!-9;-~]+:" */ + for (x = sb.buf; *x; x++) { + if (('!' <= *x && *x <= '9') || (';' <= *x && *x <= '~')) + continue; + if (*x == ':' && x != sb.buf) + break; + ret = 0; + goto done; + } + } + +done: + fclose(fp); + strbuf_release(&sb); + return ret; +} + +/** + * Attempts to detect the patch_format of the patches contained in `paths`, + * returning the PATCH_FORMAT_* enum value. Returns PATCH_FORMAT_UNKNOWN if + * detection fails. + */ +static int detect_patch_format(struct string_list *paths) +{ + enum patch_format ret = PATCH_FORMAT_UNKNOWN; + struct strbuf l1 = STRBUF_INIT; + struct strbuf l2 = STRBUF_INIT; + struct strbuf l3 = STRBUF_INIT; + FILE *fp; + + /* +* We default to mbox format if input is from stdin and for directories +*/ + if (!paths->nr || !strcmp(paths->items->string, "-") || + is_directory(paths->items->string)) { + ret = PATCH_FORMAT_MBOX; + goto done; + } + + /* +* Otherwise, check the first 3 lines of the first patch, starting +* from the first non-blank line, to try to detect its format. +*/ + fp = xfopen(paths->items->string, "r"); + while (!strbuf_getline(&l1, fp, '\n')) { + strbuf_trim(&l1); + if (l1.len) + break; + } + strbuf_getline(&l2, fp, '\n'); + strbuf_trim(&l2); + strbuf_getline(&l3, fp, '\n'); + strbuf_trim(&l3); + fclose(fp); + + if (starts_with(l1.buf, "From ") || starts_with(l1.buf, "From: ")) + ret = PATCH_FORMAT_MBOX; + else if (l1.len && l2.len && l3.len && is_email(paths->items->string)) + ret = PATCH_FORMAT_MBOX; + +done: + strbuf_release(&l1); + strbuf_release(&l2); + strbuf_release(&l3); + return ret; +} + /** * Splits out individual patches from `paths`, where each path is either a mbox * file or a Maildir. Return 0 on success, -1 on failure. @@ -177,6 +267,14 @@ static int split_patches(struct am_state *state, enum patch_format patch_format, static void am_setup(struct am_state *state, enum patch_format patch_format, struct string_list *paths) { + if (!patch_format) + patch_format = detect_patch_format(paths); + + if (!patch_format) { + fprintf_ln(stderr, _("Patch format detection failed.")); + exit(128); + } + if (mkdir(state->dir.buf, 0777) < 0 && errno != EEXIST) die_errno(_("failed to create directory '%s'"), state->dir.buf); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP v3 04/31] am: implement patch queue mechanism
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 --- builtin/am.c | 168 +++ 1 file changed, 168 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index dbc8836..af68c51 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -6,6 +6,158 @@ #include "cache.h" #include "builtin.h" #include "exec_cmd.h" +#include "parse-options.h" +#include "dir.h" + +struct am_state { + /* state directory path */ + struct strbuf dir; + + /* current and last patch numbers, 1-indexed */ + int cur; + int last; +}; + +/** + * Initializes am_state with the default values. + */ +static void am_state_init(struct am_state *state) +{ + memset(state, 0, sizeof(*state)); + + strbuf_init(&state->dir, 0); +} + +/** + * Release memory allocated by an am_state. + */ +static void am_state_release(struct am_state *state) +{ + strbuf_release(&state->dir); +} + +/** + * Returns path relative to the am_state directory. + */ +static inline const char *am_path(const struct am_state *state, const char *path) +{ + return mkpath("%s/%s", state->dir.buf, path); +} + +/** + * Returns 1 if there is an am session in progress, 0 otherwise. + */ +static int am_in_progress(const struct am_state *state) +{ + struct stat st; + + if (lstat(state->dir.buf, &st) < 0 || !S_ISDIR(st.st_mode)) + return 0; + if (lstat(am_path(state, "last"), &st) || !S_ISREG(st.st_mode)) + return 0; + if (lstat(am_path(state, "next"), &st) || !S_ISREG(st.st_mode)) + return 0; + return 1; +} + +/** + * Reads the contents of `file`. The third argument can be used to give a hint + * about the file size, to avoid reallocs. Returns number of bytes read on + * success, -1 if the file does not exist. If trim is set, trailing whitespace + * will be removed from the file contents. + */ +static int read_state_file(struct strbuf *sb, const char *file, size_t hint, int trim) +{ + strbuf_reset(sb); + if (strbuf_read_file(sb, file, hint) >= 0) { + if (trim) + strbuf_trim(sb); + + return sb->len; + } + + if (errno == ENOENT) + return -1; + + die_errno(_("could not read '%s'"), file); +} + +/** + * Loads state from disk. + */ +static void am_load(struct am_state *state) +{ + struct strbuf sb = STRBUF_INIT; + + read_state_file(&sb, am_path(state, "next"), 8, 1); + state->cur = strtol(sb.buf, NULL, 10); + + read_state_file(&sb, am_path(state, "last"), 8, 1); + state->last = strtol(sb.buf, NULL, 10); + + strbuf_release(&sb); +} + +/** + * Remove the am_state directory. + */ +static void am_destroy(const struct am_state *state) +{ + struct strbuf sb = STRBUF_INIT; + + strbuf_addstr(&sb, state->dir.buf); + remove_dir_recursively(&sb, 0); + strbuf_release(&sb); +} + +/** + * Setup a new am session for applying patches + */ +static void am_setup(struct am_state *state) +{ + if (mkdir(state->dir.buf, 0777) < 0 && errno != EEXIST) + die_errno(_("failed to create directory '%s'"), state->dir.buf); + + write_file(am_path(state, "next"), 1, "%d", state->cur); + + write_file(am_path(state, "last"), 1, "%d", state->last); +} + +/** + * Increments the patch pointer, and cleans am_state for the application of the + * next patch. + */ +static void am_next(struct am_state *state) +{ + state->cur++; + write_file(am_path(state, "next"), 1, "%d", s
[PATCH v4 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 Helped-by: Duy Nguyen Signed-off-by: Paul Tan --- Notes: v4 * Fixed the use-after-free. Thanks Duy for catching it. builtin/pull.c | 35 ++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/builtin/pull.c b/builtin/pull.c index 33a7b98..1c4fb44 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -72,7 +72,7 @@ static int opt_verbosity; static char *opt_progress; /* Options passed to git-merge or git-rebase */ -static enum rebase_type opt_rebase; +static enum rebase_type opt_rebase = -1; static char *opt_diffstat; static char *opt_log; static char *opt_squash; @@ -266,6 +266,36 @@ static const char *config_get_ff(void) } /** + * Returns the default configured value for --rebase. It first looks for the + * value of "branch.$curr_branch.rebase", where $curr_branch is the current + * branch, and if HEAD is detached or the configuration key does not exist, + * looks for the value of "pull.rebase". If both configuration keys do not + * exist, returns REBASE_FALSE. + */ +static enum rebase_type config_get_rebase(void) +{ + struct branch *curr_branch = branch_get("HEAD"); + const char *value; + + if (curr_branch) { + char *key = xstrfmt("branch.%s.rebase", curr_branch->name); + + if (!git_config_get_value(key, &value)) { + enum rebase_type ret = parse_config_rebase(key, value, 1); + free(key); + return ret; + } + + free(key); + } + + if (!git_config_get_value("pull.rebase", &value)) + return parse_config_rebase("pull.rebase", value, 1); + + return REBASE_FALSE; +} + +/** * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge * into merge_heads. */ @@ -707,6 +737,9 @@ int cmd_pull(int argc, const char **argv, const char *prefix) if (!opt_ff) opt_ff = xstrdup_or_null(config_get_ff()); + if (opt_rebase < 0) + opt_rebase = config_get_rebase(); + git_config(git_default_config, NULL); if (read_cache_unmerged()) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 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 --- builtin/pull.c | 247 - 1 file changed, 245 insertions(+), 2 deletions(-) diff --git a/builtin/pull.c b/builtin/pull.c index 98caffe..33a7b98 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -15,6 +15,53 @@ #include "dir.h" #include "refs.h" +enum rebase_type { + REBASE_INVALID = -1, + REBASE_FALSE = 0, + REBASE_TRUE, + REBASE_PRESERVE +}; + +/** + * Parses the value of --rebase. If value is a false value, returns + * REBASE_FALSE. If value is a true value, returns REBASE_TRUE. If value is + * "preserve", returns REBASE_PRESERVE. If value is a invalid value, dies with + * a fatal error if fatal is true, otherwise returns REBASE_INVALID. + */ +static enum rebase_type parse_config_rebase(const char *key, const char *value, + int fatal) +{ + int v = git_config_maybe_bool("pull.rebase", value); + + if (!v) + return REBASE_FALSE; + else if (v > 0) + return REBASE_TRUE; + else if (!strcmp(value, "preserve")) + return REBASE_PRESERVE; + + if (fatal) + die(_("Invalid value for %s: %s"), key, value); + else + error(_("Invalid value for %s: %s"), key, value); + + return REBASE_INVALID; +} + +/** + * Callback for --rebase, which parses arg with parse_config_rebase(). + */ +static int parse_opt_rebase(const struct option *opt, const char *arg, int unset) +{ + enum rebase_type *value = opt->value; + + if (arg) + *value = parse_config_rebase("--rebase", arg, 0); + else + *value = unset ? REBASE_FALSE : REBASE_TRUE; + return *value == REBASE_INVALID ? -1 : 0; +} + static const char * const pull_usage[] = { N_("git pull [options] [ [...]]"), NULL @@ -24,7 +71,8 @@ static const char * const pull_usage[] = { static int opt_verbosity; static char *opt_progress; -/* Options passed to git-merge */ +/* Options passed to git-merge or git-rebase */ +static enum rebase_type opt_rebase; static char *opt_diffstat; static char *opt_log; static char *opt_squash; @@ -58,8 +106,12 @@ static struct option pull_options[] = { N_("force progress reporting"), PARSE_OPT_NOARG), - /* Options passed to git-merge */ + /* Options passed to git-merge or git-rebase */ OPT_GROUP(N_("Options related to merging")), + { OPTION_CALLBACK, 'r', "rebase", &opt_rebase, + N_("false|true|preserve"), + N_("incorporate changes by rebasing rather than merging"), + PARSE_OPT_OPTARG, parse_opt_rebase }, OPT_PASSTHRU('n', NULL, &opt_diffstat, NULL, N_("do not show a diffstat at the end of the merge"), PARSE_OPT_NOARG | PARSE_OPT_NONEG), @@ -449,11 +501,194 @@ static int run_merge(void) return ret; } +/** + * Returns remote's upstream branch for the current branch. If remote is NULL, + * the current branch's configured default remote is used. Returns NULL if + * `remote` does not name a valid remote, HEAD does not point to a branch, + * remote is not the branch's configured remote or the branch does not have any + * configured upstream branch. + */ +static const char *get_upstream_branch(const char *remote)
[PATCH v4 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 eb2a28f..421a34d 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -431,7 +431,10 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs const char *remote = curr_branch ? curr_branch->remote_name : NULL; if (*refspecs) { - fprintf_ln(stderr, _("There are no candidates for merging among the refs that you just fetched.")); + if (opt_rebase) + fprintf_ln(stderr, _("There is no candidate for rebasing against among the refs that you just fetched.")); + else + fprintf_ln(stderr, _("There are no candidates for merging among the refs that you just fetched.")); fprintf_ln(stderr, _("Generally this means that you provided a wildcard refspec which had no\n" "matches on the remote end.")); } else if (repo && curr_branch && (!remote || strcmp(repo, remote))) { @@ -441,7 +444,10 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs repo); } else if (!curr_branch) { fprintf_ln(stderr, _("You are not currently on a branch.")); - fprintf_ln(stderr, _("Please specify which branch you want to merge with.")); + if (opt_rebase) + fprintf_ln(stderr, _("Please specify which branch you want to rebase against.")); + else + fprintf_ln(stderr, _("Please specify which branch you want to merge with.")); fprintf_ln(stderr, _("See git-pull(1) for details.")); fprintf(stderr, "\n"); fprintf_ln(stderr, "git pull "); @@ -453,7 +459,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 v4 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 492bb0e..98caffe 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -169,6 +169,25 @@ static void argv_push_force(struct argv_array *arr) } /** + * Sets the GIT_REFLOG_ACTION environment variable to the concatenation of argv + */ +static void set_reflog_message(int argc, const char **argv) +{ + int i; + struct strbuf msg = STRBUF_INIT; + + for (i = 0; i < argc; i++) { + if (i) + strbuf_addch(&msg, ' '); + strbuf_addstr(&msg, argv[i]); + } + + setenv("GIT_REFLOG_ACTION", msg.buf, 0); + + strbuf_release(&msg); +} + +/** * If pull.ff is unset, returns NULL. If pull.ff is "true", returns "--ff". If * pull.ff is "false", returns "--no-ff". If pull.ff is "only", returns * "--ff-only". Otherwise, if pull.ff is set to an invalid value, die with an @@ -443,6 +462,9 @@ int cmd_pull(int argc, const char **argv, const char *prefix) die_errno("could not exec %s", path); } + if (!getenv("GIT_REFLOG_ACTION")) + set_reflog_message(argc, argv); + argc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0); parse_repo_refspecs(argc, argv, &repo, &refspecs); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 17/19] pull --rebase: exit early when the working directory is dirty
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 1c4fb44..eb2a28f 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -14,6 +14,8 @@ #include "remote.h" #include "dir.h" #include "refs.h" +#include "revision.h" +#include "lockfile.h" enum rebase_type { REBASE_INVALID = -1, @@ -296,6 +298,73 @@ static enum rebase_type config_get_rebase(void) } /** + * Returns 1 if there are unstaged changes, 0 otherwise. + */ +static int has_unstaged_changes(const char *prefix) +{ + struct rev_info rev_info; + int result; + + init_revisions(&rev_info, prefix); + DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES); + DIFF_OPT_SET(&rev_info.diffopt, QUICK); + diff_setup_done(&rev_info.diffopt); + result = run_diff_files(&rev_info, 0); + return diff_result_code(&rev_info.diffopt, result); +} + +/** + * Returns 1 if there are uncommitted changes, 0 otherwise. + */ +static int has_uncommitted_changes(const char *prefix) +{ + struct rev_info rev_info; + int result; + + if (is_cache_unborn()) + return 0; + + init_revisions(&rev_info, prefix); + DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES); + DIFF_OPT_SET(&rev_info.diffopt, QUICK); + add_head_to_pending(&rev_info); + diff_setup_done(&rev_info.diffopt); + result = run_diff_index(&rev_info, 1); + return diff_result_code(&rev_info.diffopt, result); +} + +/** + * If the work tree has unstaged or uncommitted changes, dies with the + * appropriate message. + */ +static void die_on_unclean_work_tree(const char *prefix) +{ + struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file)); + int do_die = 0; + + hold_locked_index(lock_file, 0); + refresh_cache(REFRESH_QUIET); + update_index_if_able(&the_index, lock_file); + rollback_lock_file(lock_file); + + if (has_unstaged_changes(prefix)) { + error(_("Cannot pull with rebase: You have unstaged changes.")); + do_die = 1; + } + + if (has_uncommitted_changes(prefix)) { + if (do_die) + error(_("Additionally, your index contains uncommitted changes.")); + else + error(_("Cannot pull with rebase: Your index contains uncommitted changes.")); + do_die = 1; + } + + if (do_die) + exit(1); +} + +/** * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge * into merge_heads. */ @@ -751,9 +820,15 @@ int cmd_pull(int argc, const char **argv, const char *prefix) if (get_sha1("HEAD", orig_head)) hashclr(orig_head); - if (opt_rebase) + if (opt_rebase) { + if (is_null_sha1(orig_head) && !is_cache_unborn()) + die(_("Updating an unborn branch with changes added to the index.")); + + die_on_unclean_work_tree(prefix); + if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs)) hashclr(rebase_fork_point); + } if (run_fetch(repo, refspecs)) return 1; -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 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 b61cff5..1e688be 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -12,6 +12,7 @@ #include "run-command.h" #include "sha1-array.h" #include "remote.h" +#include "dir.h" static const char * const pull_usage[] = { N_("git pull [options] [ [...]]"), @@ -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
[PATCH v4 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 17e1136..93e4fa2 100644 --- a/Makefile +++ b/Makefile @@ -474,7 +474,6 @@ SCRIPT_SH += git-merge-octopus.sh SCRIPT_SH += git-merge-one-file.sh SCRIPT_SH += git-merge-resolve.sh SCRIPT_SH += git-mergetool.sh -SCRIPT_SH += git-pull.sh SCRIPT_SH += git-quiltimport.sh SCRIPT_SH += git-rebase.sh SCRIPT_SH += git-remote-testgit.sh diff --git a/builtin/pull.c b/builtin/pull.c index 421a34d..722a83c 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -798,13 +798,6 @@ int cmd_pull(int argc, const char **argv, const char *prefix) unsigned char orig_head[GIT_SHA1_RAWSZ], curr_head[GIT_SHA1_RAWSZ]; unsigned char rebase_fork_point[GIT_SHA1_RAWSZ]; - if (!getenv("_GIT_USE_BUILTIN_PULL")) { - const char *path = mkpath("%s/git-pull", git_exec_path()); - - if (sane_execvp(path, (char **)argv) < 0) - die_errno("could not exec %s", path); - } - if (!getenv("GIT_REFLOG_ACTION")) set_reflog_message(argc, argv); diff --git a/git-pull.sh b/contrib/examples/git-pull.sh similarity index 100% rename from git-pull.sh rename to contrib/examples/git-pull.sh -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 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 --- builtin/pull.c | 29 + 1 file changed, 29 insertions(+) diff --git a/builtin/pull.c b/builtin/pull.c index 647bcb9..b61cff5 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 v4 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 --- 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 v4 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 --- builtin/pull.c | 75 ++ 1 file changed, 75 insertions(+) diff --git a/builtin/pull.c b/builtin/pull.c index 5d9f2b5..0442da9 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -20,6 +20,18 @@ static const char * const pull_usage[] = { static int opt_verbosity; static char *opt_progress; +/* Options passed to git-merge */ +static char *opt_diffstat; +static char *opt_log; +static char *opt_squash; +static char *opt_commit; +static char *opt_edit; +static char *opt_ff; +static char *opt_verify_signatures; +static struct argv_array opt_strategies = ARGV_ARRAY_INIT; +static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT; +static char *opt_gpg_sign; + 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), + /* Options passed to git-merge */ + OPT_GROUP(N_("Options related to merging")), + OPT_PASSTHRU('n', NULL, &opt_diffstat, NULL, + N_("do not show a diffstat at the end of the merge"), + PARSE_OPT_NOARG | PARSE_OPT_NONEG), + OPT_PASSTHRU(0, "stat", &opt_diffstat, NULL, + N_("show a diffstat at the end of the merge"), + PARSE_OPT_NOARG), + OPT_PASSTHRU(0, "summary", &opt_diffstat, NULL, + N_("(synonym to --stat)"), + PARSE_OPT_NOARG | PARSE_OPT_HIDDEN), + OPT_PASSTHRU(0, "log", &opt_log, N_("n"), + N_("add (at most ) entries from shortlog to merge commit message"), + PARSE_OPT_OPTARG), + OPT_PASSTHRU(0, "squash", &opt_squash, NULL, + N_("create a single commit instead of doing a merge"), + PARSE_OPT_NOARG), + OPT_PASSTHRU(0, "commit", &opt_commit, NULL, + N_("perform a commit if the merge succeeds (default)"), + PARSE_OPT_NOARG), + OPT_PASSTHRU(0, "edit", &opt_edit, NULL, + N_("edit message before committing"), + PARSE_OPT_NOARG), + OPT_PASSTHRU(0, "ff", &opt_ff, NULL, + N_("allow fast-forward"), + PARSE_OPT_NOARG), + OPT_PASSTHRU(0, "ff-only", &opt_ff, NULL, + N_("abort if fast-forward is not possible"), + PARSE_OPT_NOARG | PARSE_OPT_NONEG), + OPT_PASSTHRU(0, "verify-signatures", &opt_verify_signatures, NULL, + N_("verify that the named commit has a valid GPG signature"), + PARSE_OPT_NOARG), + OPT_PASSTHRU_ARGV('s', "strategy", &opt_strategies, N_("strategy"), + N_("merge strategy to use"), + 0), + OPT_PASSTHRU_ARGV('X', "strategy-option", &opt_strategy_opts, + N_("option=value"), + N_("option for selected merge strategy"), + 0), + OPT_PASSTHRU('S', "gpg-sign", &opt_gpg_sign, N_("key-id"), + N_("GPG sign commit"), + PARSE_OPT_OPTARG), + OPT_END() }; @@ -101,6 +156,26 @@ static int run_merge(void) if (opt_progress) argv_array_push(&args, opt_progress); + /* Options passed to git-merge */ + if (opt_diffstat) + argv_array_push(&args, opt_diffstat); + if (opt_log) + argv_array_push(&args, opt_log); +
[PATCH v4 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 1e688be..110e719 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -412,6 +412,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()); @@ -435,12 +436,41 @@ int cmd_pull(int argc, const char **argv, const char *prefix) if (file_exists(git_path("MERGE_HEAD"))) die_conclude_merge(); + 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 v4 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 --- builtin/pull.c | 113 + 1 file changed, 113 insertions(+) diff --git a/builtin/pull.c b/builtin/pull.c index f35649c..647bcb9 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 wildcard refspec which had no\n" + "mat
[PATCH v4 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 149f1c7..17e1136 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 v4 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 110e719..492bb0e 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] [ [...]]"), @@ -368,6 +369,27 @@ static int run_fetch(const char *repo, const char **refspecs) } /** + * "Pulls into void" by branching off merge_head. + */ +static int pull_into_void(const unsigned char *merge_head, + const unsigned char *curr_head) +{ + /* +* Two-way merge: we treat the index as 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) @@ -476,5 +498,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 v4 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: Ramsay Jones 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..f35649c 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, NULL, + N_("fetch from all remotes"), + PARSE_OPT_NOARG), + OPT_PASSTHRU('a', "append", &opt_append, NULL, + 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, NULL, + N_("fetch all tags and associated objects"), + PARSE_OPT_NOARG), + OPT_PASSTHRU('p', "prune", &opt_prune, NULL, + 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, NULL, + 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, NULL, + N_("convert to a complete repository"), + PARSE_OPT_NONEG | PARSE_OPT_NOARG), + OPT_PASSTHRU(0, "update-shallow", &opt_update_shallow, NULL, + 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); +
[PATCH v4 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 --- 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 v4 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 v4 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 --- 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 v4 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 --- 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 struct option *, const char *, int); +extern int parse_opt_passthru(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)) @@ -242,5 +243,7 @@ extern int parse_opt_noop_cb(const struct option *, const char *, int); OPT_COLOR_FLAG(0, "color", (var), (h)) #define OPT_COLUMN(s, l, v, h) \ { 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 } #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 v4 00/19] Make git-pull a builtin
This is a re-roll of [v3]. It squashes in Ramsay's patch "fix some sparse warnings", and fixes the use-before-free reported by Duy. Thanks a lot for dealing with my mess :-). Other than that, there are no other changes as I'm working on the git-am side of things. Previous versions: [v1] http://thread.gmane.org/gmane.comp.version-control.git/269258 [v2] http://thread.gmane.org/gmane.comp.version-control.git/270639 [v3] http://thread.gmane.org/gmane.comp.version-control.git/271614 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| 882 ++ git-pull.sh => contrib/examples/git-pull.sh | 0 git.c | 1 + parse-options-cb.c| 69 ++ parse-options.h | 6 + 13 files changed, 992 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
Re: [PATCH/WIP v2 03/19] am: implement skeletal builtin am
On Tue, Jun 16, 2015 at 1:54 AM, Junio C Hamano wrote: > Paul Tan writes: > >>> The scripted Porcelain is spawned after applying patches 1-3 from >>> here, when you do not have _GIT_USE_BUILTIN_AM exported. Haven't >>> RUN_SETUP code did its thing by that time? >> >> Ah right, the RUN_SETUP code would have chdir()-ed to the working >> directory root, so git-am.sh will be unable to find the original >> working directory. To aid it, we would have to chdir() back to the >> original working directory, and unset GIT_DIR. > > I do not think that is a correct workaround, though. GIT_DIR may > have come from the end user, i.e. > > $ GIT_WORK_TREE=somewhere GIT_DIR=somewhere.else git am ... Ah, forgot about that >< > As the RUN_SETUP|REQUIRE_WORK_TREE bit is merely a convenence in > git.c, one workable way to keep these dual implementations is to do > what built-in commands used to do before these were invented. > Perhaps studying how cmd_diff(), which is run from git.c without > either RUN_SETUP or NEED_WORK_TREE, and taking good bits from it > would help. I think the implementation roughly would look like > this: > > int cmd_am(int ac, const char **av, const char *prefix) > { > /* > * NEEDSWORK: once we retire the dual-mode > * implementation, this preamble can be removed... > */ > if (... want to do scripted ...) { > ... spawn the scripted thing ... > } > prefix = setup_git_directory(); > setup_work_tree(); > /* ... up to this point */ > > ... your real "git am in C" comes here ... > } > Ah OK. Just to be sure, I took a look at run_builtin() in git.c, and indeed it only just does: prefix = setup_git_directory(); trace_repo_setup(prefix); setup_work_tree(); which is not bad at all. Thanks. Regards, Paul -- 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: New Defects reported by Coverity Scan for git
On Wed, Jun 17, 2015 at 9:54 PM, Duy Nguyen wrote: > I think Coverity caught this correctly. > > ** CID 1306846: Memory - illegal accesses (USE_AFTER_FREE) > /builtin/pull.c: 287 in config_get_rebase() > > > > *** CID 1306846: Memory - illegal accesses (USE_AFTER_FREE) > /builtin/pull.c: 287 in config_get_rebase() > 281 > 282 if (curr_branch) { > 283 char *key = xstrfmt("branch.%s.rebase", > curr_branch->name); > 284 > 285 if (!git_config_get_value(key, &value)) { > 286 free(key); CID 1306846: Memory - illegal accesses (USE_AFTER_FREE) Passing freed pointer "key" as an argument to "parse_config_rebase". > 287 return parse_config_rebase(key, value, 1); > 288 } > 289 > 290 free(key); > 291 } > 292 Ugh, thanks. >< 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] pull.c: fix some sparse warnings
On Wed, Jun 17, 2015 at 7:18 AM, Ramsay Jones wrote: > Hi Paul, > > If you need to re-roll your patches on the 'pt/pull-builtin' branch, > could you please squash this into the patch which corresponds to > commit 191241e5. Thanks. I must have been half-asleep because the block of code just above uses NULL instead of 0. >< 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/RFC] Revert "git am/mailinfo: Don't look at in-body headers when rebasing"
This reverts commit d25e51596be9271ad833805a3d6f9012dc24ee79, removing git-mailsplit's --no-inbody-headers option. While --no-inbody-headers was introduced to prevent commit messages from being munged by git-mailinfo while rebasing, the need for this option disappeared since 5e835ca (rebase: do not munge commit log message, 2008-04-16), as git-am bypasses git-mailinfo and gets the commit message directly from the commit ID in the patch. git-am is the only user of --no-inbody-headers, and this option is not documented. As such, it should be removed. Signed-off-by: Paul Tan --- Notes: The other direction, of course, is to turn --no-inbody-headers into a supported, documented option in both git-mailsplit and git-am. I do also wonder if we should just ensure that git-format-patch does not generate a message that start with "From" or "Date". builtin/mailinfo.c | 12 +--- git-am.sh| 9 + t/t5100-mailinfo.sh | 4 t/t5100/info0015--no-inbody-headers | 5 - t/t5100/info0016--no-inbody-headers | 5 - t/t5100/msg0015--no-inbody-headers | 3 --- t/t5100/msg0016--no-inbody-headers | 4 t/t5100/patch0015--no-inbody-headers | 8 t/t5100/patch0016--no-inbody-headers | 8 9 files changed, 2 insertions(+), 56 deletions(-) delete mode 100644 t/t5100/info0015--no-inbody-headers delete mode 100644 t/t5100/info0016--no-inbody-headers delete mode 100644 t/t5100/msg0015--no-inbody-headers delete mode 100644 t/t5100/msg0016--no-inbody-headers delete mode 100644 t/t5100/patch0015--no-inbody-headers delete mode 100644 t/t5100/patch0016--no-inbody-headers diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c index 999a525..34ea160 100644 --- a/builtin/mailinfo.c +++ b/builtin/mailinfo.c @@ -26,7 +26,6 @@ static int patch_lines; static struct strbuf **p_hdr_data, **s_hdr_data; static int use_scissors; static int add_message_id; -static int use_inbody_headers = 1; #define MAX_HDR_PARSED 10 #define MAX_BOUNDARIES 5 @@ -795,17 +794,10 @@ static int handle_commit_msg(struct strbuf *line) if (still_looking) { if (!line->len || (line->len == 1 && line->buf[0] == '\n')) return 0; - } - - if (use_inbody_headers && still_looking) { still_looking = check_header(line, s_hdr_data, 0); if (still_looking) return 0; - } else - /* Only trim the first (blank) line of the commit message -* when ignoring in-body headers. -*/ - still_looking = 0; + } /* normalize the log message to UTF-8. */ if (metainfo_charset) @@ -1062,8 +1054,6 @@ int cmd_mailinfo(int argc, const char **argv, const char *prefix) use_scissors = 1; else if (!strcmp(argv[1], "--no-scissors")) use_scissors = 0; - else if (!strcmp(argv[1], "--no-inbody-headers")) - use_inbody_headers = 0; else usage(mailinfo_usage); argc--; argv++; diff --git a/git-am.sh b/git-am.sh index 761befb..df403b0 100755 --- a/git-am.sh +++ b/git-am.sh @@ -372,7 +372,7 @@ split_patches () { prec=4 dotest="$GIT_DIR/rebase-apply" sign= utf8=t keep= keepcr= skip= interactive= resolved= rebasing= abort= -messageid= resolvemsg= resume= scissors= no_inbody_headers= +messageid= resolvemsg= resume= scissors= git_apply_opt= committer_date_is_author_date= ignore_date= @@ -579,7 +579,6 @@ Use \"git am --abort\" to remove it.")" echo "$keep" >"$dotest/keep" echo "$messageid" >"$dotest/messageid" echo "$scissors" >"$dotest/scissors" - echo "$no_inbody_headers" >"$dotest/no_inbody_headers" echo "$GIT_QUIET" >"$dotest/quiet" echo 1 >"$dotest/next" if test -n "$rebasing" @@ -644,12 +643,6 @@ t) f) scissors=--no-scissors ;; esac -if test "$(cat "$dotest/no_inbody_headers")" = t -then - no_inbody_headers=--no-inbody-headers -else - no_inbody_headers= -fi if test "$(cat "$dotest/quiet")" = t then GIT_QUIET=t diff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh index e97cfb2..b2b5be6 100755 --- a/t/t5100-mailinfo.sh +++ b/t/t5100-mailinfo.sh @@ -31,10 +31,6 @@ do then check_mailinfo $mail --scissors fi && - if test -f "$TEST_DIRECTORY"/t5100/msg$mail--no-inbody-headers - then - check_mailinfo $mail --no-inbody-h
Re: [PATCH/WIP v2 03/19] am: implement skeletal builtin am
On Tue, Jun 16, 2015 at 1:14 AM, Junio C Hamano wrote: > Paul Tan writes: > >> On Mon, Jun 15, 2015 at 6:08 AM, Junio C Hamano wrote: >>> Paul Tan writes: >>>> diff --git a/git.c b/git.c >>>> index 44374b1..42328ed 100644 >>>> --- a/git.c >>>> +++ b/git.c >>>> @@ -370,6 +370,7 @@ static int run_builtin(struct cmd_struct *p, int argc, >>>> const char **argv) >>>> >>>> static struct cmd_struct commands[] = { >>>> { "add", cmd_add, RUN_SETUP | NEED_WORK_TREE }, >>>> + { "am", cmd_am, RUN_SETUP | NEED_WORK_TREE }, >>> >>> Would this, especially having RUN_SETUP, keep the same behaviour >>> when the command is run from a subdirectory of a working tree? >>> e.g. >>> >>> save messages to ./inbox >>> $ cd sub/dir >>> $ git am ../../inbox >>> >> >> Yes, in 05/19, where the splitting of patches is implemented, we >> prefix the mbox paths with the path to the working tree. > > I wasn't wondering about your new code. > > The scripted Porcelain is spawned after applying patches 1-3 from > here, when you do not have _GIT_USE_BUILTIN_AM exported. Haven't > RUN_SETUP code did its thing by that time? Ah right, the RUN_SETUP code would have chdir()-ed to the working directory root, so git-am.sh will be unable to find the original working directory. To aid it, we would have to chdir() back to the original working directory, and unset GIT_DIR. Thanks, Paul -- 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 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 5ea2e4d..f0b6c16 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 v2 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 v2 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 Helped-by: Junio C Hamano Signed-off-by: Paul Tan --- Notes: v2 * Just pass the filename directly to perl. Hmm, I think we should add a "--" in front so that filenames that start with a dash won't be interpreted as a command-line switch by perl? git-am.sh | 3 ++- t/t4150-am.sh | 10 ++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/git-am.sh b/git-am.sh index 761befb..5ea2e4d 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) @@ -318,7 +319,7 @@ split_patches () { print "Subject: ", $_ ; $subject = 1; } - ' < "$stgit" > "$dotest/$msgnum" || clean_abort + ' -- "$stgit" >"$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 v2 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 Helped-by: Junio C Hamano Signed-off-by: Paul Tan --- Notes: v2 * Pass the filename directly to perl instead. git-am.sh | 3 ++- t/t4150-am.sh | 10 ++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/git-am.sh b/git-am.sh index f0b6c16..a8d33ef 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 )) @@ -353,7 +354,7 @@ split_patches () { print "\n", $_ ; $subject = 1; } - ' <"$hg" >"$dotest/$msgnum" || clean_abort + ' -- "$hg" >"$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 v2 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 v2 0/5] am: improve test coverage and touch up foreign patch parsing
This is a re-roll of [v1]. Previous versions: [v1] http://thread.gmane.org/gmane.comp.version-control.git/271053 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. Some 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 | 12 + t/t4150-am.sh | 82 +++ 2 files changed, 89 insertions(+), 5 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: [PATCH/WIP v2 04/19] am: implement patch queue mechanism
On Fri, Jun 12, 2015 at 1:39 AM, Stefan Beller wrote: > On Thu, Jun 11, 2015 at 3:21 AM, Paul Tan wrote: >> Notes: >> v2 >> >> * Declare struct am_state as static >> >> builtin/am.c | 164 >> +++ >> 1 file changed, 164 insertions(+) >> >> diff --git a/builtin/am.c b/builtin/am.c >> index 0ccbe33..f061d21 100644 >> --- a/builtin/am.c >> +++ b/builtin/am.c >> @@ -6,6 +6,154 @@ >> #include "cache.h" >> #include "builtin.h" >> #include "exec_cmd.h" >> +#include "parse-options.h" >> +#include "dir.h" >> + >> +struct am_state { > > Did you mean to declare all the functions below to be static or the > struct as well? Well, everything in this file, with the exception of the cmd_am() entry point, should have static linkage. The changelog comment was referring to [1], but I should have made it clearer. Sorry if it was confusing. [1] http://thread.gmane.org/gmane.comp.version-control.git/270048/focus=270205 > Reading further, you declared it static below. I thought maybe it'd be > useful to have definition > and declaration up here, but having all declarations further below may > be even better. Right, I aimed to have a strict separation between "git-am: the functionality" and "git-am: the command-line interface", where the latter depends on the former, and not the other way round (or have circular dependencies). The former perhaps could even be moved into libgit.a in the future. Thanks, Paul -- 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 v2 05/19] am: split out mbox/maildir patches with git-mailsplit
On Fri, Jun 12, 2015 at 1:45 AM, Stefan Beller wrote: > On Thu, Jun 11, 2015 at 3:21 AM, Paul Tan wrote: >> @@ -138,13 +202,33 @@ static void am_next(struct am_state *state) >> */ >> static void am_run(struct am_state *state) >> { >> - while (state->cur <= state->last) >> + while (state->cur <= state->last) { >> + >> + /* TODO: Patch application not implemented yet */ >> + >> am_next(state); >> + } > > When reviewing the previous patch I did look at this loop for awhile confused, > if you want to apply patches in am_next(state) and thought there might be > a better approach. > > Maybe you want to move this chunk with the TODO into the previous patch, > so it's clear after reading the documentation of am_run, that the actual am is > missing there. Ah right, this is a mistake. This comment should be in the previous patch. Thanks, Paul -- 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 v2 03/19] am: implement skeletal builtin am
On Mon, Jun 15, 2015 at 6:08 AM, Junio C Hamano wrote: > Paul Tan writes: >> diff --git a/git.c b/git.c >> index 44374b1..42328ed 100644 >> --- a/git.c >> +++ b/git.c >> @@ -370,6 +370,7 @@ static int run_builtin(struct cmd_struct *p, int argc, >> const char **argv) >> >> static struct cmd_struct commands[] = { >> { "add", cmd_add, RUN_SETUP | NEED_WORK_TREE }, >> + { "am", cmd_am, RUN_SETUP | NEED_WORK_TREE }, > > Would this, especially having RUN_SETUP, keep the same behaviour > when the command is run from a subdirectory of a working tree? > e.g. > > save messages to ./inbox > $ cd sub/dir > $ git am ../../inbox > Yes, in 05/19, where the splitting of patches is implemented, we prefix the mbox paths with the path to the working tree. There are also tests in t4150 to catch this, introduced in bb034f8 (am: read from the right mailbox when started from a subdirectory, 2008-03-04). Thanks, Paul -- 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 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 cb5898a..0e48a14 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -412,6 +412,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()); @@ -435,12 +436,41 @@ int cmd_pull(int argc, const char **argv, const char *prefix) if (file_exists(git_path("MERGE_HEAD"))) die_conclude_merge(); + 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 v3 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 --- Notes: v3 * style fixes builtin/pull.c | 29 - 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/builtin/pull.c b/builtin/pull.c index 0e48a14..a2dd0ba 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] [ [...]]"), @@ -368,6 +369,27 @@ static int run_fetch(const char *repo, const char **refspecs) } /** + * "Pulls into void" by branching off merge_head. + */ +static int pull_into_void(const unsigned char *merge_head, + const unsigned char *curr_head) +{ + /* +* Two-way merge: we treat the index as 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) @@ -476,5 +498,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 v3 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 e4098d0..a379c1f 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, @@ -295,6 +297,73 @@ static enum rebase_type config_get_rebase(void) } /** + * Returns 1 if there are unstaged changes, 0 otherwise. + */ +static int has_unstaged_changes(const char *prefix) +{ + struct rev_info rev_info; + int result; + + init_revisions(&rev_info, prefix); + DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES); + DIFF_OPT_SET(&rev_info.diffopt, QUICK); + diff_setup_done(&rev_info.diffopt); + result = run_diff_files(&rev_info, 0); + return diff_result_code(&rev_info.diffopt, result); +} + +/** + * Returns 1 if there are uncommitted changes, 0 otherwise. + */ +static int has_uncommitted_changes(const char *prefix) +{ + struct rev_info rev_info; + int result; + + if (is_cache_unborn()) + return 0; + + init_revisions(&rev_info, prefix); + DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES); + DIFF_OPT_SET(&rev_info.diffopt, QUICK); + add_head_to_pending(&rev_info); + diff_setup_done(&rev_info.diffopt); + result = run_diff_index(&rev_info, 1); + return diff_result_code(&rev_info.diffopt, result); +} + +/** + * If the work tree has unstaged or uncommitted changes, dies with the + * appropriate message. + */ +static void die_on_unclean_work_tree(const char *prefix) +{ + struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file)); + int do_die = 0; + + hold_locked_index(lock_file, 0); + refresh_cache(REFRESH_QUIET); + update_index_if_able(&the_index, lock_file); + rollback_lock_file(lock_file); + + if (has_unstaged_changes(prefix)) { + error(_("Cannot pull with rebase: You have unstaged changes.")); + do_die = 1; + } + + if (has_uncommitted_changes(prefix)) { + if (do_die) + error(_("Additionally, your index contains uncommitted changes.")); + else + error(_("Cannot pull with rebase: Your index contains uncommitted changes.")); + do_die = 1; + } + + if (do_die) + exit(1); +} + +/** * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge * into merge_heads. */ @@ -750,9 +819,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 v3 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 b78c67c..d690aee 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -797,13 +797,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 v3 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: v3 * use branch_get_upstream() * style fixes * adjust to removal of branch->remote in 9e3751d (remote.c: drop "remote" pointer from "struct branch", 2015-05-21) * I realised that if parse_config_rebase() handled the die()-ing and error()-ing, it would make the next patch more pleasant. builtin/pull.c | 247 - 1 file changed, 245 insertions(+), 2 deletions(-) diff --git a/builtin/pull.c b/builtin/pull.c index a2c900e..8915947 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -15,6 +15,53 @@ #include "dir.h" #include "refs.h" +enum rebase_type { + REBASE_INVALID = -1, + REBASE_FALSE = 0, + REBASE_TRUE, + REBASE_PRESERVE +}; + +/** + * Parses the value of --rebase. If value is a false value, returns + * REBASE_FALSE. If value is a true value, returns REBASE_TRUE. If value is + * "preserve", returns REBASE_PRESERVE. If value is a invalid value, dies with + * a fatal error if fatal is true, otherwise returns REBASE_INVALID. + */ +static enum rebase_type parse_config_rebase(const char *key, const char *value, + int fatal) +{ + int v = git_config_maybe_bool("pull.rebase", value); + + if (!v) + return REBASE_FALSE; + else if (v > 0) + return REBASE_TRUE; + else if (!strcmp(value, "preserve")) + return REBASE_PRESERVE; + + if (fatal) + die(_("Invalid value for %s: %s"), key, value); + else + error(_("Invalid value for %s: %s"), key, value); + + return REBASE_INVALID; +} + +/** + * Callback for --rebase, which parses arg with parse_config_rebase(). + */ +static int parse_opt_rebase(const struct option *opt, const char *arg, int unset) +{ + enum rebase_type *value = opt->value; + + if (arg) + *value = parse_config_rebase("--rebase", arg, 0); + else + *value = unset ? REBASE_FALSE : REBASE_TRUE; + return *value == REBASE_INVALID ? -1 : 0; +} + static const char * const pull_usage[] = { N_("git pull [options] [ [...]]"), NULL @@ -24,7 +71,8 @@ static const char * const pull_usage[] = { static int opt_verbosity; static char *opt_progress; -/* Options passed to git-merge */ +/* Options passed to git-merge or git-rebase */ +static enum rebase_type opt_rebase; static char *opt_diffstat; static char *opt_log; static char *opt_squash; @@ -58,8 +106,12 @@ static struct option pull_options[] = { N_("force progress reporting"), PARSE_OPT_NOARG), - /* Options passed to git-merge */ + /* Options passed to git-merge or git-rebase */ OPT_GROUP(N_("Options related to merging")), + { OPTION_CALLBACK, 'r', "rebase", &opt_rebase, + N_("false|true|preserve"), + N_("incorporate changes by rebasing rather than merging"), + PARSE_OPT_OPTARG, parse_opt_rebase }, OPT_PASSTHRU('n', NULL, &opt_diffstat, NULL, N_("do not show a diffstat at the end of the merge"), PARSE_OPT_NOARG | PARSE_OPT_NONEG), @@ -449,11 +501,194 @@ static int run_merge(void) return ret; } +/** + * Returns remote's upstream branch for the curren
[PATCH v3 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 --- builtin/pull.c | 75 ++ 1 file changed, 75 insertions(+) diff --git a/builtin/pull.c b/builtin/pull.c index 5d9f2b5..0442da9 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -20,6 +20,18 @@ static const char * const pull_usage[] = { static int opt_verbosity; static char *opt_progress; +/* Options passed to git-merge */ +static char *opt_diffstat; +static char *opt_log; +static char *opt_squash; +static char *opt_commit; +static char *opt_edit; +static char *opt_ff; +static char *opt_verify_signatures; +static struct argv_array opt_strategies = ARGV_ARRAY_INIT; +static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT; +static char *opt_gpg_sign; + 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), + /* Options passed to git-merge */ + OPT_GROUP(N_("Options related to merging")), + OPT_PASSTHRU('n', NULL, &opt_diffstat, NULL, + N_("do not show a diffstat at the end of the merge"), + PARSE_OPT_NOARG | PARSE_OPT_NONEG), + OPT_PASSTHRU(0, "stat", &opt_diffstat, NULL, + N_("show a diffstat at the end of the merge"), + PARSE_OPT_NOARG), + OPT_PASSTHRU(0, "summary", &opt_diffstat, NULL, + N_("(synonym to --stat)"), + PARSE_OPT_NOARG | PARSE_OPT_HIDDEN), + OPT_PASSTHRU(0, "log", &opt_log, N_("n"), + N_("add (at most ) entries from shortlog to merge commit message"), + PARSE_OPT_OPTARG), + OPT_PASSTHRU(0, "squash", &opt_squash, NULL, + N_("create a single commit instead of doing a merge"), + PARSE_OPT_NOARG), + OPT_PASSTHRU(0, "commit", &opt_commit, NULL, + N_("perform a commit if the merge succeeds (default)"), + PARSE_OPT_NOARG), + OPT_PASSTHRU(0, "edit", &opt_edit, NULL, + N_("edit message before committing"), + PARSE_OPT_NOARG), + OPT_PASSTHRU(0, "ff", &opt_ff, NULL, + N_("allow fast-forward"), + PARSE_OPT_NOARG), + OPT_PASSTHRU(0, "ff-only", &opt_ff, NULL, + N_("abort if fast-forward is not possible"), + PARSE_OPT_NOARG | PARSE_OPT_NONEG), + OPT_PASSTHRU(0, "verify-signatures", &opt_verify_signatures, NULL, + N_("verify that the named commit has a valid GPG signature"), + PARSE_OPT_NOARG), + OPT_PASSTHRU_ARGV('s', "strategy", &opt_strategies, N_("strategy"), + N_("merge strategy to use"), + 0), + OPT_PASSTHRU_ARGV('X', "strategy-option", &opt_strategy_opts, + N_("option=value"), + N_("option for selected merge strategy"), + 0), + OPT_PASSTHRU('S', "gpg-sign", &opt_gpg_sign, N_("key-id"), + N_("GPG sign commit"), + PARSE_OPT_OPTARG), + OPT_END() }; @@ -101,6 +156,26 @@ static int run_merge(void) if (opt_progress) argv_array_push(&args, opt_progress); + /* Options passed to git-merge */ + if (opt_diffstat) + argv_array_push(&args, opt_diffstat); + if (opt_log) + argv_array_push(&args, opt_log); +