Nim: [git] only use fast-forward merges (ie, rebase before merge) to keep linear history, allow git bisect, etc

[timotheecour]

We should almost always use rebase before merging to keep git history linear; otherwise:

• it makes it hard or impossible to use git bisect [1] to find regressions
• makes following git history more confusing [2]
• it is viral, as it makes it harder for people's PR's to rebase
• makes it hard/impossible to search in git history, eg via git log -Sfoo to search when something was added/deleted etc (see [2])
• makes it dangerous to use, for eg, git rebase -i HEAD~5 when HEAD~5 occurs in the middle of a non-linear history part
• breaks other git workflows

No-one cares when you started working on a branch; the only thing that matters is the point in which it was merged. All does one need to do is when working on a PR: git pull --rebase origin devel instead of git pull origin devel

In the rare when it creates really hard to resolve conflicts, there are still better options than doing a merge so we don't create a non-linear history, eg resolve manually by applying the branch's diff against devel into 1 commit, EDIT see automated script here https://github.com/nim-lang/Nim/pull/9356#issue-222667538

• Note: in the (somewhat, YMMV) rare cases where git rebase causes conflicts, use diff3:
  git config --global merge.conflictstyle diff3 which will help with resolving conflicts (as recommended here; yes I know the author's opinion here; no need to mention it):

<<<<<<< HEAD
TextMessage.send(:include_timestamp => true)
||||||| merged common ancestor
EmailMessage.send(:include_timestamp => true)
=======
EmailMessage.send(:include_timestamp => false)
>>>>>>> feature-branch

NOTE: we could also consider using a git hook to

• https://stackoverflow.com/questions/32657757/force-feature-branch-to-be-rebased-before-it-is-merged-or-pushed

NOTE: creating a merge commit is OK (as would happen when merging a PR via Rebase and merge): using tig:

example of things we should avoid:

another example:

proposal:

in https://github.com/nim-lang/Nim/settings, disable "Allow merge commits", as follows:

links

• https://help.github.com/articles/about-merge-methods-on-github/
• https://stackoverflow.com/questions/32657757/force-feature-branch-to-be-rebased-before-it-is-merged-or-pushed
• https://stackoverflow.com/questions/457927/git-workflow-and-rebase-vs-merge-questions

[1] see also @skilchen's remark about not being able to use git bisect after merge: https://github.com/nim-lang/Nim/issues/8432#issuecomment-408635035

[2] following git history more confusing:

• eg: using tig or git log, this commit appears twice: 'nimcache' defaults to ~/.cache on Posix; cleaned up documentation
  (there are lots of such examples)
• if I already reviewed git history on devel, then later update my devel branch, the commits I already saw are now intermingled with other commits and I have to go over those again

[kaushalmodi]

in https://github.com/nim-lang/Nim/settings, disable "Allow merge commits", as follows:

So much this!! Right now, if I want to find the commit that disallowed nil for strings, I need to dive into the whole araq-misc merge commit. It would be nice the see the recent change commits at the very top.

[Araq]

I can try to adhere to this... I don't care much. It's a git design bug that these different workflows exist when a "linear" history is so much better objectively. It also looks like a git bug if git bisect cannot deal with merges. And finally, git bisect is a pretty bad way of debugging, it's not guaranteed that the change that caused the regression is the cause of the bug. And you can just ask me, it's the change in sigmatch.nim dealing with tyString and tyNil, maybe see https://nim-lang.org/docs/intern.html#the-compiler-s-architecture for more info. The changelog also mentions --nilseqs:on to get back the old behaviour, so you can grep for that. But hey, what do I know about effective work practices.

[dom96]

I'm deeply skeptical that git bisect cannot handle merges. That would be a really big flaw.

[1] see also @skilchen's remark about not being able to use git bisect after merge: #8432 (comment)

I don't see any evidence that this was caused by a non-linear history.

Rebasing is worse for conflict resolution, for complicated cases you will need to resolve conflicts multiple times for each commit. It's easy to make mistakes and if you rebase there is no history that describes how you've resolved the conflicts. With a merge commit you see how conflicts have been resolved.

There is a place for rebasing, but this 100% pure linear history workflow doesn't seem necessary to me.

[timotheecour]

if you rebase there is no history that describes how you've resolved the conflicts. With a merge commit you see how conflicts have been resolved

that history describing how I resolved the conflict is irrelevant for anyone using the git repo and only an artifact of when I started working on this feature, which is also irrelevant

say I start working on a PR that changes Foo to Foo2 (that's all it does)
then I merge (instead of rebase) against updated devel
then suppose I have a conflict and resolve it, and finally that PR gets merged to devel.

Now the history is intermingled, and it affects everyone at that point, including all the points I mentioned above (duplicated commits, etc)

If I had used a rebase instead, I'd be the only one bearing the cost of resolving conflict, and all the PR would do is "change Foo to Foo2" at the point it was merged

[dom96]

Yes, I agree that when you're working in your own little branch, that is reflected by a PR, rebasing makes more sense. But for a large project such as Nim it's impractical to enforce a 100% linear history and there are cases where storing information about how conflicts were resolved is relevant.

Plus the git project itself doesn't use a linear history and even Linus isn't crazy about a linear Git history as far as I can tell.

[timotheecour]

yikes: looks like we're back to using merges instead of rebases:

[Araq]

I tried to use rebase and got far more merge conflicts than with merge, including having to resolve the same conflict twice sometimes. I'm sure Git has reasons for this behaviour but I care more about my productivity than about Git's design bugs.

[timotheecour]

@araq if we don't care about squashing a particular PR into a single commit (or a few commits, eg 1 per file), then this is 100% automatable with a script, see https://github.com/nim-lang/Nim/pull/9356 where I squash a very old and non-easily rebasable PR via:

git fetch origin pull/5664/head:pr_5664
git checkout pr_5664
git merge origin/devel
git checkout pull_5664_3 -b origin/devel
git checkout pr_5664 -- `git diff --name-only pr_5664`
# give proper attribution it to @krux02
git commit --amend --author="Arne Döring <[email protected]>"

[timotheecour]

finally fixed in https://github.com/nim-lang/RFCs/issues/284