subtree split includes unrelated commits

subtree split includes unrelated commits

[Daniel Pauli]

[-- Attachment #1: Type: text/plain, Size: 3064 bytes --]

Hi there

First time I post to the mailing list here, I hope I get it right...

I came across an issue where pushing a subtree resulted in unexpected 
commits in the remote repository: These commits included files that are 
not part of the subtree's directory. There's a related question on 
stackoverflow: 
https://stackoverflow.com/questions/61150709/git-subtree-push-seems-to-push-commits-that-dont-apply-to-the-subtree

In my case, the excess commits were introduced by a merge commit of a 
branch that forked off the main branch *before* the subtree was added. 
This merge commit was the result of updating my fork from the upstream 
repository (which is not aware of the subtree, it just lives in my fork 
so far).

I found the issue to be related with the following change in git-subtree.sh:

> Revision: 933cfeb90b5d03b4096db6d60494a6eedea25d03
> Author: Dave Ware <davidw@realtimegenomics.com>
> Date: 15.01.2016 01:41:43
> Message:
> contrib/subtree: fix "subtree split" skipped-merge bug
>
> 'git subtree split' can incorrectly skip a merge even when both parents
> act on the subtree, provided the merge results in a tree identical to
> one of the parents. Fix by copying the merge if at least one parent is
> non-identical, and the non-identical parent is not an ancestor of the
> identical parent.
>
> Also, add a test case which checks that a descendant remains a
> descendent on the subtree in this case.
>
> Signed-off-by: Dave Ware <davidw@realtimegenomics.com>
> Reviewed-by: David A. Greene <greened@obbligato.org>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ----
> Modified: contrib/subtree/git-subtree.sh
> Modified: contrib/subtree/t/t7900-subtree.sh

>     copycommit=
>     if [ -n "$identical" ] && [ -n "$nonidentical" ]; then
>         extras=$(git rev-list --count $identical..$nonidentical)
>         if [ "$extras" -ne 0 ]; then
>             # we need to preserve history along the other branch
>  copycommit=1 // <---------------------
>         fi
>     fi
>     if [ -n "$identical" ] && [ -z "$copycommit" ]; then
>         echo $identical
>     else
>         copy_commit $rev $tree "$p" || exit $?
>     fi

As a workaround, removing the copycommit=1 line here results in a split 
as I would expect for my case.

I attached a sample repository and log that reproduces the issue:
- From the main branch, do a "git subtree split --prefix my_subtree -b 
splitted -d". This is how the branches listed below were created.
- branch "splitted_bad" shows the unexpected merge commit f7fd955 that 
includes files that are not part of the subtree (9eeff05)
- branch "splitted_good" shows the result as I would expect it, without 
merge commit f7fd955. It was done by commenting out the copycommit=1

Now I wonder, is this expected behavior, or could it be a regression? I 
do not really understand the inner workings of git-subtree.sh, but I 
feel that where it included "too few" commits before the mentioned 
patch, it might now include "too many".

Regards
Daniel


[-- Attachment #2: subtree-issue.zip --]
[-- Type: application/x-zip-compressed, Size: 109146 bytes --]

[-- Attachment #3: split.log --]
[-- Type: text/plain, Size: 5836 bytes --]

command: {split}
quiet: {}
dir: {my_subtree}
opts: {}

Splitting my_subtree...
Using cachedir: C:/projects/indel/inos-goes-git/subtree-issue/reproduce2/.git/subtree-cache/26
Looking for prior splits...
  Main is: '9eeff050a096a3c26e11ca4a4c5fab36aac5a122'
    Prior: 9eeff050a096a3c26e11ca4a4c5fab36aac5a122 -> 10222c693436810690ae56560a7bd188c9d476f2
progress: 1/12 (0) [0]
Processing commit: 9eeff050a096a3c26e11ca4a4c5fab36aac5a122
  prior: 10222c693436810690ae56560a7bd188c9d476f2
progress: 2/12 (0) [0]
Processing commit: 10222c693436810690ae56560a7bd188c9d476f2
  prior: 10222c693436810690ae56560a7bd188c9d476f2
progress: 3/12 (0) [0]
Processing commit: bbdc6c08bae4787ad7cdae930706cf133e14ece0
  parents: 9eeff050a096a3c26e11ca4a4c5fab36aac5a122 10222c693436810690ae56560a7bd188c9d476f2
    incorrect order: 9eeff050a096a3c26e11ca4a4c5fab36aac5a122
progress: 3/12 (1) [1]
    Processing commit: 9eeff050a096a3c26e11ca4a4c5fab36aac5a122
      prior: 10222c693436810690ae56560a7bd188c9d476f2
    incorrect order: 10222c693436810690ae56560a7bd188c9d476f2
progress: 3/12 (1) [2]
    Processing commit: 10222c693436810690ae56560a7bd188c9d476f2
      prior: 10222c693436810690ae56560a7bd188c9d476f2
  newparents: 10222c693436810690ae56560a7bd188c9d476f2
10222c693436810690ae56560a7bd188c9d476f2
  tree is: dcbf87dcfeb8d5781492fad086f834d90a159402
  newrev is: 10222c693436810690ae56560a7bd188c9d476f2
progress: 4/12 (1) [2]
Processing commit: 07804dbc6f40ea3d4ecd2c37efa1ba01d8264ef3
  parents: bbdc6c08bae4787ad7cdae930706cf133e14ece0
  newparents: 10222c693436810690ae56560a7bd188c9d476f2
  tree is: dcbf87dcfeb8d5781492fad086f834d90a159402
  newrev is: 10222c693436810690ae56560a7bd188c9d476f2
progress: 5/12 (2) [2]
Processing commit: d63e478df890d9907d11fd006a53ca735b4e8a99
  parents: 07804dbc6f40ea3d4ecd2c37efa1ba01d8264ef3
  newparents: 10222c693436810690ae56560a7bd188c9d476f2
  tree is: 861359eb2c24ca3597c71701ae5050484000018a
  copy_commit {d63e478df890d9907d11fd006a53ca735b4e8a99} {861359eb2c24ca3597c71701ae5050484000018a} { -p 10222c693436810690ae56560a7bd188c9d476f2}
  newrev is: 123b70309419425b1a769037ee8109df9cc9dcb8
progress: 6/12 (3) [2]
Processing commit: 909c2643aadd556960f35a7c5f1bff708029a886
  parents: 10222c693436810690ae56560a7bd188c9d476f2
  newparents: 10222c693436810690ae56560a7bd188c9d476f2
  tree is: 
progress: 7/12 (4) [2]
Processing commit: 21446bae6f64507c5e8986336ecfe96b1ac917dd
  parents: d63e478df890d9907d11fd006a53ca735b4e8a99 909c2643aadd556960f35a7c5f1bff708029a886
    incorrect order: d63e478df890d9907d11fd006a53ca735b4e8a99
progress: 7/12 (5) [3]
    Processing commit: d63e478df890d9907d11fd006a53ca735b4e8a99
      prior: 123b70309419425b1a769037ee8109df9cc9dcb8
  newparents: 123b70309419425b1a769037ee8109df9cc9dcb8
909c2643aadd556960f35a7c5f1bff708029a886
  tree is: 2b670b8b80ff0e7439c2e52381d38cf734140f03
  copy_commit {21446bae6f64507c5e8986336ecfe96b1ac917dd} {2b670b8b80ff0e7439c2e52381d38cf734140f03} { -p 123b70309419425b1a769037ee8109df9cc9dcb8 -p 909c2643aadd556960f35a7c5f1bff708029a886}
  newrev is: cb93d85c317b4edcecb1f3537240f0d15ccec236
progress: 8/12 (5) [3]
Processing commit: 50508755f2116f6077ccb4a1d5ad96501bce9bdb
  parents: 9eeff050a096a3c26e11ca4a4c5fab36aac5a122
  newparents: 10222c693436810690ae56560a7bd188c9d476f2
  tree is: 
progress: 9/12 (6) [3]
Processing commit: 4d89a44efd33c16730fc7b621f86b8b83bc322be
  parents: 21446bae6f64507c5e8986336ecfe96b1ac917dd 50508755f2116f6077ccb4a1d5ad96501bce9bdb
    incorrect order: 21446bae6f64507c5e8986336ecfe96b1ac917dd
progress: 9/12 (7) [4]
    Processing commit: 21446bae6f64507c5e8986336ecfe96b1ac917dd
      prior: cb93d85c317b4edcecb1f3537240f0d15ccec236
  newparents: cb93d85c317b4edcecb1f3537240f0d15ccec236
50508755f2116f6077ccb4a1d5ad96501bce9bdb
  tree is: 2b670b8b80ff0e7439c2e52381d38cf734140f03
  copy_commit {4d89a44efd33c16730fc7b621f86b8b83bc322be} {2b670b8b80ff0e7439c2e52381d38cf734140f03} { -p cb93d85c317b4edcecb1f3537240f0d15ccec236 -p 50508755f2116f6077ccb4a1d5ad96501bce9bdb}
  newrev is: f7fd955503d5a43ff6410e7d9971a9995c1cfcf4
progress: 10/12 (7) [4]
Processing commit: 95dab064b04801a57fb2b740bafa0d9f47dd4d66
  parents: 07804dbc6f40ea3d4ecd2c37efa1ba01d8264ef3
  newparents: 10222c693436810690ae56560a7bd188c9d476f2
  tree is: dcbf87dcfeb8d5781492fad086f834d90a159402
  newrev is: 10222c693436810690ae56560a7bd188c9d476f2
progress: 11/12 (8) [4]
Processing commit: 70e56ea7b4f76b2c2a3c9f580b914551d2c753f7
  parents: 95dab064b04801a57fb2b740bafa0d9f47dd4d66
  newparents: 10222c693436810690ae56560a7bd188c9d476f2
  tree is: 708b8e696c72bb5a1481e766cb0aedeed900266f
  copy_commit {70e56ea7b4f76b2c2a3c9f580b914551d2c753f7} {708b8e696c72bb5a1481e766cb0aedeed900266f} { -p 10222c693436810690ae56560a7bd188c9d476f2}
  newrev is: c5abf4f38a63d0a0c12e3827ec744602f2046eca
progress: 12/12 (9) [4]
Processing commit: 1b4a51ffd795ef3ce61f633042f4773095e0548c
  parents: 4d89a44efd33c16730fc7b621f86b8b83bc322be 70e56ea7b4f76b2c2a3c9f580b914551d2c753f7
    incorrect order: 4d89a44efd33c16730fc7b621f86b8b83bc322be
progress: 12/12 (10) [5]
    Processing commit: 4d89a44efd33c16730fc7b621f86b8b83bc322be
      prior: f7fd955503d5a43ff6410e7d9971a9995c1cfcf4
    incorrect order: 70e56ea7b4f76b2c2a3c9f580b914551d2c753f7
progress: 12/12 (10) [6]
    Processing commit: 70e56ea7b4f76b2c2a3c9f580b914551d2c753f7
      prior: c5abf4f38a63d0a0c12e3827ec744602f2046eca
  newparents: f7fd955503d5a43ff6410e7d9971a9995c1cfcf4
c5abf4f38a63d0a0c12e3827ec744602f2046eca
  tree is: acc21543759a45fdfff8dd66c231cb37321f9255
  copy_commit {1b4a51ffd795ef3ce61f633042f4773095e0548c} {acc21543759a45fdfff8dd66c231cb37321f9255} { -p f7fd955503d5a43ff6410e7d9971a9995c1cfcf4 -p c5abf4f38a63d0a0c12e3827ec744602f2046eca}
  newrev is: 2c045f7700e241fb6cd73fd71e12af5499844eb8
Updated branch 'splitted'

[-- Attachment #4: main.png --]
[-- Type: image/png, Size: 33974 bytes --]

[-- Attachment #5: splitted_bad.png --]
[-- Type: image/png, Size: 27427 bytes --]

[-- Attachment #6: splitted_good.png --]
[-- Type: image/png, Size: 20999 bytes --]

Re: subtree split includes unrelated commits

[Douglas Leonard]

I hope I got this reply sent to the right thread.

> I found the issue to be related with the following change in git-subtree.sh:

> > Revision: 933cfeb90b5d03b4096db6d60494a6eedea25d03
> > Author: Dave Ware <davidw@realtimegenomics.com>
> > Date: 15.01.2016 01:41:43
> > Message:
> > contrib/subtree: fix "subtree split" skipped-merge bug

> > 'git subtree split' can incorrectly skip a merge even when both parents
> > act on the subtree, provided the merge results in a tree identical to
> > one of the parents. Fix by copying the merge if at least one parent is
> > non-identical, and the non-identical parent is not an ancestor of the
> > identical parent.

Hmm... I'm not expert on the implementation details of subtree split.
But I wonder if this should have said "if at least one parent is
non-subtree-identical"?  Maybe that's what was meant. I just went
through writing a similar "degenerate  merge" (to adopt the
filter-repo terminology for it) detection for git-alltrees, although
I'm not sure alltrees would handle this particular case to your liking
either.  I think I discard merges if either parent is
subtree-identical to the merge commit, probably keeping only the
matching branch, but I'm not sure that's always true, and that's
there, not here.

Cheers,
Doug

Re: subtree split includes unrelated commits

[Douglas Leonard]

>
> I hope I got this reply sent to the right thread.
>

Oops, probably forgetting the Re broke that.