On Mon, Aug 01, 2005 at 10:17:51AM -0700, [EMAIL PROTECTED] wrote: > Quoting David Roundy <[EMAIL PROTECTED]>: > > > Finally, the rationale for this feature is that now I can have a > > > "central" repository with a special test defined (one that sends out > > > email notification for successful patches), and I can use postget to > > > set a different test for the copies of the repository. So, when > > > someone gets a copy of the repo, the postget command will be run and > > > it will modify test so that it no longer sends out email notification. > > > > I'd rather support this feature with a hook to apply than with a hook to > > get. In spite of the fact that I created it, I don't care for the whole > > prefs and setprefs mechanism--it has too many potentially confusing > > features (see, for example, my response to your doc fix). > > > > I'd like most hooks (and get here is a special exception, since the > > repository in question doesn't exist until after the command is run) to be > > implemented as simple command-line flags that would normally be specified > > in either _darcs/prefs/defaults or ~/.darcs/defaults. Thus a post-hook > > in apply would quite naturally allow you to do your email notifications in > > the central repo. > > I've been using darcs for a while now and somehow completely missed > defaults.
This is a problem we've run into before. defaults is one of the nicer features of darcs, but noone seems to find out about it on their own. Do you have any suggestions where in the docs or on the website would be a good place to point to it? I'm not really sure where most new users look for information... Perhaps even something as simple as making the message asking for author name (when you first record in a new repository if you don't have EMAIL defined)... we could place the user input in _darcs/prefs/defaults, and instruct the user to edit it there if they change their mind? > I just checked with the documentation, and this looks like the right > place. And I agree that a post-hook in apply is cleaner if we use > defaults. At least for my target usage. How should a post-hook in apply > interact with test? I guess you make the assumption that by the time you > get to the post-hook that the test has suceeded? Perhaps it's best to > have three hooks: post-apply-test-success, post-apply-test-fail, > post-apply-always. In may situation I would only want/need > post-apply-test-success. Yeah, potentially three post-hooks would be reasonable. We *could* omit the -always and just require that the user specify the same hook for both success and failure if they want an "always" hook. I wouldn't include "apply" in the flag, so we can use the same flag for many operations. And I'd like to see the hook-response code in the generic darcs code, in DarcsCommands. We can process the hooks in consider_running. We'd have to catch exceptions with Control.Exception.catch to find out whether the command exited successfully. I'd start with just a success command (which is what you want right now anyways), since the failure command could run into excitement depending on the reason for the failure, and I'd just name it --post-hook, and let the success be implied. We can call the other --post-hook-failure. > The next question on my list, is how do you pass context to the the > command? Since I want to use it for email notifications, it would be > nice to have the patch summaries and a list of files modified sent in the > email. I'd pass information via environment variables. It's an easy-to-extend mechanism that doesn't require that the hook know much. This could be interested, since I wanted the hook code itself to be generic, but the information you pass most certainly won't be generic. I *think* you can define the environment variables in apply and they'll naturally be inherited by the hook called outside apply, but this isn't something I have much experience with. In giving information like file lists, you'll have to be careful that you don't mess up any laziness properties... and probably you'll want to check the arguments for PostHook before doing any environment variable definition, just to be a bit more efficient in the common case where there's no post hook. In case you're wondering why I want the post-hook code outside of Apply (maybe it's already obvious...) the idea is that if we do it that way, the only change needed to support a post-hook in a new command is to import the post_hook DarcsOption and include it in our list of flags. Also, the reason for making the flag name the same for all commands is so users can define a generic post-hook if they like, with the ALL mechanism of defaults. > > That said, I can imagine a few other uses of postget. One would be to > > fix permissions (or extended attributes, or anything like that). > > Another would be to to run autoconf or something as an aid to lazy or > > ignorant users. One could also use this as a chance to set up > > customized defaults... for example to set up the _darcs/prefs/defaults > > to run a permissions-fixing hook after each pull. > > Your last sentence is exactly how I had planned on using it. I'm not > sure how effective a postget command would be for fixing permissions. It > would work for the first 'get', but subsequent pulls would not run the > script and so newly added files would not have the 'fixed' permissions. > At this point I'm realizing two things 1) Post apply hooks would be > better for me 2) This is a nice feature and we should still have it. Yeah, postget would be a nice feature, but it's much harder than any other sort of post-hook (even harder than a post-init!). So it makes sense to wait a bit to see the design issues that come up in implementing the easier post-hooks. > > Actually, as far as your reason for postget is concerned, I think we'd be > > better of fixing the bug of local gets, that they copy locally modified > > prefs. The reason they do that is because there's no way to tell whether > > the pref has been locally modified without parsing all the patches, which > > is something we'd really like to avoid (for efficiency reasons). I think > > the solution would be to split the prefs into two files, one for local > > (overriding) modifications, and one for the prefs set with setpref itself. > > Then we'd only copy the latter on a local darcs get. I'm envisioning > > something like: > > > > # contains the "gettable", "global" versions of prefs --not user-modifiable: > > _darcs/setprefs > > # where the user sets "local" versions of the prefs: > > _darcs/prefs/prefs > > I don't like the naming scheme, it doesn't help me to realize that one of > them is not user modifiable. BTW, how will you make sure people don't > modify it? mark it read-only and assume they behave? I'm also not sure > if having two places with your settings is worth the confusion to the > user. Given the current set of prefs, what would be in each? I think > using defaults to store the overriding values would be cleaner. > Otherwise, it may appear to the user that their prefs are stored in 3 > files without a clear reason as to what should be where. Perhaps I'm > missing something? The idea (and perhaps it's not clear to users) is that anything in _darcs/prefs is user-editable, and anything else in _darcs must not be touched by users. We could make the user-uneditable one _darcs/dont_touch_these/prefs, or something similarly clear. We can't be sure that users won't edit their repository information, but if they do (and you're right that setprefs isn't a good name), it's their own fault. Defaults and "prefs/prefs" are very different objects... and I don't much like the latter, although I don't know of a better way of implementing such features. "prefs" is used for properties that need to be pulled from one repository to another, which defaults is private. The trouble is that users do need to be able to override prefs, which is why ideally we'd have prefs be in two places. > > I think this is two cleaner solutions to your problem (either a post-hook > > on apply, or fixing the local get prefs bug). The latter is a good idea > > anyways, since it makes local gets behave like remote gets. It also could > > allow us to make optimize --checkpoint somewhat more efficient, since we > > could trust that _darcs/setprefs (or _darcs/global_prefs?) hasn't been user > > modified, so we wouldn't have to search through the entire history looking > > for setpref patches. > > Sounds like the post-hook in apply should be implemented, and something > needs to be done with _darcs/prefs/prefs, but I'm not sold on the idea > you present here. I also think that this postget hook is still a good > idea, given the modifications to where the settings are stored that we > talked about above in the first half the email. I agree (see above). Indeed, something ought to be done about prefs/prefs, but there's no urgency... we've gotten along quite well with the current interface, so we shouldn't modify it until there's a consensus that the modification would be an improvement. -- David Roundy http://www.darcs.net _______________________________________________ darcs-devel mailing list [email protected] http://www.abridgegame.org/cgi-bin/mailman/listinfo/darcs-devel
