Merge branch 'pw/p4-use-client-spec-branch-detection'

Fix "git p4" when "--use-client-spec" and "--detect-branches" are
used together (the command used to misdetect branches).

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

Author: gitster

git-p4.py

@@ -1818,19 +1818,41 @@ def extractFilesFromCommit(self, commit):
         return files
 
     def stripRepoPath(self, path, prefixes):
-        if self.useClientSpec:
-            return self.clientSpecDirs.map_in_client(path)
+        """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.keepRepoPath:
-            prefixes = [re.sub("^(//[^/]+/).*", r'\1', prefixes[0])]
+        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):]
 
-        for p in prefixes:
-            if p4PathStartsWith(path, p):
-                path = path[len(p):]
+        else:
+            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):
@@ -1848,12 +1870,16 @@ def splitFilesIntoBranches(self, commit):
             file["type"] = commit["type%s" % fnum]
             fnum = fnum + 1
 
-            relPath = self.stripRepoPath(path, self.depotPaths)
-            relPath = wildcard_decode(relPath)
+            # 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] = []
@@ -1867,7 +1893,6 @@ def splitFilesIntoBranches(self, commit):
 
     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 +1961,6 @@ def streamOneP4File(self, file, contents):
 
     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)
@@ -2041,10 +2065,9 @@ def streamTag(self, gitStream, labelName, labelDetails, commit, epoch):
         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 +2076,7 @@ def commit(self, details, files, branch, branchPrefixes, parent = ""):
         # 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 +2093,8 @@ def commit(self, details, files, branch, branchPrefixes, parent = ""):
 
         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 +2117,7 @@ def commit(self, details, files, branch, branchPrefixes, parent = ""):
                 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 +2428,7 @@ def importChanges(self, changes):
                     for branch in branches.keys():
                         ## HACK  --hwn
                         branchPrefix = self.depotPaths[0] + branch + "/"
+                        self.branchPrefixes = [ branchPrefix ]
 
                         parent = ""
 
@@ -2449,19 +2473,19 @@ def importChanges(self, changes):
                             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 +2549,7 @@ def importHeadRevision(self, revision):
 
         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 +2707,9 @@ def run(self, args):
 
         self.depotPaths = newPaths
 
+        # --detect-branches may change this for each branch
+        self.branchPrefixes = self.depotPaths
+
         self.loadUserMapFromCache()
         self.labels = {}
         if self.detectLabels:

t/lib-git-p4.sh

@@ -115,3 +115,20 @@ 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
+}

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_success '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
 '

t/t9809-git-p4-client-view.sh

@@ -8,23 +8,6 @@ test_expect_success 'start p4d' '
 	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".