Skip to content

contrib/subtree: Add -S/-gpg-sign #1928

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

contrib/subtree: Add -S/-gpg-sign #1928

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5bbdbf0
contrib/subtree: parse using --stuck-long
pweiskircher May 30, 2025
af5b4a6
contrib/subtree: add -S/--gpg-sign
pweiskircher May 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions contrib/subtree/git-subtree.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ git-subtree - Merge subtrees together and split repository into subtrees
SYNOPSIS
--------
[verse]
'git subtree' [<options>] -P <prefix> add <local-commit>
'git subtree' [<options>] -P <prefix> add <repository> <remote-ref>
'git subtree' [<options>] -P <prefix> merge <local-commit> [<repository>]
'git subtree' [<options>] -P <prefix> split [<local-commit>]
'git subtree' [<options>] -P <prefix> [-S[<keyid>]] add <local-commit>
'git subtree' [<options>] -P <prefix> [-S[<keyid>]] add <repository> <remote-ref>
'git subtree' [<options>] -P <prefix> [-S[<keyid>]] merge <local-commit> [<repository>]
'git subtree' [<options>] -P <prefix> [-S[<keyid>]] split [<local-commit>]

[verse]
'git subtree' [<options>] -P <prefix> pull <repository> <remote-ref>
'git subtree' [<options>] -P <prefix> push <repository> <refspec>
'git subtree' [<options>] -P <prefix> [-S[<keyid>]] pull <repository> <remote-ref>
'git subtree' [<options>] -P <prefix> [-S[<keyid>]] push <repository> <refspec>

DESCRIPTION
-----------
Expand Down Expand Up @@ -149,6 +149,13 @@ OPTIONS FOR ALL COMMANDS
want to manipulate. This option is mandatory
for all commands.

-S[<keyid>]::
--gpg-sign[=<keyid>]::
--no-gpg-sign::
GPG-sign commits. The `keyid` argument is optional and
defaults to the committer identity; `--no-gpg-sign` is useful to
countermand a `--gpg-sign` option given earlier on the command line.

OPTIONS FOR 'add' AND 'merge' (ALSO: 'pull', 'split --rejoin', AND 'push --rejoin')
-----------------------------------------------------------------------------------
These options for 'add' and 'merge' may also be given to 'pull' (which
Expand Down
66 changes: 32 additions & 34 deletions contrib/subtree/git-subtree.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ then
fi

OPTS_SPEC="\
git subtree add --prefix=<prefix> <commit>
git subtree add --prefix=<prefix> <repository> <ref>
git subtree merge --prefix=<prefix> <commit>
git subtree split --prefix=<prefix> [<commit>]
git subtree pull --prefix=<prefix> <repository> <ref>
git subtree push --prefix=<prefix> <repository> <refspec>
git subtree add --prefix=<prefix> [-S[=<key-id>]] <commit>
git subtree add --prefix=<prefix> [-S[=<key-id>]] <repository> <ref>
git subtree merge --prefix=<prefix> [-S[=<key-id>]] <commit>
git subtree split --prefix=<prefix> [-S[=<key-id>]] [<commit>]
git subtree pull --prefix=<prefix> [-S[=<key-id>]] <repository> <ref>
git subtree push --prefix=<prefix> [-S[=<key-id>]] <repository> <refspec>
--
h,help! show the help
q,quiet! quiet
Expand All @@ -46,6 +46,7 @@ rejoin merge the new branch back into HEAD
options for 'add' and 'merge' (also: 'pull', 'split --rejoin', and 'push --rejoin')
squash merge subtree changes as a single commit
m,message!= use the given message as the commit message for the merge commit
S,gpg-sign?key-id GPG-sign commits. The keyid argument is optional and defaults to the committer identity
"

indent=0
Expand Down Expand Up @@ -115,7 +116,7 @@ main () {
then
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, "D. Ben Knoble" wrote (reply to this):

On Mon, Jun 2, 2025 at 12:41 PM Patrik Weiskircher via GitGitGadget
<[email protected]> wrote:
>
> From: Patrik Weiskircher <[email protected]>
>
> -S/--gpg-sign requires an optional parameter. Optional parameter
> handling only works unambiguous with git rev-parse --parseopt when using
> the --stuck-long option.

Here we mention "-S", but that flag isn't implemented yet, right?

Perhaps something like:

    Optional parameter handling only works unambiguous with git rev-parse
    --parseopt when using the --stuck-long option. To prepare for future commits
    which add flags with optional parameters, parse with --stuck-long.

>
> Signed-off-by: Patrik Weiskircher <[email protected]>
> ---
>  contrib/subtree/git-subtree.sh | 34 +++++++++++++---------------------
>  1 file changed, 13 insertions(+), 21 deletions(-)
>
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index 15ae86db1b27..60b2431b8bba 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -115,7 +115,7 @@ main () {
>         then
>                 set -- -h
>         fi
> -       set_args="$(echo "$OPTS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?)"
> +       set_args="$(echo "$OPTS_SPEC" | git rev-parse --parseopt --stuck-long -- "$@" || echo exit $?)"
>         eval "$set_args"
>         . git-sh-setup
>         require_work_tree
> @@ -131,9 +131,6 @@ main () {
>                 opt="$1"
>                 shift
>                 case "$opt" in
> -                       --annotate|-b|-P|-m|--onto)
> -                               shift
> -                               ;;
>                         --rejoin)
>                                 arg_split_rejoin=1
>                                 ;;
> @@ -177,42 +174,37 @@ main () {
>                 shift
>
>                 case "$opt" in
> -               -q)
> +               --quiet)
>                         arg_quiet=1
>                         ;;
> -               -d)
> +               --debug)
>                         arg_debug=1
>                         ;;
> -               --annotate)
> +               --annotate=*)
>                         test -n "$allow_split" || die_incompatible_opt "$opt" "$arg_command"
> -                       arg_split_annotate="$1"
> -                       shift
> +                       arg_split_annotate="${opt#*=}"
>                         ;;
>                 --no-annotate)
>                         test -n "$allow_split" || die_incompatible_opt "$opt" "$arg_command"
>                         arg_split_annotate=
>                         ;;
> -               -b)
> +               --branch=*)
>                         test -n "$allow_split" || die_incompatible_opt "$opt" "$arg_command"
> -                       arg_split_branch="$1"
> -                       shift
> +                       arg_split_branch="${opt#*=}"
>                         ;;
> -               -P)
> -                       arg_prefix="${1%/}"
> -                       shift
> +               --prefix=*)
> +                       arg_prefix="${opt#*=}"
>                         ;;
> -               -m)
> +               --message=*)
>                         test -n "$allow_addmerge" || die_incompatible_opt "$opt" "$arg_command"
> -                       arg_addmerge_message="$1"
> -                       shift
> +                       arg_addmerge_message="${opt#*=}"
>                         ;;
>                 --no-prefix)
>                         arg_prefix=
>                         ;;
> -               --onto)
> +               --onto=*)
>                         test -n "$allow_split" || die_incompatible_opt "$opt" "$arg_command"
> -                       arg_split_onto="$1"
> -                       shift
> +                       arg_split_onto="${opt#*=}"
>                         ;;
>                 --no-onto)
>                         test -n "$allow_split" || die_incompatible_opt "$opt" "$arg_command"
> --
> gitgitgadget
>
>


-- 
D. Ben Knoble

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Patrik Weiskircher wrote (reply to this):

On Tue, Jun 3, 2025 at 4:42 PM D. Ben Knoble <[email protected]> wrote:
>
> On Mon, Jun 2, 2025 at 12:41 PM Patrik Weiskircher via GitGitGadget
> <[email protected]> wrote:
> >
> > From: Patrik Weiskircher <[email protected]>
> >
> > -S/--gpg-sign requires an optional parameter. Optional parameter
> > handling only works unambiguous with git rev-parse --parseopt when using
> > the --stuck-long option.
>
> Here we mention "-S", but that flag isn't implemented yet, right?
>
> Perhaps something like:
>
>     Optional parameter handling only works unambiguous with git rev-parse
>     --parseopt when using the --stuck-long option. To prepare for future commits
>     which add flags with optional parameters, parse with --stuck-long.
>

Makes sense! Changing that. What is a good policy to resubmit
something? Should I wait longer? Sorry, very new here!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, "Kristoffer Haugsbakk" wrote (reply to this):

On Wed, Jun 4, 2025, at 15:56, Patrik Weiskircher wrote:
>>
>> Here we mention "-S", but that flag isn't implemented yet, right?
>>
>> Perhaps something like:
>>
>>     Optional parameter handling only works unambiguous with git rev-parse
>>     --parseopt when using the --stuck-long option. To prepare for future commits
>>     which add flags with optional parameters, parse with --stuck-long.
>>
>
> Makes sense! Changing that. What is a good policy to resubmit
> something? Should I wait longer? Sorry, very new here!

• Force-push your branch to gitgitgadget
• Edit the PR description with something like “Changes since v1:” to
  summarize the changes
• (`/preview` comment)
• To send the next version: `/submit` comment again

I think that’s it. :)

I don’t think there’s a need to wait if you don’t want to.

-- 
Kristoffer Haugsbakk

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Kristoffer Haugsbakk" <[email protected]> writes:

> On Wed, Jun 4, 2025, at 15:56, Patrik Weiskircher wrote:
>>>
>>> Here we mention "-S", but that flag isn't implemented yet, right?
>>>
>>> Perhaps something like:
>>>
>>>     Optional parameter handling only works unambiguous with git rev-parse
>>>     --parseopt when using the --stuck-long option. To prepare for future commits
>>>     which add flags with optional parameters, parse with --stuck-long.
>>>
>>
>> Makes sense! Changing that. What is a good policy to resubmit
>> something? Should I wait longer? Sorry, very new here!
>
> • Force-push your branch to gitgitgadget
> • Edit the PR description with something like “Changes since v1:” to
>   summarize the changes
> • (`/preview` comment)
> • To send the next version: `/submit` comment again
>
> I think that’s it. :)
>
> I don’t think there’s a need to wait if you don’t want to.

It would be nice for potential reviewers to give at least 24 hours
to ensure people anywhere on the globe have a chance to comment, and
a chance for you to respond to them, before sending your next
iteration.

Also, for future reference, when responding to a review comment that
causes you to drastically change the course of the series, you can
respond whenever you want to, but it is nice to other potential
reviewers to give at least 24 hours to voice their opinions, before
sending an updated series based on that comment, since suggested
changes in such a comment may be controversial and after seeing you
spend some time already to adjust to it, others may feel discouraged
to make you redo your series again even whey they think the
suggested changes are not taking us in the right direction.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Patrik Weiskircher wrote (reply to this):

On Wed, Jun 4, 2025 at 11:41 AM Junio C Hamano <[email protected]> wrote:
>
> "Kristoffer Haugsbakk" <[email protected]> writes:
>
> > On Wed, Jun 4, 2025, at 15:56, Patrik Weiskircher wrote:
> >>>
> >>> Here we mention "-S", but that flag isn't implemented yet, right?
> >>>
> >>> Perhaps something like:
> >>>
> >>>     Optional parameter handling only works unambiguous with git rev-parse
> >>>     --parseopt when using the --stuck-long option. To prepare for future commits
> >>>     which add flags with optional parameters, parse with --stuck-long.
> >>>
> >>
> >> Makes sense! Changing that. What is a good policy to resubmit
> >> something? Should I wait longer? Sorry, very new here!
> >
> > • Force-push your branch to gitgitgadget
> > • Edit the PR description with something like “Changes since v1:” to
> >   summarize the changes
> > • (`/preview` comment)
> > • To send the next version: `/submit` comment again
> >
> > I think that’s it. :)
> >
> > I don’t think there’s a need to wait if you don’t want to.
>
> It would be nice for potential reviewers to give at least 24 hours
> to ensure people anywhere on the globe have a chance to comment, and
> a chance for you to respond to them, before sending your next
> iteration.
>
> Also, for future reference, when responding to a review comment that
> causes you to drastically change the course of the series, you can
> respond whenever you want to, but it is nice to other potential
> reviewers to give at least 24 hours to voice their opinions, before
> sending an updated series based on that comment, since suggested
> changes in such a comment may be controversial and after seeing you
> spend some time already to adjust to it, others may feel discouraged
> to make you redo your series again even whey they think the
> suggested changes are not taking us in the right direction.

Makes sense! I'll keep that in mind for the future!

set -- -h
fi
set_args="$(echo "$OPTS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?)"
set_args="$(echo "$OPTS_SPEC" | git rev-parse --parseopt --stuck-long -- "$@" || echo exit $?)"
eval "$set_args"
. git-sh-setup
require_work_tree
Expand All @@ -131,9 +132,6 @@ main () {
opt="$1"
shift
case "$opt" in
--annotate|-b|-P|-m|--onto)
shift
;;
--rejoin)
arg_split_rejoin=1
;;
Expand Down Expand Up @@ -171,48 +169,44 @@ main () {
arg_split_annotate=
arg_addmerge_squash=
arg_addmerge_message=
arg_gpg_sign=
while test $# -gt 0
do
opt="$1"
shift

case "$opt" in
-q)
--quiet)
arg_quiet=1
;;
-d)
--debug)
arg_debug=1
;;
--annotate)
--annotate=*)
test -n "$allow_split" || die_incompatible_opt "$opt" "$arg_command"
arg_split_annotate="$1"
shift
arg_split_annotate="${opt#*=}"
;;
--no-annotate)
test -n "$allow_split" || die_incompatible_opt "$opt" "$arg_command"
arg_split_annotate=
;;
-b)
--branch=*)
test -n "$allow_split" || die_incompatible_opt "$opt" "$arg_command"
arg_split_branch="$1"
shift
arg_split_branch="${opt#*=}"
;;
-P)
arg_prefix="${1%/}"
shift
--prefix=*)
arg_prefix="${opt#*=}"
;;
-m)
--message=*)
test -n "$allow_addmerge" || die_incompatible_opt "$opt" "$arg_command"
arg_addmerge_message="$1"
shift
arg_addmerge_message="${opt#*=}"
;;
--no-prefix)
arg_prefix=
;;
--onto)
--onto=*)
test -n "$allow_split" || die_incompatible_opt "$opt" "$arg_command"
arg_split_onto="$1"
shift
arg_split_onto="${opt#*=}"
;;
--no-onto)
test -n "$allow_split" || die_incompatible_opt "$opt" "$arg_command"
Expand Down Expand Up @@ -240,6 +234,9 @@ main () {
test -n "$allow_addmerge" || die_incompatible_opt "$opt" "$arg_command"
arg_addmerge_squash=
;;
--gpg-sign=* | --gpg-sign | --no-gpg-sign)
arg_gpg_sign="$opt"
;;
--)
break
;;
Expand Down Expand Up @@ -272,6 +269,7 @@ main () {
debug "quiet: {$arg_quiet}"
debug "dir: {$dir}"
debug "opts: {$*}"
debug "gpg-sign: {$arg_gpg_sign}"
debug

"cmd_$arg_command" "$@"
Expand Down Expand Up @@ -537,7 +535,7 @@ copy_commit () {
printf "%s" "$arg_split_annotate"
cat
) |
git commit-tree "$2" $3 # reads the rest of stdin
git commit-tree $arg_gpg_sign "$2" $3 # reads the rest of stdin
) || die "fatal: can't copy commit $1"
}

Expand Down Expand Up @@ -683,10 +681,10 @@ new_squash_commit () {
if test -n "$old"
then
squash_msg "$dir" "$oldsub" "$newsub" |
git commit-tree "$tree" -p "$old" || exit $?
git commit-tree $arg_gpg_sign "$tree" -p "$old" || exit $?
else
squash_msg "$dir" "" "$newsub" |
git commit-tree "$tree" || exit $?
git commit-tree $arg_gpg_sign "$tree" || exit $?
fi
}

Expand Down Expand Up @@ -925,11 +923,11 @@ cmd_add_commit () {
then
rev=$(new_squash_commit "" "" "$rev") || exit $?
commit=$(add_squashed_msg "$rev" "$dir" |
git commit-tree "$tree" $headp -p "$rev") || exit $?
git commit-tree $arg_gpg_sign "$tree" $headp -p "$rev") || exit $?
else
revp=$(peel_committish "$rev") || exit $?
commit=$(add_msg "$dir" $headrev "$rev" |
git commit-tree "$tree" $headp -p "$revp") || exit $?
git commit-tree $arg_gpg_sign "$tree" $headp -p "$revp") || exit $?
fi
git reset "$commit" || exit $?

Expand Down Expand Up @@ -1080,9 +1078,9 @@ cmd_merge () {
if test -n "$arg_addmerge_message"
then
git merge --no-ff -Xsubtree="$arg_prefix" \
--message="$arg_addmerge_message" "$rev"
--message="$arg_addmerge_message" $arg_gpg_sign "$rev"
else
git merge --no-ff -Xsubtree="$arg_prefix" $rev
git merge --no-ff -Xsubtree="$arg_prefix" $arg_gpg_sign $rev
fi
}

Expand Down
113 changes: 113 additions & 0 deletions contrib/subtree/t/t7900-subtree.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ and push subcommands of git subtree.

TEST_DIRECTORY=$(pwd)/../../../t
. "$TEST_DIRECTORY"/test-lib.sh
. "$TEST_DIRECTORY"/lib-gpg.sh

# Use our own wrapper around test-lib.sh's test_create_repo, in order
# to set log.date=relative. `git subtree` parses the output of `git
Expand Down Expand Up @@ -1563,4 +1564,116 @@ test_expect_success 'subtree descendant check' '
)
'

test_expect_success GPG 'add subproj with GPG signing using -S flag' '
subtree_test_create_repo "$test_count" &&
subtree_test_create_repo "$test_count/sub proj" &&
test_create_commit "$test_count" main1 &&
test_create_commit "$test_count/sub proj" sub1 &&
(
cd "$test_count" &&
git fetch ./"sub proj" HEAD &&
git subtree add --prefix="sub dir" -S FETCH_HEAD &&
git verify-commit HEAD &&
test "$(last_commit_subject)" = "Add '\''sub dir/'\'' from commit '\''$(git rev-parse FETCH_HEAD)'\''"
)
'

test_expect_success GPG 'add subproj with GPG signing using --gpg-sign flag' '
subtree_test_create_repo "$test_count" &&
subtree_test_create_repo "$test_count/sub proj" &&
test_create_commit "$test_count" main1 &&
test_create_commit "$test_count/sub proj" sub1 &&
(
cd "$test_count" &&
git fetch ./"sub proj" HEAD &&
git subtree add --prefix="sub dir" --gpg-sign FETCH_HEAD &&
git verify-commit HEAD &&
test "$(last_commit_subject)" = "Add '\''sub dir/'\'' from commit '\''$(git rev-parse FETCH_HEAD)'\''"
)
'

test_expect_success GPG 'add subproj with GPG signing using specific key ID' '
subtree_test_create_repo "$test_count" &&
subtree_test_create_repo "$test_count/sub proj" &&
test_create_commit "$test_count" main1 &&
test_create_commit "$test_count/sub proj" sub1 &&
(
cd "$test_count" &&
git fetch ./"sub proj" HEAD &&
git subtree add --prefix="sub dir" -S"$GIT_COMMITTER_EMAIL" FETCH_HEAD &&
git verify-commit HEAD &&
test "$(last_commit_subject)" = "Add '\''sub dir/'\'' from commit '\''$(git rev-parse FETCH_HEAD)'\''"
)
'

test_expect_success GPG 'merge with GPG signing' '
subtree_test_create_repo "$test_count" &&
subtree_test_create_repo "$test_count/sub proj" &&
test_create_commit "$test_count" main1 &&
test_create_commit "$test_count/sub proj" sub1 &&
(
cd "$test_count" &&
git fetch ./"sub proj" HEAD &&
git subtree add --prefix="sub dir" FETCH_HEAD
) &&
test_create_commit "$test_count/sub proj" sub2 &&
(
cd "$test_count" &&
git fetch ./"sub proj" HEAD &&
git subtree merge --prefix="sub dir" -S FETCH_HEAD &&
git verify-commit HEAD
)
'

test_expect_success GPG 'split with GPG signing and --rejoin' '
subtree_test_create_repo "$test_count" &&
subtree_test_create_repo "$test_count/sub proj" &&
test_create_commit "$test_count" main1 &&
test_create_commit "$test_count/sub proj" sub1 &&
(
cd "$test_count" &&
git fetch ./"sub proj" HEAD &&
git subtree add --prefix="sub dir" FETCH_HEAD
) &&
test_create_commit "$test_count" "sub dir/main-sub1" &&
(
cd "$test_count" &&
git subtree split --prefix="sub dir" --rejoin -S &&
git verify-commit HEAD
)
'

test_expect_success GPG 'add with --squash and GPG signing' '
subtree_test_create_repo "$test_count" &&
subtree_test_create_repo "$test_count/sub proj" &&
test_create_commit "$test_count" main1 &&
test_create_commit "$test_count/sub proj" sub1 &&
(
cd "$test_count" &&
git fetch ./"sub proj" HEAD &&
git subtree add --prefix="sub dir" --squash -S FETCH_HEAD &&
git verify-commit HEAD &&
# With --squash, the commit subject should reference the squash commit (first parent of merge)
squash_commit=$(git rev-parse HEAD^2) &&
test "$(last_commit_subject)" = "Merge commit '\''$squash_commit'\'' as '\''sub dir'\''"
)
'

test_expect_success GPG 'pull with GPG signing' '
subtree_test_create_repo "$test_count" &&
subtree_test_create_repo "$test_count/sub proj" &&
test_create_commit "$test_count" main1 &&
test_create_commit "$test_count/sub proj" sub1 &&
(
cd "$test_count" &&
git subtree add --prefix="sub dir" ./"sub proj" HEAD
) &&
test_create_commit "$test_count/sub proj" sub2 &&
(
cd "$test_count" &&
git subtree pull --prefix="sub dir" -S ./"sub proj" HEAD &&
git verify-commit HEAD
)
'

test_done
Loading