llvm dev - Mar 2020 - Allowing PRs on GitHub for some subprojects

On Wed, Mar 4, 2020 at 9:42 AM Louis Dionne <ldionne at apple.com> wrote:
>
>
> On Mar 4, 2020, at 12:13, Mehdi AMINI <joker.eph at gmail.com> wrote:
>
>
>
> On Wed, Mar 4, 2020 at 8:14 AM Louis Dionne <ldionne at apple.com>
wrote:
>
>> Mehdi, Chris & others,
>>
>> I guess I did not express the main reasons for wanting to switch over
>> very well in my original message.
>>
>
> You original message was about “ commit attribution”, but now it is all
> about testing?
>
>
> It was about allowing GitHub PRs for libc++, and one of the reasons I
> cited was commit attribution.
>
>
> Instead of jumping to a solution (pull-request) why not expressing the
> actual problem (lack of pre merge testing) and discuss it and explore all
> the possible solutions?
> I think the discussion would be much more productive if we take it from
> first principles here.
>
>
> That's really what I've tried doing below. Quoting myself:
>
> > And if the solution is that Harbormaster suddenly becomes usable
without
> an unreasonable time investment from me, then I'm fine with that too.
I'm
> not looking to switch to GitHub PRs for the sake of it, I'm looking to
> solve problems that are harming libc++ in the current system.
>
> Maybe I should have stated that earlier on.
>
>
>
> Like Christian talked about, for me it's all about pre-commit testing.
I
>> believe pre-commit testing is a widely shared desire among this
community.
>> However, how badly it is missed depends on sub-projects, because they
have
>> different realities. For example, in libc++:
>>
>> 1. We have a lot of first-time contributors, which means that the
>> maintainers end up shepherding many contributions. In particular, this
>> often means fixing small breakage following their changes, which can be
>> difficult for them because they can't reproduce the failures
locally, and
>> they might not even know where to look. While these contributors can
submit
>> valuable improvements and bug fixes, we can't expect them to fix
every last
>> platform that we support in the current state of things -- it's
hard, it's
>> boring, and it's stressful.
>>
>> 2. Our testing matrix is very large, and interactions between different
>> configurations (usually #ifs/#elses) is very subtle. This means the
rate of
>> mistake-on-first-try is, I think, higher in libc++ than in most other
LLVM
>> projects. Even with careful review, I find that a large percentage of
>> changes end up breaking something somewhere, and I have to fix it
(usually
>> quickly enough to avoid reverting).
>>
>> As a result, the lack of pre-commit testing is actively harming the
>> health of libc++ as a project. It might be true for other projects as
well,
>> but I can only speak for libc++ because that's where I have first
hand
>> experience.
>>
>
> What changed recently that makes this suddenly critical compared to the
> previous years?
>
>
> At the risk of oversharing, this has been my biggest source of pain and
> frustration since I've became a libc++ maintainer. It's always been
a
> problem since I started about 2 years ago. I can still feel the hurt of
> fixing the ripples of landing aligned allocation in libc++ in 2018, which
> lasted for weeks and weeks (on and off).
>
> Last week, I spent about 4 days fixing the ripples of a high-quality
> contribution that happened to break some bot configurations (and some
> botless ones too). That was both very stressful and frustrating, and I
> swore to myself this was going to be the last week like that.
>
>
> Unfortunately, we currently don't have a good way of doing pre-commit
>> testing on Phabricator AFAICT
>>
>
>
> I thought we do now? I got a bunch of libcxx failing on my revision a few
> weeks ago.
>
>
> My understanding is that Christian was the one to setup the pre-merge
> testing we have currently (thanks!). And he says:
>
>     > This is soooooo much easier to set up and maintain than the
> Phabricator integration
>
Of course it is easier to do on GitHub, but the integration on Phabricator
can also a one-time cost for the project, which is already kind of on the
way with https://github.com/google/llvm-premerge-checks ; then it is a
matter of making it easier to add your own bot the pre-merge and increase
the coverage.



> Furthermore, what libc++ needs in this area is not necessarily the same as
> the rest of LLVM. I don't know what pre-merge bots are setup right now,
and
> it's certainly better than nothing, but we have a lot of test
> configurations that just don't really make sense for a toolchain:
> - test suite in c++03/c++11/c++14/etc
> - exceptions on/off
> - with per-TU guarantee enabled/disabled
> - all the macOS back-deployment targets against system dylibs
> - etc.
>
I just see this as different configurations / builders that you have to
provide if you care about, just like our current post-merge buildbot
infrastructure actually: http://lab.llvm.org:8011/buildslaves
For example MLIR would want to run test on machines with GPUs (to test our
CUDA and Vulkan paths), which are of no interest to most of the other
projects, yet that seems fairly orthogonal to the mechanism to register a
new bots / config with the system.


>
> I'm not expecting anyone else to set these up cause it doesn't make
sense.
> I need to be empowered to do it myself, and right now I'm not (or
I'm
> really not looking in the right spot).
>
> The LLVM-premerge-test project recently added presubmit on Linux.
> Windows will hopefully follow soon (in beta right now I believe), and Mac
> afterward!  (Even though mac is lacking on the cloud provider availability)
>
>
> . From the Harbormaster documentation [1]:
>>
>>     You'll need to write a nontrivial amount of code to get this
working
>> today. In the future, Harbormaster will become more powerful and have
more
>> builtin support for interacting with build systems.
>>
>> So while I appreciate all the efforts being made in this area, I still
>> don't even know where to start if I want to setup pre-commit
testing for
>> libc++ today. However, the path is very clear with GitHub PRs and there
are
>> many options available.
>>
>> Whenever I hear arguments of dividing the community, not being able to
>> share infrastructure, the lack of Herald -- those all make a lot of
sense
>> to me and I think they're good arguments.
>>
> However, it is clear that folks who even think about these arguments are
>> not paying the same cost for the lack of good pre-commit testing that
I'm
>> paying on a weekly basis,
>>
>
> FYI that can read quite condescending...
>
>
> Sorry, that was really not my intent. The point I tried to make is that we
> have different priorities because we work on different projects, which have
> different requirements. So what may seem like just an annoyance to some
> might be a huge quality of life and productivity issue for someone else.
>
> Louis
>
>
>
> because for me that outweighs everything else.
>>
>
>> I don't know how to come to a decision here, all I know is that
libc++
>> needs to get out of the status quo soon. And if the solution is that
>> Harbormaster suddenly becomes usable without an unreasonable time
>> investment from me, then I'm fine with that too. I'm not
looking to switch
>> to GitHub PRs for the sake of it, I'm looking to solve problems
that are
>> harming libc++ in the current system.
>>
>> Cheers,
>> Louis
>>
>> [1]:
>> https://secure.phabricator.com/book/phabricator/article/harbormaster/
>>
>>
>> On Feb 29, 2020, at 23:06, Mehdi AMINI <joker.eph at gmail.com>
wrote:
>>
>>
>>
>> On Tue, Feb 25, 2020 at 4:19 AM Christian Kühnel via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>>
>>> Hi Louis,
>>>
>>> I think this is a good idea. We should start with some local
experiments
>>> where people are willing to try it and figure out how well that
works and
>>> what does not. Why not allow this for "not significant"
changes? They are
>>> merged without review today, so we could do them with reviews (and
>>> automated tests) via pull requests instead.
>>>
>>
>> I still feel this is only a recipe for confusion if "some"
pull-requests
>> are accepted on Github but not all. So -1 from me on this.
>>
>>
>>>
>>> @Mehdi
>>>
>>>> - it does not favor to build common tooling: the recent work on
>>>> enabling pre-submit CI tests on Phabricator is valuable and
I'm looking
>>>> forward to get this extended. But splitting the various ways of
>>>> contributing to the repo just means more infrastructure to
build to sustain
>>>> this kind of efforts. (the infrastructure is easier built on
GitHub by
>>>> the way, but that is an argument in favor of migrating from
Phab to GH for
>>>> the full-project).
>>>>
>>>
>>> Oh I'm happy to add Github support as soon as someone switches
on PRs.
>>> This is soooooo much easier to set up and maintain than the
Phabricator
>>> integration. And we already have builds for the release branch (
>>> https://buildkite.com/llvm-project/llvm-release-builds) anyway. So
we
>>> could easily scale that up. And we can only get pre-merge testing
on
>>> Phabricator to a certain point, as it's not triggering builds
for ~50% of
>>> the code reviews.
>>>
>>> @Chris Lattner
>>>
>>>> Although I am one of the (many) people who would love to see us
move
>>>> from Phabricator to GitHub PRs, I think it is super important
that we
>>>> do the transition all at once to keep the LLVM community
together.  I’m
>>>> already concerned about the fragmentation the discourse server
is causing,
>>>> e.g. MLIR not using a -dev list.  I’d rather the community
processes stay
>>>> consistent.
>>>>
>>>
>>> Please allow me to disagree there. IMHO we're way too large and
diverse
>>> of a project to do binary, overnight transitions.
>>>
>>
>> You seem to be arguing the "how to transition" while there is
no
>> agreement on a transition happening in the first place.
>>
>>
>>> We're also too large to follow a one-size-fits-all approach. If
we agree,
>>>
>>
>> I don't: we went with a monorepo because we believed that the
>> one-size-fits-all would be more beneficial than splitting, both in
terms of
>> infrastructure, but also in terms of the practices of the community,
etc.
>>
>> Github PRs are the right glow, why take this step-by-step. We should
have
>>> something like a list of important and supported use
cases/interactions for
>>> the infrastructure. Then we could start working on them one-by-one
and
>>> figure out if/how they could be implemented on Github and how we
could do a
>>> smooth transition between these.
>>>
>>> If Herald rules are important: Find a way to implement something
similar
>>> for Github. Maybe there is even a market for such a tool.
>>> If transparency is the problem: Find a way to mirror PRs into
>>> Phabricator, so people can at least see them there.
>>> We're not restricted to community contributions there. We can
also pay
>>> someone to build the things we need.
>>>
>>
>> One aspect here though is that we can pay someone to build the things
we
>> need in Phabricator, we can't change GitHub though.
>> It was mentioned in the past that we should engage with GitHub and see
if
>> they would add the feature we're missing to their roadmap, if it
hasn't
>> been done I'd start there: building up this list of things that
need to
>> happens before we can agree towards a transition, and engaging with
GitHub
>> to have these.
>>
>> --
>> Mehdi
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200304/67744cd2/attachment.html>

> On Mar 4, 2020, at 20:15, Mehdi AMINI <joker.eph at gmail.com> wrote:
> 
> 
> 
> On Wed, Mar 4, 2020 at 9:42 AM Louis Dionne <ldionne at apple.com
<mailto:ldionne at apple.com>> wrote:
> 
> 
>> On Mar 4, 2020, at 12:13, Mehdi AMINI <joker.eph at gmail.com
<mailto:joker.eph at gmail.com>> wrote:
>> 
>> 
>> 
>> On Wed, Mar 4, 2020 at 8:14 AM Louis Dionne <ldionne at apple.com
<mailto:ldionne at apple.com>> wrote:
>> Mehdi, Chris & others,
>> 
>> I guess I did not express the main reasons for wanting to switch over
very well in my original message.
>> 
>> You original message was about “ commit attribution”, but now it is all
about testing?
> 
> It was about allowing GitHub PRs for libc++, and one of the reasons I cited
was commit attribution.
> 
>> 
>> Instead of jumping to a solution (pull-request) why not expressing the
actual problem (lack of pre merge testing) and discuss it and explore all the
possible solutions?
>> I think the discussion would be much more productive if we take it from
first principles here.
> 
> That's really what I've tried doing below. Quoting myself:
> 
> 	> And if the solution is that Harbormaster suddenly becomes usable
without an unreasonable time investment from me, then I'm fine with that
too. I'm not looking to switch to GitHub PRs for the sake of it, I'm
looking to solve problems that are harming libc++ in the current system.
> 
> Maybe I should have stated that earlier on.
> 
>> 
>> 
>> Like Christian talked about, for me it's all about pre-commit
testing. I believe pre-commit testing is a widely shared desire among this
community. However, how badly it is missed depends on sub-projects, because they
have different realities. For example, in libc++:
>> 
>> 1. We have a lot of first-time contributors, which means that the
maintainers end up shepherding many contributions. In particular, this often
means fixing small breakage following their changes, which can be difficult for
them because they can't reproduce the failures locally, and they might not
even know where to look. While these contributors can submit valuable
improvements and bug fixes, we can't expect them to fix every last platform
that we support in the current state of things -- it's hard, it's
boring, and it's stressful.
>> 
>> 2. Our testing matrix is very large, and interactions between different
configurations (usually #ifs/#elses) is very subtle. This means the rate of
mistake-on-first-try is, I think, higher in libc++ than in most other LLVM
projects. Even with careful review, I find that a large percentage of changes
end up breaking something somewhere, and I have to fix it (usually quickly
enough to avoid reverting).
>> 
>> As a result, the lack of pre-commit testing is actively harming the
health of libc++ as a project. It might be true for other projects as well, but
I can only speak for libc++ because that's where I have first hand
experience.
>> 
>> What changed recently that makes this suddenly critical compared to the
previous years?
> 
> At the risk of oversharing, this has been my biggest source of pain and
frustration since I've became a libc++ maintainer. It's always been a
problem since I started about 2 years ago. I can still feel the hurt of fixing
the ripples of landing aligned allocation in libc++ in 2018, which lasted for
weeks and weeks (on and off).
> 
> Last week, I spent about 4 days fixing the ripples of a high-quality
contribution that happened to break some bot configurations (and some botless
ones too). That was both very stressful and frustrating, and I swore to myself
this was going to be the last week like that.
> 
>> 
>> Unfortunately, we currently don't have a good way of doing
pre-commit testing on Phabricator AFAICT
>> 
>> 
>> I thought we do now? I got a bunch of libcxx failing on my revision a
few weeks ago.
> 
> My understanding is that Christian was the one to setup the pre-merge
testing we have currently (thanks!). And he says:
> 
>     > This is soooooo much easier to set up and maintain than the
Phabricator integration
> 
> Of course it is easier to do on GitHub, but the integration on Phabricator
can also a one-time cost for the project, which is already kind of on the way
with https://github.com/google/llvm-premerge-checks
<https://github.com/google/llvm-premerge-checks> ; then it is a matter of
making it easier to add your own bot the pre-merge and increase the coverage.
You seem to be dismissing the cost of setting up bots as being something that
has to be paid once. In my experience, however, this is actually a recurring
cost because these bots need maintenance. And whenever we add a new bot,
it's also a problem if we are confronted to this complexity again.

But like I said before, I'm only here to solve my (libc++) problems. And
right now, my problem is that I *literally can't* figure out how to setup a
pre-merge bot on Phabricator. Do you know how to do that? In all seriousness,
can you show me how to set up even a simple bot that would run the most basic
libc++ configuration? I'm sure I would be able (and certainly willing) to
extrapolate from there and set up the other missing configurations without your
help.

But if you can't help me, then who can? Honestly, I think we're all
talking out of our hats and the only person who truly has gone through the
process is Christian -- and you've seen his email.

Cheers,
Louis

>  
> Furthermore, what libc++ needs in this area is not necessarily the same as
the rest of LLVM. I don't know what pre-merge bots are setup right now, and
it's certainly better than nothing, but we have a lot of test configurations
that just don't really make sense for a toolchain:
> - test suite in c++03/c++11/c++14/etc
> - exceptions on/off
> - with per-TU guarantee enabled/disabled
> - all the macOS back-deployment targets against system dylibs
> - etc.
> 
> I just see this as different configurations / builders that you have to
provide if you care about, just like our current post-merge buildbot
infrastructure actually: http://lab.llvm.org:8011/buildslaves
<http://lab.llvm.org:8011/buildslaves>
> For example MLIR would want to run test on machines with GPUs (to test our
CUDA and Vulkan paths), which are of no interest to most of the other projects,
yet that seems fairly orthogonal to the mechanism to register a new bots /
config with the system.
> 
>  
> 
> I'm not expecting anyone else to set these up cause it doesn't make
sense. I need to be empowered to do it myself, and right now I'm not (or
I'm really not looking in the right spot).
> 
>> The LLVM-premerge-test project recently added presubmit on Linux.
>> Windows will hopefully follow soon (in beta right now I believe), and
Mac afterward!  (Even though mac is lacking on the cloud provider availability)
>> 
>> 
>> . From the Harbormaster documentation [1]:
>> 
>>     You'll need to write a nontrivial amount of code to get this
working today. In the future, Harbormaster will become more powerful and have
more builtin support for interacting with build systems.
>> 
>> So while I appreciate all the efforts being made in this area, I still
don't even know where to start if I want to setup pre-commit testing for
libc++ today. However, the path is very clear with GitHub PRs and there are many
options available.
>> 
>> Whenever I hear arguments of dividing the community, not being able to
share infrastructure, the lack of Herald -- those all make a lot of sense to me
and I think they're good arguments.
>> However, it is clear that folks who even think about these arguments
are not paying the same cost for the lack of good pre-commit testing that
I'm paying on a weekly basis,
>> 
>> FYI that can read quite condescending... 
> 
> Sorry, that was really not my intent. The point I tried to make is that we
have different priorities because we work on different projects, which have
different requirements. So what may seem like just an annoyance to some might be
a huge quality of life and productivity issue for someone else.
> 
> Louis
> 
>> 
>> 
>> because for me that outweighs everything else.
>> 
>> I don't know how to come to a decision here, all I know is that
libc++ needs to get out of the status quo soon. And if the solution is that
Harbormaster suddenly becomes usable without an unreasonable time investment
from me, then I'm fine with that too. I'm not looking to switch to
GitHub PRs for the sake of it, I'm looking to solve problems that are
harming libc++ in the current system.
>> 
>> Cheers,
>> Louis
>> 
>> [1]:
https://secure.phabricator.com/book/phabricator/article/harbormaster/
<https://secure.phabricator.com/book/phabricator/article/harbormaster/>
>> 
>> 
>>> On Feb 29, 2020, at 23:06, Mehdi AMINI <joker.eph at gmail.com
<mailto:joker.eph at gmail.com>> wrote:
>>> 
>>> 
>>> 
>>> On Tue, Feb 25, 2020 at 4:19 AM Christian Kühnel via llvm-dev
<llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>>
wrote:
>>> Hi Louis,
>>> 
>>> I think this is a good idea. We should start with some local
experiments where people are willing to try it and figure out how well that
works and what does not. Why not allow this for "not significant"
changes? They are merged without review today, so we could do them with reviews
(and automated tests) via pull requests instead.
>>> 
>>> I still feel this is only a recipe for confusion if
"some" pull-requests are accepted on Github but not all. So -1 from me
on this.
>>>  
>>> 
>>> @Mehdi
>>> - it does not favor to build common tooling: the recent work on
enabling pre-submit CI tests on Phabricator is valuable and I'm looking
forward to get this extended. But splitting the various ways of contributing to
the repo just means more infrastructure to build to sustain this kind of
efforts. (the infrastructure is easier built on GitHub by the way, but that is
an argument in favor of migrating from Phab to GH for the full-project).
>>> 
>>> Oh I'm happy to add Github support as soon as someone switches
on PRs. This is soooooo much easier to set up and maintain than the Phabricator
integration. And we already have builds for the release branch
(https://buildkite.com/llvm-project/llvm-release-builds
<https://buildkite.com/llvm-project/llvm-release-builds>) anyway. So we
could easily scale that up. And we can only get pre-merge testing on Phabricator
to a certain point, as it's not triggering builds for ~50% of the code
reviews.
>>> 
>>> @Chris Lattner
>>> Although I am one of the (many) people who would love to see us
move from Phabricator to GitHub PRs, I think it is super important that we do
the transition all at once to keep the LLVM community together.  I’m already
concerned about the fragmentation the discourse server is causing, e.g. MLIR not
using a -dev list.  I’d rather the community processes stay consistent.
>>> 
>>> Please allow me to disagree there. IMHO we're way too large and
diverse of a project to do binary, overnight transitions.
>>> 
>>> You seem to be arguing the "how to transition" while
there is no agreement on a transition happening in the first place.
>>>  
>>> We're also too large to follow a one-size-fits-all approach. If
we agree,
>>> 
>>> I don't: we went with a monorepo because we believed that the
one-size-fits-all would be more beneficial than splitting, both in terms of
infrastructure, but also in terms of the practices of the community, etc.
>>> 
>>> Github PRs are the right glow, why take this step-by-step. We
should have something like a list of important and supported use
cases/interactions for the infrastructure. Then we could start working on them
one-by-one and figure out if/how they could be implemented on Github and how we
could do a smooth transition between these.
>>> 
>>> If Herald rules are important: Find a way to implement something
similar for Github. Maybe there is even a market for such a tool.
>>> If transparency is the problem: Find a way to mirror PRs into
Phabricator, so people can at least see them there.
>>> We're not restricted to community contributions there. We can
also pay someone to build the things we need.
>>> 
>>> One aspect here though is that we can pay someone to build the
things we need in Phabricator, we can't change GitHub though.
>>> It was mentioned in the past that we should engage with GitHub and
see if they would add the feature we're missing to their roadmap, if it
hasn't been done I'd start there: building up this list of things that
need to happens before we can agree towards a transition, and engaging with
GitHub to have these.
>>> 
>>> -- 
>>> Mehdi
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200305/71eb99d0/attachment-0001.html>

On Thu, Mar 5, 2020 at 8:30 AM Louis Dionne <ldionne at apple.com> wrote:
>
>
> On Mar 4, 2020, at 20:15, Mehdi AMINI <joker.eph at gmail.com> wrote:
>
>
>
> On Wed, Mar 4, 2020 at 9:42 AM Louis Dionne <ldionne at apple.com>
wrote:
>
>>
>>
>> On Mar 4, 2020, at 12:13, Mehdi AMINI <joker.eph at gmail.com>
wrote:
>>
>>
>>
>> On Wed, Mar 4, 2020 at 8:14 AM Louis Dionne <ldionne at
apple.com> wrote:
>>
>>> Mehdi, Chris & others,
>>>
>>> I guess I did not express the main reasons for wanting to switch
over
>>> very well in my original message.
>>>
>>
>> You original message was about “ commit attribution”, but now it is all
>> about testing?
>>
>>
>> It was about allowing GitHub PRs for libc++, and one of the reasons I
>> cited was commit attribution.
>>
>>
>> Instead of jumping to a solution (pull-request) why not expressing the
>> actual problem (lack of pre merge testing) and discuss it and explore
all
>> the possible solutions?
>> I think the discussion would be much more productive if we take it from
>> first principles here.
>>
>>
>> That's really what I've tried doing below. Quoting myself:
>>
>> > And if the solution is that Harbormaster suddenly becomes usable
>> without an unreasonable time investment from me, then I'm fine with
that
>> too. I'm not looking to switch to GitHub PRs for the sake of it,
I'm
>> looking to solve problems that are harming libc++ in the current
system.
>>
>> Maybe I should have stated that earlier on.
>>
>>
>>
>> Like Christian talked about, for me it's all about pre-commit
testing. I
>>> believe pre-commit testing is a widely shared desire among this
community.
>>> However, how badly it is missed depends on sub-projects, because
they have
>>> different realities. For example, in libc++:
>>>
>>> 1. We have a lot of first-time contributors, which means that the
>>> maintainers end up shepherding many contributions. In particular,
this
>>> often means fixing small breakage following their changes, which
can be
>>> difficult for them because they can't reproduce the failures
locally, and
>>> they might not even know where to look. While these contributors
can submit
>>> valuable improvements and bug fixes, we can't expect them to
fix every last
>>> platform that we support in the current state of things -- it's
hard, it's
>>> boring, and it's stressful.
>>>
>>> 2. Our testing matrix is very large, and interactions between
different
>>> configurations (usually #ifs/#elses) is very subtle. This means the
rate of
>>> mistake-on-first-try is, I think, higher in libc++ than in most
other LLVM
>>> projects. Even with careful review, I find that a large percentage
of
>>> changes end up breaking something somewhere, and I have to fix it
(usually
>>> quickly enough to avoid reverting).
>>>
>>> As a result, the lack of pre-commit testing is actively harming the
>>> health of libc++ as a project. It might be true for other projects
as well,
>>> but I can only speak for libc++ because that's where I have
first hand
>>> experience.
>>>
>>
>> What changed recently that makes this suddenly critical compared to the
>> previous years?
>>
>>
>> At the risk of oversharing, this has been my biggest source of pain and
>> frustration since I've became a libc++ maintainer. It's always
been a
>> problem since I started about 2 years ago. I can still feel the hurt of
>> fixing the ripples of landing aligned allocation in libc++ in 2018,
which
>> lasted for weeks and weeks (on and off).
>>
>> Last week, I spent about 4 days fixing the ripples of a high-quality
>> contribution that happened to break some bot configurations (and some
>> botless ones too). That was both very stressful and frustrating, and I
>> swore to myself this was going to be the last week like that.
>>
>>
>> Unfortunately, we currently don't have a good way of doing
pre-commit
>>> testing on Phabricator AFAICT
>>>
>>
>>
>> I thought we do now? I got a bunch of libcxx failing on my revision a
few
>> weeks ago.
>>
>>
>> My understanding is that Christian was the one to setup the pre-merge
>> testing we have currently (thanks!). And he says:
>>
>>     > This is soooooo much easier to set up and maintain than the
>> Phabricator integration
>>
>
> Of course it is easier to do on GitHub, but the integration on Phabricator
> can also a one-time cost for the project, which is already kind of on the
> way with https://github.com/google/llvm-premerge-checks ; then it is a
> matter of making it easier to add your own bot the pre-merge and increase
> the coverage.
>
>
> You seem to be dismissing the cost of setting up bots as being something
> that has to be paid once.
>
Sorry if I wasn't clear on this: I was referring to the difference between
integrating a bot infrastructure with Phabricator vs GitHub as a one-time
cost. The extra cost unique to Phabricator should be paid once for the
project, adding more bots to a pre-merge system that is triggered by and
report to Phab should then be fairly equivalent afterward hopefully.


> In my experience, however, this is actually a recurring cost because these
> bots need maintenance. And whenever we add a new bot, it's also a
problem
> if we are confronted to this complexity again.
>
> But like I said before, I'm only here to solve my (libc++) problems.
And
> right now, my problem is that I *literally can't* figure out how to
setup a
> pre-merge bot on Phabricator. Do you know how to do that? In all
> seriousness, can you show me how to set up even a simple bot that would run
> the most basic libc++ configuration? I'm sure I would be able (and
> certainly willing) to extrapolate from there and set up the other missing
> configurations without your help.
>
> But if you can't help me, then who can? Honestly, I think we're all
> talking out of our hats and the only person who truly has gone through the
> process is Christian -- and you've seen his email.
>
I started working on this in September, anticipating the migration of MLIR
into the monorepo, the only reason I stopped is because I learnt that
Christian was doing it as well.
My understanding is that the goal of
https://github.com/google/llvm-premerge-checks is not to setup a rigid
one-time bot for Phabricator, but reach a point where adding a new
configuration should have a playbook similar to
https://llvm.org/docs/HowToAddABuilder.html

Best,

-- 
Mehdi



>
> Cheers,
> Louis
>
>
>
>
>> Furthermore, what libc++ needs in this area is not necessarily the same
>> as the rest of LLVM. I don't know what pre-merge bots are setup
right now,
>> and it's certainly better than nothing, but we have a lot of test
>> configurations that just don't really make sense for a toolchain:
>> - test suite in c++03/c++11/c++14/etc
>> - exceptions on/off
>> - with per-TU guarantee enabled/disabled
>> - all the macOS back-deployment targets against system dylibs
>> - etc.
>>
>
> I just see this as different configurations / builders that you have to
> provide if you care about, just like our current post-merge buildbot
> infrastructure actually: http://lab.llvm.org:8011/buildslaves
> For example MLIR would want to run test on machines with GPUs (to test our
> CUDA and Vulkan paths), which are of no interest to most of the other
> projects, yet that seems fairly orthogonal to the mechanism to register a
> new bots / config with the system.
>
>
>
>>
>> I'm not expecting anyone else to set these up cause it doesn't
make
>> sense. I need to be empowered to do it myself, and right now I'm
not (or
>> I'm really not looking in the right spot).
>>
>> The LLVM-premerge-test project recently added presubmit on Linux.
>> Windows will hopefully follow soon (in beta right now I believe), and
Mac
>> afterward!  (Even though mac is lacking on the cloud provider
availability)
>>
>>
>> . From the Harbormaster documentation [1]:
>>>
>>>     You'll need to write a nontrivial amount of code to get
this
>>> working today. In the future, Harbormaster will become more
powerful and
>>> have more builtin support for interacting with build systems.
>>>
>>> So while I appreciate all the efforts being made in this area, I
still
>>> don't even know where to start if I want to setup pre-commit
testing for
>>> libc++ today. However, the path is very clear with GitHub PRs and
there are
>>> many options available.
>>>
>>> Whenever I hear arguments of dividing the community, not being able
to
>>> share infrastructure, the lack of Herald -- those all make a lot of
sense
>>> to me and I think they're good arguments.
>>>
>> However, it is clear that folks who even think about these arguments
are
>>> not paying the same cost for the lack of good pre-commit testing
that I'm
>>> paying on a weekly basis,
>>>
>>
>> FYI that can read quite condescending...
>>
>>
>> Sorry, that was really not my intent. The point I tried to make is that
>> we have different priorities because we work on different projects,
which
>> have different requirements. So what may seem like just an annoyance to
>> some might be a huge quality of life and productivity issue for someone
>> else.
>>
>> Louis
>>
>>
>>
>> because for me that outweighs everything else.
>>>
>>
>>> I don't know how to come to a decision here, all I know is that
libc++
>>> needs to get out of the status quo soon. And if the solution is
that
>>> Harbormaster suddenly becomes usable without an unreasonable time
>>> investment from me, then I'm fine with that too. I'm not
looking to switch
>>> to GitHub PRs for the sake of it, I'm looking to solve problems
that are
>>> harming libc++ in the current system.
>>>
>>> Cheers,
>>> Louis
>>>
>>> [1]:
>>>
https://secure.phabricator.com/book/phabricator/article/harbormaster/
>>>
>>>
>>> On Feb 29, 2020, at 23:06, Mehdi AMINI <joker.eph at
gmail.com> wrote:
>>>
>>>
>>>
>>> On Tue, Feb 25, 2020 at 4:19 AM Christian Kühnel via llvm-dev <
>>> llvm-dev at lists.llvm.org> wrote:
>>>
>>>> Hi Louis,
>>>>
>>>> I think this is a good idea. We should start with some local
>>>> experiments where people are willing to try it and figure out
how well that
>>>> works and what does not. Why not allow this for "not
significant" changes?
>>>> They are merged without review today, so we could do them with
reviews (and
>>>> automated tests) via pull requests instead.
>>>>
>>>
>>> I still feel this is only a recipe for confusion if
"some" pull-requests
>>> are accepted on Github but not all. So -1 from me on this.
>>>
>>>
>>>>
>>>> @Mehdi
>>>>
>>>>> - it does not favor to build common tooling: the recent
work on
>>>>> enabling pre-submit CI tests on Phabricator is valuable and
I'm looking
>>>>> forward to get this extended. But splitting the various
ways of
>>>>> contributing to the repo just means more infrastructure to
build to sustain
>>>>> this kind of efforts. (the infrastructure is easier built
on GitHub by
>>>>> the way, but that is an argument in favor of migrating from
Phab to GH for
>>>>> the full-project).
>>>>>
>>>>
>>>> Oh I'm happy to add Github support as soon as someone
switches on PRs.
>>>> This is soooooo much easier to set up and maintain than the
Phabricator
>>>> integration. And we already have builds for the release branch
(
>>>> https://buildkite.com/llvm-project/llvm-release-builds) anyway.
So we
>>>> could easily scale that up. And we can only get pre-merge
testing on
>>>> Phabricator to a certain point, as it's not triggering
builds for ~50% of
>>>> the code reviews.
>>>>
>>>> @Chris Lattner
>>>>
>>>>> Although I am one of the (many) people who would love to
see us move
>>>>> from Phabricator to GitHub PRs, I think it is super
important that we
>>>>> do the transition all at once to keep the LLVM community
together.  I’m
>>>>> already concerned about the fragmentation the discourse
server is causing,
>>>>> e.g. MLIR not using a -dev list.  I’d rather the community
processes stay
>>>>> consistent.
>>>>>
>>>>
>>>> Please allow me to disagree there. IMHO we're way too large
and diverse
>>>> of a project to do binary, overnight transitions.
>>>>
>>>
>>> You seem to be arguing the "how to transition" while
there is no
>>> agreement on a transition happening in the first place.
>>>
>>>
>>>> We're also too large to follow a one-size-fits-all
approach. If we
>>>> agree,
>>>>
>>>
>>> I don't: we went with a monorepo because we believed that the
>>> one-size-fits-all would be more beneficial than splitting, both in
terms of
>>> infrastructure, but also in terms of the practices of the
community, etc.
>>>
>>> Github PRs are the right glow, why take this step-by-step. We
should
>>>> have something like a list of important and supported use
>>>> cases/interactions for the infrastructure. Then we could start
working on
>>>> them one-by-one and figure out if/how they could be implemented
on Github
>>>> and how we could do a smooth transition between these.
>>>>
>>>> If Herald rules are important: Find a way to implement
something
>>>> similar for Github. Maybe there is even a market for such a
tool.
>>>> If transparency is the problem: Find a way to mirror PRs into
>>>> Phabricator, so people can at least see them there.
>>>> We're not restricted to community contributions there. We
can also pay
>>>> someone to build the things we need.
>>>>
>>>
>>> One aspect here though is that we can pay someone to build the
things we
>>> need in Phabricator, we can't change GitHub though.
>>> It was mentioned in the past that we should engage with GitHub and
see
>>> if they would add the feature we're missing to their roadmap,
if it hasn't
>>> been done I'd start there: building up this list of things that
need to
>>> happens before we can agree towards a transition, and engaging with
GitHub
>>> to have these.
>>>
>>> --
>>> Mehdi
>>>
>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200305/08625e6b/attachment.html>