Merge branch 'master' of ssh://git.ikiwiki.info/srv/git/ikiwiki.info
[ikiwiki] / doc / bugs / git_mail_notification_race.mdwn
1 [[done]] (in this branch); fixed removing email notification support!
2
3 I was suprised to receive two mails from ikiwiki about one web edit:
4
5          1   F Oct 30 To joey+ikiwiki update of ikiwiki's plugins/contrib/gallery.mdwn by http://arpitjain11.myopenid.com/
6          1   F Oct 30 To joey+ikiwiki update of ikiwiki's plugins/contrib/gallery.mdwn by http://arpitjain11.myopenid.com/
7
8 The first of these had the correct diff for the changes made by the web
9 edit (00259020061577316895370ee04cf00b634db98a).
10
11 But the second had a diff for modifications I made to ikiwiki code
12 around the same time (2a6e353c205a6c2c8b8e2eaf85fe9c585c1af0cd).
13
14 I'm now fairly sure a race is involved. Note that the name of the modified
15 page is correct, while the diff was not. The git rcs_commit code first
16 gets the list of modified page(s) and then gets the diff, and apparently my
17 other commit happened in the meantime, so HEAD changed.
18
19 Seems that the rcs_notify for git assumes that HEAD is the commit
20 that triggered the ikiwiki run, and that it won't change while the function is
21 running. Both assumptions are bad. Git does no locking and another commit
22 can come in at any time.
23
24 Notice that the equivilant code for subversion is careful to look at $REV.
25 git's post-update hook doesn't have an equivilant of $REV. Its post-receive hook
26 does, but I'm not sure if that's an appropriate hook for ikiwiki to be using
27 for this. Switching existing wikis to use a different hook would be tricky..
28
29 I've avoided part of the race; now the code does:
30
31 1. Get commit info for HEAD commit.
32 2. Extract file list from that.
33 3. Extract sha1 of commit from it too.
34 4. git diff sha1^ sha1
35
36 Still seems like there can be a race if another commit comes in before step #1.
37 In this case, two post-receive hooks could run at the same time, and both
38 see the same sha1 as HEAD, so two diffs might be sent for second commit and no
39 diff for the first commit.
40
41 Ikiwiki's own locking prevents this from happenning if both commits are web
42 edits. At least one of the two commits has to be a non-web commit.
43
44 ----
45
46 A related problem is that if two commits are made separately but then
47 pushed in together, the commit code only looks at the HEAD commit, which
48 is the second one. No notification is sent for the first.
49
50 ----
51
52 Based on all of these problems with using the post-update hook, ikiwiki
53 should be changed to use the post-receive hook, which provides enough
54 information to avoid the assumuptions that led to these problems.
55 Transitioning existing wikis to using a new hook will be interesting. Also,
56 this hook is only present in git >= 1.5.0.7.
57 --[[Joey]]