Merge branch 'pw/p4-various'

Miscellaneous updates to "git p4".

By Pete Wyckoff
* pw/p4-various:
  git p4: submit files with wildcards
  git p4: fix writable file after rename or copy
  git p4: test submit
  git p4: bring back files in deleted client directory

4 files changed, 308 insertions, 35 deletions

diff --git a/git-p4.py b/git-p4.py
index eab6959..565cfbc 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -133,25 +133,29 @@ def p4_system(cmd):
     subprocess.check_call(real_cmd, shell=expand)
 
 def p4_integrate(src, dest):
-    p4_system(["integrate", "-Dt", src, dest])
+    p4_system(["integrate", "-Dt", wildcard_encode(src), wildcard_encode(dest)])
 
-def p4_sync(path):
-    p4_system(["sync", path])
+def p4_sync(f, *options):
+    p4_system(["sync"] + list(options) + [wildcard_encode(f)])
 
 def p4_add(f):
-    p4_system(["add", f])
+    # forcibly add file names with wildcards
+    if wildcard_present(f):
+        p4_system(["add", "-f", f])
+    else:
+        p4_system(["add", f])
 
 def p4_delete(f):
-    p4_system(["delete", f])
+    p4_system(["delete", wildcard_encode(f)])
 
 def p4_edit(f):
-    p4_system(["edit", f])
+    p4_system(["edit", wildcard_encode(f)])
 
 def p4_revert(f):
-    p4_system(["revert", f])
+    p4_system(["revert", wildcard_encode(f)])
 
-def p4_reopen(type, file):
-    p4_system(["reopen", "-t", type, file])
+def p4_reopen(type, f):
+    p4_system(["reopen", "-t", type, wildcard_encode(f)])
 
 #
 # Canonicalize the p4 type and return a tuple of the
@@ -248,7 +252,7 @@ def setP4ExecBit(file, mode):
 def getP4OpenedType(file):
     # Returns the perforce file type for the given file.
 
-    result = p4_read_pipe(["opened", file])
+    result = p4_read_pipe(["opened", wildcard_encode(file)])
     match = re.match(".*\((.+)\)\r?$", result)
     if match:
         return match.group(1)
@@ -658,6 +662,34 @@ def getClientRoot():
 
     return entry["Root"]
 
+#
+# P4 wildcards are not allowed in filenames.  P4 complains
+# if you simply add them, but you can force it with "-f", in
+# which case it translates them into %xx encoding internally.
+#
+def wildcard_decode(path):
+    # Search for and fix just these four characters.  Do % last so
+    # that fixing it does not inadvertently create new %-escapes.
+    # Cannot have * in a filename in windows; untested as to
+    # what p4 would do in such a case.
+    if not platform.system() == "Windows":
+        path = path.replace("%2A", "*")
+    path = path.replace("%23", "#") \
+               .replace("%40", "@") \
+               .replace("%25", "%")
+    return path
+
+def wildcard_encode(path):
+    # do % first to avoid double-encoding the %s introduced here
+    path = path.replace("%", "%25") \
+               .replace("*", "%2A") \
+               .replace("#", "%23") \
+               .replace("@", "%40")
+    return path
+
+def wildcard_present(path):
+    return path.translate(None, "*#@%") != path
+
 class Command:
     def __init__(self):
         self.usage = "usage: %prog [options]"
@@ -1038,6 +1070,7 @@ class P4Submit(Command, P4UserMap):
         filesToAdd = set()
         filesToDelete = set()
         editedFiles = set()
+        pureRenameCopy = set()
         filesToChangeExecBit = {}
 
         for line in diff:
@@ -1061,10 +1094,13 @@ class P4Submit(Command, P4UserMap):
             elif modifier == "C":
                 src, dest = diff['src'], diff['dst']
                 p4_integrate(src, dest)
+                pureRenameCopy.add(dest)
                 if diff['src_sha1'] != diff['dst_sha1']:
                     p4_edit(dest)
+                    pureRenameCopy.discard(dest)
                 if isModeExecChanged(diff['src_mode'], diff['dst_mode']):
                     p4_edit(dest)
+                    pureRenameCopy.discard(dest)
                     filesToChangeExecBit[dest] = diff['dst_mode']
                 os.unlink(dest)
                 editedFiles.add(dest)
@@ -1073,6 +1109,8 @@ class P4Submit(Command, P4UserMap):
                 p4_integrate(src, dest)
                 if diff['src_sha1'] != diff['dst_sha1']:
                     p4_edit(dest)
+                else:
+                    pureRenameCopy.add(dest)
                 if isModeExecChanged(diff['src_mode'], diff['dst_mode']):
                     p4_edit(dest)
                     filesToChangeExecBit[dest] = diff['dst_mode']
@@ -1181,7 +1219,8 @@ class P4Submit(Command, P4UserMap):
                 del(os.environ["P4DIFF"])
             diff = ""
             for editedFile in editedFiles:
-                diff += p4_read_pipe(['diff', '-du', editedFile])
+                diff += p4_read_pipe(['diff', '-du',
+                                      wildcard_encode(editedFile)])
 
             newdiff = ""
             for newFile in filesToAdd:
@@ -1226,6 +1265,12 @@ class P4Submit(Command, P4UserMap):
                         # unmarshalled.
                         changelist = self.lastP4Changelist()
                         self.modifyChangelistUser(changelist, p4User)
+
+                # The rename/copy happened by applying a patch that created a
+                # new file.  This leaves it writable, which confuses p4.
+                for f in pureRenameCopy:
+                    p4_sync(f, "-f")
+
             else:
                 # skip this patch
                 print "Submission cancelled, undoing p4 changes."
@@ -1361,12 +1406,18 @@ class P4Submit(Command, P4UserMap):
         self.oldWorkingDirectory = os.getcwd()
 
         # ensure the clientPath exists
+        new_client_dir = False
         if not os.path.exists(self.clientPath):
+            new_client_dir = True
             os.makedirs(self.clientPath)
 
         chdir(self.clientPath)
         print "Synchronizing p4 checkout..."
-        p4_sync("...")
+        if new_client_dir:
+            # old one was destroyed, and maybe nobody told p4
+            p4_sync("...", "-f")
+        else:
+            p4_sync("...")
         self.check()
 
         commits = []
@@ -1679,23 +1730,6 @@ class P4Sync(Command, P4UserMap):
         if gitConfig("git-p4.syncFromOrigin") == "false":
             self.syncWithOrigin = False
 
-    #
-    # P4 wildcards are not allowed in filenames.  P4 complains
-    # if you simply add them, but you can force it with "-f", in
-    # which case it translates them into %xx encoding internally.
-    # Search for and fix just these four characters.  Do % last so
-    # that fixing it does not inadvertently create new %-escapes.
-    #
-    def wildcard_decode(self, path):
-        # Cannot have * in a filename in windows; untested as to
-        # what p4 would do in such a case.
-        if not self.isWindows:
-            path = path.replace("%2A", "*")
-        path = path.replace("%23", "#") \
-                   .replace("%40", "@") \
-                   .replace("%25", "%")
-        return path
-
     # Force a checkpoint in fast-import and wait for it to finish
     def checkpoint(self):
         self.gitStream.write("checkpoint\n\n")
@@ -1763,6 +1797,7 @@ class P4Sync(Command, P4UserMap):
             fnum = fnum + 1
 
             relPath = self.stripRepoPath(path, self.depotPaths)
+            relPath = wildcard_decode(relPath)
 
             for branch in self.knownBranches.keys():
 
@@ -1780,7 +1815,7 @@ class P4Sync(Command, P4UserMap):
 
     def streamOneP4File(self, file, contents):
         relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes)
-        relPath = self.wildcard_decode(relPath)
+        relPath = wildcard_decode(relPath)
         if verbose:
             sys.stderr.write("%s\n" % relPath)
 
@@ -1849,6 +1884,7 @@ 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)
diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index b2f0869..0f410c4 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -163,6 +163,112 @@ test_expect_success 'wildcard files git p4 clone' '
 	)
 '
 
+test_expect_success 'wildcard files submit back to p4, add' '
+	test_when_finished cleanup_git &&
+	git p4 clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		echo git-wild-hash >git-wild#hash &&
+		echo git-wild-star >git-wild\*star &&
+		echo git-wild-at >git-wild@at &&
+		echo git-wild-percent >git-wild%percent &&
+		git add git-wild* &&
+		git commit -m "add some wildcard filenames" &&
+		git config git-p4.skipSubmitEdit true &&
+		git p4 submit
+	) &&
+	(
+		cd "$cli" &&
+		test_path_is_file git-wild#hash &&
+		test_path_is_file git-wild\*star &&
+		test_path_is_file git-wild@at &&
+		test_path_is_file git-wild%percent
+	)
+'
+
+test_expect_success 'wildcard files submit back to p4, modify' '
+	test_when_finished cleanup_git &&
+	git p4 clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		echo new-line >>git-wild#hash &&
+		echo new-line >>git-wild\*star &&
+		echo new-line >>git-wild@at &&
+		echo new-line >>git-wild%percent &&
+		git add git-wild* &&
+		git commit -m "modify the wildcard files" &&
+		git config git-p4.skipSubmitEdit true &&
+		git p4 submit
+	) &&
+	(
+		cd "$cli" &&
+		test_line_count = 2 git-wild#hash &&
+		test_line_count = 2 git-wild\*star &&
+		test_line_count = 2 git-wild@at &&
+		test_line_count = 2 git-wild%percent
+	)
+'
+
+test_expect_success 'wildcard files submit back to p4, copy' '
+	test_when_finished cleanup_git &&
+	git p4 clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		cp file2 git-wild-cp#hash &&
+		git add git-wild-cp#hash &&
+		cp git-wild\*star file-wild-3 &&
+		git add file-wild-3 &&
+		git commit -m "wildcard copies" &&
+		git config git-p4.detectCopies true &&
+		git config git-p4.detectCopiesHarder true &&
+		git config git-p4.skipSubmitEdit true &&
+		git p4 submit
+	) &&
+	(
+		cd "$cli" &&
+		test_path_is_file git-wild-cp#hash &&
+		test_path_is_file file-wild-3
+	)
+'
+
+test_expect_success 'wildcard files submit back to p4, rename' '
+	test_when_finished cleanup_git &&
+	git p4 clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		git mv git-wild@at file-wild-4 &&
+		git mv file-wild-3 git-wild-cp%percent &&
+		git commit -m "wildcard renames" &&
+		git config git-p4.detectRenames true &&
+		git config git-p4.skipSubmitEdit true &&
+		git p4 submit
+	) &&
+	(
+		cd "$cli" &&
+		test_path_is_missing git-wild@at &&
+		test_path_is_file git-wild-cp%percent
+	)
+'
+
+test_expect_success 'wildcard files submit back to p4, delete' '
+	test_when_finished cleanup_git &&
+	git p4 clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		git rm git-wild* &&
+		git commit -m "delete the wildcard files" &&
+		git config git-p4.skipSubmitEdit true &&
+		git p4 submit
+	) &&
+	(
+		cd "$cli" &&
+		test_path_is_missing git-wild#hash &&
+		test_path_is_missing git-wild\*star &&
+		test_path_is_missing git-wild@at &&
+		test_path_is_missing git-wild%percent
+	)
+'
+
 test_expect_success 'clone bare' '
 	git p4 clone --dest="$git" --bare //depot &&
 	test_when_finished cleanup_git &&
diff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh
index 1541716..f23b4c3 100755
--- a/t/t9807-git-p4-submit.sh
+++ b/t/t9807-git-p4-submit.sh
@@ -28,6 +28,11 @@ test_expect_success 'submit with no client dir' '
 		rm -rf "$cli" &&
 		git config git-p4.skipSubmitEdit true &&
 		git p4 submit
+	) &&
+	(
+		cd "$cli" &&
+		test_path_is_file file1 &&
+		test_path_is_file file2
 	)
 '
 
@@ -44,7 +49,6 @@ test_expect_success 'submit --origin' '
 	) &&
 	(
 		cd "$cli" &&
-		p4 sync &&
 		test_path_is_missing "file3.t" &&
 		test_path_is_file "file4.t"
 	)
@@ -79,12 +83,105 @@ test_expect_success 'submit with master branch name from argv' '
 	) &&
 	(
 		cd "$cli" &&
-		p4 sync &&
 		test_path_is_file "file6.t" &&
 		test_path_is_missing "file7.t"
 	)
 '
 
+#
+# Basic submit tests, the five handled cases
+#
+
+test_expect_success 'submit modify' '
+	test_when_finished cleanup_git &&
+	git p4 clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		git config git-p4.skipSubmitEdit true &&
+		echo line >>file1 &&
+		git add file1 &&
+		git commit -m file1 &&
+		git p4 submit
+	) &&
+	(
+		cd "$cli" &&
+		test_path_is_file file1 &&
+		test_line_count = 2 file1
+	)
+'
+
+test_expect_success 'submit add' '
+	test_when_finished cleanup_git &&
+	git p4 clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		git config git-p4.skipSubmitEdit true &&
+		echo file13 >file13 &&
+		git add file13 &&
+		git commit -m file13 &&
+		git p4 submit
+	) &&
+	(
+		cd "$cli" &&
+		test_path_is_file file13
+	)
+'
+
+test_expect_success 'submit delete' '
+	test_when_finished cleanup_git &&
+	git p4 clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		git config git-p4.skipSubmitEdit true &&
+		git rm file4.t &&
+		git commit -m "delete file4.t" &&
+		git p4 submit
+	) &&
+	(
+		cd "$cli" &&
+		test_path_is_missing file4.t
+	)
+'
+
+test_expect_success 'submit copy' '
+	test_when_finished cleanup_git &&
+	git p4 clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		git config git-p4.skipSubmitEdit true &&
+		git config git-p4.detectCopies true &&
+		git config git-p4.detectCopiesHarder true &&
+		cp file5.t file5.ta &&
+		git add file5.ta &&
+		git commit -m "copy to file5.ta" &&
+		git p4 submit
+	) &&
+	(
+		cd "$cli" &&
+		test_path_is_file file5.ta &&
+		test ! -w file5.ta
+	)
+'
+
+test_expect_success 'submit rename' '
+	test_when_finished cleanup_git &&
+	git p4 clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		git config git-p4.skipSubmitEdit true &&
+		git config git-p4.detectRenames true &&
+		git mv file6.t file6.ta &&
+		git commit -m "rename file6.t to file6.ta" &&
+		git p4 submit
+	) &&
+	(
+		cd "$cli" &&
+		test_path_is_missing file6.t &&
+		test_path_is_file file6.ta &&
+		test ! -w file6.ta
+	)
+'
+
 test_expect_success 'kill p4d' '
 	kill_p4d
 '
diff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh
index 796b02c..7d993ef 100755
--- a/t/t9809-git-p4-client-view.sh
+++ b/t/t9809-git-p4-client-view.sh
@@ -349,7 +349,8 @@ test_expect_success 'subdir clone, submit copy' '
 	) &&
 	(
 		cd "$cli" &&
-		test_path_is_file dir1/file11a
+		test_path_is_file dir1/file11a &&
+		test ! -w dir1/file11a
 	)
 '
 
@@ -368,14 +369,47 @@ test_expect_success 'subdir clone, submit rename' '
 	(
 		cd "$cli" &&
 		test_path_is_missing dir1/file13 &&
-		test_path_is_file dir1/file13a
+		test_path_is_file dir1/file13a &&
+		test ! -w dir1/file13a
+	)
+'
+
+# see t9800 for the non-client-spec case, and the rest of the wildcard tests
+test_expect_success 'wildcard files submit back to p4, client-spec case' '
+	client_view "//depot/... //client/..." &&
+	test_when_finished cleanup_git &&
+	git p4 clone --use-client-spec --dest="$git" //depot/dir1 &&
+	(
+		cd "$git" &&
+		echo git-wild-hash >dir1/git-wild#hash &&
+		echo git-wild-star >dir1/git-wild\*star &&
+		echo git-wild-at >dir1/git-wild@at &&
+		echo git-wild-percent >dir1/git-wild%percent &&
+		git add dir1/git-wild* &&
+		git commit -m "add some wildcard filenames" &&
+		git config git-p4.skipSubmitEditCheck true &&
+		git p4 submit
+	) &&
+	(
+		cd "$cli" &&
+		test_path_is_file dir1/git-wild#hash &&
+		test_path_is_file dir1/git-wild\*star &&
+		test_path_is_file dir1/git-wild@at &&
+		test_path_is_file dir1/git-wild%percent
+	) &&
+	(
+		# delete these carefully, cannot just do "p4 delete"
+		# on files with wildcards; but git-p4 knows how
+		cd "$git" &&
+		git rm dir1/git-wild* &&
+		git commit -m "clean up the wildcards" &&
+		git p4 submit
 	)
 '
 
 test_expect_success 'reinit depot' '
 	(
 		cd "$cli" &&
-		p4 sync -f &&
 		rm files &&
 		p4 delete */* &&
 		p4 submit -d "delete all files" &&