On Sun, Nov 16, 2008 at 04:05:19PM -0800, Jason Dagit wrote: > On Sun, Nov 16, 2008 at 3:38 PM, David Roundy <[EMAIL PROTECTED]> wrote: > > > Sun Nov 16 18:05:21 EST 2008 David Roundy <[EMAIL PROTECTED]> > > * don't run posthook if the command terminated with exitWith. > > This is a change in policy, and means that for instance darcs pull -a will > > only run the posthook if there are patches to pull. It does mean that we > > have to be careful how we exit commands, since this adds new meaning to > > "exitWith ExitSuccess", but my audit of the code shows that this is only > > used when I wouldn't want to run the posthook anyhow. > > > What is the motivation for this change? As you've described it, it sounds a > bit scary. We now have to be careful about when we 'exitWith ExitSuccess' > and it changes behavior of things like pull as you mention. So the natural > question for me, does this fix a bug? If so, what one? > > If the motivation is purely the case of 'pull' and there not being patches, > then wouldn't a better solution be to have the posthook for pull take the > list of patches as an environment variable and then have the command check > if it should run? This strikes me as a more general and more consistent > behavior (always running the hook on success). Also, think this latter > approach has been asked for in the past several times.
My feeling is that in general, when commands have no effect, the posthook shouldn't be run. This happens in mark-conflicts, unrecord, send, rollback, put, push, pull and apply. (Found by grepping for exitWith ExitSuccess...) In almost all those cases, I think it'd be reasonable to have a posthook, and I would prefer for that posthook not to be run when the command is cancelled (or otherwise has no effect). > > Actually, I found two gratuitous uses of exitWith ExitSuccess, and have > > removed them. > > I wonder if exitWith is something we should check for in haskell_policy? It > does seem like it could make some types of cleanup harder to get right. > Although, bracket would still be safe. I think on the converse, it'd be better to check for use of the catch from the prelude or from System.IO, if possible, or of the bracket defined in System.IO. But that's pretty tricky, I suspect. And, of course, there are times that the prelude catch is exactly what you want, although I doubt anyone ever should use System.IO.bracket. Note that things like ctrl-C and sigKILL are sent as asynchronous exceptions, which have the same cleaning up challenges that exitWith does--specifically that Prelude.catch and System.IO.catch don't catch them. -- David Roundy http://www.darcs.net _______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
