[darcs-devel] Any answers for IsiSetup's concerns?

[Ian Lynagh]

On Sun, Dec 23, 2007 at 08:28:03AM -0500, David Roundy wrote:
> On Sun, Dec 23, 2007 at 12:15:35PM +0000, Ian Lynagh wrote:
> > On Thu, Dec 20, 2007 at 07:10:44AM -0500, David Roundy wrote:
> > > On Thu, Dec 13, 2007 at 06:53:07PM +0000, Ian Lynagh wrote:
> > > 
> > > This sounds scary to me.  It looks like it'd have all the problems that
> > > setpref has, and I'd rather not repeat that mistake (or at least keep using
> > > the same implementation of that mistake).
> > 
> > The only problems with setpref I can think of are caused by the details
> > of its implementation, not anything fundamental.
> 
> Yes, the trouble is that we only keep one copy of the prefs, rather than
> one pristine copy + one local copy.  I believe that's what you were
> proposing to do with this permissions-mapping file.

The pristine copy is _darcs/whatever, and users should never edit this.

When they want to change it, they "darcs setperm foo bar" and this puts
something in _darcs/patches_pending.

This is similar to how directory creation is handled. The problem with
setpref is that it isn't versioned at all, as I recall.

The one thing I'm not sure of is the best way to show the situation if a
conflict happens, as there isn't an easy way to present an arbitrary
number of conflicting alternatives. One option would be to put them all
(in an arbitrary order) into _darcs/patches/pending, but it would
probably be nicer to have a different command for doing this (and if
setprefs become versioned then this command could also be used for
setprefs).

> in which you propose to implement yet another patch type that modifies
> user-preference data of which we don't keep a pristine copy.  This is
> precisely the "details of implementation" that causes setpref to be such a
> pain, and precisely why I didn't like your suggesting we repeat this
> mistake.  The result is a set of patches that must always trivially
> commute, and which manage data that isn't actually managed.  It's
> horrible.

I agree that that is horrible, but I don't think we should do it that
way. setpref is slightly more subtle, as in that case local settings can
sometimes make sense, though.

> > > I'd say that (if we went with this idea) we should not define this command,
> > > but instead use the existing setpref mechanism, perhaps to work like
> > > boring, where they can point the permissions at a file of their choice
> > > (e.g. one in the repository).
> > 
> > Which file? The mapping of file to permission class could be a file in
> > the repo, but the mapping of permission class to actual permissions is
> > per-machine.
> 
> No, it *could* be per-machine,

If it's not per-machine then we can just set permissions directly, but I
don't think that we want that.

> > > > You could then "darcs setpermissions executable myScript; darcs rec".
> > > > The permission group for a file would have to be stored under _darcs
> > > > somwehere. "darcs rec" could either warn if you change the permissions
> > > > with chmod, and it could even try to guess which group you wanted it in.
> > > 
> > > This is the problem with your approach.  Because we're now storing "extra"
> > > information, we are required to implement an extra database.
> > 
> > That is true, but don't we have plans to do that for other reasons too?
> > e.g. a mapping from FilePath to [PatchName], so we know which patches
> > affect a file without having to read them all, for optimising "darcs
> > changes filename"?
> 
> Not that I'm aware of.  I thought the mapping was going to be from
> PatchName to [FilePath].

Hmm, that would help a bit, but not as much.

> The difference is that this would be done as an
> optional optimization, while what you're proposing would be required.

I don't really see what difference that makes. The code has to be
written regardless of whether or not it's optional. In fact, if it's
required then the code is simpler as there is only one case (except that
we'd need a transition period).


Thanks
Ian