{"thread":{"id":"41045","subject":"[PATCH v1] git-p4: ignore P4 changelists that only touch files","startedAt":"2015-12-20T14:59:38Z","lastAt":"2015-12-22T23:30:02Z","messageCount":4,"participants":["larsxschneider@gmail.com","Lars Schneider","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"274779","messageId":"1450623579-17013-1-git-send-email-larsxschneider@gmail.com","threadId":"41045","inReplyTo":null,"subject":"[PATCH v1] git-p4: ignore P4 changelists that only touch files","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-12-20T14:59:38Z","receivedAt":"2015-12-20T14:59:38Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nHi,\n\nthis patch improves \"git-p4: add option to keep empty commits\" (4ae048e)\nand therefore should be applied on master.\n\nThanks,\nLars\n\nLars Schneider (1):\n git-p4: ignore P4 changelists that only touch files\n\n git-p4.py | 54 ++++++++++++++++++++++++++++++++++--\n t/t9826-git-p4-keep-empty-commits.sh | 48 ++++++++++++++++++++++++++++++--\n 2 files changed, 97 insertions(+), 5 deletions(-)\n\n--\n2.5.1\n"},{"id":"274780","messageId":"1450623579-17013-2-git-send-email-larsxschneider@gmail.com","threadId":"41045","inReplyTo":"1450623579-17013-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v1] git-p4: ignore P4 changelists that only touch files","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-12-20T14:59:39Z","receivedAt":"2015-12-20T14:59:39Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nDetect if a P4 changelists only changes the creation or modification\ndate of its files. Ignore these kind of changed files on import to avoid\nempty Git commits. This would happen because Git does not track these\nproperties.\n\nSigned-off-by: Lars Schneider \n---\n git-p4.py | 54 ++++++++++++++++++++++++++++++++++--\n t/t9826-git-p4-keep-empty-commits.sh | 48 ++++++++++++++++++++++++++++++--\n 2 files changed, 97 insertions(+), 5 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 7a9dd6a..c38a8f7 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -328,6 +328,15 @@ def split_p4_type(p4type):\n return (base, mods)\n \n #\n+# converts P4 mods to Git file modes\n+#\n+def convert_to_git_mode(p4_mods):\n+ if 'x' in p4_mods:\n+ return '100755'\n+ else:\n+ return '100644'\n+\n+#\n # return the raw p4 type of a file (text, text+ko, etc)\n #\n def p4_type(f):\n@@ -335,6 +344,35 @@ def p4_type(f):\n return results[0]['headType']\n \n #\n+# Returns true if Git would mark the diff of a file `f` in `changelist1`\n+# and `changelist2` as modified. The change detection is very conservative\n+# as a P4 file type change would usually not change the Git content.\n+# However, in case of symlinks or UTF-16 files that might happen.\n+#\n+def p4_file_changed(f, changelist1, changelist2):\n+ if not changelist1 or not changelist2:\n+ return True\n+ results = p4CmdList([\n+ 'diff2',\n+ '{}@{}'.format(wildcard_encode(f), changelist1),\n+ '{}@{}'.format(wildcard_encode(f), changelist2)\n+ ])\n+ if len(results) != 1:\n+ return True\n+ elif ('status' not in results[0] or\n+ 'type' not in results[0] or\n+ 'type2' not in results[0]):\n+ return True\n+ else:\n+ (type1, mods1) = split_p4_type(results[0]['type'])\n+ (type2, mods2) = split_p4_type(results[0]['type2'])\n+ return (\n+ results[0]['status'] != 'identical' or\n+ convert_to_git_mode(mods1) != convert_to_git_mode(mods2) or\n+ type1 != type2\n+ )\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@@ -2394,10 +2432,8 @@ class P4Sync(Command, P4UserMap):\n sys.stdout.flush()\n \n (type_base, type_mods) = split_p4_type(file[\"type\"])\n+ git_mode = convert_to_git_mode(type_mods)\n \n- git_mode = \"100644\"\n- if \"x\" in type_mods:\n- git_mode = \"100755\"\n if type_base == \"symlink\":\n git_mode = \"120000\"\n # p4 print on a symlink sometimes contains \"target\\n\";\n@@ -2656,6 +2692,14 @@ class P4Sync(Command, P4UserMap):\n files = [f for f in files\n if self.inClientSpec(f['path']) and self.hasBranchPrefix(f['path'])]\n \n+ files = [f for f in files\n+ if not f['action'] == 'edit' or p4_file_changed(\n+ f['path'],\n+ details.get('previous_change', None),\n+ details['change']\n+ )\n+ ]\n+\n if not files and not gitConfigBool('git-p4.keepEmptyCommits'):\n print('Ignoring revision {0} as it would produce an empty commit.'\n .format(details['change']))\n@@ -2993,8 +3037,10 @@ class P4Sync(Command, P4UserMap):\n \n def importChanges(self, changes):\n cnt = 1\n+ previous_change = None\n for change in changes:\n description = p4_describe(change)\n+ description['previous_change'] = previous_change\n self.updateOptionDict(description)\n \n if not self.silent:\n@@ -3069,6 +3115,8 @@ class P4Sync(Command, P4UserMap):\n self.initialParent)\n # only needed once, to connect to the previous commit\n self.initialParent = \"\"\n+\n+ previous_change = change\n except IOError:\n print self.gitError.read()\n sys.exit(1)\ndiff --git a/t/t9826-git-p4-keep-empty-commits.sh b/t/t9826-git-p4-keep-empty-commits.sh\nindex be12960..23a300a 100755\n--- a/t/t9826-git-p4-keep-empty-commits.sh\n+++ b/t/t9826-git-p4-keep-empty-commits.sh\n@@ -15,7 +15,7 @@ test_expect_success 'Create a repo' '\n \n \t\tmkdir -p subdir &&\n \n-\t\t>subdir/file1.txt &&\n+\t\techo \"content1\" >subdir/file1.txt &&\n \t\tp4 add subdir/file1.txt &&\n \t\tp4 submit -d \"Add file 1\" &&\n \n@@ -35,7 +35,21 @@ test_expect_success 'Create a repo' '\n \t\tp4 submit -d \"Remove file 3\" &&\n \n \t\tp4 delete file4.txt &&\n-\t\tp4 submit -d \"Remove file 4\"\n+\t\tp4 submit -d \"Remove file 4\" &&\n+\n+\t\tp4 edit subdir/file1.txt &&\n+\t\ttouch subdir/file1.txt &&\n+\t\tp4 submit -d \"Touch file1 - no changes\" subdir/file1.txt\n+\n+\t\tp4 edit -t text+x subdir/file1.txt &&\n+\t\tp4 submit -d \"Change execution bit\" subdir/file1.txt\n+\n+\t\tp4 edit -t binary+x subdir/file1.txt &&\n+\t\tp4 submit -d \"Change filetype\" subdir/file1.txt\n+\n+\t\tp4 edit subdir/file1.txt &&\n+\t\techo \"content1 changed\" >subdir/file1.txt &&\n+\t\tp4 submit -d \"Change content\" subdir/file1.txt\n \t)\n '\n \n@@ -47,6 +61,15 @@ test_expect_success 'Clone repo root path with all history' '\n \t\tgit init . &&\n \t\tgit p4 clone --use-client-spec --destination=\"$git\" //depot@all &&\n \t\tcat >expect <<-\\EOF &&\n+Change content\n+[git-p4: depot-paths = \"//depot/\": change = 10]\n+\n+Change filetype\n+[git-p4: depot-paths = \"//depot/\": change = 9]\n+\n+Change execution bit\n+[git-p4: depot-paths = \"//depot/\": change = 8]\n+\n Remove file 4\n [git-p4: depot-paths = \"//depot/\": change = 6]\n \n@@ -80,6 +103,18 @@ test_expect_success 'Clone repo subdir with all history but keep empty commits'\n \t\tgit config git-p4.keepEmptyCommits true &&\n \t\tgit p4 clone --use-client-spec --destination=\"$git\" //depot@all &&\n \t\tcat >expect <<-\\EOF &&\n+Change content\n+[git-p4: depot-paths = \"//depot/\": change = 10]\n+\n+Change filetype\n+[git-p4: depot-paths = \"//depot/\": change = 9]\n+\n+Change execution bit\n+[git-p4: depot-paths = \"//depot/\": change = 8]\n+\n+Touch file1 - no changes\n+[git-p4: depot-paths = \"//depot/\": change = 7]\n+\n Remove file 4\n [git-p4: depot-paths = \"//depot/\": change = 6]\n \n@@ -112,6 +147,15 @@ test_expect_success 'Clone repo subdir with all history' '\n \t\tgit init . &&\n \t\tgit p4 clone --use-client-spec --destination=\"$git\" --verbose //depot@all &&\n \t\tcat >expect <<-\\EOF &&\n+Change content\n+[git-p4: depot-paths = \"//depot/\": change = 10]\n+\n+Change filetype\n+[git-p4: depot-paths = \"//depot/\": change = 9]\n+\n+Change execution bit\n+[git-p4: depot-paths = \"//depot/\": change = 8]\n+\n Remove file 3\n [git-p4: depot-paths = \"//depot/\": change = 5]\n \n-- \n2.5.1\n"},{"id":"274847","messageId":"8EC36E87-CEC6-4E30-B606-4F4AC572B1DB@gmail.com","threadId":"41045","inReplyTo":"1450623579-17013-1-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v1] git-p4: ignore P4 changelists that only touch files","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-12-22T09:18:09Z","receivedAt":"2015-12-22T09:18:09Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 20 Dec 2015, at 15:59, larsxschneider@gmail.com wrote:\n\n> From: Lars Schneider \n> \n> Hi,\n> \n> this patch improves \"git-p4: add option to keep empty commits\" (4ae048e)\n> and therefore should be applied on master.\n> \n> Thanks,\n> Lars\n\nJunio just made me aware of the inappropriate phrasing here.\nThis is what I wanted to express:\n\nThis patch is based on master as it depends on \"git-p4: add option to keep empty commits\" (4ae048e)\"\n\n\nSorry,\nLars"},{"id":"274923","messageId":"xmqq7fk6cb9h.fsf@gitster.mtv.corp.google.com","threadId":"41045","inReplyTo":"8EC36E87-CEC6-4E30-B606-4F4AC572B1DB@gmail.com","subject":"Re: [PATCH v1] git-p4: ignore P4 changelists that only touch files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-22T23:30:02Z","receivedAt":"2015-12-22T23:30:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider writes:\n\n> Junio just made me aware of the inappropriate phrasing here.\n> This is what I wanted to express:\n>\n> This patch is based on master as it depends on \"git-p4: add option to keep empty commits\" (4ae048e)\"\n\nThanks.\n\nIt wasn't \"inappropriate\", but was merely prone to be misunderstood.\nAnd I did misunderstand it. Sorry for the noise.\n"}]}