{"thread":{"id":"44712","subject":"[PATCH] git-p4: Fix multi-path changelist empty commits","startedAt":"2016-12-15T17:28:16Z","lastAt":"2016-12-19T17:57:23Z","messageCount":6,"participants":["George Vanburgh","Luke Diamand","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"307847","messageId":"01020159037a8995-2d1da9d4-4a27-4b98-818b-432fc0ad8a52-000000@eu-west-1.amazonses.com","threadId":"44712","inReplyTo":null,"subject":"[PATCH] git-p4: Fix multi-path changelist empty commits","fromName":"George Vanburgh","fromEmail":"george@vanburgh.me","sentAt":"2016-12-15T17:14:39Z","receivedAt":"2016-12-15T17:28:16Z","isPatch":true,"sender":{"key":"george@vanburgh.me","avatar":"https://gravatar.com/avatar/575b39450d443981d8b1d1ca4b380c5af51b54adb6dbe3f737cb495e15b2b748?d=mp&s=160"},"body":"From: George Vanburgh <gvanburgh@bloomberg.net>\n\nWhen importing from multiple perforce paths - we may attempt to import a changelist that contains files from two (or more) of these depot paths. Currently, this results in multiple git commits - one containing the changes, and the other(s) as empty commits. This behavior was introduced in commit 1f90a64 (\"git-p4: reduce number of server queries for fetches\", 2015-12-19).\n\nReproduction Steps:\n\n1. Have a git repo cloned from a perforce repo using multiple depot paths (e.g. //depot/foo and //depot/bar).\n2. Submit a single change to the perforce repo that makes changes in both //depot/foo and //depot/bar.\n3. Run \"git p4 sync\" to sync the change from #2.\n\nChange is synced as multiple commits, one for each depot path that was affected.\n\nUsing a set, instead of a list inside p4ChangesForPaths() ensures that each changelist is unique to the returned list, and therefore only a single commit is generated for each changelist.\n\nReported-by: James Farwell <jfarwell@vmware.com>\nSigned-off-by: George Vanburgh <gvanburgh@bloomberg.net>\n---\n git-p4.py               |  4 ++--\n t/t9800-git-p4-basic.sh | 22 +++++++++++++++++++++-\n 2 files changed, 23 insertions(+), 3 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex fd5ca52..6307bc8 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -822,7 +822,7 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):\n                 die(\"cannot use --changes-block-size with non-numeric revisions\")\n             block_size = None\n \n-    changes = []\n+    changes = set()\n \n     # Retrieve changes a block at a time, to prevent running\n     # into a MaxResults/MaxScanRows error from the server.\n@@ -841,7 +841,7 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):\n \n         # Insert changes in chronological order\n         for line in reversed(p4_read_pipe_lines(cmd)):\n-            changes.append(int(line.split(\" \")[1]))\n+            changes.add(int(line.split(\" \")[1]))\n \n         if not block_size:\n             break\ndiff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh\nindex 0730f18..4d72e0b 100755\n--- a/t/t9800-git-p4-basic.sh\n+++ b/t/t9800-git-p4-basic.sh\n@@ -131,6 +131,26 @@ test_expect_success 'clone two dirs, @all, conflicting files' '\n \t)\n '\n \n+test_expect_success 'clone two dirs, each edited by submit, single git commit' '\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\techo sub1/f4 >sub1/f4 &&\n+\t\tp4 add sub1/f4 &&\n+\t\techo sub2/f4 >sub2/f4 &&\n+\t\tp4 add sub2/f4 &&\n+\t\tp4 submit -d \"sub1/f4 and sub2/f4\"\n+\t) &&\n+\tgit p4 clone --dest=\"$git\" //depot/sub1@all //depot/sub2@all &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\"\n+\t\tgit ls-files >lines &&\n+\t\ttest_line_count = 4 lines &&\n+\t\tgit log --oneline p4/master >lines &&\n+\t\ttest_line_count = 5 lines\n+\t)\n+'\n+\n revision_ranges=\"2000/01/01,#head \\\n \t\t 1,2080/01/01 \\\n \t\t 2000/01/01,2080/01/01 \\\n@@ -147,7 +167,7 @@ test_expect_success 'clone using non-numeric revision ranges' '\n \t\t(\n \t\t\tcd \"$git\" &&\n \t\t\tgit ls-files >lines &&\n-\t\t\ttest_line_count = 6 lines\n+\t\t\ttest_line_count = 8 lines\n \t\t)\n \tdone\n '\n\n--\nhttps://github.com/git/git/pull/311\n"},{"id":"307936","messageId":"CAE5ih7-+tahL4=OrW6F6UPKKRg1KFkw32e=pnTx6j2WTZ-BhOw@mail.gmail.com","threadId":"44712","inReplyTo":"01020159037a8995-2d1da9d4-4a27-4b98-818b-432fc0ad8a52-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH] git-p4: Fix multi-path changelist empty commits","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2016-12-17T00:47:55Z","receivedAt":"2016-12-17T00:48:03Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 15 December 2016 at 17:14, George Vanburgh <george@vanburgh.me> wrote:\n> From: George Vanburgh <gvanburgh@bloomberg.net>\n>\n> When importing from multiple perforce paths - we may attempt to import a changelist that contains files from two (or more) of these depot paths. Currently, this results in multiple git commits - one containing the changes, and the other(s) as empty commits. This behavior was introduced in commit 1f90a64 (\"git-p4: reduce number of server queries for fetches\", 2015-12-19).\n\nThat's definitely a bug, thanks for spotting that! Even more so for\nadding a test case.\n\n>\n> Reproduction Steps:\n>\n> 1. Have a git repo cloned from a perforce repo using multiple depot paths (e.g. //depot/foo and //depot/bar).\n> 2. Submit a single change to the perforce repo that makes changes in both //depot/foo and //depot/bar.\n> 3. Run \"git p4 sync\" to sync the change from #2.\n>\n> Change is synced as multiple commits, one for each depot path that was affected.\n>\n> Using a set, instead of a list inside p4ChangesForPaths() ensures that each changelist is unique to the returned list, and therefore only a single commit is generated for each changelist.\n\nThe change looks good to me apart from one missing \"&&\" in the test\ncase (see below).\n\nPossibly need to rewrap the comment line (I think there's a 72\ncharacter limit) ?\n\nLuke\n\n\n>\n> Reported-by: James Farwell <jfarwell@vmware.com>\n> Signed-off-by: George Vanburgh <gvanburgh@bloomberg.net>\n> ---\n>  git-p4.py               |  4 ++--\n>  t/t9800-git-p4-basic.sh | 22 +++++++++++++++++++++-\n>  2 files changed, 23 insertions(+), 3 deletions(-)\n>\n> diff --git a/git-p4.py b/git-p4.py\n> index fd5ca52..6307bc8 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -822,7 +822,7 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):\n>                  die(\"cannot use --changes-block-size with non-numeric revisions\")\n>              block_size = None\n>\n> -    changes = []\n> +    changes = set()\n>\n>      # Retrieve changes a block at a time, to prevent running\n>      # into a MaxResults/MaxScanRows error from the server.\n> @@ -841,7 +841,7 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):\n>\n>          # Insert changes in chronological order\n>          for line in reversed(p4_read_pipe_lines(cmd)):\n> -            changes.append(int(line.split(\" \")[1]))\n> +            changes.add(int(line.split(\" \")[1]))\n>\n>          if not block_size:\n>              break\n> diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh\n> index 0730f18..4d72e0b 100755\n> --- a/t/t9800-git-p4-basic.sh\n> +++ b/t/t9800-git-p4-basic.sh\n> @@ -131,6 +131,26 @@ test_expect_success 'clone two dirs, @all, conflicting files' '\n>         )\n>  '\n>\n> +test_expect_success 'clone two dirs, each edited by submit, single git commit' '\n> +       (\n> +               cd \"$cli\" &&\n> +               echo sub1/f4 >sub1/f4 &&\n> +               p4 add sub1/f4 &&\n> +               echo sub2/f4 >sub2/f4 &&\n> +               p4 add sub2/f4 &&\n> +               p4 submit -d \"sub1/f4 and sub2/f4\"\n> +       ) &&\n> +       git p4 clone --dest=\"$git\" //depot/sub1@all //depot/sub2@all &&\n> +       test_when_finished cleanup_git &&\n> +       (\n> +               cd \"$git\"\n\nMissing &&\n\n> +               git ls-files >lines &&\n> +               test_line_count = 4 lines &&\n> +               git log --oneline p4/master >lines &&\n> +               test_line_count = 5 lines\n> +       )\n> +'\n> +\n>  revision_ranges=\"2000/01/01,#head \\\n>                  1,2080/01/01 \\\n>                  2000/01/01,2080/01/01 \\\n> @@ -147,7 +167,7 @@ test_expect_success 'clone using non-numeric revision ranges' '\n>                 (\n>                         cd \"$git\" &&\n>                         git ls-files >lines &&\n> -                       test_line_count = 6 lines\n> +                       test_line_count = 8 lines\n>                 )\n>         done\n>  '\n>\n> --\n> https://github.com/git/git/pull/311\n"},{"id":"307947","messageId":"2BE8EE6F-F718-475A-A2BA-483F39F5B9A0@vanburgh.me","threadId":"44712","inReplyTo":"CAE5ih7-+tahL4=OrW6F6UPKKRg1KFkw32e=pnTx6j2WTZ-BhOw@mail.gmail.com","subject":"Re: [PATCH] git-p4: Fix multi-path changelist empty commits","fromName":"George Vanburgh","fromEmail":"george@vanburgh.me","sentAt":"2016-12-17T02:25:03Z","receivedAt":"2016-12-17T02:25:18Z","isPatch":true,"sender":{"key":"george@vanburgh.me","avatar":"https://gravatar.com/avatar/575b39450d443981d8b1d1ca4b380c5af51b54adb6dbe3f737cb495e15b2b748?d=mp&s=160"},"body":"\n\nOn 17 December 2016 01:47:55 CET, Luke Diamand <luke@diamand.org> wrote:\n>On 15 December 2016 at 17:14, George Vanburgh <george@vanburgh.me>\n>wrote:\n>> From: George Vanburgh <gvanburgh@bloomberg.net>\n>>\n>> When importing from multiple perforce paths - we may attempt to\n>import a changelist that contains files from two (or more) of these\n>depot paths. Currently, this results in multiple git commits - one\n>containing the changes, and the other(s) as empty commits. This\n>behavior was introduced in commit 1f90a64 (\"git-p4: reduce number of\n>server queries for fetches\", 2015-12-19).\n>\n>That's definitely a bug, thanks for spotting that! Even more so for\n>adding a test case.\n\nNot a problem - thanks to you guys for maintaining such an awesome tool!\n\n>\n>>\n>> Reproduction Steps:\n>>\n>> 1. Have a git repo cloned from a perforce repo using multiple depot\n>paths (e.g. //depot/foo and //depot/bar).\n>> 2. Submit a single change to the perforce repo that makes changes in\n>both //depot/foo and //depot/bar.\n>> 3. Run \"git p4 sync\" to sync the change from #2.\n>>\n>> Change is synced as multiple commits, one for each depot path that\n>was affected.\n>>\n>> Using a set, instead of a list inside p4ChangesForPaths() ensures\n>that each changelist is unique to the returned list, and therefore only\n>a single commit is generated for each changelist.\n>\n>The change looks good to me apart from one missing \"&&\" in the test\n>case (see below).\n\nOops - I'll correct that and resubmit :)\n\n>Possibly need to rewrap the comment line (I think there's a 72\n>character limit) ?\n\nSure - I'll fix that in the resubmission\n\n>\n>Luke\n>\n>\n>>\n>> Reported-by: James Farwell <jfarwell@vmware.com>\n>> Signed-off-by: George Vanburgh <gvanburgh@bloomberg.net>\n>> ---\n>>  git-p4.py               |  4 ++--\n>>  t/t9800-git-p4-basic.sh | 22 +++++++++++++++++++++-\n>>  2 files changed, 23 insertions(+), 3 deletions(-)\n>>\n>> diff --git a/git-p4.py b/git-p4.py\n>> index fd5ca52..6307bc8 100755\n>> --- a/git-p4.py\n>> +++ b/git-p4.py\n>> @@ -822,7 +822,7 @@ def p4ChangesForPaths(depotPaths, changeRange,\n>requestedBlockSize):\n>>                  die(\"cannot use --changes-block-size with\n>non-numeric revisions\")\n>>              block_size = None\n>>\n>> -    changes = []\n>> +    changes = set()\n>>\n>>      # Retrieve changes a block at a time, to prevent running\n>>      # into a MaxResults/MaxScanRows error from the server.\n>> @@ -841,7 +841,7 @@ def p4ChangesForPaths(depotPaths, changeRange,\n>requestedBlockSize):\n>>\n>>          # Insert changes in chronological order\n>>          for line in reversed(p4_read_pipe_lines(cmd)):\n>> -            changes.append(int(line.split(\" \")[1]))\n>> +            changes.add(int(line.split(\" \")[1]))\n>>\n>>          if not block_size:\n>>              break\n>> diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh\n>> index 0730f18..4d72e0b 100755\n>> --- a/t/t9800-git-p4-basic.sh\n>> +++ b/t/t9800-git-p4-basic.sh\n>> @@ -131,6 +131,26 @@ test_expect_success 'clone two dirs, @all,\n>conflicting files' '\n>>         )\n>>  '\n>>\n>> +test_expect_success 'clone two dirs, each edited by submit, single\n>git commit' '\n>> +       (\n>> +               cd \"$cli\" &&\n>> +               echo sub1/f4 >sub1/f4 &&\n>> +               p4 add sub1/f4 &&\n>> +               echo sub2/f4 >sub2/f4 &&\n>> +               p4 add sub2/f4 &&\n>> +               p4 submit -d \"sub1/f4 and sub2/f4\"\n>> +       ) &&\n>> +       git p4 clone --dest=\"$git\" //depot/sub1@all //depot/sub2@all\n>&&\n>> +       test_when_finished cleanup_git &&\n>> +       (\n>> +               cd \"$git\"\n>\n>Missing &&\n>\n>> +               git ls-files >lines &&\n>> +               test_line_count = 4 lines &&\n>> +               git log --oneline p4/master >lines &&\n>> +               test_line_count = 5 lines\n>> +       )\n>> +'\n>> +\n>>  revision_ranges=\"2000/01/01,#head \\\n>>                  1,2080/01/01 \\\n>>                  2000/01/01,2080/01/01 \\\n>> @@ -147,7 +167,7 @@ test_expect_success 'clone using non-numeric\n>revision ranges' '\n>>                 (\n>>                         cd \"$git\" &&\n>>                         git ls-files >lines &&\n>> -                       test_line_count = 6 lines\n>> +                       test_line_count = 8 lines\n>>                 )\n>>         done\n>>  '\n>>\n>> --\n>> https://github.com/git/git/pull/311\n\n"},{"id":"307987","messageId":"010201590ed6ecaa-740c2532-827e-4f5a-af46-0f58d0722db6-000000@eu-west-1.amazonses.com","threadId":"44712","inReplyTo":"01020159037a8995-2d1da9d4-4a27-4b98-818b-432fc0ad8a52-000000@eu-west-1.amazonses.com","subject":"[PATCH v2] git-p4: Fix multi-path changelist empty commits","fromName":"George Vanburgh","fromEmail":"george@vanburgh.me","sentAt":"2016-12-17T22:11:23Z","receivedAt":"2016-12-18T00:07:44Z","isPatch":true,"sender":{"key":"george@vanburgh.me","avatar":"https://gravatar.com/avatar/575b39450d443981d8b1d1ca4b380c5af51b54adb6dbe3f737cb495e15b2b748?d=mp&s=160"},"body":"From: George Vanburgh <gvanburgh@bloomberg.net>\n\nWhen importing from multiple perforce paths - we may attempt to import\na changelist that contains files from two (or more) of these depot\npaths. Currently, this results in multiple git commits - one\ncontaining the changes, and the other(s) as empty commit(s). This\nbehavior was introduced in commit 1f90a64\n(\"git-p4: reduce number of server queries for fetches\", 2015-12-19).\n\nReproduction Steps:\n\n1. Have a git repo cloned from a perforce repo using multiple depot\npaths (e.g. //depot/foo and //depot/bar).\n2. Submit a single change to the perforce repo that makes changes in\nboth //depot/foo and //depot/bar.\n3. Run \"git p4 sync\" to sync the change from #2.\n\nChange is synced as multiple commits, one for each depot path that was\naffected.\n\nUsing a set, instead of a list inside p4ChangesForPaths() ensures that\neach changelist is unique to the returned list, and therefore only a\nsingle commit is generated for each changelist.\n\nReported-by: James Farwell <jfarwell@vmware.com>\nSigned-off-by: George Vanburgh <gvanburgh@bloomberg.net>\n---\n git-p4.py               |  4 ++--\n t/t9800-git-p4-basic.sh | 22 +++++++++++++++++++++-\n 2 files changed, 23 insertions(+), 3 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex fd5ca52..6307bc8 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -822,7 +822,7 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):\n                 die(\"cannot use --changes-block-size with non-numeric revisions\")\n             block_size = None\n \n-    changes = []\n+    changes = set()\n \n     # Retrieve changes a block at a time, to prevent running\n     # into a MaxResults/MaxScanRows error from the server.\n@@ -841,7 +841,7 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):\n \n         # Insert changes in chronological order\n         for line in reversed(p4_read_pipe_lines(cmd)):\n-            changes.append(int(line.split(\" \")[1]))\n+            changes.add(int(line.split(\" \")[1]))\n \n         if not block_size:\n             break\ndiff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh\nindex 0730f18..4d93522 100755\n--- a/t/t9800-git-p4-basic.sh\n+++ b/t/t9800-git-p4-basic.sh\n@@ -131,6 +131,26 @@ test_expect_success 'clone two dirs, @all, conflicting files' '\n \t)\n '\n \n+test_expect_success 'clone two dirs, each edited by submit, single git commit' '\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\techo sub1/f4 >sub1/f4 &&\n+\t\tp4 add sub1/f4 &&\n+\t\techo sub2/f4 >sub2/f4 &&\n+\t\tp4 add sub2/f4 &&\n+\t\tp4 submit -d \"sub1/f4 and sub2/f4\"\n+\t) &&\n+\tgit p4 clone --dest=\"$git\" //depot/sub1@all //depot/sub2@all &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit ls-files >lines &&\n+\t\ttest_line_count = 4 lines &&\n+\t\tgit log --oneline p4/master >lines &&\n+\t\ttest_line_count = 5 lines\n+\t)\n+'\n+\n revision_ranges=\"2000/01/01,#head \\\n \t\t 1,2080/01/01 \\\n \t\t 2000/01/01,2080/01/01 \\\n@@ -147,7 +167,7 @@ test_expect_success 'clone using non-numeric revision ranges' '\n \t\t(\n \t\t\tcd \"$git\" &&\n \t\t\tgit ls-files >lines &&\n-\t\t\ttest_line_count = 6 lines\n+\t\t\ttest_line_count = 8 lines\n \t\t)\n \tdone\n '\n\n--\nhttps://github.com/git/git/pull/311\n"},{"id":"308067","messageId":"xmqqy3zbq05o.fsf@gitster.mtv.corp.google.com","threadId":"44712","inReplyTo":"010201590ed6ecaa-740c2532-827e-4f5a-af46-0f58d0722db6-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH v2] git-p4: Fix multi-path changelist empty commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-19T17:49:39Z","receivedAt":"2016-12-19T17:50:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"George Vanburgh <george@vanburgh.me> writes:\n\n> From: George Vanburgh <gvanburgh@bloomberg.net>\n>\n> When importing from multiple perforce paths - we may attempt to import\n> a changelist that contains files from two (or more) of these depot\n> paths. Currently, this results in multiple git commits - one\n> containing the changes, and the other(s) as empty commit(s). This\n> behavior was introduced in commit 1f90a64\n> (\"git-p4: reduce number of server queries for fetches\", 2015-12-19).\n>\n> Reproduction Steps:\n>\n> 1. Have a git repo cloned from a perforce repo using multiple depot\n> paths (e.g. //depot/foo and //depot/bar).\n> 2. Submit a single change to the perforce repo that makes changes in\n> both //depot/foo and //depot/bar.\n> 3. Run \"git p4 sync\" to sync the change from #2.\n>\n> Change is synced as multiple commits, one for each depot path that was\n> affected.\n>\n> Using a set, instead of a list inside p4ChangesForPaths() ensures that\n> each changelist is unique to the returned list, and therefore only a\n> single commit is generated for each changelist.\n>\n> Reported-by: James Farwell <jfarwell@vmware.com>\n> Signed-off-by: George Vanburgh <gvanburgh@bloomberg.net>\n> ---\n\nThanks, George.  Luke, can I add your \"Reviewed-by:\" here?\n\n>  git-p4.py               |  4 ++--\n>  t/t9800-git-p4-basic.sh | 22 +++++++++++++++++++++-\n>  2 files changed, 23 insertions(+), 3 deletions(-)\n>\n> diff --git a/git-p4.py b/git-p4.py\n> index fd5ca52..6307bc8 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -822,7 +822,7 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):\n>                  die(\"cannot use --changes-block-size with non-numeric revisions\")\n>              block_size = None\n>  \n> -    changes = []\n> +    changes = set()\n>  \n>      # Retrieve changes a block at a time, to prevent running\n>      # into a MaxResults/MaxScanRows error from the server.\n> @@ -841,7 +841,7 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):\n>  \n>          # Insert changes in chronological order\n>          for line in reversed(p4_read_pipe_lines(cmd)):\n> -            changes.append(int(line.split(\" \")[1]))\n> +            changes.add(int(line.split(\" \")[1]))\n>  \n>          if not block_size:\n>              break\n> diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh\n> index 0730f18..4d93522 100755\n> --- a/t/t9800-git-p4-basic.sh\n> +++ b/t/t9800-git-p4-basic.sh\n> @@ -131,6 +131,26 @@ test_expect_success 'clone two dirs, @all, conflicting files' '\n>  \t)\n>  '\n>  \n> +test_expect_success 'clone two dirs, each edited by submit, single git commit' '\n> +\t(\n> +\t\tcd \"$cli\" &&\n> +\t\techo sub1/f4 >sub1/f4 &&\n> +\t\tp4 add sub1/f4 &&\n> +\t\techo sub2/f4 >sub2/f4 &&\n> +\t\tp4 add sub2/f4 &&\n> +\t\tp4 submit -d \"sub1/f4 and sub2/f4\"\n> +\t) &&\n> +\tgit p4 clone --dest=\"$git\" //depot/sub1@all //depot/sub2@all &&\n> +\ttest_when_finished cleanup_git &&\n> +\t(\n> +\t\tcd \"$git\" &&\n> +\t\tgit ls-files >lines &&\n> +\t\ttest_line_count = 4 lines &&\n> +\t\tgit log --oneline p4/master >lines &&\n> +\t\ttest_line_count = 5 lines\n> +\t)\n> +'\n> +\n>  revision_ranges=\"2000/01/01,#head \\\n>  \t\t 1,2080/01/01 \\\n>  \t\t 2000/01/01,2080/01/01 \\\n> @@ -147,7 +167,7 @@ test_expect_success 'clone using non-numeric revision ranges' '\n>  \t\t(\n>  \t\t\tcd \"$git\" &&\n>  \t\t\tgit ls-files >lines &&\n> -\t\t\ttest_line_count = 6 lines\n> +\t\t\ttest_line_count = 8 lines\n>  \t\t)\n>  \tdone\n>  '\n>\n> --\n> https://github.com/git/git/pull/311\n"},{"id":"308068","messageId":"CAE5ih7_KKLiifY5K6k2u-VJcJvizhgXRLKz5+DLahoudwXhELg@mail.gmail.com","threadId":"44712","inReplyTo":"xmqqy3zbq05o.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] git-p4: Fix multi-path changelist empty commits","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2016-12-19T17:56:24Z","receivedAt":"2016-12-19T17:57:23Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 19 December 2016 at 17:49, Junio C Hamano <gitster@pobox.com> wrote:\n> George Vanburgh <george@vanburgh.me> writes:\n>\n>> From: George Vanburgh <gvanburgh@bloomberg.net>\n>>\n>> When importing from multiple perforce paths - we may attempt to import\n>> a changelist that contains files from two (or more) of these depot\n>> paths. Currently, this results in multiple git commits - one\n>> containing the changes, and the other(s) as empty commit(s). This\n>> behavior was introduced in commit 1f90a64\n>> (\"git-p4: reduce number of server queries for fetches\", 2015-12-19).\n>>\n>> Reproduction Steps:\n>>\n>> 1. Have a git repo cloned from a perforce repo using multiple depot\n>> paths (e.g. //depot/foo and //depot/bar).\n>> 2. Submit a single change to the perforce repo that makes changes in\n>> both //depot/foo and //depot/bar.\n>> 3. Run \"git p4 sync\" to sync the change from #2.\n>>\n>> Change is synced as multiple commits, one for each depot path that was\n>> affected.\n>>\n>> Using a set, instead of a list inside p4ChangesForPaths() ensures that\n>> each changelist is unique to the returned list, and therefore only a\n>> single commit is generated for each changelist.\n>>\n>> Reported-by: James Farwell <jfarwell@vmware.com>\n>> Signed-off-by: George Vanburgh <gvanburgh@bloomberg.net>\n>> ---\n>\n> Thanks, George.  Luke, can I add your \"Reviewed-by:\" here?\n\nYes, thanks.\n\nLuke\n"}]}