# [PATCH 1/2] git-p4: support multiple depot paths in p4 submit 6 messages from 2015-12-05 to 2015-12-08. Participants: Sam Hocevar, Lars Schneider, Luke Diamand. Thread: https://gitlist.dev/t/40940 ## Sam Hocevar, 2015-12-05 11:22 Subject: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit Message-ID: <20151205112203.GA15745@hocevar.net> URL: https://gitlist.dev/e/20151205112203.GA15745%40hocevar.net ``` When submitting from a repository that was cloned using a client spec, use the full list of paths when ruling out files that are outside the view. This fixes a bug where only files pertaining to the first path would be included in the p4 submit. Signed-off-by: Sam Hocevar --- git-p4.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/git-p4.py b/git-p4.py index a79b6d8..210f100 100755 --- a/git-p4.py +++ b/git-p4.py @@ -1253,6 +1253,8 @@ class P4Submit(Command, P4UserMap): Remove lines in the Files section that show changes to files outside the depot path we're committing into.""" + [upstream, settings] = findUpstreamBranchPoint() + template = "" inFilesSection = False for line in p4_read_pipe_lines(['change', '-o']): @@ -1265,8 +1267,13 @@ class P4Submit(Command, P4UserMap): lastTab = path.rfind("\t") if lastTab != -1: path = path[:lastTab] - if not p4PathStartsWith(path, self.depotPath): - continue + if settings.has_key('depot-paths'): + if not [p for p in settings['depot-paths'] + if p4PathStartsWith(path, p)]: + continue + else: + if not p4PathStartsWith(path, self.depotPath): + continue else: inFilesSection = False else: -- 2.6.2 ``` ## Lars Schneider, 2015-12-06 11:56 Subject: Re: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit Message-ID: URL: https://gitlist.dev/e/F328D5D9-754A-41CC-A7B2-993B9315ED33%40gmail.com In-Reply-To: <20151205112203.GA15745@hocevar.net> ``` Thanks for the patch! Do you see a way to demonstrate the bug in a test case similar to t9821 [1]? Cheers, Lars [1] https://github.com/git/git/blob/master/t/t9821-git-p4-path-variations.sh > On 05 Dec 2015, at 12:22, Sam Hocevar wrote: > > When submitting from a repository that was cloned using a client spec, > use the full list of paths when ruling out files that are outside the > view. This fixes a bug where only files pertaining to the first path > would be included in the p4 submit. > > Signed-off-by: Sam Hocevar > --- > git-p4.py | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/git-p4.py b/git-p4.py > index a79b6d8..210f100 100755 > --- a/git-p4.py > +++ b/git-p4.py > @@ -1253,6 +1253,8 @@ class P4Submit(Command, P4UserMap): > Remove lines in the Files section that show changes to files > outside the depot path we're committing into.""" > > + [upstream, settings] = findUpstreamBranchPoint() > + > template = "" > inFilesSection = False > for line in p4_read_pipe_lines(['change', '-o']): > @@ -1265,8 +1267,13 @@ class P4Submit(Command, P4UserMap): > lastTab = path.rfind("\t") > if lastTab != -1: > path = path[:lastTab] > - if not p4PathStartsWith(path, self.depotPath): > - continue > + if settings.has_key('depot-paths'): > + if not [p for p in settings['depot-paths'] > + if p4PathStartsWith(path, p)]: > + continue > + else: > + if not p4PathStartsWith(path, self.depotPath): > + continue > else: > inFilesSection = False > else: > -- > 2.6.2 ``` ## Sam Hocevar, 2015-12-07 18:51 Subject: Re: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit Message-ID: <20151207185129.GA48528@hocevar.net> URL: https://gitlist.dev/e/20151207185129.GA48528%40hocevar.net In-Reply-To: ``` On Sun, Dec 06, 2015, Lars Schneider wrote: > Thanks for the patch! Do you see a way to demonstrate the bug in a test case similar to t9821 [1]? Not yet, I'm afraid. It's proving trickier than expected because for now I can only reproduce the bug when the view uses multiples depots (solution #2 on http://answers.perforce.com/articles/KB/2437), and unfortunately the test case system in Git was designed for a single depot. Would a refactor of lib-git-p4.sh (and probably all git-p4 tests) to support multiple depots be acceptable and/or welcome? I prefer to ask before I dig into the task. Regards, -- Sam. ``` ## Lars Schneider, 2015-12-08 09:48 Subject: Re: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit Message-ID: <14B51656-26D1-4805-9F07-102CBD81B387@gmail.com> URL: https://gitlist.dev/e/14B51656-26D1-4805-9F07-102CBD81B387%40gmail.com In-Reply-To: <20151207185129.GA48528@hocevar.net> ``` On 07 Dec 2015, at 19:51, Sam Hocevar wrote: > On Sun, Dec 06, 2015, Lars Schneider wrote: >> Thanks for the patch! Do you see a way to demonstrate the bug in a test case similar to t9821 [1]? > > Not yet, I'm afraid. It's proving trickier than expected because for > now I can only reproduce the bug when the view uses multiples depots > (solution #2 on http://answers.perforce.com/articles/KB/2437), and > unfortunately the test case system in Git was designed for a single > depot. > > Would a refactor of lib-git-p4.sh (and probably all git-p4 tests) to > support multiple depots be acceptable and/or welcome? I prefer to ask > before I dig into the task. Can you outline your idea a bit? Are you aware of the following way to define client specs: [1] ? Would that help? I haven't used multiple depots, yet. Therefore please bare with me :-) Thanks, Lars [1] https://github.com/git/git/blob/362d2fc2f8ab9ee22072f76fb36ec16918511944/t/t9821-git-p4-path-variations.sh#L109-L111 ``` ## Sam Hocevar, 2015-12-08 11:41 Subject: Re: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit Message-ID: <20151208114106.GB48528@hocevar.net> URL: https://gitlist.dev/e/20151208114106.GB48528%40hocevar.net In-Reply-To: <14B51656-26D1-4805-9F07-102CBD81B387@gmail.com> ``` On Tue, Dec 08, 2015, Lars Schneider wrote: > > Would a refactor of lib-git-p4.sh (and probably all git-p4 tests) to > > support multiple depots be acceptable and/or welcome? I prefer to ask > > before I dig into the task. > > Can you outline your idea a bit? Are you aware of the following way to define client specs: [1] ? Would that help? That's the idea, but the bug occurs when the client view looks like this: //depot/... //client/dir1/... //depot2/... //client/dir2/... And is then cloned with (it is not legal in Perforce to specify //... directly to grab both depots at once): git p4 clone --use-client-spec //depot/... //depot2/... Then when a file is modified in dir2/, git p4 submit does not elect it for the changelist. A file in dir1/ will work fine. Unfortunately the current test suite assumes everything is under //depot/ so in order to write a test for this situation there are a few things to change in lib-git-p4.sh. Regards, -- Sam. ``` ## Luke Diamand, 2015-12-08 12:32 Subject: Re: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit Message-ID: URL: https://gitlist.dev/e/CAE5ih78K5zzJBK3y-MMf2tWBoPOtkPJzYRZWxH02qBE%3DOnUhVA%40mail.gmail.com In-Reply-To: <20151208114106.GB48528@hocevar.net> ``` On 8 December 2015 at 11:41, Sam Hocevar wrote: > On Tue, Dec 08, 2015, Lars Schneider wrote: > >> > Would a refactor of lib-git-p4.sh (and probably all git-p4 tests) to >> > support multiple depots be acceptable and/or welcome? I prefer to ask >> > before I dig into the task. >> >> Can you outline your idea a bit? Are you aware of the following way to define client specs: [1] ? Would that help? > > That's the idea, but the bug occurs when the client view looks like this: > > //depot/... //client/dir1/... > //depot2/... //client/dir2/... > > And is then cloned with (it is not legal in Perforce to specify //... > directly to grab both depots at once): > > git p4 clone --use-client-spec //depot/... //depot2/... > > Then when a file is modified in dir2/, git p4 submit does not elect it > for the changelist. A file in dir1/ will work fine. > > Unfortunately the current test suite assumes everything is under > //depot/ so in order to write a test for this situation there are a few > things to change in lib-git-p4.sh. I think the existing structure ought to mostly work, but it might need a bit of tweaking. You would need to create a new depot, but you can do that in your test script. And you would need a client spec that pointed at this depot, but again you can do that in your script with the client_view shell function. I've not tried it myself though, so maybe it's harder than that. Luke > > Regards, > -- > Sam. ```