* WARNING: Git merges are tricky. Rebasing is better?
@ 2022-01-17 21:46 Leo Famulari
2022-01-17 22:26 ` Kyle Meyer
0 siblings, 1 reply; 5+ messages in thread
From: Leo Famulari @ 2022-01-17 21:46 UTC (permalink / raw)
To: guix-devel
Take note that Git merges can be tricky and hide mistakes! Please
consider using a rebasing workflow instead of a merging workflow for
your branches.
For example, today we merged the version-1.4.0 branch into the master
branch, with commit 276f40fdc349d2ad62582b23ea55e061b689cfc0.
After the merge, we got a report on the #guix IRC channel [0] that the
Epiphany package's source hash was incorrect. It was using the hash of
the previously packaged version of Epiphany.
Using Git, we can see that in commit 291c4fa0ba, Epiphany was updated to
version 41.2, with a correct change in hash:
https://git.savannah.gnu.org/cgit/guix.git/commit/?id=291c4fa0baf24b9d600872fce99441f5471cdb40
Later, the version-1.4.0 branch was merged into master.
View the Git log from the commit where Epiphany was updated through the
merge:
$ git log --patch 291c4fa0baf^..276f40fdc34 gnu/packages/gnome.scm
It does not show that the update of Epiphany's source hash was reverted.
And to be sure that something is wrong, we can examine the file based on
the merge commit:
------
$ git show 276f40fdc349d2ad62582b23ea55e061b689cfc0:gnu/packages/gnome.scm | grep "define-public epiphany" -A11
(define-public epiphany
(package
(name "epiphany")
(version "41.2")
(source (origin
(method url-fetch)
(uri (string-append "mirror://gnome/sources/epiphany/"
(version-major version) "/"
"epiphany-" version ".tar.xz"))
(sha256
(base32
"0r7m34xzz3shdfxf2abxb069izak3yv3ijlg29qy4pfmyawkilfs"))
------ ^
*This is the hash of Epiphany 40.3, the old version!*
Git's automatic merge conflict resolution algorithm did the wrong thing
here. And Git does not show the reversion in `git log`, hiding it from
review.
My guess is that commit f7afefba0 ("gnu: epiphany: Fix build with
libportal-0.5."), which happened on the master branch while the
version-1.4.0 branch was forked off, made Git think that this line was
more "up to date" on master than on version-1.4.0, causing it to select
the old hash when faced with the conflict.
Once again, consider using a workflow based on rebasing instead of
merging for forked branches that will live for weeks to a month or two.
This type of mistake cannot be obscured when rebasing.
[0] http://logs.guix.gnu.org/guix/2022-01-17.log#195313
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: WARNING: Git merges are tricky. Rebasing is better?
2022-01-17 21:46 WARNING: Git merges are tricky. Rebasing is better? Leo Famulari
@ 2022-01-17 22:26 ` Kyle Meyer
2022-01-17 23:00 ` Leo Famulari
0 siblings, 1 reply; 5+ messages in thread
From: Kyle Meyer @ 2022-01-17 22:26 UTC (permalink / raw)
To: Leo Famulari; +Cc: guix-devel
Leo Famulari writes:
> ------
> $ git show 276f40fdc349d2ad62582b23ea55e061b689cfc0:gnu/packages/gnome.scm | grep "define-public epiphany" -A11
> (define-public epiphany
> (package
> (name "epiphany")
> (version "41.2")
> (source (origin
> (method url-fetch)
> (uri (string-append "mirror://gnome/sources/epiphany/"
> (version-major version) "/"
> "epiphany-" version ".tar.xz"))
> (sha256
> (base32
> "0r7m34xzz3shdfxf2abxb069izak3yv3ijlg29qy4pfmyawkilfs"))
> ------ ^
> *This is the hash of Epiphany 40.3, the old version!*
>
> Git's automatic merge conflict resolution algorithm did the wrong thing
> here. And Git does not show the reversion in `git log`, hiding it from
> review.
>
> My guess is that commit f7afefba0 ("gnu: epiphany: Fix build with
> libportal-0.5."), which happened on the master branch while the
> version-1.4.0 branch was forked off, made Git think that this line was
> more "up to date" on master than on version-1.4.0, causing it to select
> the old hash when faced with the conflict.
Fwiw I don't think Git automatically resolved that conflict:
$ git checkout 276f40fdc349d2ad62582b23ea55e061b689cfc0^
$ git merge 276f40fdc349d2ad62582b23ea55e061b689cfc0^2
$ git status -- gnu/packages/gnome.scm
HEAD detached at b2f6b6f6b9
You have unmerged paths.
Unmerged paths:
both modified: gnu/packages/gnome.scm
no changes added to commit
$ git diff -- gnu/packages/gnome.scm
diff --cc gnu/packages/gnome.scm
index d8d34c89ed,6c63b8bc59..0000000000
--- a/gnu/packages/gnome.scm
+++ b/gnu/packages/gnome.scm
@@@ -6433,13 -6415,10 +6421,12 @@@ supports playlists, song ratings, and a
name "-" version ".tar.xz"))
(sha256
(base32
- "0ddjwcd77nw0rxb5x5bz5hd671m8gya9827p8rsnb58x103kpai8"))))
+ "0ddjwcd77nw0rxb5x5bz5hd671m8gya9827p8rsnb58x103kpai8"))
+ ;; XXX: Remove when upgrading to 42.0
+ (patches (search-patches "eog-update-libportal-usage.patch"))))
(build-system meson-build-system)
(arguments
- `(#:meson ,meson-0.59 ;positional arguments error with meson 0.60
- #:configure-flags
+ `(#:configure-flags
;; Otherwise, the RUNPATH will lack the final 'eog' path component.
(list (string-append "-Dc_link_args=-Wl,-rpath="
(assoc-ref %outputs "out") "/lib/eog"))
@@@ -6798,9 -6776,8 +6784,17 @@@ a secret password store, an adblocker,
"epiphany-" version ".tar.xz"))
(sha256
(base32
++<<<<<<< HEAD
+ "0r7m34xzz3shdfxf2abxb069izak3yv3ijlg29qy4pfmyawkilfs"))
+ (patches
+ (search-patches "epiphany-update-libportal-usage.patch"))))
++||||||| d91de53caa
++ "0r7m34xzz3shdfxf2abxb069izak3yv3ijlg29qy4pfmyawkilfs"))))
++
++=======
+ "0k7b22zq3z1kllzqxgwsvwb1lp0j6rjb3k1hvhna3i573wc4mpji"))))
+
++>>>>>>> 276f40fdc349d2ad62582b23ea55e061b689cfc0^2
(build-system meson-build-system)
(arguments
`(#:glib-or-gtk? #t
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: WARNING: Git merges are tricky. Rebasing is better?
2022-01-17 22:26 ` Kyle Meyer
@ 2022-01-17 23:00 ` Leo Famulari
2022-01-17 23:19 ` Kyle Meyer
0 siblings, 1 reply; 5+ messages in thread
From: Leo Famulari @ 2022-01-17 23:00 UTC (permalink / raw)
To: Kyle Meyer; +Cc: guix-devel
On Mon, Jan 17, 2022 at 05:26:08PM -0500, Kyle Meyer wrote:
> Fwiw I don't think Git automatically resolved that conflict:
>
> $ git checkout 276f40fdc349d2ad62582b23ea55e061b689cfc0^
> $ git merge 276f40fdc349d2ad62582b23ea55e061b689cfc0^2
[...]
> ++<<<<<<< HEAD
> + "0r7m34xzz3shdfxf2abxb069izak3yv3ijlg29qy4pfmyawkilfs"))
> + (patches
> + (search-patches "epiphany-update-libportal-usage.patch"))))
> ++||||||| d91de53caa
> ++ "0r7m34xzz3shdfxf2abxb069izak3yv3ijlg29qy4pfmyawkilfs"))))
> ++
> ++=======
> + "0k7b22zq3z1kllzqxgwsvwb1lp0j6rjb3k1hvhna3i573wc4mpji"))))
> +
> ++>>>>>>> 276f40fdc349d2ad62582b23ea55e061b689cfc0^2
So, you think it was a mistake made when resolving conflicts by hand? I
can certainly understand that.
Either way, it's bad that the Git history doesn't show these types of
changes.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: WARNING: Git merges are tricky. Rebasing is better?
2022-01-17 23:00 ` Leo Famulari
@ 2022-01-17 23:19 ` Kyle Meyer
2022-02-02 14:19 ` Maxim Cournoyer
0 siblings, 1 reply; 5+ messages in thread
From: Kyle Meyer @ 2022-01-17 23:19 UTC (permalink / raw)
To: Leo Famulari; +Cc: guix-devel
Leo Famulari writes:
> On Mon, Jan 17, 2022 at 05:26:08PM -0500, Kyle Meyer wrote:
>> Fwiw I don't think Git automatically resolved that conflict:
>>
>> $ git checkout 276f40fdc349d2ad62582b23ea55e061b689cfc0^
>> $ git merge 276f40fdc349d2ad62582b23ea55e061b689cfc0^2
> [...]
>> ++<<<<<<< HEAD
>> + "0r7m34xzz3shdfxf2abxb069izak3yv3ijlg29qy4pfmyawkilfs"))
>> + (patches
>> + (search-patches "epiphany-update-libportal-usage.patch"))))
>> ++||||||| d91de53caa
>> ++ "0r7m34xzz3shdfxf2abxb069izak3yv3ijlg29qy4pfmyawkilfs"))))
>> ++
>> ++=======
>> + "0k7b22zq3z1kllzqxgwsvwb1lp0j6rjb3k1hvhna3i573wc4mpji"))))
>> +
>> ++>>>>>>> 276f40fdc349d2ad62582b23ea55e061b689cfc0^2
>
> So, you think it was a mistake made when resolving conflicts by hand? I
> can certainly understand that.
Yes.
> Either way, it's bad that the Git history doesn't show these types of
> changes.
Right, inspecting merge results can be confusing. By default that
change is pruned from diffs as "uninteresting" because the merge result
matched the content in one of the parents (discussed in the "COMBINED
DIFF FORMAT" section of git-diff's manpage and the --diff-merges
description of git-show's manpage).
$ git show 276f40fd | grep 0r7m34xzz
$ git show --diff-merges=combined 276f40fd | grep 0r7m34xzz
+ "0r7m34xzz3shdfxf2abxb069izak3yv3ijlg29qy4pfmyawkilfs"))
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: WARNING: Git merges are tricky. Rebasing is better?
2022-01-17 23:19 ` Kyle Meyer
@ 2022-02-02 14:19 ` Maxim Cournoyer
0 siblings, 0 replies; 5+ messages in thread
From: Maxim Cournoyer @ 2022-02-02 14:19 UTC (permalink / raw)
To: Kyle Meyer; +Cc: guix-devel
Hello Kyle and Leo,
Kyle Meyer <kyle@kyleam.com> writes:
> Leo Famulari writes:
>
>> On Mon, Jan 17, 2022 at 05:26:08PM -0500, Kyle Meyer wrote:
>>> Fwiw I don't think Git automatically resolved that conflict:
>>>
>>> $ git checkout 276f40fdc349d2ad62582b23ea55e061b689cfc0^
>>> $ git merge 276f40fdc349d2ad62582b23ea55e061b689cfc0^2
>> [...]
>>> ++<<<<<<< HEAD
>>> + "0r7m34xzz3shdfxf2abxb069izak3yv3ijlg29qy4pfmyawkilfs"))
>>> + (patches
>>> + (search-patches "epiphany-update-libportal-usage.patch"))))
>>> ++||||||| d91de53caa
>>> ++ "0r7m34xzz3shdfxf2abxb069izak3yv3ijlg29qy4pfmyawkilfs"))))
>>> ++
>>> ++=======
>>> + "0k7b22zq3z1kllzqxgwsvwb1lp0j6rjb3k1hvhna3i573wc4mpji"))))
>>> +
>>> ++>>>>>>> 276f40fdc349d2ad62582b23ea55e061b689cfc0^2
>>
>> So, you think it was a mistake made when resolving conflicts by hand? I
>> can certainly understand that.
>
> Yes.
>
>> Either way, it's bad that the Git history doesn't show these types of
>> changes.
>
> Right, inspecting merge results can be confusing. By default that
> change is pruned from diffs as "uninteresting" because the merge result
> matched the content in one of the parents (discussed in the "COMBINED
> DIFF FORMAT" section of git-diff's manpage and the --diff-merges
> description of git-show's manpage).
>
> $ git show 276f40fd | grep 0r7m34xzz
> $ git show --diff-merges=combined 276f40fd | grep 0r7m34xzz
> + "0r7m34xzz3shdfxf2abxb069izak3yv3ijlg29qy4pfmyawkilfs"))
Thanks for this interesting discussion, and apologies for mis-resolving
the conflict :-)
Maxim
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-02-02 15:42 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-17 21:46 WARNING: Git merges are tricky. Rebasing is better? Leo Famulari
2022-01-17 22:26 ` Kyle Meyer
2022-01-17 23:00 ` Leo Famulari
2022-01-17 23:19 ` Kyle Meyer
2022-02-02 14:19 ` Maxim Cournoyer
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.