[PATCH] am: let command-line options override saved options
When resuming, git-am ignores command-line options. For instance, when a patch fails to apply with "git am patch", subsequently running "git am --3way patch" would not cause git-am to fall back on attempting a threeway merge. This occurs because by default the --3way option is saved as "false", and the saved am options are loaded after the command-line options are parsed, thus overwriting the command-line options when resuming. Fix this by moving the am_load() function call before parse_options(), so that command-line options will override the saved am options. Reported-by: Junio C Hamano Helped-by: Jeff King Signed-off-by: Paul Tan --- builtin/am.c | 9 ++- t/t4153-am-resume-override-opts.sh | 144 + 2 files changed, 150 insertions(+), 3 deletions(-) create mode 100755 t/t4153-am-resume-override-opts.sh diff --git a/builtin/am.c b/builtin/am.c index 1116304..8a0b0e4 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -2131,6 +2131,7 @@ int cmd_am(int argc, const char **argv, const char *prefix) int keep_cr = -1; int patch_format = PATCH_FORMAT_UNKNOWN; enum resume_mode resume = RESUME_FALSE; + int in_progress; const char * const usage[] = { N_("git am [options] [(|)...]"), @@ -2226,6 +2227,10 @@ int cmd_am(int argc, const char **argv, const char *prefix) am_state_init(&state, git_path("rebase-apply")); + in_progress = am_in_progress(&state); + if (in_progress) + am_load(&state); + argc = parse_options(argc, argv, prefix, options, usage, 0); if (binary >= 0) @@ -2238,7 +2243,7 @@ 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 (in_progress) { /* * Catch user error to feed us patches when there is a session * in progress: @@ -2256,8 +2261,6 @@ int cmd_am(int argc, const char **argv, const char *prefix) if (resume == RESUME_FALSE) resume = RESUME_APPLY; - - am_load(&state); } else { struct argv_array paths = ARGV_ARRAY_INIT; int i; diff --git a/t/t4153-am-resume-override-opts.sh b/t/t4153-am-resume-override-opts.sh new file mode 100755 index 000..c49457c --- /dev/null +++ b/t/t4153-am-resume-override-opts.sh @@ -0,0 +1,144 @@ +#!/bin/sh + +test_description='git-am command-line options override saved options' + +. ./test-lib.sh + +test_expect_success 'setup' ' + test_commit initial file && + test_commit first file && + + git checkout -b side initial && + test_commit side-first file && + test_commit side-second file && + + { + echo "Message-Id: " && + git format-patch --stdout -1 side-first | sed -e "1d" + } >side-first.patch && + { + sed -ne "1,/^\$/p" side-first.patch && + echo "-- >8 --" && + sed -e "1,/^\$/d" side-first.patch + } >side-first.scissors && + + { + echo "Message-Id: " && + git format-patch --stdout -1 side-second | sed -e "1d" + } >side-second.patch && + { + sed -ne "1,/^\$/p" side-second.patch && + echo "-- >8 --" && + sed -e "1,/^\$/d" side-second.patch + } >side-second.scissors +' + +test_expect_success '--3way, --no-3way' ' + rm -fr .git/rebase-apply && + git reset --hard && + git checkout first && + test_must_fail git am --3way side-first.patch side-second.patch && + test -n "$(git ls-files -u)" && + echo will-conflict >file && + git add file && + test_must_fail git am --no-3way --continue && + test -z "$(git ls-files -u)" +' + +test_expect_success '--no-quiet, --quiet' ' + rm -fr .git/rebase-apply && + git reset --hard && + git checkout first && + test_must_fail git am --no-quiet side-first.patch side-second.patch && + test_must_be_empty out && + echo side-first >file && + git add file && + git am --quiet --continue >out && + test_must_be_empty out +' + +test_expect_success '--signoff, --no-signoff' ' + rm -fr .git/reba
Re: [PATCH] am: let command-line options override saved options
On Wed, Jul 29, 2015 at 1:09 AM, Junio C Hamano wrote: > Paul Tan writes: > >> diff --git a/t/t4153-am-resume-override-opts.sh >> b/t/t4153-am-resume-override-opts.sh >> new file mode 100755 >> index 000..c49457c >> --- /dev/null >> +++ b/t/t4153-am-resume-override-opts.sh >> @@ -0,0 +1,144 @@ >> +#!/bin/sh >> + >> +test_description='git-am command-line options override saved options' >> + >> +. ./test-lib.sh >> + >> +test_expect_success 'setup' ' >> + test_commit initial file && >> + test_commit first file && >> + >> + git checkout -b side initial && >> + test_commit side-first file && >> + test_commit side-second file && >> + >> + { >> + echo "Message-Id: " && >> + git format-patch --stdout -1 side-first | sed -e "1d" >> + } >side-first.patch && >> + { >> + sed -ne "1,/^\$/p" side-first.patch && > > sed -e "/^\$/q" would work just as well here OK. >> + echo "-- >8 --" && >> + sed -e "1,/^\$/d" side-first.patch >> + } >side-first.scissors && >> + { >> + echo "Message-Id: " && >> + git format-patch --stdout -1 side-second | sed -e "1d" >> + } >side-second.patch && >> + { >> + sed -ne "1,/^\$/p" side-second.patch && >> + echo "-- >8 --" && >> + sed -e "1,/^\$/d" side-second.patch >> + } >side-second.scissors >> +' > > A helper function that takes the branch name may be a good idea, > not just to consolidate the implementation but as a place to > document how these pairs of files are constructed and why. I think I will introduce a format_patch() function that takes a single commit-ish so that we can use tag names to name the patches: # Given a single commit $commit, formats the following patches with # git-format-patch: # # 1. $commit.eml: an email patch with a Message-Id header. # 2. $commit.scissors: like $commit.eml but contains a scissors line at the #start of the commit message body. format_patch () { { echo "Message-Id: <$1...@example.com>" && git format-patch --stdout -1 "$1" | sed -e '1d' } >"$1".eml && { sed -e '/^$/q' "$1".eml && echo '-- >8 --' && sed -e '1,/^$/d' "$1".eml } >"$1".scissors } >> +' >> + >> +test_expect_success '--signoff, --no-signoff' ' >> + rm -fr .git/rebase-apply && >> + git reset --hard && >> + git checkout first && >> + test_must_fail git am --signoff side-first.patch side-second.patch && >> + echo side-first >file && >> + git add file && >> + git am --no-signoff --continue && >> + >> + # applied side-first will be signed off >> + echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" >> >expected && >> + git cat-file commit HEAD^ | grep "Signed-off-by:" >actual && >> + test_cmp expected actual && >> + >> + # applied side-second will not be signed off >> + test $(git cat-file commit HEAD | grep -c "Signed-off-by:") -eq 0 >> +' > > Hmm, the command was run with --signoff at the start, first gets > applied with "am --no-signoff --resolved" so I would expect it does > not get signed off, but the second one will apply cleanly on top, so > shouldn't it get signed off? Or perhaps somehow I misread Peff's > idea to make these override one-shot in $gmane/274635? Ah, I was just following the structure of the code, but stepping back to think about it, I think there are 2 bugs: 1. The signoff is appended during the email-parsing stage. As such, when we are resuming, --no-signoff will have no effect, because the signoff has already been appended at that stage. A solution for this is tricky though, as there are functions of git-am that probably depend on the present behavior of the appended signoff being present in the commit message: * The applypatch-msg hook * The --interactive prompt, where the user can edit the commit message (to remove or edit the signoff maybe?) These functions are called before we attem
Re: [PATCH] pull.sh: quote $upload_pack when passing it to git-fetch
On Fri, Jul 31, 2015 at 4:40 AM, Matthieu Moy wrote: > The previous code broke for example > > git pull --upload-pack 'echo --foo' > > Reported-by: Joey Hess > Fix-suggested-by: Junio C Hamano > Signed-off-by: Matthieu Moy Thanks for cleaning up my mess! >< 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] am: let command-line options override saved options
On Sat, Aug 1, 2015 at 12:04 AM, Junio C Hamano wrote: > Paul Tan writes: > >> I think I will introduce a format_patch() function that takes a single >> commit-ish so that we can use tag names to name the patches: >> >> # Given a single commit $commit, formats the following patches with >> # git-format-patch: >> # >> # 1. $commit.eml: an email patch with a Message-Id header. >> # 2. $commit.scissors: like $commit.eml but contains a scissors line at the >> #start of the commit message body. >> format_patch () { >> { >> echo "Message-Id: <$1...@example.com>" && >> git format-patch --stdout -1 "$1" | sed -e '1d' >> } >"$1".eml && > > I only said I can "understand" what is going on, though. > > It feels a bit unnatural for a test to feed a message that lack the > "From " header line. Perhaps > > git format-patch --add-header="Message-Id: ..." --stdout -1 > > or something? Ah, okay. I wasn't aware of the --add-header option, but this is definitely better. >> These functions are called before we attempt to apply the patch, so we >> should probably call append_signoff before then. However, this still >> means that --no-signoff will have no effect should the patch >> application fail and we resume, as the signoff would still have >> already been appended... > > Ah, I see. Let's not worry about this; we cannot change the > expectation existing hook scripts depends on. Okay, although this means that with the below change, --[no-]signoff will be the oddball option that does not work when resuming. >> 2. Re-reading Peff's message, I see that he expects the command-line >> options to affect just the current patch, which makes sense. This >> patch would need to be extended to call am_load() after we finish >> processing the current patch when resuming. > > Yeah, so the idea is: > > - upon the very first invocation, we parse the command line options >and write the states out; > > - subsequent invocation, we read from the states and then override >with the command line options, but we do not write the states out >to update, so that subsequent invocations will keep reading from >the very first one. ... and we also load back the saved options after processing the patch that we resume from, so the command-line options only affect the conflicting patch, which fits in with Peff's idea on "wiggling that _one_ patch". >>>> +test_expect_success '--3way, --no-3way' ' >>>> + rm -fr .git/rebase-apply && >>>> + git reset --hard && >>>> + git checkout first && >>>> + test_must_fail git am --3way side-first.patch side-second.patch && >>>> + test -n "$(git ls-files -u)" && >>>> + echo will-conflict >file && >>>> + git add file && >>>> + test_must_fail git am --no-3way --continue && >>>> + test -z "$(git ls-files -u)" >>>> +' >>>> + >> >> ... Although if I implement the above change, I can't implement the >> test for --3way, as I think the only way to check if --3way/--no-3way >> successfully overrides the saved options for the current patch only is >> to run "git am --3way", but that does not work in the test runner as >> it expects stdin to be a TTY :-/ So I may have to remove this test. >> This shouldn't be a problem though, as all the tests in this test >> suite all test the same mechanism. > > Sorry, you lost me. Where does the TTY come into the picture only > for --3way (but not for other things like --quiet)? Ah, sorry, I should have provided more context. This is due to the following block of code: /* * 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); And it will activate when git-am is run without --continue/--abort/--skip (e.g. "git am --3way") because the test framework sets stdin to /dev/null. 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: What's cooking in git.git (Aug 2015, #01; Mon, 3)
On Tue, Aug 4, 2015 at 6:18 AM, Junio C Hamano wrote: > - I think pt/am-builtin is more or less ready, but the fix to the >issue that options given to 'git am' when restarting were >rejected must be queued on that topic before we can start >thinking of merging it to 'master' for the next release. Yeah, sorry for the lack of updates, but I'm working on it[1]. I've added a patch that adds stdin pty support to test_terminal, and made --signoff work when resuming. However, it turns out that --keep, --message-id and --scissors do not work as well, as they apply to the mail-parsing stage (git-mailinfo), which comes before the patch application stage. It's tricky, as their functionality is locked inside builtin/mailinfo.c. :-/ I'm thinking about leaving them broken for now to push this patch series forward, until the future where the git-mailinfo functionality gets moved into libgit.a or something so we can access the individual functions directly and work something out. Will send a re-roll once I look over the patches with a fresh set of eyes. [1] https://github.com/pyokagan/git/compare/pt/builtin-am...pt/am-resume-override-opts 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 v7 01/45] 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 Helped-by: Johannes Schindelin Helped-by: Junio C Hamano Signed-off-by: Paul Tan --- git-compat-util.h | 1 + wrapper.c | 35 +++ 2 files changed, 36 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..0a4502d 100644 --- a/wrapper.c +++ b/wrapper.c @@ -189,6 +189,41 @@ 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_arg() will have undefined behavior if the specified type is not +* compatible with the argument type. Since integers are promoted to +* ints, we fetch the next argument as an int, and then cast it to a +* mode_t to avoid undefined behavior. +*/ + va_start(ap, oflag); + if (oflag & O_CREAT) + mode = va_arg(ap, int); + va_end(ap); + + for (;;) { + int fd = open(path, oflag, mode); + if (fd >= 0) + return fd; + if (errno == EINTR) + continue; + + if ((oflag & O_RDWR) == O_RDWR) + die_errno(_("could not open '%s' for reading and writing"), path); + else if ((oflag & O_WRONLY) == O_WRONLY) + die_errno(_("could not open '%s' for writing"), path); + else + die_errno(_("could not open '%s' for reading"), path); + } +} + /* * xread() is the same a read(), but it automatically restarts read() * operations with a recoverable error (EAGAIN and EINTR). xread() -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 05/45] 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 --- builtin/am.c | 107 +-- 1 file changed, 104 insertions(+), 3 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index ac172c4..5f3c131 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; } /** @@ -117,13 +128,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. @@ -159,9 +228,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-format: %s"), arg); + return 0; +} + int cmd_am(int argc, const char **argv, const char *prefix) { struct am_state state; + int patch_format = PATCH_FORMAT_UNKNOWN; const char * const usage[] = { N_("git am [options] [(|)...]"), @@ -169,6 +254,9 @@ int cmd_am(int argc, const char **argv, const char *prefix) }; struct option options[] = {
[PATCH v7 10/45] builtin-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 in builtin/am.c. Signed-off-by: Paul Tan --- builtin/am.c | 45 + 1 file changed, 45 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index a2811b6..537ad62 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -14,6 +14,8 @@ #include "cache-tree.h" #include "refs.h" #include "commit.h" +#include "diff.h" +#include "diffcore.h" /** * Returns 1 if the file is empty or does not exist, 0 otherwise. @@ -565,6 +567,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 `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 @@ -726,9 +765,15 @@ static void do_commit(const struct am_state *state) static void am_run(struct am_state *state) { const char *argv_gc_auto[] = {"gc", "--auto", NULL}; + 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 *mail = am_path(state, msgnum(state)); -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 06/45] 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 --- builtin/am.c | 109 +++ 1 file changed, 109 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 5f3c131..c12566a 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 @@ -128,6 +143,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. */ @@ -185,6 +286,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) { + fprintf_ln(stderr, _("Patch format detection failed.")); + exit(128); + } + if (mkdir(state->dir, 0777) < 0 && errno != EEXIST) die_errno(_("failed to create directory '%s'"), state->dir); -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 02/45] 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. Helped-by: Jeff King Helped-by: Johannes Schindelin Helped-by: Junio C Hamano Signed-off-by: Paul Tan --- git-compat-util.h | 1 + wrapper.c | 21 + 2 files changed, 22 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 0a4502d..e451463 100644 --- a/wrapper.c +++ b/wrapper.c @@ -346,6 +346,27 @@ 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) +{ + for (;;) { + FILE *fp = fopen(path, mode); + if (fp) + return fp; + if (errno == EINTR) + continue; + + if (*mode && mode[1] == '+') + die_errno(_("could not open '%s' for reading and writing"), path); + else if (*mode == 'w' || *mode == 'a') + die_errno(_("could not open '%s' for writing"), path); + else + die_errno(_("could not open '%s' for reading"), path); + } +} + FILE *xfdopen(int fd, const char *mode) { FILE *stream = fdopen(fd, mode); -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 08/45] 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 --- builtin/am.c | 72 +++- 1 file changed, 71 insertions(+), 1 deletion(-) diff --git a/builtin/am.c b/builtin/am.c index 63f0fa4..1f198e4 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 @@ -540,6 +549,19 @@ static const char *msgnum(const struct am_state *state) } /** + * Refresh and write index. + */ +static void refresh_and_write_cache(void) +{ + struct lock_file *lock_file = xcalloc(1, sizeof(struct 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")); +} + +/** * 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 @@ -629,10 +651,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)); @@ -645,7 +692,27 @@ static void am_run(struct am_state *state) write_author_script(state); write_commit_msg(state); - /* NEEDSWORK: 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); + } + + /* +* NEEDSWORK: After the patch has been applied to the index +* with git-apply, we need to make commit as well. +*/ next: am_next(state); @@ -707,6 +774,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.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 14/45] 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(). Helped-by: Stefan Beller Signed-off-by: Paul Tan --- Notes: v7 * Add a free(curr_branch) so we don't leak memory. builtin/am.c | 103 +-- 1 file changed, 100 insertions(+), 3 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 765844b..6c24d07 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -490,6 +490,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); @@ -506,6 +508,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. @@ -522,6 +532,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]; + free(state->author_name); state->author_name = NULL; @@ -538,6 +550,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); } @@ -788,10 +805,14 @@ static void am_run(struct am_state *state, int resume) 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); @@ -980,6 +1001,75 @@ 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; + char *curr_branch; + + if (!safe_to_abort(state))
[PATCH v7 16/45] builtin-am: implement -q/--quiet
Since 0e987a1 (am, rebase: teach quiet option, 2009-06-16), git-am supported the --quiet option, 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 | 31 --- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index d4b4b86..0875e69 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -80,6 +80,9 @@ struct am_state { /* number of digits in patch filename */ int prec; + + /* various operating modes and command line options */ + int quiet; }; /** @@ -117,6 +120,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) @@ -330,6 +349,9 @@ static void am_load(struct am_state *state) read_commit_msg(state); + read_state_file(&sb, state, "quiet", 1); + state->quiet = !strcmp(sb.buf, "t"); + strbuf_release(&sb); } @@ -508,6 +530,8 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, die(_("Failed to split patches.")); } + 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); @@ -756,7 +780,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, state->author_email, @@ -833,7 +857,7 @@ static void am_run(struct am_state *state, int resume) write_commit_msg(state); } - printf_ln(_("Applying: %.*s"), linelen(state->msg), state->msg); + say(state, stdout, _("Applying: %.*s"), linelen(state->msg), state->msg); if (run_apply(state) < 0) { int advice_amworkdir = 1; @@ -869,7 +893,7 @@ static void am_resolve(struct am_state *state) { validate_resume_state(state); - printf_ln(_("Applying: %.*s"), linelen(state->msg), state->msg); + say(state, stdout, _("Applying: %.*s"), linelen(state->msg), state->msg); if (!index_has_changes(NULL)) { printf_ln(_("No changes - did you forget to use 'git add'?\n" @@ -1105,6 +1129,7 @@ int cmd_am(int argc, const char **argv, const char *prefix) }; struct option options[] = { + OPT__QUIET(&state.quiet, N_("be quiet")), OPT_CALLBACK(0, "patch-format", &patch_format, N_("format"), N_("format the patch(es) are in"), parse_opt_patchformat), -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 13/45] 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(). Helped-by: Stefan Beller Signed-off-by: Paul Tan --- builtin/am.c | 123 ++- 1 file changed, 121 insertions(+), 2 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index ec579a6..765844b 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. @@ -868,6 +870,116 @@ 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; + struct unpack_trees_options opts; + struct tree_desc t[2]; + + if (parse_tree(head) || parse_tree(remote)) + return -1; + + lock_file = xcalloc(1, sizeof(struct lock_file)); + 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; + 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)); + + lock_file = xcalloc(1, sizeof(struct lock_file)); + 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, 0); +} + +/** * parse_options() callback that validates and sets opt->value to the * PATCH_FORMAT_* enum value corresponding to `arg`. */ @@ -885,7 +997,8 @@ static int parse_opt_patchformat(const struct option *opt, const char *arg, int enum resume_mode { RESUME_FALSE = 0, RESUME_APPLY, - RESUME_RESOLVED + RESUME_RESOLVED, + RESUME_SKIP }; int cmd_am(int argc, const char **argv, const char *prefix) @@ -896,7 +1009,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 | -
[PATCH v7 40/45] builtin-am: support and auto-detect StGit series files
Since c574e68 (git-am foreign patch support: StGIT support, 2009-05-27), git-am.sh is able to read a single StGit series file and, for each StGit patch listed in the file, convert the StGit patch into a RFC2822 mail patch suitable for parsing with git-mailinfo, and queue them in the state directory for applying. Since 15ced75 (git-am foreign patch support: autodetect some patch formats, 2009-05-27), git-am.sh is able to auto-detect StGit series files by checking to see if the file starts with the string: # This series applies on GIT commit Re-implement the above in builtin/am.c. Signed-off-by: Paul Tan --- builtin/am.c | 59 ++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/builtin/am.c b/builtin/am.c index d82d07e..3c2ec15 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -80,7 +80,8 @@ static int str_isspace(const char *str) enum patch_format { PATCH_FORMAT_UNKNOWN = 0, PATCH_FORMAT_MBOX, - PATCH_FORMAT_STGIT + PATCH_FORMAT_STGIT, + PATCH_FORMAT_STGIT_SERIES }; enum keep_type { @@ -650,6 +651,11 @@ static int detect_patch_format(const char **paths) goto done; } + if (starts_with(l1.buf, "# This series applies on GIT commit")) { + ret = PATCH_FORMAT_STGIT_SERIES; + goto done; + } + strbuf_reset(&l2); strbuf_getline_crlf(&l2, fp); strbuf_reset(&l3); @@ -801,6 +807,53 @@ static int stgit_patch_to_mail(FILE *out, FILE *in, int keep_cr) } /** + * This function only supports a single StGit series file in `paths`. + * + * Given an StGit series file, converts the StGit patches in the series into + * RFC2822 messages suitable for parsing with git-mailinfo, and queues them in + * the state directory. + * + * Returns 0 on success, -1 on failure. + */ +static int split_mail_stgit_series(struct am_state *state, const char **paths, + int keep_cr) +{ + const char *series_dir; + char *series_dir_buf; + FILE *fp; + struct argv_array patches = ARGV_ARRAY_INIT; + struct strbuf sb = STRBUF_INIT; + int ret; + + if (!paths[0] || paths[1]) + return error(_("Only one StGIT patch series can be applied at once")); + + series_dir_buf = xstrdup(*paths); + series_dir = dirname(series_dir_buf); + + fp = fopen(*paths, "r"); + if (!fp) + return error(_("could not open '%s' for reading: %s"), *paths, + strerror(errno)); + + while (!strbuf_getline(&sb, fp, '\n')) { + if (*sb.buf == '#') + continue; /* skip comment lines */ + + argv_array_push(&patches, mkpath("%s/%s", series_dir, sb.buf)); + } + + fclose(fp); + strbuf_release(&sb); + free(series_dir_buf); + + ret = split_mail_conv(stgit_patch_to_mail, state, patches.argv, keep_cr); + + argv_array_clear(&patches); + return ret; +} + +/** * 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`. @@ -830,6 +883,8 @@ static int split_mail(struct am_state *state, enum patch_format patch_format, return split_mail_mbox(state, paths, keep_cr); case PATCH_FORMAT_STGIT: return split_mail_conv(stgit_patch_to_mail, state, paths, keep_cr); + case PATCH_FORMAT_STGIT_SERIES: + return split_mail_stgit_series(state, paths, keep_cr); default: die("BUG: invalid patch_format"); } @@ -1880,6 +1935,8 @@ static int parse_opt_patchformat(const struct option *opt, const char *arg, int *opt_value = PATCH_FORMAT_MBOX; else if (!strcmp(arg, "stgit")) *opt_value = PATCH_FORMAT_STGIT; + else if (!strcmp(arg, "stgit-series")) + *opt_value = PATCH_FORMAT_STGIT_SERIES; else return error(_("Invalid value for --patch-format: %s"), arg); return 0; -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 21/45] builtin-am: implement --rebasing mode
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 | 30 ++ 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index a5d5e8c..440a653 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -89,6 +89,7 @@ struct am_state { int quiet; int signoff; const char *resolvemsg; + int rebasing; }; /** @@ -364,6 +365,8 @@ static void am_load(struct am_state *state) read_state_file(&sb, state, "sign", 1); state->signoff = !strcmp(sb.buf, "t"); + state->rebasing = !!file_exists(am_path(state, "rebasing")); + strbuf_release(&sb); } @@ -542,18 +545,29 @@ 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, "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->signoff ? "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); } /* @@ -1054,8 +1068,14 @@ next: am_next(state); } - am_destroy(state); - run_command_v_opt(argv_gc_auto, RUN_GIT_CMD); + /* +* In rebasing mode, it's up to the caller to take care of +* housekeeping. +*/ + if (!state->rebasing) { + am_destroy(state); + run_command_v_opt(argv_gc_auto, RUN_GIT_CMD); + } } /** @@ -1325,6 +1345,8 @@ int cmd_am(int argc, const char **argv, const char *prefix) OPT_CMDMODE(0, "abort", &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.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 32/45] builtin-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. Helped-by: Stefan Beller 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 1561580..18611fa 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -110,6 +110,7 @@ struct am_state { const char *resolvemsg; int committer_date_is_author_date; int ignore_date; + const char *sign_commit; int rebasing; }; @@ -119,6 +120,8 @@ struct am_state { */ static void am_state_init(struct am_state *state, const char *dir) { + int gpgsign; + memset(state, 0, sizeof(*state)); assert(dir); @@ -133,6 +136,9 @@ static void am_state_init(struct am_state *state, const char *dir) state->scissors = SCISSORS_UNSET; argv_array_init(&state->git_apply_opts); + + if (!git_config_get_bool("commit.gpgsign", &gpgsign)) + state->sign_commit = gpgsign ? "" : NULL; } /** @@ -1227,7 +1233,7 @@ static void do_commit(const struct am_state *state) state->ignore_date ? "" : state->author_date, 1); if (commit_tree(state->msg, state->msg_len, tree, parents, commit, - author, NULL)) + author, state->sign_commit)) die(_("failed to write commit object")); reflog_msg = getenv("GIT_REFLOG_ACTION"); @@ -1673,6 +1679,9 @@ int cmd_am(int argc, const char **argv, const char *prefix) 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.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 27/45] builtin-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 | 29 +++-- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 8e97839..e34bc51 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -502,7 +502,7 @@ done: * 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) +static int split_mail_mbox(struct am_state *state, const char **paths, int keep_cr) { struct child_process cp = CHILD_PROCESS_INIT; struct strbuf last = STRBUF_INIT; @@ -512,6 +512,8 @@ static int split_mail_mbox(struct am_state *state, const char **paths) argv_array_pushf(&cp.args, "-d%d", state->prec); argv_array_pushf(&cp.args, "-o%s", state->dir); argv_array_push(&cp.args, "-b"); + if (keep_cr) + argv_array_push(&cp.args, "--keep-cr"); argv_array_push(&cp.args, "--"); argv_array_pushv(&cp.args, paths); @@ -536,14 +538,22 @@ static int split_mail_mbox(struct am_state *state, const char **paths) * 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. * + * 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_mail(struct am_state *state, enum patch_format patch_format, - const char **paths) + const char **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_mail_mbox(state, paths); + return split_mail_mbox(state, paths, keep_cr); default: die("BUG: invalid patch_format"); } @@ -554,7 +564,7 @@ static int split_mail(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, - const char **paths) + const char **paths, int keep_cr) { unsigned char curr_head[GIT_SHA1_RAWSZ]; const char *str; @@ -570,7 +580,7 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, 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) { + if (split_mail(state, patch_format, paths, keep_cr) < 0) { am_destroy(state); die(_("Failed to split patches.")); } @@ -1511,6 +1521,7 @@ enum resume_mode { int cmd_am(int argc, const char **argv, const char *prefix) { struct am_state state; + int keep_cr = -1; int patch_format = PATCH_FORMAT_UNKNOWN; enum resume_mode resume = RESUME_FALSE; @@ -1534,6 +1545,12 @@ int cmd_am(int argc, const char **argv, const char *prefix) 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", &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", &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", &patch_format, N_("format"), N_("format the patch(es) are in"), parse_opt_patchformat), @@ -1631,7 +1648,7 @@ int cmd_am(int argc, const char **argv, const char *prefix) argv_array_push(&paths, mkpath("%s/%s", prefix, argv[i]));
[PATCH v7 07/45] 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 --- builtin/am.c | 317 +++ 1 file changed, 317 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index c12566a..63f0fa4 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; }; @@ -62,6 +86,10 @@ static void am_state_init(struct am_state *state, const char *dir) static void am_state_release(struct am_state *state) { free(state->dir); + free(state->author_name); + free(state->author_email); + free(state->author_date); + free(state->msg); } /** @@ -112,6 +140,161 @@ 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 our parsing, as the file was meant to be eval'd in the old git-am.sh + * script, and thus if the file differs from what this function expects, it is + * better to bail out than to do something that the user does not expect. + */ +static int read_author_script(struct am_state *state) +{ + const char *filename = am_path(state, "author-script"); + FILE *fp; + + assert(!state->author_name); + assert(!state->author_email); + assert(!state->author_date); + + fp = fopen(filename, "r"); + if (!fp) { + if (errno == ENOENT) + return 0; + die_errno(_("could not open '%s' for reading"), filename); + } + + state->author_name = read_shell_var(fp, "GIT_AUTHOR_NAME"); + if (!state->author_name) { + fclose(fp); + return -1; + } + + state->author_email = read_shell_var(fp, "GIT_AUTHOR_EMAIL"); + if (!state->author_email) { + fclose(fp); + return -1; + } + + state->author_date = read_shell_var(fp, "GIT_AUTHOR_DATE"); + if (!state->author_date) { + fclose(fp); + return -1; + } + + if (fgetc(fp) != EOF) { + fclose(fp); + return -1; + } + + fclose(fp); + return 0
[PATCH v7 28/45] builtin-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 in builtin/am.c. Since the default setting of --scissors in git-mailinfo can be configured with mailinfo.scissors (and perhaps through other settings in the future), to be safe we make an explicit distinction between SCISSORS_UNSET, SCISSORS_TRUE and SCISSORS_FALSE. Signed-off-by: Paul Tan --- builtin/am.c | 48 1 file changed, 48 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index e34bc51..727cfb8 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -74,6 +74,12 @@ enum keep_type { KEEP_NON_PATCH /* pass -b flag to git-mailinfo */ }; +enum scissors_type { + SCISSORS_UNSET = -1, + SCISSORS_FALSE = 0, /* pass --no-scissors to git-mailinfo */ + SCISSORS_TRUE/* pass --scissors to git-mailinfo */ +}; + struct am_state { /* state directory path */ char *dir; @@ -99,6 +105,7 @@ struct am_state { int utf8; int keep; /* enum keep_type */ int message_id; + int scissors; /* enum scissors_type */ const char *resolvemsg; int rebasing; }; @@ -119,6 +126,8 @@ static void am_state_init(struct am_state *state, const char *dir) state->utf8 = 1; git_config_get_bool("am.messageid", &state->message_id); + + state->scissors = SCISSORS_UNSET; } /** @@ -394,6 +403,14 @@ static void am_load(struct am_state *state) read_state_file(&sb, state, "messageid", 1); state->message_id = !strcmp(sb.buf, "t"); + read_state_file(&sb, state, "scissors", 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); @@ -614,6 +631,22 @@ 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 @@ -798,6 +831,19 @@ static int parse_mail(struct am_state *state, const char *mail) 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")); @@ -1551,6 +1597,8 @@ int cmd_am(int argc, const char **argv, const char *prefix) { OPTION_SET_INT, 0, "no-keep-cr", &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", &patch_format, N_("format"), N_("format the patch(es) are in"), parse_opt_patchformat), -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 38/45] builtin-am: rerere support
git-am.sh will call git-rerere at the following events: * "git rerere" when a three-way merge fails to record the conflicted automerge results. Since 8389b52 (git-rerere: reuse recorded resolve., 2006-01-28) * Since cb6020b (Teach --[no-]rerere-autoupdate option to merge, revert and friends, 2009-12-04), git-am.sh supports the --[no-]rerere-autoupdate option as well, and would pass it to git-rerere. * "git rerere" when --resolved, to record the hand resolution. Since f131dd4 (rerere: record (or avoid misrecording) resolved, skipped or aborted rebase/am, 2006-12-08) * "git rerere clear" when --skip-ing. Since f131dd4 (rerere: record (or avoid misrecording) resolved, skipped or aborted rebase/am, 2006-12-08) * "git rerere clear" when --abort-ing. Since 3e5057a (git am --abort, 2008-07-16) Re-implement the above in builtin/am.c. Signed-off-by: Paul Tan --- builtin/am.c | 25 + 1 file changed, 25 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index c313e58..33d1f24 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -24,6 +24,7 @@ #include "revision.h" #include "log-tree.h" #include "notes-utils.h" +#include "rerere.h" /** * Returns 1 if the file is empty or does not exist, 0 otherwise. @@ -114,6 +115,7 @@ struct am_state { const char *resolvemsg; int committer_date_is_author_date; int ignore_date; + int allow_rerere_autoupdate; const char *sign_commit; int rebasing; }; @@ -1312,6 +1314,7 @@ static int fall_back_threeway(const struct am_state *state, const char *index_pa o.verbosity = 0; if (merge_recursive_generic(&o, our_tree, his_tree, 1, bases, &result)) { + rerere(state->allow_rerere_autoupdate); free(his_tree_name); return error(_("Failed to merge in the changes.")); } @@ -1531,6 +1534,8 @@ static void am_resolve(struct am_state *state) die_user_resolve(state); } + rerere(0); + do_commit(state); am_next(state); @@ -1631,12 +1636,29 @@ static int clean_index(const unsigned char *head, const unsigned char *remote) } /** + * Resets rerere's merge resolution metadata. + */ +static void am_rerere_clear(void) +{ + struct string_list merge_rr = STRING_LIST_INIT_DUP; + int fd = setup_rerere(&merge_rr, 0); + + if (fd < 0) + return; + + rerere_clear(&merge_rr); + string_list_clear(&merge_rr, 1); +} + +/** * Resume the current am session by skipping the current patch. */ static void am_skip(struct am_state *state) { unsigned char head[GIT_SHA1_RAWSZ]; + am_rerere_clear(); + if (get_sha1("HEAD", head)) hashcpy(head, EMPTY_TREE_SHA1_BIN); @@ -1694,6 +1716,8 @@ static void am_abort(struct am_state *state) return; } + am_rerere_clear(); + curr_branch = resolve_refdup("HEAD", 0, curr_head, NULL); has_curr_head = !is_null_sha1(curr_head); if (!has_curr_head) @@ -1823,6 +1847,7 @@ int cmd_am(int argc, const char **argv, const char *prefix) N_("lie about committer date")), OPT_BOOL(0, "ignore-date", &state.ignore_date, N_("use current timestamp for author date")), + OPT_RERERE_AUTOUPDATE(&state.allow_rerere_autoupdate), { OPTION_STRING, 'S', "gpg-sign", &state.sign_commit, N_("key-id"), N_("GPG-sign commits"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 12/45] builtin-am: don't parse mail when resuming
Since 271440e (git-am: make it easier after fixing up an unapplicable patch., 2005-10-25), when "git am" is run again after being paused, the current mail message will not be re-parsed, but instead the contents of the state directory's patch, msg and author-script files will be used as-is instead. Re-implement this in builtin/am.c. Signed-off-by: Paul Tan --- builtin/am.c | 34 +- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index fd26721..ec579a6 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -776,8 +776,12 @@ static void validate_resume_state(const struct am_state *state) /** * Applies all queued mail. + * + * If `resume` is true, we are "resuming". The "msg" and authorship fields, as + * well as the state directory's "patch" file is used as-is for applying the + * patch and committing it. */ -static void am_run(struct am_state *state) +static void am_run(struct am_state *state, int resume) { const char *argv_gc_auto[] = {"gc", "--auto", NULL}; struct strbuf sb = STRBUF_INIT; @@ -795,11 +799,16 @@ static void am_run(struct am_state *state) if (!file_exists(mail)) goto next; - if (parse_mail(state, mail)) - goto next; /* mail should be skipped */ + if (resume) { + validate_resume_state(state); + resume = 0; + } else { + if (parse_mail(state, mail)) + goto next; /* mail should be skipped */ - write_author_script(state); - write_commit_msg(state); + write_author_script(state); + write_commit_msg(state); + } printf_ln(_("Applying: %.*s"), linelen(state->msg), state->msg); @@ -855,7 +864,7 @@ static void am_resolve(struct am_state *state) do_commit(state); am_next(state); - am_run(state); + am_run(state, 0); } /** @@ -875,6 +884,7 @@ static int parse_opt_patchformat(const struct option *opt, const char *arg, int enum resume_mode { RESUME_FALSE = 0, + RESUME_APPLY, RESUME_RESOLVED }; @@ -927,9 +937,12 @@ 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)) { + if (resume == RESUME_FALSE) + resume = RESUME_APPLY; + am_load(&state); - else { + } else { struct argv_array paths = ARGV_ARRAY_INIT; int i; @@ -950,7 +963,10 @@ int cmd_am(int argc, const char **argv, const char *prefix) switch (resume) { case RESUME_FALSE: - am_run(&state); + am_run(&state, 0); + break; + case RESUME_APPLY: + am_run(&state, 1); break; case RESUME_RESOLVED: am_resolve(&state); -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 17/45] 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 | 30 ++ 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 0875e69..8b8f2da 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -83,6 +83,7 @@ struct am_state { /* various operating modes and command line options */ int quiet; + const char *resolvemsg; }; /** @@ -647,6 +648,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 @@ -706,7 +726,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"); @@ -871,7 +891,7 @@ static void am_run(struct am_state *state, int resume) 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); @@ -899,13 +919,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); @@ -1133,6 +1153,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.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 26/45] builtin-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 --- builtin/am.c | 13 + 1 file changed, 13 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 68dca2e..8e97839 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -98,6 +98,7 @@ struct am_state { int signoff; int utf8; int keep; /* enum keep_type */ + int message_id; const char *resolvemsg; int rebasing; }; @@ -116,6 +117,8 @@ static void am_state_init(struct am_state *state, const char *dir) state->prec = 4; state->utf8 = 1; + + git_config_get_bool("am.messageid", &state->message_id); } /** @@ -388,6 +391,9 @@ static void am_load(struct am_state *state) else state->keep = KEEP_FALSE; + read_state_file(&sb, state, "messageid", 1); + state->message_id = !strcmp(sb.buf, "t"); + state->rebasing = !!file_exists(am_path(state, "rebasing")); strbuf_release(&sb); @@ -596,6 +602,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 @@ -777,6 +785,9 @@ static int parse_mail(struct am_state *state, const char *mail) 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")); @@ -1521,6 +1532,8 @@ int cmd_am(int argc, const char **argv, const char *prefix) 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", &patch_format, N_("format"), N_("format the patch(es) are in"), parse_opt_patchformat), -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 31/45] builtin-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 | 8 1 file changed, 8 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 84d3e05..1561580 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -108,6 +108,7 @@ struct am_state { int scissors; /* enum scissors_type */ struct argv_array git_apply_opts; const char *resolvemsg; + int committer_date_is_author_date; int ignore_date; int rebasing; }; @@ -1221,6 +1222,10 @@ static void do_commit(const struct am_state *state) state->ignore_date ? NULL : state->author_date, IDENT_STRICT); + if (state->committer_date_is_author_date) + setenv("GIT_COMMITTER_DATE", + state->ignore_date ? "" : state->author_date, 1); + if (commit_tree(state->msg, state->msg_len, tree, parents, commit, author, NULL)) die(_("failed to write commit object")); @@ -1663,6 +1668,9 @@ int cmd_am(int argc, const char **argv, const char *prefix) OPT_CMDMODE(0, "abort", &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.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 18/45] builtin-am: implement -s/--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 in parse_mail() by calling append_signoff() if the option is set. Signed-off-by: Paul Tan --- Notes: v7 * Having a field named "append_signoff" takes up a lot of horizontal space when referring to it. Shorten the name to just "signoff". builtin/am.c | 12 1 file changed, 12 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 8b8f2da..12952cf 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. @@ -83,6 +84,7 @@ struct am_state { /* various operating modes and command line options */ int quiet; + int signoff; const char *resolvemsg; }; @@ -353,6 +355,9 @@ static void am_load(struct am_state *state) read_state_file(&sb, state, "quiet", 1); state->quiet = !strcmp(sb.buf, "t"); + read_state_file(&sb, state, "sign", 1); + state->signoff = !strcmp(sb.buf, "t"); + strbuf_release(&sb); } @@ -533,6 +538,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->signoff ? "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); @@ -734,6 +741,9 @@ static int parse_mail(struct am_state *state, const char *mail) die_errno(_("could not read '%s'"), am_path(state, "msg")); stripspace(&msg, 0); + if (state->signoff) + append_signoff(&msg, 0, 0); + assert(!state->author_name); state->author_name = strbuf_detach(&author_name, NULL); @@ -1150,6 +1160,8 @@ int cmd_am(int argc, const char **argv, const char *prefix) struct option options[] = { OPT__QUIET(&state.quiet, N_("be quiet")), + OPT_BOOL('s', "signoff", &state.signoff, + N_("add a Signed-off-by line to the commit message")), OPT_CALLBACK(0, "patch-format", &patch_format, N_("format"), N_("format the patch(es) are in"), parse_opt_patchformat), -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 15/45] 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 --- builtin/am.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 6c24d07..d4b4b86 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1148,6 +1148,21 @@ int cmd_am(int argc, const char **argv, const char *prefix) die(_("failed to read the index")); 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); + if (resume == RESUME_FALSE) resume = RESUME_APPLY; -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 35/45] builtin-am: invoke applypatch-msg hook
Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07), git-am.sh will invoke the applypatch-msg hooks just after extracting the patch message. If the applypatch-msg hook exits with a non-zero status, git-am.sh abort before even applying the patch to the index. Re-implement this in builtin/am.c. Signed-off-by: Paul Tan --- builtin/am.c | 24 1 file changed, 24 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 7d7f91d..f0e3aab 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -456,6 +456,27 @@ static void am_destroy(const struct am_state *state) } /** + * Runs applypatch-msg hook. Returns its exit code. + */ +static int run_applypatch_msg_hook(struct am_state *state) +{ + int ret; + + assert(state->msg); + ret = run_hook_le(NULL, "applypatch-msg", am_path(state, "final-commit"), NULL); + + if (!ret) { + free(state->msg); + state->msg = NULL; + if (read_commit_msg(state) < 0) + die(_("'%s' was deleted by the applypatch-msg hook"), + am_path(state, "final-commit")); + } + + return ret; +} + +/** * Runs post-rewrite hook. Returns it exit code. */ static int run_post_rewrite_hook(const struct am_state *state) @@ -1420,6 +1441,9 @@ static void am_run(struct am_state *state, int resume) write_commit_msg(state); } + if (run_applypatch_msg_hook(state)) + exit(1); + say(state, stdout, _("Applying: %.*s"), linelen(state->msg), state->msg); apply_status = run_apply(state, NULL); -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 24/45] 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 | 11 +++ 1 file changed, 11 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 47dd4c7..528b2c9 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -90,6 +90,7 @@ struct am_state { int threeway; int quiet; int signoff; + int utf8; const char *resolvemsg; int rebasing; }; @@ -106,6 +107,8 @@ static void am_state_init(struct am_state *state, const char *dir) state->dir = xstrdup(dir); state->prec = 4; + + state->utf8 = 1; } /** @@ -367,6 +370,9 @@ static void am_load(struct am_state *state) read_state_file(&sb, state, "sign", 1); state->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); @@ -556,6 +562,8 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, write_file(am_path(state, "sign"), 1, state->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 @@ -722,6 +730,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")); @@ -1460,6 +1469,8 @@ int cmd_am(int argc, const char **argv, const char *prefix) OPT__QUIET(&state.quiet, N_("be quiet")), OPT_BOOL('s', "signoff", &state.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.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 45/45] builtin-am: remove redirection to git-am.sh
At the beginning of the rewrite of git-am.sh to C, in order to not break existing test scripts that depended on a functional git-am, a redirection to git-am.sh was introduced that would activate if the environment variable _GIT_USE_BUILTIN_AM was not defined. Now that all of git-am.sh's functionality has been re-implemented in builtin/am.c, remove this redirection, and retire git-am.sh into contrib/examples/. Signed-off-by: Paul Tan --- Makefile| 1 - builtin/am.c| 15 --- git-am.sh => contrib/examples/git-am.sh | 0 git.c | 7 +-- 4 files changed, 1 insertion(+), 22 deletions(-) rename git-am.sh => contrib/examples/git-am.sh (100%) diff --git a/Makefile b/Makefile index da451f8..e39ca6c 100644 --- a/Makefile +++ b/Makefile @@ -467,7 +467,6 @@ TEST_PROGRAMS_NEED_X = # interactive shell sessions without exporting it. unexport CDPATH -SCRIPT_SH += git-am.sh SCRIPT_SH += git-bisect.sh SCRIPT_SH += git-difftool--helper.sh SCRIPT_SH += git-filter-branch.sh diff --git a/builtin/am.c b/builtin/am.c index 1ff74ac..84d57d4 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -2221,21 +2221,6 @@ int cmd_am(int argc, const char **argv, const char *prefix) OPT_END() }; - /* -* 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(); - } - git_config(git_default_config, NULL); am_state_init(&state, git_path("rebase-apply")); diff --git a/git-am.sh b/contrib/examples/git-am.sh similarity index 100% rename from git-am.sh rename to contrib/examples/git-am.sh diff --git a/git.c b/git.c index 38d9ad5..5feba41 100644 --- a/git.c +++ b/git.c @@ -370,12 +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 }, - /* -* 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 }, + { "am", cmd_am, RUN_SETUP | NEED_WORK_TREE }, { "annotate", cmd_annotate, RUN_SETUP }, { "apply", cmd_apply, RUN_SETUP_GENTLY }, { "archive", cmd_archive }, -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 20/45] builtin-am: implement --3way
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 --- Notes: v7 * As support for am.threeWay was removed in master, this patch now does not implement am.threeWay as well. builtin/am.c | 154 +-- 1 file changed, 150 insertions(+), 4 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 12952cf..a5d5e8c 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. @@ -83,6 +85,7 @@ struct am_state { int prec; /* various operating modes and command line options */ + int threeway; int quiet; int signoff; const char *resolvemsg; @@ -352,6 +355,9 @@ static void am_load(struct am_state *state) read_commit_msg(state); + read_state_file(&sb, state, "threeway", 1); + state->threeway = !strcmp(sb.buf, "t"); + read_state_file(&sb, state, "quiet", 1); state->quiet = !strcmp(sb.buf, "t"); @@ -536,6 +542,8 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, die(_("Failed to split patches.")); } + 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->signoff ? "t" : "f"); @@ -766,16 +774,34 @@ finish: } /** - * Applies current patch with git-apply. Returns 0 on success, -1 otherwise. + * Applies current patch with git-apply. Returns 0 on success, -1 otherwise. If + * `index_file` is not NULL, the patch will be applied to that index. */ -static int run_apply(const struct am_state *state) +static int run_apply(const struct am_state *state, const char *index_file) { struct child_process cp = CHILD_PROCESS_INIT; cp.git_cmd = 1; + if (index_file) + argv_array_pushf(&cp.env_array, "GIT_INDEX_FILE=%s", index_file); + + /* +* If we are allowed to fall back on 3-way merge, don't give false +* errors during the initial attempt. +*/ + if (state->threeway && !index_file) { + cp.no_stdout = 1; + cp.no_stderr = 1; + } + argv_array_push(&cp.args, "apply"); - argv_array_push(&cp.args, "--index"); + + if (index_file) + argv_array_push(&cp.args, "--cached"); + else + argv_array_push(&cp.args, "--index"); + argv_array_push(&cp.args, am_path(state, "patch")); if (run_command(&cp)) @@ -783,8 +809,106 @@ 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 an 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; + char *his_tree_name; + + if (get_sha1("HEAD", our_tree) < 0) + hashcpy(our_tree, EMPTY_TREE_SHA1_BIN); + + if (build_fake_ancestor(state, index_path)) + return error("coul
[PATCH v7 22/45] builtin-am: bypass git-mailinfo when --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_mail() code path in git-am.sh's --rebasing mode that bypasses git-mailinfo. Re-implement this code path in builtin/am.c as parse_mail_rebase(). Signed-off-by: Paul Tan --- Notes: v7 * Since a5481a6 (convert "enum date_mode" into a struct, 2015-06-25), show_ident_date() now takes a date_mode struct. Use the DATE_MODE() macro to pass the equivalent date_mode struct to show_ident_date(). builtin/am.c | 134 ++- 1 file changed, 133 insertions(+), 1 deletion(-) diff --git a/builtin/am.c b/builtin/am.c index 440a653..a02c84e 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. @@ -788,6 +790,129 @@ finish: } /** + * Sets commit_id to the commit hash where the mail was generated from. + * Returns 0 on success, -1 on failure. + */ +static int get_mail_commit_sha1(unsigned char *commit_id, const char *mail) +{ + struct strbuf sb = STRBUF_INIT; + FILE *fp = xfopen(mail, "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 *buffer, *ident_line, *author_date, *msg; + size_t ident_len; + struct ident_split ident_split; + struct strbuf sb = STRBUF_INIT; + + buffer = logmsg_reencode(commit, NULL, get_commit_output_encoding()); + + ident_line = find_commit_header(buffer, "author", &ident_len); + + if (split_ident_line(&ident_split, ident_line, ident_len) < 0) { + strbuf_add(&sb, ident_line, ident_len); + die(_("invalid ident line: %s"), sb.buf); + } + + assert(!state->author_name); + if (ident_split.name_begin) { + strbuf_add(&sb, ident_split.name_begin, + ident_split.name_end - ident_split.name_begin); + state->author_name = strbuf_detach(&sb, NULL); + } else + state->author_name = xstrdup(""); + + assert(!state->author_email); + if (ident_split.mail_begin) { + strbuf_add(&sb, ident_split.mail_begin, + ident_split.mail_end - ident_split.mail_begin); + state->author_email = strbuf_detach(&sb, NULL); + } else + state->author_email = xstrdup(""); + + author_date = show_ident_date(&ident_split, DATE_MODE(NORMAL)); + strbuf_addstr(&sb, author_date); + assert(!state->author_date); + state->author_date = strbuf_detach(&sb, NULL); + + assert(!state->msg); + msg = strstr(buffer, "\n\n"); + if (!msg) + die(_("unable to parse commit %s"), sha1_to_hex(commit->object.sha1)); + state->msg = xstrdup(msg + 2); + state->msg_len = strlen(state->msg); +} + +/** + * Writes `commit` as a patch to the state directory's "patch" file. + */ +static void write_commit_patch(const struct am_state *state, struct commit *commit) +{ + struct rev_info rev_info; + FILE *fp; + + fp = xfopen(am_path(state, "patch"), "w"); + init_revisions(&rev_info, NULL); + rev_info.diff = 1; + rev_info.abbrev = 0; + rev_info.disable_stdin = 1; + rev_info.show_root_diff = 1; + rev_info.diffopt.output_format = DIFF_FORMAT_PATCH; + rev_info.no_commit_id = 1; + DIFF_OPT_SET(&rev_info.diffopt, BINARY); + DIFF_OPT_SET(&rev_info.diffopt, FULL_INDEX); +
[PATCH v7 36/45] builtin-am: invoke pre-applypatch hook
Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07), git-am.sg will invoke the pre-applypatch hook after applying the patch to the index, but before a commit is made. Should the hook exit with a non-zero status, git am will exit. Re-implement this in builtin/am.c. Signed-off-by: Paul Tan --- builtin/am.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index f0e3aab..7a7da94 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1334,6 +1334,9 @@ static void do_commit(const struct am_state *state) const char *reflog_msg, *author; struct strbuf sb = STRBUF_INIT; + if (run_hook_le(NULL, "pre-applypatch", NULL)) + exit(1); + if (write_cache_as_tree(tree, 0, NULL)) die(_("git write-tree failed to write a tree")); -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 23/45] builtin-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 | 17 + 1 file changed, 17 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index a02c84e..47dd4c7 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1530,6 +1530,23 @@ int cmd_am(int argc, const char **argv, const char *prefix) struct argv_array paths = ARGV_ARRAY_INIT; int i; + /* +* Handle stray state directory in the independent-run case. In +* the --rebasing case, it is up to the caller to take care of +* stray directories. +*/ + if (file_exists(state.dir) && !state.rebasing) { + if (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); + } + if (resume) die(_("Resolve operation not in progress, we are not resuming.")); -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 33/45] builtin-am: invoke post-rewrite hook
Since 96e1948 (rebase: invoke post-rewrite hook, 2010-03-12), git-am.sh will invoke the post-rewrite hook after it successfully finishes applying all the queued patches. To do this, when parsing a mail to extract its patch and metadata, in --rebasing mode git-am.sh will also store the original commit ID in the $state_dir/original-commit file. Once it applies and commits the patch, the original commit ID, and the new commit ID, will be appended to the $state_dir/rewritten file. Once all of the queued mail have been processed, git-am.sh will then invoke the post-rewrite hook with the contents of the $state_dir/rewritten file. Re-implement this in builtin/am.c. Signed-off-by: Paul Tan --- builtin/am.c | 55 +++ 1 file changed, 55 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 18611fa..dbec9fc 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -95,6 +95,9 @@ struct am_state { char *msg; size_t msg_len; + /* when --rebasing, records the original commit the patch came from */ + unsigned char orig_commit[GIT_SHA1_RAWSZ]; + /* number of digits in patch filename */ int prec; @@ -392,6 +395,11 @@ static void am_load(struct am_state *state) read_commit_msg(state); + if (read_state_file(&sb, state, "original-commit", 1) < 0) + hashclr(state->orig_commit); + else if (get_sha1_hex(sb.buf, state->orig_commit) < 0) + die(_("could not parse %s"), am_path(state, "original-commit")); + read_state_file(&sb, state, "threeway", 1); state->threeway = !strcmp(sb.buf, "t"); @@ -447,6 +455,30 @@ static void am_destroy(const struct am_state *state) } /** + * Runs post-rewrite hook. Returns it exit code. + */ +static int run_post_rewrite_hook(const struct am_state *state) +{ + struct child_process cp = CHILD_PROCESS_INIT; + const char *hook = find_hook("post-rewrite"); + int ret; + + if (!hook) + return 0; + + argv_array_push(&cp.args, hook); + argv_array_push(&cp.args, "rebase"); + + cp.in = xopen(am_path(state, "rewritten"), O_RDONLY); + cp.stdout_to_stderr = 1; + + ret = run_command(&cp); + + close(cp.in); + return ret; +} + +/** * 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. @@ -720,6 +752,9 @@ static void am_next(struct am_state *state) unlink(am_path(state, "author-script")); unlink(am_path(state, "final-commit")); + hashclr(state->orig_commit); + unlink(am_path(state, "original-commit")); + if (!get_sha1("HEAD", head)) write_file(am_path(state, "abort-safety"), 1, "%s", sha1_to_hex(head)); else @@ -1038,6 +1073,8 @@ static void write_commit_patch(const struct am_state *state, struct commit *comm * directly. This is used in --rebasing mode to bypass git-mailinfo's munging * of patches. * + * state->orig_commit will be set to the original commit ID. + * * Will always return 0 as the patch should never be skipped. */ static int parse_mail_rebase(struct am_state *state, const char *mail) @@ -1054,6 +1091,10 @@ static int parse_mail_rebase(struct am_state *state, const char *mail) write_commit_patch(state, commit); + hashcpy(state->orig_commit, commit_sha1); + write_file(am_path(state, "original-commit"), 1, "%s", + sha1_to_hex(commit_sha1)); + return 0; } @@ -1245,6 +1286,15 @@ static void do_commit(const struct am_state *state) update_ref(sb.buf, "HEAD", commit, ptr, 0, UPDATE_REFS_DIE_ON_ERR); + if (state->rebasing) { + FILE *fp = xfopen(am_path(state, "rewritten"), "a"); + + assert(!is_null_sha1(state->orig_commit)); + fprintf(fp, "%s ", sha1_to_hex(state->orig_commit)); + fprintf(fp, "%s\n", sha1_to_hex(commit)); + fclose(fp); + } + strbuf_release(&sb); } @@ -1353,6 +1403,11 @@ next: am_next(state); } + if (!is_empty_file(am_path(state, "rewritten"))) { + assert(state->rebasing); + run_post_rewrite_hook(state); + } + /* * In rebasing mode, it's up to the caller to take care of * housekeeping. -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 37/45] builtin-am: invoke post-applypatch hook
Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07), git-am.sh will invoke the post-applypatch hook after the patch is applied and a commit is made. The exit code of the hook is ignored. Re-implement this in builtin/am.c. Signed-off-by: Paul Tan --- builtin/am.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 7a7da94..c313e58 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1378,6 +1378,8 @@ static void do_commit(const struct am_state *state) fclose(fp); } + run_hook_le(NULL, "post-applypatch", NULL); + strbuf_release(&sb); } -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 34/45] builtin-am: support automatic notes copying
Since eb2151b (rebase: support automatic notes copying, 2010-03-12), git-am.sh supported automatic notes copying in --rebasing mode by invoking "git notes copy" once it has finished applying all the patches. Re-implement this feature in builtin/am.c. Signed-off-by: Paul Tan --- builtin/am.c | 60 1 file changed, 60 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index dbec9fc..7d7f91d 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -23,6 +23,7 @@ #include "merge-recursive.h" #include "revision.h" #include "log-tree.h" +#include "notes-utils.h" /** * Returns 1 if the file is empty or does not exist, 0 otherwise. @@ -479,6 +480,64 @@ static int run_post_rewrite_hook(const struct am_state *state) } /** + * Reads the state directory's "rewritten" file, and copies notes from the old + * commits listed in the file to their rewritten commits. + * + * Returns 0 on success, -1 on failure. + */ +static int copy_notes_for_rebase(const struct am_state *state) +{ + struct notes_rewrite_cfg *c; + struct strbuf sb = STRBUF_INIT; + const char *invalid_line = _("Malformed input line: '%s'."); + const char *msg = "Notes added by 'git rebase'"; + FILE *fp; + int ret = 0; + + assert(state->rebasing); + + c = init_copy_notes_for_rewrite("rebase"); + if (!c) + return 0; + + fp = xfopen(am_path(state, "rewritten"), "r"); + + while (!strbuf_getline(&sb, fp, '\n')) { + unsigned char from_obj[GIT_SHA1_RAWSZ], to_obj[GIT_SHA1_RAWSZ]; + + if (sb.len != GIT_SHA1_HEXSZ * 2 + 1) { + ret = error(invalid_line, sb.buf); + goto finish; + } + + if (get_sha1_hex(sb.buf, from_obj)) { + ret = error(invalid_line, sb.buf); + goto finish; + } + + if (sb.buf[GIT_SHA1_HEXSZ] != ' ') { + ret = error(invalid_line, sb.buf); + goto finish; + } + + if (get_sha1_hex(sb.buf + GIT_SHA1_HEXSZ + 1, to_obj)) { + ret = error(invalid_line, sb.buf); + goto finish; + } + + if (copy_note_for_rewrite(c, from_obj, to_obj)) + ret = error(_("Failed to copy notes from '%s' to '%s'"), + sha1_to_hex(from_obj), sha1_to_hex(to_obj)); + } + +finish: + finish_copy_notes_for_rewrite(c, msg); + fclose(fp); + strbuf_release(&sb); + return ret; +} + +/** * 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. @@ -1405,6 +1464,7 @@ next: if (!is_empty_file(am_path(state, "rewritten"))) { assert(state->rebasing); + copy_notes_for_rebase(state); run_post_rewrite_hook(state); } -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 30/45] builtin-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 | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/builtin/am.c b/builtin/am.c index f842f69..84d3e05 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -108,6 +108,7 @@ struct am_state { int scissors; /* enum scissors_type */ struct argv_array git_apply_opts; const char *resolvemsg; + int ignore_date; int rebasing; }; @@ -1217,7 +1218,8 @@ static void do_commit(const struct am_state *state) } author = fmt_ident(state->author_name, state->author_email, - state->author_date, IDENT_STRICT); + state->ignore_date ? NULL : state->author_date, + IDENT_STRICT); if (commit_tree(state->msg, state->msg_len, tree, parents, commit, author, NULL)) @@ -1661,6 +1663,8 @@ int cmd_am(int argc, const char **argv, const char *prefix) OPT_CMDMODE(0, "abort", &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.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 19/45] 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.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 39/45] builtin-am: support and auto-detect StGit patches
Since c574e68 (git-am foreign patch support: StGIT support, 2009-05-27), git-am.sh supported converting StGit patches into RFC2822 mail patches that can be parsed with git-mailinfo. Implement this by introducing two functions in builtin/am.c: stgit_patch_to_mail() and split_mail_conv(). stgit_patch_to_mail() is a callback function for split_mail_conv(), and contains the logic for converting an StGit patch into an RFC2822 mail patch. split_mail_conv() implements the logic to go through each file in the `paths` list, reading from stdin where specified, and calls the callback function to write the converted patch to the corresponding output file in the state directory. This interface should be generic enough to support other foreign patch formats in the future. Since 15ced75 (git-am foreign patch support: autodetect some patch formats, 2009-05-27), git-am.sh is able to auto-detect StGit patches. Re-implement this in builtin/am.c. Helped-by: Eric Sunshine Signed-off-by: Paul Tan --- builtin/am.c | 132 ++- 1 file changed, 131 insertions(+), 1 deletion(-) diff --git a/builtin/am.c b/builtin/am.c index 33d1f24..d82d07e 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -65,9 +65,22 @@ static int linelen(const char *msg) return strchrnul(msg, '\n') - msg; } +/** + * Returns true if `str` consists of only whitespace, false otherwise. + */ +static int str_isspace(const char *str) +{ + for (; *str; str++) + if (!isspace(*str)) + return 0; + + return 1; +} + enum patch_format { PATCH_FORMAT_UNKNOWN = 0, - PATCH_FORMAT_MBOX + PATCH_FORMAT_MBOX, + PATCH_FORMAT_STGIT }; enum keep_type { @@ -610,6 +623,8 @@ static int detect_patch_format(const char **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; /* @@ -635,6 +650,23 @@ static int detect_patch_format(const char **paths) goto done; } + strbuf_reset(&l2); + strbuf_getline_crlf(&l2, fp); + strbuf_reset(&l3); + strbuf_getline_crlf(&l3, fp); + + /* +* If the second line is empty and the third is a From, Author or Date +* entry, this is likely an StGit patch. +*/ + if (l1.len && !l2.len && + (starts_with(l3.buf, "From:") || +starts_with(l3.buf, "Author:") || +starts_with(l3.buf, "Date:"))) { + ret = PATCH_FORMAT_STGIT; + goto done; + } + if (l1.len && is_mail(fp)) { ret = PATCH_FORMAT_MBOX; goto done; @@ -675,6 +707,100 @@ static int split_mail_mbox(struct am_state *state, const char **paths, int keep_ } /** + * Callback signature for split_mail_conv(). The foreign patch should be + * read from `in`, and the converted patch (in RFC2822 mail format) should be + * written to `out`. Return 0 on success, or -1 on failure. + */ +typedef int (*mail_conv_fn)(FILE *out, FILE *in, int keep_cr); + +/** + * Calls `fn` for each file in `paths` to convert the foreign patch to the + * RFC2822 mail format suitable for parsing with git-mailinfo. + * + * Returns 0 on success, -1 on failure. + */ +static int split_mail_conv(mail_conv_fn fn, struct am_state *state, + const char **paths, int keep_cr) +{ + static const char *stdin_only[] = {"-", NULL}; + int i; + + if (!*paths) + paths = stdin_only; + + for (i = 0; *paths; paths++, i++) { + FILE *in, *out; + const char *mail; + int ret; + + if (!strcmp(*paths, "-")) + in = stdin; + else + in = fopen(*paths, "r"); + + if (!in) + return error(_("could not open '%s' for reading: %s"), + *paths, strerror(errno)); + + mail = mkpath("%s/%0*d", state->dir, state->prec, i + 1); + + out = fopen(mail, "w"); + if (!out) + return error(_("could not open '%s' for writing: %s"), + mail, strerror(errno)); + + ret = fn(out, in, keep_cr); + + fclose(out); + fclose(in); + + if (ret) + return error(_("could not parse patch '%s'"), *paths); + } + + state->cur = 1; + state->last = i; + return 0; +} + +/** + * A split_mail_conv() callback that converts an StGit patch to an RFC2822 + * message
[PATCH v7 42/45] builtin-am: implement -i/--interactive
Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07), git-am.sh supported the --interactive mode. After parsing the patch mail and extracting the patch, commit message and authorship info, an interactive session will begin that allows the user to choose between: * applying the patch * applying the patch and all subsequent patches (by disabling interactive mode in subsequent patches) * skipping the patch * editing the commit message Since f89ad67 (Add [v]iew patch in git-am interactive., 2005-10-25), git-am.sh --interactive also supported viewing the patch to be applied. When --resolved-ing in --interactive mode, we need to take care to update the patch with the contents of the index, such that the correct patch will be displayed when the patch is viewed in interactive mode. Re-implement the above in builtin/am.c Signed-off-by: Paul Tan --- builtin/am.c | 105 ++- 1 file changed, 104 insertions(+), 1 deletion(-) diff --git a/builtin/am.c b/builtin/am.c index 98c10a0..589199f 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -25,6 +25,7 @@ #include "log-tree.h" #include "notes-utils.h" #include "rerere.h" +#include "prompt.h" /** * Returns 1 if the file is empty or does not exist, 0 otherwise. @@ -119,6 +120,7 @@ struct am_state { int prec; /* various operating modes and command line options */ + int interactive; int threeway; int quiet; int signoff; @@ -1171,7 +1173,7 @@ 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"; + const char *cmdline = state->interactive ? "git am -i" : "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); @@ -1404,6 +1406,36 @@ static void write_commit_patch(const struct am_state *state, struct commit *comm } /** + * Writes the diff of the index against HEAD as a patch to the state + * directory's "patch" file. + */ +static void write_index_patch(const struct am_state *state) +{ + struct tree *tree; + unsigned char head[GIT_SHA1_RAWSZ]; + struct rev_info rev_info; + FILE *fp; + + if (!get_sha1_tree("HEAD", head)) + tree = lookup_tree(head); + else + tree = lookup_tree(EMPTY_TREE_SHA1_BIN); + + fp = xfopen(am_path(state, "patch"), "w"); + init_revisions(&rev_info, NULL); + rev_info.diff = 1; + rev_info.disable_stdin = 1; + rev_info.no_commit_id = 1; + rev_info.diffopt.output_format = DIFF_FORMAT_PATCH; + rev_info.diffopt.use_color = 0; + rev_info.diffopt.file = fp; + rev_info.diffopt.close_file = 1; + add_pending_object(&rev_info, &tree->object, ""); + diff_setup_done(&rev_info.diffopt); + run_diff_index(&rev_info, 1); +} + +/** * Like parse_mail(), but parses the mail by looking up its commit ID * directly. This is used in --rebasing mode to bypass git-mailinfo's munging * of patches. @@ -1655,6 +1687,65 @@ static void validate_resume_state(const struct am_state *state) } /** + * Interactively prompt the user on whether the current patch should be + * applied. + * + * Returns 0 if the user chooses to apply the patch, 1 if the user chooses to + * skip it. + */ +static int do_interactive(struct am_state *state) +{ + assert(state->msg); + + if (!isatty(0)) + die(_("cannot be interactive without stdin connected to a terminal.")); + + for (;;) { + const char *reply; + + puts(_("Commit Body is:")); + puts("--"); + printf("%s", state->msg); + puts("--"); + + /* +* TRANSLATORS: Make sure to include [y], [n], [e], [v] and [a] +* in your translation. The program will only accept English +* input at this point. +*/ + reply = git_prompt(_("Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all: "), PROMPT_ECHO); + + if (!reply) { + continue; + } else if (*reply == 'y' || *reply == 'Y') { + return 0; + } else if (*reply == 'a' || *reply == 'A') { + state->interactive = 0; +
[PATCH v7 44/45] builtin-am: check for valid committer ident
When commit_tree() is called, if the user does not have an explicit committer ident configured, it will attempt to construct a default committer ident based on the user's and system's info (e.g. gecos field, hostname etc.) However, if a default committer ident is unable to be constructed, commit_tree() will die(), but at this point of git-am's execution, there will already be changes made to the index and work tree. This can be confusing to new users, and as such since d64e6b0 (Keep Porcelainish from failing by broken ident after making changes., 2006-02-18) git-am.sh will check to see if the committer ident has been configured, or a default one can be constructed, before even starting to apply patches. Re-implement this in builtin/am.c. Signed-off-by: Paul Tan --- builtin/am.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 3c50392..1ff74ac 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -2246,6 +2246,9 @@ int cmd_am(int argc, const char **argv, const char *prefix) fprintf_ln(stderr, _("The -b/--binary option has been a no-op for long time, and\n" "it will be removed. Please do not use it anymore.")); + /* Ensure a valid committer ident can be constructed */ + git_committer_info(IDENT_STRICT); + if (read_index_preload(&the_index, NULL) < 0) die(_("failed to read the index")); -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 29/45] builtin-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) * --reject, since b80da42 (git-am: implement --reject option passed to git-apply, 2009-01-23) Re-implement support for these options in builtin/am.c. Signed-off-by: Paul Tan --- builtin/am.c | 45 + 1 file changed, 45 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 727cfb8..f842f69 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -106,6 +106,7 @@ struct am_state { int keep; /* enum keep_type */ int message_id; int scissors; /* enum scissors_type */ + struct argv_array git_apply_opts; const char *resolvemsg; int rebasing; }; @@ -128,6 +129,8 @@ static void am_state_init(struct am_state *state, const char *dir) git_config_get_bool("am.messageid", &state->message_id); state->scissors = SCISSORS_UNSET; + + argv_array_init(&state->git_apply_opts); } /** @@ -140,6 +143,7 @@ static void am_state_release(struct am_state *state) free(state->author_email); free(state->author_date); free(state->msg); + argv_array_clear(&state->git_apply_opts); } /** @@ -411,6 +415,11 @@ static void am_load(struct am_state *state) else state->scissors = SCISSORS_UNSET; + read_state_file(&sb, state, "apply-opt", 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); @@ -585,6 +594,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); @@ -647,6 +657,9 @@ 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); + if (state->rebasing) write_file(am_path(state, "rebasing"), 1, "%s", ""); else @@ -671,6 +684,8 @@ 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); + + strbuf_release(&sb); } /** @@ -1058,6 +1073,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 @@ -1084,6 +1101,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")); @@ -1599,9 +1617,36 @@ int cmd_am(int argc, const char **argv, const char *prefix) 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, +
[PATCH v7 41/45] builtin-am: support and auto-detect mercurial patches
Since 0cfd112 (am: preliminary support for hg patches, 2011-08-29), git-am.sh could convert mercurial patches to an RFC2822 mail patch suitable for parsing with git-mailinfo, and queue them in the state directory for application. Since 15ced75 (git-am foreign patch support: autodetect some patch formats, 2009-05-27), git-am.sh was able to auto-detect mercurial patches by checking if the file begins with the line: # HG changeset patch Re-implement the above in builtin/am.c. Helped-by: Stefan Beller Signed-off-by: Paul Tan --- Notes: v7 * Since a5481a6 (convert "enum date_mode" into a struct, 2015-06-25), show_date() now takes a date_mode struct. Use the DATE_MODE() macro to pass the equivalent date_mode struct to show_date(). builtin/am.c | 74 +++- 1 file changed, 73 insertions(+), 1 deletion(-) diff --git a/builtin/am.c b/builtin/am.c index 3c2ec15..98c10a0 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -81,7 +81,8 @@ enum patch_format { PATCH_FORMAT_UNKNOWN = 0, PATCH_FORMAT_MBOX, PATCH_FORMAT_STGIT, - PATCH_FORMAT_STGIT_SERIES + PATCH_FORMAT_STGIT_SERIES, + PATCH_FORMAT_HG }; enum keep_type { @@ -656,6 +657,11 @@ static int detect_patch_format(const char **paths) goto done; } + if (!strcmp(l1.buf, "# HG changeset patch")) { + ret = PATCH_FORMAT_HG; + goto done; + } + strbuf_reset(&l2); strbuf_getline_crlf(&l2, fp); strbuf_reset(&l3); @@ -854,6 +860,68 @@ static int split_mail_stgit_series(struct am_state *state, const char **paths, } /** + * A split_patches_conv() callback that converts a mercurial patch to a RFC2822 + * message suitable for parsing with git-mailinfo. + */ +static int hg_patch_to_mail(FILE *out, FILE *in, int keep_cr) +{ + struct strbuf sb = STRBUF_INIT; + + while (!strbuf_getline(&sb, in, '\n')) { + const char *str; + + if (skip_prefix(sb.buf, "# User ", &str)) + fprintf(out, "From: %s\n", str); + else if (skip_prefix(sb.buf, "# Date ", &str)) { + unsigned long timestamp; + long tz, tz2; + char *end; + + errno = 0; + timestamp = strtoul(str, &end, 10); + if (errno) + return error(_("invalid timestamp")); + + if (!skip_prefix(end, " ", &str)) + return error(_("invalid Date line")); + + errno = 0; + tz = strtol(str, &end, 10); + if (errno) + return error(_("invalid timezone offset")); + + if (*end) + return error(_("invalid Date line")); + + /* +* mercurial's timezone is in seconds west of UTC, +* however git's timezone is in hours + minutes east of +* UTC. Convert it. +*/ + tz2 = labs(tz) / 3600 * 100 + labs(tz) % 3600 / 60; + if (tz > 0) + tz2 = -tz2; + + fprintf(out, "Date: %s\n", show_date(timestamp, tz2, DATE_MODE(RFC2822))); + } else if (starts_with(sb.buf, "# ")) { + continue; + } else { + fprintf(out, "\n%s\n", sb.buf); + break; + } + } + + strbuf_reset(&sb); + while (strbuf_fread(&sb, 8192, in) > 0) { + fwrite(sb.buf, 1, sb.len, out); + strbuf_reset(&sb); + } + + strbuf_release(&sb); + 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`. @@ -885,6 +953,8 @@ static int split_mail(struct am_state *state, enum patch_format patch_format, return split_mail_conv(stgit_patch_to_mail, state, paths, keep_cr); case PATCH_FORMAT_STGIT_SERIES: return split_mail_stgit_series(state, paths, keep_cr); + case PATCH_FORMAT_HG: + return split_mail_conv(hg_patch_to_mail, state, paths, keep_cr); default: die("BUG: invalid patch_format"); } @@ -1937,6 +2007,8 @@ static int parse_opt_patchformat(const struct option *opt, const char *arg, int *opt_value = PATCH_FORMAT_STGIT; else if (!st
[PATCH v7 43/45] builtin-am: implement legacy -b/--binary option
The -b/--binary option was initially implemented in 087b674 (git-am: --binary; document --resume and --binary., 2005-11-16). The option will pass the --binary flag to git-apply to allow it to apply binary patches. However, in 2b6eef9 (Make apply --binary a no-op., 2006-09-06), --binary was been made a no-op in git-apply. Following that, since cb3a160 (git-am: ignore --binary option, 2008-08-09), the --binary option in git-am is ignored as well. In 6c15a1c (am: officially deprecate -b/--binary option, 2012-03-13), the --binary option was tweaked to its present behavior: when set, the message: The -b/--binary option has been a no-op for long time, and it will be removed. Please do not use it anymore. will be printed. Re-implement this in builtin/am.c. Signed-off-by: Paul Tan --- builtin/am.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 589199f..3c50392 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -2126,6 +2126,7 @@ enum resume_mode { int cmd_am(int argc, const char **argv, const char *prefix) { struct am_state state; + int binary = -1; int keep_cr = -1; int patch_format = PATCH_FORMAT_UNKNOWN; enum resume_mode resume = RESUME_FALSE; @@ -2139,6 +2140,8 @@ int cmd_am(int argc, const char **argv, const char *prefix) struct option options[] = { OPT_BOOL('i', "interactive", &state.interactive, N_("run interactively")), + OPT_HIDDEN_BOOL('b', "binary", &binary, + N_("(historical option -- no-op")), OPT_BOOL('3', "3way", &state.threeway, N_("allow fall back on 3way merging if needed")), OPT__QUIET(&state.quiet, N_("be quiet")), @@ -2239,6 +2242,10 @@ int cmd_am(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, options, usage, 0); + if (binary >= 0) + fprintf_ln(stderr, _("The -b/--binary option has been a no-op for long time, and\n" + "it will be removed. Please do not use it anymore.")); + if (read_index_preload(&the_index, NULL) < 0) die(_("failed to read the index")); -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 11/45] builtin-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 | 72 +++- 1 file changed, 71 insertions(+), 1 deletion(-) diff --git a/builtin/am.c b/builtin/am.c index 537ad62..fd26721 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -760,6 +760,21 @@ static void do_commit(const struct am_state *state) } /** + * Validates the am_state for resuming -- the "msg" and authorship fields must + * be filled up. + */ +static void validate_resume_state(const struct am_state *state) +{ + if (!state->msg) + die(_("cannot resume: %s does not exist."), + am_path(state, "final-commit")); + + if (!state->author_name || !state->author_email || !state->author_date) + die(_("cannot resume: %s does not exist."), + am_path(state, "author-script")); +} + +/** * Applies all queued mail. */ static void am_run(struct am_state *state) @@ -814,6 +829,36 @@ 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) +{ + validate_resume_state(state); + + printf_ln(_("Applying: %.*s"), linelen(state->msg), state->msg); + + 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`. */ @@ -828,13 +873,20 @@ static int parse_opt_patchformat(const struct option *opt, const char *arg, int return 0; } +enum resume_mode { + RESUME_FALSE = 0, + RESUME_RESOLVED +}; + int cmd_am(int argc, const char **argv, const char *prefix) { struct am_state state; int patch_format = PATCH_FORMAT_UNKNOWN; + enum resume_mode resume = RESUME_FALSE; const char * const usage[] = { N_("git am [options] [(|)...]"), + N_("git am [options] --continue"), NULL }; @@ -842,6 +894,12 @@ 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_CMDMODE(0, "continue", &resume, + N_("continue applying patches after resolving a conflict"), + RESUME_RESOLVED), + OPT_CMDMODE('r', "resolved", &resume, + N_("synonyms for --continue"), + RESUME_RESOLVED), OPT_END() }; @@ -875,6 +933,9 @@ int cmd_am(int argc, const char **argv, const char *prefix) struct argv_array paths = ARGV_ARRAY_INIT; int i; + if (resume) + die(_("Resolve operation not in progress, we are not resuming.")); + for (i = 0; i < argc; i++) { if (is_absolute_path(argv[i]) || !prefix) argv_array_push(&paths, argv[i]); @@ -887,7 +948,16 @@ int cmd_am(int argc, const char **argv, const char *prefix) argv_array_clear(&paths); } - am_run(&state); + switch (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.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 09/45] builtin-am: implement committing 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. Since 29b6754 (am: remove rebase-apply directory before gc, 2010-02-22), git gc --auto is also invoked to pack the loose objects that are created from making the commits. Signed-off-by: Paul Tan --- builtin/am.c | 55 +++ 1 file changed, 51 insertions(+), 4 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 1f198e4..a2811b6 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -11,6 +11,9 @@ #include "run-command.h" #include "quote.h" #include "lockfile.h" +#include "cache-tree.h" +#include "refs.h" +#include "commit.h" /** * Returns 1 if the file is empty or does not exist, 0 otherwise. @@ -674,10 +677,56 @@ 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, state->author_email, + state->author_date, IDENT_STRICT); + + if (commit_tree(state->msg, 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, linelen(state->msg), + state->msg); + + update_ref(sb.buf, "HEAD", commit, ptr, 0, UPDATE_REFS_DIE_ON_ERR); + + strbuf_release(&sb); +} + +/** * Applies all queued mail. */ static void am_run(struct am_state *state) { + const char *argv_gc_auto[] = {"gc", "--auto", NULL}; + refresh_and_write_cache(); while (state->cur <= state->last) { @@ -709,16 +758,14 @@ static void am_run(struct am_state *state) exit(128); } - /* -* NEEDSWORK: 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); } am_destroy(state); + run_command_v_opt(argv_gc_auto, RUN_GIT_CMD); } /** -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 25/45] builtin-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 | 50 ++ 1 file changed, 50 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 528b2c9..68dca2e 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -68,6 +68,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 */ char *dir; @@ -91,6 +97,7 @@ struct am_state { int quiet; int signoff; int utf8; + int keep; /* enum keep_type */ const char *resolvemsg; int rebasing; }; @@ -373,6 +380,14 @@ static void am_load(struct am_state *state) read_state_file(&sb, state, "utf8", 1); state->utf8 = !strcmp(sb.buf, "t"); + read_state_file(&sb, state, "keep", 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); @@ -536,6 +551,7 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, const char **paths) { unsigned char curr_head[GIT_SHA1_RAWSZ]; + const char *str; if (!patch_format) patch_format = detect_patch_format(paths); @@ -564,6 +580,22 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, write_file(am_path(state, "utf8"), 1, state->utf8 ? "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 @@ -731,6 +763,20 @@ static int parse_mail(struct am_state *state, const char *mail) argv_array_push(&cp.args, "mailinfo"); argv_array_push(&cp.args, state->utf8 ? "-u" : "-n"); + + 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")); @@ -1471,6 +1517,10 @@ int cmd_am(int argc, const char **argv, const char *prefix) N_("add a Signed-off-by line to the commit message")), OPT_BOOL('u', "utf8", &state.utf8, N_("recode into utf8 (default)")), + 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", &patch_format, N_("format"), N_("format the patch(es) are in"), parse_opt_patchformat), -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 04/45] 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 --- builtin/am.c | 177 +++ 1 file changed, 177 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index fd32caf..ac172c4 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -6,9 +6,171 @@ #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) +{ + 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) +{ + 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_state *state) +{ + struct strbuf sb = STRBUF_INIT; + + strbuf_addstr(&sb, state->dir); + 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, 0777) < 0 && errno != EEXIST) + die_errno(_("failed to create directory '%s'"), state->dir); + + /* +* NOTE: Since the "next" and "last" files determine if an am_state +
[PATCH v7 03/45] 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 --- 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 7efedbe..da451f8 100644 --- a/Makefile +++ b/Makefile @@ -813,6 +813,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 839483d..79aaf0a 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 55c327c..38d9ad5 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.280.gd88bd6e -- To unsubscribe from this list: send the line "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 v7 00/45] Make git-am a builtin
This is a re-roll of [v6]. The changes are as follows: * removed am.threeWay config to match master * renamed am_state's "append_signoff" field to the shorter "signoff" to preserve horizontal space. * Fix memory leak in am_abort() (Noticed by Stefan, thanks!) * Rebase onto master, and adjust to the changes introduced by a5481a6 (convert "enum date_mode" into a struct, 2015-06-25). Interdiff below. 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 [v4] http://thread.gmane.org/gmane.comp.version-control.git/272876 [v5] http://thread.gmane.org/gmane.comp.version-control.git/273520 [v6] http://thread.gmane.org/gmane.comp.version-control.git/274225 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 patch series rewrites git-am.sh into 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 (45): 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: don't parse mail when resuming builtin-am: implement --skip builtin-am: implement --abort builtin-am: reject patches when there's a session in progress builtin-am: implement -q/--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 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| 2319 +++ 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 | 56 + 9 files changed, 2398 insertions(+), 13 deletions(-) create mode 100644 builtin/am.c rename git-am.sh => contrib/examples/git-am.sh (100%) diff --git a/builtin/am.c b/builtin/am.c index 1116304..84d57d4 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -123,7 +123,7 @@ struct am_state { int interactive; int threeway; int quiet; - int append_signoff; + int signoff; int utf8; int keep; /* enum keep_type */ int message_id; @@ -152,8 +152,6 @@ static void am_state_init(struct am_state *state, const char *dir) state->prec = 4; - git_config_get_bool("am.threeway", &state->threeway); - state->utf8 = 1; git_config_get_bool("am.messageid", &state->message_id); @@ -429,7 +427,7 @@ static void am_load(struct am_state *state) state->quiet = !strcmp(sb.buf, "t"); read_state_file(&sb, state, "sign", 1); - state->ap
[PATCH v2 0/3] am: let command-line options override saved options
Let command-line options override saved options in git-am when resuming This is a re-roll of [v1]. Previous versions: [v1] http://thread.gmane.org/gmane.comp.version-control.git/274789 When resuming, git-am mistakenly ignores command-line options. For instance, when a patch fails to apply with "git am patch", subsequently running "git am --3way" would not cause git-am to fall back on attempting a threeway merge. This occurs because by default the --3way option is saved as "false", and the saved am options are loaded after the command-line options are parsed, thus overwriting the command-line options when resuming. [PATCH 1/3] tweaks test-terminal.perl to redirect the stdin of the child process to a pty. This is to support the tests in [PATCH 2/3]. [PATCH 2/3] fixes builtin/am.c, enabling command-line options to override saved options. However, even with this patch, the following command-line options have no effect when resuming: * --signoff overriding --no-signoff * --no-keep overriding --keep * --message-id overriding --no-message-id * --scissors overriding --no-scissors This is because they are only taken into account during the mail-parsing stage, which is skipped over when resuming. [PATCH 3/3] adds support for the --signoff option when resuming by recognizing that we can (re-)append the signoff when the user explicitly specifies the --signoff option. Since the --keep, --message-id and --scissors options are handled by git-mailinfo, it is tricky to implement support for them without introducing lots of code complexity, and thus this patch series does not attempt to. Furthermore, it is hard to imagine a use case for e.g. --scissors overriding --no-scissors, and hence it might be preferable to wait until someone comes with a solid use case, instead of implementing potentially undesirable behavior and having to support it. Paul Tan (3): test_terminal: redirect child process' stdin to a pty am: let command-line options override saved options am: let --signoff override --no-signoff builtin/am.c | 42 --- t/t4153-am-resume-override-opts.sh | 102 + t/test-terminal.perl | 25 +++-- 3 files changed, 158 insertions(+), 11 deletions(-) create mode 100755 t/t4153-am-resume-override-opts.sh -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/3] test_terminal: redirect child process' stdin to a pty
When resuming, git-am detects if we are trying to feed it patches or not by checking if stdin is a TTY. However, the test library redirects stdin to /dev/null. This makes it difficult, for instance, to test the behavior of "git am -3" when resuming, as git-am will think we are trying to feed it patches and error out. Support this use case by extending test-terminal.perl to create a pseudo-tty for the child process' standard input as well. Note that due to the way the code is structured, the child's stdin pseudo-tty will be closed when we finish reading from our stdin. This means that in the common case, where our stdin is attached to /dev/null, the child's stdin pseudo-tty will be closed immediately. Some operations like isatty(), which git-am uses, require the file descriptor to be open, and hence if the success of the command depends on such functions, test_terminal's stdin should be redirected to a source with large amount of data to ensure that the child's stdin is not closed, e.g. test_terminal git am --3way Cc: Jeff King Signed-off-by: Paul Tan --- t/test-terminal.perl | 25 - 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/t/test-terminal.perl b/t/test-terminal.perl index 1fb373f..f6fc9ae 100755 --- a/t/test-terminal.perl +++ b/t/test-terminal.perl @@ -5,15 +5,17 @@ use warnings; use IO::Pty; use File::Copy; -# Run @$argv in the background with stdio redirected to $out and $err. +# Run @$argv in the background with stdio redirected to $in, $out and $err. sub start_child { - my ($argv, $out, $err) = @_; + my ($argv, $in, $out, $err) = @_; my $pid = fork; if (not defined $pid) { die "fork failed: $!" } elsif ($pid == 0) { + open STDIN, "<&", $in; open STDOUT, ">&", $out; open STDERR, ">&", $err; + close $in; close $out; exec(@$argv) or die "cannot exec '$argv->[0]': $!" } @@ -50,14 +52,23 @@ sub xsendfile { } sub copy_stdio { - my ($out, $err) = @_; + my ($in, $out, $err) = @_; my $pid = fork; + if (!$pid) { + close($out); + close($err); + xsendfile($in, \*STDIN); + exit 0; + } + $pid = fork; defined $pid or die "fork failed: $!"; if (!$pid) { + close($in); close($out); xsendfile(\*STDERR, $err); exit 0; } + close($in); close($err); xsendfile(\*STDOUT, $out); finish_child($pid) == 0 @@ -67,14 +78,18 @@ sub copy_stdio { if ($#ARGV < 1) { die "usage: test-terminal program args"; } +my $master_in = new IO::Pty; my $master_out = new IO::Pty; my $master_err = new IO::Pty; +$master_in->set_raw(); $master_out->set_raw(); $master_err->set_raw(); +$master_in->slave->set_raw(); $master_out->slave->set_raw(); $master_err->slave->set_raw(); -my $pid = start_child(\@ARGV, $master_out->slave, $master_err->slave); +my $pid = start_child(\@ARGV, $master_in->slave, $master_out->slave, $master_err->slave); +close $master_in->slave; close $master_out->slave; close $master_err->slave; -copy_stdio($master_out, $master_err); +copy_stdio($master_in, $master_out, $master_err); exit(finish_child($pid)); -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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/3] am: let command-line options override saved options
When resuming, git-am mistakenly ignores command-line options. For instance, when a patch fails to apply with "git am patch", subsequently running "git am --3way" would not cause git-am to fall back on attempting a threeway merge. This occurs because by default the --3way option is saved as "false", and the saved am options are loaded after the command-line options are parsed, thus overwriting the command-line options when resuming. Fix this by moving the am_load() function call before parse_options(), so that command-line options will override the saved am options. The purpose of supporting this use case is to enable users to "wiggle" that one conflicting patch. As such, it is expected that the command-line options do not affect subsequent applied patches. Implement this by calling am_load() once we apply the conflicting patch successfully. Noticed-by: Junio C Hamano Helped-by: Junio C Hamano Helped-by: Jeff King Signed-off-by: Paul Tan --- builtin/am.c | 16 ++-- t/t4153-am-resume-override-opts.sh | 82 ++ 2 files changed, 94 insertions(+), 4 deletions(-) create mode 100755 t/t4153-am-resume-override-opts.sh diff --git a/builtin/am.c b/builtin/am.c index 84d57d4..0961304 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1777,7 +1777,6 @@ static void am_run(struct am_state *state, int resume) if (resume) { validate_resume_state(state); - resume = 0; } else { int skip; @@ -1839,6 +1838,10 @@ static void am_run(struct am_state *state, int resume) next: am_next(state); + + if (resume) + am_load(state); + resume = 0; } if (!is_empty_file(am_path(state, "rewritten"))) { @@ -1893,6 +1896,7 @@ static void am_resolve(struct am_state *state) next: am_next(state); + am_load(state); am_run(state, 0); } @@ -2020,6 +2024,7 @@ static void am_skip(struct am_state *state) die(_("failed to clean index")); am_next(state); + am_load(state); am_run(state, 0); } @@ -2130,6 +2135,7 @@ int cmd_am(int argc, const char **argv, const char *prefix) int keep_cr = -1; int patch_format = PATCH_FORMAT_UNKNOWN; enum resume_mode resume = RESUME_FALSE; + int in_progress; const char * const usage[] = { N_("git am [options] [(|)...]"), @@ -2225,6 +2231,10 @@ int cmd_am(int argc, const char **argv, const char *prefix) am_state_init(&state, git_path("rebase-apply")); + in_progress = am_in_progress(&state); + if (in_progress) + am_load(&state); + argc = parse_options(argc, argv, prefix, options, usage, 0); if (binary >= 0) @@ -2237,7 +2247,7 @@ 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 (in_progress) { /* * Catch user error to feed us patches when there is a session * in progress: @@ -2255,8 +2265,6 @@ int cmd_am(int argc, const char **argv, const char *prefix) if (resume == RESUME_FALSE) resume = RESUME_APPLY; - - am_load(&state); } else { struct argv_array paths = ARGV_ARRAY_INIT; int i; diff --git a/t/t4153-am-resume-override-opts.sh b/t/t4153-am-resume-override-opts.sh new file mode 100755 index 000..39fac79 --- /dev/null +++ b/t/t4153-am-resume-override-opts.sh @@ -0,0 +1,82 @@ +#!/bin/sh + +test_description='git-am command-line options override saved options' + +. ./test-lib.sh +. "$TEST_DIRECTORY"/lib-terminal.sh + +format_patch () { + git format-patch --stdout -1 "$1" >"$1".eml +} + +test_expect_success 'setup' ' + test_commit initial file && + test_commit first file && + + git checkout initial && + git mv file file2 && + test_tick && + git commit -m renamed-file && + git tag renamed-file && + + git checkout -b side initial && + test_commit side1 file && + test_commit side2 file && + + format_patch side1 && + format_patch side2 +' + +test_expect_success TTY '--3way overrides --no-3way' ' + rm -fr .git/rebase-apply && + git reset --hard && + git checkout renamed-file && + + # Applying side1 will fail as the file has been renamed. + test_must_fai
[PATCH v2 3/3] am: let --signoff override --no-signoff
After resolving a conflicting patch, a user may wish to sign off the patch to declare that the patch has been modified. As such, the user will expect that running "git am --signoff --continue" will append the signoff to the commit message. However, the --signoff option is only taken into account during the mail-parsing stage. If the --signoff option is set, then the signoff will be appended to the commit message. Since the mail-parsing stage comes before the patch application stage, the --signoff option, if provided on the command-line when resuming, will have no effect at all. We cannot move the append_signoff() call to the patch application stage as the applypatch-msg hook and interactive mode, which run before patch application, may expect the signoff to be there. Fix this by taking note if the user explictly set the --signoff option on the command-line, and append the signoff to the commit message when resuming if so. Signed-off-by: Paul Tan --- builtin/am.c | 28 +--- t/t4153-am-resume-override-opts.sh | 20 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 0961304..8c95aec 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -98,6 +98,12 @@ enum scissors_type { SCISSORS_TRUE/* pass --scissors to git-mailinfo */ }; +enum signoff_type { + SIGNOFF_FALSE = 0, + SIGNOFF_TRUE = 1, + SIGNOFF_EXPLICIT /* --signoff was set on the command-line */ +}; + struct am_state { /* state directory path */ char *dir; @@ -123,7 +129,7 @@ struct am_state { int interactive; int threeway; int quiet; - int signoff; + int signoff; /* enum signoff_type */ int utf8; int keep; /* enum keep_type */ int message_id; @@ -1184,6 +1190,18 @@ static void NORETURN die_user_resolve(const struct am_state *state) } /** + * Appends signoff to the "msg" field of the am_state. + */ +static void am_append_signoff(struct am_state *state) +{ + struct strbuf sb = STRBUF_INIT; + + strbuf_attach(&sb, state->msg, state->msg_len, state->msg_len); + append_signoff(&sb, 0, 0); + state->msg = strbuf_detach(&sb, &state->msg_len); +} + +/** * 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 @@ -2151,8 +2169,9 @@ int cmd_am(int argc, const char **argv, const char *prefix) OPT_BOOL('3', "3way", &state.threeway, N_("allow fall back on 3way merging if needed")), OPT__QUIET(&state.quiet, N_("be quiet")), - OPT_BOOL('s', "signoff", &state.signoff, - N_("add a Signed-off-by line to the commit message")), + OPT_SET_INT('s', "signoff", &state.signoff, + N_("add a Signed-off-by line to the commit message"), + SIGNOFF_EXPLICIT), OPT_BOOL('u', "utf8", &state.utf8, N_("recode into utf8 (default)")), OPT_SET_INT('k', "keep", &state.keep, @@ -2265,6 +2284,9 @@ int cmd_am(int argc, const char **argv, const char *prefix) if (resume == RESUME_FALSE) resume = RESUME_APPLY; + + if (state.signoff == SIGNOFF_EXPLICIT) + am_append_signoff(&state); } else { struct argv_array paths = ARGV_ARRAY_INIT; int i; diff --git a/t/t4153-am-resume-override-opts.sh b/t/t4153-am-resume-override-opts.sh index 39fac79..7c013d8 100755 --- a/t/t4153-am-resume-override-opts.sh +++ b/t/t4153-am-resume-override-opts.sh @@ -64,6 +64,26 @@ test_expect_success '--no-quiet overrides --quiet' ' test_i18ncmp expected out ' +test_expect_success '--signoff overrides --no-signoff' ' + rm -fr .git/rebase-apply && + git reset --hard && + git checkout first && + + test_must_fail git am --no-signoff side[12].eml && + test_path_is_dir .git/rebase-apply && + echo side1 >file && + git add file && + git am --signoff --continue && + + # Applied side1 will be signed off + echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" >expected && + git cat-file commit HEAD^ | grep "Signed-off-by:" >actual && + test_cmp expected actual && + + # Applied side2 will not be signed off
[PATCH v2 0/3] am: let command-line options override saved options
Let command-line options override saved options in git-am when resuming This is a re-roll of [v1]. Previous versions: [v1] http://thread.gmane.org/gmane.comp.version-control.git/274789 When resuming, git-am mistakenly ignores command-line options. For instance, when a patch fails to apply with "git am patch", subsequently running "git am --3way" would not cause git-am to fall back on attempting a threeway merge. This occurs because by default the --3way option is saved as "false", and the saved am options are loaded after the command-line options are parsed, thus overwriting the command-line options when resuming. [PATCH 1/3] tweaks test-terminal.perl to redirect the stdin of the child process to a pty. This is to support the tests in [PATCH 2/3]. [PATCH 2/3] fixes builtin/am.c, enabling command-line options to override saved options. However, even with this patch, the following command-line options have no effect when resuming: * --signoff overriding --no-signoff * --no-keep overriding --keep * --message-id overriding --no-message-id * --scissors overriding --no-scissors This is because they are only taken into account during the mail-parsing stage, which is skipped over when resuming. [PATCH 3/3] adds support for the --signoff option when resuming by recognizing that we can (re-)append the signoff when the user explicitly specifies the --signoff option. Since the --keep, --message-id and --scissors options are handled by git-mailinfo, it is tricky to implement support for them without introducing lots of code complexity, and thus this patch series does not attempt to. Furthermore, it is hard to imagine a use case for e.g. --scissors overriding --no-scissors, and hence it might be preferable to wait until someone comes with a solid use case, instead of implementing potentially undesirable behavior and having to support it. Paul Tan (3): test_terminal: redirect child process' stdin to a pty am: let command-line options override saved options am: let --signoff override --no-signoff builtin/am.c | 42 --- t/t4153-am-resume-override-opts.sh | 102 + t/test-terminal.perl | 25 +++-- 3 files changed, 158 insertions(+), 11 deletions(-) create mode 100755 t/t4153-am-resume-override-opts.sh -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "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] git-am: add am.threeWay config variable
From: Remi Lespinet Add the am.threeWay configuration variable to use the -3 or --3way option of git am by default. When am.threeway is set and not desired for a specific git am command, the --no-3way option can be used to override it. Signed-off-by: Remi Lespinet Signed-off-by: Paul Tan --- I tweaked Remi's patch so it is implemented on top of builtin/am.c. Hopefully there will be no regressions this time ;) Documentation/config.txt | 8 Documentation/git-am.txt | 7 +-- builtin/am.c | 2 ++ t/t4150-am.sh| 19 +++ 4 files changed, 34 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 315f271..fb3fc57 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -769,6 +769,14 @@ am.keepcr:: by giving '--no-keep-cr' from the command line. See linkgit:git-am[1], linkgit:git-mailsplit[1]. +am.threeWay:: + By default, `git am` will fail if the patch does not apply cleanly. When + set to true, this setting tells `git am` to fall back on 3-way merge if + the patch records the identity of blobs it is supposed to apply to and + we have those blobs available locally (equivalent to giving the `--3way` + option from the command line). Defaults to `false`. + See linkgit:git-am[1]. + apply.ignoreWhitespace:: When set to 'change', tells 'git apply' to ignore changes in whitespace, in the same way as the '--ignore-space-change' diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt index 0d8ba48..dbea6e7 100644 --- a/Documentation/git-am.txt +++ b/Documentation/git-am.txt @@ -10,7 +10,7 @@ SYNOPSIS [verse] 'git am' [--signoff] [--keep] [--[no-]keep-cr] [--[no-]utf8] -[--3way] [--interactive] [--committer-date-is-author-date] +[--[no-]3way] [--interactive] [--committer-date-is-author-date] [--ignore-date] [--ignore-space-change | --ignore-whitespace] [--whitespace=] [-C] [-p] [--directory=] [--exclude=] [--include=] [--reject] [-q | --quiet] @@ -90,10 +90,13 @@ default. You can use `--no-utf8` to override this. -3:: --3way:: +--no-3way:: When the patch does not apply cleanly, fall back on 3-way merge if the patch records the identity of blobs it is supposed to apply to and we have those blobs - available locally. + available locally. `--no-3way` can be used to override + am.threeWay configuration variable. For more information, + see am.threeWay in linkgit:git-config[1]. --ignore-space-change:: --ignore-whitespace:: diff --git a/builtin/am.c b/builtin/am.c index 84d57d4..1399c8d 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -152,6 +152,8 @@ static void am_state_init(struct am_state *state, const char *dir) state->prec = 4; + git_config_get_bool("am.threeway", &state->threeway); + state->utf8 = 1; git_config_get_bool("am.messageid", &state->message_id); diff --git a/t/t4150-am.sh b/t/t4150-am.sh index e9b6f81..dd627c4 100755 --- a/t/t4150-am.sh +++ b/t/t4150-am.sh @@ -551,6 +551,25 @@ test_expect_success 'am -3 -p0 can read --no-prefix patch' ' git diff --exit-code lorem ' +test_expect_success 'am with config am.threeWay falls back to 3-way merge' ' + rm -fr .git/rebase-apply && + git reset --hard && + git checkout -b lorem4 base3way && + test_config am.threeWay 1 && + git am lorem-move.patch && + test_path_is_missing .git/rebase-apply && + git diff --exit-code lorem +' + +test_expect_success 'am with config am.threeWay overridden by --no-3way' ' + rm -fr .git/rebase-apply && + git reset --hard && + git checkout -b lorem5 base3way && + test_config am.threeWay 1 && + test_must_fail git am --no-3way lorem-move.patch && + test_path_is_dir .git/rebase-apply +' + test_expect_success 'am can rename a file' ' grep "^rename from" rename.patch && rm -fr .git/rebase-apply && -- 2.5.0.280.gd88bd6e -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/3] am: let command-line options override saved options
On Wed, Aug 05, 2015 at 08:41:44AM -0700, Junio C Hamano wrote: > Interesting. This seems to break test under prove. > > cd t && make T=t4153-am-resume-override-opts.sh prove > > does not seem to return. The new test-terminal.perl code is the culprit. It seems that if our wrapped process terminates before our stdin-writing fork does, our stdin-writing process will stall. I think this occurs with prove because prove waits until all of its child processes terminate before returning. So, the solution may be to send a SIGTERM to our stdin-writing fork should our wrapped process terminate before it does, in order to ensure that it immediately exits. The following squash fixes it for me. Thanks, Paul -- >8 -- Subject: [PATCH] squash! test_terminal: redirect child process' stdin to a pty When the child process terminates before the copy_stdio() finishes writing all of its data to the child's stdin slave pty, it will stall. As such, we first move the stdin-pty-writing logic out of copy_stdio() into its own subroutine copy_stdin() so that we can manage the forked process ourselves, and then we send SIGTERM to the forked process should the command we are wrapping terminate before copy_stdin() finishes writing all of its data to un-stall it. Signed-off-by: Paul Tan --- t/test-terminal.perl | 27 ++- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/t/test-terminal.perl b/t/test-terminal.perl index f6fc9ae..96b6a03 100755 --- a/t/test-terminal.perl +++ b/t/test-terminal.perl @@ -51,24 +51,26 @@ sub xsendfile { copy($in, $out, 4096) or $!{EIO} or die "cannot copy from child: $!"; } -sub copy_stdio { - my ($in, $out, $err) = @_; +sub copy_stdin { + my ($in) = @_; my $pid = fork; if (!$pid) { - close($out); - close($err); xsendfile($in, \*STDIN); exit 0; } - $pid = fork; + close($in); + return $pid; +} + +sub copy_stdio { + my ($out, $err) = @_; + my $pid = fork; defined $pid or die "fork failed: $!"; if (!$pid) { - close($in); close($out); xsendfile(\*STDERR, $err); exit 0; } - close($in); close($err); xsendfile(\*STDOUT, $out); finish_child($pid) == 0 @@ -91,5 +93,12 @@ my $pid = start_child(\@ARGV, $master_in->slave, $master_out->slave, $master_err close $master_in->slave; close $master_out->slave; close $master_err->slave; -copy_stdio($master_in, $master_out, $master_err); -exit(finish_child($pid)); +my $in_pid = copy_stdin($master_in); +copy_stdio($master_out, $master_err); +my $ret = finish_child($pid); +# If the child process terminates before our copy_stdin() process is able to +# write all of its data to $master_in, the copy_stdin() process could stall. +# Send SIGTERM to it to ensure it terminates. +kill 'TERM', $in_pid; +finish_child($in_pid); +exit($ret); -- 2.5.0.282.gdd6b4b0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] am: let --signoff override --no-signoff
On Fri, Aug 7, 2015 at 5:29 PM, Johannes Schindelin wrote: >> diff --git a/builtin/am.c b/builtin/am.c >> index 0961304..8c95aec 100644 >> --- a/builtin/am.c >> +++ b/builtin/am.c >> @@ -2265,6 +2284,9 @@ int cmd_am(int argc, const char **argv, const >> char *prefix) >> >> if (resume == RESUME_FALSE) >> resume = RESUME_APPLY; >> + >> + if (state.signoff == SIGNOFF_EXPLICIT) >> + am_append_signoff(&state); >> } else { > > This is clever, but I suspect there is now a chance for a double-signoff if > we passed `--signoff` to the initial `git am` call and it went through > without having to resume. It's not present in this diff context, but this hunk modifies the code path where in_progress is true. In other words, we only check for SIGNOFF_EXPLICIT if -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] am: let --signoff override --no-signoff
On Wed, Aug 12, 2015 at 11:06 AM, Paul Tan wrote: > It's not present in this diff context, but this hunk modifies the code > path where in_progress is true. In other words, we only check for > SIGNOFF_EXPLICIT if ..we are resuming. (Ugh, butter fingers) Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] test_terminal: redirect child process' stdin to a pty
On Fri, Aug 7, 2015 at 6:15 AM, Eric Sunshine wrote: > An alternative would be to have git-am detect that it is being tested > and pretend that isatty() returns true. I would vastly prefer a solution that would work for everything, for all the C code and scripts, instead of implementing a workaround in git-am :( In this case, I implemented a generic solution in test-terminal.perl that works for POSIX systems, so if there are no problems with its implementation, I do think it's better. Other than the fact that it does not work on non-Unix platforms, of course. The other approach I would consider is to implement a xisatty() function that returns true for xisatty(0) if TEST_TTY=0 or something. However, I do wonder if this would lead us to have to hack around other functions of terminals as well (e.g. if xisatty(0), tcgetattr()), which would be a big can of worms I think... > There is some precedent for > having core functionality recognize that it is being tested. See, for > instance, environment variable TEST_DATE_NOW, (Hmm, I took a look, and it seems that TEST_DATE_NOW is only checked in test-date.c...) > and rev-list --test-bitmap. > Doing so would allow the tests work on non-Unix > platforms, as well. Ehh, if the non-Unix platforms do not implement terminals, it means that the git-am logic to detect if we are attempting to feed it a patch by checking if stdin is a TTY is invalid anyway, so implementing a "yeah-it-is-a-tty" workaround for the sake of tests would be hiding the problem, I think. 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] am --abort: merge ORIG_HEAD tree into index
On Mon, Aug 17, 2015 at 10:01:29AM +0200, Johannes Schindelin wrote: > Hi Linus, > > On 2015-08-17 01:33, Linus Torvalds wrote: > > On Sun, Aug 16, 2015 at 12:46 PM, Linus Torvalds > > wrote: > >> > >> Maybe it has always done this, and I just haven't noticed (I usually > >> _just_ do the "git reset --hard" thing, don't ask me why I wanted to > >> be doubly sure this time). But maybe it's an effect of the new > >> built-in "am". > > > > I bisected this. It's definitely used to work, and the regression is > > from the new built-in am. > > This patch is a reproducer: Thanks Johannes for the test, and sorry all for the oversight. >< It's true that we need to merge the ORIG_HEAD tree into the index instead of overwriting it. Patch below. Regards, Paul -- >8 -- Subject: [PATCH] am --abort: merge ORIG_HEAD tree into index After running "git am --abort", and then running "git reset --hard", files that were not modified would still be re-checked out. This is because clean_index() in builtin/am.c mistakenly called the read_tree() function, which overwrites all entries in the index, including the stat info. Fix this by using unpack_trees() instead to merge the tree into the index, so that the stat info from the index is kept. Reported-by: Linus Torvalds Signed-off-by: Johannes Schindelin Signed-off-by: Paul Tan --- builtin/am.c| 49 - t/t4151-am-abort.sh | 12 2 files changed, 48 insertions(+), 13 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 1399c8d..6aaa85d 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1940,15 +1940,48 @@ static int fast_forward_to(struct tree *head, struct tree *remote, int reset) } /** + * Merges a tree into the index. The index's stat info will take precedence + * over the merged tree's. Returns 0 on success, -1 on failure. + */ +static int merge_tree(struct tree *tree) +{ + struct lock_file *lock_file; + struct unpack_trees_options opts; + struct tree_desc t[2]; + + if (parse_tree(tree)) + return -1; + + lock_file = xcalloc(1, sizeof(struct lock_file)); + hold_locked_index(lock_file, 1); + + memset(&opts, 0, sizeof(opts)); + opts.head_idx = 1; + opts.src_index = &the_index; + opts.dst_index = &the_index; + opts.merge = 1; + opts.fn = oneway_merge; + init_tree_desc(&t[0], tree->buffer, tree->size); + + if (unpack_trees(1, 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; 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) @@ -1973,18 +2006,8 @@ static int clean_index(const unsigned char *head, const unsigned char *remote) if (fast_forward_to(index_tree, remote_tree, 0)) return -1; - memset(&pathspec, 0, sizeof(pathspec)); - - lock_file = xcalloc(1, sizeof(struct lock_file)); - hold_locked_index(lock_file, 1); - - if (read_tree(remote_tree, 0, &pathspec)) { - rollback_lock_file(lock_file); + if (merge_tree(remote_tree)) return -1; - } - - if (write_locked_index(&the_index, lock_file, COMMIT_LOCK)) - die(_("unable to write new index file")); remove_branch_state(); diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh index 05bdc3e..9c3bbd1 100755 --- a/t/t4151-am-abort.sh +++ b/t/t4151-am-abort.sh @@ -168,4 +168,16 @@ test_expect_success 'am --abort on unborn branch will keep local commits intact' test_cmp expect actual ' +test_expect_success 'am --abort leaves index stat info alone' ' + git checkout -f --orphan stat-info && + git reset && + test_commit should-be-untouched && + test-chmtime =0 should-be-untouched.t && + git update-index --refresh && + git diff-files --exit-code --quiet && + test_must_fail git am 0001-*.patch && + git am --abort && + git diff-files --exit-code --quiet +' + test_done -- 2.5.0.331.g11c07ce -- To unsubscribe from this list: send the line "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] am --skip/--abort: merge HEAD/ORIG_HEAD tree into index
On Mon, Aug 17, 2015 at 12:33:40PM -0700, Junio C Hamano wrote: > Have you checked how this change affects that codepath? To put it > differently, does "am skip" have the same issue without this fix? Hmm, I adopted Dscho's test to run "git am --skip" and it did not fail. I think it's because am_skip() calls am_run(), which calls refresh_cache(), so the resulting index will have the updated stat info. However, there should still be a performance penalty because refresh_cache() would have to scan all files for changes. > If so, I wonder if we can have a test for that, too? So yeah, we should have a test for that too. (In addition, I fixed a small mistake with the "struct tree_desc" array size.) Thanks, Paul -- >8 -- Subject: [PATCH v2] am --skip/--abort: merge HEAD/ORIG_HEAD tree into index After running "git am --abort", and then running "git reset --hard", files that were not modified would still be re-checked out. This is because clean_index() in builtin/am.c mistakenly called the read_tree() function, which overwrites all entries in the index, including the stat info. "git am --skip" did not seem to have this issue because am_skip() called am_run(), which called refresh_cache() to update the stat info. However, there's still a performance penalty as the lack of stat info meant that refresh_cache() would have to scan all files for changes. Fix this by using unpack_trees() instead to merge the tree into the index, so that the stat info from the index is kept. Reported-by: Linus Torvalds Helped-by: Junio C Hamano Signed-off-by: Johannes Schindelin Signed-off-by: Paul Tan --- builtin/am.c| 49 - t/t4151-am-abort.sh | 24 2 files changed, 60 insertions(+), 13 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 1399c8d..3e7e66f 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1940,15 +1940,48 @@ static int fast_forward_to(struct tree *head, struct tree *remote, int reset) } /** + * Merges a tree into the index. The index's stat info will take precedence + * over the merged tree's. Returns 0 on success, -1 on failure. + */ +static int merge_tree(struct tree *tree) +{ + struct lock_file *lock_file; + struct unpack_trees_options opts; + struct tree_desc t[1]; + + if (parse_tree(tree)) + return -1; + + lock_file = xcalloc(1, sizeof(struct lock_file)); + hold_locked_index(lock_file, 1); + + memset(&opts, 0, sizeof(opts)); + opts.head_idx = 1; + opts.src_index = &the_index; + opts.dst_index = &the_index; + opts.merge = 1; + opts.fn = oneway_merge; + init_tree_desc(&t[0], tree->buffer, tree->size); + + if (unpack_trees(1, 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; 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) @@ -1973,18 +2006,8 @@ static int clean_index(const unsigned char *head, const unsigned char *remote) if (fast_forward_to(index_tree, remote_tree, 0)) return -1; - memset(&pathspec, 0, sizeof(pathspec)); - - lock_file = xcalloc(1, sizeof(struct lock_file)); - hold_locked_index(lock_file, 1); - - if (read_tree(remote_tree, 0, &pathspec)) { - rollback_lock_file(lock_file); + if (merge_tree(remote_tree)) return -1; - } - - if (write_locked_index(&the_index, lock_file, COMMIT_LOCK)) - die(_("unable to write new index file")); remove_branch_state(); diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh index 05bdc3e..ea5ace9 100755 --- a/t/t4151-am-abort.sh +++ b/t/t4151-am-abort.sh @@ -168,4 +168,28 @@ test_expect_success 'am --abort on unborn branch will keep local commits intact' test_cmp expect actual ' +test_expect_success 'am --skip leaves index stat info alone' ' + git checkout -f --orphan skip-stat-info && + git reset && + test_commit skip-should-be-untouched && + test-chmtime =0 skip-should-be-untouched.t && + git update-index --refresh && + git diff-files --exit-code --quiet && + test_mus
Re: [PATCH] git-am: add am.threeWay config variable
On Tue, Aug 18, 2015 at 5:36 PM, Matthieu Moy wrote: > I don't remember the details of the regression we had with the shell > version, but that would probably deserve an additional test to enforce > the "Hopefully there will be no regressions" part of your message. Actually, technically, I think this patch by its own would reintroduce the regression ;) The reason is that the bug was caused by the overall structure of the git-am.sh code, and not the patch itself[1]. This is fixed in another patch series[2] on top of this patch which also implements a test for "git am --3way". [1] http://thread.gmane.org/gmane.comp.version-control.git/274577 [2] http://thread.gmane.org/gmane.comp.version-control.git/275322 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] am: terminate state files with a newline
On Thu, Aug 20, 2015 at 11:40:20AM -0700, Junio C Hamano wrote: > SZEDER Gábor writes: > > > The format of the files '.git/rebase-apply/{next,last}' changed slightly > > with the recent builtin 'git am' conversion: while these files were > > newline-terminated when written by the scripted version, the ones written > > by the builtin are not. > > Thanks for noticing; that should be corrected, I think. Okay then, this patch should correct this. Did we ever explictly allow external programs to poke around the contents of the .git/rebase-apply directory? I think it may not be so good, as it means that it may not be possible to switch the storage format in the future (e.g. to allow atomic modifications, maybe?) :-/ . Regards, Paul -- >8 -- Subject: [PATCH] am: terminate state files with a newline Since builtin/am.c replaced git-am.sh in 783d7e8 (builtin-am: remove redirection to git-am.sh, 2015-08-04), the state files written by git-am did not terminate with a newline. This is because the code in builtin/am.c did not write the newline to the state files. While the git codebase has no problems with the missing newline, external software which read the contents of the state directory may be strict about the existence of the terminating newline, and would thus break. Fix this by correcting the relevant calls to write_file() to ensure that the state files written terminate with a newline, matching how git-am.sh behaves. While we are fixing the write_file() calls, fix the writing of the "dirtyindex" file as well -- we should be creating an empty file to match the behavior of git-am.sh. Reported-by: SZEDER Gábor Signed-off-by: Paul Tan --- builtin/am.c | 30 +++--- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 1399c8d..2e57fad 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -994,13 +994,13 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, if (state->rebasing) state->threeway = 1; - write_file(am_path(state, "threeway"), 1, state->threeway ? "t" : "f"); + write_file(am_path(state, "threeway"), 1, "%s\n", state->threeway ? "t" : "f"); - write_file(am_path(state, "quiet"), 1, state->quiet ? "t" : "f"); + write_file(am_path(state, "quiet"), 1, "%s\n", state->quiet ? "t" : "f"); - write_file(am_path(state, "sign"), 1, state->signoff ? "t" : "f"); + write_file(am_path(state, "sign"), 1, "%s\n", state->signoff ? "t" : "f"); - write_file(am_path(state, "utf8"), 1, state->utf8 ? "t" : "f"); + write_file(am_path(state, "utf8"), 1, "%s\n", state->utf8 ? "t" : "f"); switch (state->keep) { case KEEP_FALSE: @@ -1016,9 +1016,9 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, die("BUG: invalid value for state->keep"); } - write_file(am_path(state, "keep"), 1, "%s", str); + write_file(am_path(state, "keep"), 1, "%s\n", str); - write_file(am_path(state, "messageid"), 1, state->message_id ? "t" : "f"); + write_file(am_path(state, "messageid"), 1, "%s\n", state->message_id ? "t" : "f"); switch (state->scissors) { case SCISSORS_UNSET: @@ -1034,10 +1034,10 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, die("BUG: invalid value for state->scissors"); } - write_file(am_path(state, "scissors"), 1, "%s", str); + write_file(am_path(state, "scissors"), 1, "%s\n", str); sq_quote_argv(&sb, state->git_apply_opts.argv, 0); - write_file(am_path(state, "apply-opt"), 1, "%s", sb.buf); + write_file(am_path(state, "apply-opt"), 1, "%s\n", sb.buf); if (state->rebasing) write_file(am_path(state, "rebasing"), 1, "%s", ""); @@ -1045,7 +1045,7 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, 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)); + write_file(am_path(state, "abort-safety"), 1, "%s\n", sha1_
Re: GSoC 2015 is over
On Wed, Sep 2, 2015 at 1:43 AM, Karthik Nayak wrote: > On Tue, Sep 1, 2015 at 10:25 PM, Matthieu Moy > wrote: >> Hi, >> >> The Google Summer of Code 2015 is officially over. We had two students >> (Paul and Karthik), and both of them passed. 100 % success :-). >> > > Congrats Paul :) Thanks, you too ;-) -- To unsubscribe from this list: send the line "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: GSoC 2015 is over
On Wed, Sep 2, 2015 at 12:55 AM, Matthieu Moy wrote: > I consider this GSoC as a great success and a pleasant experience. > Congratulation to Paul and Karthik, and a warm "thank you" to everybody > who contributed: administrators, mentors, reviewers, and obviously > Junio! (not to mention Google, who made all this possible) > > Thanks all! Thanks! The products of my project are not perfect (Jeff just fixed another bug in the builtin am >< Thanks!), and there are still a few leftover bits, but I hope that I've provided a good base of code for working on. Off the top of my head, the leftover bits are: 1. Stefan noticed that the help strings of git-am's options could be more user-friendly: http://thread.gmane.org/gmane.comp.version-control.git/272876/focus=273029 2. Johannes suggested that get_tracking_branch() in builtin/pull.c could be implemented with remote_find_tracking(): http://thread.gmane.org/gmane.comp.version-control.git/269258/focus=269350 3. Junio noticed off-list that relative paths do not work with git-fetch and git-pull Other possible future developments: 1. git am --3way implemented on top of git apply --3way 2. builtin/mailinfo.c looks to me like a good candidate for moving into libgit.a, so that git am can access its functionality without spawning a separate process and writing temporary files. On my part, I have to give a big thank you to Junio, as well as my mentors Johannes and Stefan, for reviewing my patches. Their timely review of my patches played a great part in getting my project into master. Jeff, Eric and many others contributed by reporting bugs, giving ideas and cleaning up my mess. Above all, I would like to thank the organization admins as well as my mentors for running the GSoC program. Thank you for this wonderful opportunity. I've learned a lot, and had lots of fun. I've really enjoyed my time with the Git community, and will stay around for the foreseeable future. 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] am: match --signoff to the original scripted version
Hi, Thanks for handling this. On Sun, Sep 6, 2015 at 12:56 PM, Junio C Hamano wrote: > Linus noticed that the recently reimplementated "git am -s" defines s/reimplementated/reimplemented/ ? > the trailer block too rigidly, resulting an unnecessary blank line s/resulting an/resulting in an/ ? > between the existing sign-offs and his new sign-off. An e-mail > submission sent to Linus in real life ends with mixture of sign-offs > and commentaries, e.g. > > title here > > message here > > Signed-off-by: Original Author > [rv: tweaked frotz and nitfol] > Signed-off-by: Re Viewer > Signed-off-by: Other Reviewer > --- > patch here > > Because the reimplementation reused append_signoff() helper that is > used by other codepaths, which is unaware that people intermix such > comments with their sign-offs in the trailer block, such a message > was judged to end with a non-trailer, resulting in an extra blank s/extra blank/extra blank line/ ? > before adding a new sign-off. > > The original scripted version of "git am" used a lot looser > definition, i.e. "if and only if there is no line that begins with > Signed-off-by:, add a blank line before adding a new sign-off". For > the upcoming release, stop using the append_signoff() in "git am" > and reimplement the looser definition used by the scripted version > to use only in "git am" to fix this regression in "am" while > avoiding new regressions to other users of append_signoff(). > > In the longer term, we should look into loosening append_signoff() > so that other codepaths that add a new sign-off behave the same way > as "git am -s", but that is a task for post-release. > > Reported-by: Linus Torvalds > Signed-off-by: Junio C Hamano > --- > builtin/am.c | 31 +-- > t/t4150-am.sh | 48 > 2 files changed, 77 insertions(+), 2 deletions(-) > > diff --git a/builtin/am.c b/builtin/am.c > index 634f7a7..e7828e5 100644 > --- a/builtin/am.c > +++ b/builtin/am.c > @@ -1191,6 +1191,33 @@ static void NORETURN die_user_resolve(const struct > am_state *state) > exit(128); > } > > +static void am_signoff(struct strbuf *sb) > +{ Hmm, okay, but now we have two similarly named functions am_signoff() and am_append_signoff() which both do nearly similar things, the only difference being am_signoff() operates on a strbuf while am_append_signoff() operates on the "msg" char* field in the am_state, which seems a bit iffy to me. I wonder if the logic could be implemented in am_append_signoff() instead so we have only one function? > + char *cp; > + struct strbuf mine = STRBUF_INIT; > + > + /* Does it end with our own sign-off? */ > + strbuf_addf(&mine, "\n%s%s\n", > + sign_off_header, > + fmt_name(getenv("GIT_COMMITTER_NAME"), > +getenv("GIT_COMMITTER_EMAIL"))); Maybe use git_committer_info() here? > + if (mine.len < sb->len && > + !strcmp(mine.buf, sb->buf + sb->len - mine.len)) Perhaps use ends_with()? > + goto exit; /* no need to duplicate */ > + > + /* Does it have any Signed-off-by: in the text */ > + for (cp = sb->buf; > +cp && *cp && (cp = strstr(cp, sign_off_header)) != NULL; > +cp = strchr(cp, '\n')) { > + if (sb->buf == cp || cp[-1] == '\n') > + break; > + } > + > + strbuf_addstr(sb, mine.buf + !!cp); > +exit: > + strbuf_release(&mine); > +} > + > /** > * Appends signoff to the "msg" field of the am_state. > */ > @@ -1199,7 +1226,7 @@ static void am_append_signoff(struct am_state *state) > struct strbuf sb = STRBUF_INIT; > > strbuf_attach(&sb, state->msg, state->msg_len, state->msg_len); > - append_signoff(&sb, 0, 0); > + am_signoff(&sb); > state->msg = strbuf_detach(&sb, &state->msg_len); > } > > @@ -1303,7 +1330,7 @@ static int parse_mail(struct am_state *state, const > char *mail) > stripspace(&msg, 0); > > if (state->signoff) > - append_signoff(&msg, 0, 0); > + am_signoff(&msg); > > assert(!state->author_name); > state->author_name = strbuf_detach(&author_name, NULL); Thanks again, 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] am: match --signoff to the original scripted version
On Sun, Sep 6, 2015 at 12:56 PM, Junio C Hamano wrote: > diff --git a/builtin/am.c b/builtin/am.c > index 634f7a7..e7828e5 100644 > --- a/builtin/am.c > +++ b/builtin/am.c > @@ -1191,6 +1191,33 @@ static void NORETURN die_user_resolve(const struct > am_state *state) > exit(128); > } > > +static void am_signoff(struct strbuf *sb) > +{ > + char *cp; > + struct strbuf mine = STRBUF_INIT; > + > + /* Does it end with our own sign-off? */ > + strbuf_addf(&mine, "\n%s%s\n", > + sign_off_header, > + fmt_name(getenv("GIT_COMMITTER_NAME"), > +getenv("GIT_COMMITTER_EMAIL"))); > + if (mine.len < sb->len && > + !strcmp(mine.buf, sb->buf + sb->len - mine.len)) > + goto exit; /* no need to duplicate */ > + > + /* Does it have any Signed-off-by: in the text */ > + for (cp = sb->buf; > +cp && *cp && (cp = strstr(cp, sign_off_header)) != NULL; > +cp = strchr(cp, '\n')) { > + if (sb->buf == cp || cp[-1] == '\n') > + break; > + } > + > + strbuf_addstr(sb, mine.buf + !!cp); To add on, I wonder if the above "add a blank line if there is no Signed-off-by: at the beginning of a line" logic could be expressed more succinctly like this: if (!starts_with(sb->buf, "Signed-off-by: ") && !strstr(sb->buf, "\nSigned-off-by: ")) strbuf_addch(sb, '\n'); strbuf_addstr(sb, mine.buf + 1); > +exit: > + strbuf_release(&mine); > +} > + > /** > * Appends signoff to the "msg" field of the am_state. > */ 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: What's cooking in git.git (May 2015, #07; Tue, 26)
Hi, On Fri, May 29, 2015 at 5:52 AM, Junio C Hamano wrote: > Junio C Hamano writes: > >> * pt/pull-tests (2015-05-18) 8 commits >> - t5520: check reflog action in fast-forward merge >> - t5521: test --dry-run does not make any changes >> - t5520: test --rebase failure on unborn branch with index >> - t5520: test --rebase with multiple branches >> - t5520: test work tree fast-forward when fetch updates head >> - t5520: test for failure if index has unresolved entries >> - t5520: test no merge candidates cases >> - t5520: prevent field splitting in content comparisons >> >> Add more test coverage to "git pull". > > Sorry, but I lost track. What's the doneness of this one? Not done yet, but I'm working on it now, so if I don't have any issues I'll have a re-roll out by the end of today or tomorrow. 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 v5 0/8] Improve git-pull test coverage
This is a re-roll of [1]. This patch series improves test coverage of git-pull.sh, and is part of my GSoC project to rewrite git-pull into a builtin. Improving test coverage helps to prevent regressions that could occur due to the rewrite. Thanks Junio, Johannes and Stefan for the reviews last round. [1] http://thread.gmane.org/gmane.comp.version-control.git/269236 Paul Tan (8): t5520: prevent field splitting in content comparisons t5520: test no merge candidates cases t5520: test for failure if index has unresolved entries t5520: test work tree fast-forward when fetch updates head t5520: test --rebase with multiple branches t5520: test --rebase failure on unborn branch with index t5521: test --dry-run does not make any changes t5520: check reflog action in fast-forward merge t/t5520-pull.sh | 198 +++- t/t5521-pull-options.sh | 13 2 files changed, 175 insertions(+), 36 deletions(-) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 1/8] t5520: prevent field splitting in content comparisons
Many tests in t5520 used the following to test the contents of files: test `cat file` = expected or test $(cat file) = expected These 2 forms, however, will be affected by field splitting and, depending on the value of $IFS, may be split into multiple arguments, making the test fail in mysterious ways. Replace the above 2 forms with: test "$(cat file)" = expected as quoting the command substitution will prevent field splitting. Signed-off-by: Paul Tan --- t/t5520-pull.sh | 70 - 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 7efd45b..5e4db67 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -93,9 +93,9 @@ test_expect_success 'test . as a remote' ' echo updated >file && git commit -a -m updated && git checkout copy && - test `cat file` = file && + test "$(cat file)" = file && git pull && - test `cat file` = updated + test "$(cat file)" = updated ' test_expect_success 'the default remote . should not break explicit pull' ' @@ -104,9 +104,9 @@ test_expect_success 'the default remote . should not break explicit pull' ' git commit -a -m modified && git checkout copy && git reset --hard HEAD^ && - test `cat file` = file && + test "$(cat file)" = file && git pull . second && - test `cat file` = modified + test "$(cat file)" = modified ' test_expect_success '--rebase' ' @@ -119,23 +119,23 @@ test_expect_success '--rebase' ' git commit -m "new file" && git tag before-rebase && git pull --rebase . copy && - test $(git rev-parse HEAD^) = $(git rev-parse copy) && - test new = $(git show HEAD:file2) + test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && + test new = "$(git show HEAD:file2)" ' test_expect_success 'pull.rebase' ' git reset --hard before-rebase && test_config pull.rebase true && git pull . copy && - test $(git rev-parse HEAD^) = $(git rev-parse copy) && - test new = $(git show HEAD:file2) + test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && + test new = "$(git show HEAD:file2)" ' test_expect_success 'branch.to-rebase.rebase' ' git reset --hard before-rebase && test_config branch.to-rebase.rebase true && git pull . copy && - test $(git rev-parse HEAD^) = $(git rev-parse copy) && - test new = $(git show HEAD:file2) + test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && + test new = "$(git show HEAD:file2)" ' test_expect_success 'branch.to-rebase.rebase should override pull.rebase' ' @@ -143,8 +143,8 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' ' test_config pull.rebase true && test_config branch.to-rebase.rebase false && git pull . copy && - test $(git rev-parse HEAD^) != $(git rev-parse copy) && - test new = $(git show HEAD:file2) + test "$(git rev-parse HEAD^)" != "$(git rev-parse copy)" && + test new = "$(git show HEAD:file2)" ' # add a feature branch, keep-merge, that is merged into master, so the @@ -163,33 +163,33 @@ test_expect_success 'pull.rebase=false create a new merge commit' ' git reset --hard before-preserve-rebase && test_config pull.rebase false && git pull . copy && - test $(git rev-parse HEAD^1) = $(git rev-parse before-preserve-rebase) && - test $(git rev-parse HEAD^2) = $(git rev-parse copy) && - test file3 = $(git show HEAD:file3.t) + test "$(git rev-parse HEAD^1)" = "$(git rev-parse before-preserve-rebase)" && + test "$(git rev-parse HEAD^2)" = "$(git rev-parse copy)" && + test file3 = "$(git show HEAD:file3.t)" ' test_expect_success 'pull.rebase=true flattens keep-merge' ' git reset --hard before-preserve-rebase && test_config pull.rebase true && git pull . copy && - test $(git rev-parse HEAD^^) = $(git rev-parse copy) && - test file3 = $(git show HEAD:file3.t) + test "$(git rev-p
[PATCH v5 3/8] t5520: test for failure if index has unresolved entries
Commit d38a30d (Be more user-friendly when refusing to do something because of conflict., 2010-01-12) introduced code paths to git-pull which will error out with user-friendly advices if the user is in the middle of a merge or has unmerged files. Implement tests to ensure that git-pull will not run, and will print these advices, if the user is in the middle of a merge or has unmerged files in the index. Signed-off-by: Paul Tan --- Notes: v5: * use "test_commit" t/t5520-pull.sh | 19 +++ 1 file changed, 19 insertions(+) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 4a2c0a1..265c693 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -164,6 +164,25 @@ test_expect_success 'fail if upstream branch does not exist' ' test "$(cat file)" = file ' +test_expect_success 'fail if the index has unresolved entries' ' + git checkout -b third second^ && + test_when_finished "git checkout -f copy && git branch -D third" && + test "$(cat file)" = file && + test_commit modified2 file && + test -z "$(git ls-files -u)" && + test_must_fail git pull . second && + test -n "$(git ls-files -u)" && + cp file expected && + test_must_fail git pull . second 2>err && + test_i18ngrep "Pull is not possible because you have unmerged files" err && + test_cmp expected file && + git add file && + test -z "$(git ls-files -u)" && + test_must_fail git pull . second 2>err && + test_i18ngrep "You have not concluded your merge" err && + test_cmp expected file +' + test_expect_success '--rebase' ' git branch to-rebase && echo modified again > file && -- 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 v5 2/8] t5520: test no merge candidates cases
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) Implement tests for the above 5 cases to ensure that the correct code paths are triggered for each of these cases. Signed-off-by: Paul Tan --- t/t5520-pull.sh | 55 +++ 1 file changed, 55 insertions(+) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 5e4db67..4a2c0a1 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -109,6 +109,61 @@ test_expect_success 'the default remote . should not break explicit pull' ' test "$(cat file)" = modified ' +test_expect_success 'fail if wildcard spec does not match any refs' ' + git checkout -b test copy^ && + test_when_finished "git checkout -f copy && git branch -D test" && + test "$(cat file)" = file && + test_must_fail git pull . "refs/nonexisting1/*:refs/nonexisting2/*" 2>err && + test_i18ngrep "no candidates for merging" err && + test "$(cat file)" = file +' + +test_expect_success 'fail if no branches specified with non-default remote' ' + git remote add test_remote . && + test_when_finished "git remote remove test_remote" && + git checkout -b test copy^ && + test_when_finished "git checkout -f copy && git branch -D test" && + test "$(cat file)" = file && + test_config branch.test.remote origin && + test_must_fail git pull test_remote 2>err && + test_i18ngrep "specify a branch on the command line" err && + test "$(cat file)" = file +' + +test_expect_success 'fail if not on a branch' ' + git remote add origin . && + test_when_finished "git remote remove origin" && + git checkout HEAD^ && + test_when_finished "git checkout -f copy" && + test "$(cat file)" = file && + test_must_fail git pull 2>err && + test_i18ngrep "not currently on a branch" err && + test "$(cat file)" = file +' + +test_expect_success 'fail if no configuration for current branch' ' + git remote add test_remote . && + test_when_finished "git remote remove test_remote" && + git checkout -b test copy^ && + test_when_finished "git checkout -f copy && git branch -D test" && + test_config branch.test.remote test_remote && + test "$(cat file)" = file && + test_must_fail git pull 2>err && + test_i18ngrep "no tracking information" err && + test "$(cat file)" = file +' + +test_expect_success 'fail if upstream branch does not exist' ' + git checkout -b test copy^ && + test_when_finished "git checkout -f copy && git branch -D test" && + test_config branch.test.remote . && + test_config branch.test.merge refs/heads/nonexisting && + test "$(cat file)" = file && + test_must_fail git pull 2>err && + test_i18ngrep "no such ref was fetched" err && + test "$(cat file)" = file +' + test_expect_success '--rebase' ' git branch to-rebase && echo modified again > file && -- 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 v5 8/8] t5520: check reflog action in fast-forward merge
When testing a fast-forward merge with git-pull, check to see if the reflog action is "pull" with the arguments passed to git-pull. While we are in the vicinity, remove the empty line as well. Signed-off-by: Paul Tan --- Notes: v5: * Loosen up the pattern used for search-and-replace of the object ID in the reflog: it may not always be the case that the abbreviated object ID is at least 5 characters. * Only match the beginning of the reflog entry for the object ID. t/t5520-pull.sh | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index a04f55c..af31f04 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -86,7 +86,6 @@ test_expect_success 'pulling into void must not create an octopus' ' ' test_expect_success 'test . as a remote' ' - git branch copy master && git config branch.copy.remote . && git config branch.copy.merge refs/heads/master && @@ -95,7 +94,11 @@ test_expect_success 'test . as a remote' ' git checkout copy && test "$(cat file)" = file && git pull && - test "$(cat file)" = updated + test "$(cat file)" = updated && + git reflog -1 >reflog.actual && + sed "s/^[0-9a-f][0-9a-f]*/OBJID/" reflog.actual >reflog.fuzzy && + echo "OBJID HEAD@{0}: pull: Fast-forward" >reflog.expected && + test_cmp reflog.expected reflog.fuzzy ' test_expect_success 'the default remote . should not break explicit pull' ' @@ -106,7 +109,11 @@ test_expect_success 'the default remote . should not break explicit pull' ' git reset --hard HEAD^ && test "$(cat file)" = file && git pull . second && - test "$(cat file)" = modified + test "$(cat file)" = modified && + git reflog -1 >reflog.actual && + sed "s/^[0-9a-f][0-9a-f]*/OBJID/" reflog.actual >reflog.fuzzy && + echo "OBJID HEAD@{0}: pull . second: Fast-forward" >reflog.expected && + test_cmp reflog.expected reflog.fuzzy ' test_expect_success 'fail if wildcard spec does not match any refs' ' -- 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 v5 5/8] t5520: test --rebase with multiple branches
Since rebasing on top of multiple upstream branches does not make sense, since 51b2ead (disallow providing multiple upstream branches to rebase, pull --rebase, 2009-02-18), git-pull explicitly disallowed specifying multiple branches in the rebase case. Implement tests to ensure that git-pull fails and prints out the user-friendly error message in such a case. Signed-off-by: Paul Tan --- t/t5520-pull.sh | 9 + 1 file changed, 9 insertions(+) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 872d765..90728e0 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -217,6 +217,15 @@ test_expect_success '--rebase' ' test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && test new = "$(git show HEAD:file2)" ' + +test_expect_success '--rebase fails with multiple branches' ' + git reset --hard before-rebase && + test_must_fail git pull --rebase . copy master 2>err && + test "$(git rev-parse HEAD)" = "$(git rev-parse before-rebase)" && + test_i18ngrep "Cannot rebase onto multiple branches" err && + test modified = "$(git show HEAD:file)" +' + test_expect_success 'pull.rebase' ' git reset --hard before-rebase && test_config pull.rebase true && -- 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 v5 7/8] t5521: test --dry-run does not make any changes
Test that when --dry-run is provided to git-pull, it does not make any changes, namely: * --dry-run gets passed to git-fetch, so no FETCH_HEAD will be created and no refs will be fetched. * The index and work tree will not be modified. Signed-off-by: Paul Tan --- t/t5521-pull-options.sh | 13 + 1 file changed, 13 insertions(+) diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh index 453aba5..56e7377 100755 --- a/t/t5521-pull-options.sh +++ b/t/t5521-pull-options.sh @@ -117,4 +117,17 @@ test_expect_success 'git pull --all' ' ) ' +test_expect_success 'git pull --dry-run' ' + test_when_finished "rm -rf clonedry" && + git init clonedry && + ( + cd clonedry && + git pull --dry-run ../parent && + test_path_is_missing .git/FETCH_HEAD && + test_path_is_missing .git/refs/heads/master && + test_path_is_missing .git/index && + test_path_is_missing file + ) +' + test_done -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 6/8] t5520: test --rebase failure on unborn branch with index
Commit 19a7fcb (allow pull --rebase on branch yet to be born, 2009-08-11) special cases git-pull on an unborn branch in a different code path such that git-pull --rebase is still valid even though there is no HEAD yet. This code path still ensures that there is no index in order not to lose any staged changes. Implement a test to ensure that this check is triggered. Signed-off-by: Paul Tan --- Notes: v5: * Move test_i18ngrep into subshell t/t5520-pull.sh | 15 +++ 1 file changed, 15 insertions(+) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 90728e0..a04f55c 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -412,6 +412,21 @@ test_expect_success 'pull --rebase works on branch yet to be born' ' test_cmp expect actual ' +test_expect_success 'pull --rebase fails on unborn branch with staged changes' ' + test_when_finished "rm -rf empty_repo2" && + git init empty_repo2 && + ( + cd empty_repo2 && + echo staged-file >staged-file && + git add staged-file && + test "$(git ls-files)" = staged-file && + test_must_fail git pull --rebase .. master 2>err && + test "$(git ls-files)" = staged-file && + test "$(git show :staged-file)" = staged-file && + test_i18ngrep "unborn branch with changes added to the index" err + ) +' + test_expect_success 'setup for detecting upstreamed changes' ' mkdir src && (cd src && -- 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 v5 4/8] t5520: test work tree fast-forward when fetch updates head
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. Implement tests to ensure that the fast-forward occurs in such a case, as well as to ensure that the user-friendly advice is printed upon failure. Signed-off-by: Paul Tan --- t/t5520-pull.sh | 21 + 1 file changed, 21 insertions(+) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 265c693..872d765 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -183,6 +183,27 @@ test_expect_success 'fail if the index has unresolved entries' ' test_cmp expected file ' +test_expect_success 'fast-forwards working tree if branch head is updated' ' + git checkout -b third second^ && + test_when_finished "git checkout -f copy && git branch -D third" && + test "$(cat file)" = file && + git pull . second:third 2>err && + test_i18ngrep "fetch updated the current branch head" err && + test "$(cat file)" = modified && + test "$(git rev-parse third)" = "$(git rev-parse second)" +' + +test_expect_success 'fast-forward fails with conflicting work tree' ' + git checkout -b third second^ && + test_when_finished "git checkout -f copy && git branch -D third" && + test "$(cat file)" = file && + echo conflict >file && + test_must_fail git pull . second:third 2>err && + test_i18ngrep "Cannot fast-forward your working tree" err && + test "$(cat file)" = conflict && + test "$(git rev-parse third)" = "$(git rev-parse second)" +' + test_expect_success '--rebase' ' git branch to-rebase && echo modified again > file && -- 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 00/14] Make git-pull a builtin
Hi, On Tue, May 19, 2015 at 3:21 AM, Junio C Hamano wrote: > Paul Tan writes: > >> This series rewrites git-pull.sh into a C builtin, thus improving its >> performance and portability. It is part of my GSoC project to rewrite >> git-pull >> and git-am into builtins[2]. > > Earlier you were worried about 'git pull' being used in many tests > for the purpose of testing other parts of the system and not testing > 'pull' itself. For a program as complex as 'git pull', you may want > to take the 'competing implementations' approach. > > (1) write an empty cmd_pull() that relaunches "git pull" scripted > porcelain from the $GIT_EXEC_PATH with given parameters, and > wire all the necessary bits to git.c. > > (2) enhance cmd_pull() a bit by bit, but keep something like this > > if (getenv(GIT_USE_BUILTIN_PULL)) { > /* original run_command("git-pull.sh") code */ > exit $? > } > > ... your "C" version ... > > (3) add "GIT_USE_BUILTIN_PULL=Yes; export GIT_USE_BUILTIN_PULL" at > the beginning of "t55??" test scripts (but not others that rely > on working pull and that are not interested in catching bugs in > pull). > > (4) once cmd_pull() becomes fully operational, drop (3) and also the > conditional one you added in (2), and retire the environment > variable. Retire the git-pull.sh script to contrib/examples/ > boneyard. > > That way, you will always have a reference you can use throughout > the development. > > Just a suggestion, not a requirement. Okay, I'm trying this out in the next re-roll. I do agree that this patch series should not touch anything in t/ at all. One problem(?) is that putting builtins/pull.o in the BUILTIN_OBJS and leaving git-pull.sh in SCRIPT_SH in the Makefile will generate 2 targets to ./git-pull (they will clobber each other). For GNU Make, the last defined target will win, so in this case it just happens that git-pull.sh will win because the build targets for the shell scripts are defined after the build targets for the builtins, so this works in our favor I guess. Regards, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/14] Make git-pull a builtin
On Sat, May 30, 2015 at 3:29 PM, Paul Tan wrote: > Hi, > Okay, I'm trying this out in the next re-roll. I do agree that this > patch series should not touch anything in t/ at all. > > One problem(?) is that putting builtins/pull.o in the BUILTIN_OBJS and > leaving git-pull.sh in SCRIPT_SH in the Makefile will generate 2 > targets to ./git-pull (they will clobber each other). For GNU Make, > the last defined target will win, so in this case it just happens that > git-pull.sh will win because the build targets for the shell scripts > are defined after the build targets for the builtins, so this works in > our favor I guess. > > Regards, > Paul Just to add on, I just discovered that test-lib.sh unsets all GIT_* environment variables "for repeatability", so the name "GIT_USE_BUILTIN_PULL" cannot be used. I'm tempted to just add a underscore just before the name ("_GIT_USE_BUILTIN_PULL") to work around this, since it's just a temporary thing. Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/14] pull: teach git pull about --rebase
Hi Johannes, On Tue, May 19, 2015 at 9:04 PM, Johannes Schindelin wrote: > On 2015-05-18 17:06, Paul Tan wrote: >> +/** >> + * Given a refspec, returns the merge branch. Returns NULL if the refspec >> src >> + * does not refer to a branch. >> + * >> + * FIXME: It should return the tracking branch. Currently only works with >> the >> + * default mapping. >> + */ >> +static char *get_merge_branch_2(const char *repo, const char *refspec) >> +{ >> + struct refspec *spec; >> + const char *remote; >> + char *merge_branch; >> + >> + spec = parse_fetch_refspec(1, &refspec); >> + remote = spec->src; >> + if (!*remote || !strcmp(remote, "HEAD")) >> + remote = "HEAD"; >> + else if (skip_prefix(remote, "heads/", &remote)) >> + ; >> + else if (skip_prefix(remote, "refs/heads/", &remote)) >> + ; >> + else if (starts_with(remote, "refs/") || >> + starts_with(remote, "tags/") || >> + starts_with(remote, "remotes/")) >> + remote = ""; >> + >> + if (*remote) { >> + if (!strcmp(repo, ".")) >> + merge_branch = mkpathdup("refs/heads/%s", remote); >> + else >> + merge_branch = mkpathdup("refs/remotes/%s/%s", repo, >> remote); >> + } else >> + merge_branch = NULL; >> + >> + free_refspec(1, spec); >> + return merge_branch; >> +} > > I have to admit that it took me a substantial amount of time to deduce from > the code what `get_merge_branch_2()` really does (judging from the > description, I thought that it would be essentially the same as > `get_merge_branch_1()` without the hard-coded "HEAD"). If the comment above > the function would have said something like: > > /** > * Given a refspec, returns the name of the local tracking ref. > */ > > I would have had an easier time. Also, I wonder if something like this would > do the job: Yeah whoops, this came from a confusion over the difference over "merge branch" and "remote tracking branch". A merge branch would be a remote tracking branch, but a remote tracking branch is not necessarily a merge branch. > spec = parse_fetch_refspec(1, &refspec); > if (spec->dst) > return spec->dst; Hmm, I notice that get_remote_merge_branch() only looks at the src part of the refspec. However, I guess it is true that if the dst part is provided, the user may be wishing for that to be interpreted as the "remote tracking branch", so we should be looking at it to calculate the fork point. > if (!(remote = get_remote(remote_name))) > return NULL; > if (remote_find_tracking(remote, spec)) > return spec->dst; ... and if the dst part of the refspec is not provided, we fall back to see if there is any remote tracking branch in the repo for the src part of the ref, which matches the intention of get_remote_merge_branch() I think, while being better because remote_find_tracking() takes into account the actual configured fetch refspecs for the remote. However, we also need to consider if the user provided a wildcard refspec, as it will not make sense in this case. From my reading, remote_find_tracking(), which calls query_refspecs(), would just match the src part literally, so I guess we should explicitly detect and error out in this case. > return NULL; > > (I guess we'd have to `xstrdup()` the return values because the return value > of `get_merge_branch_1()` needs to be `free()`d, but maybe we can avoid so > much `malloc()/free()`ing.) Again, the function name should probably be > changed to something clearer, maybe to `get_tracking_branch()`. Yeah, it should be changed to get_tracking_branch(), since it is different from get_merge_branch(). > One thing that is not clear at all to me is whether > > git pull --rebase origin master next > > would error out as expected, or simply rebase to `origin/master`. In git-pull.sh, for the rebase fork point calculation it just used the first refspec. However, when running git-rebase it checked to see if there was only one merge base, which is replicated here: @@ -573,6 +792,10 @@ int cmd_pull(int argc, const char **argv, const char *prefix) if (merge_heads.nr > 1) die(_("Cannot merge multiple branches into empty head.")); return pull_into_void(*merge_heads.s
Re: [PATCH 11/14] pull: teach git pull about --rebase
On Sun, May 31, 2015 at 4:18 PM, Paul Tan wrote: > On Tue, May 19, 2015 at 9:04 PM, Johannes Schindelin > wrote: >> Also, I wonder if something like this would do the job: >> spec = parse_fetch_refspec(1, &refspec); >> if (spec->dst) >> return spec->dst; > > Hmm, I notice that get_remote_merge_branch() only looks at the src > part of the refspec. However, I guess it is true that if the dst part > is provided, the user may be wishing for that to be interpreted as the > "remote tracking branch", so we should be looking at it to calculate > the fork point. > >> if (!(remote = get_remote(remote_name))) >> return NULL; >> if (remote_find_tracking(remote, spec)) >> return spec->dst; > > ... and if the dst part of the refspec is not provided, we fall back > to see if there is any remote tracking branch in the repo for the src > part of the ref, which matches the intention of > get_remote_merge_branch() I think, while being better because > remote_find_tracking() takes into account the actual configured fetch > refspecs for the remote. > > However, we also need to consider if the user provided a wildcard > refspec, as it will not make sense in this case. From my reading, > remote_find_tracking(), which calls query_refspecs(), would just match > the src part literally, so I guess we should explicitly detect and > error out in this case. With all that said, after thinking about it I feel that this patch series should focus solely on rewriting git-pull.sh 1:1. While I do agree with the above suggested improvements, I think they should be implemented as separated patch(es) on top of this series since we would be technically changing git-pull's behavior, even if we are improving it. As such, the issue that I think should be focused on for this patch is: does get_merge_branch_1() and get_merge_branch_2() in this patch implement the same behavior as get_remote_merge_branch() in git-parse.remote.sh? If it does, then its purpose is fulfilled. So, I'll keep the overall logic of get_merge_branch_2() the same for the next re-roll. (Other than renaming the function and fixing code style issues). Once this series is okay, I'll look into doing a separate patch on top that changes the function to use remote_find_tracking() so that we fix the assumption that the default fetch mapping is used. The other possibility is that we fix this in git-parse-remote.sh, but I'm personally getting a bit tired from having to re-implement the same thing in shell script and C. Furthermore, the only script using get_remote_merge_branch() is git-pull.sh. > [...] > Since this is just a 1:1 rewrite I just tried to keep as close to the > original as possible. However, thinking about it, since we *are* just > using the first refspec for fork point calculation, I do agree that we > should probably give an warning() here as well if the user provided > more than one refspec, like "Cannot calculate rebase fork point as you > provided more than one refspec. git-pull will not be able to handle a > rebased upstream". I do not think it is fatal enough that we should > error() or die(), as e.g. the first refspec may be a wildcard refspec > that matches nothing, and the second refspec that matches one merge > head for rebasing. This is pretty contrived though, but still > technically legitimate. I dunno. >[...] >> We should probably `return error(_"No tracking branch found for %s@, refspec >> ? refspec : "HEAD");` so that the user has a chance to understand that there >> has been a problem and how to solve it. > > Just like the above, I don't think this is serious enough to be > considered an error() though. Sure, this means that we cannot properly > handle the case where the upstream has been rebased, but this is not > always the case. We could probably have a warning() here, but I think > the message should be improved to tell the user what exactly she is > losing out on. e.g. "No tracking branch found for %s. git-pull will > not be able to handle a rebased upstream." Likewise, I won't introduce the error()s or warning()s for now. Other than that, all other code style related issues have been/will be fixed. Thanks for the reviews. Regards, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] pull: use git-rev-parse --parseopt for option parsing
To enable unambiguous parsing of abbreviated options, bundled short options, separate form options and to provide consistent usage help, use git-rev-parse --parseopt for option parsing. With this, simplify the option parsing code. Signed-off-by: Paul Tan --- Notes: v2 * Don't use OPTIONS_KEEPDASHDASH git-pull.sh | 97 - 1 file changed, 57 insertions(+), 40 deletions(-) diff --git a/git-pull.sh b/git-pull.sh index 623ba7a..a814bf6 100755 --- a/git-pull.sh +++ b/git-pull.sh @@ -4,13 +4,53 @@ # # Fetch one or more remote refs and merge it/them into the current HEAD. -USAGE='[-n | --no-stat] [--[no-]commit] [--[no-]squash] [--[no-]ff|--ff-only] [--[no-]rebase|--rebase=preserve] [-s strategy]... [] ...' -LONG_USAGE='Fetch one or more remote refs and integrate it/them with the current HEAD.' SUBDIRECTORY_OK=Yes -OPTIONS_SPEC= +OPTIONS_KEEPDASHDASH= +OPTIONS_STUCKLONG=Yes +OPTIONS_SPEC="\ +git pull [options] [ [...]] + +Fetch one or more remote refs and integrate it/them with the current HEAD. +-- +v,verbose be more verbose +q,quietbe more quiet +progress force progress reporting + + Options related to merging +r,rebase?false|true|preserve incorporate changes by rebasing rather than merging +n! do not show a diffstat at the end of the merge +stat show a diffstat at the end of the merge +summary(synonym to --stat) +log?n add (at most ) entries from shortlog to merge commit message +squash create a single commit instead of doing a merge +commit perform a commit if the merge succeeds (default) +e,edit edit message before committing +ff allow fast-forward +ff-only! abort if fast-forward is not possible +verify-signatures verify that the named commit has a valid GPG signature +s,strategy=strategymerge strategy to use +X,strategy-option=option option for selected merge strategy +S,gpg-sign?key-id GPG sign commit + + Options related to fetching +allfetch from all remotes +a,append append to .git/FETCH_HEAD instead of overwriting +upload-pack=path path to upload pack on remote end +f,forceforce overwrite of local branch +t,tags fetch all tags and associated objects +p,pruneprune remote-tracking branches no longer on remote +recurse-submodules?on-demand control recursive fetching of submodules +dry-rundry run +k,keep keep downloaded pack +depth=depthdeepen history of shallow clone +unshallow convert to a complete repository +update-shallow accept refs that update .git/shallow +refmap=refmap specify fetch refmap +" +test $# -gt 0 && args="$*" . git-sh-setup . git-sh-i18n -set_reflog_action "pull${1+ $*}" +set_reflog_action "pull${args+ $args}" require_work_tree_exists cd_to_toplevel @@ -87,17 +127,17 @@ do diffstat=--stat ;; --log|--log=*|--no-log) log_arg="$1" ;; - --no-c|--no-co|--no-com|--no-comm|--no-commi|--no-commit) + --no-commit) no_commit=--no-commit ;; - --c|--co|--com|--comm|--commi|--commit) + --commit) no_commit=--commit ;; -e|--edit) edit=--edit ;; --no-edit) edit=--no-edit ;; - --sq|--squ|--squa|--squas|--squash) + --squash) squash=--squash ;; - --no-sq|--no-squ|--no-squa|--no-squas|--no-squash) + --no-squash) squash=--no-squash ;; --ff) no_ff=--ff ;; @@ -105,39 +145,19 @@ do no_ff=--no-ff ;; --ff-only) ff_only=--ff-only ;; - -s=*|--s=*|--st=*|--str=*|--stra=*|--strat=*|--strate=*|\ - --strateg=*|--strategy=*|\ - -s|--s|--st|--str|--stra|--strat|--strate|--strateg|--strategy) - case "$#,$1" in - *,*=*) - strategy=$(expr "z$1" : 'z-[^=]*=\(.*\)') ;; - 1,*) - usage ;; - *) - strategy="$2" - shift ;; - esac - strategy_args="${strategy_args}-s $strategy " + -s*|--strategy=*) + strategy_args="$strategy_args $1" ;; - -X*) - case "$#,$1" in - 1,-X) - usage ;; - *,-X) - xx="-X $(git rev-par
[PATCH v2 0/2] Improve git-pull's option parsing
This is a re-roll of [v1]. Thanks Johannes for the reviews last round. Previous versions: [v1] http://thread.gmane.org/gmane.comp.version-control.git/269249 This patch series is based on pt/pull-tests. While parsing the command-line arguments, git-pull stops parsing at the first unrecognized option, assuming that any subsequent options are for git-fetch, and can thus be kept in the shell's positional parameters list, so that it can be passed to git-fetch via the expansion of "$@". However, certain functions in git-pull assume that the positional parameters do not contain any options. Fix this by making git-pull handle git-fetch's options as well at the option parsing stage. With this change in place, we can move on to migrate git-pull to use git-rev-parse --parseopt such that its option parsing is consistent with the other git commands. I believe this is the last required behavior change for my rewrite of git-pull.sh to C. Paul Tan (2): pull: handle git-fetch's options as well pull: use git-rev-parse --parseopt for option parsing Documentation/git-pull.txt | 3 -- git-pull.sh| 128 +++-- t/t5520-pull.sh| 20 +++ t/t5521-pull-options.sh| 14 + 4 files changed, 122 insertions(+), 43 deletions(-) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] pull: handle git-fetch's options as well
While parsing the command-line arguments, git-pull stops parsing at the first unrecognized option, assuming that any subsequent options are for git-fetch, and can thus be kept in the shell's positional parameters list, so that it can be passed to git-fetch via the expansion of "$@". However, certain functions in git-pull assume that the positional parameters do not contain any options: * error_on_no_merge_candidates() uses the number of positional parameters to determine which error message to print out, and will thus print the wrong message if git-fetch's options are passed in as well. * the call to get_remote_merge_branch() assumes that the positional parameters only contains the optional repo and refspecs, and will thus silently fail if git-fetch's options are passed in as well. * --dry-run is a valid git-fetch option, but if provided after any git-fetch options, it is not recognized by git-pull and thus git-pull will continue to run the merge or rebase. Fix these bugs by teaching git-pull to parse git-fetch's options as well. Add tests to prevent regressions. This removes the limitation where git-fetch's options have to come after git-merge's and git-rebase's options on the command line. Update the documentation to reflect this. Signed-off-by: Paul Tan --- Notes: Improve git-pull's option parsing Previous versions: [v1] http://thread.gmane.org/gmane.comp.version-control.git/269249 This patch series is based on pt/pull-tests. While parsing the command-line arguments, git-pull stops parsing at the first unrecognized option, assuming that any subsequent options are for git-fetch, and can thus be kept in the shell's positional parameters list, so that it can be passed to git-fetch via the expansion of "$ ". However, certain functions in git-pull assume that the positional parameters do not contain any options. Fix this by making git-pull handle git-fetch's options as well at the option parsing stage. With this change in place, we can move on to migrate git-pull to use git-rev-parse --parseopt such that its option parsing is consistent with the other git commands. I believe this is the last required behavior change for my rewrite of git-pull.sh to C. v2 * Initialize variables to prevent them from being set in the command line. * Update the documentation as well. Documentation/git-pull.txt | 3 --- git-pull.sh| 37 ++--- t/t5520-pull.sh| 20 t/t5521-pull-options.sh| 14 ++ 4 files changed, 68 insertions(+), 6 deletions(-) diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt index 712ab4b..93c72a2 100644 --- a/Documentation/git-pull.txt +++ b/Documentation/git-pull.txt @@ -74,9 +74,6 @@ pulling or stash them away with linkgit:git-stash[1]. OPTIONS --- -Options meant for 'git pull' itself and the underlying 'git merge' -must be given before the options meant for 'git fetch'. - -q:: --quiet:: This is passed to both underlying git-fetch to squelch reporting of diff --git a/git-pull.sh b/git-pull.sh index 0917d0d..623ba7a 100755 --- a/git-pull.sh +++ b/git-pull.sh @@ -44,7 +44,8 @@ bool_or_string_config () { strategy_args= diffstat= no_commit= squash= no_ff= ff_only= log_arg= verbosity= progress= recurse_submodules= verify_signatures= -merge_args= edit= rebase_args= +merge_args= edit= rebase_args= all= append= upload_pack= force= tags= prune= +keep= depth= unshallow= update_shallow= refmap= curr_branch=$(git symbolic-ref -q HEAD) curr_branch_short="${curr_branch#refs/heads/}" rebase=$(bool_or_string_config branch.$curr_branch_short.rebase) @@ -166,11 +167,39 @@ do --d|--dr|--dry|--dry-|--dry-r|--dry-ru|--dry-run) dry_run=--dry-run ;; + --all|--no-all) + all=$1 ;; + -a|--append|--no-append) + append=$1 ;; + --upload-pack=*|--no-upload-pack) + upload_pack=$1 ;; + -f|--force|--no-force) + force="$force $1" ;; + -t|--tags|--no-tags) + tags=$1 ;; + -p|--prune|--no-prune) + prune=$1 ;; + -k|--keep|--no-keep) + keep=$1 ;; + --depth=*|--no-depth) + depth=$1 ;; + --unshallow|--no-unshallow) + unshallow=$1 ;; + --update-shallow|--no-update-shallow) + update_shallow=$1 ;; + --refmap=*|--no-refmap) + refmap=$1 ;; -h|--help-all) usage ;; + --) + shift + break + ;; + -*) + usage + ;; *) - # Pass thru anything tha
Re: [PATCH/WIP 4/8] am: split out mbox/maildir patches with git-mailsplit
On Fri, May 29, 2015 at 7:05 AM, Eric Sunshine wrote: > On Wed, May 27, 2015 at 9:33 AM, Paul Tan wrote: >> @@ -128,13 +190,32 @@ static void am_next(struct am_state *state) >> */ >> +/** >> + * parse_options() callback that validates and sets opt->value to the >> + * PATCH_FORMAT_* enum value corresponding to `arg`. >> + */ >> +static int parse_opt_patchformat(const struct option *opt, const char *arg, >> int unset) >> +{ >> + int *opt_value = opt->value; >> + >> + if (!strcmp(arg, "mbox")) >> + *opt_value = PATCH_FORMAT_MBOX; >> + else >> + return -1; >> + return 0; >> +} >> + >> struct am_state state; >> +int opt_patch_format; > > Should these two variables be static? Whoops. Yes, they should. Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v2 2/2] git-am: add am.threeWay config variable
Hi, On Tue, Jun 2, 2015 at 9:37 PM, Matthieu Moy wrote: > Remi Lespinet writes: > >> +if test "$(git config --bool --get am.threeWay)" = true >> +then >> +threeway=t >> +fi > > I think you missed Paul's remark on this: > > http://article.gmane.org/gmane.comp.version-control.git/270150 > > Not terribly important since am will be rewritten soon, though. As the person who had to do four preparatory patch series' to fix bugs for the rewrite of git-pull, I respectfully disagree ;-) Regards, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pull: allow dirty tree when rebase.autostash enabled
Hi, Some comments which may not necessarily be correct. On Wed, Jun 3, 2015 at 5:55 AM, Kevin Daudt wrote: > rebase learned to stash changes when it encounters a dirty work tree, but > git pull --rebase does not. > > Only verify if the working tree is dirty when rebase.autostash is not > enabled. > --- Missing sign-off. > git-pull.sh | 5 - > t/t5520-pull.sh | 17 + > 2 files changed, 21 insertions(+), 1 deletion(-) > > diff --git a/git-pull.sh b/git-pull.sh > index 0917d0d..6b9e8a3 100755 > --- a/git-pull.sh > +++ b/git-pull.sh > @@ -239,7 +239,10 @@ test true = "$rebase" && { > die "$(gettext "updating an unborn branch with > changes added to the index")" > fi > else > - require_clean_work_tree "pull with rebase" "Please commit or > stash them." > + if [ $(git config --bool --get rebase.autostash || echo > false) = "false" ] "false" doesn't need to be quoted. > + then > + require_clean_work_tree "pull with rebase" "Please > commit or stash them." > + fi > fi > oldremoteref= && > test -n "$curr_branch" && > diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh > index 7efd45b..d849a19 100755 > --- a/t/t5520-pull.sh > +++ b/t/t5520-pull.sh > @@ -297,6 +297,23 @@ test_expect_success 'pull --rebase dies early with dirty > working directory' ' > > ' > > +test_expect_success 'pull --rebase succeeds with dirty working directory and > rebase.autostash set' ' > + I know the surrounding old tests use a newline, but I think that all new tests should use the modern style of not having a newline, since t5520 already consists of a mix of old and modern styles anyway. > + test_when_finished "git rm -f file4" && There is trailing whitespace here. Furthermore, git rm -f will fail if "file4" does not exist in the index. Perhaps it should be moved below the "git add" below. > + git checkout to-rebase && > + git update-ref refs/remotes/me/copy copy^ && > + COPY=$(git rev-parse --verify me/copy) && $COPY is not used anywhere in the test. > + git rebase --onto $COPY copy && > + test_config branch.to-rebase.remote me && > + test_config branch.to-rebase.merge refs/heads/copy && > + test_config branch.to-rebase.rebase true && > + test_config rebase.autostash true && > + echo dirty >> file4 && file4 does not exist, so we don't need to append to it. I know the above few tests do not adhere to it, but CodingGuidelines says that redirection operators do not have a space after > + git add file4 && > + git pull I think we should check for file contents to ensure that git-pull/git-stash/git-rebase is doing its job properly. > + Same as above, no need the newline. > +' > + With all that said, I wonder if this test, and the test above ("pull --rebase dies early with dirty working directory") could be vastly simplified, since we are not testing if we can handle a rebased upstream. E.g., my simplified version for the above test would be something like: git checkout -f to-rebase && git rebase --onto copy^ copy && test_config rebase.autostash true && echo dirty >file4 && git add file4 && test_when_finished "git rm -f file4" && git pull --rebase . me/copy && test "$(cat file4)" = dirty && test "$(cat file2)" = file It's still confusing though, because we cannot take advantage of the 'before-rebase' tag introduced in the above tests. I would much prefer if this test and the ("pull --rebase dies with dirty working directory") test could be moved to the --rebase tests at lines 214+. Also, this section in the t5520 test suite always gives me a headache trying to decipher what it is trying to do >< Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 00/19] Make git-pull a builtin
This is a re-roll of [v1]. Thanks Johannes, Stefan and Junio for the reviews last round. Previous versions: [v1] http://thread.gmane.org/gmane.comp.version-control.git/269258 git-pull is a commonly executed command to check for new changes in the upstream repository and, if there are, fetch and integrate them into the current branch. Currently it is implemented by the shell script git-pull.sh. However, compared to C, shell scripts have certain deficiencies -- they need to spawn a lot of processes, introduce a lot of dependencies and cannot take advantage of git's internal caches. This series rewrites git-pull.sh into a C builtin, thus improving its performance and portability. It is part of my GSoC project to rewrite git-pull and git-am into builtins[1]. [1] https://gist.github.com/pyokagan/1b7b0d1f4dab6ba3cef1 Paul Tan (19): parse-options-cb: implement parse_opt_pass_strbuf() parse-options-cb: implement parse_opt_pass_argv_array() argv-array: implement argv_array_pushv() pull: implement skeletal builtin pull pull: implement fetch + merge pull: pass verbosity, --progress flags to fetch and merge pull: pass git-merge's options to git-merge pull: pass git-fetch's options to git-fetch pull: error on no merge candidates pull: support pull.ff config pull: check if in unresolved merge state pull: fast-forward working tree if head is updated pull: implement pulling into an unborn branch pull: set reflog message pull: teach git pull about --rebase pull: configure --rebase via branch..rebase or pull.rebase pull --rebase: exit early when the working directory is dirty pull --rebase: error on no merge candidate cases pull: remove redirection to git-pull.sh Documentation/technical/api-argv-array.txt | 3 + Makefile| 2 +- advice.c| 8 + advice.h| 1 + argv-array.c| 6 + argv-array.h| 1 + builtin.h | 1 + builtin/pull.c | 888 git-pull.sh => contrib/examples/git-pull.sh | 0 git.c | 1 + parse-options-cb.c | 61 ++ parse-options.h | 2 + 12 files changed, 973 insertions(+), 1 deletion(-) create mode 100644 builtin/pull.c rename git-pull.sh => contrib/examples/git-pull.sh (100%) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 02/19] parse-options-cb: implement parse_opt_pass_argv_array()
Certain git commands, such as git-pull, are simply wrappers around other git commands like git-fetch, git-merge and git-rebase. As such, these wrapper commands will typically need to "pass through" command-line options of the commands they wrap. Implement the parse_opt_pass_argv_array() parse-options callback, which will reconstruct all the provided command-line options into an argv_array, such that it can be passed to another git command. This is useful for passing command-line options that can be specified multiple times. Signed-off-by: Paul Tan --- Notes: v2 * This function is a requirement for the rewrite of git-am to handle passing git-apply's options to git-apply. Since it would be implemented anyway I thought it would be good if git-pull could take advantage of it as well to handle --strategy and --strategy-option. parse-options-cb.c | 32 parse-options.h| 1 + 2 files changed, 33 insertions(+) diff --git a/parse-options-cb.c b/parse-options-cb.c index 5b1dbcf..7330506 100644 --- a/parse-options-cb.c +++ b/parse-options-cb.c @@ -4,6 +4,7 @@ #include "commit.h" #include "color.h" #include "string-list.h" +#include "argv-array.h" /*- some often used options -*/ @@ -163,3 +164,34 @@ int parse_opt_pass_strbuf(const struct option *opt, const char *arg, int unset) return 0; } + +/** + * For an option opt, recreate the command-line option, appending it to + * opt->value which must be a argv_array. This is useful when we need to pass + * the command-line option, which can be specified multiple times, to another + * command. + */ +int parse_opt_pass_argv_array(const struct option *opt, const char *arg, int unset) +{ + struct strbuf sb = STRBUF_INIT; + struct argv_array *opt_value = opt->value; + + if (opt->long_name) { + strbuf_addstr(&sb, unset ? "--no-" : "--"); + strbuf_addstr(&sb, opt->long_name); + if (arg) { + strbuf_addch(&sb, '='); + strbuf_addstr(&sb, arg); + } + } else if (opt->short_name && !unset) { + strbuf_addch(&sb, '-'); + strbuf_addch(&sb, opt->short_name); + if (arg) + strbuf_addstr(&sb, arg); + } else + return -1; + + argv_array_push(opt_value, sb.buf); + strbuf_release(&sb); + return 0; +} diff --git a/parse-options.h b/parse-options.h index 1d21398..b663f87 100644 --- a/parse-options.h +++ b/parse-options.h @@ -225,6 +225,7 @@ extern int parse_opt_tertiary(const struct option *, const char *, int); extern int parse_opt_string_list(const struct option *, const char *, int); extern int parse_opt_noop_cb(const struct option *, const char *, int); extern int parse_opt_pass_strbuf(const struct option *, const char *, int); +extern int parse_opt_pass_argv_array(const struct option *, const char *, int); #define OPT__VERBOSE(var, h) OPT_COUNTUP('v', "verbose", (var), (h)) #define OPT__QUIET(var, h)OPT_COUNTUP('q', "quiet", (var), (h)) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 01/19] parse-options-cb: implement parse_opt_pass_strbuf()
Certain git commands, such as git-pull, are simply wrappers around other git commands like git-fetch, git-merge and git-rebase. As such, these wrapper commands will typically need to "pass through" command-line options of the commands they wrap. Implement the parse_opt_pass_strbuf() parse-options callback, which will reconstruct the command-line option into an strbuf, such that it can be passed to another git command. Helped-by: Johannes Schindelin Signed-off-by: Paul Tan --- Notes: v2 * Previously implemented as a static function in builtin/pull.c. It now uses an strbuf instead of returning newly-allocated strings, to make memory management easier. * We don't use defval anymore. Instead, we use long_name and short_name. parse-options-cb.c | 29 + parse-options.h| 1 + 2 files changed, 30 insertions(+) diff --git a/parse-options-cb.c b/parse-options-cb.c index be8c413..5b1dbcf 100644 --- a/parse-options-cb.c +++ b/parse-options-cb.c @@ -134,3 +134,32 @@ int parse_opt_noop_cb(const struct option *opt, const char *arg, int unset) { return 0; } + +/** + * For an option opt, recreates the command-line option in opt->value which + * must be an strbuf. This is useful when we need to pass the command-line + * option to another command. + */ +int parse_opt_pass_strbuf(const struct option *opt, const char *arg, int unset) +{ + struct strbuf *sb = opt->value; + + strbuf_reset(sb); + + if (opt->long_name) { + strbuf_addstr(sb, unset ? "--no-" : "--"); + strbuf_addstr(sb, opt->long_name); + if (arg) { + strbuf_addch(sb, '='); + strbuf_addstr(sb, arg); + } + } else if (opt->short_name && !unset) { + strbuf_addch(sb, '-'); + strbuf_addch(sb, opt->short_name); + if (arg) + strbuf_addstr(sb, arg); + } else + return -1; + + return 0; +} diff --git a/parse-options.h b/parse-options.h index c71e9da..1d21398 100644 --- a/parse-options.h +++ b/parse-options.h @@ -224,6 +224,7 @@ extern int parse_opt_with_commit(const struct option *, const char *, int); extern int parse_opt_tertiary(const struct option *, const char *, int); extern int parse_opt_string_list(const struct option *, const char *, int); extern int parse_opt_noop_cb(const struct option *, const char *, int); +extern int parse_opt_pass_strbuf(const struct option *, const char *, int); #define OPT__VERBOSE(var, h) OPT_COUNTUP('v', "verbose", (var), (h)) #define OPT__QUIET(var, h)OPT_COUNTUP('q', "quiet", (var), (h)) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 07/19] pull: pass git-merge's options to git-merge
Specify git-merge's options in the option list, and pass any specified options to git-merge. These options are: * -n, --stat, --summary: since d8abe14 (merge, pull: introduce '--(no-)stat' option, 2008-04-06) * --log: since efb779f (merge, pull: add '--(no-)log' command line option, 2008-04-06) * --squash: since 7d0c688 (git-merge --squash, 2006-06-23) * --commit: since 5072a32 (Teach git-pull about --[no-]ff, --no-squash and --commit, 2007-10-29) * --edit: since 8580830 ("git pull" doesn't know "--edit", 2012-02-11) * --ff, --ff-only: since 5072a32 (Teach git-pull about --[no-]ff, --no-squash and --commit, 2007-10-29) * --verify-signatures: since efed002 (merge/pull: verify GPG signatures of commits being merged, 2013-03-31) * -s, --strategy: since 60fb5b2 (Use git-merge in git-pull (second try)., 2005-09-25) * -X, --strategy-option: since ee2c795 (Teach git-pull to pass -X to git-merge, 2009-11-25) * -S, --gpg-sign: since ea230d8 (pull: add the --gpg-sign option., 2014-02-10) Signed-off-by: Paul Tan --- Notes: v2 * Use opt_parse_pass_strbuf(), opt_parse_pass_argv_array() and argv_array_pushv() builtin/pull.c | 75 ++ 1 file changed, 75 insertions(+) diff --git a/builtin/pull.c b/builtin/pull.c index c9c2cc0..5f08634 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -20,6 +20,18 @@ static const char * const pull_usage[] = { static int opt_verbosity; static struct strbuf opt_progress = STRBUF_INIT; +/* Options passed to git-merge */ +static struct strbuf opt_diffstat = STRBUF_INIT; +static struct strbuf opt_log = STRBUF_INIT; +static struct strbuf opt_squash = STRBUF_INIT; +static struct strbuf opt_commit = STRBUF_INIT; +static struct strbuf opt_edit = STRBUF_INIT; +static struct strbuf opt_ff = STRBUF_INIT; +static struct strbuf opt_verify_signatures = STRBUF_INIT; +static struct argv_array opt_strategies = ARGV_ARRAY_INIT; +static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT; +static struct strbuf opt_gpg_sign = STRBUF_INIT; + static struct option pull_options[] = { /* Shared options */ OPT__VERBOSITY(&opt_verbosity), @@ -27,6 +39,49 @@ static struct option pull_options[] = { N_("force progress reporting"), PARSE_OPT_NOARG, parse_opt_pass_strbuf}, + /* Options passed to git-merge */ + OPT_GROUP(N_("Options related to merging")), + { OPTION_CALLBACK, 'n', NULL, &opt_diffstat, NULL, + N_("do not show a diffstat at the end of the merge"), + PARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 0, "stat", &opt_diffstat, NULL, + N_("show a diffstat at the end of the merge"), + PARSE_OPT_NOARG, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 0, "summary", &opt_diffstat, NULL, + N_("(synonym to --stat)"), + PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 0, "log", &opt_log, N_("n"), + N_("add (at most ) entries from shortlog to merge commit message"), + PARSE_OPT_OPTARG, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 0, "squash", &opt_squash, NULL, + N_("create a single commit instead of doing a merge"), + PARSE_OPT_NOARG, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 0, "commit", &opt_commit, NULL, + N_("perform a commit if the merge succeeds (default)"), + PARSE_OPT_NOARG, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 0, "edit", &opt_edit, NULL, + N_("edit message before committing"), + PARSE_OPT_NOARG, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 0, "ff", &opt_ff, NULL, + N_("allow fast-forward"), + PARSE_OPT_NOARG, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 0, "ff-only", &opt_ff, NULL, + N_("abort if fast-forward is not possible"), + PARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 0, "verify-signatures", &opt_verify_signatures, NULL, + N_("verify that the named commit has a valid GPG signature"), + PARSE_OPT_NOARG, parse_opt_pass_strbuf }, + { OPTION_CALLBACK, 's', "strategy", &opt_strategies, N_("strategy"), + N_("merge strategy to use"), + 0, parse_opt_pass_argv_array }, + { OPTION_CALLBACK, 'X', "strategy-option", &opt_strategy_opts, + N_("option=value"), + N_("option for selected merge strategy"), + 0, parse_opt_pass_argv_array }, + { OPTION_CAL
[PATCH v2 09/19] pull: error on no merge candidates
Commit a8c9bef (pull: improve advice for unconfigured error case, 2009-10-05) fully established the current advices given by git-pull for the different cases where git-fetch will not have anything marked for merge: 1. We fetched from a specific remote, and a refspec was given, but it ended up not fetching anything. This is usually because the user provided a wildcard refspec which had no matches on the remote end. 2. We fetched from a non-default remote, but didn't specify a branch to merge. We can't use the configured one because it applies to the default remote, and thus the user must specify the branches to merge. 3. We fetched from the branch's or repo's default remote, but: a. We are not on a branch, so there will never be a configured branch to merge with. b. We are on a branch, but there is no configured branch to merge with. 4. We fetched from the branch's or repo's default remote, but the configured branch to merge didn't get fetched (either it doesn't exist, or wasn't part of the configured fetch refspec) Re-implement the above behavior by implementing get_merge_heads() to parse the heads in FETCH_HEAD for merging, and implementing die_no_merge_candidates(), which will be called when FETCH_HEAD has no heads for merging. Helped-by: Johannes Schindelin Signed-off-by: Paul Tan --- Notes: v2 * Switched to using fprintf_ln() which will append the trailing newline at the end for us. * The die_no_merge_candidates() code has been reorganized a bit to support the later patch which will tweak the messages to be aware of git-pull --rebase. builtin/pull.c | 113 + 1 file changed, 113 insertions(+) diff --git a/builtin/pull.c b/builtin/pull.c index 0b66b43..42a061d 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -10,6 +10,8 @@ #include "parse-options.h" #include "exec_cmd.h" #include "run-command.h" +#include "sha1-array.h" +#include "remote.h" static const char * const pull_usage[] = { N_("git pull [options] [ [...]]"), @@ -165,6 +167,111 @@ static void argv_push_force(struct argv_array *arr) } /** + * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge + * into merge_heads. + */ +static void get_merge_heads(struct sha1_array *merge_heads) +{ + const char *filename = git_path("FETCH_HEAD"); + FILE *fp; + struct strbuf sb = STRBUF_INIT; + unsigned char sha1[GIT_SHA1_RAWSZ]; + + if (!(fp = fopen(filename, "r"))) + die_errno(_("could not open '%s' for reading"), filename); + while(strbuf_getline(&sb, fp, '\n') != EOF) { + if (get_sha1_hex(sb.buf, sha1)) + continue; /* invalid line: does not start with SHA1 */ + if (starts_with(sb.buf + GIT_SHA1_HEXSZ, "\tnot-for-merge")) + continue; /* ref is not-for-merge */ + sha1_array_append(merge_heads, sha1); + } + fclose(fp); + strbuf_release(&sb); +} + +/** + * Used by die_no_merge_candidates() as a for_each_remote() callback to + * retrieve the name of the remote if the repository only has one remote. + */ +static int get_only_remote(struct remote *remote, void *cb_data) +{ + const char **remote_name = cb_data; + + if (*remote_name) + return -1; + + *remote_name = remote->name; + return 0; +} + +/** + * Dies with the appropriate reason for why there are no merge candidates: + * + * 1. We fetched from a specific remote, and a refspec was given, but it ended + *up not fetching anything. This is usually because the user provided a + *wildcard refspec which had no matches on the remote end. + * + * 2. We fetched from a non-default remote, but didn't specify a branch to + *merge. We can't use the configured one because it applies to the default + *remote, thus the user must specify the branches to merge. + * + * 3. We fetched from the branch's or repo's default remote, but: + * + *a. We are not on a branch, so there will never be a configured branch to + * merge with. + * + *b. We are on a branch, but there is no configured branch to merge with. + * + * 4. We fetched from the branch's or repo's default remote, but the configured + *branch to merge didn't get fetched. (Either it doesn't exist, or wasn't + *part of the configured fetch refspec.) + */ +static void NORETURN die_no_merge_candidates(const char *repo, const char **refspecs) +{ + struct branch *curr_branch = branch_get("HEAD"); + const char *remote = curr_branch ? curr_branch->remote_name : NULL; + + if (*refspe
[PATCH v2 12/19] pull: fast-forward working tree if head is updated
Since b10ac50 (Fix pulling into the same branch., 2005-08-25), git-pull, upon detecting that git-fetch updated the current head, will fast-forward the working tree to the updated head commit. Re-implement this behavior. Signed-off-by: Paul Tan --- builtin/pull.c | 30 ++ 1 file changed, 30 insertions(+) diff --git a/builtin/pull.c b/builtin/pull.c index 703fc1d..8db0db2 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -411,6 +411,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix) { const char *repo, **refspecs; struct sha1_array merge_heads = SHA1_ARRAY_INIT; + unsigned char orig_head[GIT_SHA1_RAWSZ], curr_head[GIT_SHA1_RAWSZ]; if (!getenv("_GIT_USE_BUILTIN_PULL")) { const char *path = mkpath("%s/git-pull", git_exec_path()); @@ -434,12 +435,41 @@ int cmd_pull(int argc, const char **argv, const char *prefix) if (!opt_ff.len) config_get_ff(&opt_ff); + if (get_sha1("HEAD", orig_head)) + hashclr(orig_head); + if (run_fetch(repo, refspecs)) return 1; if (opt_dry_run) return 0; + if (get_sha1("HEAD", curr_head)) + hashclr(curr_head); + + if (!is_null_sha1(orig_head) && !is_null_sha1(curr_head) && + hashcmp(orig_head, curr_head)) { + /* +* The fetch involved updating the current branch. +* +* The working tree and the index file are still based on +* orig_head commit, but we are merging into curr_head. +* Update the working tree to match curr_head. +*/ + + warning(_("fetch updated the current branch head.\n" + "fast-forwarding your working tree from\n" + "commit %s."), sha1_to_hex(orig_head)); + + if (checkout_fast_forward(orig_head, curr_head, 0)) + die(_("Cannot fast-forward your working tree.\n" + "After making sure that you saved anything precious from\n" + "$ git diff %s\n" + "output, run\n" + "$ git reset --hard\n" + "to recover."), sha1_to_hex(orig_head)); + } + get_merge_heads(&merge_heads); if (!merge_heads.nr) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 11/19] pull: check if in unresolved merge state
Since d38a30d (Be more user-friendly when refusing to do something because of conflict., 2010-01-12), git-pull will error out with user-friendly advices if the user is in the middle of a merge or has unmerged files. Re-implement this behavior. While the "has unmerged files" case can be handled by die_resolve_conflict(), we introduce a new function die_conclude_merge() for printing a different error message for when there are no unmerged files but the merge has not been finished. Signed-off-by: Paul Tan --- advice.c | 8 advice.h | 1 + builtin/pull.c | 9 + 3 files changed, 18 insertions(+) diff --git a/advice.c b/advice.c index 575bec2..4965686 100644 --- a/advice.c +++ b/advice.c @@ -96,6 +96,14 @@ void NORETURN die_resolve_conflict(const char *me) die("Exiting because of an unresolved conflict."); } +void NORETURN die_conclude_merge(void) +{ + error(_("You have not concluded your merge (MERGE_HEAD exists).")); + if (advice_resolve_conflict) + advise(_("Please, commit your changes before you can merge.")); + die(_("Exiting because of unfinished merge.")); +} + void detach_advice(const char *new_name) { const char fmt[] = diff --git a/advice.h b/advice.h index 5ecc6c1..b341a55 100644 --- a/advice.h +++ b/advice.h @@ -24,6 +24,7 @@ __attribute__((format (printf, 1, 2))) void advise(const char *advice, ...); int error_resolve_conflict(const char *me); extern void NORETURN die_resolve_conflict(const char *me); +void NORETURN die_conclude_merge(void); void detach_advice(const char *new_name); #endif /* ADVICE_H */ diff --git a/builtin/pull.c b/builtin/pull.c index 1c1883d..703fc1d 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -12,6 +12,7 @@ #include "run-command.h" #include "sha1-array.h" #include "remote.h" +#include "dir.h" static const char * const pull_usage[] = { N_("git pull [options] [ [...]]"), @@ -422,6 +423,14 @@ int cmd_pull(int argc, const char **argv, const char *prefix) parse_repo_refspecs(argc, argv, &repo, &refspecs); + git_config(git_default_config, NULL); + + if (read_cache_unmerged()) + die_resolve_conflict("Pull"); + + if (file_exists(git_path("MERGE_HEAD"))) + die_conclude_merge(); + if (!opt_ff.len) config_get_ff(&opt_ff); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 19/19] pull: remove redirection to git-pull.sh
At the beginning of the rewrite of git-pull.sh to C, we introduced a redirection to git-pull.sh if the environment variable _GIT_USE_BUILTIN_PULL was not defined in order to not break test scripts that relied on a functional git-pull. Now that all of git-pull's functionality has been re-implemented in builtin/pull.c, remove this redirection, and retire the old git-pull.sh into contrib/examples/. Signed-off-by: Paul Tan --- Makefile| 1 - builtin/pull.c | 7 --- git-pull.sh => contrib/examples/git-pull.sh | 0 3 files changed, 8 deletions(-) rename git-pull.sh => contrib/examples/git-pull.sh (100%) diff --git a/Makefile b/Makefile index 2057a9d..67cef1c 100644 --- a/Makefile +++ b/Makefile @@ -474,7 +474,6 @@ SCRIPT_SH += git-merge-octopus.sh SCRIPT_SH += git-merge-one-file.sh SCRIPT_SH += git-merge-resolve.sh SCRIPT_SH += git-mergetool.sh -SCRIPT_SH += git-pull.sh SCRIPT_SH += git-quiltimport.sh SCRIPT_SH += git-rebase.sh SCRIPT_SH += git-remote-testgit.sh diff --git a/builtin/pull.c b/builtin/pull.c index 4e1ab5b..dad49cf 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -805,13 +805,6 @@ int cmd_pull(int argc, const char **argv, const char *prefix) unsigned char orig_head[GIT_SHA1_RAWSZ], curr_head[GIT_SHA1_RAWSZ]; unsigned char rebase_fork_point[GIT_SHA1_RAWSZ]; - if (!getenv("_GIT_USE_BUILTIN_PULL")) { - const char *path = mkpath("%s/git-pull", git_exec_path()); - - if (sane_execvp(path, (char**) argv) < 0) - die_errno("could not exec %s", path); - } - if (!getenv("GIT_REFLOG_ACTION")) set_reflog_message(argc, argv); diff --git a/git-pull.sh b/contrib/examples/git-pull.sh similarity index 100% rename from git-pull.sh rename to contrib/examples/git-pull.sh -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 03/19] argv-array: implement argv_array_pushv()
When we have a null-terminated array, it would be useful to convert it or append it to an argv_array for further manipulation. Implement argv_array_pushv() which will push a null-terminated array of strings on to an argv_array. Signed-off-by: Paul Tan --- Documentation/technical/api-argv-array.txt | 3 +++ argv-array.c | 6 ++ argv-array.h | 1 + 3 files changed, 10 insertions(+) diff --git a/Documentation/technical/api-argv-array.txt b/Documentation/technical/api-argv-array.txt index 1a79781..8076172 100644 --- a/Documentation/technical/api-argv-array.txt +++ b/Documentation/technical/api-argv-array.txt @@ -46,6 +46,9 @@ Functions Format a string and push it onto the end of the array. This is a convenience wrapper combining `strbuf_addf` and `argv_array_push`. +`argv_array_pushv`:: + Push a null-terminated array of strings onto the end of the array. + `argv_array_pop`:: Remove the final element from the array. If there are no elements in the array, do nothing. diff --git a/argv-array.c b/argv-array.c index 256741d..eaed477 100644 --- a/argv-array.c +++ b/argv-array.c @@ -49,6 +49,12 @@ void argv_array_pushl(struct argv_array *array, ...) va_end(ap); } +void argv_array_pushv(struct argv_array *array, const char **argv) +{ + for (; *argv; argv++) + argv_array_push(array, *argv); +} + void argv_array_pop(struct argv_array *array) { if (!array->argc) diff --git a/argv-array.h b/argv-array.h index c65e6e8..a2fa0aa 100644 --- a/argv-array.h +++ b/argv-array.h @@ -17,6 +17,7 @@ __attribute__((format (printf,2,3))) void argv_array_pushf(struct argv_array *, const char *fmt, ...); LAST_ARG_MUST_BE_NULL void argv_array_pushl(struct argv_array *, ...); +void argv_array_pushv(struct argv_array *, const char **); void argv_array_pop(struct argv_array *); void argv_array_clear(struct argv_array *); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 06/19] pull: pass verbosity, --progress flags to fetch and merge
7f87aff (Teach/Fix pull/fetch -q/-v options, 2008-11-15) taught git-pull to accept the verbosity -v and -q options and pass them to git-fetch and git-merge. Re-implement support for the verbosity flags by adding it to the options list and introducing argv_push_verbosity() to push the flags into the argv array used to execute git-fetch and git-merge. 9839018 (fetch and pull: learn --progress, 2010-02-24) and bebd2fd (pull: propagate --progress to merge, 2011-02-20) taught git-pull to accept the --progress option and pass it to git-fetch and git-merge. Re-implement support for this flag by introducing the option callback handler parse_opt_passthru(). This callback is used to pass the "--progress" or "--no-progress" command-line switch to git-fetch and git-merge. Signed-off-by: Paul Tan --- Notes: v2 * Use parse_opt_pass_strbuf(). builtin/pull.c | 36 1 file changed, 36 insertions(+) diff --git a/builtin/pull.c b/builtin/pull.c index 0ca23a3..c9c2cc0 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -16,11 +16,35 @@ static const char * const pull_usage[] = { NULL }; +/* Shared options */ +static int opt_verbosity; +static struct strbuf opt_progress = STRBUF_INIT; + static struct option pull_options[] = { + /* Shared options */ + OPT__VERBOSITY(&opt_verbosity), + { OPTION_CALLBACK, 0, "progress", &opt_progress, NULL, + N_("force progress reporting"), + PARSE_OPT_NOARG, parse_opt_pass_strbuf}, + OPT_END() }; /** + * Pushes "-q" or "-v" switches into arr to match the opt_verbosity level. + */ +static void argv_push_verbosity(struct argv_array *arr) +{ + int verbosity; + + for (verbosity = opt_verbosity; verbosity > 0; verbosity--) + argv_array_push(arr, "-v"); + + for (verbosity = opt_verbosity; verbosity < 0; verbosity++) + argv_array_push(arr, "-q"); +} + +/** * Parses argv into [ [...]], returning their values in `repo` * as a string and `refspecs` as a null-terminated array of strings. If `repo` * is not provided in argv, it is set to NULL. @@ -46,6 +70,12 @@ static int run_fetch(const char *repo, const char **refspecs) int ret; argv_array_pushl(&args, "fetch", "--update-head-ok", NULL); + + /* Shared options */ + argv_push_verbosity(&args); + if (opt_progress.len) + argv_array_push(&args, opt_progress.buf); + if (repo) argv_array_push(&args, repo); while (*refspecs) @@ -64,6 +94,12 @@ static int run_merge(void) struct argv_array args = ARGV_ARRAY_INIT; argv_array_pushl(&args, "merge", NULL); + + /* Shared options */ + argv_push_verbosity(&args); + if (opt_progress.len) + argv_array_push(&args, opt_progress.buf); + argv_array_push(&args, "FETCH_HEAD"); ret = run_command_v_opt(args.argv, RUN_GIT_CMD); argv_array_clear(&args); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 14/19] pull: set reflog message
f947413 (Use GIT_REFLOG_ACTION environment variable instead., 2006-12-28) established git-pull's method for setting the reflog message, which is to set the environment variable GIT_REFLOG_ACTION to the evaluation of "pull${1+ $*}" if it has not already been set. Re-implement this behavior. Helped-by: Junio C Hamano Signed-off-by: Paul Tan --- Notes: v2 * Don't use strbuf_rtrim(). builtin/pull.c | 22 ++ 1 file changed, 22 insertions(+) diff --git a/builtin/pull.c b/builtin/pull.c index f0d4710..c44cc90 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -169,6 +169,25 @@ static void argv_push_force(struct argv_array *arr) } /** + * Sets the GIT_REFLOG_ACTION environment variable to the concatenation of argv + */ +static void set_reflog_message(int argc, const char **argv) +{ + int i; + struct strbuf msg = STRBUF_INIT; + + for (i = 0; i < argc; i++) { + if (i) + strbuf_addch(&msg, ' '); + strbuf_addstr(&msg, argv[i]); + } + + setenv("GIT_REFLOG_ACTION", msg.buf, 0); + + strbuf_release(&msg); +} + +/* * If pull.ff is "true", sets sb to "--ff". If pull.ff is "false", sets sb to * "--no-ff". If pull.ff is "only", sets sb to "--ff-only". If pull.ff is * set to an invalid value, die with an error. @@ -442,6 +461,9 @@ int cmd_pull(int argc, const char **argv, const char *prefix) die_errno("could not exec %s", path); } + if (!getenv("GIT_REFLOG_ACTION")) + set_reflog_message(argc, argv); + argc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0); parse_repo_refspecs(argc, argv, &repo, &refspecs); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html