In browsing the gentoo-commits list, I came across this strange merge. https://archives.gentoo.org/gentoo-commits/message/5245af860ef1243f54bc45f17d704ed4 From the commit summary, it would seem that Ian merged changes to several thousand unrelated packages. This makes it very difficult to review. It generally makes our commit graph look like spaghetti. This is not the first time I have seen this, and I have brought this issue to Ian's attention several times in the past. Despite this, he makes no attempt to correct his process. I believe he is merging pull requests against an out-of-date copy of the master branch, where the HEAD on github is actually newer than the HEAD he has on his local machine. This causes the left and right side of the merge to get flipped. He denies that he is doing this; I conclude he is lying or simply does not know what he is doing. I would like to request that QA/comrel resolve this in some fashion, be it via some troubleshooting, or by suspending his access to merge pull requests. I do not seem to be able to get through to him.
Some additional examples of "flipped" merges: https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=c4161fdc https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=a23c9aee https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=b6d9e1b2 https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=22c62713
Here is the chain of commits that lead to the bad merge on March 7. floppym@naomi gentoo % git show 2ce51dd9de78846a6d3a34877c097b8939056f15 commit 2ce51dd9de78846a6d3a34877c097b8939056f15 Merge: f493d1c eb6c9fb Author: Ian Delaney <idella4@gentoo.org> Date: Mon Mar 7 14:42:48 2016 +0800 Merge remote-tracking branch 'remotes/sbraz/syncthing' Pull Request: https://github.com/gentoo/gentoo/pull/990 floppym@naomi gentoo % git show f493d1c commit f493d1cbee3399697c189b6bdc38bbc61b9457e3 Merge: 8970f95 ebe0dbd Author: Ian Delaney <idella4@gentoo.org> Date: Mon Mar 7 13:44:09 2016 +0800 Merge remote-tracking branch 'remotes/sbraz/pysrt' Pull request: https://github.com/gentoo/gentoo/pull/991 l be ignored, and an empty message aborts floppym@naomi gentoo % git show 8970f95 commit 8970f95de5b962f7545d56b0a7a2767a68b12079 Merge: c8cffe3 098b7e4 Author: Ian Delaney <idella4@gentoo.org> Date: Fri Mar 4 21:31:05 2016 +0800 Merge remote-tracking branch 'remotes/sbraz/bdsup' Pull request: https://github.com/gentoo/gentoo/pull/974 floppym@naomi gentoo % git show c8cffe3 commit c8cffe3c4aecdc5f2a9bbc7171ce2916e2edb142 Author: Anthony G. Basile <blueness@gentoo.org> Date: Fri Mar 4 07:23:14 2016 -0500 sys-kernel/hardened-sources: version bump to 4.4.4 vanilla-4.4.4 + genpatches-4.4-5 + grsecurity-3.1-4.4.4-201603032158 Package-Manager: portage-2.2.26 Following this in reverse, it is possible to recreate the bad merge by issuing the following commands in the gentoo repo. git reset --hard c8cffe3c4aecdc5f2a9bbc7171ce2916e2edb142 git merge -m merge1 098b7e4 git merge -m merge2 ebe0dbd git merge -m merge3 eb6c9fb In summary, 3 pull requests got merged without having pushed/pulled from git.gentoo.org/repo/gentoo.git over a 3 day period from March 4 to March 7. The proper procedure would be to push any merge immediately after committing it, and to pull immediately before executing any subsequent merge. This was not done.
We should probably look into signature verification hook as well, since it seems that this kind of process would cause non-developer-signed commits on A branch that should cause push to be rejected.
I think that it might be possible to implement an update hook to prevent developers from pushing these crazy merges. We would reject the ref update if the old object (oldrev) is unreachable from the new object (newrev) by only following first parent on any merges. However, my git-foo is not quite up to the task.
Why don't we forbid merges on master altogether?
(In reply to Ulrich Müller from comment #5) > Why don't we forbid merges on master altogether? There are legitimate reasons someone might want to introduce a merge commit to master, but they should be uncommon considering the typical Gentoo workflow. I do not believe that the average pull request requires an explicit merge commit, and these should almost always be rebased prior to being pushed by a dev.
(In reply to Michael Palimaka (kensington) from comment #6) > There are legitimate reasons someone might want to introduce a merge commit > to master, Are there? Looking at the last few weeks of our commit history, I don't see a single merge commit that would help to improve the structure of the commit history. There are lots of pointless github merges though which make the commit graph look like a mess and hard to follow. > but they should be uncommon considering the typical Gentoo workflow. > > I do not believe that the average pull request requires an explicit merge > commit, and these should almost always be rebased prior to being pushed by > a dev. The latter would have the additional advantage that every user contributed commit would have the proxy committer in its "Commit:" header.
I'm making the summary less personal. Comrel, feel free to remove yourselves; I think a technical solution is the way to go on this.
(In reply to Ulrich Müller from comment #7) > (In reply to Michael Palimaka (kensington) from comment #6) > > There are legitimate reasons someone might want to introduce a merge commit > > to master, > > Are there? Looking at the last few weeks of our commit history, I don't see > a single merge commit that would help to improve the structure of the commit > history. There are lots of pointless github merges though which make the > commit graph look like a mess and hard to follow. It has been suggested that it could be useful for grouping commits that are related, but distinct (such as porting an eclass to a new EAPI). Check out "git log --graph 149d25b8480027b97ca84fcea975a4630bacb125" where I did such an experiment. I did however make a mistake rebasing there and accidentally included a couple of unrelated commits. While I see how merge commits can be useful in certain situations, I do agree that the majority of those introduced so far are pointless and have only served to clutter up the history. > > but they should be uncommon considering the typical Gentoo workflow. > > > > I do not believe that the average pull request requires an explicit merge > > commit, and these should almost always be rebased prior to being pushed by > > a dev. > > The latter would have the additional advantage that every user contributed > commit would have the proxy committer in its "Commit:" header. Agreed. For those not familiar, consider the difference between the header of a single commit produced by a rebase (https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=086d38b7bccc6890437cf096ba0440f968584ea6), and the two commits produced by a merge (https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=1a253a63eba4d96ce846a3ebee955576e07165ec and https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=6a62df6d50083405974e121a38351dfe5a59799b). In the former, it is much clearer exactly who did what.
It's a new pure QA thing. Removing comrel.
One fancy command and its output: ``` $ git log --grep='Merge remote-tracking branch' --format=format:'%an|%ae' | awk -F\| '{f[$2]++;n[$2]=$1;}END{for(e in f){printf "%3d %-20s <%s>\n",f[e],n[e],e;}}' | sort -nrk1 193 Patrice Clement <monsieurp@gentoo.org> 150 Ian Delaney <idella4@gentoo.org> 26 Alexis Ballier <aballier@gentoo.org> 24 Andreas K. Huettel (dilfridge) <dilfridge@gentoo.org> 2 Markos Chandras <hwoarang@gentoo.org> 2 Justin Lecher <jlec@gentoo.org> 2 Jauhien Piatlicki <jauhien@gentoo.org> 2 James Le Cuirot <chewi@gentoo.org> 2 Alexandre Rostovtsev <tetromino@gentoo.org> 1 Tony Vroon <chainsaw@gentoo.org> 1 Sven Wegener <swegener@gentoo.org> 1 Robin H. Johnson <robbat2@gentoo.org> 1 Ole Reifschneider <tranquility@gentoo.org> 1 Matt Turner <mattst88@gentoo.org> 1 Kacper Kowalik <xarthisius@gentoo.org> 1 Julian Ospald <hasufell@gentoo.org> 1 Jason Zaman <perfinion@gentoo.org> 1 Ben de Groot <yngwin@gentoo.org> 1 Andrew Savchenko <bircoph@gentoo.org> ```
GLEP 66 addresses the issue of merge commits, and I haven't seen any crazy ones in quite some time.
