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

32 messages, 11 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:05

Andy Parkins [off-list ref] writes:
No parsing of the keyword itself is performed, the content is simply
dropped.
You are sidestepping the most important problem by doing this.

The only sensible keyword you could have, without destroying
what git is, is blob id.  No commit id, no date, no author.

In http://article.gmane.org/gmane.comp.version-control.git/44654,
Linus said:

    I'll finish off trying to explain the problem in fundamental git terms: 
    say you have a repository with two branches, A and B, and different 
    history  on a file "xyzzy" in those two branches, but because they both 
    ended up applying the same patches, the actual file contents do end up 
    being 100% identical. So they have the same SHA1.

    What is

            git diff A..B -- xyzzy

    supposed to print?

    And *I* claim that if you don't get an immediate and empty diff, your 
    system is TOTALLY BROKEN.

Another thing he could have said is this:

	When you have such two branches, A and B, and you are on
	branch A:

	$ git checkout B

	should be immediate and instantaneous.

If you try to keyword expand commit id, date or anything that is
sensitive to *how* you got there, even though A and B have the
exact same set of blobs, you have to essentially update all of
them.  Computing what to expand to takes (perhaps prohibitively
expensive) time, but more importantly rewriting the whole 20k
(or howmanyever you have in your project) files out becomes
necessary, if your keyword expansion wants to say "oh, this file
was taken from a checkout of branch B", for obvious reasons.

Keyword expanding blob-id, or munging line-endings to CRLF form
on platforms that want it, do not have this problem, as how you
reached to the blob content does not affect the result of
expansion, therefore not just the blobs in commit A and commit B
but the working tree checked out of them must match with each
other.

Having reiterated what Linus already said why keyword expansion
and git are not friendly with each other (perhaps the reason is
because the former is stupid and git is smart), I'd try to be a
bit constructive and point out the areas you _could_ help with
in the nearby codepaths:

 * When 'diff' borrows from the working tree because the
   filesystem data matches the blob we are interested in, we
   already have a call to convert_to_git().  The diff machinery
   operates on the canonicalized representation (i.e. this is an
   area we do not need help from you). 

 * When 'checkout', 'read-tree -u' and 'merge-recursive' write
   things, we already have calls to convert_to_working_tree() to
   munge blob representation to working tree representation
   (i.e. again, this is an area we do not need help from you).

 * We do not do the borrowing from working tree when doing
   grep_sha1(), but when we grep inside a file from working tree
   with grep_file(), we do not currently make it go through
   convert_to_git() to fix line endings.  Maybe we should, if
   only for consistency.

 * We do not currently run convert_to_git() on the patch text
   given to git-apply; we could do so in parse_single_patch().

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

From: Andy Parkins <hidden>
Date: 2016-06-15 22:43:05

On Tuesday 2007 April 17 11:09, Junio C Hamano wrote:
In http://article.gmane.org/gmane.comp.version-control.git/44654,
Linus said:
    And *I* claim that if you don't get an immediate and empty diff, your
    system is TOTALLY BROKEN.
Well that one is easy - the file is normalised to contain collapsed keywords 
upon checkin, so diff works the same as it ever did.  The output would be 
immediate and empty so is not TOTALLY BROKEN.
	$ git checkout B

	should be immediate and instantaneous.
Now - that's a much better argument.  However, it's not relevant, 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.
If you try to keyword expand commit id, date or anything that is
sensitive to *how* you got there, even though A and B have the
exact same set of blobs, you have to essentially update all of
them.  Computing what to expand to takes (perhaps prohibitively
expensive) time, but more importantly rewriting the whole 20k
(or howmanyever you have in your project) files out becomes
necessary, if your keyword expansion wants to say "oh, this file
was taken from a checkout of branch B", for obvious reasons.
Ignoring the fact that expansion is only when a file is checked out; I'd argue 
that it's your own fault if you enable keyword expansion on twenty thousand 
files.  A lot of the discussion has been about how useless keyword expansion 
is in almost every case.  I only want it for a few files in my repository; so 
am willing to pay the small computing cost.  Obviously keywords would be 
disabled by default - in which case, you get what you deserve if you enable 
them on everything.

Putting my own selfish requirements aside, from a purely "mine is better than 
yours" point of view, git can't do something that CVS (in all it's 
horridness) can.  It's distinctly off-putting to people when they 
say "keyword expansion", that the response is "YOU'RE AN IDIOT - GO AWAY - 
YOU DON'T DESERVE TO USE GIT"; and back they'll scurry to CVS/subversion.
Keyword expanding blob-id, or munging line-endings to CRLF form
on platforms that want it, do not have this problem, as how you
reached to the blob content does not affect the result of
expansion, therefore not just the blobs in commit A and commit B
but the working tree checked out of them must match with each
other.
That's true - however, even if the only keyword git supports is $BlobID$, that 
would address a large proportion of people's needs.  As I said above though, 
the keywords are only expanded on checkout (and checkin to be consistent).
Having reiterated what Linus already said why keyword expansion
and git are not friendly with each other (perhaps the reason is
because the former is stupid and git is smart), I'd try to be a
(This is were my "YOU'RE AN IDIOT - YOU CAN'T USE GIT" alarm goes off).  Git 
is better than CVS/subversion in every respect - save this one.  It's almost 
completely free to do (apart from the initial coding of it of course) because 
of these two factors:
 - The keywords are collapsed in the repository
 - The keywords are only expanded on checkout
It doesn't fundamentally alter anything that git does right now.
 * We do not do the borrowing from working tree when doing
   grep_sha1(), but when we grep inside a file from working tree
   with grep_file(), we do not currently make it go through
   convert_to_git() to fix line endings.  Maybe we should, if
   only for consistency.
I'd actually argue not - git-grep searches the working tree.  The expanded 
keywords are in the working tree.  Take the CRLF case - I'm a clueless user, 
who only understands the system I'm working on.  I want to search for all the 
line endings, so I do git-grep "\r\n" - that should work, because I'm 
searching my working tree.
 * We do not currently run convert_to_git() on the patch text
   given to git-apply; we could do so in parse_single_patch().
Yep - definitely; the applied patch should certainly be normalised before 
application.  I'd have to add it if I wanted keywords anyway wouldn't I?



Andy
-- 
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com

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

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: 2016-06-15 22:43:05


On Tue, 17 Apr 2007, Junio C Hamano wrote:
Andy Parkins [off-list ref] writes:
quoted
No parsing of the keyword itself is performed, the content is simply
dropped.
You are sidestepping the most important problem by doing this.
I obviosly agree (and I agree with everything in your email), but:
The only sensible keyword you could have, without destroying
what git is, is blob id.  No commit id, no date, no author.
Yes. And I already talked about some of the very fundamental problems that 
keyword expansion has (ie switching branches is basically impossible to do 
without checking out _every_single_file_ with the "keyword" attribute 
set. There are others).

Now, unexpansion is trivial to do (it really *is* the same as the 
"CRLF->LF" translation: that's technically really just an "unexpansion" 
too). And it should work. 

The way this does unexpansion also breaks "git diff" in that it bassically 
always makes diff *ignore* the keywords. In other words, when you do

	git diff A..B

and send the diff to somebody else, they'll never see any keywords at all! 

Now, that obviously fulfills my requirement that the diff be empty if A 
and B are the same, so you should expect me to be happy. But I'm not 
happy, because if the other person also is using git, HE CANNOT EVEN APPLY 
THE DIFF! Even if he's at "A", and thus gets a diff that is supposed to 
apply *exactly*, he'll get rejects if there were other changes around the 
unexpanded keyword (which *he* will have expanded in his working tree, of 
course!)

See? Keywords simply *cannot* work. They're broken. Either you can ignore 
them (and not show them in diffs), in which case the diff is broken, or 
you can not ignore them (and show them in diffs) in which case the diff is 
*also* broken, just differently.

The only sane and workable case is to not have them at all. Any keyword 
expansion will *always* result in problems. You simply cannot do it right. 

As I mentioned originally, it results in problems in CVS too, it's just 
that CVS really has so many other issues that you seldom see the problems.

Ok, after that new rant against keywords, I will say one positive thing:

 - keyword *unexpansion* is certainly easy (exactly because it's 
   stateless)

 - if we want to support a git that only does "unexpansion", you can 
   probably hack around stupid release scripting more easily. You can add 
   your keywords *outside* of git, and git will simply ignore them. 

So I'm actually not against keyword un-expansion. It has none of the 
fundamental problems that actually expanding the keywords has. It's 
literally no different from CRLF->LF translation. It can cause confusion, 
but if it has to be explicitly enabled with an attribute and is never done 
automatically, then having some support for unexpansion and letting the 
user who wants to use keywords use his own "wrapper scripts" around git to 
do his own expansion, be my guest..

You would be unable to do fundamental operations like "git checkout B" to 
jump to another branch, but if you don't support multiple branches and 
want to just act like CVS, maybe git unexpanding the crap will help you: 
you can add your own keywords, happy in the knowledge that git simply 
won't *care* about them, and will never see them.

So I absolutely detest keyword expansion and actually have a lot of 
arguments for why I don't think it *can* work even in theory (except by 
being totally unusable), but I don't have the *un*expansion. 

		Linus

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

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: 2016-06-15 22:43:05


On Tue, 17 Apr 2007, Andy Parkins wrote:
On Tuesday 2007 April 17 11:09, Junio C Hamano wrote:
quoted
In http://article.gmane.org/gmane.comp.version-control.git/44654,
Linus said:
    And *I* claim that if you don't get an immediate and empty diff, your
    system is TOTALLY BROKEN.
Well that one is easy - the file is normalised to contain collapsed keywords 
upon checkin, so diff works the same as it ever did.  The output would be 
immediate and empty so is not TOTALLY BROKEN.
No, it *is* TOTALLY BROKEN, because your keywords guaranteed that it 
doesn't even *apply*.

That's such a fundamental part of a patch that I didn't even _mention_ it, 
but I obviously should have.

If you cannot apply the diff you generate, what the hell is the *point* of 
a diff?

Try this:

 - File-A in revision 1:

	$ID: some random crap about rev1 $
	Line 2

 - same file in revision 2:
	$ID: some other random crap about rev2 $
	Line 2 got modified

and think about it. Your diff will be something like

	@@ -1,2 +1,2 @@
	 $ID:$
	-Line 2
	+Line 2 got modified

and the diff WON'T EVEN APPLY!

What kind of diff is that? Would you call it perhaps "totally broken"?

In other words, there's no way in hell you can make this work. You'll end 
up always having to edit the keywords parts of diffs to make them apply if 
they are part of the context.

(This, btw, is something that a CVS person says "so what?" about. They're 
_used_ to having to do it. It's how you do merges in CVS. Really. How many 
people have actually *worked* with branches in CVS on any complex project 
with any nontrivial work happening on the branch? I have. I hated CVS for 
many reasons. Keywords was just a small small detail in that hate 
relationship, but it was one of them!)

		Linus

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

From: Andy Parkins <hidden>
Date: 2016-06-15 22:43:05

On Tuesday 2007, April 17, Linus Torvalds wrote:
Try this:

 - File-A in revision 1:

	$ID: some random crap about rev1 $
	Line 2

 - same file in revision 2:
	$ID: some other random crap about rev2 $
	Line 2 got modified

and think about it. Your diff will be something like

	@@ -1,2 +1,2 @@
	 $ID:$
	-Line 2
	+Line 2 got modified

and the diff WON'T EVEN APPLY!
Why on earth would it not apply?  It's being applied using git-apply, 
which will unexpand the keywords as it goes - as I keep saying.  When 
the apply engine is looking for the context it's going to collapse the 
keyword, so the context will match and the diff WILL EVEN APPLY.

As Junio said in his reply, git-apply doesn't currently call 
convert_to_git(), but that's easily implemented.
In other words, there's no way in hell you can make this work. You'll
You keep saying these sweepingly general things.  It can be made to 
work.
end up always having to edit the keywords parts of diffs to make them
apply if they are part of the context.
No I don't.  If I had to then the keyword code would be broken.  No one 
in their right mind would think that was an acceptable thing to do.
(This, btw, is something that a CVS person says "so what?" about.
They're _used_ to having to do it. It's how you do merges in CVS.
Really. How many people have actually *worked* with branches in CVS
That's because CVS is rubbish.  What has that got to do with it?
on any complex project with any nontrivial work happening on the
branch? I have. I hated CVS for many reasons. Keywords was just a
small small detail in that hate relationship, but it was one of
them!)
You really can stop trying to persuade me that CVS is no good for 
version control - I agree, a thousand times I agree.  There are a lot 
of things that CVS does in a broken manner, that doesn't mean that git 
does the same thing in a broken manner.



Andy

-- 
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com

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

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: 2016-06-15 22:43:05


On Tue, 17 Apr 2007, Andy Parkins wrote:
Why on earth would it not apply?  It's being applied using git-apply, 
which will unexpand the keywords as it goes - as I keep saying.
So you will never work with anybody outside of git?

What about tar-files when you export the tree? Should they have the 
expanded version? 
You keep saying these sweepingly general things.  It can be made to 
work.
No, it CANNOT.

Trust me. There's NO WAY IN HELL it will "work" in any other sense than 
"limp along and not be usable".

Yes, you can make it "work" if you:

 - make sure that you never _ever_ leave the git environment

   But why do you want keyword expansion then? The whole point is if you 
   have other tools than the git tools that look at a file. Even your svg 
   example was literally about having non-git tools work with the data. 
   What if you ever email the file to somebody else? 

 - you make all git tools explicitly always strip them.

   Again, what's the point again? You add keyword expansion, and then the 
   only tools that you really allow to touch it (except your "print it 
   out" example) will have to remove the keyword expansion just to work.

That's not "work". That's just stupid. Yes, you can make your "print it 
out" example work, but as alreadyt mentioned, you could have done that 
some other way, with a simple makefile rule, quite independently (and much 
better) than the SCM ever did.

		Linus

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

From: Andy Parkins <hidden>
Date: 2016-06-15 22:43:05

On Tuesday 2007, April 17, Linus Torvalds wrote:
So you will never work with anybody outside of git?
For my projects - correct; I don't care about the rest of the world.  
For projects that do - don't enable keywords, it's an option, all I 
want is to have that option.
What about tar-files when you export the tree? Should they have the
expanded version?
If I have to pick one then: no.  I think out-of-tree keywords are too 
much trouble for exactly the reasons you say; however, I wouldn't like 
to presume what other people think is too much trouble so I suppose it 
would have to be an option.
quoted
You keep saying these sweepingly general things.  It can be made to
work.
No, it CANNOT.

Trust me. There's NO WAY IN HELL it will "work" in any other sense
than "limp along and not be usable".
Well I'm making progress, "limp along" is a significant step up from 
impossible.  :-)

Look, my primary objection to this is the SHOUTING about how impossible 
it is even though I've tried to address every problem you've thrown at 
me - I'm finding it really difficult to figure out why you're trying so 
hard to dissuade me from even _trying_.  If it all goes wrong (as I 
fully accept it might), so be it, I can live with that; I'll even be 
happy to tell you you're right and I'm wrong.  Why is this such a 
problem?

Keywords are so hated by everyone that I doubt they would ever be 
accepted into git - it's an intellectual exercise for me at this stage 
really. 
Yes, you can make it "work" if you:

 - make sure that you never _ever_ leave the git environment
As it happens, _I_ never ever leave the git environment.  Can I use 
keywords then?

You don't seem to have such a problem with git's extended diffs for 
renames or subprojects - "make sure that you never _ever_ leave the git 
environment".
   But why do you want keyword expansion then? The whole point is if
you have other tools than the git tools that look at a file. Even
your svg example was literally about having non-git tools work with
the data. What if you ever email the file to somebody else?
If by "tools" you mean other version control systems, then I don't 
intend them to work.  If by "tools" you mean gcc, inkscape, gv, bash, 
web browsers or any other fileformat that allows comments in the file 
then I expect it to be fine.  If I publish a web page, it'd be nice to 
show the ID on the page - that's all just "nice" not "necessary" 
or "I'm throwing git away if I don't get it".

Emailing to others isn't a problem either: let's say I email them my SVG 
(with keywords expanded), they make some edits and send it me back - 
worse, they send me a diff back.  I'm going to apply that diff using 
git-apply; which will collapse the keywords and apply the diff.
 - you make all git tools explicitly always strip them.
Well, not "all", so far I've added one call to convert_to_git() in 
builtin-apply.c - it was a one line addition.  It needed doing anyway 
to deal with the CRLF correctly.  I can't see there being that many 
places that this needs doing.  I may well be wrong, if I end up 
scattering calls to convert_to_git() everywhere I'll give up.
   Again, what's the point again? You add keyword expansion, and then
the only tools that you really allow to touch it (except your "print
it out" example) will have to remove the keyword expansion just to
work.
(I don't see why my tiny "print it out" example isn't enough - it 
matters to me)

However, most tools don't care about the keywords, it's only non-git 
diff and non-git patch that are affected.  As long as the file format 
supports comments, then keyword expansion will be just fine.
That's not "work". That's just stupid. Yes, you can make your "print
it out" example work, but as alreadyt mentioned, you could have done
that some other way, with a simple makefile rule, quite independently
(and much better) than the SCM ever did.
That's just being obtuse - no other tool cares in the slightest about 
the keywords, there are more "tools" in the world than just the VCS.



Andy

-- 
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com

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

From: Nicolas Pitre <hidden>
Date: 2016-06-15 22:43:05

On Tue, 17 Apr 2007, Andy Parkins wrote:
On Tuesday 2007, April 17, Linus Torvalds wrote:
quoted
Trust me. There's NO WAY IN HELL it will "work" in any other sense
than "limp along and not be usable".
Well I'm making progress, "limp along" is a significant step up from 
impossible.  :-)

Look, my primary objection to this is the SHOUTING about how impossible 
it is even though I've tried to address every problem you've thrown at 
me - I'm finding it really difficult to figure out why you're trying so 
hard to dissuade me from even _trying_.  If it all goes wrong (as I 
fully accept it might), so be it, I can live with that; I'll even be 
happy to tell you you're right and I'm wrong.  Why is this such a 
problem?

Keywords are so hated by everyone that I doubt they would ever be 
accepted into git - it's an intellectual exercise for me at this stage 
really. 
I cannot do otherwise than ask at this point in the debate: why isn't 
the makefile rule sufficient for your needs?  Why going through a 
complicated path that no one else will support due to its numerous 
pitfalls?
quoted
Yes, you can make your "print it out" example work, but as alreadyt 
mentioned, you could have done that some other way, with a simple 
makefile rule, quite independently (and much better) than the SCM 
ever did.
That's just being obtuse - no other tool cares in the slightest about 
the keywords, there are more "tools" in the world than just the VCS.
... which reinforces my question: why force a task on the VCS if it 
doesn't fit well with its fundamental design?


Nicolas

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

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: 2016-06-15 22:43:05


On Tue, 17 Apr 2007, Andy Parkins wrote:
Look, my primary objection to this is the SHOUTING about how impossible 
it is even though I've tried to address every problem you've thrown at 
me
No, you haven't. You've "addressed" them by stating they don't matter. It 
doesn't "matter" that a diff won't actually apply to a checked-out tree, 
because you fix it up in another tool.

And it doesn't "matter" that switching branches will just result in the 
wrong keyword expansion, because you don't care about the keywords 
actually being "correct" - they are just random strings, and it apparently 
doesn't really have to "work" as far as you're concerned.

And the "git grep" concern you just dismissed by stating that it should 
use the filesystem copy, never mind that this just means that a clean 
working tree gets different results from doing the same thing based on 
that same revision.

In other words, you simply don't seem to worry about TRUSTING the results. 
It's ok if patches don't apply, or if you get different results on working 
trees than "inside" the revision control.

And the reaon I'm shouting is that "it doesn't matter that it's a bit 
hacky" mentality is what gets you things like CVS in the end. Bit-for-bit 
results actually matter. Guarantees actually matter. And you should not be 
able to see a differece in the working tree just because you happened to 
be on a different branch before.

Those are the kind of nasty surprises that make people go: "I don't know 
what the end result is, because there is an element of 'just how did you 
happen to do that operation' to it".

I want to *trust* the SCM I use.
I'm finding it really difficult to figure out why you're trying so 
hard to dissuade me from even _trying_.
You can try, but you are *ignoring* the things that I say. The end result 
will either perform really badly, or you cannot trust it, or *both*. And 
you'll introduce interesting semantics like "diffs won't actually apply to 
the working tree with normal tools".

(And yes, git diffs are extended, but they *do* apply to working trees in 
all cases where normal "patch" can even support the notion in the first 
place.)

And it's not just things like diff and switching branches. If you want 
your keywords to generate things like "last modified by Xyzzy", you 
haven't even explained *how* you'd do that. Yeah, you can do

	git log --pretty=oneline --abbrev-commit -1 -- filename

etc, and you probably think it's instantaneous, but do the timings for a 
big repository with a file that hasn't been modified in months, and then 
imagine doing that for an initial checkout (say, after you set the 
"keyword" attribute for all *.c files).

Whoops. The checkout took an hour. Is that really a path you want to go 
down?
Keywords are so hated by everyone that I doubt they would ever be 
accepted into git - it's an intellectual exercise for me at this stage 
really. 
If that's what it is, fine. But people on the list seem to actually *want* 
it. They must be educated what a *disaster* it would be to actually try to 
really support something like it in real life, and not just as a mental 
exercise.

		Linus

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

From: David Lang <hidden>
Date: 2016-06-15 22:43:05

On Tue, 17 Apr 2007, Nicolas Pitre wrote:
On Tue, 17 Apr 2007, Andy Parkins wrote:
quoted
On Tuesday 2007, April 17, Linus Torvalds wrote:
quoted
Trust me. There's NO WAY IN HELL it will "work" in any other sense
than "limp along and not be usable".
Well I'm making progress, "limp along" is a significant step up from
impossible.  :-)

Look, my primary objection to this is the SHOUTING about how impossible
it is even though I've tried to address every problem you've thrown at
me - I'm finding it really difficult to figure out why you're trying so
hard to dissuade me from even _trying_.  If it all goes wrong (as I
fully accept it might), so be it, I can live with that; I'll even be
happy to tell you you're right and I'm wrong.  Why is this such a
problem?

Keywords are so hated by everyone that I doubt they would ever be
accepted into git - it's an intellectual exercise for me at this stage
really.
I cannot do otherwise than ask at this point in the debate: why isn't
the makefile rule sufficient for your needs?  Why going through a
complicated path that no one else will support due to its numerous
pitfalls?
not all uses of VCS's involve useing make
quoted
quoted
Yes, you can make your "print it out" example work, but as alreadyt
mentioned, you could have done that some other way, with a simple
makefile rule, quite independently (and much better) than the SCM
ever did.
That's just being obtuse - no other tool cares in the slightest about
the keywords, there are more "tools" in the world than just the VCS.
... which reinforces my question: why force a task on the VCS if it
doesn't fit well with its fundamental design?
becouse the VCS can do the job better then anything else? even if there are 
limits to what the VCS can do.

David Lang

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

From: Nicolas Pitre <hidden>
Date: 2016-06-15 22:43:05

On Tue, 17 Apr 2007, David Lang wrote:
On Tue, 17 Apr 2007, Nicolas Pitre wrote:
quoted
I cannot do otherwise than ask at this point in the debate: why isn't
the makefile rule sufficient for your needs?  Why going through a
complicated path that no one else will support due to its numerous
pitfalls?
not all uses of VCS's involve useing make
Use perl then.  Or a shell script.  Or even a command.com batch script.  
Or your own tool.
quoted
quoted
That's just being obtuse - no other tool cares in the slightest about
the keywords, there are more "tools" in the world than just the VCS.
... which reinforces my question: why force a task on the VCS if it
doesn't fit well with its fundamental design?
becouse the VCS can do the job better then anything else?
On what basis?
even if there are
limits to what the VCS can do.
In the context of keyword expansion I don't agree at all with this 
statement.  Git can *not* do better than an external tool and it has 
been demonstrated a few times already.


Nicolas

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

From: David Lang <hidden>
Date: 2016-06-15 22:43:05

On Tue, 17 Apr 2007, Nicolas Pitre wrote:
On Tue, 17 Apr 2007, David Lang wrote:
quoted
On Tue, 17 Apr 2007, Nicolas Pitre wrote:
quoted
I cannot do otherwise than ask at this point in the debate: why isn't
the makefile rule sufficient for your needs?  Why going through a
complicated path that no one else will support due to its numerous
pitfalls?
not all uses of VCS's involve useing make
Use perl then.  Or a shell script.  Or even a command.com batch script.
Or your own tool.
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.

however now it sounds as if people are saying that doing this is so evil that it 
shouldn't ever be allowed.
quoted
quoted
quoted
That's just being obtuse - no other tool cares in the slightest about
the keywords, there are more "tools" in the world than just the VCS.
... which reinforces my question: why force a task on the VCS if it
doesn't fit well with its fundamental design?
becouse the VCS can do the job better then anything else?
On what basis?
quoted
even if there are
limits to what the VCS can do.
In the context of keyword expansion I don't agree at all with this
statement.  Git can *not* do better than an external tool and it has
been demonstrated a few times already.
the VCS can make sure that the appropriate external code is always run when 
things are checked in/out. external tools (unless they are a complete set of 
wrappers for git) can't do that.

David Lang

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

From: Andy Parkins <hidden>
Date: 2016-06-15 22:43:05

On Tuesday 2007, April 17, Linus Torvalds wrote:
No, you haven't. You've "addressed" them by stating they don't
matter. It doesn't "matter" that a diff won't actually apply to a
checked-out tree, because you fix it up in another tool.
Okay.  I think this is a matter of perspective - my perspective is that 
if it supplies what svn/cvs supply then that would please the people 
who want it (of whom I am one); yours is obviously that if it isn't 
perfect, it's not worth doing.  That's a reasonable thing to demand, 
and I'm not going to try and argue you out of it.
And it doesn't "matter" that switching branches will just result in
the wrong keyword expansion, because you don't care about the
keywords actually being "correct" - they are just random strings, and
it apparently doesn't really have to "work" as far as you're
concerned.
If you define "work" as "works like cvs/svn does", then I was fine with 
it.  I don't like it when my favourite VCS, that I want everyone to 
use, doesn't have an answer to "but does it do X?".
And the "git grep" concern you just dismissed by stating that it
should use the filesystem copy, never mind that this just means that
a clean working tree gets different results from doing the same thing
based on that same revision.
As I said at the time, I just picked one of the two options.  If you 
don't like that, pick the other option - collapse the keywords during 
the grep...
And the reaon I'm shouting is that "it doesn't matter that it's a bit
hacky" mentality is what gets you things like CVS in the end.
Bit-for-bit results actually matter. Guarantees actually matter. And
you should not be able to see a differece in the working tree just
because you happened to be on a different branch before.
Bit-for-bit as in CRLF is untouched?  No?  Bit-for-bit as in you said 
you were okay with keyword-collapsing but not expansion?  You're just 
as willing to compromise as me, you've just drawn the line in a 
different place.

Incidentally: for future reference, I'll read what you write regardless 
of whether you shout it or not.
You can try, but you are *ignoring* the things that I say. The end
I've tried very hard to respond to every point you've put to me; I've 
not selectively chopped out bits, and I've tried to give answers that 
make it work as you ask.  Now, none of those things were acceptable to 
you - which is fine - but I certinaly wasn't ignoring what you say - 
_disagreeing with_ is not the same as ignoring.
If that's what it is, fine. But people on the list seem to actually
*want* it. They must be educated what a *disaster* it would be to
actually try to really support something like it in real life, and
not just as a mental exercise.
People wanting something "wrong" so much is not a sign that they need 
educating, it's a sign that they need a solution.   In every other 
respect git has a solution for them; rather than explaining to them 
that what they want is stupid, I'd offer that it's more appropriate to 
offer something better in exchange.  So my keyword expansion idea is 
wrong - fine - where's the something better?  Writing custom scripts 
and makefiles for every project I ever run is /not/ "something better".

Anyway, it's late, and I'm tired - this has turned into a battle of 
wills, and I'm not that into battling.   Enough antihistamine has been 
poured on my itch that I no longer want to scratch it.  I'll send my 
most recent patch for the sake of history, and then abandon this 
project.

Thanks for your time on this, I appreciate your detailed responses, even 
if we don't agree.



Andy
-- 
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com

[PATCH] Add keyword collapse support to convert.c

From: Andy Parkins <hidden>
Date: 2016-06-15 22:43:05

This patch adds expansion of keywords support.  The collapse is only
performed when the "keywords" attribute it found for a file.  The check
for this attribute is done in the same way as the "crlf" attribute
check.

The actual collapse is performed by keyword_collapse_git() which is
called from convert_to_git() when the "keywords" attribute is found.

keyword_collapse_git() finds strings of the form

 $KEYWORD: ARBITRARY STRING$

And collapses them into

 $KEYWORD:$

No parsing of the keyword itself is performed, the content is simply
dropped.

Despite the fact that this doesn't do anything useful from the users
perspective, this patch forms the more important half of keyword
expansion support - because it prevents the expansion from entering the
repository.  It effectively creates blind spots that git tools won't
see.

convert_to_git() has also been changed so that it no longer only does
CRLF conversion.  Instead, a flag is kept to say whether any conversion
was done by the CRLF code, and then that converted buffer is passed to
keyword_collapse_git() and the flag again updated.  It then returns 1 if
either of these conversion functions actually changed anything.

I've also included a test script to show that the keyword collapse is
working.  It particular demonstrates that the diff between a file with
keywords and the repository is blind to the expanded keyword.

git-apply is patched to perform the collapse as well on each fragment.

Signed-off-by: Andy Parkins <redacted>
---

This is on top of 1ddfc1ad616550764056077b9e12a35533298c89.

I'm positing it for posterity.  It's not going anywhere though, so I'm not
submitting it for inclusion.


 builtin-apply.c     |    2 +
 convert.c           |  123 +++++++++++++++++++++++++++++++++++++++++++++++++-
 t/t0030-keywords.sh |   95 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 217 insertions(+), 3 deletions(-)
 create mode 100755 t/t0030-keywords.sh
diff --git a/builtin-apply.c b/builtin-apply.c
index fd92ef7..212c7d4 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1056,6 +1056,8 @@ static int parse_single_patch(char *line, unsigned long size, struct patch *patc
 	unsigned long oldlines = 0, newlines = 0, context = 0;
 	struct fragment **fragp = &patch->fragments;
 
+	convert_to_git( patch->new_name, &line, &size );
+
 	while (size > 4 && !memcmp(line, "@@ -", 4)) {
 		struct fragment *fragment;
 		int len;
diff --git a/convert.c b/convert.c
index d0d4b81..a18e7ea 100644
--- a/convert.c
+++ b/convert.c
@@ -230,16 +230,133 @@ static int git_path_check_crlf(const char *path)
 	return attr_crlf_check.isset;
 }
 
+/* ------------------ keywords -------------------- */
+
+static void setup_keyword_check(struct git_attr_check *check)
+{
+	static struct git_attr *attr_keyword;
+
+	if (!attr_keyword)
+		attr_keyword = git_attr("keywords", 8);
+	check->attr = attr_keyword;
+}
+
+static int git_path_check_keyword(const char *path)
+{
+	struct git_attr_check attr_keyword_check;
+
+	setup_keyword_check(&attr_keyword_check);
+
+	if (git_checkattr(path, 1, &attr_keyword_check))
+		return -1;
+	return attr_keyword_check.isset;
+}
+
+static int keyword_collapse_git(const char *path, char **bufp, unsigned long *sizep)
+{
+	char *buffer, *nbuf, *keyword;
+	unsigned long size, keywordlength;
+	int changes = 0;
+	enum {
+		IN_VOID,
+		PRE_KEYWORD,
+		IN_KEYWORD,
+		IN_EXPANSION,
+		END_KEYWORD
+	} parser_state = IN_VOID;
+
+	size = *sizep;
+	if (!size)
+		return 0;
+	buffer = *bufp;
+
+	/*
+	 * Allocate an identically sized buffer, keyword collapse can
+	 * only reduce the size so we'll never overflow (although we might
+	 * waste a few bytes
+	 */
+	nbuf = xmalloc(size);
+	*bufp = nbuf;
+
+	while (size) {
+		unsigned char c;
+
+		c = *buffer;
+
+		switch( parser_state ) {
+		case IN_VOID:        /* Normal characters, wait for '$' */
+			if (c == '$')
+				parser_state = PRE_KEYWORD;
+			break;
+		case PRE_KEYWORD:    /* Gap between '$' and keyword */
+			keywordlength = 0;
+			keyword = buffer;
+			if (!isspace(c))
+				parser_state = IN_KEYWORD;
+			else
+				break;
+		case IN_KEYWORD:     /* Keyword itself */
+			if (c == ':')
+				parser_state = IN_EXPANSION;
+			else if (c == '$' || c == '\n' || c == '\r' || c == '\0' )
+				parser_state = END_KEYWORD;
+			else
+				keywordlength++;
+			break;
+		case IN_EXPANSION:   /* The expansion gets silently removed */
+			if (c == '$' || c == '\n' || c == '\r' || c == '\0' )
+				parser_state = END_KEYWORD;
+			else {
+				changes = 1;
+				/* Every character we skip reduces the overall size */
+				(*sizep)--;
+				buffer++;
+				size--;
+			}
+			continue;
+		case END_KEYWORD:    /* End of keyword */
+			parser_state = IN_VOID;
+			break;
+		}
+
+		*nbuf++ = c;
+		buffer++;
+		size--;
+	}
+
+	return (changes != 0);
+}
+
+
+/* ------------------------------------------------ */
 int convert_to_git(const char *path, char **bufp, unsigned long *sizep)
 {
+	int changes = 0;
+
 	switch (git_path_check_crlf(path)) {
 	case 0:
-		return 0;
+		changes += 0;
+		break;
+	case 1:
+		changes += forcecrlf_to_git(path, bufp, sizep);
+		break;
+	default:
+		changes += autocrlf_to_git(path, bufp, sizep);
+		break;
+	}
+
+	switch (git_path_check_keyword(path)) {
+	case 0:
+		changes += 0;
+		break;
 	case 1:
-		return forcecrlf_to_git(path, bufp, sizep);
+		changes += keyword_collapse_git(path, bufp, sizep);
+		break;
 	default:
-		return autocrlf_to_git(path, bufp, sizep);
+		changes += 0;
+		break;
 	}
+	return (changes != 0);
 }
 
 int convert_to_working_tree(const char *path, char **bufp, unsigned long *sizep)
diff --git a/t/t0030-keywords.sh b/t/t0030-keywords.sh
new file mode 100755
index 0000000..5180b0e
--- /dev/null
+++ b/t/t0030-keywords.sh
@@ -0,0 +1,95 @@
+#!/bin/sh
+
+cd $(dirname $0)
+
+test_description='Keyword expansion'
+
+. ./test-lib.sh
+
+# Adding the attribute "keywords" turns the keyword expansion on
+# I've used "notkeywords" as an attribute as a placeholder attribute
+# but this is just "somerandomattribute", it has no meaning
+
+# Expect success because the keyword attribute should be found
+test_expect_success 'Keywords attribute present' '
+
+	echo "keywordsfile keywords" >.gitattributes &&
+
+	echo "\$keyword: anythingcangohere\$" > keywordsfile &&
+
+	git add keywordsfile &&
+	git add .gitattributes &&
+	git commit -m test-keywords &&
+
+	git check-attr keywords -- keywordsfile
+'
+
+# Expect failure because the repository version should be different from the
+# working tree version.
+#
+#  In repository : $keyword:$
+#  In working dir: $keyword: anythingcangohere$
+#
+test_expect_failure 'Keywords collapse active' '
+
+	git show HEAD:keywordsfile > keywordsfile.cmp &&
+	cmp keywordsfile keywordsfile.cmp
+
+'
+
+# expect success because we want to find the keyword line collapsed in the
+# and hence appearing unchanged in the output of git-diff
+test_expect_success 'git-diff with keywords present' '
+	echo "Non-keyword containing line" >> keywordsfile &&
+	git diff -- keywordsfile | grep -qs "^ \$keyword:\$$"
+'
+
+# Check git-apply blindness
+cat > keyword-patch.diff << EOF
+diff --git a/keywordsfile b/keywordsfile
+--- a/keywordsfile
++++ b/keywordsfile
+@@ -1,2 +1,2 @@
+ \$keyword:\$
+-Non-keyword containing line
++Another non-keyword containing line
+EOF
+
+test_expect_success 'patch application with keywords active' '
+	git-apply --check keyword-patch.diff
+'
+
+# Expect failure because the keywords attribute should NOT be found
+test_expect_failure 'Keywords attribute absent' '
+
+	echo "keywordsfile notkeywords" >.gitattributes &&
+
+	git add .gitattributes &&
+	git commit -m test-not-keywords &&
+
+	git check-attr keywords -- keywordsfile
+
+'
+
+# If keywords are later disabled on that file, then the keyword collapsed
+# will be ignored, so a diff should now show differences, because git is no
+# longer keyword blind
+test_expect_success 'git-diff with keywords in file but disabled' '
+	git diff -- keywordsfile | grep -qs "^diff"
+'
+
+# Expect success because the repository should be identical to the working tree
+test_expect_success 'Keywords collapse inactive' '
+
+	git add keywordsfile &&
+	git commit -m "test-not-keywords"
+
+	git show HEAD:keywordsfile > keywordsfile.cmp &&
+	cmp keywordsfile keywordsfile.cmp
+'
+
+test_expect_failure 'patch application without keywords active' '
+	git-apply --check keyword-patch.diff
+'
+
+test_done
-- 
1.5.1.1.822.g0049

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

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: 2016-06-15 22:43:05


On Tue, 17 Apr 2007, Andy Parkins wrote:
If you define "work" as "works like cvs/svn does", then I was fine with 
it.
I can't really argue against that. Yes, I agree 100% that we can "work" in 
the sense that "cvs/svn works". There's clearly no fundamental reasons why 
you can't, since svn/cvs obviously do it.

I just do have higher standards. I really dislike CVS, and in many ways I 
actually think that SVN is even worse (not because it's really "worse", 
but because I think it is such a waste - it fixes the _trivial_ things 
about CVS, but doesn't really fix any of the underlying problems).

So I don't actually think that CVS "works". 
Bit-for-bit as in CRLF is untouched?  No?  Bit-for-bit as in you said 
you were okay with keyword-collapsing but not expansion?  You're just 
as willing to compromise as me, you've just drawn the line in a 
different place.
Bit-for-bit as in "you have to be able to trust every single bit".

And no, I don't actually love CRLF either. But it doesn't have quite the 
same fundamental problems. It has issues too, but they are fundamentally 
smaller, and I think making "git compatible with Windows" is also a lot 
more important than making "git compatible with CVS users".

Windows we cannot change. CVS users we can try to help. 

		Linus

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

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: 2016-06-15 22:43:05


On Tue, 17 Apr 2007, Linus Torvalds wrote:
Windows we cannot change. CVS users we can try to help. 
.. and if it wasn't clear, "helping" CVS users is not in my opinion to try 
to make git act like CVS, and lettign them do stupid things, but to try to 
help them become *more* than CVS users.

Because they too can become upstanding members of society, and leave their 
dark past behind them. I firmly believe that nobody is past saving.

		Linus

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

From: Nicolas Pitre <hidden>
Date: 2016-06-15 22:43:05

On Tue, 17 Apr 2007, David Lang wrote:
On Tue, 17 Apr 2007, Nicolas Pitre wrote:
quoted
On Tue, 17 Apr 2007, David Lang wrote:
quoted
On Tue, 17 Apr 2007, Nicolas Pitre wrote:
quoted
I cannot do otherwise than ask at this point in the debate: why isn't
the makefile rule sufficient for your needs?  Why going through a
complicated path that no one else will support due to its numerous
pitfalls?
not all uses of VCS's involve useing make
Use perl then.  Or a shell script.  Or even a command.com batch script.
Or your own tool.
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.


Nicolas

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

From: Martin Langhoff <hidden>
Date: 2016-06-15 22:43:05

On 4/18/07, Andy Parkins [off-list ref] wrote:
On Tuesday 2007, April 17, Linus Torvalds wrote:
quoted
So you will never work with anybody outside of git?
For my projects - correct; I don't care about the rest of the world.
For projects that do - don't enable keywords, it's an option, all I
want is to have that option.
GIT's fundamental respect for the contents its tracking (in not
munging them with keyword expansion) means that it works great in
contexts where other SCMs tools are used. And being content-centric at
the SCM layer means that it is possible to track a git project with -
say - Mercurial (and vice-versa) with nothing more than a bit of perl
glue and no guessing at all. The content is the content is the
content.

When the SCM has "munge-the-content" options, your "upstream" can make
things completely un-trackable. Tracking CVS with git is a breeze but
there is breakage related to keyword expansion. I should write some
better heuristics for it, but it's impossible to know with 100%
certainty that you are doing the right thing, and getting the correct
content from CVS.

All this talk of breaking non-git-patch goes back to the same. With
the current design, projects that use git are easily trackable if you
just look at the content, and ignore the SCM. That's an outstanding
property and quite central to the design, and I wouldn't include an
option to "turn it off" even if the patch to implement it turns out to
be trivial.

Probably using make will help - you might be able to wire it to work
off a post-update-hook so it's completely transparent.

cheers,


martin

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:05

Andy Parkins [off-list ref] writes:
On Tuesday 2007 April 17 11:09, Junio C Hamano wrote:
quoted
	$ git checkout B

	should be immediate and instantaneous.
Now - that's a much better argument.  However, it's not
relevant, keywords (in other VCSs, and so why not in git) are
only updated when a file is checked out.
It _is_ very much relevant.

If you have the keyword in your svg drawing, and if branch A and
branch B happen to have textually the same contents but the way
they got there are different, I do not think not checking it out
upon branch switching is correct.  Otherwise your printed copy
would have information from the version in branch A, even after
switching to B.

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

From: David Lang <hidden>
Date: 2016-06-15 22:43:05

On Tue, 17 Apr 2007, Nicolas Pitre wrote:
Subject: Re: [PATCH 2/2] Add keyword unexpansion support to convert.c

On Tue, 17 Apr 2007, David Lang wrote:
quoted
On Tue, 17 Apr 2007, Nicolas Pitre wrote:
quoted
On Tue, 17 Apr 2007, David Lang wrote:
quoted
On Tue, 17 Apr 2007, Nicolas Pitre wrote:
quoted
I cannot do otherwise than ask at this point in the debate: why isn't
the makefile rule sufficient for your needs?  Why going through a
complicated path that no one else will support due to its numerous
pitfalls?
not all uses of VCS's involve useing make
Use perl then.  Or a shell script.  Or even a command.com batch script.
Or your own tool.
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.
I'm pretty sure that hooks for an external helper would satisfy Andy with his 
keyword expanstion as well.

David Lang

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

From: Andy Parkins <hidden>
Date: 2016-06-15 22:43:05

On Tuesday 2007, April 17, David Lang wrote:
I'm pretty sure that hooks for an external helper would satisfy Andy
with his keyword expanstion as well.
It would.

Andy
-- 
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com

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

From: Rogan Dawes <hidden>
Date: 2016-06-15 22:43:05

Nicolas Pitre wrote:
On Tue, 17 Apr 2007, David Lang wrote:
quoted
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.


Nicolas
One of the examples that has been given in the past has been taking a 
zipped OpenDocumentFormat file, unzipping it to its component parts, and 
then committing the individual files rather than the aggregate.

But I can't figure out how this might work.

One idea is to store the binary ODF file in the index (and in the packs, 
etc) as a directory with the individual text (and other) files as 
entries within that directory. Then, when various git operations want to 
use the directory, the operation is redirected via an attribute match to 
an external script that knows how to checkout an ODF "directory", or 
diff an ODF "directory", etc.

Or similarly, when checking an "ODF" file in, the attribute would lead 
to an appropriate script creating the "tree" of individual files.

Does this sound workable?

Rogan

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

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:05

Hi,

On Tue, 17 Apr 2007, Linus Torvalds wrote:
On Tue, 17 Apr 2007, Linus Torvalds wrote:
quoted
Windows we cannot change. CVS users we can try to help. 
.. and if it wasn't clear, "helping" CVS users is not in my opinion to 
try to make git act like CVS, and lettign them do stupid things, but to 
try to help them become *more* than CVS users.
I am quite certain that we also can help Windows users see the light. Once 
we have them not only complaining, but actually doing something about it.
Because they too can become upstanding members of society, and leave their 
dark past behind them. I firmly believe that nobody is past saving.
Well, it depends. If you clicked on that File Menu button, and then 
clicked on the "Save" item, you are past saving.

Ciao,
Dscho

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

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: 2016-06-15 22:43:05


On Wed, 18 Apr 2007, Rogan Dawes wrote:
Or similarly, when checking an "ODF" file in, the attribute would lead to an
appropriate script creating the "tree" of individual files.

Does this sound workable?
I think it sounds very interesting, and I'd much rather do _those_ kinds 
of rewrites than keyword unexpansion. And yes, some kind of generic 
support for rewriting might give people effectively the keywords they want 
(I think the CVS semantics are not likely to be logical, but people can 
probably do something that works for them), and at that point maybe the 
keyword discussion goes away too.

However, I don't know if it is "workable".

The thing is, it's easy enough (although potentially _very_ expensive) to 
run some per-file script at each commit and at each checkout. But there 
are some fundamental operations that are even more common:

 - checking for "file changed", aka the "git status" kind of thing

   Anything we do would have to follow the same "stat" rules, at a 
   minimum. You can *not* afford to have to check the file manually.

   So especially if you combine several pieces into one, or split one file 
   into several pieces, your index would have to contain the entry 
   that matches the _filesystem_ (because that's what the index is all 
   about), but then the *tree* would contain the pieces (or the single 
   entry that matches several filesystem entries).

 - what about diffs (once the stat information says something has 
   potentially changed)? You'd have to script those too, and it really 
   sounds like some very basic operations get a _lot_ more expensive and 
   complex.

   This is also related to the above: one of the most fundamental diffs is 
   the diff of the index and a tree - so if the index matches the 
   "filesystem state" and the trees contain some "combined entry" or 
   "split entry", you'd have to teach some very core diff functionality 
   about that kind of mapping.

In other words, I think it's too complicated. Not necessarily impossible, 
but likely harder and more complex than it's really worth.

Having a 1:1 file mapping (like the CRLF<->LF object mapping is) is a lot 
easier. You just have to make sure that the index has the *stat* 
information from the filesystem, but the *sha1* identity information from 
the git internal format, and things automatically just fall out right. But 
if you have anything but a 1:1 relationship, it gets hugely more complex.

			Linus

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

From: Nicolas Pitre <hidden>
Date: 2016-06-15 22:43:05

On Wed, 18 Apr 2007, Linus Torvalds wrote:

On Wed, 18 Apr 2007, Rogan Dawes wrote:
quoted
Or similarly, when checking an "ODF" file in, the attribute would lead to an
appropriate script creating the "tree" of individual files.

Does this sound workable?
I think it sounds very interesting, and I'd much rather do _those_ kinds 
of rewrites than keyword unexpansion. And yes, some kind of generic 
support for rewriting might give people effectively the keywords they want 
(I think the CVS semantics are not likely to be logical, but people can 
probably do something that works for them), and at that point maybe the 
keyword discussion goes away too.
Exactly my point.
However, I don't know if it is "workable".

The thing is, it's easy enough (although potentially _very_ expensive) to 
run some per-file script at each commit and at each checkout. But there 
are some fundamental operations that are even more common:

 - checking for "file changed", aka the "git status" kind of thing

   Anything we do would have to follow the same "stat" rules, at a 
   minimum. You can *not* afford to have to check the file manually.

   So especially if you combine several pieces into one, or split one file 
   into several pieces, your index would have to contain the entry 
   that matches the _filesystem_ (because that's what the index is all 
   about), but then the *tree* would contain the pieces (or the single 
   entry that matches several filesystem entries).
For that the external script would need the ability to alter the index 
itself.  That becomes a bit yucky.  Or maybe something could be made 
with a mechanism like dnotify/inotify to "touch" the single placeholder 
entry referenced by the index whenever one of the split component 
changes.
 - what about diffs (once the stat information says something has 
   potentially changed)? You'd have to script those too, and it really 
   sounds like some very basic operations get a _lot_ more expensive and 
   complex.
Of course an attribute for external diff script is certainly something 
that could be useful independently of this case, as some particular 
binary formats might have a way of their own to display their 
differences.

The whole idea of having the ability to call external tools is exactly 
to delegate complex/bizarre/unusual tasks to separate and independent 
agents.  The whole checkout operation becomes much more expensive but 
everyone using such facility might expect it.  It just cannot be as bad 
as a straight checkout with CVS from a remote server though (OK I know 
it can but you know what I mean).
   This is also related to the above: one of the most fundamental diffs is 
   the diff of the index and a tree - so if the index matches the 
   "filesystem state" and the trees contain some "combined entry" or 
   "split entry", you'd have to teach some very core diff functionality 
   about that kind of mapping.
Well, if the split components are represented by a single placeholder in 
the index and the filesystem, and the filesystem placeholder is 
"touched" whenever a split component is modified, then the mapping can 
as well be limited to the external scripts for checkin/checkout/diff 
only without the Git core having the slightest idea about it.

Sure it might be slow and unusual, but at least not impossible.  And 
again, with an attribute providing a facility for external tools it is 
then not our problem anymore.


Nicolas

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

From: Rogan Dawes <hidden>
Date: 2016-06-15 22:43:05

Linus Torvalds wrote:
On Wed, 18 Apr 2007, Rogan Dawes wrote:
quoted
Or similarly, when checking an "ODF" file in, the attribute would lead to an
appropriate script creating the "tree" of individual files.

Does this sound workable?
I think it sounds very interesting, and I'd much rather do _those_ kinds 
of rewrites than keyword unexpansion. And yes, some kind of generic 
support for rewriting might give people effectively the keywords they want 
(I think the CVS semantics are not likely to be logical, but people can 
probably do something that works for them), and at that point maybe the 
keyword discussion goes away too.

However, I don't know if it is "workable".

The thing is, it's easy enough (although potentially _very_ expensive) to 
run some per-file script at each commit and at each checkout. But there 
are some fundamental operations that are even more common:

 - checking for "file changed", aka the "git status" kind of thing

   Anything we do would have to follow the same "stat" rules, at a 
   minimum. You can *not* afford to have to check the file manually.

   So especially if you combine several pieces into one, or split one file 
   into several pieces, your index would have to contain the entry 
   that matches the _filesystem_ (because that's what the index is all 
   about), but then the *tree* would contain the pieces (or the single 
   entry that matches several filesystem entries).
Right. I would imagine that the script would have to take care of 
setting timestamps in the filesystem appropriately, as well as passing 
them back to git when queried.

e.g. expanding test.odf/: (since we store it as a directory)

git calls "odf.sh checkout test.odf/ <sha1> <perms> <stat>"

odf checkout calls back into git to find out the details of the files 
under test.odf/, and creates a zip file containing the individual files, 
with appropriate timestamps.

User then opens the file using OO.o or whatever, makes some changes and 
saves the file.

The user then runs git status:

git calls "odf.sh stat test.odf/" (again, triggered by an attribute)

odf.sh does the equivalent of "zip -l" to get up to date stat info for 
the component files, and passes it back to git (via stdout?)

User commits his changes:

git calls "odf.sh checkin test.odf/"

odf.sh unpacks the individual files, calls back into git to create 
individual objects (using a fast-import-alike protocol over stdout?)

 - what about diffs (once the stat information says something has 
   potentially changed)? You'd have to script those too, and it really 
   sounds like some very basic operations get a _lot_ more expensive and 
   complex.
 >
   This is also related to the above: one of the most fundamental diffs is 
   the diff of the index and a tree - so if the index matches the 
   "filesystem state" and the trees contain some "combined entry" or 
   "split entry", you'd have to teach some very core diff functionality 
   about that kind of mapping.

In other words, I think it's too complicated. Not necessarily impossible, 
but likely harder and more complex than it's really worth.

Having a 1:1 file mapping (like the CRLF<->LF object mapping is) is a lot 
easier. You just have to make sure that the index has the *stat* 
information from the filesystem, but the *sha1* identity information from 
the git internal format, and things automatically just fall out right. But 
if you have anything but a 1:1 relationship, it gets hugely more complex.

			Linus
Absolutely. I just raised it now since it was originally mentioned quite 
a long time ago as a possible feature of git, and I couldn't see how it 
might work.

Thanks for your time,

Rogan

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

From: Nicolas Pitre <hidden>
Date: 2016-06-15 22:43:05

On Wed, 18 Apr 2007, Rogan Dawes wrote:
Right. I would imagine that the script would have to take care of setting
timestamps in the filesystem appropriately, as well as passing them back to
git when queried.

e.g. expanding test.odf/: (since we store it as a directory)

git calls "odf.sh checkout test.odf/ <sha1> <perms> <stat>"

odf checkout calls back into git to find out the details of the files under
test.odf/, and creates a zip file containing the individual files, with
appropriate timestamps.
Why would you need to store the document as multiple files into Git?

The only reasons I can see for external filters are:

 1) Normalization, e.g. the LF->CRLF thing.

    Some might want to do keyword expansion which would fall into this
    category as well.

 2) Better archiving with Git's deltas.

    That means storing files uncompressed into Git since Git will
    compress them anyway, after significant space reduction due to 
    deltas which cannot occur on already compressed data.

So if your .odf file is actually a zip with multiple files, then all you 
have to do is to convert that zip archive into a non compressed tar 
archive on checkins, and the reverse transformation on checkouts.  The 
non compressed tar content will delta well, the Git archive will be 
small, and no tricks with the index will be needed.

Or am I missing something?


Nicolas

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

From: Rogan Dawes <hidden>
Date: 2016-06-15 22:43:05

Nicolas Pitre wrote:
On Wed, 18 Apr 2007, Rogan Dawes wrote:
quoted
Right. I would imagine that the script would have to take care of setting
timestamps in the filesystem appropriately, as well as passing them back to
git when queried.

e.g. expanding test.odf/: (since we store it as a directory)

git calls "odf.sh checkout test.odf/ <sha1> <perms> <stat>"

odf checkout calls back into git to find out the details of the files under
test.odf/, and creates a zip file containing the individual files, with
appropriate timestamps.
Why would you need to store the document as multiple files into Git?

The only reasons I can see for external filters are:

 1) Normalization, e.g. the LF->CRLF thing.

    Some might want to do keyword expansion which would fall into this
    category as well.

 2) Better archiving with Git's deltas.

    That means storing files uncompressed into Git since Git will
    compress them anyway, after significant space reduction due to 
    deltas which cannot occur on already compressed data.

So if your .odf file is actually a zip with multiple files, then all you 
have to do is to convert that zip archive into a non compressed tar 
archive on checkins, and the reverse transformation on checkouts.  The 
non compressed tar content will delta well, the Git archive will be 
small, and no tricks with the index will be needed.

Or am I missing something?


Nicolas
Probably not! ;-)

I was just thinking that it would be easier to see diffs between 
individual files, rather than between entries in a zip. But if we are 
calling out to a specialized handler, the handler can do that just as 
easily, and without the added complexity in the index, etc.

It also means that someone without the attributes and specialized 
handler would not be able to use the file (if it is stored as a directory).

Clearly a bad idea! Just ignore me, I'm used to it! ;-)

Rogan

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

From: Alon Ziv <hidden>
Date: 2016-06-15 22:43:05

On Wed, 2007-04-18 at 11:59 -0400, Nicolas Pitre wrote:
So if your .odf file is actually a zip with multiple files, then all you 
have to do is to convert that zip archive into a non compressed tar 
archive on checkins, and the reverse transformation on checkouts.  The 
non compressed tar content will delta well, the Git archive will be 
small, and no tricks with the index will be needed.
In fact, for the specific case of OO.o files, I would claim the proper
transformation is just converting to non-compressed zip on checkin...

(Non-compressed zip is just as good here as tar, and has the added
advantage that there is no need for a reverse transformation on
checkout :))

	-az

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

From: Jakub Narebski <hidden>
Date: 2016-06-15 22:43:06

Andy Parkins wrote:
quoted
 * We do not do the borrowing from working tree when doing
   grep_sha1(), but when we grep inside a file from working tree
   with grep_file(), we do not currently make it go through
   convert_to_git() to fix line endings.  Maybe we should, if
   only for consistency.
I'd actually argue not - git-grep searches the working tree.  The expanded 
keywords are in the working tree.  Take the CRLF case - I'm a clueless user, 
who only understands the system I'm working on.  I want to search for all the 
line endings, so I do git-grep "\r\n" - that should work, because I'm 
searching my working tree.
Actually, "git grep" can search both the working tree (default), but also
an index (--cached), or specified tree (or tree-ish). The same with
"git diff": it can work on tree (repository), index, working tree version,
now I think in [almost] any combination. 

Think what keyword expansion means to all this... Well, you can have -kk
to expand/not expand keywords, but this is avoiding issue, not solving it
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

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

From: Nikolai Weibull <hidden>
Date: 2016-06-15 22:43:06

On 4/17/07, Andy Parkins [off-list ref] wrote:
On Tuesday 2007, April 17, Linus Torvalds wrote:
quoted
No, you haven't. You've "addressed" them by stating they don't
matter. It doesn't "matter" that a diff won't actually apply to a
checked-out tree, because you fix it up in another tool.
Okay.  I think this is a matter of perspective - my perspective is that
if it supplies what svn/cvs supply then that would please the people
who want it (of whom I am one); yours is obviously that if it isn't
perfect, it's not worth doing.  That's a reasonable thing to demand,
and I'm not going to try and argue you out of it.
Loads of people would be pleased if marijuana was legalized.  For some
reason, few governments seem willing to cater to their needs.

  nikolai

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

From: David Lang <hidden>
Date: 2016-06-15 22:43:06

On Fri, 20 Apr 2007, Jakub Narebski wrote:
Andy Parkins wrote:
quoted
quoted
 * We do not do the borrowing from working tree when doing
   grep_sha1(), but when we grep inside a file from working tree
   with grep_file(), we do not currently make it go through
   convert_to_git() to fix line endings.  Maybe we should, if
   only for consistency.
I'd actually argue not - git-grep searches the working tree.  The expanded
keywords are in the working tree.  Take the CRLF case - I'm a clueless user,
who only understands the system I'm working on.  I want to search for all the
line endings, so I do git-grep "\r\n" - that should work, because I'm
searching my working tree.
Actually, "git grep" can search both the working tree (default), but also
an index (--cached), or specified tree (or tree-ish). The same with
"git diff": it can work on tree (repository), index, working tree version,
now I think in [almost] any combination.

Think what keyword expansion means to all this... Well, you can have -kk
to expand/not expand keywords, but this is avoiding issue, not solving it
how is git-grep on the working tree any different than just useing grep? the 
value in the git-* versions of system commands are that they work on the 
history, index, etc wher ethe normal system tools don't.

in this particular case, since the git user can do a git-grep of the working 
tree, or a git-grep of HEAD (or of the index), I don't think that it hurts much 
either way.

if git-grep of the working tree converts things to the checked-in version before 
the pattern match, the user can still use grep  to go through the checked-out 
version

if git-grep of the working tree doesn't convert things to the checked-in version 
before the pattern match, the user can stil use git-grep HEAD or --cached to go 
through the checked-in version.

David Lang
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help