Re: [PATCH v3 8/9] rebase: add the --gpg-sign option
On 07/02/14 23:50, brian m. carlson wrote: On Mon, Feb 03, 2014 at 01:42:06PM -0800, Junio C Hamano wrote: + --gpg-sign) + gpg_sign_opt=-S + ;; + --gpg-sign=*) + # Try to quote only the argument, as this will appear in human-readable + # output as well as being passed to commands. + gpg_sign_opt=-S$(git rev-parse --sq-quote ${1#--gpg-sign=} | + sed 's/^ //') Isn't an invocation of sed excessive? gpg_sign_opt=$(git rev-parse --sq-quote ${1#--gpg-sign=}) gpg_sign_opt=-S${gpg_sign_opt# } if you really need to strip the leading SP, which I do not think is a necessary thing to do. It is sufficient to remove the SP before the variable substitution in the human-readable messages, e.g. I'm not sure that command line parsing of -S 'foo x...@example.tld' will work exactly as expected due to the fact that -S doesn't always take an argument. Your suggestion to use # seems fine, though. I'm a little embarrassed to admit that in my fifteen years of Unix experience, I've never learned the variable modifiers for shell, so it didn't occur to me to use them in this case. Guess it's time to learn them now. Same here: For other readers, I found most google results were only partial on the issue, missing the '#' symbol option. Here's a more complete ref http://www.gnu.org/software/bash/manual/bashref.html#Shell-Parameter-Expansion-1 for fellow learners. Philip -- -- 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 v3 8/9] rebase: add the --gpg-sign option
On Mon, Feb 03, 2014 at 01:42:06PM -0800, Junio C Hamano wrote: + --gpg-sign) + gpg_sign_opt=-S + ;; + --gpg-sign=*) + # Try to quote only the argument, as this will appear in human-readable + # output as well as being passed to commands. + gpg_sign_opt=-S$(git rev-parse --sq-quote ${1#--gpg-sign=} | + sed 's/^ //') Isn't an invocation of sed excessive? gpg_sign_opt=$(git rev-parse --sq-quote ${1#--gpg-sign=}) gpg_sign_opt=-S${gpg_sign_opt# } if you really need to strip the leading SP, which I do not think is a necessary thing to do. It is sufficient to remove the SP before the variable substitution in the human-readable messages, e.g. I'm not sure that command line parsing of -S 'foo x...@example.tld' will work exactly as expected due to the fact that -S doesn't always take an argument. Your suggestion to use # seems fine, though. I'm a little embarrassed to admit that in my fifteen years of Unix experience, I've never learned the variable modifiers for shell, so it didn't occur to me to use them in this case. Guess it's time to learn them now. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH v3 8/9] rebase: add the --gpg-sign option
brian m. carlson sand...@crustytoothpaste.net writes: diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 43c19e0..73d32dd 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -181,7 +181,7 @@ exit_with_patch () { git rev-parse --verify HEAD $amend warn You can amend the commit now, with warn - warn git commit --amend + warn git commit --amend $gpg_sign_opt warn warn Once you are satisfied with your changes, run warn @@ -248,7 +248,8 @@ pick_one () { test -d $rewritten pick_one_preserving_merges $@ return - output eval git cherry-pick $strategy_args $empty_args $ff $@ + output eval git cherry-pick ${gpg_sign_opt:+$gpg_sign_opt} \ + $strategy_args $empty_args $ff $@ This uses $gpg_sign_opt on eval, which means that the variable's contents must be properly shell quoted, e.g. gpg_sign_opt='-S'\''brian m. carson sand...@c.net'\' throughout this script, so that everything between the first double-quote and closing ket is passed as a single parameter without being broken up. @@ -359,7 +360,8 @@ pick_one_preserving_merges () { echo $sha1 $(git rev-parse HEAD^0) $rewritten_list ;; *) - output eval git cherry-pick $strategy_args $@ || + output eval git cherry-pick ${gpg_sign_opt:+$gpg_sign_opt} \ And this part has the same expectation. However... @@ -470,7 +472,8 @@ do_pick () { --no-post-rewrite -n -q -C $1 pick_one -n $1 git commit --allow-empty --allow-empty-message \ ---amend --no-post-rewrite -n -q -C $1 || +--amend --no-post-rewrite -n -q -C $1 \ +${gpg_sign_opt:+$gpg_sign_opt} || This does not want that extra level of quoting. It would want to have something like this instead: gpg_sign_opt='-Sbrian m. carson sand...@c.net' I am not sure how you are managing these two conflicting needs of the use sites. @@ -497,7 +500,7 @@ do_next () { mark_action_done do_pick $sha1 $rest - git commit --amend --no-post-rewrite || { + git commit --amend --no-post-rewrite ${gpg_sign_opt:+$gpg_sign_opt} || { Ditto. diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh index e7d96de..5381857 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 --no-verify -C $cmt + if ! git commit ${gpg_sign_opt:+$gpg_sign_opt} --no-verify -C $cmt Ditto. 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 842d7d4..055af1b 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -37,6 +37,7 @@ ignore-date! passed to 'git am' whitespace=! passed to 'git apply' ignore-whitespace! passed to 'git apply' C=!passed to 'git apply' +S,gpg-sign?GPG-sign commits Actions: continue! continue abort! abort and check out the original branch @@ -85,6 +86,7 @@ preserve_merges= autosquash= keep_empty= test $(git config --bool rebase.autosquash) = true autosquash=t +gpg_sign_opt= read_basic_state () { test -f $state_dir/head-name @@ -107,6 +109,8 @@ read_basic_state () { strategy_opts=$(cat $state_dir/strategy_opts) test -f $state_dir/allow_rerere_autoupdate 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) } write_basic_state () { @@ -120,6 +124,7 @@ write_basic_state () { $state_dir/strategy_opts 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 } output () { @@ -324,6 +329,15 @@ do --rerere-autoupdate|--no-rerere-autoupdate) allow_rerere_autoupdate=$1 ;; + --gpg-sign) + gpg_sign_opt=-S + ;; + --gpg-sign=*) + # Try to quote only the argument, as this will appear in human-readable + # output as well as being passed to commands. + gpg_sign_opt=-S$(git rev-parse --sq-quote ${1#--gpg-sign=} | + sed 's/^ //') Isn't an invocation of sed excessive? gpg_sign_opt=$(git rev-parse --sq-quote
[PATCH v3 8/9] rebase: add the --gpg-sign option
From: Nicolas Vigier bo...@mars-attacks.org Signed-off-by: Nicolas Vigier bo...@mars-attacks.org Signed-off-by: brian m. carlson sand...@crustytoothpaste.net --- Documentation/git-rebase.txt | 4 git-rebase--am.sh| 8 +--- git-rebase--interactive.sh | 32 git-rebase--merge.sh | 2 +- git-rebase.sh| 14 ++ 5 files changed, 44 insertions(+), 16 deletions(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 2889be6..2a93c64 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -281,6 +281,10 @@ which makes little sense. specified, `-s recursive`. Note the reversal of 'ours' and 'theirs' as noted above for the `-m` option. +-S[keyid]:: +--gpg-sign[=keyid]:: + GPG-sign commits. + -q:: --quiet:: Be quiet. Implies --no-stat. diff --git a/git-rebase--am.sh b/git-rebase--am.sh index a4f683a..df46f4c 100644 --- a/git-rebase--am.sh +++ b/git-rebase--am.sh @@ -6,7 +6,8 @@ case $action in continue) - git am --resolved --resolvemsg=$resolvemsg + git am --resolved --resolvemsg=$resolvemsg \ + ${gpg_sign_opt:+$gpg_sign_opt} move_to_original_branch return ;; @@ -26,7 +27,7 @@ then # 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 --allow-empty $revisions + git cherry-pick ${gpg_sign_opt:+$gpg_sign_opt} --allow-empty $revisions ret=$? else rm -f $GIT_DIR/rebased-patches @@ -60,7 +61,8 @@ else return $? fi - git am $git_am_opt --rebasing --resolvemsg=$resolvemsg $GIT_DIR/rebased-patches + git am $git_am_opt --rebasing --resolvemsg=$resolvemsg \ + ${gpg_sign_opt:+$gpg_sign_opt} $GIT_DIR/rebased-patches ret=$? rm -f $GIT_DIR/rebased-patches diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 43c19e0..73d32dd 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -181,7 +181,7 @@ exit_with_patch () { git rev-parse --verify HEAD $amend warn You can amend the commit now, with warn - warn git commit --amend + warn git commit --amend $gpg_sign_opt warn warn Once you are satisfied with your changes, run warn @@ -248,7 +248,8 @@ pick_one () { test -d $rewritten pick_one_preserving_merges $@ return - output eval git cherry-pick $strategy_args $empty_args $ff $@ + output eval git cherry-pick ${gpg_sign_opt:+$gpg_sign_opt} \ + $strategy_args $empty_args $ff $@ } pick_one_preserving_merges () { @@ -359,7 +360,8 @@ pick_one_preserving_merges () { echo $sha1 $(git rev-parse HEAD^0) $rewritten_list ;; *) - output eval git cherry-pick $strategy_args $@ || + output eval git cherry-pick ${gpg_sign_opt:+$gpg_sign_opt} \ + $strategy_args $@ || die_with_patch $sha1 Could not pick $sha1 ;; esac @@ -470,7 +472,8 @@ do_pick () { --no-post-rewrite -n -q -C $1 pick_one -n $1 git commit --allow-empty --allow-empty-message \ - --amend --no-post-rewrite -n -q -C $1 || + --amend --no-post-rewrite -n -q -C $1 \ + ${gpg_sign_opt:+$gpg_sign_opt} || die_with_patch $1 Could not apply $1... $2 else pick_one $1 || @@ -497,7 +500,7 @@ do_next () { mark_action_done do_pick $sha1 $rest - git commit --amend --no-post-rewrite || { + git commit --amend --no-post-rewrite ${gpg_sign_opt:+$gpg_sign_opt} || { warn Could not amend commit after successfully picking $sha1... $rest warn This is most likely due to an empty commit message, or the pre-commit hook warn failed. If the pre-commit hook failed, you may need to resolve the issue before @@ -542,19 +545,22 @@ do_next () { squash|s|fixup|f) # This is an intermediate commit; its message will only be # used in case of trouble. So use the long version: - do_with_author output git commit --amend --no-verify -F $squash_msg || + do_with_author output git commit --amend --no-verify -F $squash_msg \ + ${gpg_sign_opt:+$gpg_sign_opt} ||