git-p4: fix submit regression with clientSpec and subdir clone

When the --use-client-spec is given to clone, and the clone
path is a subset of the full tree as specified in the client,
future submits will go to the wrong place.

Factor out getClientSpec() so both clone/sync and submit can
use it.  Introduce getClientRoot() that is needed for the client
spec case, and use it instead of p4Where().

Test the five possible submit behaviors (add, modify, rename,
copy, delete).

Reported-by: Laurent Charrière <lcharriere@promptu.com>
Signed-off-by: Pete Wyckoff <pw@padd.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

2 files changed, 185 insertions, 43 deletions

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 94f0a12d61..9ccc87b205 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -555,6 +555,46 @@ def p4PathStartsWith(path, prefix):
         return path.lower().startswith(prefix.lower())
     return path.startswith(prefix)
 
+def getClientSpec():
+    """Look at the p4 client spec, create a View() object that contains
+       all the mappings, and return it."""
+
+    specList = p4CmdList("client -o")
+    if len(specList) != 1:
+        die('Output from "client -o" is %d lines, expecting 1' %
+            len(specList))
+
+    # dictionary of all client parameters
+    entry = specList[0]
+
+    # just the keys that start with "View"
+    view_keys = [ k for k in entry.keys() if k.startswith("View") ]
+
+    # hold this new View
+    view = View()
+
+    # append the lines, in order, to the view
+    for view_num in range(len(view_keys)):
+        k = "View%d" % view_num
+        if k not in view_keys:
+            die("Expected view key %s missing" % k)
+        view.append(entry[k])
+
+    return view
+
+def getClientRoot():
+    """Grab the client directory."""
+
+    output = p4CmdList("client -o")
+    if len(output) != 1:
+        die('Output from "client -o" is %d lines, expecting 1' % len(output))
+
+    entry = output[0]
+    if "Root" not in entry:
+        die('Client has no "Root"')
+
+    return entry["Root"]
+
 class Command:
     def __init__(self):
         self.usage = "usage: %prog [options]"
@@ -1119,11 +1159,20 @@ class P4Submit(Command, P4UserMap):
             print "Internal error: cannot locate perforce depot path from existing branches"
             sys.exit(128)
 
-        self.clientPath = p4Where(self.depotPath)
+        self.useClientSpec = False
+        if gitConfig("git-p4.useclientspec", "--bool") == "true":
+            self.useClientSpec = True
+        if self.useClientSpec:
+            self.clientSpecDirs = getClientSpec()
+
+        if self.useClientSpec:
+            # all files are relative to the client spec
+            self.clientPath = getClientRoot()
+        else:
+            self.clientPath = p4Where(self.depotPath)
 
-        if len(self.clientPath) == 0:
-            print "Error: Cannot locate perforce checkout of %s in client view" % self.depotPath
-            sys.exit(128)
+        if self.clientPath == "":
+            die("Error: Cannot locate perforce checkout of %s in client view" % self.depotPath)
 
         print "Perforce checkout for depot path %s located at %s" % (self.depotPath, self.clientPath)
         self.oldWorkingDirectory = os.getcwd()
@@ -2078,33 +2127,6 @@ class P4Sync(Command, P4UserMap):
             print self.gitError.read()
 
 
-    def getClientSpec(self):
-        specList = p4CmdList("client -o")
-        if len(specList) != 1:
-            die('Output from "client -o" is %d lines, expecting 1' %
-                len(specList))
-
-        # dictionary of all client parameters
-        entry = specList[0]
-
-        # just the keys that start with "View"
-        view_keys = [ k for k in entry.keys() if k.startswith("View") ]
-
-        # hold this new View
-        view = View()
-
-        # append the lines, in order, to the view
-        for view_num in range(len(view_keys)):
-            k = "View%d" % view_num
-            if k not in view_keys:
-                die("Expected view key %s missing" % k)
-            view.append(entry[k])
-
-        self.clientSpecDirs = view
-        if self.verbose:
-            for i, m in enumerate(self.clientSpecDirs.mappings):
-                    print "clientSpecDirs %d: %s" % (i, str(m))
-
     def run(self, args):
         self.depotPaths = []
         self.changeRange = ""
@@ -2145,7 +2167,7 @@ class P4Sync(Command, P4UserMap):
             if gitConfig("git-p4.useclientspec", "--bool") == "true":
                 self.useClientSpec = True
         if self.useClientSpec:
-            self.getClientSpec()
+            self.clientSpecDirs = getClientSpec()
 
         # TODO: should always look at previous commits,
         # merge with previous imports, if possible.
diff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh
index 25e01a469d..96426414aa 100755
--- a/t/t9809-git-p4-client-view.sh
+++ b/t/t9809-git-p4-client-view.sh
@@ -71,20 +71,24 @@ git_verify() {
 #   - dir2
 #     - file21
 #     - file22
+init_depot() {
+	for d in 1 2 ; do
+		mkdir -p dir$d &&
+		for f in 1 2 ; do
+			echo dir$d/file$d$f >dir$d/file$d$f &&
+			p4 add dir$d/file$d$f &&
+			p4 submit -d "dir$d/file$d$f"
+		done
+	done &&
+	find . -type f ! -name files >files &&
+	check_files_exist dir1/file11 dir1/file12 \
+			  dir2/file21 dir2/file22
+}
+
 test_expect_success 'init depot' '
 	(
 		cd "$cli" &&
-		for d in 1 2 ; do
-			mkdir -p dir$d &&
-			for f in 1 2 ; do
-				echo dir$d/file$d$f >dir$d/file$d$f &&
-				p4 add dir$d/file$d$f &&
-				p4 submit -d "dir$d/file$d$f"
-			done
-		done &&
-		find . -type f ! -name files >files &&
-		check_files_exist dir1/file11 dir1/file12 \
-				  dir2/file21 dir2/file22
+		init_depot
 	)
 '
 
@@ -257,6 +261,122 @@ test_expect_success 'clone --use-client-spec sets useClientSpec' '
 	)
 '
 
+# clone just a subdir of the client spec
+test_expect_success 'subdir clone' '
+	client_view "//depot/... //client/..." &&
+	files="dir1/file11 dir1/file12 dir2/file21 dir2/file22" &&
+	client_verify $files &&
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --use-client-spec --dest="$git" //depot/dir1 &&
+	git_verify dir1/file11 dir1/file12
+'
+
+#
+# submit back, see what happens:  five cases
+#
+test_expect_success 'subdir clone, submit modify' '
+	client_view "//depot/... //client/..." &&
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --use-client-spec --dest="$git" //depot/dir1 &&
+	(
+		cd "$git" &&
+		git config git-p4.skipSubmitEdit true &&
+		echo line >>dir1/file12 &&
+		git add dir1/file12 &&
+		git commit -m dir1/file12 &&
+		"$GITP4" submit
+	) &&
+	(
+		cd "$cli" &&
+		test_path_is_file dir1/file12 &&
+		test_line_count = 2 dir1/file12
+	)
+'
+
+test_expect_success 'subdir clone, submit add' '
+	client_view "//depot/... //client/..." &&
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --use-client-spec --dest="$git" //depot/dir1 &&
+	(
+		cd "$git" &&
+		git config git-p4.skipSubmitEdit true &&
+		echo file13 >dir1/file13 &&
+		git add dir1/file13 &&
+		git commit -m dir1/file13 &&
+		"$GITP4" submit
+	) &&
+	(
+		cd "$cli" &&
+		test_path_is_file dir1/file13
+	)
+'
+
+test_expect_success 'subdir clone, submit delete' '
+	client_view "//depot/... //client/..." &&
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --use-client-spec --dest="$git" //depot/dir1 &&
+	(
+		cd "$git" &&
+		git config git-p4.skipSubmitEdit true &&
+		git rm dir1/file12 &&
+		git commit -m "delete dir1/file12" &&
+		"$GITP4" submit
+	) &&
+	(
+		cd "$cli" &&
+		test_path_is_missing dir1/file12
+	)
+'
+
+test_expect_success 'subdir clone, submit copy' '
+	client_view "//depot/... //client/..." &&
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --use-client-spec --dest="$git" //depot/dir1 &&
+	(
+		cd "$git" &&
+		git config git-p4.skipSubmitEdit true &&
+		git config git-p4.detectCopies true &&
+		cp dir1/file11 dir1/file11a &&
+		git add dir1/file11a &&
+		git commit -m "copy to dir1/file11a" &&
+		"$GITP4" submit
+	) &&
+	(
+		cd "$cli" &&
+		test_path_is_file dir1/file11a
+	)
+'
+
+test_expect_success 'subdir clone, submit rename' '
+	client_view "//depot/... //client/..." &&
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --use-client-spec --dest="$git" //depot/dir1 &&
+	(
+		cd "$git" &&
+		git config git-p4.skipSubmitEdit true &&
+		git config git-p4.detectRenames true &&
+		git mv dir1/file13 dir1/file13a &&
+		git commit -m "rename dir1/file13 to dir1/file13a" &&
+		"$GITP4" submit
+	) &&
+	(
+		cd "$cli" &&
+		test_path_is_missing dir1/file13 &&
+		test_path_is_file dir1/file13a
+	)
+'
+
+test_expect_success 'reinit depot' '
+	(
+		cd "$cli" &&
+		p4 sync -f &&
+		rm files &&
+		p4 delete */* &&
+		p4 submit -d "delete all files" &&
+		init_depot
+	)
+'
+
 #
 # Rename directories to test quoting in depot-side mappings
 # //depot