llvm dev - Dec 2019 - Flang landing in the monorepo

On Wed, Dec 18, 2019 at 4:56 AM Peter Waller <Peter.Waller at arm.com>
wrote:
> On 17/12/2019 22:25, James Y Knight wrote:
> > I think it would probably make the most sense to land this as a single
> > merge-commit (from the 2700-commit rewritten history you've
created)
> > onto llvm-project master, rather than as 2700 individual toplevel
> > commits to master. (Which means: disable the merge-commit prohibition
> > in the github configuration, temporarily, push this commit, and then
> > enable it again).
>
> I understood that the LLVM project did not accept any merge commit so
> far. It seems a shame to make an exception if we don't have to
>
Can you elaborate on the perceived benefits of having it as a
merge?
>
The rule against merge commits is primarily because we want to encourage
people to make reasonable separate commits to master which are sensible
(reviewable, buildable, etc) in isolation. And, to make it so the history
is more easily understandable to humans. It's not only that we don't
want
merge commits -- we actually don't really want people doing merges, in
general.

But, here we actually *do* have a merge, because flang was developed
externally. This is an exceptional circumstance (and I'm sure we'll have
a
few more). Using a merge commit in this circumstance makes it easier for
humans looking at history to understand what's going on here, rather than
harder -- because it actually marks the merge as being a merge. That's the
main reason why I think we ought to use a merge commit here.

Additionally (and less importantly), I'm going to presume that many/most of
the 2700 commits cannot actually be built. So, having a single merge commit
within which the unbuildable sub-commits are contained will also make
things better for autobuilders.

>  It's of course technically straightforward to push either
"follow-on
> commits" or a merge commit.
>
> I think there could be a benefit to having it as a "follow-on
commits",
> in that checking out an old flang commit won't have the effect of
> deleting the rest of the llvm monorepo. Granted, this may be a slim
> benefit since it will be a fairly arbitrary revision of the llvm
> monorepo, taken at the point of the initial push.
>
I'm suggesting to build the history the exact same way you are currently,
and then simply merging it onto master with a single merge commit ("git
merge --no-ff"), rather than "fast-forwarding" the entire set of
commits.
So this is not a benefit, it'd be the same either way.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20191218/585a5757/attachment.html>

On 18/12/2019 16:03, James Y Knight wrote:
I'm suggesting to build the history the exact same way you are currently,
and then simply merging it onto master with a single merge commit ("git
merge --no-ff"), rather than "fast-forwarding" the entire set of
commits. So this is not a benefit, it'd be the same either way.

Ah, thanks for clarifying. I was imagining merging the 0-parent history. I agree
with you.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20191218/d5decabe/attachment.html>

I think that it would be beneficial for the maintainers of the downstream
llvm-project repositories if there was a single merge commit instead of
2700. In my organization, the auto merging infrastructure tests and merges
upstream changes one commit at a time, without exceptions, unless there's a
span of commits with a broken build. It's not cut out to handle 2700
incoming commits, but it can handle a merge commit that merges in 2700
commits just fine, as it's only merging in the "first-parent"
commits from
the upstream branch. If the community were to decide to commit 2700 commits
directly, we would need to shut down our infrastructure for several hours,
and merge things through manually. Additionally, I'm not sure if our CI (
http://lab.llvm.org:8080/green/) will be able to handle 2700 commits coming
in at the same time, so we'd need to possibly manually hand hold it through
as the commits are coming in.

Thanks,
Alex

On Wed, 18 Dec 2019 at 08:04, James Y Knight via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> On Wed, Dec 18, 2019 at 4:56 AM Peter Waller <Peter.Waller at
arm.com> wrote:
>
>> On 17/12/2019 22:25, James Y Knight wrote:
>> > I think it would probably make the most sense to land this as a
single
>> > merge-commit (from the 2700-commit rewritten history you've
created)
>> > onto llvm-project master, rather than as 2700 individual toplevel
>> > commits to master. (Which means: disable the merge-commit
prohibition
>> > in the github configuration, temporarily, push this commit, and
then
>> > enable it again).
>>
>> I understood that the LLVM project did not accept any merge commit so
>> far. It seems a shame to make an exception if we don't have to
>
>
>>
> Can you elaborate on the perceived benefits of having it as a merge?
>>
>
> The rule against merge commits is primarily because we want to encourage
> people to make reasonable separate commits to master which are sensible
> (reviewable, buildable, etc) in isolation. And, to make it so the history
> is more easily understandable to humans. It's not only that we
don't want
> merge commits -- we actually don't really want people doing merges, in
> general.
>
> But, here we actually *do* have a merge, because flang was developed
> externally. This is an exceptional circumstance (and I'm sure we'll
have a
> few more). Using a merge commit in this circumstance makes it easier for
> humans looking at history to understand what's going on here, rather
than
> harder -- because it actually marks the merge as being a merge. That's
the
> main reason why I think we ought to use a merge commit here.
>
> Additionally (and less importantly), I'm going to presume that
many/most
> of the 2700 commits cannot actually be built. So, having a single merge
> commit within which the unbuildable sub-commits are contained will also
> make things better for autobuilders.
>
>
>>  It's of course technically straightforward to push either
"follow-on
>> commits" or a merge commit.
>>
>> I think there could be a benefit to having it as a "follow-on
commits",
>> in that checking out an old flang commit won't have the effect of
>> deleting the rest of the llvm monorepo. Granted, this may be a slim
>> benefit since it will be a fairly arbitrary revision of the llvm
>> monorepo, taken at the point of the initial push.
>>
>
> I'm suggesting to build the history the exact same way you are
currently,
> and then simply merging it onto master with a single merge commit
("git
> merge --no-ff"), rather than "fast-forwarding" the entire
set of commits.
> So this is not a benefit, it'd be the same either way.
> _______________________________________________
> 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/20191218/2ae74cb8/attachment-0001.html>

On Wed, 18 Dec 2019 at 17:44, Alex L via llvm-dev <llvm-dev at
lists.llvm.org>
wrote:
> I think that it would be beneficial for the maintainers of the downstream
> llvm-project repositories if there was a single merge commit instead of
> 2700. In my organization, the auto merging infrastructure tests and merges
> upstream changes one commit at a time, without exceptions, unless
there's a
> span of commits with a broken build. It's not cut out to handle 2700
> incoming commits, but it can handle a merge commit that merges in 2700
> commits just fine, as it's only merging in the "first-parent"
commits from
> the upstream branch. If the community were to decide to commit 2700 commits
> directly, we would need to shut down our infrastructure for several hours,
> and merge things through manually. Additionally, I'm not sure if our CI
(
> http://lab.llvm.org:8080/green/) will be able to handle 2700 commits
> coming in at the same time, so we'd need to possibly manually hand hold
it
> through as the commits are coming in.
>
> Thanks,
> Alex
>

In order to allow bisecting, we also merge one commit at a time in the
CHERI fork of LLVM.
However, we don't (yet) have the infrastructure to test each individual
commit. Therefore, in our case this would just increase the time it takes
us to perform the merge (~0-3 seconds per commit, so about 1 hour of CPU
time since there shouldn't be any conflicts) and will generate 2700
additional merge commits in our history.
I would prefer a single merge commit since that is easier for us but I
don't have a strong preference.

Alex

>
> On Wed, 18 Dec 2019 at 08:04, James Y Knight via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> On Wed, Dec 18, 2019 at 4:56 AM Peter Waller <Peter.Waller at
arm.com>
>> wrote:
>>
>>> On 17/12/2019 22:25, James Y Knight wrote:
>>> > I think it would probably make the most sense to land this as
a single
>>> > merge-commit (from the 2700-commit rewritten history
you've created)
>>> > onto llvm-project master, rather than as 2700 individual
toplevel
>>> > commits to master. (Which means: disable the merge-commit
prohibition
>>> > in the github configuration, temporarily, push this commit,
and then
>>> > enable it again).
>>>
>>> I understood that the LLVM project did not accept any merge commit
so
>>> far. It seems a shame to make an exception if we don't have to
>>
>>
>>>
>> Can you elaborate on the perceived benefits of having it as a merge?
>>>
>>
>> The rule against merge commits is primarily because we want to
encourage
>> people to make reasonable separate commits to master which are sensible
>> (reviewable, buildable, etc) in isolation. And, to make it so the
history
>> is more easily understandable to humans. It's not only that we
don't want
>> merge commits -- we actually don't really want people doing merges,
in
>> general.
>>
>> But, here we actually *do* have a merge, because flang was developed
>> externally. This is an exceptional circumstance (and I'm sure
we'll have a
>> few more). Using a merge commit in this circumstance makes it easier
for
>> humans looking at history to understand what's going on here,
rather than
>> harder -- because it actually marks the merge as being a merge.
That's the
>> main reason why I think we ought to use a merge commit here.
>>
>> Additionally (and less importantly), I'm going to presume that
many/most
>> of the 2700 commits cannot actually be built. So, having a single merge
>> commit within which the unbuildable sub-commits are contained will also
>> make things better for autobuilders.
>>
>>
>>>  It's of course technically straightforward to push either
"follow-on
>>> commits" or a merge commit.
>>>
>>> I think there could be a benefit to having it as a "follow-on
commits",
>>> in that checking out an old flang commit won't have the effect
of
>>> deleting the rest of the llvm monorepo. Granted, this may be a slim
>>> benefit since it will be a fairly arbitrary revision of the llvm
>>> monorepo, taken at the point of the initial push.
>>>
>>
>> I'm suggesting to build the history the exact same way you are
currently,
>> and then simply merging it onto master with a single merge commit
("git
>> merge --no-ff"), rather than "fast-forwarding" the
entire set of commits.
>> So this is not a benefit, it'd be the same either way.
>> _______________________________________________
>> 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/20191218/2328826e/attachment-0001.html>

> On Dec 18, 2019, at 8:03 AM, James Y Knight via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> The rule against merge commits is primarily because we want to encourage
people to make reasonable separate commits to master which are sensible
(reviewable, buildable, etc) in isolation. And, to make it so the history is
more easily understandable to humans. It's not only that we don't want
merge commits -- we actually don't really want people doing merges, in
general.
> 
> But, here we actually do have a merge, because flang was developed
externally. This is an exceptional circumstance (and I'm sure we'll have
a few more). Using a merge commit in this circumstance makes it easier for
humans looking at history to understand what's going on here, rather than
harder -- because it actually marks the merge as being a merge. That's the
main reason why I think we ought to use a merge commit here.
This is a great summary of both sides of the policy, thanks James.  I agree,

-Chris
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20191218/c22d77e7/attachment.html>

I take it from this conversation that we should do a --no-ff merge of 
the rewritten history.

The final history will look like this:

[llvm project/master] ---------------------> [empty merge commit]
                       \_ 2,700ish commits _/^

This means that `git log --first-parent` will only show the merge 
commit, and not those from the "initial flang" branch.

Currently it looks like I'm the person chosen to execute the rewrite, 
merge and push.

So far, I know I need to coordinate with Tom & Mike for the build 
emailers and the existing f18 repository to freeze submissions there.

Who is able to suspend merge-commit prohibition in github? 
Alternatively, could I (github: peterwaller-arm) be given the power 
temporarily whilst executing this? That way I could turn it off only for 
the few minutes required to do the push, and give up the permissions 
immediately after. Alternatively this could be synchronized with a phone 
call or email with someone with the capability. I intend to do this in 
the morning GMT (around 10:00am) leaving plenty of time in the workday 
to fix problems.

If anyone else wants to coordinate with me with respect to when the 
merge happens, please get in touch.

The current plan lives at 
<https://github.com/flang-compiler/f18/issues/876>, if there are other 
considerations please email me or comment on that thread and I will 
update the check-list before it is executed.

The release branch is scheduled for Wed 15th January. I propose to 
execute the plan on the Mon 13th. That gives the week beginning 6th Jan 
to finalize the plans and gather any remaining feedback.

Is everyone happy with 13th January? Please speak up if not.

I'm on leave until the 6th of January and will respond to any further 
comments as soon as I can after that point.

Regards,

- Peter

On 19/12/2019 06:20, Chris Lattner wrote:
>
>
>> On Dec 18, 2019, at 8:03 AM, James Y Knight via llvm-dev 
>> <llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>> wrote:
>>
>> The rule against merge commits is primarily because we want to 
>> encourage people to make reasonable separate commits to master which 
>> are sensible (reviewable, buildable, etc) in isolation. And, to make 
>> it so the history is more easily understandable to humans. It's not
>> only that we don't want merge commits -- we actually don't
really
>> want people doing merges, in general.
>>
>> But, here we actually/do/have a merge, because flang was developed 
>> externally. This is an exceptional circumstance (and I'm sure
we'll
>> have a few more). Using a merge commit in this circumstance makes it 
>> easier for humans looking at history to understand what's going on 
>> here, rather than harder -- because it actually marks the merge as 
>> being a merge. That's the main reason why I think we ought to use a
>> merge commit here.
>
> This is a great summary of both sides of the policy, thanks James.  I 
> agree,
>
> -Chris