web commit by http://willu.myopenid.com/: Add patch to fix a security FIXME
[ikiwiki.git] / doc / bugs / Monotone_rcs_support.mdwn
1 #Ikiwiki plugin for the Monotone revision control system.
2
3 I've just made a patch to the ikiwiki code that allows it to use the [[rcs/Monotone]] revision control system.  It is available at:
4
5 <http://www.cse.unsw.edu.au/~willu/monotone-ikiwiki.diff>
6
7 At the moment it is basically complete.  At present rcs_notify() is implemeted but untested, the rest is implemented and tested.
8
9 The current version of the patch handles conflicts through the web interface.  It is still not perfect as it will break if there is a rename that conflicts with a web change (but so will the other Rcs plugins I think).  It also commits a revision with conflict markers if there is a conflict requiring such markers... ick.
10
11 Note: This patch requires a rather recent Monotone perl module (18 August 2007 or later).  It is available from the monotone repository here: <http://viewmtn.angrygoats.net/branch/changes/net.venge.monotone>.
12
13 > The setup instructions to add 40 lines of code to monotonerc is pretty frightning stuff.
14 > Is there some way this can be automated? --[[Joey]]
15
16 >> I've committed a bunch of this to monotone so that in future it could be removed.
17 >> I've also just fixed this so it is in a separate, automagically generated, rc file.
18
19 >>> Fair enough. Didn't realize you were a monotone committer. :-)
20
21 >>>> I am, but still a little newish.  Feedback is good.  In particular, this is my first major bit of PERL.
22
23 > Having rcs_commit return a warning message when there's an unresolved conflict
24 > isn't right; that message will populate the page edit box. You might want
25 > to use the error() function here?
26
27 >> It should never reach that case, so I have changed that to error.
28
29 > There'an incomplete comment ending with "note, this relies on the fact that"
30
31 >> erg... sorry, fixed.
32
33 [[tag patch]]
34
35 >> I've [[accepted|done]] this patch, thank you! 
36
37 >>> Thanks for committing it.  I hate keeping my own diffs. :)
38
39 >> I did make a few changes. Please review, and make sure it still works
40 >> (a test case like we have for some of the other RCSes would be nice..)
41
42 >>> Tested.  It still works at least as well as it did.  I'll try to get to a test case soon.
43 >>> In checking the source I noticed a few bogus comments I left in when editing,
44 >>> and a bug in page adding.
45 >>> Here is a small patch for them:
46
47 >>>> applied
48
49 Here is another patch.  It fixes a FIXME you added.  I was using $file within backticks because
50 I was getting an error trying to do it right.  I've figured out the error, and now do it right.  This
51 should also speed things up (very slightly)
52
53     Index: IkiWiki/Rcs/monotone.pm
54     ===================================================================
55     --- IkiWiki/Rcs/monotone.pm (revision 4234)
56     +++ IkiWiki/Rcs/monotone.pm (working copy)
57     @@ -239,17 +239,11 @@
58      
59                 # Something has been committed, has this file changed?
60                 my ($out, $err);
61     -           #$automator->setOpts("-r", $oldrev, "-r", $rev);
62     -           #my ($out, $err) = $automator->call("content_diff", $file);
63     -           #debug("Problem committing $file") if ($err ne "");
64     -           # FIXME: use of $file in these backticks is not wise from a
65     -           # security POV. Probably safe, but should be avoided
66     -           # anyway.
67     -           # At the moment the backticks are used because the above call using the automate
68     -           # interface was failing.  When that bug in monotone is fixed, we should switch
69     -           # back.
70     -           my $diff = `mtn --root=$config{mtnrootdir} au content_diff -r $oldrev -r $rev $file`; # was just $out;
71     -
72     +           $automator->setOpts("r", $oldrev, "r", $rev);
73     +           ($out, $err) = $automator->call("content_diff", $file);
74     +           debug("Problem committing $file") if ($err ne "");
75     +           my $diff = $out;
76     +           
77                 if ($diff) {
78                         # Commit a revision with just this file changed off
79                         # the old revision.
80
81
82 >> BTW, will all the monotone output parsing work if LANG != C?
83
84 >>> It should (he says crossing fingers).
85 >>> In the places where I do any complex parsing I'm using a special
86 >>> version of the mtn commands designed for scripting.  They have a
87 >>> stable, easy to parse, output that doesn't get translated (I think).
88
89 >> Do monotone post-commit hooks actually use REV?
90
91 >>> Monotone post-commit hooks are written in Lua and can do
92 >>> what they please.  Setting the REV environment var before
93 >>> calling Ikiwiki seems reasonable, but I've not written the
94 >>> Lua hook.
95
96 >>>> So the rcs_notify support is not just untested, but can't work
97 >>>> at all w/o further development.