On 2019-10-29 01:21, Reid Kleckner via llvm-dev wrote:> At the dev meeting I heard Doug Gregor say something like, "what kind of > dirty animals are you, you just push directly to master!?" Based on > that, I think other communities may set up workflows where they push > branches to places, and some automation rebases and updates master > asynchronously, optionally conditioned on some (light) testing or approval.I think in most open source projects the workflow is to create a pull request. Then the CI infrastructure will build and test that PR which is hopefully up to date with the master branch. When that passes the tests and review it will be merged into master. Then the CI infrastructure builds and tests it again. In the mean time some changes might have been made to master. In my experience it's rare that a PR that passes breaks something when it gets merged. But most projects are not as large as LLVM. So yes, one definitely needs to rely on the CI infrastructure. In my experience it's rare that a contributor will run the tests on another platforms than the contributor's main development platform. The CI infrastructure for the D programming language works a bit different (a quite large project but not as large as LLVM). The CI infrastructure will pull down the PR and the latest master and merge that and then run all the builds and tests. If a commit has been made to master it will invalidate the test results of all existing PRs. If a PR is too old it won't be tested again (until someone makes a new commit to that PR). This is to avoid old inactive PRs to delay new active ones. But nothing will automatically rebase existing PRs. With the monorepo there will be more traffic/commits but it will also be less likely that a new commit will break something because it might be to completely different project. For example, if you work on some changes to the compiler (LLVM or Clang) and someone else makes a commit to libc++. It's less likely that that commit will break my changes. After a while one gets a feeling on what's likely and less likely to break something. -- /Jacob Carlborg
> On Nov 3, 2019, at 7:37 AM, Jacob Carlborg via llvm-dev <llvm-dev at lists.llvm.org> wrote: > > On 2019-10-29 01:21, Reid Kleckner via llvm-dev wrote: > >> At the dev meeting I heard Doug Gregor say something like, "what kind of dirty animals are you, you just push directly to master!?" Based on that, I think other communities may set up workflows where they push branches to places, and some automation rebases and updates master asynchronously, optionally conditioned on some (light) testing or approval. > > I think in most open source projects the workflow is to create a pull request. Then the CI infrastructure will build and test that PR which is hopefully up to date with the master branch.For each pull request if it merges cleanly GitHub will automatically create (and keep up to date) a ref (refs/pull/${pull-request-id}/merge). Generally speaking PR test builders target that ref instead of the PR head so that it ensures that the tests are running against an up-to-date head.> When that passes the tests and review it will be merged into master. Then the CI infrastructure builds and tests it again. In the mean time some changes might have been made to master. In my experience it's rare that a PR that passes breaks something when it gets merged. But most projects are not as large as LLVM.For a project as large as LLVM (and even many much smaller) there is a balance between how much testing you can do on a PR without disrupting development and how long that testing takes. There are some LLVM testing configurations that take many hours to run, and if every change was validated by those tests development would slow to a halt. If LLVM were to adopt a PR testing workflow we would likely need to come up with a timeframe in which PR tests are expected to complete, and that would limit what testing we would be able to do pre-merge.> > So yes, one definitely needs to rely on the CI infrastructure. In my experience it's rare that a contributor will run the tests on another platforms than the contributor's main development platform.This varies with LLVM based on the scope of the change. My main development platform is macOS, but I have frequently needed to test changes on other platforms because I was working in platform-specific layers in LLVM.> > The CI infrastructure for the D programming language works a bit different (a quite large project but not as large as LLVM). The CI infrastructure will pull down the PR and the latest master and merge that and then run all the builds and tests. If a commit has been made to master it will invalidate the test results of all existing PRs. If a PR is too old it won't be tested again (until someone makes a new commit to that PR). This is to avoid old inactive PRs to delay new active ones. But nothing will automatically rebase existing PRs. > > With the monorepo there will be more traffic/commits but it will also be less likely that a new commit will break something because it might be to completely different project. For example, if you work on some changes to the compiler (LLVM or Clang) and someone else makes a commit to libc++. It's less likely that that commit will break my changes. After a while one gets a feeling on what's likely and less likely to break something.I think there is a lot of balancing act we're going to have to figure out here. If we invalidate the test results after every merge that could cause a huge backlog of unrelated changes to require re-testing. One thing we would likely need in any PR testing strategy is a way to determine which project(s) to build and test. Since the mono-repo has all projects together, and many of them are largely decoupled the PR testing infrastructure would need to take that into account. For example, a change to libcxxabi shouldn't need LLVM to build and pass tests, but it should probably build and test libcxx. -Chris> > -- > /Jacob Carlborg > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
(resending with the list cc restored) Jacob Carlborg wrote:> I think in most open source projects the workflow is to create a pull > request. Then the CI infrastructure will build and test that PR which is > hopefully up to date with the master branch. When that passes the tests > and review it will be merged into master. Then the CI infrastructure > builds and tests it again. In the mean time some changes might have been > made to master. In my experience it's rare that a PR that passes breaks > something when it gets merged. But most projects are not as large as LLVM.Remember that SVN inherently did something like a rebase on every commit (because SVN does not require the entire checkout to be up-to-date, only the files touched by the commit). I think it was rare for this to be the source of a problem, and I would expect a PR model with automatic rebase generally would not have a problem either.> So yes, one definitely needs to rely on the CI infrastructure. In my > experience it's rare that a contributor will run the tests on another > platforms than the contributor's main development platform.This is more likely to be an issue, however it is not a new issue and I doubt that a PR model would have any detectable effect on the frequency. We already rely on CI to find these things, and without actually taking any statistics, my impression is that CI finds something nearly every day. We fix it or revert, and move on. --paulr
On 2019-11-03 18:42, Chris Bieneman via llvm-dev wrote:> For a project as large as LLVM (and even many much smaller) there is a balance between how much testing you can do on a PR without disrupting development and how long that testing takes. There are some LLVM testing configurations that take many hours to run, and if every change was validated by those tests development would slow to a halt.Perhaps not the full test suite needs to be run on every PR.> I think there is a lot of balancing act we're going to have to figure out here. If we invalidate the test results after every merge that could cause a huge backlog of unrelated changes to require re-testing. One thing we would likely need in any PR testing strategy is a way to determine which project(s) to build and test. Since the mono-repo has all projects together, and many of them are largely decoupled the PR testing infrastructure would need to take that into account. For example, a change to libcxxabi shouldn't need LLVM to build and pass tests, but it should probably build and test libcxx.Yeah, that's a downside with the mono-repo approach. I still don't understand why that was chosen. -- /Jacob Carlborg