Re: metastore

[david@lang.hm]

some of this duplicates thoughts from other messages in this thread. 
apologies for the duplication, but I want to be clear the response to 
Junio's concerns here as well

On Sun, 16 Sep 2007, Junio C Hamano wrote:

> david@lang.hm writes:
>
>> git has pre-commit hooks that could be used to gather the permission
>> information and store it into a file.
>>
>> git now has the ability to define cusom merge strategies for specific
>> file types, which could be used to handle merges for the permission
>> files.
>> ...
>> There are some significant advantages of having the permission store
>> be just a text file.
>>
>> 1. it doesn't require a special API to a new datastore in git
>>
>> 2. when working in an environment that doesn't allow for implementing the
>>    permissions (either a filesystem that can't store the permissions or
>>    when not working as root so that you can't set the ownership) the file
>>    can just be written and then edited with normal tools.
>>
>> 3. normal merge tools do a reasonable job of merging them.
>>
>> however to do this git would need to gain the ability to say 'this
>> filename is special, it must be checked out before any other file is
>> checked out' (either on a per-directory or per-repository level)
>
> I'd rather not implement it at such a low level where a true
> "checkout" happens.  For one thing, I am afraid that the special
> casing will affect the normal codepath too much and would make
> it into a maintenance nightmare.

as I understand it, at this point you already choose between three 
options.

1. write to a file (and set the write bit if needed)
2. write to stdout
3. write to a pager program

I am suggesting adding

4. write to a .gitattributes defined program and pass it some parameters.
    (and only if the .gitattributes tell you to)

this should be a very small change to the codepath

or am I missing something major here?

if this program can get the contents of the permission file out of the 
index, then the requirement I listed before to make sure the permission 
file gets written before anything else goes away, and the only requirement 
left is the ability to specify a different write method

> But more importantly, if you
> are switching between commits (this includes switching branches,
> checking out a different commit to a detached HEAD, or
> pulling/merging updates your HEAD and updates your work tree),
> and the contents of a path does not change between the original
> commit and the switched-to commit, you may still have to
> "checkout" the external information for that path if your
> "permission information file" are different between these two
> commits.  To the underlying checkout aka "two tree merge"
> operation, that kind of change is invisible and it should stay
> so for performance reasons, not to harm the normal operation.

I had not thought of this condition.

however, I think this may be easier then you are thinking

we have two conditions.

1. the permission file hasn't changed.

Solution:  do nothing

2. the permission file has changed

Solution: set all the permissions to match the new file

this could be done by useing .gitattributes to specify a different program 
for checking out the permission file, and that program goes through the 
file and sets the permssions on everything. yes this is a bit inefficiant 
compared to diffing the two permission files and only touching the files 
that have changed, but is the efficiancy at this point that critical? if 
so then instead of feeding the program the contents of the new file you 
could feed it the diff between the old and the new file.

in theory you could do this for any file, and it would be a win for some 
files (a large file that has a few changes to it would possibly be more 
efficiant to modify in place then to re-write), but I'm not sure the 
results would be worth the complications. if .gitattributes gains the 
ability to specify the program to be used to write the file, it could also 
gain the ability to specify feeding that file the diff instead of the full 
contents.

the one drawback to just setting all the permissions is that this will 
overrule any local changes to files that weren't otherwise modified. how 
big of a problem is this?

> IOW, I do not want the core level to even know about the
> existence of "permission information file", even the code that
> implements it is well isolated, ifdefed out or made conditional
> based on some config variable.

nobody is suggesting anything that wouldn't be at least conditional based 
on some config variable.

> I however think your idea to have extra "permission information
> file" is very interesting.  What would be more palatable, than
> mucking with the core level git, would be to have an external
> command that takes two tree object names that tells it what the
> old and new trees our work tree is switching between, and have
> that command to:
>
> - inspect the diff-tree output to find out what were checked
>   out and might need their permission information tweaked;
>
> - inspect the differences between the "permission information
>   file" in these trees to find out what were _not_ checked out,
>   but still need their permission information tweaked.
>
> - tweak whatever external information you are interested in
>   expressing in your "permission information file" in the work
>   tree for the paths it discovered in the above two steps.
>   This step may involve actions specific to projects and call
>   hook scripts with <path, info from "permission information
>   file" for that path> tuples to carry out the actual tweaking.

this is an area I wasn't aware of, but it doesn't seem that difficult to 
do. the issue (as I address above) is if this needs to be done as a diff 
or if it can be done simply by setting all the permissions according to 
the new file.

> If we go that route, I am not deeply opposed to add code to
> Porcelains to call that new command after they "checkout" a new
> commit at the very end of their processing (namely, git-commit,
> git-merge, git-am, and git-rebase).

this is saying you want a wrapper around git instead of a hook in git.

> Yes, I am very well aware that somebody already mentioned "there
> is a window between the true checkout and permission tweaking".
> If you need to touch the core level in order to close that
> window, I am not interested.

no matter how small the change? (see the above comments) If so this 
converstion isn't worth continuing, if you are just concerned about 
maintainability and are willing to consider small changes that won't cause 
big maintinance problems then we can continue to discuss if the changes I 
am suggesting are small enough. the need to be able to close the 
vunerability window is a showstopper to many uses.

>> The ability to handle /etc comes up every few months. it's got to be
>> the most common unimplemented request git has seen.
>
> Asking a pony for many times does not necessary make it the
> right for you to have the pony.  The sane way to implement this
> is in your Makefile, as Randal and other people with more
> experience have already pointed out, and I happen to agree with
> them.

you don't always have a makefile. if other tools that you use make 
modifications to the files in the locations where they reside, having to 
pull those changes back before you can do a checking is a complication as 
well

> My gut feeling is that the approach to use an external hook that
> reads your "permission information file" could be done with
> negligible impact to the normal operation of git.  I suspect
> that the "new command" I suggested above that would run after
> "checkout" actions would perform what people need to do in their
> Makefiles' "install" rules (if they have the work tree vs target
> tree distinction), or "post-checkout" rules (if they want to use
> the work tree in-place), and not having to write/reinvent a
> Makefile target for this in every project would hopefully make
> it easier to use.  That is the only reason I am writing this
> message on this topic.

but you are not willing to allow the hook to be created, you are saying 
that there would need to be an external wrapper instead.

at this point it appears that having a hook to be able to specify external 
programs at a point where you are already deciding between different 
options would be sufficiant.

>> so would changes like this be acceptable?
>
> That is a different question.  Is having an extention to help
> people who want to manage perm bits a worthy goal?  Perhaps, but
> it depends.  Is it worthy enough goal to complicate the really
> core parts of the code and add huge maintenance burden?
> Absolutely not.  Can it be made in such a way that it does not
> have much impact to the core parts?  We need to see how it is
> done.

this is why I was asking about this approach. do changes like this seem 
small enough to be worth the effort of coding and submitting?

David Lang