I've started reviewing this, and the main thing I don't like is the post-commit wrapper wrapper that ikiwiki-makerepo is patched to set up. That just seems unnecessarily complicated. Why can't ikiwiki itself detect the "cvs add " call and avoid doing anything in that case? --[[Joey]] > The wrapper wrapper does three things: > > 7. It ignores `cvs add `, since this is a weird CVS > behavior that ikiwiki gets confused by and doesn't need to act on. > 7. It prevents `cvs` locking against itself: `cvs commit` takes a > write lock and runs the post-commit hook, which runs `cvs update`, > which wants a read lock and sleeps forever -- unless the post-commit > hook runs in the background so the commit can "finish". > 7. It fails silently if the ikiwiki post-commit hook is missing. > CVS doesn't have any magic post-commit filenames, so hooks have to > be configured explicitly. I don't think a commit will actually fail > if a configured post-commit hook is missing (though I can't test > this at the moment). > > Thing 1 can probably be handled within ikiwiki, if that seems less > gross to you. >> It seems like it might be. You can use a `getopt` hook to check >> `@ARGV` to see how it was called. --[[Joey]] >>> This does the trick iff the post-commit wrapper passes its args >>> along. Committed on my branch. This seems potentially dangerous, >>> since the args passed to ikiwiki are influenced by web commits. >>> I don't see an exploit, but for paranoia's sake, maybe the wrapper >>> should only be built with execv() if the cvs plugin is loaded? >>> --[[schmonz]] > Thing 2 I'm less sure of. (I'd like to see the web UI return > immediately on save anyway, to a temporary "rebuilding, please wait > if you feel like knowing when it's done" page, but this problem > with CVS happens with any kind of commit, and could conceivably > happen with some other VCS.) >> None of the other VCSes let a write lock block a read lock, apparently. >> >> Anyway, re the backgrounding, when committing via the web, the >> post-commit hook doesn't run anyway; the rendering is done via the >> ikiwiki CGI. It would certianly be nice if it popped up a quick "working" >> page and replaced it with the updated page when done, but that's >> unrelated; the post-commit >> hook only does rendering when committing using the VCS directly. The >> backgrounding you do actually seems safe enough -- but tacking >> on a " &" to the ikiwiki wrapper call doesn't need a wrapper script, >> does it? --[[Joey]] >>> Nope, it works fine to append it to the `CVSROOT/loginfo` line. >>> Fixed on my branch. --[[schmonz]] > Thing 3 I think I did in order to squelch the error messages that > were bollixing up the CGI. It was easy to do this in the wrapper > wrapper, but if that's going away, it can be done just as easily > with output redirection in `CVSROOT/loginfo`. > > --[[schmonz]] >> If the error messages screw up the CGI they must go to stdout. >> I thought we had stderr even in the the CVS dark ages. ;-) --[[Joey]] >>> Some messages go to stderr, but definitely not all. That's why >>> I wound up reaching for IPC::Cmd, to execute the command line >>> safely while shutting CVS up. Anyway, I've tested what happens >>> if a configured post-commit hook is missing, and it seems fine, >>> probably also thanks to IPC::Cmd. >>> --[[schmonz]]