From a852ec7f273cf61296a80ddfc26c23acf2163f2f Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Tue, 20 Mar 2018 11:10:55 +0000 Subject: [PATCH 1/3] rebase: extend --signoff support Allow --signoff to be used with --interactive and --merge. In interactive mode only commits marked to be picked, edited or reworded will be signed off. The main motivation for this patch was to allow one to run 'git rebase --exec "make check" --signoff' which is useful when preparing a patch series for publication and is more convenient than doing the signoff with another --exec command. This change also allows --root without --onto to work with --signoff as well (--root with --onto was already supported). Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- Documentation/git-rebase.txt | 7 ++++--- git-rebase--interactive.sh | 6 +++--- git-rebase--merge.sh | 2 +- git-rebase.sh | 20 ++++++++++++++++++- sequencer.c | 8 +++++++- t/t3428-rebase-signoff.sh | 38 ++++++++++++++++++++++++++++++++++++ 6 files changed, 72 insertions(+), 9 deletions(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 8a861c1e0d..86a887a9c5 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -354,9 +354,10 @@ default is `--no-fork-point`, otherwise the default is `--fork-point`. Incompatible with the --interactive option. --signoff:: - This flag is passed to 'git am' to sign off all the rebased - commits (see linkgit:git-am[1]). Incompatible with the - --interactive option. + Add a Signed-off-by: trailer to all the rebased commits. Note + that if `--interactive` is given then only commits marked to be + picked, edited or reworded will have the trailer added. Incompatible + with the `--preserve-merges` option. -i:: --interactive:: diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index d47bd29593..2bcdb67982 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -283,7 +283,7 @@ pick_one () { pick_one_preserving_merges "$@" && return output eval git cherry-pick $allow_rerere_autoupdate \ ${gpg_sign_opt:+$(git rev-parse --sq-quote "$gpg_sign_opt")} \ - "$strategy_args" $empty_args $ff "$@" + $signoff "$strategy_args" $empty_args $ff "$@" # If cherry-pick dies it leaves the to-be-picked commit unrecorded. Reschedule # previous task so this commit is not lost. @@ -524,10 +524,10 @@ do_pick () { # resolve before manually running git commit --amend then git # rebase --continue. git commit --allow-empty --allow-empty-message --amend \ - --no-post-rewrite -n -q -C $sha1 && + --no-post-rewrite -n -q -C $sha1 $signoff && pick_one -n $sha1 && git commit --allow-empty --allow-empty-message \ - --amend --no-post-rewrite -n -q -C $sha1 \ + --amend --no-post-rewrite -n -q -C $sha1 $signoff \ ${gpg_sign_opt:+"$gpg_sign_opt"} || die_with_patch $sha1 "$(eval_gettext "Could not apply \$sha1... \$rest")" else diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh index 06a4723d4d..9a60cef052 100644 --- a/git-rebase--merge.sh +++ b/git-rebase--merge.sh @@ -27,7 +27,7 @@ continue_merge () { cmt=$(cat "$state_dir/current") if ! git diff-index --quiet --ignore-submodules HEAD -- then - if ! git commit ${gpg_sign_opt:+"$gpg_sign_opt"} --no-verify -C "$cmt" + if ! git commit ${gpg_sign_opt:+"$gpg_sign_opt"} $signoff --no-verify -C "$cmt" then echo "Commit failed, please do not call \"git commit\"" echo "directly, but instead do one of the following: " diff --git a/git-rebase.sh b/git-rebase.sh index 37b8f13971..5a5a65f3d2 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -90,6 +90,7 @@ action= preserve_merges= autosquash= keep_empty= +signoff= test "$(git config --bool rebase.autosquash)" = "true" && autosquash=t case "$(git config --bool commit.gpgsign)" in true) gpg_sign_opt=-S ;; @@ -119,6 +120,10 @@ read_basic_state () { allow_rerere_autoupdate="$(cat "$state_dir"/allow_rerere_autoupdate)" test -f "$state_dir"/gpg_sign_opt && gpg_sign_opt="$(cat "$state_dir"/gpg_sign_opt)" + test -f "$state_dir"/signoff && { + signoff="$(cat "$state_dir"/signoff)" + force_rebase=t + } } write_basic_state () { @@ -133,6 +138,7 @@ write_basic_state () { test -n "$allow_rerere_autoupdate" && echo "$allow_rerere_autoupdate" > \ "$state_dir"/allow_rerere_autoupdate test -n "$gpg_sign_opt" && echo "$gpg_sign_opt" > "$state_dir"/gpg_sign_opt + test -n "$signoff" && echo "$signoff" >"$state_dir"/signoff } output () { @@ -328,7 +334,13 @@ do --ignore-whitespace) git_am_opt="$git_am_opt $1" ;; - --committer-date-is-author-date|--ignore-date|--signoff|--no-signoff) + --signoff) + signoff=--signoff + ;; + --no-signoff) + signoff= + ;; + --committer-date-is-author-date|--ignore-date) git_am_opt="$git_am_opt $1" force_rebase=t ;; @@ -458,6 +470,12 @@ then git_format_patch_opt="$git_format_patch_opt --progress" fi +if test -n "$signoff" +then + git_am_opt="$git_am_opt $signoff" + force_rebase=t +fi + if test -z "$rebase_root" then case "$#" in diff --git a/sequencer.c b/sequencer.c index 45aa7e3072..4aeaef20e7 100644 --- a/sequencer.c +++ b/sequencer.c @@ -127,6 +127,7 @@ static GIT_PATH_FUNC(rebase_path_rewritten_pending, static GIT_PATH_FUNC(rebase_path_gpg_sign_opt, "rebase-merge/gpg_sign_opt") static GIT_PATH_FUNC(rebase_path_orig_head, "rebase-merge/orig-head") static GIT_PATH_FUNC(rebase_path_verbose, "rebase-merge/verbose") +static GIT_PATH_FUNC(rebase_path_signoff, "rebase-merge/signoff") static GIT_PATH_FUNC(rebase_path_head_name, "rebase-merge/head-name") static GIT_PATH_FUNC(rebase_path_onto, "rebase-merge/onto") static GIT_PATH_FUNC(rebase_path_autostash, "rebase-merge/autostash") @@ -1605,7 +1606,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit, } } - if (opts->signoff) + if (opts->signoff && !is_fixup(command)) append_signoff(&msgbuf, 0, 0); if (is_rebase_i(opts) && write_author_script(msg.message) < 0) @@ -2035,6 +2036,11 @@ static int read_populate_opts(struct replay_opts *opts) if (file_exists(rebase_path_verbose())) opts->verbose = 1; + if (file_exists(rebase_path_signoff())) { + opts->allow_ff = 0; + opts->signoff = 1; + } + read_strategy_opts(opts, &buf); strbuf_release(&buf); diff --git a/t/t3428-rebase-signoff.sh b/t/t3428-rebase-signoff.sh index 2afb564701..f6993b7e14 100755 --- a/t/t3428-rebase-signoff.sh +++ b/t/t3428-rebase-signoff.sh @@ -12,6 +12,13 @@ cat >file <expected-initial-signed <.*/>/") +EOF + # Expected commit message after rebase --signoff cat >expected-signed <actual && + test_cmp expected-signed actual +' + +test_expect_success 'rebase --root --signoff adds a sign-off line' ' + git commit --amend -m "first" && + git rebase --root --keep-empty --signoff && + git cat-file commit HEAD^ | sed -e "1,/^\$/d" >actual && + test_cmp expected-initial-signed actual && + git cat-file commit HEAD | sed -e "1,/^\$/d" >actual && + test_cmp expected-signed actual +' + +test_expect_success 'rebase -i --signoff fails' ' + git commit --amend -m "first" && + git rebase -i --signoff HEAD^ && + git cat-file commit HEAD | sed -e "1,/^\$/d" >actual && + test_cmp expected-signed actual +' + +test_expect_success 'rebase -m --signoff fails' ' + git commit --amend -m "first" && + git rebase -m --signoff HEAD^ && + git cat-file commit HEAD | sed -e "1,/^\$/d" >actual && + test_cmp expected-signed actual +' test_done From b79966aa386fc58801e93496cd4e97d82acd53a5 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Tue, 20 Mar 2018 11:10:56 +0000 Subject: [PATCH 2/3] rebase -p: error out if --signoff is given rebase --preserve-merges does not support --signoff so error out rather than just silently ignoring it so that the user knows the commits will not be signed off. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- git-rebase.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/git-rebase.sh b/git-rebase.sh index 5a5a65f3d2..e65b65acb4 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -472,6 +472,8 @@ fi if test -n "$signoff" then + test -n "$preserve_merges" && + die "$(gettext "error: cannot combine '--signoff' with '--preserve-merges'")" git_am_opt="$git_am_opt $signoff" force_rebase=t fi From da27a6fbd50861149b32cfd1f9e5c36a935c575a Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Tue, 20 Mar 2018 11:10:57 +0000 Subject: [PATCH 3/3] rebase --keep-empty: always use interactive rebase rebase --merge accepts --keep-empty but just ignores it, by using an implicit interactive rebase the user still gets the rename detection of a merge based rebase but with with --keep-empty support. If rebase --keep-empty without --interactive or --merge stops for the user to resolve merge conflicts then 'git rebase --continue' will fail. This is because it uses a different code path that does not create $git_dir/rebase-apply. As rebase --keep-empty was implemented using cherry-pick it has never supported the am options and now that interactive rebases support --signoff there is no loss of functionality by using an implicit interactive rebase. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- git-rebase--am.sh | 80 +++++++++++++------------------ git-rebase.sh | 5 ++ t/t3421-rebase-topology-linear.sh | 4 +- 3 files changed, 41 insertions(+), 48 deletions(-) diff --git a/git-rebase--am.sh b/git-rebase--am.sh index 14c50782e0..91381d5872 100644 --- a/git-rebase--am.sh +++ b/git-rebase--am.sh @@ -38,60 +38,48 @@ else fi ret=0 -if test -n "$keep_empty" +rm -f "$GIT_DIR/rebased-patches" + +git format-patch -k --stdout --full-index --cherry-pick --right-only \ + --src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \ + --pretty=mboxrd \ + $git_format_patch_opt \ + "$revisions" ${restrict_revision+^$restrict_revision} \ + >"$GIT_DIR/rebased-patches" +ret=$? + +if test 0 != $ret then - # we have to do this the hard way. git format-patch completely squashes - # empty commits and even if it didn't the format doesn't really lend - # itself well to recording empty patches. fortunately, cherry-pick - # makes this easy - git cherry-pick ${gpg_sign_opt:+"$gpg_sign_opt"} --allow-empty \ - $allow_rerere_autoupdate --right-only "$revisions" \ - ${restrict_revision+^$restrict_revision} - ret=$? -else rm -f "$GIT_DIR/rebased-patches" + case "$head_name" in + refs/heads/*) + git checkout -q "$head_name" + ;; + *) + git checkout -q "$orig_head" + ;; + esac - git format-patch -k --stdout --full-index --cherry-pick --right-only \ - --src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \ - --pretty=mboxrd \ - $git_format_patch_opt \ - "$revisions" ${restrict_revision+^$restrict_revision} \ - >"$GIT_DIR/rebased-patches" - ret=$? + cat >&2 <<-EOF - if test 0 != $ret - then - rm -f "$GIT_DIR/rebased-patches" - case "$head_name" in - refs/heads/*) - git checkout -q "$head_name" - ;; - *) - git checkout -q "$orig_head" - ;; - esac + git encountered an error while preparing the patches to replay + these revisions: - cat >&2 <<-EOF + $revisions - git encountered an error while preparing the patches to replay - these revisions: - - $revisions - - As a result, git cannot rebase them. - EOF - return $ret - fi - - git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" \ - --patch-format=mboxrd \ - $allow_rerere_autoupdate \ - ${gpg_sign_opt:+"$gpg_sign_opt"} <"$GIT_DIR/rebased-patches" - ret=$? - - rm -f "$GIT_DIR/rebased-patches" + As a result, git cannot rebase them. + EOF + return $ret fi +git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" \ + --patch-format=mboxrd \ + $allow_rerere_autoupdate \ + ${gpg_sign_opt:+"$gpg_sign_opt"} <"$GIT_DIR/rebased-patches" +ret=$? + +rm -f "$GIT_DIR/rebased-patches" + if test 0 != $ret then test -d "$state_dir" && write_basic_state diff --git a/git-rebase.sh b/git-rebase.sh index e65b65acb4..ee8c77ad99 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -452,6 +452,11 @@ then test -z "$interactive_rebase" && interactive_rebase=implied fi +if test -n "$keep_empty" +then + test -z "$interactive_rebase" && interactive_rebase=implied +fi + if test -n "$interactive_rebase" then type=interactive diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh index 52fc6885e5..b078f93046 100755 --- a/t/t3421-rebase-topology-linear.sh +++ b/t/t3421-rebase-topology-linear.sh @@ -199,7 +199,7 @@ test_run_rebase () { " } test_run_rebase success '' -test_run_rebase failure -m +test_run_rebase success -m test_run_rebase success -i test_run_rebase failure -p @@ -214,7 +214,7 @@ test_run_rebase () { " } test_run_rebase success '' -test_run_rebase failure -m +test_run_rebase success -m test_run_rebase success -i test_run_rebase failure -p