[BUG] submodule move badly handled by git-rebase

[BUG] submodule move badly handled by git-rebase

[ydirson]

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

Hello all,

When rebasing commits involving move of a submodule, git-rebase fails to
record in index the "add" part of the rename.  This leaves the workdir
dirty and the rebase gets stopped.

fast-export of a testcase is attached.  To reproduce, just 
"git rebase -i", add a "break" before the move commit,
use this to introduce some noise, and watch.

Best regards,
-- 
Yann


(master)$ git rebase -i HEAD^^
hint: Waiting for your editor to close the file... Waiting for Emacs...
Stopped at b0e1b00... add submodule

(master|REBASE 2/3)$ echo >>README 

(master|REBASE 2/3)$ git commit -a -m noise
[detached HEAD d67c886] noise
 1 file changed, 1 insertion(+)

(master|REBASE 2/3)$ git rebase --continue 
Adding as subdir/gitlab-oe~08e230f... move submodule instead
error: could not apply 08e230f... move submodule
Resolve all conflicts manually, mark them as resolved with
"git add/rm <conflicted_files>", then run "git rebase --continue".
You can instead skip this commit: run "git rebase --skip".
To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 08e230f... move submodule

(master|REBASE 3/3)$ git st
interactive rebase in progress; onto c21ef8e
Last commands done (3 commands done):
   break
   pick 08e230f move submodule
  (see more in file .git/rebase-merge/done)
No commands remaining.
You are currently rebasing branch 'master' on 'c21ef8e'.
  (fix conflicts and then run "git rebase --continue")
  (use "git rebase --skip" to skip this patch)
  (use "git rebase --abort" to check out the original branch)

Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
        modified:   .gitmodules
        deleted:    gitlab-oe

Unmerged paths:
  (use "git restore --staged <file>..." to unstage)
  (use "git add <file>..." to mark resolution)
        added by them:   subdir/gitlab-oe

(master|REBASE 3/3)$ 

[-- Attachment #2: submodule-move.fexp --]
[-- Type: application/octet-stream, Size: 1034 bytes --]

blob
mark :1
data 4
foo

reset refs/heads/master
commit refs/heads/master
mark :2
author Yann Dirson <yann@blade-group.com> 1586276255 +0200
committer Yann Dirson <yann@blade-group.com> 1586276255 +0200
data 8
initial
M 100644 :1 README

blob
mark :3
data 93
[submodule "gitlab-oe"]
	path = gitlab-oe
	url = https://github.com/BladeGroup/gitlab-oe.git

commit refs/heads/master
mark :4
author Yann Dirson <yann@blade-group.com> 1586276297 +0200
committer Yann Dirson <yann@blade-group.com> 1586276297 +0200
data 14
add submodule
from :2
M 100644 :3 .gitmodules
M 160000 dd943dc6c97664d409feaaff3647c242dbb300df gitlab-oe

blob
mark :5
data 100
[submodule "gitlab-oe"]
	path = subdir/gitlab-oe
	url = https://github.com/BladeGroup/gitlab-oe.git

commit refs/heads/master
mark :6
author Yann Dirson <yann@blade-group.com> 1586276328 +0200
committer Yann Dirson <yann@blade-group.com> 1586276328 +0200
data 15
move submodule
from :4
M 100644 :5 .gitmodules
D gitlab-oe
M 160000 dd943dc6c97664d409feaaff3647c242dbb300df subdir/gitlab-oe

Re: [BUG] submodule move badly handled by git-rebase

[Elijah Newren]

Hi Yann,

On Tue, Apr 7, 2020 at 9:36 AM <ydirson@free.fr> wrote:
>
> Hello all,
>
> When rebasing commits involving move of a submodule, git-rebase fails to
> record in index the "add" part of the rename.  This leaves the workdir
> dirty and the rebase gets stopped.
>
> fast-export of a testcase is attached.  To reproduce, just
> "git rebase -i", add a "break" before the move commit,
> use this to introduce some noise, and watch.
>
> Best regards,
> --
> Yann
>
>
> (master)$ git rebase -i HEAD^^
> hint: Waiting for your editor to close the file... Waiting for Emacs...
> Stopped at b0e1b00... add submodule
>
> (master|REBASE 2/3)$ echo >>README
>
> (master|REBASE 2/3)$ git commit -a -m noise
> [detached HEAD d67c886] noise
>  1 file changed, 1 insertion(+)
>
> (master|REBASE 2/3)$ git rebase --continue
> Adding as subdir/gitlab-oe~08e230f... move submodule instead
> error: could not apply 08e230f... move submodule
> Resolve all conflicts manually, mark them as resolved with
> "git add/rm <conflicted_files>", then run "git rebase --continue".
> You can instead skip this commit: run "git rebase --skip".
> To abort and get back to the state before "git rebase", run "git rebase --abort".
> Could not apply 08e230f... move submodule
>
> (master|REBASE 3/3)$ git st
> interactive rebase in progress; onto c21ef8e
> Last commands done (3 commands done):
>    break
>    pick 08e230f move submodule
>   (see more in file .git/rebase-merge/done)
> No commands remaining.
> You are currently rebasing branch 'master' on 'c21ef8e'.
>   (fix conflicts and then run "git rebase --continue")
>   (use "git rebase --skip" to skip this patch)
>   (use "git rebase --abort" to check out the original branch)
>
> Changes to be committed:
>   (use "git restore --staged <file>..." to unstage)
>         modified:   .gitmodules
>         deleted:    gitlab-oe
>
> Unmerged paths:
>   (use "git restore --staged <file>..." to unstage)
>   (use "git add <file>..." to mark resolution)
>         added by them:   subdir/gitlab-oe
>
> (master|REBASE 3/3)$

I couldn't figure out how to duplicate.  Maybe I did something wrong,
but it was:
    # download your fast-export stream
    git init temp
    cd temp
    cat ~/Downloads/submodule-move.fexp | git fast-import --quiet
    git checkout master
    git rebase -i HEAD^^
    # Insert a line with just 'b' between the two pick lines; save and
exit and when it breaks:
    echo >>README
    git commit -a -m noise
    git rebase --continue

After the rebase --continue, the rebase completes just fine applying
the patch with the submodule move.  git range-diff master@{1}... will
show that I inserted a new commit in the middle.  git log --raw looks
good, showing all four commits including the moved submodule at the
end.


What git version did you use?  Do you need special settings (what's in
your ~/.gitconfig and your .git/config)?

Re: [BUG] submodule move badly handled by git-rebase

[ydirson]

Hi Elijah,

> Hi Yann,
> 
> On Tue, Apr 7, 2020 at 9:36 AM <ydirson@free.fr> wrote:
> >
> > Hello all,
> >
> > When rebasing commits involving move of a submodule, git-rebase
> > fails to
> > record in index the "add" part of the rename.  This leaves the
> > workdir
> > dirty and the rebase gets stopped.
> >
> > fast-export of a testcase is attached.  To reproduce, just
> > "git rebase -i", add a "break" before the move commit,
> > use this to introduce some noise, and watch.
> >
> > Best regards,
> > --
> > Yann
> >
> >
> > (master)$ git rebase -i HEAD^^
> > hint: Waiting for your editor to close the file... Waiting for
> > Emacs...
> > Stopped at b0e1b00... add submodule
> >
> > (master|REBASE 2/3)$ echo >>README
> >
> > (master|REBASE 2/3)$ git commit -a -m noise
> > [detached HEAD d67c886] noise
> >  1 file changed, 1 insertion(+)
> >
> > (master|REBASE 2/3)$ git rebase --continue
> > Adding as subdir/gitlab-oe~08e230f... move submodule instead
> > error: could not apply 08e230f... move submodule
> > Resolve all conflicts manually, mark them as resolved with
> > "git add/rm <conflicted_files>", then run "git rebase --continue".
> > You can instead skip this commit: run "git rebase --skip".
> > To abort and get back to the state before "git rebase", run "git
> > rebase --abort".
> > Could not apply 08e230f... move submodule
> >
> > (master|REBASE 3/3)$ git st
> > interactive rebase in progress; onto c21ef8e
> > Last commands done (3 commands done):
> >    break
> >    pick 08e230f move submodule
> >   (see more in file .git/rebase-merge/done)
> > No commands remaining.
> > You are currently rebasing branch 'master' on 'c21ef8e'.
> >   (fix conflicts and then run "git rebase --continue")
> >   (use "git rebase --skip" to skip this patch)
> >   (use "git rebase --abort" to check out the original branch)
> >
> > Changes to be committed:
> >   (use "git restore --staged <file>..." to unstage)
> >         modified:   .gitmodules
> >         deleted:    gitlab-oe
> >
> > Unmerged paths:
> >   (use "git restore --staged <file>..." to unstage)
> >   (use "git add <file>..." to mark resolution)
> >         added by them:   subdir/gitlab-oe
> >
> > (master|REBASE 3/3)$
> 
> I couldn't figure out how to duplicate.  Maybe I did something wrong,
> but it was:
>     # download your fast-export stream
>     git init temp
>     cd temp
>     cat ~/Downloads/submodule-move.fexp | git fast-import --quiet
>     git checkout master
>     git rebase -i HEAD^^
>     # Insert a line with just 'b' between the two pick lines; save
>     and
> exit and when it breaks:
>     echo >>README
>     git commit -a -m noise
>     git rebase --continue
> 
> After the rebase --continue, the rebase completes just fine applying
> the patch with the submodule move.  git range-diff master@{1}... will
> show that I inserted a new commit in the middle.  git log --raw looks
> good, showing all four commits including the moved submodule at the
> end.

You're right, I missed crucial point: no problem appears unless the submodule
is initialized.  After checking out the master branch (and possibly issuing
"git reset --hard" to make sure everything is clean), it is necessary to
run "git submodule update --init".


> What git version did you use?  Do you need special settings (what's
> in
> your ~/.gitconfig and your .git/config)?

This is 2.26.0, but like my other report I had it with 2.25.1 already.
Just tested with an clear config (HOME and GIT_CONFIG_NOSYSTEM set) and
the problem triggers as well.

Best regards,
-- 
Yann

Re: [BUG] submodule move badly handled by git-rebase

[Elijah Newren]

Hi Yann,

On Wed, Apr 8, 2020 at 12:52 AM <ydirson@free.fr> wrote:
>
> Hi Elijah,
>
> > Hi Yann,
> >
> > On Tue, Apr 7, 2020 at 9:36 AM <ydirson@free.fr> wrote:
> > >
> > > Hello all,
> > >
> > > When rebasing commits involving move of a submodule, git-rebase
> > > fails to
> > > record in index the "add" part of the rename.  This leaves the
> > > workdir
> > > dirty and the rebase gets stopped.
> > >
> > > fast-export of a testcase is attached.  To reproduce, just
> > > "git rebase -i", add a "break" before the move commit,
> > > use this to introduce some noise, and watch.
> > >
> > > Best regards,
> > > --
> > > Yann
> > >
> > >
> > > (master)$ git rebase -i HEAD^^
> > > hint: Waiting for your editor to close the file... Waiting for
> > > Emacs...
> > > Stopped at b0e1b00... add submodule
> > >
> > > (master|REBASE 2/3)$ echo >>README
> > >
> > > (master|REBASE 2/3)$ git commit -a -m noise
> > > [detached HEAD d67c886] noise
> > >  1 file changed, 1 insertion(+)
> > >
> > > (master|REBASE 2/3)$ git rebase --continue
> > > Adding as subdir/gitlab-oe~08e230f... move submodule instead
> > > error: could not apply 08e230f... move submodule
> > > Resolve all conflicts manually, mark them as resolved with
> > > "git add/rm <conflicted_files>", then run "git rebase --continue".
> > > You can instead skip this commit: run "git rebase --skip".
> > > To abort and get back to the state before "git rebase", run "git
> > > rebase --abort".
> > > Could not apply 08e230f... move submodule
> > >
> > > (master|REBASE 3/3)$ git st
> > > interactive rebase in progress; onto c21ef8e
> > > Last commands done (3 commands done):
> > >    break
> > >    pick 08e230f move submodule
> > >   (see more in file .git/rebase-merge/done)
> > > No commands remaining.
> > > You are currently rebasing branch 'master' on 'c21ef8e'.
> > >   (fix conflicts and then run "git rebase --continue")
> > >   (use "git rebase --skip" to skip this patch)
> > >   (use "git rebase --abort" to check out the original branch)
> > >
> > > Changes to be committed:
> > >   (use "git restore --staged <file>..." to unstage)
> > >         modified:   .gitmodules
> > >         deleted:    gitlab-oe
> > >
> > > Unmerged paths:
> > >   (use "git restore --staged <file>..." to unstage)
> > >   (use "git add <file>..." to mark resolution)
> > >         added by them:   subdir/gitlab-oe
> > >
> > > (master|REBASE 3/3)$
> >
> > I couldn't figure out how to duplicate.  Maybe I did something wrong,
> > but it was:
> >     # download your fast-export stream
> >     git init temp
> >     cd temp
> >     cat ~/Downloads/submodule-move.fexp | git fast-import --quiet
> >     git checkout master
> >     git rebase -i HEAD^^
> >     # Insert a line with just 'b' between the two pick lines; save
> >     and
> > exit and when it breaks:
> >     echo >>README
> >     git commit -a -m noise
> >     git rebase --continue
> >
> > After the rebase --continue, the rebase completes just fine applying
> > the patch with the submodule move.  git range-diff master@{1}... will
> > show that I inserted a new commit in the middle.  git log --raw looks
> > good, showing all four commits including the moved submodule at the
> > end.
>
> You're right, I missed crucial point: no problem appears unless the submodule
> is initialized.  After checking out the master branch (and possibly issuing
> "git reset --hard" to make sure everything is clean), it is necessary to
> run "git submodule update --init".
>
>
> > What git version did you use?  Do you need special settings (what's
> > in
> > your ~/.gitconfig and your .git/config)?
>
> This is 2.26.0, but like my other report I had it with 2.25.1 already.
> Just tested with an clear config (HOME and GIT_CONFIG_NOSYSTEM set) and
> the problem triggers as well.

Ah, gotcha.  With the "git submodule update --init", I can duplicate
with 2.26.0 and 2.24.1 and I probably could with any other git version
going back to when submodules were introduced.

I decided to also test with the "ort" merge strategy[1] that I haven't
worked on in over a month.  It fixes this testcase.  So this is yet
another example of it having better correctness than merge-recursive.
Thanks for the report.

Elijah

[1] https://github.com/newren/git/tree/git-merge-2020-demo

Re: [BUG] submodule move badly handled by git-rebase

[ydirson]

This may be related to another funky behaviour I just noticed, linked
to moving submodules around:

- when initially created, the $TOP/orig-name submodule's git-dir gets created in
  $TOP/.git/modules/orig-name, with $TOP/.git/modules/orig-name/config
  containing a core.worktree value pointing to $TOP/orig-name
- when moving the submodule, only the submodule worktree is moved, the git-dir
  being the same $TOP/.git/modules/orig-name, where the core.worktree still
  points to the old location

Other unwanted behaviour include "git clean" reporting (and possibly cleaning)
files from the wrong work tree - it took me head-scratching to understand why
"git clean -fdx" was ignoring all the cruft I had in this worktree...

Why is it that we need a core.worktree setting at all in there ?  Removing it
allows "git clean" to do what's expected of it.  OTOH it does not make the
original problem go away...

----- Mail original -----
> De: ydirson@free.fr
> À: "Elijah Newren" <newren@gmail.com>
> Cc: "git" <git@vger.kernel.org>
> Envoyé: Mercredi 8 Avril 2020 09:52:59
> Objet: Re: [BUG] submodule move badly handled by git-rebase
> 
> Hi Elijah,
> 
> > Hi Yann,
> > 
> > On Tue, Apr 7, 2020 at 9:36 AM <ydirson@free.fr> wrote:
> > >
> > > Hello all,
> > >
> > > When rebasing commits involving move of a submodule, git-rebase
> > > fails to
> > > record in index the "add" part of the rename.  This leaves the
> > > workdir
> > > dirty and the rebase gets stopped.
> > >
> > > fast-export of a testcase is attached.  To reproduce, just
> > > "git rebase -i", add a "break" before the move commit,
> > > use this to introduce some noise, and watch.
> > >
> > > Best regards,
> > > --
> > > Yann
> > >
> > >
> > > (master)$ git rebase -i HEAD^^
> > > hint: Waiting for your editor to close the file... Waiting for
> > > Emacs...
> > > Stopped at b0e1b00... add submodule
> > >
> > > (master|REBASE 2/3)$ echo >>README
> > >
> > > (master|REBASE 2/3)$ git commit -a -m noise
> > > [detached HEAD d67c886] noise
> > >  1 file changed, 1 insertion(+)
> > >
> > > (master|REBASE 2/3)$ git rebase --continue
> > > Adding as subdir/gitlab-oe~08e230f... move submodule instead
> > > error: could not apply 08e230f... move submodule
> > > Resolve all conflicts manually, mark them as resolved with
> > > "git add/rm <conflicted_files>", then run "git rebase
> > > --continue".
> > > You can instead skip this commit: run "git rebase --skip".
> > > To abort and get back to the state before "git rebase", run "git
> > > rebase --abort".
> > > Could not apply 08e230f... move submodule
> > >
> > > (master|REBASE 3/3)$ git st
> > > interactive rebase in progress; onto c21ef8e
> > > Last commands done (3 commands done):
> > >    break
> > >    pick 08e230f move submodule
> > >   (see more in file .git/rebase-merge/done)
> > > No commands remaining.
> > > You are currently rebasing branch 'master' on 'c21ef8e'.
> > >   (fix conflicts and then run "git rebase --continue")
> > >   (use "git rebase --skip" to skip this patch)
> > >   (use "git rebase --abort" to check out the original branch)
> > >
> > > Changes to be committed:
> > >   (use "git restore --staged <file>..." to unstage)
> > >         modified:   .gitmodules
> > >         deleted:    gitlab-oe
> > >
> > > Unmerged paths:
> > >   (use "git restore --staged <file>..." to unstage)
> > >   (use "git add <file>..." to mark resolution)
> > >         added by them:   subdir/gitlab-oe
> > >
> > > (master|REBASE 3/3)$
> > 
> > I couldn't figure out how to duplicate.  Maybe I did something
> > wrong,
> > but it was:
> >     # download your fast-export stream
> >     git init temp
> >     cd temp
> >     cat ~/Downloads/submodule-move.fexp | git fast-import --quiet
> >     git checkout master
> >     git rebase -i HEAD^^
> >     # Insert a line with just 'b' between the two pick lines; save
> >     and
> > exit and when it breaks:
> >     echo >>README
> >     git commit -a -m noise
> >     git rebase --continue
> > 
> > After the rebase --continue, the rebase completes just fine
> > applying
> > the patch with the submodule move.  git range-diff master@{1}...
> > will
> > show that I inserted a new commit in the middle.  git log --raw
> > looks
> > good, showing all four commits including the moved submodule at the
> > end.
> 
> You're right, I missed crucial point: no problem appears unless the
> submodule
> is initialized.  After checking out the master branch (and possibly
> issuing
> "git reset --hard" to make sure everything is clean), it is necessary
> to
> run "git submodule update --init".
> 
> 
> > What git version did you use?  Do you need special settings (what's
> > in
> > your ~/.gitconfig and your .git/config)?
> 
> This is 2.26.0, but like my other report I had it with 2.25.1
> already.
> Just tested with an clear config (HOME and GIT_CONFIG_NOSYSTEM set)
> and
> the problem triggers as well.
> 
> Best regards,
> --
> Yann
> 
> 

Re: [BUG] submodule move badly handled by git-rebase

[Elijah Newren]

On Wed, Apr 8, 2020 at 9:23 AM <ydirson@free.fr> wrote:
>
> This may be related to another funky behaviour I just noticed, linked
> to moving submodules around:
>
> - when initially created, the $TOP/orig-name submodule's git-dir gets created in
>   $TOP/.git/modules/orig-name, with $TOP/.git/modules/orig-name/config
>   containing a core.worktree value pointing to $TOP/orig-name
> - when moving the submodule, only the submodule worktree is moved, the git-dir
>   being the same $TOP/.git/modules/orig-name, where the core.worktree still
>   points to the old location
>
> Other unwanted behaviour include "git clean" reporting (and possibly cleaning)
> files from the wrong work tree - it took me head-scratching to understand why
> "git clean -fdx" was ignoring all the cruft I had in this worktree...
>
> Why is it that we need a core.worktree setting at all in there ?  Removing it
> allows "git clean" to do what's expected of it.  OTOH it does not make the
> original problem go away...

Not knowing much about submodules, I'm going to leave submodule issues
that don't touch on the merge-machinery or rebase code to someone else
to handle. (I'd probably do the same with the merge-machinery and
rebase side if I wasn't worried about 2.26.0 regressions in rebase and
if I hadn't find a clever way to re-use checkout code to avoid lots of
submodule issues while also deleting code in the "ort" merge
strategy).

Hopefully someone else on the list who knows more about submodules can
chime in on the worktree related bits.