The hgpoller picks up the wrong changeset with the new pushlog: I pushed two commits at once, 3590fdfcfe26 and 6d88fe34dc34. The new pushlog shows the first changeset pushed, not the second one, which causes the buildbot to build the wrong one. This could be pretty bad when pushing large branches. http://hg.mozilla.org/mozilla-central/index.cgi/pushloghtml I think the pushlog hook really needs to record the last changeset pushed, not the first one, the way it used to.
Are you sure the pushlog ever worked this way? The documentation says that the node parameter is "The changeset ID of the first changeset in the group that was added. All changesets between this and tip, inclusive, were added by a single “hg pull”, “hg push” or “hg unbundle”." See http://hgbook.red-bean.com/hgbookch10.html section 10.9.1 We could certainly change the pushlog to record all the changesets pushed in the pushlog if that makes more sense.
Yes, because it didn't use "node", it used "tip": http://hg.mozilla.org/users/bsmedberg_mozilla.com/hghooks/index.cgi/file/e01e14e32151/hg_record_changeset_info
It's pretty bad if you ever push with a merge changeset, because it picks up the first one (pre-merge), which means you're effectively running with the merge-inducing "trunk" changes backed out! And then we don't notice and start another build later with the right changeset, so you basically need to always push a single changeset at a time right now for build-on-commit to be useful. I discovered this the hard way, did not enjoy it, please let me know if I can help rather than starting to litter my repo with spurious build-inducing commits!
I am probably halfway to fixing this. Because my changes to the pushlog changed the semantics, I've rewritten the convert-pushlog-db script we used to import from the flatfile to the db. It will now migrate the contents of the db to a new schema, and read the old pushlog to know which pushes were logged with which system. The new db schema is going to have two tables, a pushlog table that logs (user, date), and a changesets table that logs (pushid, node) for *every* changeset pushed with this push. This will make some code in the pushlog web interface easier to deal with as well.
Created attachment 329087 [details] [diff] [review] pushlog revamp, redux Ok. This rewrites the pushlog converter script and the pushlog hook as described in my previous comment. I've tested it on a test repo with a combination of groups of changesets and individual changesets being pushed in both the old pushlog and the new pushlog, and it appears to do the right thing. (Correctly stores all changesets in the db grouped correctly into pushes.) The modified pushlog hook logs each push and all associated changesets, to avoid any ambiguity here.
Comment on attachment 329087 [details] [diff] [review] pushlog revamp, redux Hrm, I'd love a testcase... e.g. a shell script or python script which creates a new repo, pushes things to it, and runs sqlite3 to verify that the data is what we expect.
Yeah, I've done some manual testing, I can create an automated test, although you have to have the module installed first.
Could the manual test munge PYTHONPATH so you can test with the local version?
Created attachment 329548 [details] [diff] [review] [checked in] hook/converter changes, with tests Ok, this adds a test for the hook, and a test for the converter script. I modified the schema slightly to include the revision number in the changesets table, for easier ordering.
Comment on attachment 329548 [details] [diff] [review] [checked in] hook/converter changes, with tests Pushed in changeset 17acc0151c12
Created attachment 329681 [details] [diff] [review] pushlog-feed changes Here's the corresponding changes to the pushlog-feed extension. It also adds browsing forwards/backwards on the pushloghtml page.
Created attachment 329682 [details] [diff] [review] template changes I changed pushloghtml to use a template so it's less broken. This patch contains the template changes.
Created attachment 329717 [details] [diff] [review] slightly tweaked template changes I changed the JS-date stuff a little bit in the template, I think it might be confusing to do that by default, especially when I get around to adding query-by-date.
Aravind: are you interested in reviewing these changes, or should I ask bsmedberg? (Or does anyone really care?)
(In reply to comment #14) > Aravind: are you interested in reviewing these changes, or should I ask > bsmedberg? (Or does anyone really care?) > Nope.. I am cool if it works for you guys. One Q. though, do I need to do any back processing to fix/convert the old sqlite dbs? Or is this just a fix in the pushlog-feed thingy?
This is going to require running the converter script again. I discovered that they way I was logging data in the new pushlog was inconsistent with the old pushlog, hence this bug. We'll need to update the hook script, run the converter, and then update the templates and the pushlog-feed extension. I'll push all these changes out, then file an IT bug for you for deployment.
Pushed the other changes: http://hg.mozilla.org/users/bsmedberg_mozilla.com/hgpoller/index.cgi/rev/646e1437fccf http://hg.mozilla.org/index.cgi/hg_templates/rev/b9416f6d97ce
So I wish this problem had been publicized more broadly and/or fixed faster. I wasted a good bit of time on bug 363706 (see bug 363706 comment 44 through bug 363706 comment 57) because I didn't know that this was happening when multiple changesets were pushed together.
I think you should announce this on dev-planning (or someplace else appropriate) so that anyone else who backed out changes due to unexplained unit test failures caused by this bug knows that this is a possible cause of those failures (and that it might be safe to repush the changes in question one changeset at a time).
Filed bug 445399 on IT deployment of this code to hg.mozilla.org. dbaron: I'm really sorry about that. I wish this had been discovered earlier, or I had had more time to fix it after it was discovered. Hopefully we can get this deployed soon and stop wasting developer's time. I'll post something to dev.planning if you think that'd be useful.
I should have raised the severity when I realized what it was causing, too. A post to dev.planning would be good.