llvm dev - Jan 2019 - [Github] RFC: linear history vs merge commits

Hi,

As part of the migration of LLVM's source code to github, we need to update
our developer policy with instructions about how to interact with the new git
repository.  There are a lot of different topics we will need to discuss, but
I would like to start by initiating a discussion about our merge commit
policy.  Should we:

1. Disallow merge commits and enforce a linear history by requiring a
   rebase before push.

2. Allow merge commits.

3. Require merge commits and disallow rebase before push.

I'm going to propose that if we cannot reach a consensus that we
adopt policy #1, because this is essentially what we have now
with SVN.

What does everyone think?

Thanks,
Tom

A linear history makes it much easier to git-bisect, since each commit can be
rolled back individually. Easier cross-project bisection is one of the
advantages of having the monorepo, and I wouldn't want to loose that.

-- adrian

Tom Stellard via Openmp-dev <openmp-dev at lists.llvm.org> writes:
> As part of the migration of LLVM's source code to github, we need to
update
> our developer policy with instructions about how to interact with the new
git
> repository.  There are a lot of different topics we will need to discuss,
but
> I would like to start by initiating a discussion about our merge commit
> policy.  Should we:
>
> 1. Disallow merge commits and enforce a linear history by requiring a
>    rebase before push.
>
> 2. Allow merge commits.
>
> 3. Require merge commits and disallow rebase before push.
>
> I'm going to propose that if we cannot reach a consensus that we
> adopt policy #1, because this is essentially what we have now
> with SVN.
I agree with proposing #1 in general.  It results in a nice clean
history and will be familiar to everyone working on the project.

However, there is a place for merge commits.  If there's a bug in a
release and we want to make a point release, it might make sense to make
a commit on the release branch and then merge the release branch to
master in order to ensure the fix lands there as well.  Another option
is to commit to master first and then cherry-pick to release.  A third
option is to use the "hotfix branch" method of git flow [1], which
would
result in a merge commit to the release branch and another merge commit
to master.

I've seen projects that commit to release first and then merge release
to master and also projects that commit to master and cherry-pick to
release.  I personally haven't seen projects employ the git flow hotfix
branch method rigorously.

But GitHub is read-only for almost a year, right?  So we really don't
have to decide this for a while.  I have not tried using the monorepo
and committing to SVN with it.  How does that work?  What would it do
with merge commits?

                              -David

[1] https://nvie.com/posts/a-successful-git-branching-model

On Tue, Jan 29, 2019 at 5:34 PM Tom Stellard via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> As part of the migration of LLVM's source code to github, we need to
update
> our developer policy with instructions about how to interact with the new
> git
> repository.  There are a lot of different topics we will need to discuss,
> but
> I would like to start by initiating a discussion about our merge commit
> policy.  Should we:
>
> 1. Disallow merge commits and enforce a linear history by requiring a
>    rebase before push.
>
I agree that this should be considered the status quo. The other options
need compelling benefits to justify a change.

>
> 2. Allow merge commits.
>
For the purposes of the canonical version of the project, I believe that
history of side-branches (the main value of merge commits for me) is not of
great interest.

> 3. Require merge commits and disallow rebase before push.
>
I do not believe #3 is enforceable (the disallow rebase part) unless if you
meant to just remove the option from the GitHub UI interface.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190129/32e4b8dd/attachment.html>

[Armchair observer...]

If any merges are allowed, they should be rare, have recent parent commits,
or the history graph becomes useless.

4. Each reviewed bug / feature must be rebased onto the current "known
good" commit, merged into a "probably good" commit, tested by
build bots,
and only then pushed to trunk. Keeping trunk's history more usable, with
most bad patches reworked and resubmitted instead of reverted.

5. When a new feature is broken up into a patch series, the series should
be rebased then immediately merged to help identify the individual patches
in the history graph.


On Wed, 30 Jan 2019 at 09:04, Tom Stellard via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> Hi,
>
> As part of the migration of LLVM's source code to github, we need to
update
> our developer policy with instructions about how to interact with the new
> git
> repository.  There are a lot of different topics we will need to discuss,
> but
> I would like to start by initiating a discussion about our merge commit
> policy.  Should we:
>
> 1. Disallow merge commits and enforce a linear history by requiring a
>    rebase before push.
>
> 2. Allow merge commits.
>
> 3. Require merge commits and disallow rebase before push.
>
> I'm going to propose that if we cannot reach a consensus that we
> adopt policy #1, because this is essentially what we have now
> with SVN.
>
> What does everyone think?
>
> Thanks,
> Tom
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190130/0a8e0f32/attachment.html>

How about:

Require a rebase, followed by git merge --no-ff

This creates a linear history, but with extra null merge commits
delimiting each related series of patches.

I believe it is compatible with bisect.

https://linuxhint.com/git_merge_noff_option/

On Tue, Jan 29, 2019 at 2:33 PM Tom Stellard via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
>
> Hi,
>
> As part of the migration of LLVM's source code to github, we need to
update
> our developer policy with instructions about how to interact with the new
git
> repository.  There are a lot of different topics we will need to discuss,
but
> I would like to start by initiating a discussion about our merge commit
> policy.  Should we:
>
> 1. Disallow merge commits and enforce a linear history by requiring a
>    rebase before push.
>
> 2. Allow merge commits.
>
> 3. Require merge commits and disallow rebase before push.
>
> I'm going to propose that if we cannot reach a consensus that we
> adopt policy #1, because this is essentially what we have now
> with SVN.
>
> What does everyone think?
>
> Thanks,
> Tom
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

On Tue, Jan 29, 2019 at 10:31 PM Bruce Hoult via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> How about:
>
> Require a rebase, followed by git merge --no-ff
>
For personal use, I like this the most. Unfortunately, I think this is more
of a command-line workflow that the GitHub UI is not friendly with.
In particular, the UI has roughly: merge (creates yet another merge
commit), squash, and rebase. All of these prevent the intended result.

>
> This creates a linear history, but with extra null merge commits
> delimiting each related series of patches.
>
> I believe it is compatible with bisect.
>
> https://linuxhint.com/git_merge_noff_option/
>
> On Tue, Jan 29, 2019 at 2:33 PM Tom Stellard via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
> >
> > Hi,
> >
> > As part of the migration of LLVM's source code to github, we need
to
> update
> > our developer policy with instructions about how to interact with the
> new git
> > repository.  There are a lot of different topics we will need to
> discuss, but
> > I would like to start by initiating a discussion about our merge
commit
> > policy.  Should we:
> >
> > 1. Disallow merge commits and enforce a linear history by requiring a
> >    rebase before push.
> >
> > 2. Allow merge commits.
> >
> > 3. Require merge commits and disallow rebase before push.
> >
> > I'm going to propose that if we cannot reach a consensus that we
> > adopt policy #1, because this is essentially what we have now
> > with SVN.
> >
> > What does everyone think?
> >
> > Thanks,
> > Tom
> > _______________________________________________
> > LLVM Developers mailing list
> > llvm-dev at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190129/80cef541/attachment.html>

On Tue, Jan 29, 2019 at 5:33 PM Tom Stellard via lldb-dev
<lldb-dev at lists.llvm.org> wrote:
>
> Hi,
>
> As part of the migration of LLVM's source code to github, we need to
update
> our developer policy with instructions about how to interact with the new
git
> repository.  There are a lot of different topics we will need to discuss,
but
> I would like to start by initiating a discussion about our merge commit
> policy.  Should we:
>
> 1. Disallow merge commits and enforce a linear history by requiring a
>    rebase before push.
>
> 2. Allow merge commits.
>
> 3. Require merge commits and disallow rebase before push.
>
> I'm going to propose that if we cannot reach a consensus that we
> adopt policy #1, because this is essentially what we have now
> with SVN.
>
> What does everyone think?
I'm strongly in favour of #1: no merges allowed, enforced by the
upstream repository, and that for release branches we use cherry-picks
like we do currently.

Thanks,
Hans

> On Jan 29, 2019, at 4:55 PM, Jeremy Lakeman via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> 5. When a new feature is broken up into a patch series, the series should
be rebased then immediately merged to help identify the individual patches in
the history graph.
Typically the LLVM development model discourages merging big features in one go
and instead gravitates towards breaking patches up into small, easy to reason
about patches that gradually morph the code towards the goal. This makes it
easier for reviewers and to track down regressions after each patch landed. For
this reason we rarely have series of connected patches coming in at once.

-- adrian

Strongly in favor of #1.

If we decide to move away from #1, I strongly believe it should be done 
well after the github migration.  (i.e. lets not change everything at once!)

Philip

On 1/29/19 2:33 PM, Tom Stellard via cfe-dev wrote:
> Hi,
>
> As part of the migration of LLVM's source code to github, we need to
update
> our developer policy with instructions about how to interact with the new
git
> repository.  There are a lot of different topics we will need to discuss,
but
> I would like to start by initiating a discussion about our merge commit
> policy.  Should we:
>
> 1. Disallow merge commits and enforce a linear history by requiring a
>     rebase before push.
>
> 2. Allow merge commits.
>
> 3. Require merge commits and disallow rebase before push.
>
> I'm going to propose that if we cannot reach a consensus that we
> adopt policy #1, because this is essentially what we have now
> with SVN.
>
> What does everyone think?
>
> Thanks,
> Tom
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

Bruce Hoult via llvm-dev <llvm-dev at lists.llvm.org> writes:
> How about:
>
> Require a rebase, followed by git merge --no-ff
>
> This creates a linear history, but with extra null merge commits
> delimiting each related series of patches.
>
> I believe it is compatible with bisect.
>
> https://linuxhint.com/git_merge_noff_option/
We've done both and I personally prefer the strict linear history by a
lot.  It's just much easier to understand a linear history.

                      -David

Jeremy Lakeman via llvm-dev <llvm-dev at lists.llvm.org> writes:
> 4. Each reviewed bug / feature must be rebased onto the current "known
> good" commit, merged into a "probably good" commit, tested
by build
> bots, and only then pushed to trunk. Keeping trunk's history more
> usable, with most bad patches reworked and resubmitted instead of
> reverted.
If you mean having a submitted PR trigger builds and only allow merging
if all builds pass, that may be doable.  Of course by the time it's
merged it will be against some later commit (so it should be rebased)
and there's no guarantee it will build against *that* commit.  But it
will tend to filter out most problems.

Trying to keep a branch buildable at all times is a hard problem, but
the above is probably a 90% solution.

                                -David

On 1/29/2019 4:33 PM, Tom Stellard via llvm-dev wrote:
> 
> 1. Disallow merge commits and enforce a linear history by requiring a
>     rebase before push.
I'm very much in favor of this option.

-Krzysztof

On 01/29/2019 02:48 PM, David Greene wrote:
> Tom Stellard via Openmp-dev <openmp-dev at lists.llvm.org> writes:
> 
>> As part of the migration of LLVM's source code to github, we need
to update
>> our developer policy with instructions about how to interact with the
new git
>> repository.  There are a lot of different topics we will need to
discuss, but
>> I would like to start by initiating a discussion about our merge commit
>> policy.  Should we:
>>
>> 1. Disallow merge commits and enforce a linear history by requiring a
>>    rebase before push.
>>
>> 2. Allow merge commits.
>>
>> 3. Require merge commits and disallow rebase before push.
>>
>> I'm going to propose that if we cannot reach a consensus that we
>> adopt policy #1, because this is essentially what we have now
>> with SVN.
> 
> I agree with proposing #1 in general.  It results in a nice clean
> history and will be familiar to everyone working on the project.
> 
> However, there is a place for merge commits.  If there's a bug in a
> release and we want to make a point release, it might make sense to make
> a commit on the release branch and then merge the release branch to
> master in order to ensure the fix lands there as well.  Another option
> is to commit to master first and then cherry-pick to release.  A third
> option is to use the "hotfix branch" method of git flow [1],
which would
> result in a merge commit to the release branch and another merge commit
> to master.
> 
Our current bug-fix fix process is to commit to master first and then
cherry-pick to release branches.
> I've seen projects that commit to release first and then merge release
> to master and also projects that commit to master and cherry-pick to
> release.  I personally haven't seen projects employ the git flow hotfix
> branch method rigorously.
> 
> But GitHub is read-only for almost a year, right?  So we really don't
> have to decide this for a while.  I have not tried using the monorepo
> and committing to SVN with it.  How does that work?  What would it do
> with merge commits?
> 
It will be read-only until October 2019, but I think it's important to
decide
on this issue early so we have time to research and implement ways to
automatically enforce our policy to make things as smooth as possible
when we do switch.  I really want us to be able to keep our current migration
schedule.

The GettingStarted patch has been updated with instructions for how to use
the new repo: https://llvm.org/docs/GettingStarted.html#checkout-llvm-from-git

-Tom
>                               -David
> 
> [1] https://nvie.com/posts/a-successful-git-branching-model
>