Re: Bug report: git rebase ignores different context, resulting in subtle breakage

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Michel

Sorry for the slow response, this fell through the Christmas cracks.

On 14/12/2020 14:37, Michel Dänzer wrote:
> 
> (Originally reported as a GitLab issue: 
> https://gitlab.com/gitlab-org/gitlab/-/issues/292754)
> 
> 
> All output below is from Debian git 2.29.2-1.
> 
> 
> The following branches of 
> https://gitlab.freedesktop.org/daenzer/mesa.git can be used to reproduce:
> 
> gitlab-issue-292754/pre-rebase (pre-rebase state)
> gitlab-issue-292754/base (new base)
> gitlab-issue-292754/bad-rebase (bad post-rebase state)
> 
> 
> The pre-rebase commit b9f18d0ddb6b075819bc2c6b9fa36dee483ef443 contains 
> this (truncated) hunk:
> 
> @@ -480,13 +491,9 @@ sanity:
> 
>     rules:
> 
>       - if: *is-pre-merge
> 
>         when: on_success
> 
> -    - if: *is-forked-branch
> 
> -      when: manual
> 
>       # Other cases default to never
> 
>     script:
> 
>       # ci-fairy check-commits --junit-xml=check-commits.xml
> 
> 
> 
> The new base commit f20153536087079f39f1ab9995ac3d36dd3c467f contains 
> this hunk:
> 
> @@ -484,10 +484,8 @@ sanity:
> 
>       - .fdo.ci-fairy
> 
>     stage: sanity
> 
>     rules:
> 
> -    - if: *is-pre-merge
> 
> +    - if: *is-forked-branch-or-pre-merge
> 
>         when: on_success
> 
> -    - if: *is-forked-branch
> 
> -      when: manual
> 
>       # Other cases default to never
> 
>     variables:
> 
>       GIT_STRATEGY: none
> 
> 
> 
> Both remove the same 2 lines, but the context is different both before 
> and after those lines.
> 
> My expectation for this case would be that
> 
>   git rebase gitlab-issue-292754/base gitlab-issue-292754/pre-rebase
> 
> fails with a conflict. However, it succeeds without any indication of 
> trouble, resulting in these contents in commit 
> 4e549e1ac3354f465d8afe0174902e62143a6ff4:
> 
>     rules:
> 
> 
>      - if: *is-forked-branch-or-pre-merge
> 
> 
>         when: on_success
> 
> 
>       # Other cases default to never
> 
> 
>     variables:
> 
> 
>       GIT_STRATEGY: none
> 
>     script:
> 
> 
>       # ci-fairy check-commits --junit-xml=check-commits.xml
> 
> 
> I.e. identical to the new base.

This looks like the correct merge to me as the changes in the original 
commit are already in the new base. Rebase has detected that the context 
lines do not match and used a 3-way merge instead of a simple patch 
application. This matches the behavior of the merge based rebase backend 
which is the default in recent versions of git. Git tracks content not 
changes and so rebasing a branch means cherry-picking each commit onto 
the rebased version of it's parent, not applying each patch on top of 
the new base.

Best Wishes

Phillip


> However, the 2 removed lines had a different effect in the original 
> pre-rebase context, and the post-rebase state no longer matches the 
> original intention. git rebase silently introduced a subtle bug.
> 
> 
> git rebase --apply results in this output:
> 
>   First, rewinding head to replay your work on top of it...
> 
>   Applying: ci: Run sanity job only in pre-merge pipelines
> 
>   Using index info to reconstruct a base tree...
> 
>   M    .gitlab-ci.yml
> 
>   Falling back to patching base and 3-way merge...
> 
>   Auto-merging .gitlab-ci.yml
> 
> 
> Looks like the applying strategy does detect the conflict, but there's 
> an automatic fallback to the merging strategy, which again succeeds 
> (with the same broken result).
> 
> 
> The only way I've found to avoid the broken behaviour is git rebase -s 
> octopus [...]:
> 
>   error: could not apply b9f18d0ddb6... ci: Run sanity job only in pre
>   merge pipelines
> 
>   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 b9f18d0ddb6... ci: Run sanity job only in pre-merge
>   pipelines
> 
>