{"thread":{"id":"29628","subject":"[PATCHv3] git-p4: RCS keyword handling","startedAt":"2012-02-14T22:33:54Z","lastAt":"2012-02-21T17:59:35Z","messageCount":7,"participants":["Luke Diamand","Junio C Hamano","Pete Wyckoff","Eric Scouten"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"184716","messageId":"1329258835-17223-1-git-send-email-luke@diamand.org","threadId":"29628","inReplyTo":null,"subject":"[PATCHv3] git-p4: RCS keyword handling","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2012-02-14T22:33:54Z","receivedAt":"2012-02-14T22:33:54Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"Updated patch to handle RCS keyword expansion performed by perforce\nin git-p4. This incorporates Pete Wyckoff's suggestions and test\ncases, and also adds a test for handling deletion of a file containing\nRCS keywords.\n\nEric - could I ask you to take a look at this and see if it\nsolves your particular problem.\n\nThanks.\n\nLuke Diamand (1):\n  git-p4: add initial support for RCS keywords\n\n Documentation/git-p4.txt   |    5 +\n contrib/fast-import/git-p4 |  116 +++++++++++++++--\n t/t9810-git-p4-rcs.sh      |  304 ++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 415 insertions(+), 10 deletions(-)\n create mode 100755 t/t9810-git-p4-rcs.sh\n\n-- \n1.7.9.259.ga92e\n"},{"id":"184717","messageId":"1329258835-17223-2-git-send-email-luke@diamand.org","threadId":"29628","inReplyTo":"1329258835-17223-1-git-send-email-luke@diamand.org","subject":"[PATCHv3] git-p4: add initial support for RCS keywords","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2012-02-14T22:33:55Z","receivedAt":"2012-02-14T22:33:55Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"RCS keywords cause problems for git-p4 as perforce always\nexpands them (if +k is set) and so when applying the patch,\ngit reports that the files have been modified by both sides,\nwhen in fact they haven't.\n\nThis change means that when git-p4 detects a problem applying\na patch, it will check to see if keyword expansion could be\nthe culprit. If it is, it strips the keywords in the p4\nrepository so that they match what git is expecting. It then\nhas another go at applying the patch.\n\nThis behaviour is enabled with a new git-p4 configuration\noption and is off by default.\n\nImproved-by: Pete Wyckoff <pw@padd.com>\nSigned-off-by: Luke Diamand <luke@diamand.org>\n---\n Documentation/git-p4.txt   |    5 +\n contrib/fast-import/git-p4 |  116 +++++++++++++++--\n t/t9810-git-p4-rcs.sh      |  304 ++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 415 insertions(+), 10 deletions(-)\n create mode 100755 t/t9810-git-p4-rcs.sh\n\ndiff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt\nindex 8b92cc0..7fa47c8 100644\n--- a/Documentation/git-p4.txt\n+++ b/Documentation/git-p4.txt\n@@ -483,6 +483,11 @@ git-p4.skipUserNameCheck::\n \tuser map, 'git p4' exits.  This option can be used to force\n \tsubmission regardless.\n \n+git-p4.attemptRCSCleanup:\n+    If enabled, 'git p4 submit' will attempt to sort cleanup RCS keywords\n+    ($Header$, etc). These would otherwise cause merge conflicts and prevent\n+    the submit going ahead. This option should be considered experimental at\n+    present.\n \n IMPLEMENTATION DETAILS\n ----------------------\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex a78d9c5..8130447 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -10,7 +10,7 @@\n \n import optparse, sys, os, marshal, subprocess, shelve\n import tempfile, getopt, os.path, time, platform\n-import re\n+import re, shutil\n \n verbose = False\n \n@@ -186,6 +186,44 @@ def split_p4_type(p4type):\n         mods = s[1]\n     return (base, mods)\n \n+#\n+# return the raw p4 type of a file (text, text+ko, etc)\n+#\n+def p4_type(file):\n+    files = p4_read_pipe_lines([\"files\", file])\n+    info = files[0]\n+    m = re.search(r'\\(([a-z0-9A-Z+]+)\\)\\s*$', info)\n+    if m:\n+        ret = m.group(1)\n+        if verbose:\n+            print \"%s => %s\" % (file, ret)\n+        return ret\n+    else:\n+        die(\"Could not extract file type from '%s'\" % info)\n+\n+#\n+# Given a type base and modifier, return a regexp matching\n+# the keywords that can be expanded in the file\n+#\n+def p4_keywords_regexp_for_type(base, type_mods):\n+    if base in (\"text\", \"unicode\", \"binary\"):\n+        if \"ko\" in type_mods:\n+            return r'\\$(Id|Header)[^$]*\\$'\n+        elif \"k\" in type_mods:\n+            return r'\\$(Id|Header|Author|Date|DateTime|Change|File|Revision)[^$]*\\$'\n+        else:\n+            return None\n+    else:\n+        return None\n+\n+#\n+# Given a file, return a regexp matching the possible\n+# RCS keywords that will be expanded, or None for files\n+# with kw expansion turned off.\n+#\n+def p4_keywords_regexp_for_file(file):\n+    (type_base, type_mods) = split_p4_type(p4_type(file))\n+    return p4_keywords_regexp_for_type(type_base, type_mods)\n \n def setP4ExecBit(file, mode):\n     # Reopens an already open file and changes the execute bit to match\n@@ -753,6 +791,28 @@ class P4Submit(Command, P4UserMap):\n \n         return result\n \n+    def patchRCSKeywords(self, file, pattern):\n+        # Attempt to zap the RCS keywords in a p4 controlled file matching the given pattern\n+        (handle, outFileName) = tempfile.mkstemp(dir='.')\n+        try:\n+            outFile = os.fdopen(handle, \"w+\")\n+            inFile = open(file, \"r\")\n+            for line in inFile.readlines():\n+                line = re.sub(pattern, r'$\\1$', line)\n+                outFile.write(line)\n+            inFile.close()\n+            outFile.close()\n+            # Forcibly overwrite the original file\n+            os.unlink(file)\n+            shutil.move(outFileName, file)\n+        except:\n+            # cleanup our temporary file\n+            os.unlink(outFileName)\n+            print \"Failed to strip RCS keywords in %s\" % file\n+            raise\n+\n+        print \"Patched up RCS keywords in %s\" % file\n+\n     def p4UserForCommit(self,id):\n         # Return the tuple (perforce user,git email) for a given git commit id\n         self.getUserMapFromPerforceServer()\n@@ -918,6 +978,7 @@ class P4Submit(Command, P4UserMap):\n         filesToDelete = set()\n         editedFiles = set()\n         filesToChangeExecBit = {}\n+\n         for line in diff:\n             diff = parseDiffTreeEntry(line)\n             modifier = diff['status']\n@@ -964,9 +1025,48 @@ class P4Submit(Command, P4UserMap):\n         patchcmd = diffcmd + \" | git apply \"\n         tryPatchCmd = patchcmd + \"--check -\"\n         applyPatchCmd = patchcmd + \"--check --apply -\"\n+        patch_succeeded = True\n \n         if os.system(tryPatchCmd) != 0:\n+            fixed_rcs_keywords = False\n+            patch_succeeded = False\n             print \"Unfortunately applying the change failed!\"\n+\n+            # Patch failed, maybe it's just RCS keyword woes. Look through\n+            # the patch to see if that's possible.\n+            if gitConfig(\"git-p4.attemptRCSCleanup\",\"--bool\") == \"true\":\n+                file = None\n+                pattern = None\n+                kwfiles = {}\n+                for line in read_pipe_lines(diffcmd):\n+                    # read diff lines: for each file reported, if it can have\n+                    # keywords expanded, and the diff contains keywords, then\n+                    # try zapping the p4 file.\n+                    m = re.match(r'^diff --git a/(.*)\\s+b/(.*)', line)\n+                    if m:\n+                        file = m.group(1)\n+                        pattern = p4_keywords_regexp_for_file(file)\n+                        next\n+\n+                    if pattern:\n+                        print line\n+\n+                    if pattern and re.search(pattern, line):\n+                        print \"got match on %s in %s in %s\" % (pattern, line, file)\n+                        kwfiles[file] = pattern\n+\n+                for file in kwfiles:\n+                    if verbose:\n+                        print \"zapping %s with %s\" % (line,pattern)\n+                    self.patchRCSKeywords(file, kwfiles[file])\n+                    fixed_rcs_keywords = True\n+\n+            if fixed_rcs_keywords:\n+                print \"Retrying the patch with RCS keywords cleaned up\"\n+                if os.system(tryPatchCmd) == 0:\n+                    patch_succeeded = True\n+\n+        if not patch_succeeded:\n             print \"What do you want to do?\"\n             response = \"x\"\n             while response != \"s\" and response != \"a\" and response != \"w\":\n@@ -1585,15 +1685,11 @@ class P4Sync(Command, P4UserMap):\n \n         # Note that we do not try to de-mangle keywords on utf16 files,\n         # even though in theory somebody may want that.\n-        if type_base in (\"text\", \"unicode\", \"binary\"):\n-            if \"ko\" in type_mods:\n-                text = ''.join(contents)\n-                text = re.sub(r'\\$(Id|Header):[^$]*\\$', r'$\\1$', text)\n-                contents = [ text ]\n-            elif \"k\" in type_mods:\n-                text = ''.join(contents)\n-                text = re.sub(r'\\$(Id|Header|Author|Date|DateTime|Change|File|Revision):[^$]*\\$', r'$\\1$', text)\n-                contents = [ text ]\n+        pattern = p4_keywords_regexp_for_type(type_base, type_mods)\n+        if pattern:\n+            text = ''.join(contents)\n+            text = re.sub(pattern, r'$\\1$', text)\n+            contents = [ text ]\n \n         self.gitStream.write(\"M %s inline %s\\n\" % (git_mode, relPath))\n \ndiff --git a/t/t9810-git-p4-rcs.sh b/t/t9810-git-p4-rcs.sh\nnew file mode 100755\nindex 0000000..77eebd2\n--- /dev/null\n+++ b/t/t9810-git-p4-rcs.sh\n@@ -0,0 +1,304 @@\n+#!/bin/sh\n+\n+test_description='git-p4 rcs keywords'\n+\n+. ./lib-git-p4.sh\n+\n+test_expect_success 'start p4d' '\n+\tstart_p4d\n+'\n+\n+#\n+# Make one file with keyword lines at the top, and\n+# enough plain text to be able to test modifications\n+# far away from the keywords.\n+#\n+test_expect_success 'init depot' '\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tcat <<-\\EOF >filek &&\n+\t\t$Id$\n+\t\t/* $Revision$ */\n+\t\t# $Change$\n+\t\tline4\n+\t\tline5\n+\t\tline6\n+\t\tline7\n+\t\tline8\n+\t\tEOF\n+\t\tcp filek fileko &&\n+\t\tsed -i \"s/Revision/Revision: do not scrub me/\" fileko\n+\t\tcp fileko file_text &&\n+\t\tsed -i \"s/Id/Id: do not scrub me/\" file_text\n+\t\tp4 add -t text+k filek &&\n+\t\tp4 submit -d \"filek\" &&\n+\t\tp4 add -t text+ko fileko &&\n+\t\tp4 submit -d \"fileko\" &&\n+\t\tp4 add -t text file_text &&\n+\t\tp4 submit -d \"file_text\"\n+\t)\n+'\n+\n+#\n+# Generate these in a function to make it easy to use single quote marks.\n+#\n+write_scrub_scripts() {\n+\tcat >\"$TRASH_DIRECTORY/scrub_k.py\" <<-\\EOF &&\n+\timport re, sys\n+\tsys.stdout.write(re.sub(r'(?i)\\$(Id|Header|Author|Date|DateTime|Change|File|Revision):[^$]*\\$', r'$\\1$', sys.stdin.read()))\n+\tEOF\n+\tcat >\"$TRASH_DIRECTORY/scrub_ko.py\" <<-\\EOF\n+\timport re, sys\n+\tsys.stdout.write(re.sub(r'(?i)\\$(Id|Header):[^$]*\\$', r'$\\1$', sys.stdin.read()))\n+\tEOF\n+}\n+\n+test_expect_success 'scrub scripts' '\n+\twrite_scrub_scripts\n+'\n+\n+#\n+# Compare $cli/file to its scrubbed version, should be different.\n+# Compare scrubbed $cli/file to $git/file, should be same.\n+#\n+scrub_k_check() {\n+\tfile=$1 &&\n+\tscrub=\"$TRASH_DIRECTORY/$file\" &&\n+\t\"$PYTHON_PATH\" \"$TRASH_DIRECTORY/scrub_k.py\" <\"$git/$file\" >\"$scrub\" &&\n+\t! test_cmp \"$cli/$file\" \"$scrub\" &&\n+\ttest_cmp \"$git/$file\" \"$scrub\" &&\n+\trm \"$scrub\"\n+}\n+scrub_ko_check() {\n+\tfile=$1 &&\n+\tscrub=\"$TRASH_DIRECTORY/$file\" &&\n+\t\"$PYTHON_PATH\" \"$TRASH_DIRECTORY/scrub_ko.py\" <\"$git/$file\" >\"$scrub\" &&\n+\t! test_cmp \"$cli/$file\" \"$scrub\" &&\n+\ttest_cmp \"$git/$file\" \"$scrub\" &&\n+\trm \"$scrub\"\n+}\n+\n+#\n+# Modify far away from keywords.  If no RCS lines show up\n+# in the diff, there is no conflict.\n+#\n+test_expect_success 'edit far away from RCS lines' '\n+\ttest_when_finished cleanup_git &&\n+\t\"$GITP4\" clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\tsed -i \"s/^line7/line7 edit/\" filek &&\n+\t\tgit commit -m \"filek line7 edit\" filek &&\n+\t\t\"$GITP4\" submit &&\n+\t\tscrub_k_check filek\n+\t)\n+'\n+\n+#\n+# Modify near the keywords.  This will require RCS scrubbing.\n+#\n+test_expect_success 'edit near RCS lines' '\n+\ttest_when_finished cleanup_git &&\n+\t\"$GITP4\" clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\tgit config git-p4.attemptRCSCleanup true &&\n+\t\tsed -i \"s/^line4/line4 edit/\" filek &&\n+\t\tgit commit -m \"filek line4 edit\" filek &&\n+\t\t\"$GITP4\" submit &&\n+\t\tscrub_k_check filek\n+\t)\n+'\n+\n+#\n+# Modify the keywords themselves.  This also will require RCS scrubbing.\n+#\n+test_expect_success 'edit keyword lines' '\n+\ttest_when_finished cleanup_git &&\n+\t\"$GITP4\" clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\tgit config git-p4.attemptRCSCleanup true &&\n+\t\tsed -i \"/Revision/d\" filek &&\n+\t\tgit commit -m \"filek remove Revision line\" filek &&\n+\t\t\"$GITP4\" submit &&\n+\t\tscrub_k_check filek\n+\t)\n+'\n+\n+#\n+# Scrubbing text+ko files should not alter all keywords, just Id, Header.\n+#\n+test_expect_success 'scrub ko files differently' '\n+\ttest_when_finished cleanup_git &&\n+\t\"$GITP4\" clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\tgit config git-p4.attemptRCSCleanup true &&\n+\t\tsed -i \"s/^line4/line4 edit/\" fileko &&\n+\t\tgit commit -m \"fileko line4 edit\" fileko &&\n+\t\t\"$GITP4\" submit &&\n+\t\tscrub_ko_check fileko &&\n+\t\t! scrub_k_check fileko\n+\t)\n+'\n+\n+# hack; git-p4 submit should do it on its own\n+test_expect_success 'cleanup after failure' '\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tp4 revert ...\n+\t)\n+'\n+\n+#\n+# Do not scrub anything but +k or +ko files.  Sneak a change into\n+# the cli file so that submit will get a conflict.  Make sure that\n+# scrubbing doesn't make a mess of things.\n+#\n+# Assumes that git-p4 exits leaving the p4 file open, with the\n+# conflict-generating patch unapplied.\n+#\n+# This might happen only if the git repo is behind the p4 repo at\n+# submit time, and there is a conflict.\n+#\n+test_expect_success 'do not scrub plain text' '\n+\ttest_when_finished cleanup_git &&\n+\t\"$GITP4\" clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\tgit config git-p4.attemptRCSCleanup true &&\n+\t\tsed -i \"s/^line4/line4 edit/\" file_text &&\n+\t\tgit commit -m \"file_text line4 edit\" file_text &&\n+\t\t(\n+\t\t\tcd \"$cli\" &&\n+\t\t\tp4 open file_text &&\n+\t\t\tsed -i \"s/^line5/line5 p4 edit/\" file_text &&\n+\t\t\tp4 submit -d \"file5 p4 edit\"\n+\t\t) &&\n+\t\t! \"$GITP4\" submit &&\n+\t\t(\n+\t\t\t# exepct something like:\n+\t\t\t#    file_text - file(s) not opened on this client\n+\t\t\t# but not copious diff output\n+\t\t\tcd \"$cli\" &&\n+\t\t\tp4 diff file_text >wc &&\n+\t\t\ttest_line_count = 1 wc\n+\t\t)\n+\t)\n+'\n+\n+# hack; git-p4 submit should do it on its own\n+test_expect_success 'cleanup after failure 2' '\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tp4 revert ...\n+\t)\n+'\n+\n+create_kw_file() {\n+\tcat <<\\EOF >\"$1\"\n+/* A file\n+\tId: $Id$\n+\tRevision: $Revision$\n+\tFile: $File$\n+ */\n+int main(int argc, const char **argv) {\n+\treturn 0;\n+}\n+EOF\n+}\n+\n+test_expect_success 'add kwfile' '\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\techo file1 >file1 &&\n+\t\tp4 add file1 &&\n+\t\tp4 submit -d \"file 1\" &&\n+\t\tcreate_kw_file kwfile1.c &&\n+\t\tp4 add kwfile1.c &&\n+\t\tp4 submit -d \"Add rcw kw file\" kwfile1.c\n+\t)\n+'\n+\n+p4_append_to_file() {\n+\tf=$1 &&\n+\tp4 edit -t ktext \"$f\" &&\n+\techo \"/* $(date) */\" >>\"$f\" &&\n+\tp4 submit -d \"appending a line in p4\"\n+}\n+\n+# Create some files with RCS keywords. If they get modified\n+# elsewhere then the version number gets bumped which then\n+# results in a merge conflict if we touch the RCS kw lines,\n+# even though the change itself would otherwise apply cleanly.\n+test_expect_success 'cope with rcs keyword expansion damage' '\n+\ttest_when_finished cleanup_git &&\n+\t\"$GITP4\" clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\tgit config git-p4.attemptRCSCleanup true &&\n+\t\t(cd ../cli && p4_append_to_file kwfile1.c) &&\n+\t\told_lines=$(wc -l <kwfile1.c) &&\n+\t\tperl -n -i -e \"print unless m/Revision:/\" kwfile1.c &&\n+\t\tnew_lines=$(wc -l <kwfile1.c) &&\n+\t\texpr $new_lines == $old_lines - 1 &&\n+\n+\t\tgit add kwfile1.c &&\n+\t\tgit commit -m \"Zap an RCS kw line\" &&\n+\t\t\"$GITP4\" submit &&\n+\t\t\"$GITP4\" rebase &&\n+\t\tgit diff p4/master &&\n+\t\t\"$GITP4\" commit &&\n+\t\techo \"try modifying in both\" &&\n+\t\tcd \"$cli\" &&\n+\t\tp4 edit kwfile1.c &&\n+\t\techo \"line from p4\" >>kwfile1.c &&\n+\t\tp4 submit -d \"add a line in p4\" kwfile1.c &&\n+\t\tcd \"$git\" &&\n+\t\techo \"line from git at the top\" | cat - kwfile1.c >kwfile1.c.new &&\n+\t\tmv kwfile1.c.new kwfile1.c &&\n+\t\tgit commit -m \"Add line in git at the top\" kwfile1.c &&\n+\t\t\"$GITP4\" rebase &&\n+\t\t\"$GITP4\" submit\n+\t)\n+'\n+\n+test_expect_success 'cope with rcs keyword file deletion' '\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\techo \"\\$Revision\\$\" >kwdelfile.c &&\n+\t\tp4 add -t ktext kwdelfile.c &&\n+\t\tp4 submit -d \"Add file to be deleted\" &&\n+\t\tcat kwdelfile.c &&\n+\t\tgrep -q 1 kwdelfile.c\n+\t) &&\n+\t\"$GITP4\" clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgrep -q Revision kwdelfile.c &&\n+\t\tgit rm -f kwdelfile.c &&\n+\t\tgit commit -m \"Delete a file containing RCS keywords\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\tgit config git-p4.attemptRCSCleanup true &&\n+\t\t\"$GITP4\" submit\n+\t) &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tp4 sync &&\n+\t\t[ ! -f kwdelfile.c ]\n+\t)\n+'\n+\n+test_expect_success 'kill p4d' '\n+\tkill_p4d\n+'\n+\n+test_done\n-- \n1.7.9.259.ga92e\n"},{"id":"184727","messageId":"7vvcn93tvr.fsf@alter.siamese.dyndns.org","threadId":"29628","inReplyTo":"1329258835-17223-2-git-send-email-luke@diamand.org","subject":"Re: [PATCHv3] git-p4: add initial support for RCS keywords","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-14T22:58:00Z","receivedAt":"2012-02-14T22:58:00Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"It seems that this seems to have been further refactored by introducing a\ncouple of new helper functions.  I'll replae what was queued in 'pu' with\nthis, with the following minor fix-ups.\n\nThanks.\n\n t/t9810-git-p4-rcs.sh |   10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t9810-git-p4-rcs.sh b/t/t9810-git-p4-rcs.sh\nindex 77eebd2..3013bab 100755\n--- a/t/t9810-git-p4-rcs.sh\n+++ b/t/t9810-git-p4-rcs.sh\n@@ -201,7 +201,7 @@ test_expect_success 'cleanup after failure 2' '\n \t)\n '\n \n-create_kw_file() {\n+create_kw_file () {\n \tcat <<\\EOF >\"$1\"\n /* A file\n \tId: $Id$\n@@ -226,7 +226,7 @@ test_expect_success 'add kwfile' '\n \t)\n '\n \n-p4_append_to_file() {\n+p4_append_to_file () {\n \tf=$1 &&\n \tp4 edit -t ktext \"$f\" &&\n \techo \"/* $(date) */\" >>\"$f\" &&\n@@ -248,7 +248,7 @@ test_expect_success 'cope with rcs keyword expansion damage' '\n \t\told_lines=$(wc -l <kwfile1.c) &&\n \t\tperl -n -i -e \"print unless m/Revision:/\" kwfile1.c &&\n \t\tnew_lines=$(wc -l <kwfile1.c) &&\n-\t\texpr $new_lines == $old_lines - 1 &&\n+\t\ttest $new_lines = $(($old_lines - 1)) &&\n \n \t\tgit add kwfile1.c &&\n \t\tgit commit -m \"Zap an RCS kw line\" &&\n@@ -278,12 +278,12 @@ test_expect_success 'cope with rcs keyword file deletion' '\n \t\tp4 add -t ktext kwdelfile.c &&\n \t\tp4 submit -d \"Add file to be deleted\" &&\n \t\tcat kwdelfile.c &&\n-\t\tgrep -q 1 kwdelfile.c\n+\t\tgrep 1 kwdelfile.c\n \t) &&\n \t\"$GITP4\" clone --dest=\"$git\" //depot &&\n \t(\n \t\tcd \"$git\" &&\n-\t\tgrep -q Revision kwdelfile.c &&\n+\t\tgrep Revision kwdelfile.c &&\n \t\tgit rm -f kwdelfile.c &&\n \t\tgit commit -m \"Delete a file containing RCS keywords\" &&\n \t\tgit config git-p4.skipSubmitEdit true &&\n"},{"id":"185071","messageId":"20120221121834.GB18317@padd.com","threadId":"29628","inReplyTo":"1329258835-17223-2-git-send-email-luke@diamand.org","subject":"Re: [PATCHv3] git-p4: add initial support for RCS keywords","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-02-21T12:18:34Z","receivedAt":"2012-02-21T12:18:34Z","isPatch":false,"sender":{"key":"pw@padd.com","avatar":null},"body":"luke@diamand.org wrote on Tue, 14 Feb 2012 22:33 +0000:\n> diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt\n[..]\n> +git-p4.attemptRCSCleanup:\n> +    If enabled, 'git p4 submit' will attempt to sort cleanup RCS keywords\n> +    ($Header$, etc). These would otherwise cause merge conflicts and prevent\n> +    the submit going ahead. This option should be considered experimental at\n> +    present.\n\n=> \"attempt to cleanup\"\n\n> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\n[..]\n> +#\n> +# return the raw p4 type of a file (text, text+ko, etc)\n> +#\n> +def p4_type(file):\n> +    files = p4_read_pipe_lines([\"files\", file])\n> +    info = files[0]\n> +    m = re.search(r'\\(([a-z0-9A-Z+]+)\\)\\s*$', info)\n> +    if m:\n> +        ret = m.group(1)\n> +        if verbose:\n> +            print \"%s => %s\" % (file, ret)\n> +        return ret\n> +    else:\n> +        die(\"Could not extract file type from '%s'\" % info)\n\np4 fstat -T headType\ngives just the type, no need to parse the info string\n\n> +#\n> +# Given a type base and modifier, return a regexp matching\n> +# the keywords that can be expanded in the file\n> +#\n> +def p4_keywords_regexp_for_type(base, type_mods):\n> +    if base in (\"text\", \"unicode\", \"binary\"):\n> +        if \"ko\" in type_mods:\n> +            return r'\\$(Id|Header)[^$]*\\$'\n> +        elif \"k\" in type_mods:\n> +            return r'\\$(Id|Header|Author|Date|DateTime|Change|File|Revision)[^$]*\\$'\n\nStill no \":\"?  Won't that match too much?\n\n> +    def patchRCSKeywords(self, file, pattern):\n\nNice, clean.\n\n>      def p4UserForCommit(self,id):\n[..]\n> +                for line in read_pipe_lines(diffcmd):\n> +                    # read diff lines: for each file reported, if it can have\n> +                    # keywords expanded, and the diff contains keywords, then\n> +                    # try zapping the p4 file.\n> +                    m = re.match(r'^diff --git a/(.*)\\s+b/(.*)', line)\n\nI'm still sort of iffy on this.  You have editedFiles, and could\njust use that directly.  Grepping through the diff won't give you\nany more information.\n\n> +                    if pattern:\n> +                        print line\n\nDebugging?  Maybe add a leading comment saying why this line.\n\n> @@ -1585,15 +1685,11 @@ class P4Sync(Command, P4UserMap):\n>  \n>          # Note that we do not try to de-mangle keywords on utf16 files,\n>          # even though in theory somebody may want that.\n> -        if type_base in (\"text\", \"unicode\", \"binary\"):\n> -            if \"ko\" in type_mods:\n> -                text = ''.join(contents)\n> -                text = re.sub(r'\\$(Id|Header):[^$]*\\$', r'$\\1$', text)\n> -                contents = [ text ]\n> -            elif \"k\" in type_mods:\n> -                text = ''.join(contents)\n> -                text = re.sub(r'\\$(Id|Header|Author|Date|DateTime|Change|File|Revision):[^$]*\\$', r'$\\1$', text)\n> -                contents = [ text ]\n> +        pattern = p4_keywords_regexp_for_type(type_base, type_mods)\n> +        if pattern:\n> +            text = ''.join(contents)\n> +            text = re.sub(pattern, r'$\\1$', text)\n> +            contents = [ text ]\n\nNice.  Glad you refactored this.\n\nFix the colon thing at least, then happy to add my Acked-By.\n\nSorry for the delay.\n\n\t\t-- Pete\n"},{"id":"185086","messageId":"CAEe=O8qui8PryuZiZNDwLk39+tKVDnh+5eP9m_WrHi=K9ekMNQ@mail.gmail.com","threadId":"29628","inReplyTo":"20120221121834.GB18317@padd.com","subject":"Re: [PATCHv3] git-p4: add initial support for RCS keywords","fromName":"Eric Scouten","fromEmail":"eric@scouten.com","sentAt":"2012-02-21T16:54:17Z","receivedAt":"2012-02-21T16:54:17Z","isPatch":false,"sender":{"key":"eric@scouten.com","avatar":"https://gravatar.com/avatar/2f15dca16692989615c8c7f83e7a06bb25bc3121425c8ba2998b93868eb01aa2?d=mp&s=160"},"body":"[Resent in plain-text. Apologies for dupe content to Pete and Luke.]\n\nOn Tue, Feb 21, 2012 at 04:18, Pete Wyckoff <pw@padd.com> wrote:\n\n> luke@diamand.org wrote on Tue, 14 Feb 2012 22:33 +0000:\n\n> > diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\n> > +#\n> > +# Given a type base and modifier, return a regexp matching\n> > +# the keywords that can be expanded in the file\n> > +#\n> > +def p4_keywords_regexp_for_type(base, type_mods):\n> > +    if base in (\"text\", \"unicode\", \"binary\"):\n> > +        if \"ko\" in type_mods:\n> > +            return r'\\$(Id|Header)[^$]*\\$'\n> > +        elif \"k\" in type_mods:\n> > +            return\n> > r'\\$(Id|Header|Author|Date|DateTime|Change|File|Revision)[^$]*\\$'\n>\n> Still no \":\"?  Won't that match too much?\n\n> Fix the colon thing at least, then happy to add my Acked-By.\n\nNo, that would be an incorrect change. The colon is added by P4 when\nit expands the keyword pattern, but it is *not* part of the pattern\nrequired by P4 to trigger a keyword expansion.\n\nhttp://kb.perforce.com/article/54/using-rcs-keywords\n\n-Eric\n\n\n--\nEric Scouten :: software developer, photographer :: Poulsbo, WA (near Seattle)\nhttp://ericscouten.com :: click for Flickr, Facebook, Twitter, LinkedIn links\n"},{"id":"185087","messageId":"7vobssgkt6.fsf@alter.siamese.dyndns.org","threadId":"29628","inReplyTo":"CAEe=O8qui8PryuZiZNDwLk39+tKVDnh+5eP9m_WrHi=K9ekMNQ@mail.gmail.com","subject":"Re: [PATCHv3] git-p4: add initial support for RCS keywords","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-21T17:25:57Z","receivedAt":"2012-02-21T17:25:57Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Scouten <eric@scouten.com> writes:\n\n>> > r'\\$(Id|Header|Author|Date|DateTime|Change|File|Revision)[^$]*\\$'\n>>\n>> Still no \":\"?  Won't that match too much?\n>\n>> Fix the colon thing at least, then happy to add my Acked-By.\n>\n> No, that would be an incorrect change. The colon is added by P4 when\n> it expands the keyword pattern, but it is *not* part of the pattern\n> required by P4 to trigger a keyword expansion.\n>\n> http://kb.perforce.com/article/54/using-rcs-keywords\n\nI have this suspicion that both Pete and your last sentence is correct,\nbut the regexp in the patch and your \"would be an incorrect change\" are\nwrong.\n\nI am not a P4 expert, but I would be very surprised if P4 expands \"$Ida$\"\nas if it is \"$Id$\" or \"$Id: old expansion$\", which the regexp would match.\n\nWouldn't it be more like this?\n\n\t\\$\t\t\t# begins with a dollar, followed by...\n        ( Id | Header | ... )\t# one of these keywords, followed by ...\n        ( :[^$]+ )?\t\t# possibly an old expansion, followed by\n\t\\$\t\t\t# another dollar sign\n"},{"id":"185090","messageId":"CAEe=O8pnYF1Zxh33Wkm7LfFWGZwDLNRRCDdPYJ1QWjgR8OR+7w@mail.gmail.com","threadId":"29628","inReplyTo":"7vobssgkt6.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv3] git-p4: add initial support for RCS keywords","fromName":"Eric Scouten","fromEmail":"eric@scouten.com","sentAt":"2012-02-21T17:59:35Z","receivedAt":"2012-02-21T17:59:35Z","isPatch":false,"sender":{"key":"eric@scouten.com","avatar":"https://gravatar.com/avatar/2f15dca16692989615c8c7f83e7a06bb25bc3121425c8ba2998b93868eb01aa2?d=mp&s=160"},"body":"On Tue, Feb 21, 2012 at 09:25, Junio C Hamano <gitster@pobox.com> wrote:\n> Eric Scouten <eric@scouten.com> writes:\n>\n>>> > r'\\$(Id|Header|Author|Date|DateTime|Change|File|Revision)[^$]*\\$'\n>>>\n>>> Still no \":\"?  Won't that match too much?\n>>\n>>> Fix the colon thing at least, then happy to add my Acked-By.\n>>\n>> No, that would be an incorrect change. The colon is added by P4 when\n>> it expands the keyword pattern, but it is *not* part of the pattern\n>> required by P4 to trigger a keyword expansion.\n>>\n>> http://kb.perforce.com/article/54/using-rcs-keywords\n>\n> I have this suspicion that both Pete and your last sentence is correct,\n> but the regexp in the patch and your \"would be an incorrect change\" are\n> wrong.\n>\n> I am not a P4 expert, but I would be very surprised if P4 expands \"$Ida$\"\n> as if it is \"$Id$\" or \"$Id: old expansion$\", which the regexp would match.\n>\n> Wouldn't it be more like this?\n>\n>        \\$                      # begins with a dollar, followed by...\n>        ( Id | Header | ... )   # one of these keywords, followed by ...\n>        ( :[^$]+ )?             # possibly an old expansion, followed by\n>        \\$                      # another dollar sign\n\nGood catch. Yes, you're probably right.\n\n-- \nEric Scouten :: software developer, photographer :: Poulsbo, WA (near Seattle)\nhttp://ericscouten.com :: click for Flickr, Facebook, Twitter, LinkedIn links\n"}]}