On Sun, Nov 16, 2008 at 4:25 PM, David Roundy <[EMAIL PROTECTED]> wrote:
> 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). Hmm...okay the patch description made it sound like you removed the last of the exitWith ExitSuccess's. So, really this changes the behavior in each of the places you mentioned above. Not just pull. I don't have a strong argument either way so it would be nice if someone who is using posthooks for one of those commands could comment. > > > 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. I'm a bit confused here. I don't see a System.IO bracket: http://www.haskell.org/ghc/docs/latest/html/libraries/base/System-IO-Error.html http://www.haskell.org/ghc/docs/latest/html/libraries/base/System-IO.html http://haskell.org/hoogle Lists a Control.Exception.bracket though. What I've read about bracket in the past is that if it's catchable bracket will catch it. Something like kill -9 wouldn't be catchable, but most everything else is. What is the reason that it shouldn't be used? Just sigINT? I 'raised' a question with Simon Marlow on the GHC mailing list recently about how to properly catch exceptions: http://www.haskell.org/pipermail/glasgow-haskell-users/2008-November/015881.html What I learned is that we'll have to do a bit of work to switch to base4. Thankfully that is a switch we can delay or we could use the compatibility library mentioned in that thread. 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. Then you'll be happy to hear that sigINT is a catchable exception in base4. Thanks, Jason
_______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
