[FFmpeg-devel] Confusing git merges

[Cory Fields]

On Mon, Mar 21, 2011 at 6:37 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Mon, Mar 21, 2011 at 10:53:56AM +0100, Tomas Härdin wrote:
>> Michael Niedermayer skrev 2011-03-21 10:42:
>>> On Mon, Mar 21, 2011 at 09:02:02AM +0100, Tomas Härdin wrote:
>>>> Michael Niedermayer skrev 2011-03-19 20:32:
>>>>> Hi
>>>>>
>>>>> I know alot of people have been confused seeing changes and then seeing them
>>>>> reverted, or seeing changes on the ML that arent in the merged version
>>>>>
>>>>> I really hate it as well, but this is how 'git pull' works
>>>>> Unless we modify git or at least the mailing list hook these things will
>>>>> keep showing up.
>>>>>
>>>>> when theres a merge, the hook posts all changes of the merged branch no matter
>>>>> if they are in the actual result or not.
>>>>>
>>>>> i cant really do anything about it, except hacking the ML hook to filter it
>>>>> out
>>>>
>>>> Why not squash the reverts into the merge before pushing?
>>>
>>> i agree that this is better, especially in the sense of not poluting
>>> history. Ive done it also at least twice, and will probably keep using it
>>> but it doesnt stop the "unmerged" commits from showing up
>>
>> You could always cherry-pick
>
> I know, i guess it depends on how bad the tree is, currently the commit quality
> is definitly on a downward trend there
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> When you are offended at any man's fault, turn to yourself and study your
> own failings. Then you will forget your anger. -- Epictetus
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
>
> iEYEARECAAYFAk2HKnUACgkQYR7HhwQLD6s67ACgh+mq4Mqd5qkJEv+/y99wil+I
> JBsAnjph4sjsWQVOMd9fcbDOYY3FQ/Sx
> =d2hA
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>

I'm not an ffmpeg dev, but we (XBMC) have dealt with many of these
issues already, so maybe our experiences can help here. This is
intended as constructive criticism from an outsider's point-of-view
only.

Here are a few basic git merge principles:

- History should be clean before pull. This means that the author is
responsible for rebasing if necessary before upstream pulls. Fixup
commits should never land upstream. If a commit is not wanted, it
should be removed or squashed by the dev, not reverted after the pull.

- Upstream should avoid cherry-picking or rebasing before push
(obviously i mean fast-forward here, non-fast-forward goes without
saying). This changes the author's commits, voiding any sign-offs.
Cherry-picking is an svn'ism born out of years of manual patching ;)

- Remember that merges keep things inline. If the dev rebases he can
re-validate his code. But git can't figure out how to resolve
conflicts properly when everything is shoved on top, so it's not good
practice for a pusher to do these things in his tree before pushing.

- Use descriptive commit messages for merges. I see many merges from
'newdev/master', but the commit messages don't mention what was
committed or why. Merge commits should show the actual merged commits
in the commit message as well as the conflicts*. This makes life much
easier because you can simply eyeball a merge commit, rather than
having to step through the log. For ex: http://tinyurl.com/62mzf3a
*you can do this by using 'git merge --log', or set it as default:
'git config merge.log true'

For a project as big as ffmpeg, a model where everyone is trying to
make their way into master may not be sustainable. It may make sense
to consider a subsystem model (like the linux kernel), where a few
people accept cleaned up code, which is then merged into master in
chunks. But obviously that should wait until everyone has a handle on
git. There are many suitable workflows with git, but trying to treat
it like svn can lead to problems like these.

Cory