Message ID | 20190907115034.14933-1-rohit.ashiwal265@gmail.com (mailing list archive) |
---|---|
Headers | show |
Series | rebase -i: support more options | expand |
Rohit Ashiwal <rohit.ashiwal265@gmail.com> writes: > Following the suggestion of Phillip I've rebased my patch on master (745f681289) > and cherry-picking b0a3186140. Sorry, but that's horrible. The latter does not even cleanly apply on the former. Let me see if I can find time to whip this into a reasonable shape. Thanks.
On 09/09/2019 19:02, Junio C Hamano wrote: > Rohit Ashiwal <rohit.ashiwal265@gmail.com> writes: > >> Following the suggestion of Phillip I've rebased my patch on master (745f681289) >> and cherry-picking b0a3186140. > > Sorry, but that's horrible. The latter does not even cleanly apply > on the former. Yes I had assumed that the cherry pick would become the first patch of this series and be dropped from pw/rebase-i-show-HEAD-to-reword. I should have been more explicit about that. > Let me see if I can find time to whip this into a reasonable shape. As pw/rebase-i-show-HEAD-to-reword is slated for next perhaps these could build on that. The first patch needs am -3 to apply to that branch but the result looks ok and the rest apply as is. Best Wishes Phillip > Thanks. >
Phillip Wood <phillip.wood123@gmail.com> writes: > Yes I had assumed that the cherry pick would become the first patch of > this series and be dropped from pw/rebase-i-show-HEAD-to-reword. Ah, such an arrangement would have made a log more sense, indeed. > As pw/rebase-i-show-HEAD-to-reword is slated for next perhaps these > could build on that. The first patch needs am -3 to apply to that > branch but the result looks ok and the rest apply as is. As this is more or less "new feature" series, I do not mind too much to make it depend on another topic in flight that is getting solid. Thanks for helping.
Following the suggestion of Phillip I've rebased my patch on master (745f681289) and cherry-picking b0a3186140. Base: 745f6812895b31c02b29bdfe4ae8e5498f776c26 with cherry-picked b0a3186140dbc7bd64cbc6ef733386a0f1eb6a4d Outline: 2/6: - Change commit message and description 3/6: - add test for rebase -r - add strict bounds while pushing ident date 5/6: - bug fix: (following am's code) if --ignore-date is provided, we should setenv GIT_COMMITTER_DATE to "". - catch error when ident line is malformed - push GIT_AUTHOR_DATE instead of providing it with --date so that "git merge" can also use push_dates function - add test for rebase -r Rohit Ashiwal (6): rebase -i: add --ignore-whitespace flag sequencer: allow callers of read_author_script() to ignore fields rebase -i: support --committer-date-is-author-date sequencer: rename amend_author to author_to_rename rebase -i: support --ignore-date rebase: add --reset-author-date Documentation/git-rebase.txt | 26 +++-- builtin/rebase.c | 55 +++++++--- sequencer.c | 134 +++++++++++++++++++++-- sequencer.h | 2 + t/t3422-rebase-incompatible-options.sh | 2 - t/t3433-rebase-options-compatibility.sh | 137 ++++++++++++++++++++++++ 6 files changed, 328 insertions(+), 28 deletions(-) create mode 100755 t/t3433-rebase-options-compatibility.sh Range-diff: --- snip --- 1: e82ed8cad5 = 49: 77af1d66db rebase -i: add --ignore-whitespace flag 2: 209057b361 ! 50: 1f7f1407b2 sequencer: add NULL checks under read_author_script @@ -1,19 +1,14 @@ Author: Rohit Ashiwal <rohit.ashiwal265@gmail.com> - sequencer: add NULL checks under read_author_script + sequencer: allow callers of read_author_script() to ignore fields - read_author_script reads name, email and author date from the author - script. However, it does not check if the arguments are NULL. Adding - NULL checks will allow us to selectively get the required value, for - example: + The current callers of the read_author_script() function read name, + email and date from the author script. Allow callers to signal that + they are not interested in some among these three fields by passing + NULL. - char *date; - if (read_author_script(_path_, NULL, NULL, &date, _int_)) - die(_("failed to read author date")); - /* needs to be free()'d */ - return date; - - Add NULL checks for better control over the information retrieved. + Note that fields that are ignored still must exist and be formatted + correctly in the author script. Signed-off-by: Rohit Ashiwal <rohit.ashiwal265@gmail.com> 3: a4e6644ef8 ! 51: cc1614154e rebase -i: support --committer-date-is-author-date @@ -128,35 +128,34 @@ + return date; +} + - /* Read author-script and return an ident line (author <email> timestamp) */ - static const char *read_author_ident(struct strbuf *buf) - { + static const char staged_changes_advice[] = + N_("you have staged changes in your working tree\n" + "If these changes are meant to be squashed into the previous commit, run:\n" @@ - { - struct child_process cmd = CHILD_PROCESS_INIT; -+ if (opts->committer_date_is_author_date) { -+ size_t len; -+ int res = -1; -+ struct strbuf datebuf = STRBUF_INIT; -+ char *date = read_author_date_or_null(); + cmd.git_cmd = 1; + ++ if (opts->committer_date_is_author_date) { ++ int res = -1; ++ struct strbuf datebuf = STRBUF_INIT; ++ char *date = read_author_date_or_null(); + -+ if (!date) -+ return -1; ++ if (!date) ++ return -1; + -+ strbuf_addf(&datebuf, "@%s", date); -+ free(date); ++ strbuf_addf(&datebuf, "@%s", date); ++ res = setenv("GIT_COMMITTER_DATE", datebuf.buf, 1); + -+ date = strbuf_detach(&datebuf, &len); -+ res = setenv("GIT_COMMITTER_DATE", date, 1); -+ free(date); ++ strbuf_release(&datebuf); ++ free(date); + -+ if (res) -+ return -1; -+ } - if ((flags & CREATE_ROOT_COMMIT) && !(flags & AMEND_MSG)) { - struct strbuf msg = STRBUF_INIT, script = STRBUF_INIT; - const char *author = NULL; ++ if (res) ++ return -1; ++ } ++ + if (is_rebase_i(opts) && read_env_script(&cmd.env_array)) { + const char *gpg_opt = gpg_sign_opt_quoted(opts); + @@ if (parse_head(r, ¤t_head)) @@ -174,14 +173,19 @@ + struct ident_split ident; + struct strbuf date = STRBUF_INIT; + -+ split_ident_line(&ident, author, len); -+ ++ if (split_ident_line(&ident, author, len) < 0) ++ return error(_("malformed ident line")); + if (!ident.date_begin) + return error(_("corrupted author without date information")); + -+ strbuf_addf(&date, "@%s", ident.date_begin); -+ setenv("GIT_COMMITTER_DATE", date.buf, 1); ++ strbuf_addf(&date, "@%.*s %.*s", ++ (int)(ident.date_end - ident.date_begin), ident.date_begin, ++ (int)(ident.tz_end - ident.tz_begin), ident.tz_begin); ++ res = setenv("GIT_COMMITTER_DATE", date.buf, 1); + strbuf_release(&date); ++ ++ if (res) ++ goto out; + } + if (write_index_as_tree(&tree, r->index, r->index_file, 0, NULL)) { @@ -256,6 +260,20 @@ # This is a special case in which both am and interactive backends # provide the same output. It was done intentionally because # both the backends fall short of optimal behaviour. +@@ + EOF + git commit -am "update file" && + git tag side && ++ test_commit commit1 foo foo1 && ++ test_commit commit2 foo foo2 && ++ test_commit commit3 foo foo3 && + + git checkout --orphan master && ++ git rm --cached foo && ++ rm foo && + sed -e "s/^|//" >file <<-\EOF && + |line 1 + | line 2 @@ test_cmp expect file ' @@ -275,5 +293,18 @@ + git show HEAD --pretty="format:%ci" >committertime && + test_cmp authortime committertime +' ++ ++test_expect_success '--committer-date-is-author-date works with rebase -r' ' ++ git checkout side && ++ git merge commit3 && ++ git rebase -r --root --committer-date-is-author-date && ++ git rev-list HEAD >rev_list && ++ while read HASH ++ do ++ git show $HASH --pretty="format:%ai" >authortime ++ git show $HASH --pretty="format:%ci" >committertime ++ test_cmp authortime committertime ++ done <rev_list ++' + test_done 4: 6ac1885c54 = 52: 9e92c79bda sequencer: rename amend_author to author_to_rename 5: a69749dd67 ! 53: fc68e55e78 rebase -i: support --ignore-date @@ -83,8 +83,10 @@ } - if (options.committer_date_is_author_date) -+ if (options.ignore_date) ++ if (options.ignore_date) { + options.committer_date_is_author_date = 0; ++ setenv("GIT_COMMITTER_DATE", "", 1); ++ } + if (options.committer_date_is_author_date || + options.ignore_date) options.flags |= REBASE_FORCE; @@ -110,7 +112,7 @@ static GIT_PATH_FUNC(rebase_path_verbose, "rebase-merge/verbose") static GIT_PATH_FUNC(rebase_path_quiet, "rebase-merge/quiet") @@ - return buf->buf; + return date; } +/* Construct a free()able author string with current time as the author date */ @@ -120,7 +122,10 @@ + struct ident_split ident; + struct strbuf new_author = STRBUF_INIT; + -+ split_ident_line(&ident, author, len); ++ if (split_ident_line(&ident, author, len) < 0) { ++ error(_("malformed ident line")); ++ return NULL; ++ } + len = ident.mail_end - ident.name_begin + 1; + + strbuf_addf(&new_author, "%.*s ", len, author); @@ -134,7 +139,7 @@ + struct strbuf date = STRBUF_INIT; + + strbuf_addf(&date, "@%"PRIuMAX, (uintmax_t)now); -+ argv_array_pushf(&child->args, "--date=%s", date.buf); ++ argv_array_pushf(&child->env_array, "GIT_AUTHOR_DATE=%s", date.buf); + argv_array_pushf(&child->env_array, "GIT_COMMITTER_DATE=%s", date.buf); + strbuf_release(&date); +} @@ -143,28 +148,6 @@ N_("you have staged changes in your working tree\n" "If these changes are meant to be squashed into the previous commit, run:\n" @@ - - if (res <= 0) - res = error_errno(_("could not read '%s'"), defmsg); -- else -+ else { -+ if (opts->ignore_date) { -+ char *new_author = ignore_author_date(author); -+ if (!author) -+ BUG("ignore-date can only be used with " -+ "rebase, which must set the author " -+ "before committing the tree"); -+ free((void *)author); -+ author = new_author; -+ } - res = commit_tree(msg.buf, msg.len, cache_tree_oid, - NULL, &root_commit, author, - opts->gpg_sign); -+ } - - strbuf_release(&msg); - strbuf_release(&script); -@@ argv_array_push(&cmd.args, "--amend"); if (opts->gpg_sign) argv_array_pushf(&cmd.args, "-S%s", opts->gpg_sign); @@ -179,6 +162,10 @@ + if (opts->ignore_date) { + author = ignore_author_date(author); ++ if (!author) { ++ res = -1; ++ goto out; ++ } + free(author_to_free); + author_to_free = (char *)author; + } @@ -242,7 +229,7 @@ --- a/t/t3433-rebase-options-compatibility.sh +++ b/t/t3433-rebase-options-compatibility.sh @@ - test_cmp authortime committertime + done <rev_list ' +# Checking for +0000 in author time is enough since default @@ -252,13 +239,32 @@ + git commit --amend --date="$GIT_AUTHOR_DATE" && + git rebase --ignore-date HEAD^ && + git show HEAD --pretty="format:%ai" >authortime && -+ grep "+0000" authortime ++ git show HEAD --pretty="format:%ci" >committertime && ++ grep "+0000" authortime && ++ grep "+0000" committertime +' + +test_expect_success '--ignore-date works with interactive backend' ' + git commit --amend --date="$GIT_AUTHOR_DATE" && + git rebase --ignore-date -i HEAD^ && + git show HEAD --pretty="format:%ai" >authortime && -+ grep "+0000" authortime ++ git show HEAD --pretty="format:%ci" >committertime && ++ grep "+0000" authortime && ++ grep "+0000" committertime +' ++ ++test_expect_success '--ignore-date works with rebase -r' ' ++ git checkout side && ++ git merge commit3 && ++ git rebase -r --root --ignore-date && ++ git rev-list HEAD >rev_list && ++ while read HASH ++ do ++ git show $HASH --pretty="format:%ai" >authortime ++ git show $HASH --pretty="format:%ci" >committertime ++ grep "+0000" authortime ++ grep "+0000" committertime ++ done <rev_list ++' ++ test_done 6: 210d15cca0 = 54: 396d5f16eb rebase: add --reset-author-date