[darcs-devel] darcs patch: added postget to prefs (and 2 more)

[Jason Dagit]

On Aug 3, 2005, at 5:45 AM, David Roundy wrote:

> On Tue, Aug 02, 2005 at 10:26:22AM -0700,  
> dagit at eecs.oregonstate.edu wrote:
>
>> Quoting David Roundy <droundy at darcs.net>:
>>
>>
>>> 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?
>>>
>>
>> darcs init and darcs get would alse be great times to remind the  
>> user.
>> Perhaps:
>>
>
>
>> [Messages from init/get...]
>> Per-repository defaults for darcs commands are stored in the file
>> _darcs/prefs/defaults.  See the darcs manual for more information  
>> [url to
>> relevant section of the manual].
>>
>> I think typically when people start using darcs, init and get are the
>> first things they have to try.
>>
>
> Yeah, that's not a bad idea (although too much verbosity can be  
> annoying).
> We could always *also* do the "author" modification, which would  
> give users
> one more exposure to defaults.

Heh, we could always add a --beginner-mode/--advanced-mode intended  
for use in defaults.  Beginner mode would remind you all the time and  
advanced-mode would say less.  I'm half joking here, as we already  
have tons of command line switches. But you could imagine it being  
somewhat helpful.

>> I wonder, do the verbose options tell the user when a default is  
>> being
>> used from one of the defaults files?  This could be an important  
>> thing to
>> have in the future when people are trouble shooting odd darcs  
>> behavior.
>> People don't see ~/.darcs and _darcs/prefs everyday so it's easy to
>> forget about them and what might be stored there.
>>
>
> Hmmm.  No, the verbose option doesn't have that effect.  I don't  
> think I'd
> want to add it to --verbose itself, since often verbose is used for  
> other
> reasons than debugging.  But we could either support -vv (--verbose  
> twice),
> which could have that effect, or we could add a separate --debug-flags
> command.  I'd lean towards -vv, which is used in other programs,  
> and avoids
> adding a separate flag.  More --verbose means more verbosity! :)  
> But yeah,
> notifying users where the flags come from would be very helpful.

I think, this would also be hard to implement in the current code  
base.  For one thing, when defaults don't parse correctly they are  
thrown away as if they didn't exist.  Although, it still makes a nice  
long term goal.

> The problem with passing a filename is that we'd have to decide on the
> format for that file, which isn't readily extensible.  There are a
> disturbing number of bits of information that might be handy to  
> pass to the
> users (names of patches, name of command being performed, arguments to
> command being performed, flags passed to command being performed,
> repository in which the command was run, what files were modified  
> by the
> patches, hashes of the patches, actual content of the patches,  
> etc), and
> the precise information passed will be different for different  
> commands,
> but one would like a single hook script to work with multiple  
> commands.
> e.g. one would like to be able to write a simple log script that  
> logs every
> darcs command, which ideally would be as simple
> as something like (in defaults):
>
> ALL --post-hook "echo darcs $COMMAND @FLAGS @ARGS >> /path/to/log"
>
> Environment variables allow someone later to pass some new and  
> interesting
> information to the post-hook without breaking older posthook  
> scripts by
> changing the format of a temporary file, and without requiring that  
> hook
> scripts parse a (possibly quite long) file containing this  
> information.

You make a convincing argument for using env variables, but, if I  
understand the env variables correctly they allow for some security  
problems:
1) in your example, what if $COMMAND is expanded in a way that ends  
the call to echo and then injects something malicious?
2) other users on a unix system can see other user's command lines.   
so we'd be asking people to only put non-confidential information in  
patches.

I think if we combine our ideas we'd be okay.  Have darcs always  
write the details to files (one file for each type of detail), and  
then the names of the files are stored in the env variables you  
mention.  This way, no confidential information is in the env, and  
since darcs would have control over the names of the files there is  
no security risk for using them on the command line.  Your example  
would then be:

ALL --post-hook "echo -n \"darcs \"; cat $COMMAND @FLAGS @ARGS >> / 
path/to/log"

Speaking of security, the posthook for apply opens a can of worms if  
people want to place the posthook script under darcs's control.  I  
don't know how often people will realize it's a problem, or how we  
could discourage it.  For one thing, we can point it out in the  
manual and that's probably a good place to raise awareness that if  
your posthook script is versioned then someone could send in a patch  
that writes over it before it runs.

> [snip]
> It seems to me like this would be dependent on the remote defaults  
> stuff
> needed for postget, so I'm thinking we can view it as a motivation to
> plough on through that (including the prompting stuff), with the  
> idea that
> if we get it right, we can leverage that code to implement both the  
> user
> interface and the repository-side interface for inherited  
> defaults.  :)

I think I need to spend some time refining the posthook stuff that i  
already sent in (like trying to get it to be a natural part of each  
command), and sending in some tests and documentation before I work  
on any of this stuff.  But it sounds promising.

> (And this means that we'd be adding a --test-command, --boring-file  
> command
> etc... which would presumably override the contents of _darcs/prefs/ 
> prefs
> and make that file obsolete.)

That would be a nice goal indeed.

Jason