From: Junio C Hamano <gitster@pobox•com>
To: "brian m. carlson" <sandals@crustytoothpaste•net>
Cc: git@vger•kernel.org, Nicolas Vigier <boklm@mars-attacks•org>
Subject: Re: [PATCH v3 8/9] rebase: add the --gpg-sign option
Date: Mon, 03 Feb 2014 13:42:06 -0800 [thread overview]
Message-ID: <xmqqlhxrdgb5.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: 1391221086-1044384-9-git-send-email-sandals@crustytoothpaste.net
"brian m. carlson" <sandals@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" <sandals@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='-S"brian m. carson" <sandals@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 "${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.
echo "run this command: git commit$gpg_sign_opt"
> + ;;
> --)
> shift
> break
next prev parent reply other threads:[~2014-02-03 21:42 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-01 2:17 [PATCH v3 0/9] add --gpg-sign to rebase and pull brian m. carlson
2014-02-01 2:17 ` [PATCH v3 1/9] cherry-pick, revert: add the --gpg-sign option brian m. carlson
2014-02-03 20:50 ` Junio C Hamano
2014-02-01 2:17 ` [PATCH v3 2/9] git-sh-setup.sh: add variable to use the stuck-long mode brian m. carlson
2014-02-01 2:18 ` [PATCH v3 3/9] am: parse options in " brian m. carlson
2014-02-01 2:18 ` [PATCH v3 4/9] am: add the --gpg-sign option brian m. carlson
2014-02-01 2:18 ` [PATCH v3 5/9] rebase: remove useless arguments check brian m. carlson
2014-02-01 2:18 ` [PATCH v3 6/9] rebase: don't try to match -M option brian m. carlson
2014-02-01 2:18 ` [PATCH v3 7/9] rebase: parse options in stuck-long mode brian m. carlson
2014-02-01 2:18 ` [PATCH v3 8/9] rebase: add the --gpg-sign option brian m. carlson
2014-02-03 21:42 ` Junio C Hamano [this message]
2014-02-07 23:50 ` brian m. carlson
2014-02-08 13:04 ` Philip Oakley
2014-02-01 2:18 ` [PATCH v3 9/9] pull: " brian m. carlson
2014-02-03 20:31 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqlhxrdgb5.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=boklm@mars-attacks$(echo .)org \
--cc=git@vger$(echo .)kernel.org \
--cc=sandals@crustytoothpaste$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox