[PATCH 0/2] git-p4: fix clone @all regression

[PATCH 0/2] git-p4: fix clone @all regression

[Pete Wyckoff]

My commit e32e00dc88948a730b8b1f3b8129f30c313713e7
"git-p4: better message for "git-p4 sync" when not cloned"
broke clone @all.  This fixes it per Tor Arvid's patch
and adds a test to make sure it doesn't happen again.

    git-p4: fix clone @all regression
    git-p4: test clone @all

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

Please queue to maint.  Thanks,

		-- Pete

[PATCH 1/2] git-p4: fix clone @all regression

[Pete Wyckoff]

Commit e32e00dc88948a730b8b1f3b8129f30c313713e7
"git-p4: better message for "git-p4 sync" when not cloned"
broke the use of the "@all" revision specifier, e.g.,
"git-p4 clone //depot/xxx@all".

Fix it as per Tor Arvid's quick patch.

Signed-off-by: Pete Wyckoff <pw@padd.com>
Reported-by: Anatol Pomozov <anatol.pomozov@gmail.com>
Based-on-patch-by: Tor Arvid Lund <torarvid@gmail.com>
---
 contrib/fast-import/git-p4 |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 2df3bb2..7cb479c 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -1763,7 +1763,7 @@ class P4Sync(Command):
 
                 changes.sort()
             else:
-                if not self.p4BranchesInGit:
+                if not isinstance(self, P4Clone) 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

[PATCH 2/2] git-p4: test clone @all

[Pete Wyckoff]

Cloning a p4 depot by default generates a single commit.  The use
of the "@all" revision specifier instead tells git-p4 to import
all commits.  Check to make sure both these invocations work as
expected.

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

diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
index 1969e6b..abe7c64 100755
--- a/t/t9800-git-p4.sh
+++ b/t/t9800-git-p4.sh
@@ -37,11 +37,27 @@ test_expect_success 'add p4 files' '
 	echo file1 >file1 &&
 	p4 add file1 &&
 	p4 submit -d "file1" &&
+	echo file2 >file2 &&
+	p4 add file2 &&
+	p4 submit -d "file2" &&
 	cd "$TRASH_DIRECTORY"
 '
 
 test_expect_success 'basic git-p4 clone' '
 	"$GITP4" clone --dest="$git" //depot &&
+	cd "$git" &&
+	git log --oneline >lines &&
+	test_line_count = 1 lines &&
+	cd .. &&
+	rm -rf "$git" && mkdir "$git"
+'
+
+test_expect_success 'git-p4 clone @all' '
+	"$GITP4" clone --dest="$git" //depot@all &&
+	cd "$git" &&
+	git log --oneline >lines &&
+	test_line_count = 2 lines &&
+	cd .. &&
 	rm -rf "$git" && mkdir "$git"
 '
 
-- 
1.7.4.1

Re: [PATCH 0/2] git-p4: fix clone @all regression

[Junio C Hamano]

Pete Wyckoff <pw@padd.com> writes:

> My commit e32e00dc88948a730b8b1f3b8129f30c313713e7
> "git-p4: better message for "git-p4 sync" when not cloned"
> broke clone @all.  This fixes it per Tor Arvid's patch
> and adds a test to make sure it doesn't happen again.
>
>     git-p4: fix clone @all regression
>     git-p4: test clone @all
>
>  contrib/fast-import/git-p4 |    2 +-
>  t/t9800-git-p4.sh          |   16 ++++++++++++++++
>  2 files changed, 17 insertions(+), 1 deletions(-)
>
> Please queue to maint.  Thanks,

The buggy e32e00d itself is not in maint, even though the branch it is on
whose tip is at 3820007 (git-p4: support clone --bare, 2011-02-19) could
be merged to maint if we really wanted to.  But it doesn't look like that
the changes on the branch are all fixes, so it is not entirely clear if we
want to.

In any case, will merge the result of applying these two on top of 3820007
(git-p4: support clone --bare, 2011-02-19) to master.

Thanks.