From: Michel Dänzer Date: Wed, 06 Jan 2021 16:04:05 GMT Subject: Re: Bug report: git rebase ignores different context, resulting in subtle breakage Message-ID: <8b4da09f-cc0f-8ea4-1b65-95a7fa9858d0@daenzer.net> In-Reply-To: <502b710e-6347-5fa7-0461-21a84ae2250d@gmail.com> On 2021-01-06 4:40 p.m., Phillip Wood wrote: > 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. Are you referring to the same two lines which have been removed already? Since the context is different, removing those two lines has a different effect than intended. > 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. There is a single commit that needs rebasing, and the context in the new base commit is different from what it's based on. To me this seems like an obvious conflict. -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and X developer