{"thread":{"id":"7715","subject":"[PATCH 2/2] Add keyword unexpansion support to convert.c","startedAt":"2007-04-17T09:41:45Z","lastAt":"2007-04-21T23:31:10Z","messageCount":66,"participants":["Andy Parkins","Junio C Hamano","Johannes Sixt","Linus Torvalds","Rogan Dawes","Nicolas Pitre","David Lang","Matthieu Moy","Martin Langhoff","Robin H. Johnson","J. Bruce Fields","Daniel Barkalow","David Kågedal","Johannes Schindelin","Alon Ziv","Jakub Narebski","Nikolai Weibull"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"39628","messageId":"200704171041.46176.andyparkins@gmail.com","threadId":"7715","inReplyTo":null,"subject":"[PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-04-17T09:41:45Z","receivedAt":"2007-04-17T09:41:45Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"This patch adds expansion of keywords support.  The unexpansion is only\nperformed when the \"keywords\" attribute it found for a file.  The check\nfor this attribute is done in the same way as the \"crlf\" attribute\ncheck.\n\nThe actual unexpansion is performed by keyword_unexpand_git() which is\ncalled from convert_to_git() when the \"keywords\" attribute is found.\n\nkeyword_unexpand_git() finds strings of the form\n\n $KEYWORD: ARBITRARY STRING$\n\nAnd collapses them into\n\n $KEYWORD:$\n\nNo parsing of the keyword itself is performed, the content is simply\ndropped.\n\nDespite the fact that this doesn't do anything useful from the users\nperspective, this patch forms the more important half of keyword\nexpansion support - because it prevents the expansion from entering the\nrepository.  It effectively creates blind spots that git tools won't\nsee.\n\nconvert_to_git() has also been changed so that it no longer only does\nCRLF conversion.  Instead, a flag is kept to say whether any conversion\nwas done by the CRLF code, and then that converted buffer is passed to\nkeyword_unexpand_git() and the flag again updated.  It then returns 1 if\neither of these conversion functions actually changed anything.\n\nI've also included a test script to show that the keyword unexpansion is\nworking.  It particular demonstrates that the diff between a file with\nkeywords and the repository is blind to the expanded keyword.\n\nSigned-off-by: Andy Parkins <andyparkins@gmail.com>\n---\nI'm not submitting this for application; I've not polished it, and I've not\nwritten the expansion half yet.\n\nHowever, I did want to show what I've been banging on about with keyword\nexpansion, and this does a reasonable job.  The test code shows that the idea\nis sound - what goes in the repository is stable, and what appears in the\nworking directory can contain any arbitrary keyword expansion.\n\nAdding expansion is harder, as I have no clue which calls to make to find\neven the most basic information about an object; but I thought I'd get\nfeedback before I expend that effort.\n\nAreas that might cause problems are the git-apply type of commands, I haven't\nchecked to see if they use convert_to_git() on their input to normalise it\nfor entry into the repository.  I hope so, as the CRLF support relies on it as\nwell :-)\n\n convert.c           |  115 +++++++++++++++++++++++++++++++++++++++++++++++++-\n t/t0030-keywords.sh |   76 +++++++++++++++++++++++++++++++++\n 2 files changed, 188 insertions(+), 3 deletions(-)\n create mode 100755 t/t0030-keywords.sh\n\ndiff --git a/convert.c b/convert.c\nindex d0d4b81..0c7b270 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -230,16 +230,125 @@ static int git_path_check_crlf(const char *path)\n \treturn attr_crlf_check.isset;\n }\n \n+/* ------------------ keywords -------------------- */\n+\n+static void setup_keyword_check(struct git_attr_check *check)\n+{\n+\tstatic struct git_attr *attr_keyword;\n+\n+\tif (!attr_keyword)\n+\t\tattr_keyword = git_attr(\"keywords\", 8);\n+\tcheck->attr = attr_keyword;\n+}\n+\n+static int git_path_check_keyword(const char *path)\n+{\n+\tstruct git_attr_check attr_keyword_check;\n+\n+\tsetup_keyword_check(&attr_keyword_check);\n+\n+\tif (git_checkattr(path, 1, &attr_keyword_check))\n+\t\treturn -1;\n+\treturn attr_keyword_check.isset;\n+}\n+\n+static int keyword_unexpand_git(const char *path, char **bufp, unsigned long *sizep)\n+{\n+\tchar *buffer, *nbuf, *keyword;\n+\tunsigned long size, keywordlength;\n+\tint changes = 0;\n+\tenum {\n+\t\tIN_VOID,\n+\t\tPRE_KEYWORD,\n+\t\tIN_KEYWORD,\n+\t\tIN_EXPANSION,\n+\t\tEND_KEYWORD\n+\t} parser_state = IN_VOID;\n+\n+\tsize = *sizep;\n+\tif (!size)\n+\t\treturn 0;\n+\tbuffer = *bufp;\n+\n+\t/*\n+\t * Allocate an identically sized buffer, keyword unexpansion can\n+\t * only reduce the size so we'll never overflow (although we might\n+\t * waste a few bytes\n+\t */\n+\tnbuf = xmalloc(size);\n+\t*bufp = nbuf;\n+\n+\twhile (size) {\n+\t\tunsigned char c;\n+\n+\t\tc = *buffer;\n+\n+\t\tswitch( parser_state ) {\n+\t\tcase IN_VOID:        /* Normal characters, wait for '$' */\n+\t\t\tif (c == '$')\n+\t\t\t\tparser_state = PRE_KEYWORD;\n+\t\t\tbreak;\n+\t\tcase PRE_KEYWORD:    /* Gap between '$' and keyword */\n+\t\t\tkeywordlength = 0;\n+\t\t\tkeyword = buffer;\n+\t\t\tif (!isspace(c))\n+\t\t\t\tparser_state = IN_KEYWORD;\n+\t\t\telse\n+\t\t\t\tbreak;\n+\t\tcase IN_KEYWORD:     /* Keyword itself */\n+\t\t\tif (c == ':')\n+\t\t\t\tparser_state = IN_EXPANSION;\n+\t\t\telse if (c == '$' || c == '\\n' || c == '\\r')\n+\t\t\t\tparser_state = END_KEYWORD;\n+\t\t\telse\n+\t\t\t\tkeywordlength++;\n+\t\t\tbreak;\n+\t\tcase IN_EXPANSION:   /* The expansion gets silently removed */\n+\t\t\tif (c == '$' || c == '\\n')\n+\t\t\t\tparser_state = END_KEYWORD;\n+\t\t\telse {\n+\t\t\t\tchanges = 1;\n+\t\t\t\t/* Every character we skip reduces the overall size */\n+\t\t\t\t(*sizep)--;\n+\t\t\t\tbuffer++;\n+\t\t\t\tsize--;\n+\t\t\t}\n+\t\t\tcontinue;\n+\t\tcase END_KEYWORD:    /* End of keyword */\n+\t\t\tparser_state = IN_VOID;\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\t*nbuf++ = c;\n+\t\tbuffer++;\n+\t\tsize--;\n+\t}\n+\n+\treturn (changes != 0);\n+}\n+\n+\n+/* ------------------------------------------------ */\n int convert_to_git(const char *path, char **bufp, unsigned long *sizep)\n {\n+\tint changes = 0;\n+\n \tswitch (git_path_check_crlf(path)) {\n \tcase 0:\n-\t\treturn 0;\n+\t\tchanges += 0;\n \tcase 1:\n-\t\treturn forcecrlf_to_git(path, bufp, sizep);\n+\t\tchanges += forcecrlf_to_git(path, bufp, sizep);\n \tdefault:\n-\t\treturn autocrlf_to_git(path, bufp, sizep);\n+\t\tchanges += autocrlf_to_git(path, bufp, sizep);\n+\t}\n+\n+\tswitch (git_path_check_keyword(path)) {\n+\tcase 0:\n+\t\tchanges += 0;\n+\tcase 1:\n+\t\tchanges += keyword_unexpand_git(path, bufp, sizep);\n \t}\n+\treturn (changes != 0);\n }\n \n int convert_to_working_tree(const char *path, char **bufp, unsigned long *sizep)\ndiff --git a/t/t0030-keywords.sh b/t/t0030-keywords.sh\nnew file mode 100755\nindex 0000000..375acb8\n--- /dev/null\n+++ b/t/t0030-keywords.sh\n@@ -0,0 +1,76 @@\n+#!/bin/sh\n+\n+cd $(dirname $0)\n+\n+test_description='Keyword expansion'\n+\n+. ./test-lib.sh\n+\n+# Adding the attribute \"keywords\" turns the keyword expansion on\n+# I've used \"notkeywords\" as an attribute as a placeholder attribute\n+# but this is just \"somerandomattribute\", it has no meaning\n+\n+# Expect success because the keyword attribute should be found\n+test_expect_success 'Keywords attribute present' '\n+\n+\techo \"keywordsfile keywords\" >.gitattributes &&\n+\n+\techo \"\\$keyword: anythingcangohere\\$\" > keywordsfile &&\n+\n+\tgit add keywordsfile &&\n+\tgit add .gitattributes &&\n+\tgit commit -m test-keywords &&\n+\n+\tgit check-attr keywords -- keywordsfile\n+'\n+\n+# Expect failure because the repository version should be different from the\n+# working tree version.\n+#\n+#  In repository : $keyword:$\n+#  In working dir: $keyword: anythingcangohere$\n+#\n+test_expect_failure 'Keywords unexpansion active' '\n+\n+\tgit show HEAD:keywordsfile > keywordsfile.cmp &&\n+\tcmp keywordsfile keywordsfile.cmp\n+\n+'\n+\n+# expect success because we want to find the keyword line unexpanded in the\n+# and hence appearing unchanged in the output of git-diff\n+test_expect_success 'git-diff with keywords present' '\n+\techo \"Non-keyword containing line\" >> keywordsfile &&\n+\tgit diff -- keywordsfile | grep -qs \"^ \\$keyword:\\$$\"\n+'\n+\n+# Expect failure because the keywords attribute should NOT be found\n+test_expect_failure 'Keywords attribute absent' '\n+\n+\techo \"keywordsfile notkeywords\" >.gitattributes &&\n+\n+\tgit add .gitattributes &&\n+\tgit commit -m test-not-keywords &&\n+\n+\tgit check-attr keywords -- keywordsfile\n+\n+'\n+\n+# If keywords are later disabled on that file, then the keyword unexpansion\n+# will be ignored, so a diff should now show differences, because git is no\n+# longer keyword blind\n+test_expect_success 'git-diff with keywords in file but disabled' '\n+\tgit diff -- keywordsfile | grep -qs \"^diff\"\n+'\n+\n+# Expect success because the repository should be identical to the working tree\n+test_expect_success 'Keywords unexpansion inactive' '\n+\n+\tgit add keywordsfile &&\n+\tgit commit -m \"test-not-keywords\"\n+\n+\tgit show HEAD:keywordsfile > keywordsfile.cmp &&\n+\tcmp keywordsfile keywordsfile.cmp\n+'\n+\n+test_done\n-- \n1.5.1.1.821.g88bdb\n"},{"id":"39632","messageId":"7v7isbpb0p.fsf@assigned-by-dhcp.cox.net","threadId":"7715","inReplyTo":"200704171041.46176.andyparkins@gmail.com","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-17T10:09:58Z","receivedAt":"2007-04-17T10:09:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andy Parkins <andyparkins@gmail.com> writes:\n\n> No parsing of the keyword itself is performed, the content is simply\n> dropped.\n\nYou are sidestepping the most important problem by doing this.\n\nThe only sensible keyword you could have, without destroying\nwhat git is, is blob id.  No commit id, no date, no author.\n\nIn http://article.gmane.org/gmane.comp.version-control.git/44654,\nLinus said:\n\n    I'll finish off trying to explain the problem in fundamental git terms: \n    say you have a repository with two branches, A and B, and different \n    history  on a file \"xyzzy\" in those two branches, but because they both \n    ended up applying the same patches, the actual file contents do end up \n    being 100% identical. So they have the same SHA1.\n\n    What is\n\n            git diff A..B -- xyzzy\n\n    supposed to print?\n\n    And *I* claim that if you don't get an immediate and empty diff, your \n    system is TOTALLY BROKEN.\n\nAnother thing he could have said is this:\n\n\tWhen you have such two branches, A and B, and you are on\n\tbranch A:\n\n\t$ git checkout B\n\n\tshould be immediate and instantaneous.\n\nIf you try to keyword expand commit id, date or anything that is\nsensitive to *how* you got there, even though A and B have the\nexact same set of blobs, you have to essentially update all of\nthem.  Computing what to expand to takes (perhaps prohibitively\nexpensive) time, but more importantly rewriting the whole 20k\n(or howmanyever you have in your project) files out becomes\nnecessary, if your keyword expansion wants to say \"oh, this file\nwas taken from a checkout of branch B\", for obvious reasons.\n\nKeyword expanding blob-id, or munging line-endings to CRLF form\non platforms that want it, do not have this problem, as how you\nreached to the blob content does not affect the result of\nexpansion, therefore not just the blobs in commit A and commit B\nbut the working tree checked out of them must match with each\nother.\n\nHaving reiterated what Linus already said why keyword expansion\nand git are not friendly with each other (perhaps the reason is\nbecause the former is stupid and git is smart), I'd try to be a\nbit constructive and point out the areas you _could_ help with\nin the nearby codepaths:\n\n * When 'diff' borrows from the working tree because the\n   filesystem data matches the blob we are interested in, we\n   already have a call to convert_to_git().  The diff machinery\n   operates on the canonicalized representation (i.e. this is an\n   area we do not need help from you). \n\n * When 'checkout', 'read-tree -u' and 'merge-recursive' write\n   things, we already have calls to convert_to_working_tree() to\n   munge blob representation to working tree representation\n   (i.e. again, this is an area we do not need help from you).\n\n * We do not do the borrowing from working tree when doing\n   grep_sha1(), but when we grep inside a file from working tree\n   with grep_file(), we do not currently make it go through\n   convert_to_git() to fix line endings.  Maybe we should, if\n   only for consistency.\n\n * We do not currently run convert_to_git() on the patch text\n   given to git-apply; we could do so in parse_single_patch().\n"},{"id":"39638","messageId":"4624A474.77756C86@eudaptics.com","threadId":"7715","inReplyTo":"200704171041.46176.andyparkins@gmail.com","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Johannes Sixt","fromEmail":"j.sixt@eudaptics.com","sentAt":"2007-04-17T10:41:56Z","receivedAt":"2007-04-17T10:41:56Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Andy Parkins wrote:\n>         switch (git_path_check_crlf(path)) {\n>         case 0:\n> -               return 0;\n> +               changes += 0;\n>         case 1:\n> -               return forcecrlf_to_git(path, bufp, sizep);\n> +               changes += forcecrlf_to_git(path, bufp, sizep);\n>         default:\n> -               return autocrlf_to_git(path, bufp, sizep);\n> +               changes += autocrlf_to_git(path, bufp, sizep);\n> +       }\n> +\n> +       switch (git_path_check_keyword(path)) {\n> +       case 0:\n> +               changes += 0;\n> +       case 1:\n> +               changes += keyword_unexpand_git(path, bufp, sizep);\n>         }\n\nI think there are 'break's missing all along the way.\n\n-- Hannes\n"},{"id":"39640","messageId":"200704171235.34793.andyparkins@gmail.com","threadId":"7715","inReplyTo":"7v7isbpb0p.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-04-17T11:35:33Z","receivedAt":"2007-04-17T11:35:33Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"On Tuesday 2007 April 17 11:09, Junio C Hamano wrote:\n\n> In http://article.gmane.org/gmane.comp.version-control.git/44654,\n> Linus said:\n>     And *I* claim that if you don't get an immediate and empty diff, your\n>     system is TOTALLY BROKEN.\n\nWell that one is easy - the file is normalised to contain collapsed keywords \nupon checkin, so diff works the same as it ever did.  The output would be \nimmediate and empty so is not TOTALLY BROKEN.\n\n> \t$ git checkout B\n>\n> \tshould be immediate and instantaneous.\n\nNow - that's a much better argument.  However, it's not relevant, keywords (in \nother VCSs, and so why not in git) are only updated when a file is checked \nout.  There is no need to touch every file.  It's actually beneficial, \nbecause the keyword in the file is the state of the file at the time it was \nchecked in - which is actually more useful than updating it to the latest \ncommit every time.\n\nThat means you're only ever expanding in a file that your changing anyway - so \nit's effectively free.  git-checkout would still be immediate and \ninstantaneous.\n\n> If you try to keyword expand commit id, date or anything that is\n> sensitive to *how* you got there, even though A and B have the\n> exact same set of blobs, you have to essentially update all of\n> them.  Computing what to expand to takes (perhaps prohibitively\n> expensive) time, but more importantly rewriting the whole 20k\n> (or howmanyever you have in your project) files out becomes\n> necessary, if your keyword expansion wants to say \"oh, this file\n> was taken from a checkout of branch B\", for obvious reasons.\n\nIgnoring the fact that expansion is only when a file is checked out; I'd argue \nthat it's your own fault if you enable keyword expansion on twenty thousand \nfiles.  A lot of the discussion has been about how useless keyword expansion \nis in almost every case.  I only want it for a few files in my repository; so \nam willing to pay the small computing cost.  Obviously keywords would be \ndisabled by default - in which case, you get what you deserve if you enable \nthem on everything.\n\nPutting my own selfish requirements aside, from a purely \"mine is better than \nyours\" point of view, git can't do something that CVS (in all it's \nhorridness) can.  It's distinctly off-putting to people when they \nsay \"keyword expansion\", that the response is \"YOU'RE AN IDIOT - GO AWAY - \nYOU DON'T DESERVE TO USE GIT\"; and back they'll scurry to CVS/subversion.\n\n> Keyword expanding blob-id, or munging line-endings to CRLF form\n> on platforms that want it, do not have this problem, as how you\n> reached to the blob content does not affect the result of\n> expansion, therefore not just the blobs in commit A and commit B\n> but the working tree checked out of them must match with each\n> other.\n\nThat's true - however, even if the only keyword git supports is $BlobID$, that \nwould address a large proportion of people's needs.  As I said above though, \nthe keywords are only expanded on checkout (and checkin to be consistent).\n\n> Having reiterated what Linus already said why keyword expansion\n> and git are not friendly with each other (perhaps the reason is\n> because the former is stupid and git is smart), I'd try to be a\n\n(This is were my \"YOU'RE AN IDIOT - YOU CAN'T USE GIT\" alarm goes off).  Git \nis better than CVS/subversion in every respect - save this one.  It's almost \ncompletely free to do (apart from the initial coding of it of course) because \nof these two factors:\n - The keywords are collapsed in the repository\n - The keywords are only expanded on checkout\nIt doesn't fundamentally alter anything that git does right now.\n\n>  * We do not do the borrowing from working tree when doing\n>    grep_sha1(), but when we grep inside a file from working tree\n>    with grep_file(), we do not currently make it go through\n>    convert_to_git() to fix line endings.  Maybe we should, if\n>    only for consistency.\n\nI'd actually argue not - git-grep searches the working tree.  The expanded \nkeywords are in the working tree.  Take the CRLF case - I'm a clueless user, \nwho only understands the system I'm working on.  I want to search for all the \nline endings, so I do git-grep \"\\r\\n\" - that should work, because I'm \nsearching my working tree.\n\n>  * We do not currently run convert_to_git() on the patch text\n>    given to git-apply; we could do so in parse_single_patch().\n\nYep - definitely; the applied patch should certainly be normalised before \napplication.  I'd have to add it if I wanted keywords anyway wouldn't I?\n\n\n\nAndy\n-- \nDr Andy Parkins, M Eng (hons), MIET\nandyparkins@gmail.com\n"},{"id":"39658","messageId":"Pine.LNX.4.64.0704170829500.5473@woody.linux-foundation.org","threadId":"7715","inReplyTo":"200704171041.46176.andyparkins@gmail.com","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-17T15:32:23Z","receivedAt":"2007-04-17T15:32:23Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 17 Apr 2007, Andy Parkins wrote:\n> \n> Adding expansion is harder, as I have no clue which calls to make to find\n> even the most basic information about an object; but I thought I'd get\n> feedback before I expend that effort.\n\nAdding expansion is not just \"harder\". It's basically impossible to do \nwith any kind of performance.\n\nThink \"git checkout newbranch\".\n\nAnd think what we do about files (and whole subdirectories!) that haven't \neven changed. And finally, think about how important that optimization is \nin an SCM like git that supports branches.\n\nI think you'll find that keyword expansion simply isn't acceptable.\n\nBut hey, you didn't believe me, so I'm happy you are trying to write the \npatches. Either you'll prove me wrong, or you'll realize just *how* broken \nthe feature is.\n\n(Yeah, \"unexpansion\" is easy. It's easy for all the same reasons CRLF is \neasy: it has no state!)\n\n\t\t\tLinus\n"},{"id":"39661","messageId":"Pine.LNX.4.64.0704170833560.5473@woody.linux-foundation.org","threadId":"7715","inReplyTo":"7v7isbpb0p.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-17T15:46:27Z","receivedAt":"2007-04-17T15:46:27Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 17 Apr 2007, Junio C Hamano wrote:\n>\n> Andy Parkins <andyparkins@gmail.com> writes:\n> \n> > No parsing of the keyword itself is performed, the content is simply\n> > dropped.\n> \n> You are sidestepping the most important problem by doing this.\n\nI obviosly agree (and I agree with everything in your email), but:\n\n> The only sensible keyword you could have, without destroying\n> what git is, is blob id.  No commit id, no date, no author.\n\nYes. And I already talked about some of the very fundamental problems that \nkeyword expansion has (ie switching branches is basically impossible to do \nwithout checking out _every_single_file_ with the \"keyword\" attribute \nset. There are others).\n\nNow, unexpansion is trivial to do (it really *is* the same as the \n\"CRLF->LF\" translation: that's technically really just an \"unexpansion\" \ntoo). And it should work. \n\nThe way this does unexpansion also breaks \"git diff\" in that it bassically \nalways makes diff *ignore* the keywords. In other words, when you do\n\n\tgit diff A..B\n\nand send the diff to somebody else, they'll never see any keywords at all! \n\nNow, that obviously fulfills my requirement that the diff be empty if A \nand B are the same, so you should expect me to be happy. But I'm not \nhappy, because if the other person also is using git, HE CANNOT EVEN APPLY \nTHE DIFF! Even if he's at \"A\", and thus gets a diff that is supposed to \napply *exactly*, he'll get rejects if there were other changes around the \nunexpanded keyword (which *he* will have expanded in his working tree, of \ncourse!)\n\nSee? Keywords simply *cannot* work. They're broken. Either you can ignore \nthem (and not show them in diffs), in which case the diff is broken, or \nyou can not ignore them (and show them in diffs) in which case the diff is \n*also* broken, just differently.\n\nThe only sane and workable case is to not have them at all. Any keyword \nexpansion will *always* result in problems. You simply cannot do it right. \n\nAs I mentioned originally, it results in problems in CVS too, it's just \nthat CVS really has so many other issues that you seldom see the problems.\n\nOk, after that new rant against keywords, I will say one positive thing:\n\n - keyword *unexpansion* is certainly easy (exactly because it's \n   stateless)\n\n - if we want to support a git that only does \"unexpansion\", you can \n   probably hack around stupid release scripting more easily. You can add \n   your keywords *outside* of git, and git will simply ignore them. \n\nSo I'm actually not against keyword un-expansion. It has none of the \nfundamental problems that actually expanding the keywords has. It's \nliterally no different from CRLF->LF translation. It can cause confusion, \nbut if it has to be explicitly enabled with an attribute and is never done \nautomatically, then having some support for unexpansion and letting the \nuser who wants to use keywords use his own \"wrapper scripts\" around git to \ndo his own expansion, be my guest..\n\nYou would be unable to do fundamental operations like \"git checkout B\" to \njump to another branch, but if you don't support multiple branches and \nwant to just act like CVS, maybe git unexpanding the crap will help you: \nyou can add your own keywords, happy in the knowledge that git simply \nwon't *care* about them, and will never see them.\n\nSo I absolutely detest keyword expansion and actually have a lot of \narguments for why I don't think it *can* work even in theory (except by \nbeing totally unusable), but I don't have the *un*expansion. \n\n\t\tLinus\n"},{"id":"39664","messageId":"Pine.LNX.4.64.0704170847380.5473@woody.linux-foundation.org","threadId":"7715","inReplyTo":"200704171235.34793.andyparkins@gmail.com","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-17T15:53:46Z","receivedAt":"2007-04-17T15:53:46Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 17 Apr 2007, Andy Parkins wrote:\n\n> On Tuesday 2007 April 17 11:09, Junio C Hamano wrote:\n> \n> > In http://article.gmane.org/gmane.comp.version-control.git/44654,\n> > Linus said:\n> >     And *I* claim that if you don't get an immediate and empty diff, your\n> >     system is TOTALLY BROKEN.\n> \n> Well that one is easy - the file is normalised to contain collapsed keywords \n> upon checkin, so diff works the same as it ever did.  The output would be \n> immediate and empty so is not TOTALLY BROKEN.\n\nNo, it *is* TOTALLY BROKEN, because your keywords guaranteed that it \ndoesn't even *apply*.\n\nThat's such a fundamental part of a patch that I didn't even _mention_ it, \nbut I obviously should have.\n\nIf you cannot apply the diff you generate, what the hell is the *point* of \na diff?\n\nTry this:\n\n - File-A in revision 1:\n\n\t$ID: some random crap about rev1 $\n\tLine 2\n\n - same file in revision 2:\n\t$ID: some other random crap about rev2 $\n\tLine 2 got modified\n\nand think about it. Your diff will be something like\n\n\t@@ -1,2 +1,2 @@\n\t $ID:$\n\t-Line 2\n\t+Line 2 got modified\n\nand the diff WON'T EVEN APPLY!\n\nWhat kind of diff is that? Would you call it perhaps \"totally broken\"?\n\nIn other words, there's no way in hell you can make this work. You'll end \nup always having to edit the keywords parts of diffs to make them apply if \nthey are part of the context.\n\n(This, btw, is something that a CVS person says \"so what?\" about. They're \n_used_ to having to do it. It's how you do merges in CVS. Really. How many \npeople have actually *worked* with branches in CVS on any complex project \nwith any nontrivial work happening on the branch? I have. I hated CVS for \nmany reasons. Keywords was just a small small detail in that hate \nrelationship, but it was one of them!)\n\n\t\tLinus\n"},{"id":"39677","messageId":"200704171803.58940.andyparkins@gmail.com","threadId":"7715","inReplyTo":"Pine.LNX.4.64.0704170847380.5473@woody.linux-foundation.org","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-04-17T17:03:57Z","receivedAt":"2007-04-17T17:03:57Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"On Tuesday 2007, April 17, Linus Torvalds wrote:\n\n> Try this:\n>\n>  - File-A in revision 1:\n>\n> \t$ID: some random crap about rev1 $\n> \tLine 2\n>\n>  - same file in revision 2:\n> \t$ID: some other random crap about rev2 $\n> \tLine 2 got modified\n>\n> and think about it. Your diff will be something like\n>\n> \t@@ -1,2 +1,2 @@\n> \t $ID:$\n> \t-Line 2\n> \t+Line 2 got modified\n>\n> and the diff WON'T EVEN APPLY!\n\nWhy on earth would it not apply?  It's being applied using git-apply, \nwhich will unexpand the keywords as it goes - as I keep saying.  When \nthe apply engine is looking for the context it's going to collapse the \nkeyword, so the context will match and the diff WILL EVEN APPLY.\n\nAs Junio said in his reply, git-apply doesn't currently call \nconvert_to_git(), but that's easily implemented.\n\n> In other words, there's no way in hell you can make this work. You'll\n\nYou keep saying these sweepingly general things.  It can be made to \nwork.\n\n> end up always having to edit the keywords parts of diffs to make them\n> apply if they are part of the context.\n\nNo I don't.  If I had to then the keyword code would be broken.  No one \nin their right mind would think that was an acceptable thing to do.\n\n> (This, btw, is something that a CVS person says \"so what?\" about.\n> They're _used_ to having to do it. It's how you do merges in CVS.\n> Really. How many people have actually *worked* with branches in CVS\n\nThat's because CVS is rubbish.  What has that got to do with it?\n\n> on any complex project with any nontrivial work happening on the\n> branch? I have. I hated CVS for many reasons. Keywords was just a\n> small small detail in that hate relationship, but it was one of\n> them!)\n\nYou really can stop trying to persuade me that CVS is no good for \nversion control - I agree, a thousand times I agree.  There are a lot \nof things that CVS does in a broken manner, that doesn't mean that git \ndoes the same thing in a broken manner.\n\n\n\nAndy\n\n-- \nDr Andy Parkins, M Eng (hons), MIET\nandyparkins@gmail.com\n"},{"id":"39680","messageId":"200704171810.31797.andyparkins@gmail.com","threadId":"7715","inReplyTo":"Pine.LNX.4.64.0704170829500.5473@woody.linux-foundation.org","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-04-17T17:10:30Z","receivedAt":"2007-04-17T17:10:30Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"On Tuesday 2007, April 17, Linus Torvalds wrote:\n\n> And think what we do about files (and whole subdirectories!) that\n> haven't even changed. And finally, think about how important that\n> optimization is in an SCM like git that supports branches.\n>\n> I think you'll find that keyword expansion simply isn't acceptable.\n\nAs I said in my reply to Junio; other VCSs only expand keywords when the \nfile itself is checked out - when git's uber-fast switching decides \nthat fileA is the same in the source and target checkouts, then the \nkeywords won't be updated - fine.\n\nIn this one respect git would be \"as good as\" instead of \"infinitely \nbetter\".  I can live with that.\n\n> But hey, you didn't believe me, so I'm happy you are trying to write\n> the patches. Either you'll prove me wrong, or you'll realize just\n> *how* broken the feature is.\n\nCould be - I'm happy to disagree.  I'm even happy to accept that I might \nfail miserably.\n\n\nAndy\n-- \nDr Andy Parkins, M Eng (hons), MIET\nandyparkins@gmail.com\n"},{"id":"39682","messageId":"46250175.4020300@dawes.za.net","threadId":"7715","inReplyTo":"Pine.LNX.4.64.0704170829500.5473@woody.linux-foundation.org","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Rogan Dawes","fromEmail":"lists@dawes.za.net","sentAt":"2007-04-17T17:18:45Z","receivedAt":"2007-04-17T17:18:45Z","isPatch":true,"sender":{"key":"lists@dawes.za.net","avatar":null},"body":"Linus Torvalds wrote:\n> \n> On Tue, 17 Apr 2007, Andy Parkins wrote:\n>> Adding expansion is harder, as I have no clue which calls to make to find\n>> even the most basic information about an object; but I thought I'd get\n>> feedback before I expend that effort.\n> \n> Adding expansion is not just \"harder\". It's basically impossible to do \n> with any kind of performance.\n> \n> Think \"git checkout newbranch\".\n> \n> And think what we do about files (and whole subdirectories!) that haven't \n> even changed. And finally, think about how important that optimization is \n> in an SCM like git that supports branches.\n\nWell, if the only keyword we support is $BlobId:$, then if the \ntree/object hasn't changed, then we still don't need to touch the object.\n\nNot so?\n\nRogan\n"},{"id":"39686","messageId":"Pine.LNX.4.64.0704171107510.5473@woody.linux-foundation.org","threadId":"7715","inReplyTo":"200704171803.58940.andyparkins@gmail.com","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-17T18:12:34Z","receivedAt":"2007-04-17T18:12:34Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 17 Apr 2007, Andy Parkins wrote:\n> \n> Why on earth would it not apply?  It's being applied using git-apply, \n> which will unexpand the keywords as it goes - as I keep saying.\n\nSo you will never work with anybody outside of git?\n\nWhat about tar-files when you export the tree? Should they have the \nexpanded version? \n\n> You keep saying these sweepingly general things.  It can be made to \n> work.\n\nNo, it CANNOT.\n\nTrust me. There's NO WAY IN HELL it will \"work\" in any other sense than \n\"limp along and not be usable\".\n\nYes, you can make it \"work\" if you:\n\n - make sure that you never _ever_ leave the git environment\n\n   But why do you want keyword expansion then? The whole point is if you \n   have other tools than the git tools that look at a file. Even your svg \n   example was literally about having non-git tools work with the data. \n   What if you ever email the file to somebody else? \n\n - you make all git tools explicitly always strip them.\n\n   Again, what's the point again? You add keyword expansion, and then the \n   only tools that you really allow to touch it (except your \"print it \n   out\" example) will have to remove the keyword expansion just to work.\n\nThat's not \"work\". That's just stupid. Yes, you can make your \"print it \nout\" example work, but as alreadyt mentioned, you could have done that \nsome other way, with a simple makefile rule, quite independently (and much \nbetter) than the SCM ever did.\n\n\t\tLinus\n"},{"id":"39687","messageId":"Pine.LNX.4.64.0704171121090.5473@woody.linux-foundation.org","threadId":"7715","inReplyTo":"46250175.4020300@dawes.za.net","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-17T18:23:03Z","receivedAt":"2007-04-17T18:23:03Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 17 Apr 2007, Rogan Dawes wrote:\n> \n> Well, if the only keyword we support is $BlobId:$, then if the tree/object\n> hasn't changed, then we still don't need to touch the object.\n> \n> Not so?\n\nCorrect. However, is that actually a useful expansion?\n\nMost of the time, I'd expect people to want things like \"last committer, \ntime, story of their life\" etc.. I don't think the SHA1 ID's are pretty \nenough that anybody would ever want to see them. But yes, they are \ncertainly stable.\n\n\t\t\tLinus\n"},{"id":"39688","messageId":"200704172012.31280.andyparkins@gmail.com","threadId":"7715","inReplyTo":"Pine.LNX.4.64.0704171107510.5473@woody.linux-foundation.org","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-04-17T19:12:27Z","receivedAt":"2007-04-17T19:12:27Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"On Tuesday 2007, April 17, Linus Torvalds wrote:\n\n> So you will never work with anybody outside of git?\n\nFor my projects - correct; I don't care about the rest of the world.  \nFor projects that do - don't enable keywords, it's an option, all I \nwant is to have that option.\n\n> What about tar-files when you export the tree? Should they have the\n> expanded version?\n\nIf I have to pick one then: no.  I think out-of-tree keywords are too \nmuch trouble for exactly the reasons you say; however, I wouldn't like \nto presume what other people think is too much trouble so I suppose it \nwould have to be an option.\n\n> > You keep saying these sweepingly general things.  It can be made to\n> > work.\n>\n> No, it CANNOT.\n>\n> Trust me. There's NO WAY IN HELL it will \"work\" in any other sense\n> than \"limp along and not be usable\".\n\nWell I'm making progress, \"limp along\" is a significant step up from \nimpossible.  :-)\n\nLook, my primary objection to this is the SHOUTING about how impossible \nit is even though I've tried to address every problem you've thrown at \nme - I'm finding it really difficult to figure out why you're trying so \nhard to dissuade me from even _trying_.  If it all goes wrong (as I \nfully accept it might), so be it, I can live with that; I'll even be \nhappy to tell you you're right and I'm wrong.  Why is this such a \nproblem?\n\nKeywords are so hated by everyone that I doubt they would ever be \naccepted into git - it's an intellectual exercise for me at this stage \nreally. \n\n> Yes, you can make it \"work\" if you:\n>\n>  - make sure that you never _ever_ leave the git environment\n\nAs it happens, _I_ never ever leave the git environment.  Can I use \nkeywords then?\n\nYou don't seem to have such a problem with git's extended diffs for \nrenames or subprojects - \"make sure that you never _ever_ leave the git \nenvironment\".\n\n>    But why do you want keyword expansion then? The whole point is if\n> you have other tools than the git tools that look at a file. Even\n> your svg example was literally about having non-git tools work with\n> the data. What if you ever email the file to somebody else?\n\nIf by \"tools\" you mean other version control systems, then I don't \nintend them to work.  If by \"tools\" you mean gcc, inkscape, gv, bash, \nweb browsers or any other fileformat that allows comments in the file \nthen I expect it to be fine.  If I publish a web page, it'd be nice to \nshow the ID on the page - that's all just \"nice\" not \"necessary\" \nor \"I'm throwing git away if I don't get it\".\n\nEmailing to others isn't a problem either: let's say I email them my SVG \n(with keywords expanded), they make some edits and send it me back - \nworse, they send me a diff back.  I'm going to apply that diff using \ngit-apply; which will collapse the keywords and apply the diff.\n\n>  - you make all git tools explicitly always strip them.\n\nWell, not \"all\", so far I've added one call to convert_to_git() in \nbuiltin-apply.c - it was a one line addition.  It needed doing anyway \nto deal with the CRLF correctly.  I can't see there being that many \nplaces that this needs doing.  I may well be wrong, if I end up \nscattering calls to convert_to_git() everywhere I'll give up.\n\n>    Again, what's the point again? You add keyword expansion, and then\n> the only tools that you really allow to touch it (except your \"print\n> it out\" example) will have to remove the keyword expansion just to\n> work.\n\n(I don't see why my tiny \"print it out\" example isn't enough - it \nmatters to me)\n\nHowever, most tools don't care about the keywords, it's only non-git \ndiff and non-git patch that are affected.  As long as the file format \nsupports comments, then keyword expansion will be just fine.\n\n> That's not \"work\". That's just stupid. Yes, you can make your \"print\n> it out\" example work, but as alreadyt mentioned, you could have done\n> that some other way, with a simple makefile rule, quite independently\n> (and much better) than the SCM ever did.\n\nThat's just being obtuse - no other tool cares in the slightest about \nthe keywords, there are more \"tools\" in the world than just the VCS.\n\n\n\nAndy\n\n-- \nDr Andy Parkins, M Eng (hons), MIET\nandyparkins@gmail.com\n"},{"id":"39689","messageId":"alpine.LFD.0.98.0704171530220.4504@xanadu.home","threadId":"7715","inReplyTo":"200704172012.31280.andyparkins@gmail.com","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-04-17T19:41:29Z","receivedAt":"2007-04-17T19:41:29Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Tue, 17 Apr 2007, Andy Parkins wrote:\n\n> On Tuesday 2007, April 17, Linus Torvalds wrote:\n> \n> > Trust me. There's NO WAY IN HELL it will \"work\" in any other sense\n> > than \"limp along and not be usable\".\n> \n> Well I'm making progress, \"limp along\" is a significant step up from \n> impossible.  :-)\n> \n> Look, my primary objection to this is the SHOUTING about how impossible \n> it is even though I've tried to address every problem you've thrown at \n> me - I'm finding it really difficult to figure out why you're trying so \n> hard to dissuade me from even _trying_.  If it all goes wrong (as I \n> fully accept it might), so be it, I can live with that; I'll even be \n> happy to tell you you're right and I'm wrong.  Why is this such a \n> problem?\n> \n> Keywords are so hated by everyone that I doubt they would ever be \n> accepted into git - it's an intellectual exercise for me at this stage \n> really. \n\nI cannot do otherwise than ask at this point in the debate: why isn't \nthe makefile rule sufficient for your needs?  Why going through a \ncomplicated path that no one else will support due to its numerous \npitfalls?\n\n> > Yes, you can make your \"print it out\" example work, but as alreadyt \n> > mentioned, you could have done that some other way, with a simple \n> > makefile rule, quite independently (and much better) than the SCM \n> > ever did.\n> \n> That's just being obtuse - no other tool cares in the slightest about \n> the keywords, there are more \"tools\" in the world than just the VCS.\n\n... which reinforces my question: why force a task on the VCS if it \ndoesn't fit well with its fundamental design?\n\n\nNicolas\n"},{"id":"39693","messageId":"Pine.LNX.4.63.0704171244450.1696@qynat.qvtvafvgr.pbz","threadId":"7715","inReplyTo":"alpine.LFD.0.98.0704171530220.4504@xanadu.home","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"David Lang","fromEmail":"david.lang@digitalinsight.com","sentAt":"2007-04-17T19:45:54Z","receivedAt":"2007-04-17T19:45:54Z","isPatch":true,"sender":{"key":"david.lang@digitalinsight.com","avatar":null},"body":"On Tue, 17 Apr 2007, Nicolas Pitre wrote:\n\n> \n> On Tue, 17 Apr 2007, Andy Parkins wrote:\n>\n>> On Tuesday 2007, April 17, Linus Torvalds wrote:\n>>\n>>> Trust me. There's NO WAY IN HELL it will \"work\" in any other sense\n>>> than \"limp along and not be usable\".\n>>\n>> Well I'm making progress, \"limp along\" is a significant step up from\n>> impossible.  :-)\n>>\n>> Look, my primary objection to this is the SHOUTING about how impossible\n>> it is even though I've tried to address every problem you've thrown at\n>> me - I'm finding it really difficult to figure out why you're trying so\n>> hard to dissuade me from even _trying_.  If it all goes wrong (as I\n>> fully accept it might), so be it, I can live with that; I'll even be\n>> happy to tell you you're right and I'm wrong.  Why is this such a\n>> problem?\n>>\n>> Keywords are so hated by everyone that I doubt they would ever be\n>> accepted into git - it's an intellectual exercise for me at this stage\n>> really.\n>\n> I cannot do otherwise than ask at this point in the debate: why isn't\n> the makefile rule sufficient for your needs?  Why going through a\n> complicated path that no one else will support due to its numerous\n> pitfalls?\n\nnot all uses of VCS's involve useing make\n\n>>> Yes, you can make your \"print it out\" example work, but as alreadyt\n>>> mentioned, you could have done that some other way, with a simple\n>>> makefile rule, quite independently (and much better) than the SCM\n>>> ever did.\n>>\n>> That's just being obtuse - no other tool cares in the slightest about\n>> the keywords, there are more \"tools\" in the world than just the VCS.\n>\n> ... which reinforces my question: why force a task on the VCS if it\n> doesn't fit well with its fundamental design?\n\nbecouse the VCS can do the job better then anything else? even if there are \nlimits to what the VCS can do.\n\nDavid Lang\n"},{"id":"39691","messageId":"Pine.LNX.4.64.0704171229360.5473@woody.linux-foundation.org","threadId":"7715","inReplyTo":"200704172012.31280.andyparkins@gmail.com","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-17T19:54:49Z","receivedAt":"2007-04-17T19:54:49Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 17 Apr 2007, Andy Parkins wrote:\n> \n> Look, my primary objection to this is the SHOUTING about how impossible \n> it is even though I've tried to address every problem you've thrown at \n> me\n\nNo, you haven't. You've \"addressed\" them by stating they don't matter. It \ndoesn't \"matter\" that a diff won't actually apply to a checked-out tree, \nbecause you fix it up in another tool.\n\nAnd it doesn't \"matter\" that switching branches will just result in the \nwrong keyword expansion, because you don't care about the keywords \nactually being \"correct\" - they are just random strings, and it apparently \ndoesn't really have to \"work\" as far as you're concerned.\n\nAnd the \"git grep\" concern you just dismissed by stating that it should \nuse the filesystem copy, never mind that this just means that a clean \nworking tree gets different results from doing the same thing based on \nthat same revision.\n\nIn other words, you simply don't seem to worry about TRUSTING the results. \nIt's ok if patches don't apply, or if you get different results on working \ntrees than \"inside\" the revision control.\n\nAnd the reaon I'm shouting is that \"it doesn't matter that it's a bit \nhacky\" mentality is what gets you things like CVS in the end. Bit-for-bit \nresults actually matter. Guarantees actually matter. And you should not be \nable to see a differece in the working tree just because you happened to \nbe on a different branch before.\n\nThose are the kind of nasty surprises that make people go: \"I don't know \nwhat the end result is, because there is an element of 'just how did you \nhappen to do that operation' to it\".\n\nI want to *trust* the SCM I use.\n\n> I'm finding it really difficult to figure out why you're trying so \n> hard to dissuade me from even _trying_.\n\nYou can try, but you are *ignoring* the things that I say. The end result \nwill either perform really badly, or you cannot trust it, or *both*. And \nyou'll introduce interesting semantics like \"diffs won't actually apply to \nthe working tree with normal tools\".\n\n(And yes, git diffs are extended, but they *do* apply to working trees in \nall cases where normal \"patch\" can even support the notion in the first \nplace.)\n\nAnd it's not just things like diff and switching branches. If you want \nyour keywords to generate things like \"last modified by Xyzzy\", you \nhaven't even explained *how* you'd do that. Yeah, you can do\n\n\tgit log --pretty=oneline --abbrev-commit -1 -- filename\n\netc, and you probably think it's instantaneous, but do the timings for a \nbig repository with a file that hasn't been modified in months, and then \nimagine doing that for an initial checkout (say, after you set the \n\"keyword\" attribute for all *.c files).\n\nWhoops. The checkout took an hour. Is that really a path you want to go \ndown?\n\n> Keywords are so hated by everyone that I doubt they would ever be \n> accepted into git - it's an intellectual exercise for me at this stage \n> really. \n\nIf that's what it is, fine. But people on the list seem to actually *want* \nit. They must be educated what a *disaster* it would be to actually try to \nreally support something like it in real life, and not just as a mental \nexercise.\n\n\t\tLinus\n"},{"id":"39697","messageId":"Pine.LNX.4.63.0704171302200.1696@qynat.qvtvafvgr.pbz","threadId":"7715","inReplyTo":"alpine.LFD.0.98.0704171624190.4504@xanadu.home","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"David Lang","fromEmail":"david.lang@digitalinsight.com","sentAt":"2007-04-17T20:05:03Z","receivedAt":"2007-04-17T20:05:03Z","isPatch":true,"sender":{"key":"david.lang@digitalinsight.com","avatar":null},"body":"On Tue, 17 Apr 2007, Nicolas Pitre wrote:\n\n> On Tue, 17 Apr 2007, David Lang wrote:\n>\n>> On Tue, 17 Apr 2007, Nicolas Pitre wrote:\n>>\n>>> I cannot do otherwise than ask at this point in the debate: why isn't\n>>> the makefile rule sufficient for your needs?  Why going through a\n>>> complicated path that no one else will support due to its numerous\n>>> pitfalls?\n>>\n>> not all uses of VCS's involve useing make\n>\n> Use perl then.  Or a shell script.  Or even a command.com batch script.\n> Or your own tool.\n\nI would like to, however this doesn't currently integrate well with git. I've \nbeen told in the past that once .gitattributes is in place then the hooks for \nthe crlf stuff can be generalized to allow for calls out to custom code to do \nthis sort of thing.\n\nhowever now it sounds as if people are saying that doing this is so evil that it \nshouldn't ever be allowed.\n\n>>>> That's just being obtuse - no other tool cares in the slightest about\n>>>> the keywords, there are more \"tools\" in the world than just the VCS.\n>>>\n>>> ... which reinforces my question: why force a task on the VCS if it\n>>> doesn't fit well with its fundamental design?\n>>\n>> becouse the VCS can do the job better then anything else?\n>\n> On what basis?\n>\n>> even if there are\n>> limits to what the VCS can do.\n>\n> In the context of keyword expansion I don't agree at all with this\n> statement.  Git can *not* do better than an external tool and it has\n> been demonstrated a few times already.\n\nthe VCS can make sure that the appropriate external code is always run when \nthings are checked in/out. external tools (unless they are a complete set of \nwrappers for git) can't do that.\n\nDavid Lang\n"},{"id":"39695","messageId":"46252DAE.4020604@dawes.za.net","threadId":"7715","inReplyTo":"Pine.LNX.4.64.0704171121090.5473@woody.linux-foundation.org","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Rogan Dawes","fromEmail":"lists@dawes.za.net","sentAt":"2007-04-17T20:27:26Z","receivedAt":"2007-04-17T20:27:26Z","isPatch":true,"sender":{"key":"lists@dawes.za.net","avatar":null},"body":"Linus Torvalds wrote:\n> \n> On Tue, 17 Apr 2007, Rogan Dawes wrote:\n>> Well, if the only keyword we support is $BlobId:$, then if the tree/object\n>> hasn't changed, then we still don't need to touch the object.\n>>\n>> Not so?\n> \n> Correct. However, is that actually a useful expansion?\n> \n> Most of the time, I'd expect people to want things like \"last committer, \n> time, story of their life\" etc.. I don't think the SHA1 ID's are pretty \n> enough that anybody would ever want to see them. But yes, they are \n> certainly stable.\n> \n> \t\t\tLinus\n\nWell, one example for wanting a keyword expansion option was where \npeople modify the entire file, and just email it back to the maintainer. \nIt surely helps to have the SHA1 of the original object when applying \nthe changes.\n\nYou also stated in another email that doing keyword expansion prevents \npeople from using non-git tools. I agree that you'd probably end up with \ndiffs that may include the keyword (object id) being mailed to you if \nthe submitter is not using git. But when a git maintainer applies those \ndiffs using git-apply, the keyword unexpansion could still take place, \nmaking the diffs usable in practice.\n\nNone of what I said necessarily supports the view that it is a good idea \nfrom the perspective of trusting the results, of course.\n\nRegards,\n\nRogan\n"},{"id":"39696","messageId":"alpine.LFD.0.98.0704171624190.4504@xanadu.home","threadId":"7715","inReplyTo":"Pine.LNX.4.63.0704171244450.1696@qynat.qvtvafvgr.pbz","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-04-17T20:29:49Z","receivedAt":"2007-04-17T20:29:49Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Tue, 17 Apr 2007, David Lang wrote:\n\n> On Tue, 17 Apr 2007, Nicolas Pitre wrote:\n> \n> > I cannot do otherwise than ask at this point in the debate: why isn't\n> > the makefile rule sufficient for your needs?  Why going through a\n> > complicated path that no one else will support due to its numerous\n> > pitfalls?\n> \n> not all uses of VCS's involve useing make\n\nUse perl then.  Or a shell script.  Or even a command.com batch script.  \nOr your own tool.\n\n> > > That's just being obtuse - no other tool cares in the slightest about\n> > > the keywords, there are more \"tools\" in the world than just the VCS.\n> > \n> > ... which reinforces my question: why force a task on the VCS if it\n> > doesn't fit well with its fundamental design?\n> \n> becouse the VCS can do the job better then anything else?\n\nOn what basis?\n\n> even if there are\n> limits to what the VCS can do.\n\nIn the context of keyword expansion I don't agree at all with this \nstatement.  Git can *not* do better than an external tool and it has \nbeen demonstrated a few times already.\n\n\nNicolas\n"},{"id":"39699","messageId":"200704172146.33665.andyparkins@gmail.com","threadId":"7715","inReplyTo":"Pine.LNX.4.64.0704171229360.5473@woody.linux-foundation.org","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-04-17T20:46:20Z","receivedAt":"2007-04-17T20:46:20Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"On Tuesday 2007, April 17, Linus Torvalds wrote:\n\n> No, you haven't. You've \"addressed\" them by stating they don't\n> matter. It doesn't \"matter\" that a diff won't actually apply to a\n> checked-out tree, because you fix it up in another tool.\n\nOkay.  I think this is a matter of perspective - my perspective is that \nif it supplies what svn/cvs supply then that would please the people \nwho want it (of whom I am one); yours is obviously that if it isn't \nperfect, it's not worth doing.  That's a reasonable thing to demand, \nand I'm not going to try and argue you out of it.\n\n> And it doesn't \"matter\" that switching branches will just result in\n> the wrong keyword expansion, because you don't care about the\n> keywords actually being \"correct\" - they are just random strings, and\n> it apparently doesn't really have to \"work\" as far as you're\n> concerned.\n\nIf you define \"work\" as \"works like cvs/svn does\", then I was fine with \nit.  I don't like it when my favourite VCS, that I want everyone to \nuse, doesn't have an answer to \"but does it do X?\".\n\n> And the \"git grep\" concern you just dismissed by stating that it\n> should use the filesystem copy, never mind that this just means that\n> a clean working tree gets different results from doing the same thing\n> based on that same revision.\n\nAs I said at the time, I just picked one of the two options.  If you \ndon't like that, pick the other option - collapse the keywords during \nthe grep...\n\n> And the reaon I'm shouting is that \"it doesn't matter that it's a bit\n> hacky\" mentality is what gets you things like CVS in the end.\n> Bit-for-bit results actually matter. Guarantees actually matter. And\n> you should not be able to see a differece in the working tree just\n> because you happened to be on a different branch before.\n\nBit-for-bit as in CRLF is untouched?  No?  Bit-for-bit as in you said \nyou were okay with keyword-collapsing but not expansion?  You're just \nas willing to compromise as me, you've just drawn the line in a \ndifferent place.\n\nIncidentally: for future reference, I'll read what you write regardless \nof whether you shout it or not.\n\n> You can try, but you are *ignoring* the things that I say. The end\n\nI've tried very hard to respond to every point you've put to me; I've \nnot selectively chopped out bits, and I've tried to give answers that \nmake it work as you ask.  Now, none of those things were acceptable to \nyou - which is fine - but I certinaly wasn't ignoring what you say - \n_disagreeing with_ is not the same as ignoring.\n\n> If that's what it is, fine. But people on the list seem to actually\n> *want* it. They must be educated what a *disaster* it would be to\n> actually try to really support something like it in real life, and\n> not just as a mental exercise.\n\nPeople wanting something \"wrong\" so much is not a sign that they need \neducating, it's a sign that they need a solution.   In every other \nrespect git has a solution for them; rather than explaining to them \nthat what they want is stupid, I'd offer that it's more appropriate to \noffer something better in exchange.  So my keyword expansion idea is \nwrong - fine - where's the something better?  Writing custom scripts \nand makefiles for every project I ever run is /not/ \"something better\".\n\nAnyway, it's late, and I'm tired - this has turned into a battle of \nwills, and I'm not that into battling.   Enough antihistamine has been \npoured on my itch that I no longer want to scratch it.  I'll send my \nmost recent patch for the sake of history, and then abandon this \nproject.\n\nThanks for your time on this, I appreciate your detailed responses, even \nif we don't agree.\n\n\n\nAndy\n-- \nDr Andy Parkins, M Eng (hons), MIET\nandyparkins@gmail.com\n"},{"id":"39701","messageId":"200704172152.58870.andyparkins@gmail.com","threadId":"7715","inReplyTo":"200704172146.33665.andyparkins@gmail.com","subject":"[PATCH] Add keyword collapse support to convert.c","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-04-17T20:52:58Z","receivedAt":"2007-04-17T20:52:58Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"This patch adds expansion of keywords support.  The collapse is only\nperformed when the \"keywords\" attribute it found for a file.  The check\nfor this attribute is done in the same way as the \"crlf\" attribute\ncheck.\n\nThe actual collapse is performed by keyword_collapse_git() which is\ncalled from convert_to_git() when the \"keywords\" attribute is found.\n\nkeyword_collapse_git() finds strings of the form\n\n $KEYWORD: ARBITRARY STRING$\n\nAnd collapses them into\n\n $KEYWORD:$\n\nNo parsing of the keyword itself is performed, the content is simply\ndropped.\n\nDespite the fact that this doesn't do anything useful from the users\nperspective, this patch forms the more important half of keyword\nexpansion support - because it prevents the expansion from entering the\nrepository.  It effectively creates blind spots that git tools won't\nsee.\n\nconvert_to_git() has also been changed so that it no longer only does\nCRLF conversion.  Instead, a flag is kept to say whether any conversion\nwas done by the CRLF code, and then that converted buffer is passed to\nkeyword_collapse_git() and the flag again updated.  It then returns 1 if\neither of these conversion functions actually changed anything.\n\nI've also included a test script to show that the keyword collapse is\nworking.  It particular demonstrates that the diff between a file with\nkeywords and the repository is blind to the expanded keyword.\n\ngit-apply is patched to perform the collapse as well on each fragment.\n\nSigned-off-by: Andy Parkins <andyparkins@gmail.com>\n---\n\nThis is on top of 1ddfc1ad616550764056077b9e12a35533298c89.\n\nI'm positing it for posterity.  It's not going anywhere though, so I'm not\nsubmitting it for inclusion.\n\n\n builtin-apply.c     |    2 +\n convert.c           |  123 +++++++++++++++++++++++++++++++++++++++++++++++++-\n t/t0030-keywords.sh |   95 +++++++++++++++++++++++++++++++++++++++\n 3 files changed, 217 insertions(+), 3 deletions(-)\n create mode 100755 t/t0030-keywords.sh\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex fd92ef7..212c7d4 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -1056,6 +1056,8 @@ static int parse_single_patch(char *line, unsigned long size, struct patch *patc\n \tunsigned long oldlines = 0, newlines = 0, context = 0;\n \tstruct fragment **fragp = &patch->fragments;\n \n+\tconvert_to_git( patch->new_name, &line, &size );\n+\n \twhile (size > 4 && !memcmp(line, \"@@ -\", 4)) {\n \t\tstruct fragment *fragment;\n \t\tint len;\ndiff --git a/convert.c b/convert.c\nindex d0d4b81..a18e7ea 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -230,16 +230,133 @@ static int git_path_check_crlf(const char *path)\n \treturn attr_crlf_check.isset;\n }\n \n+/* ------------------ keywords -------------------- */\n+\n+static void setup_keyword_check(struct git_attr_check *check)\n+{\n+\tstatic struct git_attr *attr_keyword;\n+\n+\tif (!attr_keyword)\n+\t\tattr_keyword = git_attr(\"keywords\", 8);\n+\tcheck->attr = attr_keyword;\n+}\n+\n+static int git_path_check_keyword(const char *path)\n+{\n+\tstruct git_attr_check attr_keyword_check;\n+\n+\tsetup_keyword_check(&attr_keyword_check);\n+\n+\tif (git_checkattr(path, 1, &attr_keyword_check))\n+\t\treturn -1;\n+\treturn attr_keyword_check.isset;\n+}\n+\n+static int keyword_collapse_git(const char *path, char **bufp, unsigned long *sizep)\n+{\n+\tchar *buffer, *nbuf, *keyword;\n+\tunsigned long size, keywordlength;\n+\tint changes = 0;\n+\tenum {\n+\t\tIN_VOID,\n+\t\tPRE_KEYWORD,\n+\t\tIN_KEYWORD,\n+\t\tIN_EXPANSION,\n+\t\tEND_KEYWORD\n+\t} parser_state = IN_VOID;\n+\n+\tsize = *sizep;\n+\tif (!size)\n+\t\treturn 0;\n+\tbuffer = *bufp;\n+\n+\t/*\n+\t * Allocate an identically sized buffer, keyword collapse can\n+\t * only reduce the size so we'll never overflow (although we might\n+\t * waste a few bytes\n+\t */\n+\tnbuf = xmalloc(size);\n+\t*bufp = nbuf;\n+\n+\twhile (size) {\n+\t\tunsigned char c;\n+\n+\t\tc = *buffer;\n+\n+\t\tswitch( parser_state ) {\n+\t\tcase IN_VOID:        /* Normal characters, wait for '$' */\n+\t\t\tif (c == '$')\n+\t\t\t\tparser_state = PRE_KEYWORD;\n+\t\t\tbreak;\n+\t\tcase PRE_KEYWORD:    /* Gap between '$' and keyword */\n+\t\t\tkeywordlength = 0;\n+\t\t\tkeyword = buffer;\n+\t\t\tif (!isspace(c))\n+\t\t\t\tparser_state = IN_KEYWORD;\n+\t\t\telse\n+\t\t\t\tbreak;\n+\t\tcase IN_KEYWORD:     /* Keyword itself */\n+\t\t\tif (c == ':')\n+\t\t\t\tparser_state = IN_EXPANSION;\n+\t\t\telse if (c == '$' || c == '\\n' || c == '\\r' || c == '\\0' )\n+\t\t\t\tparser_state = END_KEYWORD;\n+\t\t\telse\n+\t\t\t\tkeywordlength++;\n+\t\t\tbreak;\n+\t\tcase IN_EXPANSION:   /* The expansion gets silently removed */\n+\t\t\tif (c == '$' || c == '\\n' || c == '\\r' || c == '\\0' )\n+\t\t\t\tparser_state = END_KEYWORD;\n+\t\t\telse {\n+\t\t\t\tchanges = 1;\n+\t\t\t\t/* Every character we skip reduces the overall size */\n+\t\t\t\t(*sizep)--;\n+\t\t\t\tbuffer++;\n+\t\t\t\tsize--;\n+\t\t\t}\n+\t\t\tcontinue;\n+\t\tcase END_KEYWORD:    /* End of keyword */\n+\t\t\tparser_state = IN_VOID;\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\t*nbuf++ = c;\n+\t\tbuffer++;\n+\t\tsize--;\n+\t}\n+\n+\treturn (changes != 0);\n+}\n+\n+\n+/* ------------------------------------------------ */\n int convert_to_git(const char *path, char **bufp, unsigned long *sizep)\n {\n+\tint changes = 0;\n+\n \tswitch (git_path_check_crlf(path)) {\n \tcase 0:\n-\t\treturn 0;\n+\t\tchanges += 0;\n+\t\tbreak;\n+\tcase 1:\n+\t\tchanges += forcecrlf_to_git(path, bufp, sizep);\n+\t\tbreak;\n+\tdefault:\n+\t\tchanges += autocrlf_to_git(path, bufp, sizep);\n+\t\tbreak;\n+\t}\n+\n+\tswitch (git_path_check_keyword(path)) {\n+\tcase 0:\n+\t\tchanges += 0;\n+\t\tbreak;\n \tcase 1:\n-\t\treturn forcecrlf_to_git(path, bufp, sizep);\n+\t\tchanges += keyword_collapse_git(path, bufp, sizep);\n+\t\tbreak;\n \tdefault:\n-\t\treturn autocrlf_to_git(path, bufp, sizep);\n+\t\tchanges += 0;\n+\t\tbreak;\n \t}\n+\treturn (changes != 0);\n }\n \n int convert_to_working_tree(const char *path, char **bufp, unsigned long *sizep)\ndiff --git a/t/t0030-keywords.sh b/t/t0030-keywords.sh\nnew file mode 100755\nindex 0000000..5180b0e\n--- /dev/null\n+++ b/t/t0030-keywords.sh\n@@ -0,0 +1,95 @@\n+#!/bin/sh\n+\n+cd $(dirname $0)\n+\n+test_description='Keyword expansion'\n+\n+. ./test-lib.sh\n+\n+# Adding the attribute \"keywords\" turns the keyword expansion on\n+# I've used \"notkeywords\" as an attribute as a placeholder attribute\n+# but this is just \"somerandomattribute\", it has no meaning\n+\n+# Expect success because the keyword attribute should be found\n+test_expect_success 'Keywords attribute present' '\n+\n+\techo \"keywordsfile keywords\" >.gitattributes &&\n+\n+\techo \"\\$keyword: anythingcangohere\\$\" > keywordsfile &&\n+\n+\tgit add keywordsfile &&\n+\tgit add .gitattributes &&\n+\tgit commit -m test-keywords &&\n+\n+\tgit check-attr keywords -- keywordsfile\n+'\n+\n+# Expect failure because the repository version should be different from the\n+# working tree version.\n+#\n+#  In repository : $keyword:$\n+#  In working dir: $keyword: anythingcangohere$\n+#\n+test_expect_failure 'Keywords collapse active' '\n+\n+\tgit show HEAD:keywordsfile > keywordsfile.cmp &&\n+\tcmp keywordsfile keywordsfile.cmp\n+\n+'\n+\n+# expect success because we want to find the keyword line collapsed in the\n+# and hence appearing unchanged in the output of git-diff\n+test_expect_success 'git-diff with keywords present' '\n+\techo \"Non-keyword containing line\" >> keywordsfile &&\n+\tgit diff -- keywordsfile | grep -qs \"^ \\$keyword:\\$$\"\n+'\n+\n+# Check git-apply blindness\n+cat > keyword-patch.diff << EOF\n+diff --git a/keywordsfile b/keywordsfile\n+--- a/keywordsfile\n++++ b/keywordsfile\n+@@ -1,2 +1,2 @@\n+ \\$keyword:\\$\n+-Non-keyword containing line\n++Another non-keyword containing line\n+EOF\n+\n+test_expect_success 'patch application with keywords active' '\n+\tgit-apply --check keyword-patch.diff\n+'\n+\n+# Expect failure because the keywords attribute should NOT be found\n+test_expect_failure 'Keywords attribute absent' '\n+\n+\techo \"keywordsfile notkeywords\" >.gitattributes &&\n+\n+\tgit add .gitattributes &&\n+\tgit commit -m test-not-keywords &&\n+\n+\tgit check-attr keywords -- keywordsfile\n+\n+'\n+\n+# If keywords are later disabled on that file, then the keyword collapsed\n+# will be ignored, so a diff should now show differences, because git is no\n+# longer keyword blind\n+test_expect_success 'git-diff with keywords in file but disabled' '\n+\tgit diff -- keywordsfile | grep -qs \"^diff\"\n+'\n+\n+# Expect success because the repository should be identical to the working tree\n+test_expect_success 'Keywords collapse inactive' '\n+\n+\tgit add keywordsfile &&\n+\tgit commit -m \"test-not-keywords\"\n+\n+\tgit show HEAD:keywordsfile > keywordsfile.cmp &&\n+\tcmp keywordsfile keywordsfile.cmp\n+'\n+\n+test_expect_failure 'patch application without keywords active' '\n+\tgit-apply --check keyword-patch.diff\n+'\n+\n+test_done\n-- \n1.5.1.1.822.g0049\n"},{"id":"39710","messageId":"Pine.LNX.4.63.0704171352280.1696@qynat.qvtvafvgr.pbz","threadId":"7715","inReplyTo":"alpine.LFD.0.98.0704171708360.4504@xanadu.home","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"David Lang","fromEmail":"david.lang@digitalinsight.com","sentAt":"2007-04-17T20:53:09Z","receivedAt":"2007-04-17T20:53:09Z","isPatch":true,"sender":{"key":"david.lang@digitalinsight.com","avatar":null},"body":"On Tue, 17 Apr 2007, Nicolas Pitre wrote:\n\n> Subject: Re: [PATCH 2/2] Add keyword unexpansion support to convert.c\n> \n> On Tue, 17 Apr 2007, David Lang wrote:\n>\n>> On Tue, 17 Apr 2007, Nicolas Pitre wrote:\n>>\n>>> On Tue, 17 Apr 2007, David Lang wrote:\n>>>\n>>>> On Tue, 17 Apr 2007, Nicolas Pitre wrote:\n>>>>\n>>>>> I cannot do otherwise than ask at this point in the debate: why isn't\n>>>>> the makefile rule sufficient for your needs?  Why going through a\n>>>>> complicated path that no one else will support due to its numerous\n>>>>> pitfalls?\n>>>>\n>>>> not all uses of VCS's involve useing make\n>>>\n>>> Use perl then.  Or a shell script.  Or even a command.com batch script.\n>>> Or your own tool.\n>>\n>> I would like to, however this doesn't currently integrate well with git. I've\n>> been told in the past that once .gitattributes is in place then the hooks for\n>> the crlf stuff can be generalized to allow for calls out to custom code to do\n>> this sort of thing.\n>\n> And I agree that this is a perfectly sensible thing to do.  The facility\n> should be there for you to apply any kind of transformation with\n> external tools on data going in or out from Git.  There are good and bad\n> things you can do with such a facility, but at least it becomes your\n> responsibility to screw^H^H^H^Hfilter your data and not something that\n> is enforced by Git itself.\n\nI'm pretty sure that hooks for an external helper would satisfy Andy with his \nkeyword expanstion as well.\n\nDavid Lang\n"},{"id":"39702","messageId":"vpqslay1zty.fsf@bauges.imag.fr","threadId":"7715","inReplyTo":"200704171041.46176.andyparkins@gmail.com","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2007-04-17T21:00:09Z","receivedAt":"2007-04-17T21:00:09Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Andy Parkins <andyparkins@gmail.com> writes:\n\n> This patch adds expansion of keywords support.\n\nI didn't get time to read the whole thread, but just my 2 cents:\n\nDid you have a look at the way Mercurial deals with this?\n\nThey have a system where files can be ran through arbitrary filters\nwhen commited:\n\n  http://www.selenic.com/mercurial/wiki/index.cgi/EncodeDecodeFilter\n\nand it can be somehow abused to do keyword expansion.\n\n  http://www.selenic.com/mercurial/wiki/index.cgi/TipsAndTricks#head-f348f796b9560a1cfbdaf3f3f0f7d9d4339266e9\n\nIt surely doesn't solve all the problems mentionned in this thread,\nbut the experience can be interesting to look at.\n\n-- \nMatthieu\n"},{"id":"39704","messageId":"Pine.LNX.4.64.0704171405060.5473@woody.linux-foundation.org","threadId":"7715","inReplyTo":"200704172146.33665.andyparkins@gmail.com","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-17T21:10:54Z","receivedAt":"2007-04-17T21:10:54Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 17 Apr 2007, Andy Parkins wrote:\n> \n> If you define \"work\" as \"works like cvs/svn does\", then I was fine with \n> it.\n\nI can't really argue against that. Yes, I agree 100% that we can \"work\" in \nthe sense that \"cvs/svn works\". There's clearly no fundamental reasons why \nyou can't, since svn/cvs obviously do it.\n\nI just do have higher standards. I really dislike CVS, and in many ways I \nactually think that SVN is even worse (not because it's really \"worse\", \nbut because I think it is such a waste - it fixes the _trivial_ things \nabout CVS, but doesn't really fix any of the underlying problems).\n\nSo I don't actually think that CVS \"works\". \n\n> Bit-for-bit as in CRLF is untouched?  No?  Bit-for-bit as in you said \n> you were okay with keyword-collapsing but not expansion?  You're just \n> as willing to compromise as me, you've just drawn the line in a \n> different place.\n\nBit-for-bit as in \"you have to be able to trust every single bit\".\n\nAnd no, I don't actually love CRLF either. But it doesn't have quite the \nsame fundamental problems. It has issues too, but they are fundamentally \nsmaller, and I think making \"git compatible with Windows\" is also a lot \nmore important than making \"git compatible with CVS users\".\n\nWindows we cannot change. CVS users we can try to help. \n\n\t\tLinus\n"},{"id":"39705","messageId":"Pine.LNX.4.64.0704171412020.5473@woody.linux-foundation.org","threadId":"7715","inReplyTo":"Pine.LNX.4.64.0704171405060.5473@woody.linux-foundation.org","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-17T21:13:20Z","receivedAt":"2007-04-17T21:13:20Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 17 Apr 2007, Linus Torvalds wrote:\n> \n> Windows we cannot change. CVS users we can try to help. \n\n.. and if it wasn't clear, \"helping\" CVS users is not in my opinion to try \nto make git act like CVS, and lettign them do stupid things, but to try to \nhelp them become *more* than CVS users.\n\nBecause they too can become upstanding members of society, and leave their \ndark past behind them. I firmly believe that nobody is past saving.\n\n\t\tLinus\n"},{"id":"39706","messageId":"alpine.LFD.0.98.0704171708360.4504@xanadu.home","threadId":"7715","inReplyTo":"Pine.LNX.4.63.0704171302200.1696@qynat.qvtvafvgr.pbz","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-04-17T21:16:52Z","receivedAt":"2007-04-17T21:16:52Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Tue, 17 Apr 2007, David Lang wrote:\n\n> On Tue, 17 Apr 2007, Nicolas Pitre wrote:\n> \n> > On Tue, 17 Apr 2007, David Lang wrote:\n> > \n> > > On Tue, 17 Apr 2007, Nicolas Pitre wrote:\n> > > \n> > > > I cannot do otherwise than ask at this point in the debate: why isn't\n> > > > the makefile rule sufficient for your needs?  Why going through a\n> > > > complicated path that no one else will support due to its numerous\n> > > > pitfalls?\n> > > \n> > > not all uses of VCS's involve useing make\n> > \n> > Use perl then.  Or a shell script.  Or even a command.com batch script.\n> > Or your own tool.\n> \n> I would like to, however this doesn't currently integrate well with git. I've\n> been told in the past that once .gitattributes is in place then the hooks for\n> the crlf stuff can be generalized to allow for calls out to custom code to do\n> this sort of thing.\n\nAnd I agree that this is a perfectly sensible thing to do.  The facility \nshould be there for you to apply any kind of transformation with \nexternal tools on data going in or out from Git.  There are good and bad \nthings you can do with such a facility, but at least it becomes your \nresponsibility to screw^H^H^H^Hfilter your data and not something that \nis enforced by Git itself.\n\n\nNicolas\n"},{"id":"39707","messageId":"46a038f90704171418p354cde19h8ab3c47eed36f04e@mail.gmail.com","threadId":"7715","inReplyTo":"200704172012.31280.andyparkins@gmail.com","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2007-04-17T21:18:56Z","receivedAt":"2007-04-17T21:18:56Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On 4/18/07, Andy Parkins <andyparkins@gmail.com> wrote:\n> On Tuesday 2007, April 17, Linus Torvalds wrote:\n> > So you will never work with anybody outside of git?\n> For my projects - correct; I don't care about the rest of the world.\n> For projects that do - don't enable keywords, it's an option, all I\n> want is to have that option.\n\nGIT's fundamental respect for the contents its tracking (in not\nmunging them with keyword expansion) means that it works great in\ncontexts where other SCMs tools are used. And being content-centric at\nthe SCM layer means that it is possible to track a git project with -\nsay - Mercurial (and vice-versa) with nothing more than a bit of perl\nglue and no guessing at all. The content is the content is the\ncontent.\n\nWhen the SCM has \"munge-the-content\" options, your \"upstream\" can make\nthings completely un-trackable. Tracking CVS with git is a breeze but\nthere is breakage related to keyword expansion. I should write some\nbetter heuristics for it, but it's impossible to know with 100%\ncertainty that you are doing the right thing, and getting the correct\ncontent from CVS.\n\nAll this talk of breaking non-git-patch goes back to the same. With\nthe current design, projects that use git are easily trackable if you\njust look at the content, and ignore the SCM. That's an outstanding\nproperty and quite central to the design, and I wouldn't include an\noption to \"turn it off\" even if the patch to implement it turns out to\nbe trivial.\n\nProbably using make will help - you might be able to wire it to work\noff a post-update-hook so it's completely transparent.\n\ncheers,\n\n\nmartin\n"},{"id":"39708","messageId":"7v3b2yofrt.fsf@assigned-by-dhcp.cox.net","threadId":"7715","inReplyTo":"200704171235.34793.andyparkins@gmail.com","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-17T21:24:54Z","receivedAt":"2007-04-17T21:24:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andy Parkins <andyparkins@gmail.com> writes:\n\n> On Tuesday 2007 April 17 11:09, Junio C Hamano wrote:\n>\n>> \t$ git checkout B\n>>\n>> \tshould be immediate and instantaneous.\n>\n> Now - that's a much better argument.  However, it's not\n> relevant, keywords (in other VCSs, and so why not in git) are\n> only updated when a file is checked out.\n\nIt _is_ very much relevant.\n\nIf you have the keyword in your svg drawing, and if branch A and\nbranch B happen to have textually the same contents but the way\nthey got there are different, I do not think not checking it out\nupon branch switching is correct.  Otherwise your printed copy\nwould have information from the version in branch A, even after\nswitching to B.\n"},{"id":"39712","messageId":"200704172252.03622.andyparkins@gmail.com","threadId":"7715","inReplyTo":"Pine.LNX.4.63.0704171352280.1696@qynat.qvtvafvgr.pbz","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-04-17T21:52:01Z","receivedAt":"2007-04-17T21:52:01Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"On Tuesday 2007, April 17, David Lang wrote:\n\n> I'm pretty sure that hooks for an external helper would satisfy Andy\n> with his keyword expanstion as well.\n\nIt would.\n\nAndy\n-- \nDr Andy Parkins, M Eng (hons), MIET\nandyparkins@gmail.com\n"},{"id":"39716","messageId":"7vy7kqlj5r.fsf@assigned-by-dhcp.cox.net","threadId":"7715","inReplyTo":"alpine.LFD.0.98.0704171708360.4504@xanadu.home","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-17T22:40:00Z","receivedAt":"2007-04-17T22:40:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Pitre <nico@cam.org> writes:\n\n>> I would like to, however this doesn't currently integrate\n>> well with git. I've been told in the past that once\n>> .gitattributes is in place then the hooks for the crlf stuff\n>> can be generalized to allow for calls out to custom code to\n>> do this sort of thing.\n>\n> And I agree that this is a perfectly sensible thing to do.  The facility \n> should be there for you to apply any kind of transformation with \n> external tools on data going in or out from Git.  There are good and bad \n> things you can do with such a facility, but at least it becomes your \n> responsibility to screw^H^H^H^Hfilter your data and not something that \n> is enforced by Git itself.\n\nYou have to be careful, though.  Depending on what kind of\ntransformation you implement with the external tools, you would\nend up having to slow down everything we would do.\n\nIt boils down to this statement from Andy:\n\n    ..., keywords (in other VCSs, and so why not in git) are\n    only updated when a file is checked out.  There is no need\n    to touch every file.  It's actually beneficial, because the\n    keyword in the file is the state of the file at the time it\n    was checked in - which is actually more useful than updating\n    it to the latest commit every time.\n\n    That means you're only ever expanding in a file that your\n    changing anyway - so it's effectively free.  git-checkout\n    would still be immediate and instantaneous.\n\nBack up a bit and think what \"when a file is checked out\" means.\nHis argument assumes the current behaviour of not checking out\nwhen the underlying blob objects before munging are the same.\n\nBut with keyword expansion and fancier \"external tools\" whose\nsemantics are not well defined (iow, defined to be \"do whatever\nthey please\"), does it still make sense to consider two blobs\nthat appear in totally different context \"the same\" and omit\nchecking out (and causing the external tools hook not getting\nrun)?  I already pointed out to Andy that the branch name the\nfile was taken from, if it were to take part of the keyword\nexpansion, would come out incorrectly in his printed svg\ndrawing.\n\nIf you want somebody's earlier example of \"giving a file with\nembedded keyword to somebody, who modifies and sends the result\nback in full, now you would want to incorporate the change by\nidentifying the origin\" to work, you would want \"$Source$\" (I am\nlooking at CVS documentation, \"Keyword substitution/Keyword\nList\") to identify where that file came from (after all, a\nsource tree could have duplicated files) so that you can tell\nwhich file the update is about, and this keyword would expand\ndifferently depending on where in the project tree the blob\nappears.\n\nIt is not just the checkout codepath.  We omit diffs when we\nknow from SHA-1 that the blobs are the same before decoration.\nWe even omit diffs when we know from SHA-1 that two trees are\nthe same without taking possible decorations that can be applied\ndifferently to the blobs they contain into account.  Earlier,\nAndy said he wanted to grep for the expanded text if he is\ngrepping in the working tree, and I think that makes sense, but\nthat means git-grep cannot do the same \"borrow from working tree\nwhen expanding from blob object is more expensive\" optimization\nwe have for diff.  We also need to disable that optimization\nfrom the diff, regardless of what the correct semantics for\ngrepping in working trees should be.\n\nI suspect that you would have to play safe and say \"when\nexternal tools are involved, we need to disable the existing\ncontent SHA-1 based optimization for all paths that ask for\nthem\" to keep your sanity.\n"},{"id":"39720","messageId":"20070417235649.GE31488@curie-int.orbis-terrarum.net","threadId":"7715","inReplyTo":"Pine.LNX.4.64.0704171121090.5473@woody.linux-foundation.org","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Robin H. Johnson","fromEmail":"robbat2@gentoo.org","sentAt":"2007-04-17T23:56:49Z","receivedAt":"2007-04-17T23:56:49Z","isPatch":true,"sender":{"key":"robbat2@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/373898?v=4"},"body":"On Tue, Apr 17, 2007 at 11:23:03AM -0700, Linus Torvalds wrote:\n> > Well, if the only keyword we support is $BlobId:$, then if the tree/object\n> > hasn't changed, then we still don't need to touch the object.\n> > \n> > Not so?\n> Correct. However, is that actually a useful expansion?\n> \n> Most of the time, I'd expect people to want things like \"last committer, \n> time, story of their life\" etc.. I don't think the SHA1 ID's are pretty \n> enough that anybody would ever want to see them. But yes, they are \n> certainly stable.\nI'd certainly settle for having only $Blobid:$. It fits my requirements\nperfectly.\n\nThis is perhaps a reasonable wording of my requirement.\n\"Files from from the VCS should contain a stable machine-usable\nidentifier that is unique for that revision of the file, without\npost-processing to insert the identifier.\"\n\nIn the case of CVS, $Header$ contains the path and revision number of a\nfile, which serve to identify the content uniquely.\n\nIn the case of Git, $BlobId$ fills the same requirement.\n\nAs for a usage case:\n- J.PEBKAC.User gets a a tree (from a tarball or GIT, we should gain the\n  same output)\n- Copies some file outside of the tree (the user is NOT smart enough,\n  and resists all reasonable attempts at edumacation)\n- Modifies said file outside of tree.\n- Contacts maintainer with entire changed file.\n- User vanishes off the internet.\n\nThe entire file he sent if it's CVS, contains a $Header$ that uniquely\nidentifies the file (path and revision), and the maintainer can simply\ndrop the file in, and 'cvs diff -r$OLDREV $FILE'.\nIf it's git, the maintainer drops the file in, and does 'git diff\n$OLDSHA1 $FILE'.\n\n-- \nRobin Hugh Johnson\nGentoo Linux Developer & Council Member\nE-Mail     : robbat2@gentoo.org\nGnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85\n"},{"id":"39721","messageId":"7vps62lfbw.fsf@assigned-by-dhcp.cox.net","threadId":"7715","inReplyTo":"20070417235649.GE31488@curie-int.orbis-terrarum.net","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-18T00:02:43Z","receivedAt":"2007-04-18T00:02:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Robin H. Johnson\" <robbat2@gentoo.org> writes:\n\n> As for a usage case:\n> - J.PEBKAC.User gets a a tree (from a tarball or GIT, we should gain the\n>   same output)\n> - Copies some file outside of the tree (the user is NOT smart enough,\n>   and resists all reasonable attempts at edumacation)\n> - Modifies said file outside of tree.\n> - Contacts maintainer with entire changed file.\n> - User vanishes off the internet.\n>\n> The entire file he sent if it's CVS, contains a $Header$ that uniquely\n> identifies the file (path and revision), and the maintainer can simply\n> drop the file in, and 'cvs diff -r$OLDREV $FILE'.\n> If it's git, the maintainer drops the file in, and does 'git diff\n> $OLDSHA1 $FILE'.\n\nI personally hope that the maintainer drops such a non-patch\nthat originates from a PEBKAC.  At least I hope the tools that I\npersonally use are not maintained by such a maintainer ;-)\n"},{"id":"39724","messageId":"20070418002658.GA18683@fieldses.org","threadId":"7715","inReplyTo":"7vps62lfbw.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"J. Bruce Fields","fromEmail":"bfields@fieldses.org","sentAt":"2007-04-18T00:26:58Z","receivedAt":"2007-04-18T00:26:58Z","isPatch":true,"sender":{"key":"bfields@citi.umich.edu","avatar":null},"body":"On Tue, Apr 17, 2007 at 05:02:43PM -0700, Junio C Hamano wrote:\n> \"Robin H. Johnson\" <robbat2@gentoo.org> writes:\n> \n> > As for a usage case:\n> > - J.PEBKAC.User gets a a tree (from a tarball or GIT, we should gain the\n> >   same output)\n> > - Copies some file outside of the tree (the user is NOT smart enough,\n> >   and resists all reasonable attempts at edumacation)\n> > - Modifies said file outside of tree.\n> > - Contacts maintainer with entire changed file.\n> > - User vanishes off the internet.\n> >\n> > The entire file he sent if it's CVS, contains a $Header$ that uniquely\n> > identifies the file (path and revision), and the maintainer can simply\n> > drop the file in, and 'cvs diff -r$OLDREV $FILE'.\n> > If it's git, the maintainer drops the file in, and does 'git diff\n> > $OLDSHA1 $FILE'.\n> \n> I personally hope that the maintainer drops such a non-patch\n> that originates from a PEBKAC.  At least I hope the tools that I\n> personally use are not maintained by such a maintainer ;-)\n\nThat may not be quite fair--note the 'git diff $OLDSHA1 $FILE'.  So the\n$Header$ here is a hint telling the maintainer how to produce a\n(hopefully) reviewable patch, not an invitation to blindly drop random\nfiles into the tree.  (Other objections to accepting code from random\nnon-reachable people aside....)\n\nI've occasionally wondered before whether git could offer any help in\nthe case where, say, somebody hands me a file, I know it's based on\nsrc/widget/widget.c from somewhere in v0.5..v0.7, and I'd like a guess\nat the most likely candidates.\n\nI haven't wondered that often enough that I'd consider it worth\nembedding the blob SHA1 in every checked-out file, though!\n\n--b.\n"},{"id":"39725","messageId":"20070418010637.GF31488@curie-int.orbis-terrarum.net","threadId":"7715","inReplyTo":"7vps62lfbw.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Robin H. Johnson","fromEmail":"robbat2@gentoo.org","sentAt":"2007-04-18T01:06:37Z","receivedAt":"2007-04-18T01:06:37Z","isPatch":true,"sender":{"key":"robbat2@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/373898?v=4"},"body":"On Tue, Apr 17, 2007 at 05:02:43PM -0700, Junio C Hamano wrote:\n> > As for a usage case:\n> > - J.PEBKAC.User gets a a tree (from a tarball or GIT, we should gain the\n> >   same output)\n> > - Copies some file outside of the tree (the user is NOT smart enough,\n> >   and resists all reasonable attempts at edumacation)\n> > - Modifies said file outside of tree.\n> > - Contacts maintainer with entire changed file.\n> > - User vanishes off the internet.\n> >\n> > The entire file he sent if it's CVS, contains a $Header$ that uniquely\n> > identifies the file (path and revision), and the maintainer can simply\n> > drop the file in, and 'cvs diff -r$OLDREV $FILE'.\n> > If it's git, the maintainer drops the file in, and does 'git diff\n> > $OLDSHA1 $FILE'.\n> I personally hope that the maintainer drops such a non-patch\n> that originates from a PEBKAC.  At least I hope the tools that I\n> personally use are not maintained by such a maintainer ;-)\nI certainly wasn't stating blindly commit the file. Any Gentoo developer\ndoing that should not have made it through the recruitment process.\n\nDo the diff, separate the wheat from the chaff, and then put the useful\n(and reviewed) changes back into the tree.\n\nGlancing at the Gentoo bugs I've dealt with over the last 2 months as a\nquick survey, there are a few levels: \nA - Able to submit a good diff\nB - Able to do a good implementation\nC - Able to come up with a good idea for improvement\n\nB are in short supply, and even of those, the number that can do A are\nsmaller :-(. Category C is vastly bigger than B, and those that don't\nmake B throw up a lot of chaff of bad implementations.\n\nBeing able to extract the good ideas is what's important.\n\n-- \nRobin Hugh Johnson\nGentoo Linux Developer & Council Member\nE-Mail     : robbat2@gentoo.org\nGnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85\n"},{"id":"39726","messageId":"7vejmilbyt.fsf@assigned-by-dhcp.cox.net","threadId":"7715","inReplyTo":"20070418010637.GF31488@curie-int.orbis-terrarum.net","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-18T01:15:22Z","receivedAt":"2007-04-18T01:15:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Robin H. Johnson\" <robbat2@gentoo.org> writes:\n\n> Glancing at the Gentoo bugs I've dealt with over the last 2 months as a\n> quick survey, there are a few levels: \n> A - Able to submit a good diff\n> B - Able to do a good implementation\n> C - Able to come up with a good idea for improvement\n>\n> B are in short supply, and even of those, the number that can do A are\n> smaller :-(. Category C is vastly bigger than B, and those that don't\n> make B throw up a lot of chaff of bad implementations.\n>\n> Being able to extract the good ideas is what's important.\n\nTrue, to a certain degree.  Maybe you and your fellow Gentoo\npeople are very much more accomodating, but my fear is that a\nmaintainer that goes length to sift through chaff himself\nquickly runs out of time, becomes exhausted, and ends up being\ncareless.\n\nMaybe I am spoiled by having only the best people around me, and\non git list.\n\nBut we are straying to a tangent.\n\nI do not have much against an optional \"only blob id\" expansion\nmyself, as I do not see any more downside than CRLF expansion in\nit.  But I suspect that once people see the $id$ expanded to\nblob, they would not stop, because simply they do not understand\nwhy blob-id and CRLF are much less evil than other things.\n"},{"id":"39727","messageId":"Pine.LNX.4.64.0704171800290.5473@woody.linux-foundation.org","threadId":"7715","inReplyTo":"20070418002658.GA18683@fieldses.org","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-18T01:19:34Z","receivedAt":"2007-04-18T01:19:34Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 17 Apr 2007, J. Bruce Fields wrote:\n> \n> I've occasionally wondered before whether git could offer any help in\n> the case where, say, somebody hands me a file, I know it's based on\n> src/widget/widget.c from somewhere in v0.5..v0.7, and I'd like a guess\n> at the most likely candidates.\n\nIt's actually fairly easy to do.\n\nGet the git hash of the blob: use \"git hash-object\" to do so (although \nyou can do it without git too, see later), then just do\n\n\tgit whatchanged v0.5..v0.7 -- src/widget/widget.c\n\nand just look for the hash. If it's an exact match, you'd find it there, \nand it will tell you when it changed.\n\nIf it's *not* an exact match, you have to come up with some \"measure of \nminimality\" for the thing (the size of the diff might be a good one), and \nyou can do\n\n\tgit rev-list --no-merges --full-history v0.5..v0.7 -- src/widget/widget.c > rev-list\n\nwhich will get you a full set of commits that changed that file. Then you \ncan just do something like\n\n\tbest_commit=none\n\tbest=1000000\n\twhile read commit\n\tdo \n\t\tgit cat-file blob \"$commit:src/widget/widget.c\" > tmpfile\n\t\tlines=$(diff reference-file tmpfile | wc -l)\n\t\tif [ \"$lines\" -lt \"$best\" ]\n\t\tthen\n\t\t\techo Best so far: $commit $lines\n\t\t\tbest=$lines\n\t\tfi\n\tdone < rev-list\n\nand you're done!\n\n(Yeah, I'm sure that script could be improved, but it's probably really \nnot that bad even as-is! The initial \"git rev-list\" will have done all \nthe heavy lifting, and picked out the commits that matter)\n\n> I haven't wondered that often enough that I'd consider it worth\n> embedding the blob SHA1 in every checked-out file, though!\n\nIt really doesn't pay.\n\nBesides, if you actually have the file, you can trivially get the SHA1 \n_without_ embedding it into the file. Just do\n\n\t(echo -e -n \"blob <size>\\0\" ; cat file) | sha1sum\n\nwhere \"size\" is just the size in bytes of the file.\n\nSo embedding the SHA1 doesn't actually buy you anything: every blob BY \nDEFINITION has their SHA1 embedded into them.\n\nIn fact, embedding the SHA1 (or doing any other modifications) just makes \nit harder to do this, since then you have to filter it out again.\n\n\t\tLinus\n"},{"id":"39728","messageId":"7v647ulbcv.fsf@assigned-by-dhcp.cox.net","threadId":"7715","inReplyTo":"Pine.LNX.4.64.0704171800290.5473@woody.linux-foundation.org","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-18T01:28:32Z","receivedAt":"2007-04-18T01:28:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> Besides, if you actually have the file, you can trivially get the SHA1 \n> _without_ embedding it into the file. Just do\n>\n> \t(echo -e -n \"blob <size>\\0\" ; cat file) | sha1sum\n>\n> where \"size\" is just the size in bytes of the file.\n>\n> So embedding the SHA1 doesn't actually buy you anything: every blob BY \n> DEFINITION has their SHA1 embedded into them.\n>\n> In fact, embedding the SHA1 (or doing any other modifications) just makes \n> it harder to do this, since then you have to filter it out again.\n\nThe use case the thread you are responding to assumes that you\ndo *not* have a preimage before the change.\n\nYou give a file out, somebody says \"here is my updated version\"\nand returns the whole file.  You may recognize, from the\ncontents, which path the file was taken from, but you may not\nknow which revision to diff against, as the \"update version\" was\nnot sent to you along with the version before he started\nupdating.\n\nIn my day job, I made a protocol, with a diff-challenged person\nin our Japanese subsidiary, for him to send *both* preimage and\npostimage of his changes when sending updates to me, and that\nhas worked reasonably well without embedded ID. I can always do\na file-level 3-way merge to forward port his change to whatever\nversion I am working on.\n\nBut if it is not an option to insist getting the preimage back,\nembedded blob ID would be one way to help that exchange.\n"},{"id":"39729","messageId":"alpine.LFD.0.98.0704171831380.31155@woody.linux-foundation.org","threadId":"7715","inReplyTo":"7v647ulbcv.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-18T01:33:45Z","receivedAt":"2007-04-18T01:33:45Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 17 Apr 2007, Junio C Hamano wrote:\n> \n> The use case the thread you are responding to assumes that you\n> do *not* have a preimage before the change.\n\nSure. However, see my other script to actually find the \"closest version\". \nIt's pretty easy.\n\nIn fact, even if you don't know which file it is, we could easily first \nhave a separate \"try to guess filename\" (based on the same kind of \nheurstics) and then dig deeper on that filename.\n\nSo I think there are better ways to get the same effect without embedding \nany information.\n\n\t\tLinus\n"},{"id":"39730","messageId":"7vy7kqjw4x.fsf@assigned-by-dhcp.cox.net","threadId":"7715","inReplyTo":"7vejmilbyt.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-18T01:42:38Z","receivedAt":"2007-04-18T01:42:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <junkio@cox.net> writes:\n\n> I do not have much against an optional \"only blob id\" expansion\n> myself, as I do not see any more downside than CRLF expansion in\n> it...\n\nActually, there is one.  Somebody makes a patch against a file\nwith $id$ expanded.  Gives it to somebody else who is git\nchallenged and does not have git-apply.  The patch is useless.\n\nSo it is not without more downsides than CRLF.\n"},{"id":"39732","messageId":"alpine.LFD.0.98.0704172154160.4504@xanadu.home","threadId":"7715","inReplyTo":"7vy7kqlj5r.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-04-18T02:39:41Z","receivedAt":"2007-04-18T02:39:41Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Tue, 17 Apr 2007, Junio C Hamano wrote:\n\n> Nicolas Pitre <nico@cam.org> writes:\n> \n> >> I would like to, however this doesn't currently integrate\n> >> well with git. I've been told in the past that once\n> >> .gitattributes is in place then the hooks for the crlf stuff\n> >> can be generalized to allow for calls out to custom code to\n> >> do this sort of thing.\n> >\n> > And I agree that this is a perfectly sensible thing to do.  The facility \n> > should be there for you to apply any kind of transformation with \n> > external tools on data going in or out from Git.  There are good and bad \n> > things you can do with such a facility, but at least it becomes your \n> > responsibility to screw^H^H^H^Hfilter your data and not something that \n> > is enforced by Git itself.\n> \n> You have to be careful, though.  Depending on what kind of\n> transformation you implement with the external tools, you would\n> end up having to slow down everything we would do.\n\nSo what?  \n\nWe provide a rope with proper caveat emptor.  Up to others to hang \nthemselves with it if they so desire.  It is not our problem anymore.\n\n> It boils down to this statement from Andy:\n> \n>     ..., keywords (in other VCSs, and so why not in git) are\n>     only updated when a file is checked out.  There is no need\n>     to touch every file.  It's actually beneficial, because the\n>     keyword in the file is the state of the file at the time it\n>     was checked in - which is actually more useful than updating\n>     it to the latest commit every time.\n> \n>     That means you're only ever expanding in a file that your\n>     changing anyway - so it's effectively free.  git-checkout\n>     would still be immediate and instantaneous.\n> \n> Back up a bit and think what \"when a file is checked out\" means.\n> His argument assumes the current behaviour of not checking out\n> when the underlying blob objects before munging are the same.\n\nAnd I think that should remain.  If someone really wants full \ntransformation aka keyword expansion or whatever then he'd just need to \nforce a full read-tree after switching branch.\n\n> But with keyword expansion and fancier \"external tools\" whose\n> semantics are not well defined (iow, defined to be \"do whatever\n> they please\"), does it still make sense to consider two blobs\n> that appear in totally different context \"the same\" and omit\n> checking out (and causing the external tools hook not getting\n> run)?\n\nI think so.  At least by default.  And if we really want to be kind we \ncould provide a special attribute just for disabling such optimization \ni.e. to force a checkout of everything marked with such an attribute \neverytime.  But we might as well wait to see if someone actually ask \nabout that.\n\nBut for many other cases having such a facility would be just nice \nespecially for those cases that don't depend on the branch/commit but \nonly on the content itself.  For example it was pointed out that Open \nOffice documents are gzipped XML files.  In that case it would be \nextremely advantageous to have the ability to specify an input filter as\n\"gzip -d\" and an output filter as \"gzip -c\" so Git has a chance to \nactually perform some kind of delta compression.\n\nI'm sure there might be other type of filters for situation we've not \nthought about yet, and that we might not want to carry as a builtin \nfeature.  Who knows, maybe someone might want to port Git to the \nSystem/360 and will need an EBCDIC filter on checked out text files.  Or \nmaybe a byte swapping filter for some kind of binary files when on a \nsystem with a different endianness.\n\n> I already pointed out to Andy that the branch name the\n> file was taken from, if it were to take part of the keyword\n> expansion, would come out incorrectly in his printed svg\n> drawing.\n\nTough.\n\n> If you want somebody's earlier example of \"giving a file with\n> embedded keyword to somebody, who modifies and sends the result\n> back in full, now you would want to incorporate the change by\n> identifying the origin\" to work, you would want \"$Source$\" (I am\n> looking at CVS documentation, \"Keyword substitution/Keyword\n> List\") to identify where that file came from (after all, a\n> source tree could have duplicated files) so that you can tell\n> which file the update is about, and this keyword would expand\n> differently depending on where in the project tree the blob\n> appears.\n\nLike we don't record renames, I don't think we should record such thing \nin checked out files either.  Using a search for the closest match (like \nwe do for rename detection) is probably a better avenue than trusting \nthat the ID embedded in the file wasn't messed with.  Linus already \nprovided a small script that would do that with pretty good results.\n\n> It is not just the checkout codepath.  We omit diffs when we\n> know from SHA-1 that the blobs are the same before decoration.\n> We even omit diffs when we know from SHA-1 that two trees are\n> the same without taking possible decorations that can be applied\n> differently to the blobs they contain into account.  Earlier,\n> Andy said he wanted to grep for the expanded text if he is\n> grepping in the working tree, and I think that makes sense, but\n> that means git-grep cannot do the same \"borrow from working tree\n> when expanding from blob object is more expensive\" optimization\n> we have for diff.  We also need to disable that optimization\n> from the diff, regardless of what the correct semantics for\n> grepping in working trees should be.\n> \n> I suspect that you would have to play safe and say \"when\n> external tools are involved, we need to disable the existing\n> content SHA-1 based optimization for all paths that ask for\n> them\" to keep your sanity.\n\nMaybe.  If that is what's really needed then so be it.  People who \nreally want to do strange things will have the flexibility to do so, but \nthey'll have to pay the price in loss of performance.\n\n\nNicolas\n"},{"id":"39733","messageId":"46a038f90704171950g4b408fedm1028e7f934a9b53c@mail.gmail.com","threadId":"7715","inReplyTo":"20070417235649.GE31488@curie-int.orbis-terrarum.net","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2007-04-18T02:50:00Z","receivedAt":"2007-04-18T02:50:00Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On 4/18/07, Robin H. Johnson <robbat2@gentoo.org> wrote:\n> As for a usage case:\n> - J.PEBKAC.User gets a a tree (from a tarball or GIT, we should gain the\n>   same output)\n> - Copies some file outside of the tree (the user is NOT smart enough,\n>   and resists all reasonable attempts at edumacation)\n> - Modifies said file outside of tree.\n> - Contacts maintainer with entire changed file.\n> - User vanishes off the internet.\n\nThat's a valid case, but as Linus hints, that's a snippet of perl/bash\naway to find \"closest matches\". We could have a\n\n      git-findclosestmatch <head> path/to/scan/ randomfile.c\n\nThat would quickly return a few candidates together with the commit\nthey appear in.\n\ncheers,\n\n\n\nmartin\n"},{"id":"39735","messageId":"20070418025338.GG31488@curie-int.orbis-terrarum.net","threadId":"7715","inReplyTo":"7vy7kqjw4x.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Robin H. Johnson","fromEmail":"robbat2@gentoo.org","sentAt":"2007-04-18T02:53:38Z","receivedAt":"2007-04-18T02:53:38Z","isPatch":true,"sender":{"key":"robbat2@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/373898?v=4"},"body":"On Tue, Apr 17, 2007 at 06:42:38PM -0700, Junio C Hamano wrote:\n> Junio C Hamano <junkio@cox.net> writes:\n> > I do not have much against an optional \"only blob id\" expansion\n> > myself, as I do not see any more downside than CRLF expansion in\n> > it...\n> Actually, there is one.  Somebody makes a patch against a file\n> with $id$ expanded.  Gives it to somebody else who is git\n> challenged and does not have git-apply.  The patch is useless.\nSo they have diff'd outside of Git, and the recipient is applying outside of\nGit:\nA - If they are applying the patch on top of the same base revision, it will\n    apply fine, because the keywords are identical. \nB - If they are applying the patch on top of a different revision, the keywords\n    won't apply, and most probably other content in the patch won't apply either.\n\nAdditional with B  the longer your individual files, the more likely that the\ndiff hunk containing the keyword change does not contain any other changes, and\ncan be easily discarded. More that the changes are likely to be further away\nfrom the keyword ;-).\n\nDiscarding portions of patches is already wide-spread (not just for CVS\nkeywords, the architecture keywords in Gentoo ebuilds change rapidly as well),\nand if git-apply can discard the keyword, it only serves to accelerate the\nusage of git.\n\nSome quick stats I hacked together on lengths of Gentoo ebuilds.\n23161 ebuilds total.\n51% of the Gentoo ebuilds are less than 36 lines long.\n76% are less than 56 lines long.\n90% are less than 92 lines long.\n(thereafter the tail gets VERY long).\n0.21% are more than 500 lines long.\n\n> So it is not without more downsides than CRLF.\nA cleaner version of my earlier command to find the changes between revisions.\ndiff -Nuar <(git-cat-file blob $SHA1:$FILE) $TMPFILE\nwhere $TMPFILE is a temporary filename for the file from the user.\nThis saves having to overwrite the local $FILE and restore it afterwards.\nIt would be nice if git-diff could handle this case directly.\n\nOn a tangent, has any work gone into specialized patch mergers for specific\nfile formats?\n\n-- \nRobin Hugh Johnson\nGentoo Linux Developer & Council Member\nE-Mail     : robbat2@gentoo.org\nGnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85\n"},{"id":"39739","messageId":"Pine.LNX.4.64.0704172346190.27922@iabervon.org","threadId":"7715","inReplyTo":"7vps62lfbw.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2007-04-18T04:15:02Z","receivedAt":"2007-04-18T04:15:02Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 17 Apr 2007, Junio C Hamano wrote:\n\n> \"Robin H. Johnson\" <robbat2@gentoo.org> writes:\n> \n> > As for a usage case:\n> > - J.PEBKAC.User gets a a tree (from a tarball or GIT, we should gain the\n> >   same output)\n> > - Copies some file outside of the tree (the user is NOT smart enough,\n> >   and resists all reasonable attempts at edumacation)\n> > - Modifies said file outside of tree.\n> > - Contacts maintainer with entire changed file.\n> > - User vanishes off the internet.\n> >\n> > The entire file he sent if it's CVS, contains a $Header$ that uniquely\n> > identifies the file (path and revision), and the maintainer can simply\n> > drop the file in, and 'cvs diff -r$OLDREV $FILE'.\n> > If it's git, the maintainer drops the file in, and does 'git diff\n> > $OLDSHA1 $FILE'.\n> \n> I personally hope that the maintainer drops such a non-patch\n> that originates from a PEBKAC.  At least I hope the tools that I\n> personally use are not maintained by such a maintainer ;-)\n\nAs a concrete example, say I'm not a Gentoo developer at all, but I'm \ntrying to get some package to install in a slightly odd situation. (E.g., \nI want to build a version of gcc for ARM microcontrollers, which requires \nflags to be set that aren't normally available for the ARM architecture.) \nIn order to do this, I need to make some changes to the gcc ebuild to pass \nthose USE flags through to configure. Since I'm not a Gentoo developer, I \ndon't have the version-controlled tree, just: (1) the tree that gets \nreverted to the official state every time I sync and (2) my tree of local \noverlays. (2) also contains other packages I've made modifications to \n(adding patches to packages where those patches are only in unreleased \nversion, but solve my problems, e.g.), so it's clearly the place to put \nthe gcc change.\n\nNow, if I figure out how to get the ebuild working, I'll be happy just to \nhave a working compiler for this weird target, and I don't care too much \nfurther. But then if word gets out that I managed this, or if I notice a \nbug report that other people are failing to get it to build, I may want to \npost my working ebuild. And maybe the maintainer decides that my method \nwas good, and wants to use it. But then it could be a lot of work to \nfigure out what differences are from me beating on this ebuild, what are \nimportant, and what are reverts of changes make upstream after I made my \ncopy (particularly because the same ebuild for gcc also gets a lot more \ndevelopment making it better as the system native compiler).\n\nIf the ebuild has the blob ID that the file had when it left Gentoo \nversion control and went out into local hack land, it would be relatively \neasy to figure out what patch should be applies to get the useful changes.\n\n(In case you're wondering, I actually eventually gave up and installed a \ngnu-arm binary distribution, because I was in a hurry to get the project \ngoing, and building from source kept failing to get configured properly; \nbut if my first line of attack had worked, I would have ended up with the \ndescribed hacked ebuild.)\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"39740","messageId":"7vlkgqjmsa.fsf@assigned-by-dhcp.cox.net","threadId":"7715","inReplyTo":"alpine.LFD.0.98.0704172154160.4504@xanadu.home","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-18T05:04:37Z","receivedAt":"2007-04-18T05:04:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Pitre <nico@cam.org> writes:\n\n> On Tue, 17 Apr 2007, Junio C Hamano wrote:\n>\n>> You have to be careful, though.  Depending on what kind of\n>> transformation you implement with the external tools, you would\n>> end up having to slow down everything we would do.\n>\n> So what?  \n>\n> We provide a rope with proper caveat emptor.  Up to others to hang \n> themselves with it if they so desire.  It is not our problem anymore.\n\nI sort-of find it hard to believe hearing this from somebody who\nmuttered something about importance of perception a few days ago.\n\n>> I suspect that you would have to play safe and say \"when\n>> external tools are involved, we need to disable the existing\n>> content SHA-1 based optimization for all paths that ask for\n>> them\" to keep your sanity.\n>\n> Maybe.  If that is what's really needed then so be it.  People who \n> really want to do strange things will have the flexibility to do so, but \n> they'll have to pay the price in loss of performance.\n\nNot just that.  We end up having to pay the price of maintaining\nhooks to let them do crazy things.\n"},{"id":"39742","messageId":"4625B99D.9090409@dawes.za.net","threadId":"7715","inReplyTo":"alpine.LFD.0.98.0704171708360.4504@xanadu.home","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Rogan Dawes","fromEmail":"lists@dawes.za.net","sentAt":"2007-04-18T06:24:29Z","receivedAt":"2007-04-18T06:24:29Z","isPatch":true,"sender":{"key":"lists@dawes.za.net","avatar":null},"body":"Nicolas Pitre wrote:\n> On Tue, 17 Apr 2007, David Lang wrote:\n> \n\n>> I've\n>> been told in the past that once .gitattributes is in place then the hooks for\n>> the crlf stuff can be generalized to allow for calls out to custom code to do\n>> this sort of thing.\n> \n> And I agree that this is a perfectly sensible thing to do.  The facility \n> should be there for you to apply any kind of transformation with \n> external tools on data going in or out from Git.  There are good and bad \n> things you can do with such a facility, but at least it becomes your \n> responsibility to screw^H^H^H^Hfilter your data and not something that \n> is enforced by Git itself.\n> \n> \n> Nicolas\n\nOne of the examples that has been given in the past has been taking a \nzipped OpenDocumentFormat file, unzipping it to its component parts, and \nthen committing the individual files rather than the aggregate.\n\nBut I can't figure out how this might work.\n\nOne idea is to store the binary ODF file in the index (and in the packs, \netc) as a directory with the individual text (and other) files as \nentries within that directory. Then, when various git operations want to \nuse the directory, the operation is redirected via an attribute match to \nan external script that knows how to checkout an ODF \"directory\", or \ndiff an ODF \"directory\", etc.\n\nOr similarly, when checking an \"ODF\" file in, the attribute would lead \nto an appropriate script creating the \"tree\" of individual files.\n\nDoes this sound workable?\n\nRogan\n"},{"id":"39758","messageId":"87tzve9etj.fsf@morpheus.local","threadId":"7715","inReplyTo":"20070417235649.GE31488@curie-int.orbis-terrarum.net","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"David Kågedal","fromEmail":"davidk@lysator.liu.se","sentAt":"2007-04-18T10:06:48Z","receivedAt":"2007-04-18T10:06:48Z","isPatch":true,"sender":{"key":"davidk@lysator.liu.se","avatar":"https://avatars.githubusercontent.com/u/60530?v=4"},"body":"\"Robin H. Johnson\" <robbat2@gentoo.org> writes:\n\n> This is perhaps a reasonable wording of my requirement.\n> \"Files from from the VCS should contain a stable machine-usable\n> identifier that is unique for that revision of the file, without\n> post-processing to insert the identifier.\"\n\nBut what is the \"revision of the file\"?  The blob ID is just a hash of\nthe contents, and doesn't say anything about where in the history of\nthe project it appears.  It will usually appear in many \"project\nrevisions\", i.e. commits.\n\nIf you want to mark the \"revision\" of a file, the only sensible thing\nis to use the commit ID, which will give you all the problems\ndescribed in this thread.\n\n-- \nDavid Kågedal\n"},{"id":"39764","messageId":"20070418110834.GK31488@curie-int.orbis-terrarum.net","threadId":"7715","inReplyTo":"87tzve9etj.fsf@morpheus.local","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Robin H. Johnson","fromEmail":"robbat2@gentoo.org","sentAt":"2007-04-18T11:08:34Z","receivedAt":"2007-04-18T11:08:34Z","isPatch":true,"sender":{"key":"robbat2@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/373898?v=4"},"body":"On Wed, Apr 18, 2007 at 12:06:48PM +0200, David K??gedal wrote:\n> > This is perhaps a reasonable wording of my requirement.\n> > \"Files from from the VCS should contain a stable machine-usable\n> > identifier that is unique for that revision of the file, without\n> > post-processing to insert the identifier.\"\n> But what is the \"revision of the file\"?  The blob ID is just a hash of\n> the contents, and doesn't say anything about where in the history of\n> the project it appears.  It will usually appear in many \"project\n> revisions\", i.e. commits.\nThe location/context in history of the file is not needed by the\nrequirement I wrote above.\n\nSince the BlobID is the hash of the contents (taken with all keywords\ncollapsed obviously) - if the contents are identical, then the blobid is\nidentical.\n\nSince the contents and blobid are the same, it doesn't matter which\ncommit you take the file from when you don't care about the history of\nthat point (eg cat-file, diff).\n\nThe file goes out, and when a user throws it (modified) back at us, we\njust grab the $BlobId$ and use that to identify what it originally\nlooked like.\n\n-- \nRobin Hugh Johnson\nGentoo Linux Developer & Council Member\nE-Mail     : robbat2@gentoo.org\nGnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85\n"},{"id":"39765","messageId":"Pine.LNX.4.64.0704181310170.12094@racer.site","threadId":"7715","inReplyTo":"Pine.LNX.4.64.0704171412020.5473@woody.linux-foundation.org","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-04-18T11:11:36Z","receivedAt":"2007-04-18T11:11:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 17 Apr 2007, Linus Torvalds wrote:\n\n> On Tue, 17 Apr 2007, Linus Torvalds wrote:\n> > \n> > Windows we cannot change. CVS users we can try to help. \n> \n> .. and if it wasn't clear, \"helping\" CVS users is not in my opinion to \n> try to make git act like CVS, and lettign them do stupid things, but to \n> try to help them become *more* than CVS users.\n\nI am quite certain that we also can help Windows users see the light. Once \nwe have them not only complaining, but actually doing something about it.\n\n> Because they too can become upstanding members of society, and leave their \n> dark past behind them. I firmly believe that nobody is past saving.\n\nWell, it depends. If you clicked on that File Menu button, and then \nclicked on the \"Save\" item, you are past saving.\n\nCiao,\nDscho\n"},{"id":"39766","messageId":"Pine.LNX.4.64.0704181313060.12094@racer.site","threadId":"7715","inReplyTo":"alpine.LFD.0.98.0704172154160.4504@xanadu.home","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-04-18T11:14:47Z","receivedAt":"2007-04-18T11:14:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 17 Apr 2007, Nicolas Pitre wrote:\n\n> So what?\n> \n> We provide a rope with proper caveat emptor.  Up to others to hang \n> themselves with it if they so desire.  It is not our problem anymore.\n\nThe people will complain. On this list. And I have to check the mails \nbefore deleting, because the Subject: does not say \"I just took the rope, \nignored your caveat emptor, and now I am dead. What should I do now?\".\n\nCiao,\nDscho\n"},{"id":"39767","messageId":"Pine.LNX.4.64.0704181330450.12094@racer.site","threadId":"7715","inReplyTo":"7vps62lfbw.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-04-18T11:32:06Z","receivedAt":"2007-04-18T11:32:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 17 Apr 2007, Junio C Hamano wrote:\n\n> \"Robin H. Johnson\" <robbat2@gentoo.org> writes:\n> \n> > As for a usage case:\n> > - J.PEBKAC.User gets a a tree (from a tarball or GIT, we should gain the\n> >   same output)\n> > - Copies some file outside of the tree (the user is NOT smart enough,\n> >   and resists all reasonable attempts at edumacation)\n> > - Modifies said file outside of tree.\n> > - Contacts maintainer with entire changed file.\n> > - User vanishes off the internet.\n> >\n> > The entire file he sent if it's CVS, contains a $Header$ that uniquely\n> > identifies the file (path and revision), and the maintainer can simply\n> > drop the file in, and 'cvs diff -r$OLDREV $FILE'.\n> > If it's git, the maintainer drops the file in, and does 'git diff\n> > $OLDSHA1 $FILE'.\n> \n> I personally hope that the maintainer drops such a non-patch\n> that originates from a PEBKAC.\n\nMe, too. Although people really believe strange things. When I asked such \na guy on another list, if he could send me a patch instead of a complete \nfile, he shouted loudly at me that patches were obsolete. Yes. Really. I \nbegged to differ, but I guess he still believes that.\n\nCiao,\nDscho\n"},{"id":"39773","messageId":"alpine.LFD.0.98.0704181036280.4504@xanadu.home","threadId":"7715","inReplyTo":"7vlkgqjmsa.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-04-18T14:56:40Z","receivedAt":"2007-04-18T14:56:40Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Tue, 17 Apr 2007, Junio C Hamano wrote:\n\n> Nicolas Pitre <nico@cam.org> writes:\n> \n> > On Tue, 17 Apr 2007, Junio C Hamano wrote:\n> >\n> >> You have to be careful, though.  Depending on what kind of\n> >> transformation you implement with the external tools, you would\n> >> end up having to slow down everything we would do.\n> >\n> > So what?  \n> >\n> > We provide a rope with proper caveat emptor.  Up to others to hang \n> > themselves with it if they so desire.  It is not our problem anymore.\n> \n> I sort-of find it hard to believe hearing this from somebody who\n> muttered something about importance of perception a few days ago.\n\nSure!  And that applies in this case as well.\n\nWith such a _generic_ hook, Git will be perceived as much more powerful \nand flexible.  I insist on \"generic\" because people could experiment \nwith their own filters without endless debate on the mailing list and \npressure to include this or that feature in the core, and we don't have \nto commit to those feature we're not in agreement with.\n\nAnd let's face it: there are probably legitimate and possibly more \nuseful things to do with such a hook than keyword expansion.\n\nIf you go to Home Hardware you can buy rope.  Of course you can hang \nyourself with it, but the rope manufacturers won't commit to that I'm \nsure.  But if rope was banned by law because it represents a threath to \nlife then governments would be perceived really strangely even if their \nintention are good.\n\nSure we might have a strong opinion against keyword expansion and that \nis reflected by the fact that Git will most probably never ship with the \nability to perform keyword expansion.  That doesn't mean we should deny \nall possibilities for external filters _even_ if they can be used for \nkeyword expansion.\n\n> >> I suspect that you would have to play safe and say \"when\n> >> external tools are involved, we need to disable the existing\n> >> content SHA-1 based optimization for all paths that ask for\n> >> them\" to keep your sanity.\n> >\n> > Maybe.  If that is what's really needed then so be it.  People who \n> > really want to do strange things will have the flexibility to do so, but \n> > they'll have to pay the price in loss of performance.\n> \n> Not just that.  We end up having to pay the price of maintaining\n> hooks to let them do crazy things.\n\nWeight that against the price of fighting them against the crazy things \nthey won't quit wanting to do.  At some point it is just a matter of \ngetting out of the way.\n\n\nNicolas\n"},{"id":"39774","messageId":"alpine.LFD.0.98.0704180748460.2828@woody.linux-foundation.org","threadId":"7715","inReplyTo":"4625B99D.9090409@dawes.za.net","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-18T15:02:45Z","receivedAt":"2007-04-18T15:02:45Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 18 Apr 2007, Rogan Dawes wrote:\n> \n> Or similarly, when checking an \"ODF\" file in, the attribute would lead to an\n> appropriate script creating the \"tree\" of individual files.\n> \n> Does this sound workable?\n\nI think it sounds very interesting, and I'd much rather do _those_ kinds \nof rewrites than keyword unexpansion. And yes, some kind of generic \nsupport for rewriting might give people effectively the keywords they want \n(I think the CVS semantics are not likely to be logical, but people can \nprobably do something that works for them), and at that point maybe the \nkeyword discussion goes away too.\n\nHowever, I don't know if it is \"workable\".\n\nThe thing is, it's easy enough (although potentially _very_ expensive) to \nrun some per-file script at each commit and at each checkout. But there \nare some fundamental operations that are even more common:\n\n - checking for \"file changed\", aka the \"git status\" kind of thing\n\n   Anything we do would have to follow the same \"stat\" rules, at a \n   minimum. You can *not* afford to have to check the file manually.\n\n   So especially if you combine several pieces into one, or split one file \n   into several pieces, your index would have to contain the entry \n   that matches the _filesystem_ (because that's what the index is all \n   about), but then the *tree* would contain the pieces (or the single \n   entry that matches several filesystem entries).\n\n - what about diffs (once the stat information says something has \n   potentially changed)? You'd have to script those too, and it really \n   sounds like some very basic operations get a _lot_ more expensive and \n   complex.\n\n   This is also related to the above: one of the most fundamental diffs is \n   the diff of the index and a tree - so if the index matches the \n   \"filesystem state\" and the trees contain some \"combined entry\" or \n   \"split entry\", you'd have to teach some very core diff functionality \n   about that kind of mapping.\n\nIn other words, I think it's too complicated. Not necessarily impossible, \nbut likely harder and more complex than it's really worth.\n\nHaving a 1:1 file mapping (like the CRLF<->LF object mapping is) is a lot \neasier. You just have to make sure that the index has the *stat* \ninformation from the filesystem, but the *sha1* identity information from \nthe git internal format, and things automatically just fall out right. But \nif you have anything but a 1:1 relationship, it gets hugely more complex.\n\n\t\t\tLinus\n"},{"id":"39775","messageId":"alpine.LFD.0.98.0704181058190.4504@xanadu.home","threadId":"7715","inReplyTo":"Pine.LNX.4.64.0704181313060.12094@racer.site","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-04-18T15:10:41Z","receivedAt":"2007-04-18T15:10:41Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Wed, 18 Apr 2007, Johannes Schindelin wrote:\n\n> Hi,\n> \n> On Tue, 17 Apr 2007, Nicolas Pitre wrote:\n> \n> > So what?\n> > \n> > We provide a rope with proper caveat emptor.  Up to others to hang \n> > themselves with it if they so desire.  It is not our problem anymore.\n> \n> The people will complain. On this list. And I have to check the mails \n> before deleting, because the Subject: does not say \"I just took the rope, \n> ignored your caveat emptor, and now I am dead. What should I do now?\".\n\nWell... in the case of keyword expansion (since this is really the \ncontentious case here), with such a _generic_ facility to implement \nexternal filters, people will at least have the opportunity to try it.  \nSure they might complain that it doesn't work well, but 1) it is much \neasier to *understand* why it doesn't work well after experimenting with \nit, and 2) some people *will* be perfectly happy with something that \ndoesn't work well but happens to just work in their own particular case.\n\nAnd because it now becomes a case by case issue it is much easier for \nus to simply provide a generic mechanism and let people figure out by \nthemselves what works and what doesn't work instead of having \nphilosophical discussions on the merits of keyword expansions on the \nlist.\n\nAnd because people _will_ complain *anyway*, it might lead to more \nproductive discussion if those who complain had the chance to realize \nwhat the issues really are by experience if theoretic demonstrations alone \ndoesn't convey the problem fully.\n\n\nNicolas\n"},{"id":"39776","messageId":"alpine.LFD.0.98.0704181114040.4504@xanadu.home","threadId":"7715","inReplyTo":"alpine.LFD.0.98.0704180748460.2828@woody.linux-foundation.org","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-04-18T15:34:25Z","receivedAt":"2007-04-18T15:34:25Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Wed, 18 Apr 2007, Linus Torvalds wrote:\n\n> \n> \n> On Wed, 18 Apr 2007, Rogan Dawes wrote:\n> > \n> > Or similarly, when checking an \"ODF\" file in, the attribute would lead to an\n> > appropriate script creating the \"tree\" of individual files.\n> > \n> > Does this sound workable?\n> \n> I think it sounds very interesting, and I'd much rather do _those_ kinds \n> of rewrites than keyword unexpansion. And yes, some kind of generic \n> support for rewriting might give people effectively the keywords they want \n> (I think the CVS semantics are not likely to be logical, but people can \n> probably do something that works for them), and at that point maybe the \n> keyword discussion goes away too.\n\nExactly my point.\n\n> However, I don't know if it is \"workable\".\n> \n> The thing is, it's easy enough (although potentially _very_ expensive) to \n> run some per-file script at each commit and at each checkout. But there \n> are some fundamental operations that are even more common:\n> \n>  - checking for \"file changed\", aka the \"git status\" kind of thing\n> \n>    Anything we do would have to follow the same \"stat\" rules, at a \n>    minimum. You can *not* afford to have to check the file manually.\n> \n>    So especially if you combine several pieces into one, or split one file \n>    into several pieces, your index would have to contain the entry \n>    that matches the _filesystem_ (because that's what the index is all \n>    about), but then the *tree* would contain the pieces (or the single \n>    entry that matches several filesystem entries).\n\nFor that the external script would need the ability to alter the index \nitself.  That becomes a bit yucky.  Or maybe something could be made \nwith a mechanism like dnotify/inotify to \"touch\" the single placeholder \nentry referenced by the index whenever one of the split component \nchanges.\n\n>  - what about diffs (once the stat information says something has \n>    potentially changed)? You'd have to script those too, and it really \n>    sounds like some very basic operations get a _lot_ more expensive and \n>    complex.\n\nOf course an attribute for external diff script is certainly something \nthat could be useful independently of this case, as some particular \nbinary formats might have a way of their own to display their \ndifferences.\n\nThe whole idea of having the ability to call external tools is exactly \nto delegate complex/bizarre/unusual tasks to separate and independent \nagents.  The whole checkout operation becomes much more expensive but \neveryone using such facility might expect it.  It just cannot be as bad \nas a straight checkout with CVS from a remote server though (OK I know \nit can but you know what I mean).\n\n>    This is also related to the above: one of the most fundamental diffs is \n>    the diff of the index and a tree - so if the index matches the \n>    \"filesystem state\" and the trees contain some \"combined entry\" or \n>    \"split entry\", you'd have to teach some very core diff functionality \n>    about that kind of mapping.\n\nWell, if the split components are represented by a single placeholder in \nthe index and the filesystem, and the filesystem placeholder is \n\"touched\" whenever a split component is modified, then the mapping can \nas well be limited to the external scripts for checkin/checkout/diff \nonly without the Git core having the slightest idea about it.\n\nSure it might be slow and unusual, but at least not impossible.  And \nagain, with an attribute providing a facility for external tools it is \nthen not our problem anymore.\n\n\nNicolas\n"},{"id":"39778","messageId":"46263B8E.9080500@dawes.za.net","threadId":"7715","inReplyTo":"alpine.LFD.0.98.0704180748460.2828@woody.linux-foundation.org","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Rogan Dawes","fromEmail":"lists@dawes.za.net","sentAt":"2007-04-18T15:38:54Z","receivedAt":"2007-04-18T15:38:54Z","isPatch":true,"sender":{"key":"lists@dawes.za.net","avatar":null},"body":"Linus Torvalds wrote:\n> \n> On Wed, 18 Apr 2007, Rogan Dawes wrote:\n>> Or similarly, when checking an \"ODF\" file in, the attribute would lead to an\n>> appropriate script creating the \"tree\" of individual files.\n>>\n>> Does this sound workable?\n> \n> I think it sounds very interesting, and I'd much rather do _those_ kinds \n> of rewrites than keyword unexpansion. And yes, some kind of generic \n> support for rewriting might give people effectively the keywords they want \n> (I think the CVS semantics are not likely to be logical, but people can \n> probably do something that works for them), and at that point maybe the \n> keyword discussion goes away too.\n> \n> However, I don't know if it is \"workable\".\n> \n> The thing is, it's easy enough (although potentially _very_ expensive) to \n> run some per-file script at each commit and at each checkout. But there \n> are some fundamental operations that are even more common:\n> \n>  - checking for \"file changed\", aka the \"git status\" kind of thing\n> \n>    Anything we do would have to follow the same \"stat\" rules, at a \n>    minimum. You can *not* afford to have to check the file manually.\n> \n>    So especially if you combine several pieces into one, or split one file \n>    into several pieces, your index would have to contain the entry \n>    that matches the _filesystem_ (because that's what the index is all \n>    about), but then the *tree* would contain the pieces (or the single \n>    entry that matches several filesystem entries).\n\nRight. I would imagine that the script would have to take care of \nsetting timestamps in the filesystem appropriately, as well as passing \nthem back to git when queried.\n\ne.g. expanding test.odf/: (since we store it as a directory)\n\ngit calls \"odf.sh checkout test.odf/ <sha1> <perms> <stat>\"\n\nodf checkout calls back into git to find out the details of the files \nunder test.odf/, and creates a zip file containing the individual files, \nwith appropriate timestamps.\n\nUser then opens the file using OO.o or whatever, makes some changes and \nsaves the file.\n\nThe user then runs git status:\n\ngit calls \"odf.sh stat test.odf/\" (again, triggered by an attribute)\n\nodf.sh does the equivalent of \"zip -l\" to get up to date stat info for \nthe component files, and passes it back to git (via stdout?)\n\nUser commits his changes:\n\ngit calls \"odf.sh checkin test.odf/\"\n\nodf.sh unpacks the individual files, calls back into git to create \nindividual objects (using a fast-import-alike protocol over stdout?)\n\n\n> \n>  - what about diffs (once the stat information says something has \n>    potentially changed)? You'd have to script those too, and it really \n>    sounds like some very basic operations get a _lot_ more expensive and \n>    complex.\n >\n>    This is also related to the above: one of the most fundamental diffs is \n>    the diff of the index and a tree - so if the index matches the \n>    \"filesystem state\" and the trees contain some \"combined entry\" or \n>    \"split entry\", you'd have to teach some very core diff functionality \n>    about that kind of mapping.\n> \n> In other words, I think it's too complicated. Not necessarily impossible, \n> but likely harder and more complex than it's really worth.\n> \n> Having a 1:1 file mapping (like the CRLF<->LF object mapping is) is a lot \n> easier. You just have to make sure that the index has the *stat* \n> information from the filesystem, but the *sha1* identity information from \n> the git internal format, and things automatically just fall out right. But \n> if you have anything but a 1:1 relationship, it gets hugely more complex.\n> \n> \t\t\tLinus\n\nAbsolutely. I just raised it now since it was originally mentioned quite \na long time ago as a possible feature of git, and I couldn't see how it \nmight work.\n\nThanks for your time,\n\nRogan\n"},{"id":"39781","messageId":"alpine.LFD.0.98.0704181147330.4504@xanadu.home","threadId":"7715","inReplyTo":"46263B8E.9080500@dawes.za.net","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-04-18T15:59:22Z","receivedAt":"2007-04-18T15:59:22Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Wed, 18 Apr 2007, Rogan Dawes wrote:\n\n> Right. I would imagine that the script would have to take care of setting\n> timestamps in the filesystem appropriately, as well as passing them back to\n> git when queried.\n> \n> e.g. expanding test.odf/: (since we store it as a directory)\n> \n> git calls \"odf.sh checkout test.odf/ <sha1> <perms> <stat>\"\n> \n> odf checkout calls back into git to find out the details of the files under\n> test.odf/, and creates a zip file containing the individual files, with\n> appropriate timestamps.\n\nWhy would you need to store the document as multiple files into Git?\n\nThe only reasons I can see for external filters are:\n\n 1) Normalization, e.g. the LF->CRLF thing.\n\n    Some might want to do keyword expansion which would fall into this\n    category as well.\n\n 2) Better archiving with Git's deltas.\n\n    That means storing files uncompressed into Git since Git will\n    compress them anyway, after significant space reduction due to \n    deltas which cannot occur on already compressed data.\n\nSo if your .odf file is actually a zip with multiple files, then all you \nhave to do is to convert that zip archive into a non compressed tar \narchive on checkins, and the reverse transformation on checkouts.  The \nnon compressed tar content will delta well, the Git archive will be \nsmall, and no tricks with the index will be needed.\n\nOr am I missing something?\n\n\nNicolas\n"},{"id":"39786","messageId":"462642AE.2060708@dawes.za.net","threadId":"7715","inReplyTo":"alpine.LFD.0.98.0704181147330.4504@xanadu.home","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Rogan Dawes","fromEmail":"lists@dawes.za.net","sentAt":"2007-04-18T16:09:18Z","receivedAt":"2007-04-18T16:09:18Z","isPatch":true,"sender":{"key":"lists@dawes.za.net","avatar":null},"body":"Nicolas Pitre wrote:\n> On Wed, 18 Apr 2007, Rogan Dawes wrote:\n> \n>> Right. I would imagine that the script would have to take care of setting\n>> timestamps in the filesystem appropriately, as well as passing them back to\n>> git when queried.\n>>\n>> e.g. expanding test.odf/: (since we store it as a directory)\n>>\n>> git calls \"odf.sh checkout test.odf/ <sha1> <perms> <stat>\"\n>>\n>> odf checkout calls back into git to find out the details of the files under\n>> test.odf/, and creates a zip file containing the individual files, with\n>> appropriate timestamps.\n> \n> Why would you need to store the document as multiple files into Git?\n> \n> The only reasons I can see for external filters are:\n> \n>  1) Normalization, e.g. the LF->CRLF thing.\n> \n>     Some might want to do keyword expansion which would fall into this\n>     category as well.\n> \n>  2) Better archiving with Git's deltas.\n> \n>     That means storing files uncompressed into Git since Git will\n>     compress them anyway, after significant space reduction due to \n>     deltas which cannot occur on already compressed data.\n> \n> So if your .odf file is actually a zip with multiple files, then all you \n> have to do is to convert that zip archive into a non compressed tar \n> archive on checkins, and the reverse transformation on checkouts.  The \n> non compressed tar content will delta well, the Git archive will be \n> small, and no tricks with the index will be needed.\n> \n> Or am I missing something?\n> \n> \n> Nicolas\n\nProbably not! ;-)\n\nI was just thinking that it would be easier to see diffs between \nindividual files, rather than between entries in a zip. But if we are \ncalling out to a specialized handler, the handler can do that just as \neasily, and without the added complexity in the index, etc.\n\nIt also means that someone without the attributes and specialized \nhandler would not be able to use the file (if it is stored as a directory).\n\nClearly a bad idea! Just ignore me, I'm used to it! ;-)\n\nRogan\n"},{"id":"39800","messageId":"1176919099.14664.4.camel@bruno.nolaviz.org","threadId":"7715","inReplyTo":"alpine.LFD.0.98.0704181147330.4504@xanadu.home","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Alon Ziv","fromEmail":"alonz@nolaviz.org","sentAt":"2007-04-18T17:58:19Z","receivedAt":"2007-04-18T17:58:19Z","isPatch":true,"sender":{"key":"alonz@nolaviz.org","avatar":null},"body":"On Wed, 2007-04-18 at 11:59 -0400, Nicolas Pitre wrote:\n> So if your .odf file is actually a zip with multiple files, then all you \n> have to do is to convert that zip archive into a non compressed tar \n> archive on checkins, and the reverse transformation on checkouts.  The \n> non compressed tar content will delta well, the Git archive will be \n> small, and no tricks with the index will be needed.\n> \n\nIn fact, for the specific case of OO.o files, I would claim the proper\ntransformation is just converting to non-compressed zip on checkin...\n\n(Non-compressed zip is just as good here as tar, and has the added\nadvantage that there is no need for a reverse transformation on\ncheckout :))\n\n\t-az\n"},{"id":"39868","messageId":"Pine.LNX.4.64.0704191019100.8822@racer.site","threadId":"7715","inReplyTo":"alpine.LFD.0.98.0704181058190.4504@xanadu.home","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-04-19T08:19:42Z","receivedAt":"2007-04-19T08:19:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 18 Apr 2007, Nicolas Pitre wrote:\n\n> And because it now becomes a case by case issue it is much easier for us \n> to simply provide a generic mechanism and let people figure out by \n> themselves what works and what doesn't work instead of having \n> philosophical discussions on the merits of keyword expansions on the \n> list.\n\nThat's a very good point.\n\nCiao,\nDscho\n"},{"id":"39927","messageId":"f091c7$grp$1@sea.gmane.org","threadId":"7715","inReplyTo":"200704171235.34793.andyparkins@gmail.com","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-04-20T00:30:41Z","receivedAt":"2007-04-20T00:30:41Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Andy Parkins wrote:\n\n>>  * We do not do the borrowing from working tree when doing\n>>    grep_sha1(), but when we grep inside a file from working tree\n>>    with grep_file(), we do not currently make it go through\n>>    convert_to_git() to fix line endings.  Maybe we should, if\n>>    only for consistency.\n> \n> I'd actually argue not - git-grep searches the working tree.  The expanded \n> keywords are in the working tree.  Take the CRLF case - I'm a clueless user, \n> who only understands the system I'm working on.  I want to search for all the \n> line endings, so I do git-grep \"\\r\\n\" - that should work, because I'm \n> searching my working tree.\n\nActually, \"git grep\" can search both the working tree (default), but also\nan index (--cached), or specified tree (or tree-ish). The same with\n\"git diff\": it can work on tree (repository), index, working tree version,\nnow I think in [almost] any combination. \n\nThink what keyword expansion means to all this... Well, you can have -kk\nto expand/not expand keywords, but this is avoiding issue, not solving it\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"39968","messageId":"dbfc82860704200432x5db9f1car7fe0a5e6e5c1f994@mail.gmail.com","threadId":"7715","inReplyTo":"200704172146.33665.andyparkins@gmail.com","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Nikolai Weibull","fromEmail":"now@bitwi.se","sentAt":"2007-04-20T11:32:04Z","receivedAt":"2007-04-20T11:32:04Z","isPatch":true,"sender":{"key":"now@bitwi.se","avatar":"https://gravatar.com/avatar/d9242f067845cf9a72be23e4213c3b6e53492178e5df97372088a441af846133?d=mp&s=160"},"body":"On 4/17/07, Andy Parkins <andyparkins@gmail.com> wrote:\n> On Tuesday 2007, April 17, Linus Torvalds wrote:\n>\n> > No, you haven't. You've \"addressed\" them by stating they don't\n> > matter. It doesn't \"matter\" that a diff won't actually apply to a\n> > checked-out tree, because you fix it up in another tool.\n>\n> Okay.  I think this is a matter of perspective - my perspective is that\n> if it supplies what svn/cvs supply then that would please the people\n> who want it (of whom I am one); yours is obviously that if it isn't\n> perfect, it's not worth doing.  That's a reasonable thing to demand,\n> and I'm not going to try and argue you out of it.\n\nLoads of people would be pleased if marijuana was legalized.  For some\nreason, few governments seem willing to cater to their needs.\n\n  nikolai\n"},{"id":"40019","messageId":"Pine.LNX.4.63.0704201605580.4634@qynat.qvtvafvgr.pbz","threadId":"7715","inReplyTo":"7vy7kqlj5r.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"David Lang","fromEmail":"david.lang@digitalinsight.com","sentAt":"2007-04-21T00:42:08Z","receivedAt":"2007-04-21T00:42:08Z","isPatch":true,"sender":{"key":"david.lang@digitalinsight.com","avatar":null},"body":"On Tue, 17 Apr 2007, Junio C Hamano wrote:\n\n> Nicolas Pitre <nico@cam.org> writes:\n>\n>>> I would like to, however this doesn't currently integrate\n>>> well with git. I've been told in the past that once\n>>> .gitattributes is in place then the hooks for the crlf stuff\n>>> can be generalized to allow for calls out to custom code to\n>>> do this sort of thing.\n>>\n>> And I agree that this is a perfectly sensible thing to do.  The facility\n>> should be there for you to apply any kind of transformation with\n>> external tools on data going in or out from Git.  There are good and bad\n>> things you can do with such a facility, but at least it becomes your\n>> responsibility to screw^H^H^H^Hfilter your data and not something that\n>> is enforced by Git itself.\n>\n> You have to be careful, though.  Depending on what kind of\n> transformation you implement with the external tools, you would\n> end up having to slow down everything we would do.\n\nyou can slow down everything that you do on your system if you defined too much \nwork for external tools, that won't slow other people down who don't define any \nwork for external tools.\n\n> It boils down to this statement from Andy:\n>\n>    ..., keywords (in other VCSs, and so why not in git) are\n>    only updated when a file is checked out.  There is no need\n>    to touch every file.  It's actually beneficial, because the\n>    keyword in the file is the state of the file at the time it\n>    was checked in - which is actually more useful than updating\n>    it to the latest commit every time.\n>\n>    That means you're only ever expanding in a file that your\n>    changing anyway - so it's effectively free.  git-checkout\n>    would still be immediate and instantaneous.\n>\n> Back up a bit and think what \"when a file is checked out\" means.\n> His argument assumes the current behaviour of not checking out\n> when the underlying blob objects before munging are the same.\n\ncorrect.\n\n> But with keyword expansion and fancier \"external tools\" whose\n> semantics are not well defined (iow, defined to be \"do whatever\n> they please\"), does it still make sense to consider two blobs\n> that appear in totally different context \"the same\" and omit\n> checking out (and causing the external tools hook not getting\n> run)?  I already pointed out to Andy that the branch name the\n> file was taken from, if it were to take part of the keyword\n> expansion, would come out incorrectly in his printed svg\n> drawing.\n\nthis is part of the rope you are handing out. the external tool could do a lot \nof things that don't make sense. you could have the tool include the serial \nnumber of the cpu you happen to be running on at the moment, it wouldn't make \nsense to do this, but it could be done. the fact that the rope could be used to \nhang someone doesn't mean that you should outlaw rope.\n\n> If you want somebody's earlier example of \"giving a file with\n> embedded keyword to somebody, who modifies and sends the result\n> back in full, now you would want to incorporate the change by\n> identifying the origin\" to work, you would want \"$Source$\" (I am\n> looking at CVS documentation, \"Keyword substitution/Keyword\n> List\") to identify where that file came from (after all, a\n> source tree could have duplicated files) so that you can tell\n> which file the update is about, and this keyword would expand\n> differently depending on where in the project tree the blob\n> appears.\n>\n> It is not just the checkout codepath.  We omit diffs when we\n> know from SHA-1 that the blobs are the same before decoration.\n> We even omit diffs when we know from SHA-1 that two trees are\n> the same without taking possible decorations that can be applied\n> differently to the blobs they contain into account.  Earlier,\n> Andy said he wanted to grep for the expanded text if he is\n> grepping in the working tree, and I think that makes sense, but\n> that means git-grep cannot do the same \"borrow from working tree\n> when expanding from blob object is more expensive\" optimization\n> we have for diff.  We also need to disable that optimization\n> from the diff, regardless of what the correct semantics for\n> grepping in working trees should be.\n\ngit would not be able to borrow from the working tree just becouse the index \nthinks that the file is the same (and frankly, I'm not sure this is really a \nsafe thing to do in any case, it's just something that works frequently enough \nthat we get away with it)\n\nthe diff optimizations could (and should) stay.\n\n> I suspect that you would have to play safe and say \"when\n> external tools are involved, we need to disable the existing\n> content SHA-1 based optimization for all paths that ask for\n> them\" to keep your sanity.\n\nAndy and I are both expecting that if the blobs are the same that none of the \ngit tools would flag them as different. this maintains the huge speedups that \ngit achieves by doing these checks.\n\nif you want to make an option somewhere that disables this optimization, I guess \nit would be Ok, but I wouldn't do so until someone came up with a situation \nwhere they really needed it, nothing in what Andy or I have asked for needs \nthis.\n\nboth of us are treating the keyword expansion as decorations to the file. it's \nuseful, but the core meaning of 'what this file is' is the checked in version \nwith the keywords unexpanded. all the optmizations that only look at what's \nchecked in will remain as valid as they are today, it's only things that look at \nyour working directory that would change.\n\nDavid Lang\n"},{"id":"40020","messageId":"Pine.LNX.4.63.0704201743130.4634@qynat.qvtvafvgr.pbz","threadId":"7715","inReplyTo":"f091c7$grp$1@sea.gmane.org","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"David Lang","fromEmail":"david.lang@digitalinsight.com","sentAt":"2007-04-21T00:47:10Z","receivedAt":"2007-04-21T00:47:10Z","isPatch":true,"sender":{"key":"david.lang@digitalinsight.com","avatar":null},"body":"On Fri, 20 Apr 2007, Jakub Narebski wrote:\n\n> Andy Parkins wrote:\n>\n>>>  * We do not do the borrowing from working tree when doing\n>>>    grep_sha1(), but when we grep inside a file from working tree\n>>>    with grep_file(), we do not currently make it go through\n>>>    convert_to_git() to fix line endings.  Maybe we should, if\n>>>    only for consistency.\n>>\n>> I'd actually argue not - git-grep searches the working tree.  The expanded\n>> keywords are in the working tree.  Take the CRLF case - I'm a clueless user,\n>> who only understands the system I'm working on.  I want to search for all the\n>> line endings, so I do git-grep \"\\r\\n\" - that should work, because I'm\n>> searching my working tree.\n>\n> Actually, \"git grep\" can search both the working tree (default), but also\n> an index (--cached), or specified tree (or tree-ish). The same with\n> \"git diff\": it can work on tree (repository), index, working tree version,\n> now I think in [almost] any combination.\n>\n> Think what keyword expansion means to all this... Well, you can have -kk\n> to expand/not expand keywords, but this is avoiding issue, not solving it\n\nhow is git-grep on the working tree any different than just useing grep? the \nvalue in the git-* versions of system commands are that they work on the \nhistory, index, etc wher ethe normal system tools don't.\n\nin this particular case, since the git user can do a git-grep of the working \ntree, or a git-grep of HEAD (or of the index), I don't think that it hurts much \neither way.\n\nif git-grep of the working tree converts things to the checked-in version before \nthe pattern match, the user can still use grep  to go through the checked-out \nversion\n\nif git-grep of the working tree doesn't convert things to the checked-in version \nbefore the pattern match, the user can stil use git-grep HEAD or --cached to go \nthrough the checked-in version.\n\nDavid Lang"},{"id":"40023","messageId":"7vbqhiwky4.fsf@assigned-by-dhcp.cox.net","threadId":"7715","inReplyTo":"Pine.LNX.4.63.0704201605580.4634@qynat.qvtvafvgr.pbz","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-21T01:54:59Z","receivedAt":"2007-04-21T01:54:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Lang <david.lang@digitalinsight.com> writes:\n\n>> But with keyword expansion and fancier \"external tools\" whose\n>> semantics are not well defined (iow, defined to be \"do whatever\n>> they please\"), does it still make sense to consider two blobs\n>> that appear in totally different context \"the same\" and omit\n>> checking out (and causing the external tools hook not getting\n>> run)?  I already pointed out to Andy that the branch name the\n>> file was taken from, if it were to take part of the keyword\n>> expansion, would come out incorrectly in his printed svg\n>> drawing.\n>\n> this is part of the rope you are handing out. the external tool could\n> do a lot of things that don't make sense. you could have the tool\n> include the serial number of the cpu you happen to be running on at\n> the moment, it wouldn't make sense to do this, but it could be\n> done. the fact that the rope could be used to hang someone doesn't\n> mean that you should outlaw rope.\n\nI do not think you understand, especially after reading the part\nyou say \"Andy and I both...\".\n\nThe point of my comment was that with Andy's definition of when\nthe \"external tools\" should trigger, that CPU serial number\nembedder would _NOT_ trigger for a path when you switch branches\nthat have the same contents at that path.  External tools can do\nstupid things and that is what you are calling the rope.  But\nthe case I am talking about is that we deliberately do _not_\ncall external tools, so even if external tools can do sensible\nthings if given a chance to, they are not given a chance to do\nso, and deciding not to call them in some cases is made by us.\nI think that's different from \"we gave you rope, you hang\nyourself and that is not our problem\".\n\nPeople have every right to say \"if you consistently call these\nexternal tools, they behave sensibly, but you only call them\nwhen you choose, and that is where the idiocy is coming from\".\nHow would you respond to that?\n"},{"id":"40024","messageId":"alpine.LFD.0.98.0704202203200.4504@xanadu.home","threadId":"7715","inReplyTo":"7vbqhiwky4.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-04-21T02:06:56Z","receivedAt":"2007-04-21T02:06:56Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Fri, 20 Apr 2007, Junio C Hamano wrote:\n\n> The point of my comment was that with Andy's definition of when\n> the \"external tools\" should trigger, that CPU serial number\n> embedder would _NOT_ trigger for a path when you switch branches\n> that have the same contents at that path.  External tools can do\n> stupid things and that is what you are calling the rope.  But\n> the case I am talking about is that we deliberately do _not_\n> call external tools, so even if external tools can do sensible\n> things if given a chance to, they are not given a chance to do\n> so, and deciding not to call them in some cases is made by us.\n\nAnd that's a fairly acceptable limitation IMHO, which doesn't make the \nthing any less useful for many cases.  We just need to document it \nappropriately.\n\n\nNicolas\n"},{"id":"40075","messageId":"Pine.LNX.4.63.0704211620250.5655@qynat.qvtvafvgr.pbz","threadId":"7715","inReplyTo":"7vbqhiwky4.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] Add keyword unexpansion support to convert.c","fromName":"David Lang","fromEmail":"david.lang@digitalinsight.com","sentAt":"2007-04-21T23:31:10Z","receivedAt":"2007-04-21T23:31:10Z","isPatch":true,"sender":{"key":"david.lang@digitalinsight.com","avatar":null},"body":"On Fri, 20 Apr 2007, Junio C Hamano wrote:\n\n> David Lang <david.lang@digitalinsight.com> writes:\n>\n>>> But with keyword expansion and fancier \"external tools\" whose\n>>> semantics are not well defined (iow, defined to be \"do whatever\n>>> they please\"), does it still make sense to consider two blobs\n>>> that appear in totally different context \"the same\" and omit\n>>> checking out (and causing the external tools hook not getting\n>>> run)?  I already pointed out to Andy that the branch name the\n>>> file was taken from, if it were to take part of the keyword\n>>> expansion, would come out incorrectly in his printed svg\n>>> drawing.\n>>\n>> this is part of the rope you are handing out. the external tool could\n>> do a lot of things that don't make sense. you could have the tool\n>> include the serial number of the cpu you happen to be running on at\n>> the moment, it wouldn't make sense to do this, but it could be\n>> done. the fact that the rope could be used to hang someone doesn't\n>> mean that you should outlaw rope.\n>\n> I do not think you understand, especially after reading the part\n> you say \"Andy and I both...\".\n\nsorry for not being clear\n\n> The point of my comment was that with Andy's definition of when\n> the \"external tools\" should trigger, that CPU serial number\n> embedder would _NOT_ trigger for a path when you switch branches\n> that have the same contents at that path.  External tools can do\n> stupid things and that is what you are calling the rope.  But\n> the case I am talking about is that we deliberately do _not_\n> call external tools, so even if external tools can do sensible\n> things if given a chance to, they are not given a chance to do\n> so, and deciding not to call them in some cases is made by us.\n> I think that's different from \"we gave you rope, you hang\n> yourself and that is not our problem\".\n\nthe cpu serial number would be different for each cpu in a system, so you could \nget different answers, even in the same branch (let alone on different systems, \nwhich is what I was thinking of when I wrote that example)\n\nmy point was that while it's possible to define external tools that will cause \nproblems (my having effectvly random changes to the files), and when external \ntools are used it will slow things down (how much depends on the tools), it's \nalso possible to define external tools that only have well defined, easily \nreversable effects, that only touch a few files, and so don't cause a huge \nperformance hit.\n\n> People have every right to say \"if you consistently call these\n> external tools, they behave sensibly, but you only call them\n> when you choose, and that is where the idiocy is coming from\".\n> How would you respond to that?\n\nconsistantly calling the external tools is not the same thing as calling them \nfor every possible thing that refrences the file, it's calling them every time a \nparticular type of access to the file is made (and it helps to have well defined \nand well documented rules for when they are used)\n\nI think the basic rule of 'git commands work against the checked-in version of \nthe file' is a solid basis to work from. This means that you can't optimize by \nsometimes looking at the checked-out version, and there may still be some corner \ncases to explain/clarify/define (like the git-diff against the working tree \nmentioned in other messages)\n\nDavid Lang\n"}]}