Bizarre git merge behaviour

Bizarre git merge behaviour

[Matthew Cengia]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

The other day I was merging a feature branch (shown below as
origin/22869-new-kernel) into my staging release (shown below as 'wtf'),
and it *reverted* a bucket-load of changes I'd made on other branches
which had been merged into staging before. I can't for the life of me
work out why this happened, and would appreciate any ideas. Output below
shows symptoms:


  mattcen@sonar:prisonpc(wtf)$ git log --numstat --oneline origin/22869-new-kernel ^wtf | cat
  a8879c2 Oops, name .config consistently with deb.
  2797    0       client/kernel/config-3.5.7.20-1
  0       2797    client/kernel/config-3.5.7.21-1
  4c34686 Further lock down the inmate kernel.
  366     1094    client/kernel/config-3.5.7.21-1
  -       -       client/kernel/linux-firmware-image_3.5.7.21-1_i386.deb
  -       -       client/kernel/linux-image-3.5.7.20_3.5.7.20-1_i386.deb
  -       -       client/kernel/linux-image-3.5.7.21_3.5.7.21-1_i386.deb
  6       7       client/scm/50-kernel-prisoner.scm
  4a8acd0 Add build notes about how we generated these kernels.
  37      0       client/kernel/README
  e96adfe Merge remote-tracking branch 'origin/NNNNN-multicast-kernel' into 22869-new-kernel
  4b8afeb Merge commit '8ef01f2' into 22869-new-kernel
  fc4a5e6 Allow staff machines to receive IPTV stuff (per russm).
  1       0       client/scm/50-staff.scm
  117e646 Merge remote branch 'origin/staging' into NNNNN-multicast-kernel
  deb3b38 allow IGMP reports out through iptables
  1       0       client/scm/20-security.scm
  8ef01f2 Enable multicast in inmate kernel.
  0       3034    client/kernel/config-2.6.32.46+drm33.20
  3036    0       client/kernel/config-2.6.32.61+drm33.26
  -       -       client/kernel/linux-firmware-image_2.6.32.46+drm33.20-1_all.deb
  -       -       client/kernel/linux-firmware-image_2.6.32.61+drm33.26-1_all.deb
  -       -       client/kernel/linux-image-2.6.32.46+drm33.20_2.6.32.46+drm33.20-1_i386.deb
  -       -       client/kernel/linux-image-2.6.32.61+drm33.26_2.6.32.61+drm33.26-1_i386.deb
  11      11      client/scm/50-prisoner.scm
  mattcen@sonar:prisonpc(wtf)$ git merge origin/22869-new-kernel
  Auto-merging tca/parts.d/pxelinux-cfg
  Auto-merging pre-install.sh
  Removing ppcadm/templates/wwwfilter.tpl
  Removing ppcadm/templates/streaming_media.tpl
  Removing ppcadm/templates/disc_summary.tpl
  Removing ppcadm/templates/disc_access.tpl
  Auto-merging ppcadm/modules/quarantine.py
  CONFLICT (content): Merge conflict in ppcadm/modules/quarantine.py
  Auto-merging ppcadm/modules/media.py
  CONFLICT (content): Merge conflict in ppcadm/modules/media.py
  CONFLICT (modify/delete): ppcadm/modules/emailstats.py deleted in origin/22869-new-kernel and modified in HEAD. Version HEAD of ppcadm/modules/emailstats.py left in tree.
  Removing generate-mail-stats
  Removing doc/adminguide/images/streaming_media-screen.png
  Auto-merging debian/rules
  Auto-merging debian/prisonpc-internet.install
  CONFLICT (content): Merge conflict in debian/prisonpc-internet.install
  Auto-merging debian/prisonpc-core.cron.d
  Auto-merging debian/NEWS
  CONFLICT (content): Merge conflict in debian/NEWS
  Auto-merging client/scm/50-kernel-prisoner.scm
  CONFLICT (content): Merge conflict in client/scm/50-kernel-prisoner.scm
  Removing client/kernel/linux-image-3.5.7.21_3.5.7.21-1_i386.deb
  Removing client/kernel/linux-firmware-image_3.5.7.21-1_i386.deb
  Auto-merging client/kernel/config-3.5.7.20-1
  Resolved 'client/scm/50-kernel-prisoner.scm' using previous resolution.
  Resolved 'debian/NEWS' using previous resolution.
  Resolved 'debian/prisonpc-internet.install' using previous resolution.
  Resolved 'ppcadm/modules/media.py' using previous resolution.
  Resolved 'ppcadm/modules/quarantine.py' using previous resolution.
  Automatic merge failed; fix conflicts and then commit the result.
  mattcen@sonar:prisonpc(wtf)$ git status --porcelain
  M  client/kernel/README
  R  client/kernel/config-3.5.7.21-1 -> client/kernel/config-3.5.7.20-1
  D  client/kernel/linux-firmware-image_3.5.7.21-1_i386.deb
  A  client/kernel/linux-image-3.5.7.20_3.5.7.20-1_i386.deb
  D  client/kernel/linux-image-3.5.7.21_3.5.7.21-1_i386.deb
  M  client/scm/20-security.scm
  M  client/scm/30-apps.scm
  UU client/scm/50-kernel-prisoner.scm
  M  client/scm/50-staff.scm
  UU debian/NEWS
  M  debian/changelog
  M  debian/prisonpc-core.cron.d
  UU debian/prisonpc-internet.install
  M  debian/rules
  A  doc/adminguide/images/streaming_media-screen.jpg
  D  doc/adminguide/images/streaming_media-screen.png
  M  doc/adminguide/images/web_filtering.png
  M  doc/adminguide/streaming_media.rst
  M  doc/adminguide/system-status.rst
  M  doc/adminguide/web_filtering.rst
  M  dovecot/dovecot-staff.conf
  M  eric/apps/complain.py
  M  eric/apps/links.py
  M  eric/apps/media.py
  M  eric/apps/whitelist.py
  M  eric/eric
  M  eric/eric_cfg.py
  D  generate-mail-stats
  M  ppcadm/modules/disc_access.py
  UD ppcadm/modules/emailstats.py
  UU ppcadm/modules/media.py
  UU ppcadm/modules/quarantine.py
  M  ppcadm/modules/wwwfilter.py
  M  ppcadm/resources/admin.css
  D  ppcadm/templates/disc_access.tpl
  D  ppcadm/templates/disc_summary.tpl
  M  ppcadm/templates/home.tpl
  D  ppcadm/templates/streaming_media.tpl
  M  ppcadm/templates/user.tpl
  D  ppcadm/templates/wwwfilter.tpl
  M  pre-install.sh
  M  tca/parts.d/pxelinux-cfg
  ?? old/
  ?? paste
  mattcen@sonar:prisonpc(wtf)$

- -- 
Regards,
Matthew Cengia
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iQJ8BAEBCgBmBQJSjDFAXxSAAAAAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25zLm9w
ZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXQwMjI1MjY2QUFCMThBOUQwNEMxOUQ0QjY3
QTdFMzdGMzNCMjI3MkU4AAoJEHp+N/M7InLoJ5UP/0Ws9afdJjQ7h7qP/EQuOOcq
zB/caURaNJbrqioZ/leaLTPKEeSLLL0p27Rooa+dt20qQU4Iq8I5K8tDPau5dh9z
L6AnX1i5nS6Qikz6xqgBpfnnm8MYy/Bi9iW1o8k3lZrNuoVnmrqav/JxrygQLRFv
HEtB5nGAOdsONtlFnGd7QrErf6d+bU5yCE1bN2CDwGYwghl/ApvCIMk16iRiguDA
XmMGl3Y7agQHCOiy8ghCnDwq125BBKpIhLbV485YTTM+4SvbSAwfAWy9ja3KGYO/
hmQeKmKTs3jgJhlV8qNFPCBVbuX6GgQCZeafd0aCK/taJY7AtFcn0aPQL7/d8cSc
5mE0g7Cs2BAUEiN+whYaQbUPjLs5WX8ACqSpQY1eC+dQFVJcr5enQ65bsNqy0TXb
vMmYzT5XHiMnkusdU3gC7D8yhOO0V4JYvPp+o7BEVqAKpwP4tuC3F8shsKXnIK1y
m4aQDmQAVPBcC33FtgZNLlGR7XO5OC47xKfuCmBgxLhyiYb979HUZKm3vktKsDra
ghp6I/TV7i4kkETb9/h5U4mIoTXEtdA1fyvverE+6s4gMx6C+vn10GbB2z898cuT
7OXYMONkcR+r8XtXm0/h5d64Zn82Xk/x52a/eHD6PgAvsfM3RwMiWAVVAKeRxrD9
IvjRBri6se6aBkdL+z9v
=sMCO
-----END PGP SIGNATURE-----

Re: Bizarre git merge behaviour

[Johannes Sixt]

Am 11/20/2013 4:49, schrieb Matthew Cengia:
> The other day I was merging a feature branch (shown below as
> origin/22869-new-kernel) into my staging release (shown below as 'wtf'),
> and it *reverted* a bucket-load of changes I'd made on other branches
> which had been merged into staging before. I can't for the life of me
> work out why this happened, and would appreciate any ideas. Output below
> shows symptoms:

Not really. It's impossible to tell what's wrong if you

- show only ..topic
- but not topic..
- and you keep secret which of the changes is unexpected.

Perhaps you did the following:

- On one branch, you made a change A and then another change B that reverts A.
- On the other branch, you also made the same changes as A, but there is
no equivalent of B.

In this case, it is expected that the changes made by B (i.e. the reversal
of change A) are *NOT* in the merge, i.e., the changes of A survive. It
looks as if the merge reverted change B.

If that is not the case, it could be helpful that you pick one of the
files that contains an unexpected change and post the output of

git log --left-right --oneline wtf...origin/22869-new-kernel \
    file/with/changes | cut -c1-70

-- Hannes

Re: Bizarre git merge behaviour

[Matthew Cengia]

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

On 2013-11-20 08:20, Johannes Sixt wrote:
[...]
> Not really. It's impossible to tell what's wrong if you

Hi Hannes,

Thanks for your response, and sorry for providing insufficient
information; this is a company Git repo (it's also about 200MB), so I've
got be careful what I post, but I can certainly give more than I've
shown already.

> 
> - show only ..topic
> - but not topic..

I won't show the same (git log) output as I did previously, because
there are lots of commits and is a bit difficult to read (but can do so
as an attachment if you genuinely think it may be useful), but here's a
diff from wtf to merge-base:

mattcen@sonar:prisonpc(wtf)$ git --no-pager diff --numstat --oneline \
    "$(git merge-base wtf origin/22869-new-kernel)" wtf
1       1       .gitignore
1       1       bios-monitoring/bios-alert
4       4       bios-monitoring/check_bios.sh
1       7       bios-monitoring/ppc-bios.conf
3       0       client/scm/20-security.scm
14      0       client/scm/30-apps.scm
5       10      client/scm/30-branding.scm
2       7       client/scm/30-password-reset.scm
7       9       client/scm/50-kernel-prisoner.scm
7       9       client/scm/50-kernel-staff.scm
0       10      client/scm/50-prisoner.scm
4       1       client/transmute
2       1       client/transmute.scm.in
0       1       config-files-internet.txt
66      1       debian/NEWS
24      0       debian/changelog
6       3       debian/control
0       13      debian/postinst-cfg.sh
0       6       debian/prisonpc-core.config
8       5       debian/prisonpc-core.cron.d
12      6       debian/prisonpc-core.cron.daily
2       1       debian/prisonpc-core.install
35      94      debian/prisonpc-core.postinst
25      0       debian/prisonpc-core.ppcadm-yuk.init
0       4       debian/prisonpc-core.ppcadm.upstart
14      0       debian/prisonpc-core.preinst
27      0       debian/prisonpc-core.squid-yuk.init
0       11      debian/prisonpc-core.templates
16      2       debian/prisonpc-dev.cron.weekly
6       6       debian/prisonpc-dev.install
7       0       debian/prisonpc-dev.postinst
1       0       debian/prisonpc-internet.cron.daily
6       5       debian/prisonpc-internet.install
4       19      debian/prisonpc-internet.postinst
7       1       debian/rules
11      10      disc/disc_access
7       0       doc/Test_plan.txt
11      0       doc/Vectors.txt
-       -       doc/adminguide/images/streaming_media-screen.jpg
-       -       doc/adminguide/images/streaming_media-screen.png
-       -       doc/adminguide/images/web_filtering.png
8       17      doc/adminguide/streaming_media.rst
8       3       doc/adminguide/system-status.rst
13      16      doc/adminguide/web_filtering.rst
127     0       doc/current-mail-status.txt
55      0       doc/prisonpc-mail-plan.dot
90      0       doc/prisonpc-mail.dot
2       0       doc/user_acceptance_tests/.gitignore
22      0       doc/user_acceptance_tests/README
581     0       doc/user_acceptance_tests/default.css
1011    0       doc/user_acceptance_tests/user_acceptance_tests.rst
2       2       dovecot/dovecot-staff.conf
0       579     dovecot/email_quarantine
8       5       eric/apps/complain.py
5       3       eric/apps/links.py
7       3       eric/apps/media.py
5       3       eric/apps/whitelist.py
4       0       eric/eric
1       0       eric/eric_cfg.py
48      0       generate-mail-stats
12      2       post-install.sh
10      0       postfix/alias-list
2       0       postfix/by-week.procmailrc
0       11      postfix/flush-queued-mail
2       0       postfix/invariant.procmailrc
49      67      postfix/main.cf.in
3       36      postfix/master-basic.cf.in
51      144     postfix/master-filter.cf.in
145     156     postfix/maxwell
0       15      postfix/maxwell.cfg.in
31      0       postfix/maybe
76      0       postfix/plainify
0       0       postfix/plainify.d/cur/00-nothing
6       0       postfix/plainify.d/cur/01-nonmime
12      0       postfix/plainify.d/cur/02-th
8       0       postfix/plainify.d/cur/02-tp
35761   0       postfix/plainify.d/cur/f0-many-types
832     0       postfix/plainify.d/cur/f0-rfc822
244     0       postfix/plainify.d/cur/f0-th_tbird_cascade
0       167     postfix/smtp_archiver
0       240     postfix/strip_quarantine_markers
119     248     postfix/whitelist
85      37      ppcadm/modules/disc_access.py
80      0       ppcadm/modules/emailstats.py
8       7       ppcadm/modules/mailfilter.py
130     49      ppcadm/modules/media.py
58      74      ppcadm/modules/quarantine.py
87      37      ppcadm/modules/wwwfilter.py
3       0       ppcadm/resources/admin.css
31      0       ppcadm/templates/disc_access.tpl
5       0       ppcadm/templates/disc_summary.tpl
7       6       ppcadm/templates/home.tpl
2       2       ppcadm/templates/mailfilter_menu.tpl
1       2       ppcadm/templates/quarantine_list.tpl
33      17      ppcadm/templates/quarantine_mail.tpl
67      0       ppcadm/templates/streaming_media.tpl
3       3       ppcadm/templates/user.tpl
100     0       ppcadm/templates/wwwfilter.tpl
36      27      pre-install.sh
17      21      squid/check_greylist
2       1       squid/make_url_regex
8       34      squid/remove_user.sh
8       24      squid/update_user.sh
7       16      squid/validate_url_comment
2       1       streaming-media/autodvdencode
2       1       streaming-media/m3u-slurp
4       4       streaming-media/media-stream
2       1       streaming-media/vobcopy_wrapper
2       1       switch/switch-cmd
23      1       tca/parts.d/pxelinux-cfg
14      6       tca/rebuild_prisoner_realm_access
7       4       tca/rebuild_prisoner_realm_service_access
48      147     tca/tca.pm
18      10      tca/tca_realm_access
6       5       tca/tca_realm_service_access
0       9       trimserver/trimserver.cfg.in
14      43      trimserver/trimserver.pm
0       42      webmin/rcs_ci
6       0       webmin/reset_user_desktop_config
10      49      webmin/sort_ci
1       4       webmin/sort_ci.8
0       12      webmin/sort_ci_correct
11      14      webmin/whitelist_update

> - and you keep secret which of the changes is unexpected.

The only changes I expect are these:

mattcen@sonar:prisonpc(wtf)$ git --no-pager diff --numstat --oneline \
    "$(git merge-base wtf origin/22869-new-kernel)" origin/22869-new-kernel
37      0       client/kernel/README
2797    0       client/kernel/config-3.5.7.20-1
0       3525    client/kernel/config-3.5.7.21-1
-       -       client/kernel/linux-firmware-image_3.5.7.21-1_i386.deb
-       -       client/kernel/linux-image-3.5.7.20_3.5.7.20-1_i386.deb
-       -       client/kernel/linux-image-3.5.7.21_3.5.7.21-1_i386.deb
1       0       client/scm/20-security.scm
6       7       client/scm/50-kernel-prisoner.scm
1       0       client/scm/50-staff.scm
22      0       doc/user_acceptance_tests/README
581     0       doc/user_acceptance_tests/default.css
1548    0       doc/user_acceptance_tests/user_acceptance_tests.html
4268    0       doc/user_acceptance_tests/user_acceptance_tests.pdf
940     0       doc/user_acceptance_tests/user_acceptance_tests.rst

> Perhaps you did the following:
> 
> - On one branch, you made a change A and then another change B that reverts A.
> - On the other branch, you also made the same changes as A, but there is
> no equivalent of B.
> 
> In this case, it is expected that the changes made by B (i.e. the reversal
> of change A) are *NOT* in the merge, i.e., the changes of A survive. It
> looks as if the merge reverted change B.

I have found no evidence that this happened, but am not confident enough
to rule it out.

> 
> If that is not the case, it could be helpful that you pick one of the
> files that contains an unexpected change and post the output of
> 
> git log --left-right --oneline wtf...origin/22869-new-kernel \
>     file/with/changes | cut -c1-70

Sure. I don't expect the merge to cause any changes to quarantine.py:

mattcen@sonar:prisonpc(wtf)$ git log --left-right --oneline \
    wtf...origin/22869-new-kernel ppcadm/modules/quarantine.py | cut -c1-70
< 5b5f552 Fixed msg vs msg.as_string because smtp.sendmail is picky
< 03618f2 Use Return-Path, not X-Original-Envelope-From.
< dc1169e Bug squashing.
< 8e20216 Changed quarantine module to run maybe and send directly to
< d79ad42 Change final port, name the magic number.
< e44d1b3 Merge remote-tracking branch 'origin/staging' into 22912-ppc
< a03db6d Modified quarantine UI to handle the new reports format

Further, there are other files that don't change at all on either side
of the symetric diff:

mattcen@sonar:prisonpc(wtf)$ git log -M -C  --left-right --oneline \
    wtf...origin/22869-new-kernel generate-mail-stats | cut -c1-70
<no output>

I'm truly stumped. It's also worth noting that I've gone through and
manually resolved this merge one file at a time, and I'm about 90% sure
I ended up with the correct result, but it'd be nice to have had the
merge do the right thing in the first place, and I obviously want to
avoid having to do this again in a few months' time.

-- 
Regards,
Matthew Cengia

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 966 bytes --]

Re: Bizarre git merge behaviour

[Johannes Sixt]

Am 11/20/2013 13:03, schrieb Matthew Cengia:
> The only changes I expect are these:
> 
> mattcen@sonar:prisonpc(wtf)$ git --no-pager diff --numstat --oneline
> \ "$(git merge-base wtf origin/22869-new-kernel)"
> origin/22869-new-kernel 37      0       client/kernel/README 2797
> 0       client/kernel/config-3.5.7.20-1 0       3525
> client/kernel/config-3.5.7.21-1 -       -
> client/kernel/linux-firmware-image_3.5.7.21-1_i386.deb -       -
> client/kernel/linux-image-3.5.7.20_3.5.7.20-1_i386.deb -       -
> client/kernel/linux-image-3.5.7.21_3.5.7.21-1_i386.deb 1       0
> client/scm/20-security.scm 6       7
> client/scm/50-kernel-prisoner.scm 1       0
> client/scm/50-staff.scm 22      0
> doc/user_acceptance_tests/README 581     0
> doc/user_acceptance_tests/default.css 1548    0
> doc/user_acceptance_tests/user_acceptance_tests.html 4268    0
> doc/user_acceptance_tests/user_acceptance_tests.pdf 940     0
> doc/user_acceptance_tests/user_acceptance_tests.rst

> mattcen@sonar:prisonpc(wtf)$ git log --left-right --oneline \ 
> wtf...origin/22869-new-kernel ppcadm/modules/quarantine.py | cut
> -c1-70 < 5b5f552 Fixed msg vs msg.as_string because smtp.sendmail is
> picky < 03618f2 Use Return-Path, not X-Original-Envelope-From. <
> dc1169e Bug squashing. < 8e20216 Changed quarantine module to run
> maybe and send directly to < d79ad42 Change final port, name the
> magic number. < e44d1b3 Merge remote-tracking branch 'origin/staging'
> into 22912-ppc < a03db6d Modified quarantine UI to handle the new
> reports format

So, ppcadm/modules/quarantine.py changed only on our side, but not on
their side. This is in accordance with your listing above. Nevertheless,
the file is marked conflicted in your git status --porcelain output in
the original post:

UU ppcadm/modules/quarantine.py

That's strange. I can't tell what is going on. Perhaps you have some
criss-cross merges in your history that merge-recursive trips over?

Sorry, I don't know how to help you further.

-- Hannes

Re: Bizarre git merge behaviour

[Matthew Cengia]

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

On 2013-11-20 13:29, Johannes Sixt wrote:
[...]
> That's strange. I can't tell what is going on. Perhaps you have some
> criss-cross merges in your history that merge-recursive trips over?

That's as good a guess as any, but I suspect tracking it down may
involve needles and haystacks...

> 
> Sorry, I don't know how to help you further.

No worries, thanks very much for your help so far nonetheless.

-- 
Regards,
Matthew Cengia

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 966 bytes --]

Re: Bizarre git merge behaviour

[Matthew Cengia]

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

On 2013-11-20 13:29, Johannes Sixt wrote:
[...]
> UU ppcadm/modules/quarantine.py
> 
> That's strange. I can't tell what is going on. Perhaps you have some
> criss-cross merges in your history that merge-recursive trips over?
> 
> Sorry, I don't know how to help you further.

Hah! I worked it out!

$coworker erroneously did a "git merge -s ours" into the branch I was
merging in, and it undid more changes than he intended, which is what
resulted in *everything* breaking for me.

I've appropriately chastised him and most importantly understand why the
problem occured, which puts my mind at ease.

Thanks again for all your help.

-- 
Regards,
Matthew Cengia

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 966 bytes --]

Re: Bizarre git merge behaviour

[Jakub Narębski]

W dniu 2013-11-20 13:03, Matthew Cengia pisze:
> On 2013-11-20 08:20, Johannes Sixt wrote:
> [...]
>> Not really. It's impossible to tell what's wrong if you
>
> Hi Hannes,
>
> Thanks for your response, and sorry for providing insufficient
> information; this is a company Git repo (it's also about 200MB), so I've
> got be careful what I post, but I can certainly give more than I've
> shown already.

I think there is some anonymizing tool for git, which replaces data
(blobs contents and file names) with artificial names, while preserving
history.  It was intended to help debug repos with proprietary data...
but unfortunately I have not bookmarked it (or lost bookmark).

[...]
> I'm truly stumped. It's also worth noting that I've gone through and
> manually resolved this merge one file at a time, and I'm about 90% sure
> I ended up with the correct result, but it'd be nice to have had the
> merge do the right thing in the first place, and I obviously want to
> avoid having to do this again in a few months' time.

Well, there is git-rerere (reuse recorded resolutions) which records 
file-level (textual) merge resolutions and tries to reapply if 
appropriate when merging.

Perhaps git-imerge tool would be of help in complicated merges?

-- 
Jakub Narębski