Re: [PATCH] git-p4: Fix depot path stripping

On 27 February 2018 at 08:43, Nuno Subtil [off-list ref] wrote:

The issue is that passing in --use-client-spec will always cause git-p4 to
preserve the full depot path in the working tree that it creates, even when
--keep-path is not used.

The repro case is fairly simple: cloning a given p4 path that is nested 2 or
more levels below the depot root will have different results with and
without --use-client-spec (even when the client spec is just a
straightforward map of the entire depot).

E.g., 'git p4 clone //p4depot/path/to/some/project/...' will create a
working tree rooted at project. However, 'git p4 clone --use-client-spec
//p4depot/path/to/some/project/...' will create a working tree rooted at the
root of the depot.

I think it _might_ be by design.

At least, the test ./t9809-git-p4-client-view.sh seems to fail for me
with your change, although I haven't investigated what's going on:

$ ./t9809-git-p4-client-view.sh
...
...
Doing initial import of //depot/dir1/ from revision #head into
refs/remotes/p4/master
./t9809-git-p4-client-view.sh: 10: eval: cannot create dir1/file13:
Directory nonexistent
not ok 23 - subdir clone, submit add

I think originally the logic came from this change:

   21ef5df43 git p4: make branch detection work with --use-client-spec

which was fixing what seems like the same problem but with branch
detection enabled.

Thanks,
Nuno


On Tue, Feb 27, 2018 at 12:10 AM, Luke Diamand [off-list ref] wrote:

quoted

On 27 February 2018 at 04:16, Nuno Subtil [off-list ref] wrote:

quoted

When useClientSpec=true, stripping of P4 depot paths doesn't happen
correctly on sync. Modifies stripRepoPath to handle this case better.

Can you give an example of how this shows up? I could quickly write a
test case for this if I knew what was going on.

Thanks
Luke

quoted

Signed-off-by: Nuno Subtil <redacted>
---
 git-p4.py | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 7bb9cadc69738..3df95d0fd1d83 100755
--- a/git-p4.py
+++ b/git-p4.py

@@ -2480,7 +2480,7 @@ def stripRepoPath(self, path, prefixes):
                     if path.startswith(b + "/"):
                         path = path[len(b)+1:]

-        elif self.keepRepoPath:
+        if self.keepRepoPath:
             # Preserve everything in relative path name except leading
             # //depot/; just look at first prefix as they all should
             # be in the same depot.

@@ -2490,6 +2490,12 @@ def stripRepoPath(self, path, prefixes):

         else:
             for p in prefixes:
+               if self.useClientSpec and not self.keepRepoPath:
+                    # when useClientSpec is false, the prefix will

contain the depot name but the path will not
+                    # extract the depot name and add it to the path so
the match below will do the right thing
+                    depot = re.sub("^(//[^/]+/).*", r'\1', p)
+                    path = depot + path
+
                 if p4PathStartsWith(path, p):
                     path = path[len(p):]
                     break

@@ -2526,8 +2532,8 @@ def splitFilesIntoBranches(self, commit):
             # go in a p4 client
             if self.useClientSpec:
                 relPath = self.clientSpecDirs.map_in_client(path)
-            else:
-                relPath = self.stripRepoPath(path, self.depotPaths)
+
+            relPath = self.stripRepoPath(path, self.depotPaths)

             for branch in self.knownBranches.keys():
                 # add a trailing slash so that a commit into qt/4.2foo

--

https://github.com/git/git/pull/463