* [PATCH 1/2] git-p4: support multiple depot paths in p4 submit
@ 2015-12-05 11:22 Sam Hocevar
2015-12-06 11:56 ` Lars Schneider
0 siblings, 1 reply; 6+ messages in thread
From: Sam Hocevar @ 2015-12-05 11:22 UTC (permalink / raw)
To: Git Users; +Cc: Luke Diamand, Pete Wyckoff, Lars Schneider
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 <sam@hocevar.net>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit
2015-12-05 11:22 [PATCH 1/2] git-p4: support multiple depot paths in p4 submit Sam Hocevar
@ 2015-12-06 11:56 ` Lars Schneider
2015-12-07 18:51 ` Sam Hocevar
0 siblings, 1 reply; 6+ messages in thread
From: Lars Schneider @ 2015-12-06 11:56 UTC (permalink / raw)
To: Sam Hocevar; +Cc: Git Users, Luke Diamand, Pete Wyckoff
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 <sam@hocevar.net> 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 <sam@hocevar.net>
> ---
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit
2015-12-06 11:56 ` Lars Schneider
@ 2015-12-07 18:51 ` Sam Hocevar
2015-12-08 9:48 ` Lars Schneider
0 siblings, 1 reply; 6+ messages in thread
From: Sam Hocevar @ 2015-12-07 18:51 UTC (permalink / raw)
To: Lars Schneider; +Cc: Git Users, Luke Diamand, Pete Wyckoff
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit
2015-12-07 18:51 ` Sam Hocevar
@ 2015-12-08 9:48 ` Lars Schneider
2015-12-08 11:41 ` Sam Hocevar
0 siblings, 1 reply; 6+ messages in thread
From: Lars Schneider @ 2015-12-08 9:48 UTC (permalink / raw)
To: Sam Hocevar; +Cc: Git Users, Luke Diamand, Pete Wyckoff
On 07 Dec 2015, at 19:51, Sam Hocevar <sam@hocevar.net> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit
2015-12-08 9:48 ` Lars Schneider
@ 2015-12-08 11:41 ` Sam Hocevar
2015-12-08 12:32 ` Luke Diamand
0 siblings, 1 reply; 6+ messages in thread
From: Sam Hocevar @ 2015-12-08 11:41 UTC (permalink / raw)
To: Lars Schneider; +Cc: Git Users, Luke Diamand, Pete Wyckoff
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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit
2015-12-08 11:41 ` Sam Hocevar
@ 2015-12-08 12:32 ` Luke Diamand
0 siblings, 0 replies; 6+ messages in thread
From: Luke Diamand @ 2015-12-08 12:32 UTC (permalink / raw)
To: Sam Hocevar; +Cc: Lars Schneider, Git Users, Pete Wyckoff
On 8 December 2015 at 11:41, Sam Hocevar <sam@hocevar.net> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-12-08 12:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-05 11:22 [PATCH 1/2] git-p4: support multiple depot paths in p4 submit Sam Hocevar
2015-12-06 11:56 ` Lars Schneider
2015-12-07 18:51 ` Sam Hocevar
2015-12-08 9:48 ` Lars Schneider
2015-12-08 11:41 ` Sam Hocevar
2015-12-08 12:32 ` Luke Diamand
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).