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

[David Roundy]

On Sun, Dec 23, 2007 at 02:34:40PM +0000, Ian Lynagh wrote:
> 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.

Ah, I had thought that you'd suggested putting this in _darcs/prefs/, since
it was a per-repository property.  How were you going to handle the
per-repository permissions file that maps the arbitrarily permission tags
onto actual permisions? It would seem a shame to make all users manage this
by hand.

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

This is to change the permissions of a file.  You suggested a patch (I
thought) that created a new permissions type.

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

Well, it depends what you mean by versioned, but it's not versioned
properly anyhow.

> 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.

If we want to dictate this complicated mechanism to our users, I think we
should give them the choice of defining permissions as they like, which
means that they should be forced to manually edit a file in every
repository (and then do what, call darcs fix-prefs?) to make use of this
feature.

> > > > > 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.

It would help some commands much more, and other commands a bit less.  Of
course, how much these differ would depend on the dimensions of one's
repository.
-- 
David Roundy
Department of Physics
Oregon State University