Re: [PATCH 2/2] Add keyword unexpansion support to convert.c

Nicolas Pitre [off-list ref] writes:

quoted

I would like to, however this doesn't currently integrate
well with git. I've been told in the past that once
.gitattributes is in place then the hooks for the crlf stuff
can be generalized to allow for calls out to custom code to
do this sort of thing.

And I agree that this is a perfectly sensible thing to do.  The facility 
should be there for you to apply any kind of transformation with 
external tools on data going in or out from Git.  There are good and bad 
things you can do with such a facility, but at least it becomes your 
responsibility to screw^H^H^H^Hfilter your data and not something that 
is enforced by Git itself.

You have to be careful, though.  Depending on what kind of
transformation you implement with the external tools, you would
end up having to slow down everything we would do.

It boils down to this statement from Andy:

    ..., keywords (in other VCSs, and so why not in git) are
    only updated when a file is checked out.  There is no need
    to touch every file.  It's actually beneficial, because the
    keyword in the file is the state of the file at the time it
    was checked in - which is actually more useful than updating
    it to the latest commit every time.

    That means you're only ever expanding in a file that your
    changing anyway - so it's effectively free.  git-checkout
    would still be immediate and instantaneous.

Back up a bit and think what "when a file is checked out" means.
His argument assumes the current behaviour of not checking out
when the underlying blob objects before munging are the same.

But with keyword expansion and fancier "external tools" whose
semantics are not well defined (iow, defined to be "do whatever
they please"), does it still make sense to consider two blobs
that appear in totally different context "the same" and omit
checking out (and causing the external tools hook not getting
run)?  I already pointed out to Andy that the branch name the
file was taken from, if it were to take part of the keyword
expansion, would come out incorrectly in his printed svg
drawing.

If you want somebody's earlier example of "giving a file with
embedded keyword to somebody, who modifies and sends the result
back in full, now you would want to incorporate the change by
identifying the origin" to work, you would want "$Source$" (I am
looking at CVS documentation, "Keyword substitution/Keyword
List") to identify where that file came from (after all, a
source tree could have duplicated files) so that you can tell
which file the update is about, and this keyword would expand
differently depending on where in the project tree the blob
appears.

It is not just the checkout codepath.  We omit diffs when we
know from SHA-1 that the blobs are the same before decoration.
We even omit diffs when we know from SHA-1 that two trees are
the same without taking possible decorations that can be applied
differently to the blobs they contain into account.  Earlier,
Andy said he wanted to grep for the expanded text if he is
grepping in the working tree, and I think that makes sense, but
that means git-grep cannot do the same "borrow from working tree
when expanding from blob object is more expensive" optimization
we have for diff.  We also need to disable that optimization
from the diff, regardless of what the correct semantics for
grepping in working trees should be.

I suspect that you would have to play safe and say "when
external tools are involved, we need to disable the existing
content SHA-1 based optimization for all paths that ask for
them" to keep your sanity.

On Tue, 17 Apr 2007, Junio C Hamano wrote:

Nicolas Pitre [off-list ref] writes:

quoted

quoted

I would like to, however this doesn't currently integrate
well with git. I've been told in the past that once
.gitattributes is in place then the hooks for the crlf stuff
can be generalized to allow for calls out to custom code to
do this sort of thing.

And I agree that this is a perfectly sensible thing to do.  The facility 
should be there for you to apply any kind of transformation with 
external tools on data going in or out from Git.  There are good and bad 
things you can do with such a facility, but at least it becomes your 
responsibility to screw^H^H^H^Hfilter your data and not something that 
is enforced by Git itself.

You have to be careful, though.  Depending on what kind of
transformation you implement with the external tools, you would
end up having to slow down everything we would do.

So what?  

We provide a rope with proper caveat emptor.  Up to others to hang 
themselves with it if they so desire.  It is not our problem anymore.

It boils down to this statement from Andy:

    ..., keywords (in other VCSs, and so why not in git) are
    only updated when a file is checked out.  There is no need
    to touch every file.  It's actually beneficial, because the
    keyword in the file is the state of the file at the time it
    was checked in - which is actually more useful than updating
    it to the latest commit every time.

    That means you're only ever expanding in a file that your
    changing anyway - so it's effectively free.  git-checkout
    would still be immediate and instantaneous.

Back up a bit and think what "when a file is checked out" means.
His argument assumes the current behaviour of not checking out
when the underlying blob objects before munging are the same.

And I think that should remain.  If someone really wants full 
transformation aka keyword expansion or whatever then he'd just need to 
force a full read-tree after switching branch.

But with keyword expansion and fancier "external tools" whose
semantics are not well defined (iow, defined to be "do whatever
they please"), does it still make sense to consider two blobs
that appear in totally different context "the same" and omit
checking out (and causing the external tools hook not getting
run)?

I think so.  At least by default.  And if we really want to be kind we 
could provide a special attribute just for disabling such optimization 
i.e. to force a checkout of everything marked with such an attribute 
everytime.  But we might as well wait to see if someone actually ask 
about that.

But for many other cases having such a facility would be just nice 
especially for those cases that don't depend on the branch/commit but 
only on the content itself.  For example it was pointed out that Open 
Office documents are gzipped XML files.  In that case it would be 
extremely advantageous to have the ability to specify an input filter as
"gzip -d" and an output filter as "gzip -c" so Git has a chance to 
actually perform some kind of delta compression.

I'm sure there might be other type of filters for situation we've not 
thought about yet, and that we might not want to carry as a builtin 
feature.  Who knows, maybe someone might want to port Git to the 
System/360 and will need an EBCDIC filter on checked out text files.  Or 
maybe a byte swapping filter for some kind of binary files when on a 
system with a different endianness.

I already pointed out to Andy that the branch name the
file was taken from, if it were to take part of the keyword
expansion, would come out incorrectly in his printed svg
drawing.

Tough.

If you want somebody's earlier example of "giving a file with
embedded keyword to somebody, who modifies and sends the result
back in full, now you would want to incorporate the change by
identifying the origin" to work, you would want "$Source$" (I am
looking at CVS documentation, "Keyword substitution/Keyword
List") to identify where that file came from (after all, a
source tree could have duplicated files) so that you can tell
which file the update is about, and this keyword would expand
differently depending on where in the project tree the blob
appears.

Like we don't record renames, I don't think we should record such thing 
in checked out files either.  Using a search for the closest match (like 
we do for rename detection) is probably a better avenue than trusting 
that the ID embedded in the file wasn't messed with.  Linus already 
provided a small script that would do that with pretty good results.

It is not just the checkout codepath.  We omit diffs when we
know from SHA-1 that the blobs are the same before decoration.
We even omit diffs when we know from SHA-1 that two trees are
the same without taking possible decorations that can be applied
differently to the blobs they contain into account.  Earlier,
Andy said he wanted to grep for the expanded text if he is
grepping in the working tree, and I think that makes sense, but
that means git-grep cannot do the same "borrow from working tree
when expanding from blob object is more expensive" optimization
we have for diff.  We also need to disable that optimization
from the diff, regardless of what the correct semantics for
grepping in working trees should be.

I suspect that you would have to play safe and say "when
external tools are involved, we need to disable the existing
content SHA-1 based optimization for all paths that ask for
them" to keep your sanity.

Maybe.  If that is what's really needed then so be it.  People who 
really want to do strange things will have the flexibility to do so, but 
they'll have to pay the price in loss of performance.


Nicolas

Nicolas Pitre [off-list ref] writes:

On Tue, 17 Apr 2007, Junio C Hamano wrote:

quoted

You have to be careful, though.  Depending on what kind of
transformation you implement with the external tools, you would
end up having to slow down everything we would do.

So what?  

We provide a rope with proper caveat emptor.  Up to others to hang 
themselves with it if they so desire.  It is not our problem anymore.

I sort-of find it hard to believe hearing this from somebody who
muttered something about importance of perception a few days ago.

quoted

I suspect that you would have to play safe and say "when
external tools are involved, we need to disable the existing
content SHA-1 based optimization for all paths that ask for
them" to keep your sanity.

Maybe.  If that is what's really needed then so be it.  People who 
really want to do strange things will have the flexibility to do so, but 
they'll have to pay the price in loss of performance.

Not just that.  We end up having to pay the price of maintaining
hooks to let them do crazy things.

Hi,

On Tue, 17 Apr 2007, Nicolas Pitre wrote:

So what?

We provide a rope with proper caveat emptor.  Up to others to hang 
themselves with it if they so desire.  It is not our problem anymore.

The people will complain. On this list. And I have to check the mails 
before deleting, because the Subject: does not say "I just took the rope, 
ignored your caveat emptor, and now I am dead. What should I do now?".

Ciao,
Dscho

On Tue, 17 Apr 2007, Junio C Hamano wrote:

Nicolas Pitre [off-list ref] writes:

quoted

On Tue, 17 Apr 2007, Junio C Hamano wrote:

quoted

You have to be careful, though.  Depending on what kind of
transformation you implement with the external tools, you would
end up having to slow down everything we would do.

So what?  

We provide a rope with proper caveat emptor.  Up to others to hang 
themselves with it if they so desire.  It is not our problem anymore.

I sort-of find it hard to believe hearing this from somebody who
muttered something about importance of perception a few days ago.

Sure!  And that applies in this case as well.

With such a _generic_ hook, Git will be perceived as much more powerful 
and flexible.  I insist on "generic" because people could experiment 
with their own filters without endless debate on the mailing list and 
pressure to include this or that feature in the core, and we don't have 
to commit to those feature we're not in agreement with.

And let's face it: there are probably legitimate and possibly more 
useful things to do with such a hook than keyword expansion.

If you go to Home Hardware you can buy rope.  Of course you can hang 
yourself with it, but the rope manufacturers won't commit to that I'm 
sure.  But if rope was banned by law because it represents a threath to 
life then governments would be perceived really strangely even if their 
intention are good.

Sure we might have a strong opinion against keyword expansion and that 
is reflected by the fact that Git will most probably never ship with the 
ability to perform keyword expansion.  That doesn't mean we should deny 
all possibilities for external filters _even_ if they can be used for 
keyword expansion.

quoted

quoted

I suspect that you would have to play safe and say "when
external tools are involved, we need to disable the existing
content SHA-1 based optimization for all paths that ask for
them" to keep your sanity.

Maybe.  If that is what's really needed then so be it.  People who 
really want to do strange things will have the flexibility to do so, but 
they'll have to pay the price in loss of performance.

Not just that.  We end up having to pay the price of maintaining
hooks to let them do crazy things.

Weight that against the price of fighting them against the crazy things 
they won't quit wanting to do.  At some point it is just a matter of 
getting out of the way.


Nicolas

On Wed, 18 Apr 2007, Johannes Schindelin wrote:

Hi,

On Tue, 17 Apr 2007, Nicolas Pitre wrote:

quoted

So what?

We provide a rope with proper caveat emptor.  Up to others to hang 
themselves with it if they so desire.  It is not our problem anymore.

The people will complain. On this list. And I have to check the mails 
before deleting, because the Subject: does not say "I just took the rope, 
ignored your caveat emptor, and now I am dead. What should I do now?".

Well... in the case of keyword expansion (since this is really the 
contentious case here), with such a _generic_ facility to implement 
external filters, people will at least have the opportunity to try it.  
Sure they might complain that it doesn't work well, but 1) it is much 
easier to *understand* why it doesn't work well after experimenting with 
it, and 2) some people *will* be perfectly happy with something that 
doesn't work well but happens to just work in their own particular case.

And because it now becomes a case by case issue it is much easier for 
us to simply provide a generic mechanism and let people figure out by 
themselves what works and what doesn't work instead of having 
philosophical discussions on the merits of keyword expansions on the 
list.

And because people _will_ complain *anyway*, it might lead to more 
productive discussion if those who complain had the chance to realize 
what the issues really are by experience if theoretic demonstrations alone 
doesn't convey the problem fully.


Nicolas

Hi,

On Wed, 18 Apr 2007, Nicolas Pitre wrote:

And because it now becomes a case by case issue it is much easier for us 
to simply provide a generic mechanism and let people figure out by 
themselves what works and what doesn't work instead of having 
philosophical discussions on the merits of keyword expansions on the 
list.

That's a very good point.

Ciao,
Dscho

On Tue, 17 Apr 2007, Junio C Hamano wrote:

Nicolas Pitre [off-list ref] writes:

quoted

quoted

I would like to, however this doesn't currently integrate
well with git. I've been told in the past that once
.gitattributes is in place then the hooks for the crlf stuff
can be generalized to allow for calls out to custom code to
do this sort of thing.

And I agree that this is a perfectly sensible thing to do.  The facility
should be there for you to apply any kind of transformation with
external tools on data going in or out from Git.  There are good and bad
things you can do with such a facility, but at least it becomes your
responsibility to screw^H^H^H^Hfilter your data and not something that
is enforced by Git itself.

You have to be careful, though.  Depending on what kind of
transformation you implement with the external tools, you would
end up having to slow down everything we would do.

you can slow down everything that you do on your system if you defined too much 
work for external tools, that won't slow other people down who don't define any 
work for external tools.

It boils down to this statement from Andy:

   ..., keywords (in other VCSs, and so why not in git) are
   only updated when a file is checked out.  There is no need
   to touch every file.  It's actually beneficial, because the
   keyword in the file is the state of the file at the time it
   was checked in - which is actually more useful than updating
   it to the latest commit every time.

   That means you're only ever expanding in a file that your
   changing anyway - so it's effectively free.  git-checkout
   would still be immediate and instantaneous.

Back up a bit and think what "when a file is checked out" means.
His argument assumes the current behaviour of not checking out
when the underlying blob objects before munging are the same.

correct.

But with keyword expansion and fancier "external tools" whose
semantics are not well defined (iow, defined to be "do whatever
they please"), does it still make sense to consider two blobs
that appear in totally different context "the same" and omit
checking out (and causing the external tools hook not getting
run)?  I already pointed out to Andy that the branch name the
file was taken from, if it were to take part of the keyword
expansion, would come out incorrectly in his printed svg
drawing.

this is part of the rope you are handing out. the external tool could do a lot 
of things that don't make sense. you could have the tool include the serial 
number of the cpu you happen to be running on at the moment, it wouldn't make 
sense to do this, but it could be done. the fact that the rope could be used to 
hang someone doesn't mean that you should outlaw rope.

If you want somebody's earlier example of "giving a file with
embedded keyword to somebody, who modifies and sends the result
back in full, now you would want to incorporate the change by
identifying the origin" to work, you would want "$Source$" (I am
looking at CVS documentation, "Keyword substitution/Keyword
List") to identify where that file came from (after all, a
source tree could have duplicated files) so that you can tell
which file the update is about, and this keyword would expand
differently depending on where in the project tree the blob
appears.

It is not just the checkout codepath.  We omit diffs when we
know from SHA-1 that the blobs are the same before decoration.
We even omit diffs when we know from SHA-1 that two trees are
the same without taking possible decorations that can be applied
differently to the blobs they contain into account.  Earlier,
Andy said he wanted to grep for the expanded text if he is
grepping in the working tree, and I think that makes sense, but
that means git-grep cannot do the same "borrow from working tree
when expanding from blob object is more expensive" optimization
we have for diff.  We also need to disable that optimization
from the diff, regardless of what the correct semantics for
grepping in working trees should be.

git would not be able to borrow from the working tree just becouse the index 
thinks that the file is the same (and frankly, I'm not sure this is really a 
safe thing to do in any case, it's just something that works frequently enough 
that we get away with it)

the diff optimizations could (and should) stay.

I suspect that you would have to play safe and say "when
external tools are involved, we need to disable the existing
content SHA-1 based optimization for all paths that ask for
them" to keep your sanity.

Andy and I are both expecting that if the blobs are the same that none of the 
git tools would flag them as different. this maintains the huge speedups that 
git achieves by doing these checks.

if you want to make an option somewhere that disables this optimization, I guess 
it would be Ok, but I wouldn't do so until someone came up with a situation 
where they really needed it, nothing in what Andy or I have asked for needs 
this.

both of us are treating the keyword expansion as decorations to the file. it's 
useful, but the core meaning of 'what this file is' is the checked in version 
with the keywords unexpanded. all the optmizations that only look at what's 
checked in will remain as valid as they are today, it's only things that look at 
your working directory that would change.

David Lang