On Mon, Aug 15, 2011 at 9:29 PM, Ian Hinder ian.hinder@aei.mpg.de wrote:
Applying a patch to the Einstein Toolkit requires commonly several
stages:
- A problem or missing feature needs to be identified
- There is a bit of discussion on the mailing list or in TRAC
- A patch is created
- The test cases are run
- A patch is proposed
- The patch is reviewed
- The patch may be modified or updated
- The test cases are run (again), hopefully on several systems
- The committer applies the patch to a clean checkout, maybe runs the
test cases again
- The committer writes a commit message and commits/pushes the patch
This is a bit more complicated if several thorns are involved, if the original patch author doesn't have commit rights, or if the underlying problem only occurs only on a specific system.
I suggest to define a self-contained patch format, probably stealing the idea from git. This format would define the complete patch, possibly for several components, in such a way that a single command (PushComponents) can apply the patch, check for conflicts, create commit messages, and push the commits upstream. Ideally, it would also keep the distinction between author and committer.
Yes - this is a good idea. Do you know if it has been done before? We might be able to re-use an existing format and tools.
Yes, many of these things have been solved by the Android project which is similar to Cactus in that it is comprised of a large number of components pulled from a large number of sources. Their solution was to develop "Gerrit code review" [http://code.google.com/p/gerrit/], a system built on top of git. Their system [https://review.source.android.com/#/q/status:open,n,z] serves as a good example of how this works. This provides a very convenient way of managing the submission and review of patches and also serves as a system for serving git repositories. Along with this, there is the 'repo' tool which is designed to make managing a collection of git repositories easier.
This all relies on the use of git, so I'm not sure whether it would be an option, although any non-git repositories could have git mirrors provided. It also might be a bit too complicated for these purposes, but even then it would still be a good idea to learn from their design choices.
One could imagine a web interface to the build and test system which
accepted an uploaded "super-patch", as described above, and ran the automated tests on a fresh checkout with that patch applied. This could probably be integrated with TRAC. When finished, the user would get an email with the results and a URL to the web report for those results. Perhaps this could also add an automatic comment to the corresponding TRAC ticket. The email would say "OK to apply" if no tests newly failed. The user could then run PushComponents on the patch file, which performs the commit. If we wanted to get very fancy eventually, the test system could sign the commit.
To test whether the test suite returns the expected result, I suggest to keep track in which test cases failures may be temporarily ignored. This will allow us to use this new system right away, even before we have corrected all currently failing test cases. That is, each new commit may only reduce the number of failures.
Do you propose to enforce the system, or just make it easy for people to do the right thing? Would the shame of an email to the ET list every time a test newly failed, along with the list of commits and authors that had changed since the last successful test run, be enough encouragement to avoid the need for enforcement?
In my opinion, even a webpage which shows the test results and is updated after each commit would probably be sufficient. People would be encouraged to check the webpage after they commit a patch and it would be quite clear who was responsible for breaking anything.
Barry