-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix #5809: Run the community-build on every PR #5834
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix #5809: Run the community-build on every PR #5834
Conversation
What about we add |
982dd8c
to
04e0dd5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also left some comments on dotty-staging/pdbp@604c06c since I'd like to see less modifications of the builds of each project.
Why does this rely on git submodules? The community build did not. Adding this many submodules seems like a bad idea. They are a pain to work with and will make cloning the repo super slow... |
@allanrenucci because we don't want to break the community build. Pointing to branches makes it very fragile, commits are more stable. It will also let us document potential changes required to community build to make a PR go tought, rather than having stuff happening silently over +10 repos.
A follow up PR will remove the backend and scala2-lib submodules, which means we will get ride of the submodule requirement for using and testing dotty, and will only need submodules for the community build. In particular we can update drone and the docs to remove the |
@smarter I didn't notice that pdbp upstream uses the sbt-dotty plugin. I think in every other project the plugin was added to make it compile with dotty, so removing the plugin makes the build closer to it's upstream state. If you think it's worth the time I can do another pass to try minimizing the diff of every build... |
Eventually, all upstream projects should compile with dotty, so we shouldn't have to modify their build (especially a modification that removes the plugin and therefore breaks the build). |
When I was maintaining the community build, I thought it was rather convenient. I would rebase the forks every now and then without needing to update the submodules. The community build never broke because a branch changed. We own the forks, we control branch changes. The forks also have their own CI, so as long as you PR changes to them, there is no reason it will break the community build. If you think depending on a specific commit is desirable then I would add a new setting to the tests. E.g. @Test def scalap = test(
project = "scalap",
command = "scalap/compile",
commit = "ff707"
)
// We now run something like `git reset --hard <commit>` Relying on submodule also makes it harder to add a new project to the community. It wouldn't help if we want external contribution to grow the community build. |
Yes the setup was acceptable when its a single person maintaining it and it was not run on every PR, but now I really don't want to have PRs on Dotty being blocked because somebody fucked up something in the community build. Also I really dislike that we do random stuff like this without anyway to relate them to changes in Dotty. Submodules solve both problems.
You are suggesting to have a file with a list of commit per project, this is exactly what submodules are!
It's literally |
499ee95
to
f72e41b
Compare
I'm not very happy with Travis, running time are very unpredictable. For instance, doing |
With the cache setup sequentially running the entire full community build seams to be faster than test_bootstrapped, so I think it's better to stick with drone for now. @smarter Do you want to have a second look? |
community-build/test/scala/dotty/communitybuild/CommunityBuildTest.scala
Outdated
Show resolved
Hide resolved
This commit is meant to replace the infrastructure previously setup in https://github.com/lampepfl/dotty-community-build. A new sbt project, community-build, is added to dotty's Build.scala and contains all the integration tests for the community build. In case of failure the tests will output instructions on how to reproduce the error without JUnit. Hopefully these instructions are sufficient to have everyone fix regressions as they are introduced, or as a very last resort, update the code for the community build. This should be the main improvement over the previous approach, where the community build maintenance was centralized (Allan did all the work) and asynchronous (it's now broken for more than a month...). Another improvement is that a published local version of the sbt plugin is used when running the integration test, so breakage to sbt-dotty should be caught earlier than before.
9fef733
to
5967bff
Compare
This PR is meant to replace the infrastructure previously setup in https://github.com/lampepfl/dotty-community-build. A new sbt project, community-build, is added to dotty's Build.scala and contains all the integration tests for the community build.
In case of failure the tests will output instructions on how to reproduce the error without JUnit. Hopefully these instructions are sufficient to have everyone fix regressions as they are introduced, or as a very last resort, update the code for the community build.
This should be the main improvement over the previous approach, where the community build maintenance was centralized (Allan did all the work) and asynchronous (it's now broken for more than a month...).
Another improvement is that a published local version of the sbt plugin is used when running the integration test, so breakage to sbt-dotty should be cough earlier than before.
Before merging, we should probably generate a new docker image for the CI where we cache all dependencies of the community projects.