{"thread":{"id":"40940","subject":"[PATCH 1/2] git-p4: support multiple depot paths in p4 submit","startedAt":"2015-12-05T11:22:03Z","lastAt":"2015-12-08T12:32:55Z","messageCount":6,"participants":["Sam Hocevar","Lars Schneider","Luke Diamand"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"274053","messageId":"20151205112203.GA15745@hocevar.net","threadId":"40940","inReplyTo":null,"subject":"[PATCH 1/2] git-p4: support multiple depot paths in p4 submit","fromName":"Sam Hocevar","fromEmail":"sam@hocevar.net","sentAt":"2015-12-05T11:22:03Z","receivedAt":"2015-12-05T11:22:03Z","isPatch":true,"sender":{"key":"sam@hocevar.net","avatar":"https://avatars.githubusercontent.com/u/245089?v=4"},"body":"When submitting from a repository that was cloned using a client spec,\nuse the full list of paths when ruling out files that are outside the\nview. This fixes a bug where only files pertaining to the first path\nwould be included in the p4 submit.\n\nSigned-off-by: Sam Hocevar \n---\n git-p4.py | 11 +++++++++--\n 1 file changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex a79b6d8..210f100 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1253,6 +1253,8 @@ class P4Submit(Command, P4UserMap):\n Remove lines in the Files section that show changes to files\n outside the depot path we're committing into.\"\"\"\n \n+ [upstream, settings] = findUpstreamBranchPoint()\n+\n template = \"\"\n inFilesSection = False\n for line in p4_read_pipe_lines(['change', '-o']):\n@@ -1265,8 +1267,13 @@ class P4Submit(Command, P4UserMap):\n lastTab = path.rfind(\"\\t\")\n if lastTab != -1:\n path = path[:lastTab]\n- if not p4PathStartsWith(path, self.depotPath):\n- continue\n+ if settings.has_key('depot-paths'):\n+ if not [p for p in settings['depot-paths']\n+ if p4PathStartsWith(path, p)]:\n+ continue\n+ else:\n+ if not p4PathStartsWith(path, self.depotPath):\n+ continue\n else:\n inFilesSection = False\n else:\n-- \n2.6.2\n"},{"id":"274067","messageId":"F328D5D9-754A-41CC-A7B2-993B9315ED33@gmail.com","threadId":"40940","inReplyTo":"20151205112203.GA15745@hocevar.net","subject":"Re: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-12-06T11:56:35Z","receivedAt":"2015-12-06T11:56:35Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"Thanks for the patch! Do you see a way to demonstrate the bug in a test case similar to t9821 [1]?\n\nCheers,\nLars\n\n[1] https://github.com/git/git/blob/master/t/t9821-git-p4-path-variations.sh\n\n> On 05 Dec 2015, at 12:22, Sam Hocevar wrote:\n> \n> When submitting from a repository that was cloned using a client spec,\n> use the full list of paths when ruling out files that are outside the\n> view. This fixes a bug where only files pertaining to the first path\n> would be included in the p4 submit.\n> \n> Signed-off-by: Sam Hocevar \n> ---\n> git-p4.py | 11 +++++++++--\n> 1 file changed, 9 insertions(+), 2 deletions(-)\n> \n> diff --git a/git-p4.py b/git-p4.py\n> index a79b6d8..210f100 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -1253,6 +1253,8 @@ class P4Submit(Command, P4UserMap):\n> Remove lines in the Files section that show changes to files\n> outside the depot path we're committing into.\"\"\"\n> \n> + [upstream, settings] = findUpstreamBranchPoint()\n> +\n> template = \"\"\n> inFilesSection = False\n> for line in p4_read_pipe_lines(['change', '-o']):\n> @@ -1265,8 +1267,13 @@ class P4Submit(Command, P4UserMap):\n> lastTab = path.rfind(\"\\t\")\n> if lastTab != -1:\n> path = path[:lastTab]\n> - if not p4PathStartsWith(path, self.depotPath):\n> - continue\n> + if settings.has_key('depot-paths'):\n> + if not [p for p in settings['depot-paths']\n> + if p4PathStartsWith(path, p)]:\n> + continue\n> + else:\n> + if not p4PathStartsWith(path, self.depotPath):\n> + continue\n> else:\n> inFilesSection = False\n> else:\n> -- \n> 2.6.2\n"},{"id":"274120","messageId":"20151207185129.GA48528@hocevar.net","threadId":"40940","inReplyTo":"F328D5D9-754A-41CC-A7B2-993B9315ED33@gmail.com","subject":"Re: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit","fromName":"Sam Hocevar","fromEmail":"sam@hocevar.net","sentAt":"2015-12-07T18:51:29Z","receivedAt":"2015-12-07T18:51:29Z","isPatch":true,"sender":{"key":"sam@hocevar.net","avatar":"https://avatars.githubusercontent.com/u/245089?v=4"},"body":"On Sun, Dec 06, 2015, Lars Schneider wrote:\n> Thanks for the patch! Do you see a way to demonstrate the bug in a test case similar to t9821 [1]?\n\n Not yet, I'm afraid. It's proving trickier than expected because for\nnow I can only reproduce the bug when the view uses multiples depots\n(solution #2 on http://answers.perforce.com/articles/KB/2437), and\nunfortunately the test case system in Git was designed for a single\ndepot.\n\n Would a refactor of lib-git-p4.sh (and probably all git-p4 tests) to\nsupport multiple depots be acceptable and/or welcome? I prefer to ask\nbefore I dig into the task.\n\nRegards,\n-- \nSam.\n"},{"id":"274170","messageId":"14B51656-26D1-4805-9F07-102CBD81B387@gmail.com","threadId":"40940","inReplyTo":"20151207185129.GA48528@hocevar.net","subject":"Re: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-12-08T09:48:21Z","receivedAt":"2015-12-08T09:48:21Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 07 Dec 2015, at 19:51, Sam Hocevar wrote:\n\n> On Sun, Dec 06, 2015, Lars Schneider wrote:\n>> Thanks for the patch! Do you see a way to demonstrate the bug in a test case similar to t9821 [1]?\n> \n> Not yet, I'm afraid. It's proving trickier than expected because for\n> now I can only reproduce the bug when the view uses multiples depots\n> (solution #2 on http://answers.perforce.com/articles/KB/2437), and\n> unfortunately the test case system in Git was designed for a single\n> depot.\n> \n> Would a refactor of lib-git-p4.sh (and probably all git-p4 tests) to\n> support multiple depots be acceptable and/or welcome? I prefer to ask\n> before I dig into the task.\n\nCan you outline your idea a bit? Are you aware of the following way to define client specs: [1] ? Would that help?\nI haven't used multiple depots, yet. Therefore please bare with me :-)\n\nThanks,\nLars\n\n[1] https://github.com/git/git/blob/362d2fc2f8ab9ee22072f76fb36ec16918511944/t/t9821-git-p4-path-variations.sh#L109-L111\n"},{"id":"274172","messageId":"20151208114106.GB48528@hocevar.net","threadId":"40940","inReplyTo":"14B51656-26D1-4805-9F07-102CBD81B387@gmail.com","subject":"Re: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit","fromName":"Sam Hocevar","fromEmail":"sam@hocevar.net","sentAt":"2015-12-08T11:41:06Z","receivedAt":"2015-12-08T11:41:06Z","isPatch":true,"sender":{"key":"sam@hocevar.net","avatar":"https://avatars.githubusercontent.com/u/245089?v=4"},"body":"On Tue, Dec 08, 2015, Lars Schneider wrote:\n\n> > Would a refactor of lib-git-p4.sh (and probably all git-p4 tests) to\n> > support multiple depots be acceptable and/or welcome? I prefer to ask\n> > before I dig into the task.\n> \n> Can you outline your idea a bit? Are you aware of the following way to define client specs: [1] ? Would that help?\n\n That's the idea, but the bug occurs when the client view looks like this:\n\n //depot/... //client/dir1/...\n //depot2/... //client/dir2/...\n\n And is then cloned with (it is not legal in Perforce to specify //...\ndirectly to grab both depots at once):\n\n git p4 clone --use-client-spec //depot/... //depot2/...\n\n Then when a file is modified in dir2/, git p4 submit does not elect it\nfor the changelist. A file in dir1/ will work fine.\n\n Unfortunately the current test suite assumes everything is under\n//depot/ so in order to write a test for this situation there are a few\nthings to change in lib-git-p4.sh.\n\nRegards,\n-- \nSam.\n"},{"id":"274173","messageId":"CAE5ih78K5zzJBK3y-MMf2tWBoPOtkPJzYRZWxH02qBE=OnUhVA@mail.gmail.com","threadId":"40940","inReplyTo":"20151208114106.GB48528@hocevar.net","subject":"Re: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-12-08T12:32:55Z","receivedAt":"2015-12-08T12:32:55Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 8 December 2015 at 11:41, Sam Hocevar wrote:\n> On Tue, Dec 08, 2015, Lars Schneider wrote:\n>\n>> > Would a refactor of lib-git-p4.sh (and probably all git-p4 tests) to\n>> > support multiple depots be acceptable and/or welcome? I prefer to ask\n>> > before I dig into the task.\n>>\n>> Can you outline your idea a bit? Are you aware of the following way to define client specs: [1] ? Would that help?\n>\n> That's the idea, but the bug occurs when the client view looks like this:\n>\n> //depot/... //client/dir1/...\n> //depot2/... //client/dir2/...\n>\n> And is then cloned with (it is not legal in Perforce to specify //...\n> directly to grab both depots at once):\n>\n> git p4 clone --use-client-spec //depot/... //depot2/...\n>\n> Then when a file is modified in dir2/, git p4 submit does not elect it\n> for the changelist. A file in dir1/ will work fine.\n>\n> Unfortunately the current test suite assumes everything is under\n> //depot/ so in order to write a test for this situation there are a few\n> things to change in lib-git-p4.sh.\n\nI think the existing structure ought to mostly work, but it might need\na bit of tweaking.\n\nYou would need to create a new depot, but you can do that in your test script.\n\nAnd you would need a client spec that pointed at this depot, but again\nyou can do that in your script with the client_view shell function.\n\nI've not tried it myself though, so maybe it's harder than that.\n\nLuke\n\n\n>\n> Regards,\n> --\n> Sam.\n"}]}