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>

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 94f0a12d61ddc01f535615c751daa5800506e0d9..9ccc87b2055a8f90fbe1fcc80649f6a72bb09a5e 100755 (executable)
--- 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 25e01a469dcc262f09d72b6a81549523a210a9b7..96426414aadca543d650cd607b385c75f377260d 100755 (executable)
--- 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