git-p4 migrates perforce “main” branch into git branches as subdirectories (doubled code in git branches)

7 messages, 2 authors, 2016-06-15 · open the first message on its own page

git-p4 migrates perforce “main” branch into git branches as subdirectories (doubled code in git branches)

From: Matthew Korich <hidden>
Date: 2016-06-15 22:54:27

Using git p4 on git version 1.7.12.rc2 has path issues. Standard
clone/sync ops apparently place detected master and branches on
independent and parallel directory structures instead of git branches.
See http://stackoverflow.com/q/11893688/1588831 for a full demo of the problem.
Thanks,
-Matt Korich

[PATCH 0/5] git p4: fix branch detection with --use-client-spec

From: Pete Wyckoff <hidden>
Date: 2016-06-15 22:54:28

matthew@korich.net wrote on Fri, 10 Aug 2012 12:14 -0700:
Using git p4 on git version 1.7.12.rc2 has path issues. Standard
clone/sync ops apparently place detected master and branches on
independent and parallel directory structures instead of git branches.
See http://stackoverflow.com/q/11893688/1588831 for a full demo of the problem.
Thank you for the detailed report.  It is a bug in 1.7.12-rc2.
This series fixes it, on top of origin/master.

The crux of the matter is that files are mapped into the wrong
locations in git when both --use-client-spec and --branch-detection
are enabled.

Pete Wyckoff (5):
  git p4 test: move client_view() function to library
  git p4 test: add broken --use-client-spec --detect-branches tests
  git p4: set self.branchPrefixes in initialization
  git p4: do wildcard decoding in stripRepoPath
  git p4: make branch detection work with --use-client-spec

 git-p4.py                     | 75 +++++++++++++++++++++++++++--------------
 t/lib-git-p4.sh               | 18 ++++++++++
 t/t9801-git-p4-branch.sh      | 77 +++++++++++++++++++++++++++++++++++++++++++
 t/t9809-git-p4-client-view.sh | 17 ----------
 4 files changed, 146 insertions(+), 41 deletions(-)

-- 
1.7.12.rc2.24.gc304662

[PATCH 1/5] git p4 test: move client_view() function to library

From: Pete Wyckoff <hidden>
Date: 2016-06-15 22:54:28

This code will be useful in --detect-branches --use-client-spec tests.

Signed-off-by: Pete Wyckoff <redacted>
---
 t/lib-git-p4.sh               | 18 ++++++++++++++++++
 t/t9809-git-p4-client-view.sh | 17 -----------------
 2 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 2d753ab..5c952d6 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -115,3 +115,21 @@ marshal_dump() {
 	EOF
 	"$PYTHON_PATH" "$TRASH_DIRECTORY/marshal-dump.py"
 }
+
+#
+# Construct a client with this list of View lines
+#
+client_view() {
+	(
+		cat <<-EOF &&
+		Client: client
+		Description: client
+		Root: $cli
+		View:
+		EOF
+		for arg ; do
+			printf "\t$arg\n"
+		done
+	) | p4 client -i
+}
+
diff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh
index 7d993ef..281be29 100755
--- a/t/t9809-git-p4-client-view.sh
+++ b/t/t9809-git-p4-client-view.sh
@@ -9,23 +9,6 @@ test_expect_success 'start p4d' '
 '
 
 #
-# Construct a client with this list of View lines
-#
-client_view() {
-	(
-		cat <<-EOF &&
-		Client: client
-		Description: client
-		Root: $cli
-		View:
-		EOF
-		for arg ; do
-			printf "\t$arg\n"
-		done
-	) | p4 client -i
-}
-
-#
 # Verify these files exist, exactly.  Caller creates
 # a list of files in file "files".
 #
-- 
1.7.12.rc2.24.gc304662

[PATCH 2/5] git p4 test: add broken --use-client-spec --detect-branches tests

From: Pete Wyckoff <hidden>
Date: 2016-06-15 22:54:28

Signed-off-by: Pete Wyckoff <redacted>
---
 t/t9801-git-p4-branch.sh | 77 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)
diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh
index 99fe16b..ca3a7f9 100755
--- a/t/t9801-git-p4-branch.sh
+++ b/t/t9801-git-p4-branch.sh
@@ -410,6 +410,83 @@ test_expect_failure 'git p4 clone file subset branch' '
 		test_path_is_missing file3
 	)
 '
+
+# From a report in http://stackoverflow.com/questions/11893688
+# where --use-client-spec caused branch prefixes not to be removed;
+# every file in git appeared into a subdirectory of the branch name.
+test_expect_success 'use-client-spec detect-branches setup' '
+	rm -rf "$cli" &&
+	mkdir "$cli" &&
+	(
+		cd "$cli" &&
+		client_view "//depot/usecs/... //client/..." &&
+		mkdir b1 &&
+		echo b1/b1-file1 >b1/b1-file1 &&
+		p4 add b1/b1-file1 &&
+		p4 submit -d "b1/b1-file1" &&
+
+		p4 integrate //depot/usecs/b1/... //depot/usecs/b2/... &&
+		p4 submit -d "b1 -> b2" &&
+		p4 branch -i <<-EOF &&
+		Branch: b2
+		View: //depot/usecs/b1/... //depot/usecs/b2/...
+		EOF
+
+		echo b2/b2-file2 >b2/b2-file2 &&
+		p4 add b2/b2-file2 &&
+		p4 submit -d "b2/b2-file2"
+	)
+'
+
+test_expect_failure 'use-client-spec detect-branches files in top-level' '
+	test_when_finished cleanup_git &&
+	test_create_repo "$git" &&
+	(
+		cd "$git" &&
+		git p4 sync --detect-branches --use-client-spec //depot/usecs@all &&
+		git checkout -b master p4/usecs/b1 &&
+		test_path_is_file b1-file1 &&
+		test_path_is_missing b2-file2 &&
+		test_path_is_missing b1 &&
+		test_path_is_missing b2 &&
+
+		git checkout -b b2 p4/usecs/b2 &&
+		test_path_is_file b1-file1 &&
+		test_path_is_file b2-file2 &&
+		test_path_is_missing b1 &&
+		test_path_is_missing b2
+	)
+'
+
+test_expect_success 'use-client-spec detect-branches skips branches setup' '
+	(
+		cd "$cli" &&
+
+		p4 integrate //depot/usecs/b1/... //depot/usecs/b3/... &&
+		p4 submit -d "b1 -> b3" &&
+		p4 branch -i <<-EOF &&
+		Branch: b3
+		View: //depot/usecs/b1/... //depot/usecs/b3/...
+		EOF
+
+		echo b3/b3-file3 >b3/b3-file3 &&
+		p4 add b3/b3-file3 &&
+		p4 submit -d "b3/b3-file3"
+	)
+'
+
+test_expect_success 'use-client-spec detect-branches skips branches' '
+	client_view "//depot/usecs/... //client/..." \
+	            "-//depot/usecs/b3/... //client/b3/..." &&
+	test_when_finished cleanup_git &&
+	test_create_repo "$git" &&
+	(
+		cd "$git" &&
+		git p4 sync --detect-branches --use-client-spec //depot/usecs@all &&
+		test_must_fail git rev-parse refs/remotes/p4/usecs/b3
+	)
+'
+
 test_expect_success 'kill p4d' '
 	kill_p4d
 '
-- 
1.7.12.rc2.24.gc304662

[PATCH 3/5] git p4: set self.branchPrefixes in initialization

From: Pete Wyckoff <hidden>
Date: 2016-06-15 22:54:28

This instance variable is needed during commit() to map
files from p4 to their relative locations in git.  Set
it when initializing P4Sync to avoid passing it to every
commit() call.

Signed-off-by: Pete Wyckoff <redacted>
---
 git-p4.py | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index e67d37d..6d07115 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2041,10 +2041,9 @@ class P4Sync(Command, P4UserMap):
         gitStream.write(description)
         gitStream.write("\n")
 
-    def commit(self, details, files, branch, branchPrefixes, parent = ""):
+    def commit(self, details, files, branch, parent = ""):
         epoch = details["time"]
         author = details["user"]
-        self.branchPrefixes = branchPrefixes
 
         if self.verbose:
             print "commit into %s" % branch
@@ -2053,7 +2052,7 @@ class P4Sync(Command, P4UserMap):
         # create a commit.
         new_files = []
         for f in files:
-            if [p for p in branchPrefixes if p4PathStartsWith(f['path'], p)]:
+            if [p for p in self.branchPrefixes if p4PathStartsWith(f['path'], p)]:
                 new_files.append (f)
             else:
                 sys.stderr.write("Ignoring file outside of prefix: %s\n" % f['path'])
@@ -2070,8 +2069,8 @@ class P4Sync(Command, P4UserMap):
 
         self.gitStream.write("data <<EOT\n")
         self.gitStream.write(details["desc"])
-        self.gitStream.write("\n[git-p4: depot-paths = \"%s\": change = %s"
-                             % (','.join (branchPrefixes), details["change"]))
+        self.gitStream.write("\n[git-p4: depot-paths = \"%s\": change = %s" %
+                             (','.join(self.branchPrefixes), details["change"]))
         if len(details['options']) > 0:
             self.gitStream.write(": options = %s" % details['options'])
         self.gitStream.write("]\nEOT\n\n")
@@ -2094,7 +2093,7 @@ class P4Sync(Command, P4UserMap):
                 print "Change %s is labelled %s" % (change, labelDetails)
 
             files = p4CmdList(["files"] + ["%s...@%s" % (p, change)
-                                                    for p in branchPrefixes])
+                                                for p in self.branchPrefixes])
 
             if len(files) == len(labelRevisions):
 
@@ -2405,6 +2404,7 @@ class P4Sync(Command, P4UserMap):
                     for branch in branches.keys():
                         ## HACK  --hwn
                         branchPrefix = self.depotPaths[0] + branch + "/"
+                        self.branchPrefixes = [ branchPrefix ]
 
                         parent = ""
 
@@ -2449,19 +2449,19 @@ class P4Sync(Command, P4UserMap):
                             tempBranch = os.path.join(self.tempBranchLocation, "%d" % (change))
                             if self.verbose:
                                 print "Creating temporary branch: " + tempBranch
-                            self.commit(description, filesForCommit, tempBranch, [branchPrefix])
+                            self.commit(description, filesForCommit, tempBranch)
                             self.tempBranches.append(tempBranch)
                             self.checkpoint()
                             blob = self.searchParent(parent, branch, tempBranch)
                         if blob:
-                            self.commit(description, filesForCommit, branch, [branchPrefix], blob)
+                            self.commit(description, filesForCommit, branch, blob)
                         else:
                             if self.verbose:
                                 print "Parent of %s not found. Committing into head of %s" % (branch, parent)
-                            self.commit(description, filesForCommit, branch, [branchPrefix], parent)
+                            self.commit(description, filesForCommit, branch, parent)
                 else:
                     files = self.extractFilesFromCommit(description)
-                    self.commit(description, files, self.branch, self.depotPaths,
+                    self.commit(description, files, self.branch,
                                 self.initialParent)
                     self.initialParent = ""
             except IOError:
@@ -2525,7 +2525,7 @@ class P4Sync(Command, P4UserMap):
 
         self.updateOptionDict(details)
         try:
-            self.commit(details, self.extractFilesFromCommit(details), self.branch, self.depotPaths)
+            self.commit(details, self.extractFilesFromCommit(details), self.branch)
         except IOError:
             print "IO error with git fast-import. Is your git version recent enough?"
             print self.gitError.read()
@@ -2683,6 +2683,9 @@ class P4Sync(Command, P4UserMap):
 
         self.depotPaths = newPaths
 
+        # --detect-branches may change this for each branch
+        self.branchPrefixes = self.depotPaths
+
         self.loadUserMapFromCache()
         self.labels = {}
         if self.detectLabels:
-- 
1.7.12.rc2.24.gc304662

[PATCH 4/5] git p4: do wildcard decoding in stripRepoPath

From: Pete Wyckoff <hidden>
Date: 2016-06-15 22:54:28

Instead of having to remember to do it after each call to
stripRepoPath, make it part of that function.

Signed-off-by: Pete Wyckoff <redacted>
---
 git-p4.py | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 6d07115..e20ff5d 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1819,15 +1819,17 @@ class P4Sync(Command, P4UserMap):
 
     def stripRepoPath(self, path, prefixes):
         if self.useClientSpec:
-            return self.clientSpecDirs.map_in_client(path)
+            path = self.clientSpecDirs.map_in_client(path)
 
-        if self.keepRepoPath:
-            prefixes = [re.sub("^(//[^/]+/).*", r'\1', prefixes[0])]
+        else:
+            if self.keepRepoPath:
+                prefixes = [re.sub("^(//[^/]+/).*", r'\1', prefixes[0])]
 
-        for p in prefixes:
-            if p4PathStartsWith(path, p):
-                path = path[len(p):]
+            for p in prefixes:
+                if p4PathStartsWith(path, p):
+                    path = path[len(p):]
 
+        path = wildcard_decode(path)
         return path
 
     def splitFilesIntoBranches(self, commit):
@@ -1849,7 +1851,6 @@ class P4Sync(Command, P4UserMap):
             fnum = fnum + 1
 
             relPath = self.stripRepoPath(path, self.depotPaths)
-            relPath = wildcard_decode(relPath)
 
             for branch in self.knownBranches.keys():
 
@@ -1867,7 +1868,6 @@ class P4Sync(Command, P4UserMap):
 
     def streamOneP4File(self, file, contents):
         relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes)
-        relPath = wildcard_decode(relPath)
         if verbose:
             sys.stderr.write("%s\n" % relPath)
 
@@ -1936,7 +1936,6 @@ class P4Sync(Command, P4UserMap):
 
     def streamOneP4Deletion(self, file):
         relPath = self.stripRepoPath(file['path'], self.branchPrefixes)
-        relPath = wildcard_decode(relPath)
         if verbose:
             sys.stderr.write("delete %s\n" % relPath)
         self.gitStream.write("D %s\n" % relPath)
-- 
1.7.12.rc2.24.gc304662

[PATCH 5/5] git p4: make branch detection work with --use-client-spec

From: Pete Wyckoff <hidden>
Date: 2016-06-15 22:54:28

The bug report in http://stackoverflow.com/questions/11893688
observes that files are mapped into the wrong locations in
git when both --use-client-spec and --branch-detection are enabled.

Fix this by changing the relative path prefix to match discovered
branches when using a client spec.

The problem was likely introduced with ecb7cf9 (git-p4: rewrite view
handling, 2012-01-02).

Signed-off-by: Pete Wyckoff <redacted>
---
 git-p4.py                | 37 +++++++++++++++++++++++++++++++------
 t/t9801-git-p4-branch.sh |  2 +-
 2 files changed, 32 insertions(+), 7 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index e20ff5d..aed1a2d 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1818,21 +1818,41 @@ class P4Sync(Command, P4UserMap):
         return files
 
     def stripRepoPath(self, path, prefixes):
+        """When streaming files, this is called to map a p4 depot path
+           to where it should go in git.  The prefixes are either
+           self.depotPaths, or self.branchPrefixes in the case of
+           branch detection."""
+
         if self.useClientSpec:
+            # branch detection moves files up a level (the branch name)
+            # from what client spec interpretation gives
             path = self.clientSpecDirs.map_in_client(path)
+            if self.detectBranches:
+                for b in self.knownBranches:
+                    if path.startswith(b + "/"):
+                        path = path[len(b)+1:]
+
+        elif 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.
+            depot = re.sub("^(//[^/]+/).*", r'\1', prefixes[0])
+            if p4PathStartsWith(path, depot):
+                path = path[len(depot):]
 
         else:
-            if self.keepRepoPath:
-                prefixes = [re.sub("^(//[^/]+/).*", r'\1', prefixes[0])]
-
             for p in prefixes:
                 if p4PathStartsWith(path, p):
                     path = path[len(p):]
+                    break
 
         path = wildcard_decode(path)
         return path
 
     def splitFilesIntoBranches(self, commit):
+        """Look at each depotFile in the commit to figure out to what
+           branch it belongs."""
+
         branches = {}
         fnum = 0
         while commit.has_key("depotFile%s" % fnum):
@@ -1850,11 +1870,16 @@ class P4Sync(Command, P4UserMap):
             file["type"] = commit["type%s" % fnum]
             fnum = fnum + 1
 
-            relPath = self.stripRepoPath(path, self.depotPaths)
+            # start with the full relative path where this file would
+            # go in a p4 client
+            if self.useClientSpec:
+                relPath = self.clientSpecDirs.map_in_client(path)
+            else:
+                relPath = self.stripRepoPath(path, self.depotPaths)
 
             for branch in self.knownBranches.keys():
-
-                # add a trailing slash so that a commit into qt/4.2foo doesn't end up in qt/4.2
+                # add a trailing slash so that a commit into qt/4.2foo
+                # doesn't end up in qt/4.2, e.g.
                 if relPath.startswith(branch + "/"):
                     if branch not in branches:
                         branches[branch] = []
diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh
index ca3a7f9..c5f0977 100755
--- a/t/t9801-git-p4-branch.sh
+++ b/t/t9801-git-p4-branch.sh
@@ -438,7 +438,7 @@ test_expect_success 'use-client-spec detect-branches setup' '
 	)
 '
 
-test_expect_failure 'use-client-spec detect-branches files in top-level' '
+test_expect_success 'use-client-spec detect-branches files in top-level' '
 	test_when_finished cleanup_git &&
 	test_create_repo "$git" &&
 	(
-- 
1.7.12.rc2.24.gc304662
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help