[PATCH] git-p4: Fix multi-path changelist empty commits

* [PATCH] git-p4: Fix multi-path changelist empty commits
@ 2016-12-15 17:14 George Vanburgh
  2016-12-17  0:47 ` Luke Diamand
  2016-12-17 22:11 ` [PATCH v2] " George Vanburgh
  0 siblings, 2 replies; 6+ messages in thread
From: George Vanburgh @ 2016-12-15 17:14 UTC (permalink / raw)
  To: git

From: George Vanburgh <gvanburgh@bloomberg.net>

When importing from multiple perforce paths - we may attempt to import a changelist that contains files from two (or more) of these depot paths. Currently, this results in multiple git commits - one containing the changes, and the other(s) as empty commits. This behavior was introduced in commit 1f90a64 ("git-p4: reduce number of server queries for fetches", 2015-12-19).

Reproduction Steps:

1. Have a git repo cloned from a perforce repo using multiple depot paths (e.g. //depot/foo and //depot/bar).
2. Submit a single change to the perforce repo that makes changes in both //depot/foo and //depot/bar.
3. Run "git p4 sync" to sync the change from #2.

Change is synced as multiple commits, one for each depot path that was affected.

Using a set, instead of a list inside p4ChangesForPaths() ensures that each changelist is unique to the returned list, and therefore only a single commit is generated for each changelist.

Reported-by: James Farwell <jfarwell@vmware.com>
Signed-off-by: George Vanburgh <gvanburgh@bloomberg.net>
---
 git-p4.py               |  4 ++--
 t/t9800-git-p4-basic.sh | 22 +++++++++++++++++++++-
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index fd5ca52..6307bc8 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -822,7 +822,7 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):
                 die("cannot use --changes-block-size with non-numeric revisions")
             block_size = None
 
-    changes = []
+    changes = set()
 
     # Retrieve changes a block at a time, to prevent running
     # into a MaxResults/MaxScanRows error from the server.
@@ -841,7 +841,7 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):
 
         # Insert changes in chronological order
         for line in reversed(p4_read_pipe_lines(cmd)):
-            changes.append(int(line.split(" ")[1]))
+            changes.add(int(line.split(" ")[1]))
 
         if not block_size:
             break
diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 0730f18..4d72e0b 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -131,6 +131,26 @@ test_expect_success 'clone two dirs, @all, conflicting files' '
 	)
 '
 
+test_expect_success 'clone two dirs, each edited by submit, single git commit' '
+	(
+		cd "$cli" &&
+		echo sub1/f4 >sub1/f4 &&
+		p4 add sub1/f4 &&
+		echo sub2/f4 >sub2/f4 &&
+		p4 add sub2/f4 &&
+		p4 submit -d "sub1/f4 and sub2/f4"
+	) &&
+	git p4 clone --dest="$git" //depot/sub1@all //depot/sub2@all &&
+	test_when_finished cleanup_git &&
+	(
+		cd "$git"
+		git ls-files >lines &&
+		test_line_count = 4 lines &&
+		git log --oneline p4/master >lines &&
+		test_line_count = 5 lines
+	)
+'
+
 revision_ranges="2000/01/01,#head \
 		 1,2080/01/01 \
 		 2000/01/01,2080/01/01 \
@@ -147,7 +167,7 @@ test_expect_success 'clone using non-numeric revision ranges' '
 		(
 			cd "$git" &&
 			git ls-files >lines &&
-			test_line_count = 6 lines
+			test_line_count = 8 lines
 		)
 	done
 '

--
https://github.com/git/git/pull/311

^ permalink raw reply related	[flat|nested] 6+ messages in thread