{"thread":{"id":"29579","subject":"[RFC/PATCHv1 0/2] git-p4: possible RCS keyword fixes","startedAt":"2012-02-09T11:03:27Z","lastAt":"2012-02-09T22:55:39Z","messageCount":4,"participants":["Luke Diamand","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"184230","messageId":"1328785409-30936-1-git-send-email-luke@diamand.org","threadId":"29579","inReplyTo":null,"subject":"[RFC/PATCHv1 0/2] git-p4: possible RCS keyword fixes","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2012-02-09T11:03:27Z","receivedAt":"2012-02-09T11:03:27Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"This contains a possible way to fixup RCS keyword woes in git-p4.\n\nThe first patch adds a test case to demonstrate the problem. It would\nbe useful to get this into git proper.\n\nThe second patch shows a possible way to solve the problem, by\npatching up the RCS keywords in the p4 checked-out tree. This\npatch does not cover all cases and is poorly tested. I'd like to\nsee if this seems like a plausible approach.\n\nLuke Diamand (2):\n  git-p4: add test case for RCS keywords\n  git-p4: initial demonstration of possible RCS keyword fixup\n\n contrib/fast-import/git-p4 |   43 ++++++++++++++++++++++-\n t/t9810-git-p4-rcs.sh      |   82 ++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 123 insertions(+), 2 deletions(-)\n create mode 100755 t/t9810-git-p4-rcs.sh\n\n-- \n1.7.9.rc2.128.gfce41.dirty\n"},{"id":"184231","messageId":"1328785409-30936-2-git-send-email-luke@diamand.org","threadId":"29579","inReplyTo":"1328785409-30936-1-git-send-email-luke@diamand.org","subject":"[PATCHv4 1/2] git-p4: add test case for RCS keywords","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2012-02-09T11:03:28Z","receivedAt":"2012-02-09T11:03:28Z","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 adds a failing test case to demonstrate the problem.\n\nSigned-off-by: Luke Diamand <luke@diamand.org>\n---\n t/t9810-git-p4-rcs.sh |   81 +++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 81 insertions(+), 0 deletions(-)\n create mode 100755 t/t9810-git-p4-rcs.sh\n\ndiff --git a/t/t9810-git-p4-rcs.sh b/t/t9810-git-p4-rcs.sh\nnew file mode 100755\nindex 0000000..bad6272\n--- /dev/null\n+++ b/t/t9810-git-p4-rcs.sh\n@@ -0,0 +1,81 @@\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+create_kw_file() {\n+ cat <<'EOF' > $1\n+/* A file\n+   Id: $Id$\n+   Revision: $Revision$\n+   File: $File$\n+ */\n+int main(int argc, const char **argv) {\n+ return 0;\n+}\n+EOF\n+}\n+\n+test_expect_success 'init depot' '\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\techo file1 >file1 &&\n+\t\tp4 add file1 &&\n+\t\tp4 submit -d \"change 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+ f=$1\n+ p4 edit -t ktext $f &&\n+ echo \"/* $(date) */\" >> $f &&\n+ p4 submit -d \"appending a line in p4\" &&\n+ cat $f\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_failure 'cope with rcs keyword expansion damage' '\n+ \"$GITP4\" clone --dest=\"$git\" //depot &&\n+ cd \"$git\" &&\n+ git config git-p4.skipSubmitEdit true &&\n+ (cd ../cli && p4_append_to_file kwfile1.c) &&\n+ perl -n -i -e \"print unless m/Revision:/\" kwfile1.c &&\n+ git add kwfile1.c &&\n+ git commit -m \"Zap an RCS kw line\" &&\n+ \"$GITP4\" submit &&\n+ \"$GITP4\" rebase &&\n+ git diff p4/master &&\n+ \"$GITP4\" commit &&\n+ echo \"try modifying in both\" &&\n+ cd \"$cli\" &&\n+ p4 edit kwfile1.c &&\n+ echo \"line from p4\" >> kwfile1.c &&\n+ p4 submit -d \"add a line in p4\" kwfile1.c &&\n+ cd \"$git\" &&\n+ echo \"line from git at the top\" | cat - kwfile1.c > kwfile1.c.new &&\n+ mv kwfile1.c.new kwfile1.c &&\n+ git commit -m \"Add line in git at the top\" kwfile1.c &&\n+ \"$GITP4\" rebase &&\n+ \"$GITP4\" submit &&\n+\n+ cd \"$TRASH_DIRECTORY\" &&\n+ rm -rf \"$git\" && mkdir \"$git\"\n+'\n+\n+\n+test_expect_success 'kill p4d' '\n+\tkill_p4d\n+'\n+\n+test_done\n-- \n1.7.9.rc2.128.gfce41.dirty\n"},{"id":"184232","messageId":"1328785409-30936-3-git-send-email-luke@diamand.org","threadId":"29579","inReplyTo":"1328785409-30936-1-git-send-email-luke@diamand.org","subject":"[PATCHv4 2/2] git-p4: initial demonstration of possible RCS keyword fixup","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2012-02-09T11:03:29Z","receivedAt":"2012-02-09T11:03:29Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"This change has a go at showing a possible way to fixup RCS\nkeyword handling in git-p4.\n\nIt does not cope with deleted files.\nIt does not have good test coverage.\nIt does not solve the problem of the incorrect error messages.\nBut it does at least work after a fashion, and could provide\na starting point.\n\nSigned-off-by: Luke Diamand <luke@diamand.org>\n---\n contrib/fast-import/git-p4 |   43 +++++++++++++++++++++++++++++++++++++++++--\n t/t9810-git-p4-rcs.sh      |    1 +\n 2 files changed, 42 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex a78d9c5..205fefd 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -753,6 +753,23 @@ class P4Submit(Command, P4UserMap):\n \n         return result\n \n+    def patchRCSKeywords(self, file):\n+        # Attempt to zap the RCS keywords in a p4 controlled file\n+        p4_edit(file)\n+        (handle, outFileName) = tempfile.mkstemp()\n+        outFile = os.fdopen(handle, \"w+\")\n+        inFile = open(file, \"r\")\n+        for line in inFile.readlines():\n+            line = re.sub(r'\\$(Id|Header|Author|Date|DateTime|Change|File|Revision)[^$]*\\$',\n+                   r'$\\1$', line)\n+            outFile.write(line)\n+        inFile.close()\n+        outFile.close()\n+        # Forcibly overwrite the original file\n+        system(\"cat %s\" % outFileName)\n+        system([\"mv\", \"-f\", outFileName, file])\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 +935,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 +982,30 @@ 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+                for line in read_pipe_lines(diffcmd):\n+                    m = re.match(r'^diff --git a/(.*)\\s+b/(.*)', line)\n+                    if m:\n+                        file = m.group(1)\n+                    if re.match(r'.*\\$(Id|Header|Author|Date|DateTime|Change|File|Revision)[^$]*\\$', line):\n+                        self.patchRCSKeywords(file)\n+                        fixed_rcs_keywords = True\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@@ -1588,11 +1627,11 @@ class P4Sync(Command, P4UserMap):\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+                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+                text = re.sub(r'\\$(Id|Header|Author|Date|DateTime|Change|File|Revision)[^$]*\\$', r'$\\1$', text)\n                 contents = [ text ]\n \n         self.gitStream.write(\"M %s inline %s\\n\" % (git_mode, relPath))\ndiff --git a/t/t9810-git-p4-rcs.sh b/t/t9810-git-p4-rcs.sh\nindex bad6272..dc38dcc 100755\n--- a/t/t9810-git-p4-rcs.sh\n+++ b/t/t9810-git-p4-rcs.sh\n@@ -49,6 +49,7 @@ test_expect_failure 'cope with rcs keyword expansion damage' '\n  \"$GITP4\" clone --dest=\"$git\" //depot &&\n  cd \"$git\" &&\n  git config git-p4.skipSubmitEdit true &&\n+ git config git-p4.attemptRCSCleanup true &&\n  (cd ../cli && p4_append_to_file kwfile1.c) &&\n  perl -n -i -e \"print unless m/Revision:/\" kwfile1.c &&\n  git add kwfile1.c &&\n-- \n1.7.9.rc2.128.gfce41.dirty\n"},{"id":"184280","messageId":"7v8vkbvcpg.fsf@alter.siamese.dyndns.org","threadId":"29579","inReplyTo":"1328785409-30936-2-git-send-email-luke@diamand.org","subject":"Re: [PATCHv4 1/2] git-p4: add test case for RCS keywords","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-09T22:55:39Z","receivedAt":"2012-02-09T22:55:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Luke Diamand <luke@diamand.org> writes:\n\n> +create_kw_file() {\n> + cat <<'EOF' > $1\n\nPlease start this shell function like this instead:\n\n        create_kw_file () {\n                cat <<\\EOF >\"$1\"\n\nThe first line is merely cosmetic but matches the convention used in our\nshell scripts better.\n\nAs to the second line:\n\n * Quoting \"$1\" should not be strictly necessary according to POSIX rules,\n   but we saw reports that some shells \"helpfully\" give unnecessary\n   warnings when $1 (or whatever variable that holds the name of the file\n   the output is redirected into) contains $IFS characters.\n\n * An indent in our shell scripts is a HT, not a SP (or four spaces or\n   whatever).\n\n * And our convention is not to have an extra SP after redirection '>' (or\n   '<') operator.\n\n> +test_expect_success 'init depot' '\n> +\t(\n> +\t\tcd \"$cli\" &&\n> +\t\techo file1 >file1 &&\n> +\t\tp4 add file1 &&\n> +\t\tp4 submit -d \"change 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> + f=$1\n> + p4 edit -t ktext $f &&\n> + echo \"/* $(date) */\" >> $f &&\n> + p4 submit -d \"appending a line in p4\" &&\n> + cat $f\n> +}\n\nSurround all occurrences of '$f' with double-quotes, e.g.\n\n\tcat \"$f\"\n\nto avoid unexpected breakage when later somebody tries to use a file whose\nname happens to contain a space. The first assignment \"f=$1\" can stay as-is.\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_failure 'cope with rcs keyword expansion damage' '\n> + \"$GITP4\" clone --dest=\"$git\" //depot &&\n> + cd \"$git\" &&\n\nPlease do not cd around in test script outside a subshell.  Otherwise, any\nfailure in a subsequent command in this && chain before you reach the next\n\"cd\" will leave you in an unexpected directory and can break later tests.\n\nThe worst example would be\n\n\ttest_expect_something 'one' '\n\t\tmkdir -p git/play/pen &&\n                cd git/play/pen &&\n                do something &&\n        \tcd ../../.. &&\n                do something that potentially can fail &&\n                cd git/play/pen &&\n                do something\n\t'\n\t# blindly assuming the above succeeded up to the last 'cd git'\n        test_expect_something 'two' '\n\t\tcd ../../.. &&\n        \trm -fr git &&\n                do something else\n\t'\n\nIf the first test fails in the middle before it brings you back down to\n'git/play/pen' with the last 'cd git/playpen', then the next test moves\nyou up three levels and you will be running \"rm -fr\" on a wrong 'git'!\n"}]}