Re: Bug Report: Subtrees and GPG Signed Commits

[Stephen R Guglielmo <srguglielmo@gmail.com>]

On Mon, Feb 5, 2018 at 9:30 AM, Stephen R Guglielmo
<srguglielmo@gmail.com> wrote:
> On Wed, Jan 31, 2018 at 7:33 AM, Stephen R Guglielmo
> <srguglielmo@gmail.com> wrote:
>> On Tue, Jan 30, 2018 at 6:37 PM, Avery Pennarun <apenwarr@gmail.com> wrote:
>>> On Tue, Jan 30, 2018 at 6:24 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>>> Stefan Beller <sbeller@google.com> writes:
>>>>> There has not been feedback for a while on this thread.
>>>>> I think that is because subtrees are not in anyone's hot
>>>>> interest area currently.
>>>>>
>>>>> This is definitely the right place to submit&discuss bugs.
>>>>> Looking through "git log --format="%ae %s" -S subtree",
>>>>> it seems as if Avery (apenwarr@gmail.com) was mostly
>>>>> interested in developing subtrees, though I think he has
>>>>> moved on. Originally it was invented by Junio, who is
>>>>> the active maintainer of the project in 68faf68938
>>>>> (A new merge stragety 'subtree'., 2007-02-15)
>>>>
>>>> Thanks for trying to help, but I have *NOTHING* to do with the "git
>>>> subtree" subcommand (and I personally have no interest in it).  What
>>>> I did was a subtree merge strategy (i.e. "git merge -s subtree"),
>>>> which is totally a different thing.
>>>>
>>>> David Greene offered to take it over in 2015, and then we saw some
>>>> activity by David Aguilar in 2016, but otherwise the subcommand from
>>>> contrib/ has pretty much been dormant these days.
>>>
>>> Strictly speaking, the 'git subtree' command does in fact use 'git
>>> merge -s subtree' under the covers, so Junio is at least partly
>>> responsible for giving me the idea :)
>>>
>>> I actually have never looked into how signed commits work and although
>>> I still use git-subtree occasionally (it hasn't needed any
>>> maintenance, for my simple use cases), I have never used it with
>>> signed commits.
>>>
>>> git-subtree maintains a cache that maps commit ids in the "original
>>> project" with their equivalents in the "merged project."  If there's
>>> something magic about how commit ids work with signed commits, I could
>>> imagine that causing the "no a valid object name" problems.  Or,
>>> git-subtree in --squash mode actually generates new commit objects
>>> using some magic of its own.  If it were to accidentally copy a
>>> signature into a commit that no longer matches the original, I imagine
>>> that new object might get rejected.
>>>
>>> Unfortunately I don't have time to look into it.  The git-subtree code
>>> is pretty straightforward, though, so if Stephen has an hour or two to
>>> look deeper it's probably possible to fix it up.  The tool is not
>>> actually as magical and difficult as it might seem at first glance :)
>>>
>>> Sorry I can't help more.
>>>
>>> Good luck,
>>>
>>> Avery
>>
>> Thanks all for the discussion/replies.
>>
>> We use subtrees extensively in our environment right now. The "sub"
>> repos (90+) are located on GitHub, while the "main/parent" repo is
>> provided by a vendor on website hosting infrastructure.
>>
>> I will take a look at:
>> git/Documentation/CodingGuidelines
>> git/Documentation/SubmittingPatches
>> git/contrib/subtree/
>>
>> Should I follow up in this thread with a patch (it might be a while)?
>>
>> Thanks!
>> Steve
>
> Hi all,
>
> It looks like I've found the cause of the issue. I have
> log.showsignature=true in my gitconfig. The toptree_for_commit()
> function calls `git log` and passes the output to `git commit-tree` in
> new_squash_commit(). Apparently commit-tree doesn't like GPG sigs.
>
> The fix was simple: --no-show-signature. However, I believe this was
> added in git v2.10.0, so it's not fully backwards compatible. I'm open
> to suggestions on a better fix if this is not acceptable.
>
> Thanks!
>
>
> https://github.com/srguglielmo/git/commit/822c8a45d049f86ea5c59c0b434303964e4e6f3d
>
>
>
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index cc033af73..dec085a23 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -475,7 +475,7 @@ squash_msg () {
>
>  toptree_for_commit () {
>         commit="$1"
> -       git log -1 --pretty=format:'%T' "$commit" -- || exit $?
> +       git log --no-show-signature -1 --pretty=format:'%T' "$commit"
> -- || exit $?
>  }
>
>  subtree_for_commit () {

Hey again,

Actually, to follow up on this, I added --no-show-signature to several
other locations. The above patch fixes the fatal, however the GPG sig
info is still included in the commit merge message for `subtree pull`.
This fixes that as well.

https://github.com/srguglielmo/git/commit/ebd2f628ddb960931aac5087c45a54b953976e99


diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index cc033af73..8126132dc 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -297,7 +297,7 @@ find_latest_squash () {
        main=
        sub=
        git log --grep="^git-subtree-dir: $dir/*\$" \
-               --pretty=format:'START %H%n%s%n%n%b%nEND%n' HEAD |
+               --no-show-signature --pretty=format:'START
%H%n%s%n%n%b%nEND%n' HEAD |
        while read a b junk
        do
                debug "$a $b $junk"
@@ -341,7 +341,7 @@ find_existing_splits () {
        main=
        sub=
        git log --grep="^git-subtree-dir: $dir/*\$" \
-               --pretty=format:'START %H%n%s%n%n%b%nEND%n' $revs |
+               --no-show-signature --pretty=format:'START
%H%n%s%n%n%b%nEND%n' $revs |
        while read a b junk
        do
                case "$a" in
@@ -382,7 +382,7 @@ copy_commit () {
        # We're going to set some environment vars here, so
        # do it in a subshell to get rid of them safely later
        debug copy_commit "{$1}" "{$2}" "{$3}"
-       git log -1 --pretty=format:'%an%n%ae%n%aD%n%cn%n%ce%n%cD%n%B' "$1" |
+       git log --no-show-signature -1
--pretty=format:'%an%n%ae%n%aD%n%cn%n%ce%n%cD%n%B' "$1" |
        (
                read GIT_AUTHOR_NAME
                read GIT_AUTHOR_EMAIL
@@ -462,8 +462,8 @@ squash_msg () {
                oldsub_short=$(git rev-parse --short "$oldsub")
                echo "Squashed '$dir/' changes from
$oldsub_short..$newsub_short"
                echo
-               git log --pretty=tformat:'%h %s' "$oldsub..$newsub"
-               git log --pretty=tformat:'REVERT: %h %s' "$newsub..$oldsub"
+               git log --no-show-signature --pretty=tformat:'%h %s'
"$oldsub..$newsub"
+               git log --no-show-signature --pretty=tformat:'REVERT:
%h %s' "$newsub..$oldsub"
        else
                echo "Squashed '$dir/' content from commit $newsub_short"
        fi