Re: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit

[Lars Schneider]

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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit

[Luke Diamand]

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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit

[Sam Hocevar]

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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit

[Sam Hocevar]

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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit

[Lars Schneider]

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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] git-p4: support multiple depot paths in p4 submit

[Sam Hocevar]

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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html