threads / patch / 26755

patch, 2 partsgit-p4: sync new branch regression

Subject: [PATCH 0/2] git-p4: sync new branch regression

## tl;dr

3 messages between Mar 16, 2011 and Mar 16, 2011. Diffs are folded; open one to read it.

replies: 2people: 1as markdown or json

Pete Wyckoff· Mar 16, 2011, 20:50 UTC · lore

Here is another fix for e32e00d (git-p4: better message for "git-p4 sync" when not cloned, 2011-02-19). Mike pointed out that his use case breaks too.

These two patches fix it, and add two new test cases to check for future breakage. At least we're coming up with a good set of tests.

    git-p4: fix sync new branch regression
    git-p4: test sync new branch

 contrib/fast-import/git-p4 |    4 +++-
 t/t9800-git-p4.sh          |   23 +++++++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletions(-)

These would be good on master, beyond e32e00d that is in your pw/p4. Thanks,

                -- Pete

Pete Wyckoff· Mar 16, 2011, 20:52 UTC · re: Pete Wyckoff · lore

[PATCH 1/2] git-p4: fix sync new branch regression

e32e00d (git-p4: better message for "git-p4 sync" when not cloned, 2011-02-19) broke another use case, that of using "git-p4 sync" to import a new branch into an existing repository.

Refine the fix again, on top of the fix in ac34efc.

Reported-by: Michael Horowitz <michael.horowitz@ieee.org>
Signed-off-by: Pete Wyckoff <pw@padd.com>
Tested-by: Michael Horowitz <michael.horowitz@ieee.org>
---
 contrib/fast-import/git-p4 |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

Show changes to contrib/fast-import/git-p4 +3 −1

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 7cb479c..2eacb95 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -1763,7 +1763,9 @@ class P4Sync(Command):
 
                 changes.sort()
             else:
-                if not isinstance(self, P4Clone) and not self.p4BranchesInGit:
+                # catch "git-p4 sync" with no new branches, in a repo that
+                # does not have any existing git-p4 branches
+                if len(args) == 0 and not self.p4BranchesInGit:
                     die("No remote p4 branches.  Perhaps you never did \"git p4 clone\" in here.");
                 if self.verbose:
                     print "Getting p4 changes for %s...%s" % (', '.join(self.depotPaths),

-- 
1.7.4.1

Pete Wyckoff· Mar 16, 2011, 20:53 UTC · re: Pete Wyckoff · lore

[PATCH 2/2] git-p4: test sync new branch

Add two new unit tests. One to test the feature that that was added in e32e00d, and another to test the regression that was fixed in the parent to this commit.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 t/t9800-git-p4.sh |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

Show changes to t/t9800-git-p4.sh +23 −0

diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
index abe7c64..a523473 100755
--- a/t/t9800-git-p4.sh
+++ b/t/t9800-git-p4.sh
@@ -61,6 +61,29 @@ test_expect_success 'git-p4 clone @all' '
 	rm -rf "$git" && mkdir "$git"
 '
 
+test_expect_success 'git-p4 sync uninitialized repo' '
+	test_create_repo "$git" &&
+	cd "$git" &&
+	test_must_fail "$GITP4" sync &&
+	rm -rf "$git" && mkdir "$git"
+'
+
+#
+# Create a git repo by hand.  Add a commit so that HEAD is valid.
+# Test imports a new p4 repository into a new git branch.
+#
+test_expect_success 'git-p4 sync new branch' '
+	test_create_repo "$git" &&
+	cd "$git" &&
+	test_commit head &&
+	"$GITP4" sync --branch=refs/remotes/p4/depot //depot@all &&
+	git log --oneline p4/depot >lines &&
+	cat lines &&
+	test_line_count = 2 lines &&
+	cd .. &&
+	rm -rf "$git" && mkdir "$git"
+'
+
 test_expect_success 'exit when p4 fails to produce marshaled output' '
 	badp4dir="$TRASH_DIRECTORY/badp4dir" &&
 	mkdir -p "$badp4dir" &&

-- 
1.7.4.1