[PATCHv2 0/4] git-p4: small fixes to branches and labels; tests

[PATCHv2 0/4] git-p4: small fixes to branches and labels; tests

[Luke Diamand]

This is a small set of patches to git-p4 to fix a couple of issues with
branches and labels.

This is the second version of this patch series; the first one vanished
without trace :-)

Firstly, I've added the fixes needed so that branches and labels can
contain shell metacharacters (missed from the previous series). Added
a test case for this.

In adding the test case for labels I also found and fixed a few other
small bugs in the label handling:

 - labels missing a description or "EOT" in their text cause problems;
 - labels without an owner cause problems.

I also noticed, but did not fix, that you can't have more than one label
per commit (the others are silently dropped) and the documentation for
branch import could be improved. I've added a (failing) test case for
the multiple label problem.


Luke Diamand (4):
  git-p4: handle p4 branches and labels containing shell chars
  git-p4: cope with labels with empty descriptions
  git-p4: importing labels should cope with missing owner
  git-p4: add test for p4 labels

 contrib/fast-import/git-p4 |   61 +++++++++++++-----------
 t/t9801-git-p4-branch.sh   |   48 ++++++++++++++++++
 t/t9804-git-p4-label.sh    |  114 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 195 insertions(+), 28 deletions(-)
 create mode 100755 t/t9804-git-p4-label.sh

-- 
1.7.8.rc1.209.geac91.dirty

[PATCHv2 1/4] git-p4: handle p4 branches and labels containing shell chars

[Luke Diamand]

Don't use shell expansion when detecting branches, as it will
fail if the branch name contains a shell metachar. Similarly
for labels.

Add additional test for branches with shell metachars.

Signed-off-by: Luke Diamand <luke@diamand.org>
---
 contrib/fast-import/git-p4 |    8 +++---
 t/t9801-git-p4-branch.sh   |   48 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index b975d67..bcac6ec 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -793,7 +793,7 @@ class P4Submit(Command, P4UserMap):
     def canChangeChangelists(self):
         # check to see if we have p4 admin or super-user permissions, either of
         # which are required to modify changelists.
-        results = p4CmdList("protects %s" % self.depotPath)
+        results = p4CmdList(["protects", self.depotPath])
         for r in results:
             if r.has_key('perm'):
                 if r['perm'] == 'admin':
@@ -1528,7 +1528,7 @@ class P4Sync(Command, P4UserMap):
     def getLabels(self):
         self.labels = {}
 
-        l = p4CmdList("labels %s..." % ' '.join (self.depotPaths))
+        l = p4CmdList(["labels", "%s..." % ' '.join (self.depotPaths)])
         if len(l) > 0 and not self.silent:
             print "Finding files belonging to labels in %s" % `self.depotPaths`
 
@@ -1570,7 +1570,7 @@ class P4Sync(Command, P4UserMap):
             command = "branches"
 
         for info in p4CmdList(command):
-            details = p4Cmd("branch -o %s" % info["branch"])
+            details = p4Cmd(["branch", "-o", info["branch"]])
             viewIdx = 0
             while details.has_key("View%s" % viewIdx):
                 paths = details["View%s" % viewIdx].split(" ")
@@ -1708,7 +1708,7 @@ class P4Sync(Command, P4UserMap):
         sourceRef = self.gitRefForBranch(sourceBranch)
         #print "source " + sourceBranch
 
-        branchParentChange = int(p4Cmd("changes -m 1 %s...@1,%s" % (sourceDepotPath, firstChange))["change"])
+        branchParentChange = int(p4Cmd(["changes", "-m", "1", "%s...@1,%s" % (sourceDepotPath, firstChange)])["change"])
         #print "branch parent: %s" % branchParentChange
         gitParent = self.gitCommitByP4Change(sourceRef, branchParentChange)
         if len(gitParent) > 0:
diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh
index a25f18d..bd08dff 100755
--- a/t/t9801-git-p4-branch.sh
+++ b/t/t9801-git-p4-branch.sh
@@ -226,6 +226,54 @@ test_expect_success 'git-p4 clone simple branches' '
 	)
 '
 
+# Create a branch with a shell metachar in its name
+#
+# 1. //depot/main
+# 2. //depot/branch$3
+
+test_expect_success 'branch with shell char' '
+	test_when_finished cleanup_git &&
+	test_create_repo "$git" &&
+	(
+		cd "$cli" &&
+
+		mkdir -p main &&
+
+		echo f1 >main/f1 &&
+		p4 add main/f1 &&
+		p4 submit -d "main/f1" &&
+
+		p4 integrate //depot/main/... //depot/branch\$3/... &&
+		p4 submit -d "integrate main to branch\$3" &&
+
+		echo f1 >branch\$3/shell_char_branch_file &&
+		p4 add branch\$3/shell_char_branch_file &&
+		p4 submit -d "branch\$3/shell_char_branch_file" &&
+
+		p4 branch -i <<-EOF &&
+		Branch: branch\$3
+		View: //depot/main/... //depot/branch\$3/...
+		EOF
+
+		p4 edit main/f1 &&
+		echo "a change" >> main/f1 &&
+		p4 submit -d "a change" main/f1 &&
+
+		p4 integrate -b branch\$3 &&
+		p4 resolve -am branch\$3/... &&
+		p4 submit -d "integrate main to branch\$3" &&
+
+		cd "$git" &&
+
+		git config git-p4.branchList main:branch\$3 &&
+		"$GITP4" clone --dest=. --detect-branches //depot@all &&
+		git log --all --graph --decorate --stat &&
+		git reset --hard p4/depot/branch\$3 &&
+		test -f shell_char_branch_file &&
+		test -f f1
+	)
+'
+
 test_expect_success 'kill p4d' '
 	kill_p4d
 '
-- 
1.7.8.rc1.209.geac91.dirty

[PATCHv2 2/4] git-p4: cope with labels with empty descriptions

[Luke Diamand]

Use an explicit length for the data in a label, rather
than EOT, so that labels with empty descriptions are
passed through correctly.

Signed-off-by: Luke Diamand <luke@diamand.org>
---
 contrib/fast-import/git-p4 |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index bcac6ec..02f0f54 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -1511,9 +1511,11 @@ class P4Sync(Command, P4UserMap):
                     else:
                         tagger = "%s <a@b> %s %s" % (owner, epoch, self.tz)
                     self.gitStream.write("tagger %s\n" % tagger)
-                    self.gitStream.write("data <<EOT\n")
-                    self.gitStream.write(labelDetails["Description"])
-                    self.gitStream.write("EOT\n\n")
+
+                    description = labelDetails["Description"]
+                    self.gitStream.write("data %d\n" % len(description))
+                    self.gitStream.write(description)
+                    self.gitStream.write("\n")
 
                 else:
                     if not self.silent:
-- 
1.7.8.rc1.209.geac91.dirty

[PATCHv2 3/4] git-p4: importing labels should cope with missing owner

[Luke Diamand]

In p4, the Owner field is optional. If it is missing,
construct something sensible rather than crashing.

Signed-off-by: Luke Diamand <luke@diamand.org>
---
 contrib/fast-import/git-p4 |   45 +++++++++++++++++++++++--------------------
 1 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 02f0f54..d97f927 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -553,6 +553,27 @@ class Command:
     def __init__(self):
         self.usage = "usage: %prog [options]"
         self.needsGit = True
+        self.myP4UserId = None
+
+    def p4UserId(self):
+        if self.myP4UserId:
+            return self.myP4UserId
+
+        results = p4CmdList("user -o")
+        for r in results:
+            if r.has_key('User'):
+                self.myP4UserId = r['User']
+                return r['User']
+        die("Could not find your p4 user id")
+
+    def p4UserIsMe(self, p4User):
+        # return True if the given p4 user is actually me
+        me = self.p4UserId()
+        if not p4User or p4User != me:
+            return False
+        else:
+            return True
+
 
 class P4UserMap:
     def __init__(self):
@@ -694,7 +715,6 @@ class P4Submit(Command, P4UserMap):
         self.verbose = False
         self.preserveUser = gitConfig("git-p4.preserveUser").lower() == "true"
         self.isWindows = (platform.system() == "Windows")
-        self.myP4UserId = None
 
     def check(self):
         if len(p4CmdList("opened ...")) > 0:
@@ -802,25 +822,6 @@ class P4Submit(Command, P4UserMap):
                     return 1
         return 0
 
-    def p4UserId(self):
-        if self.myP4UserId:
-            return self.myP4UserId
-
-        results = p4CmdList("user -o")
-        for r in results:
-            if r.has_key('User'):
-                self.myP4UserId = r['User']
-                return r['User']
-        die("Could not find your p4 user id")
-
-    def p4UserIsMe(self, p4User):
-        # return True if the given p4 user is actually me
-        me = self.p4UserId()
-        if not p4User or p4User != me:
-            return False
-        else:
-            return True
-
     def prepareSubmitTemplate(self):
         # remove lines in the Files section that show changes to files outside the depot path we're committing into
         template = ""
@@ -1506,7 +1507,9 @@ class P4Sync(Command, P4UserMap):
 
                     owner = labelDetails["Owner"]
                     tagger = ""
-                    if author in self.users:
+                    if not owner:
+                        tagger = "%s <a@b> %s %s" % (self.p4UserId(), epoch, self.tz)
+                    elif author in self.users:
                         tagger = "%s %s %s" % (self.users[owner], epoch, self.tz)
                     else:
                         tagger = "%s <a@b> %s %s" % (owner, epoch, self.tz)
-- 
1.7.8.rc1.209.geac91.dirty

[PATCHv2 4/4] git-p4: add test for p4 labels

[Luke Diamand]

Add basic test of p4 label import. Checks label import and
import with shell metachars; labels with different length
descriptions.

Add failure test case for multiple labels.

Signed-off-by: Luke Diamand <luke@diamand.org>
---
 t/t9804-git-p4-label.sh |  114 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 114 insertions(+), 0 deletions(-)
 create mode 100755 t/t9804-git-p4-label.sh

diff --git a/t/t9804-git-p4-label.sh b/t/t9804-git-p4-label.sh
new file mode 100755
index 0000000..a46763f
--- /dev/null
+++ b/t/t9804-git-p4-label.sh
@@ -0,0 +1,114 @@
+test_description='git-p4 p4 label tests'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+	start_p4d
+'
+
+# Basic p4 label tests.
+#
+# Note: can't have more than one label per commit - others
+# are silently discarded.
+#
+test_expect_success 'basic p4 labels' '
+	test_when_finished cleanup_git &&
+	(
+		cd "$cli" &&
+		mkdir -p main &&
+
+		echo f1 >main/f1 &&
+		p4 add main/f1 &&
+		p4 submit -d "main/f1" &&
+
+		echo f2 >main/f2 &&
+		p4 add main/f2 &&
+		p4 submit -d "main/f2" &&
+
+		echo f3 >main/file_with_\$metachar &&
+		p4 add main/file_with_\$metachar &&
+		p4 submit -d "file with metachar" &&
+
+		p4 tag -l tag_f1_only main/f1 &&
+		p4 tag -l tag_with\$_shell_char main/... &&
+
+		echo f4 >main/f4 &&
+		p4 add main/f4 &&
+		p4 submit -d "main/f4" &&
+
+		p4 label -i <<-EOF &&
+		Label: long_label
+		Description:
+		   A Label first line
+		   A Label second line
+		View:	//depot/...
+		EOF
+
+		p4 tag -l long_label ... &&
+
+		p4 labels ... &&
+
+		cd "$git" &&
+		pwd &&
+		"$GITP4" clone --dest=. --detect-labels //depot@all &&
+
+		git tag &&
+		git tag >taglist &&
+		test_line_count = 3 taglist &&
+
+		cd main &&
+		git checkout tag_tag_f1_only &&
+		! test -f f2 &&
+		git checkout tag_tag_with\$_shell_char &&
+		test -f f1 && test -f f2 && test -f file_with_\$metachar &&
+
+		git show tag_long_label | grep -q "A Label second line"
+	)
+'
+
+# Test some label corner cases:
+#
+# - two tags on the same file; both should be available
+# - a tag that is only on one file; the other file should
+#   not be checked out on the tag.
+
+test_expect_failure 'two labels on the same changelist' '
+	test_when_finished cleanup_git &&
+	(
+		cd "$cli" &&
+		mkdir -p main &&
+
+		p4 edit main/f1 main/f2 &&
+		echo "hello world" >main/f1 &&
+		echo "not in the tag" >main/f2 &&
+		p4 submit -d "main/f[12]: testing two labels" &&
+
+		p4 tag -l tag_f1_1 main/f1 &&
+		p4 tag -l tag_f1_2 main/f1 &&
+
+		p4 labels ... &&
+
+		cd "$git" &&
+		pwd &&
+		"$GITP4" clone --dest=. --detect-labels //depot@all &&
+
+		git tag | grep -q tag_f1_1 &&
+		git tag | grep -q tag_f1_2 &&
+
+		cd main &&
+
+		git checkout tag_tag_f1_1 &&
+		test -f f1 &&
+
+		git checkout tag_tag_f1_1 &&
+		test -f f1 &&
+
+		! test -f f2
+	)
+'
+
+test_expect_success 'kill p4d' '
+	kill_p4d
+'
+
+test_done
-- 
1.7.8.rc1.209.geac91.dirty

Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests

[Vitor Antunes]

Luke Diamand <luke <at> diamand.org> writes:

> In adding the test case for labels I also found and fixed a few other
> small bugs in the label handling:
> 
>  - labels missing a description or "EOT" in their text cause problems;
>  - labels without an owner cause problems.
> 
> I also noticed, but did not fix, that you can't have more than one label
> per commit (the others are silently dropped) and the documentation for
> branch import could be improved. I've added a (failing) test case for
> the multiple label problem.

Hi Luke,

Seeing that you have some experience using labels, could I kindly ask you to
include some description of it in git-p4.txt?

Thanks,
Vitor

Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests

[Luke Diamand]

On 30/11/11 14:55, Vitor Antunes wrote:
> Luke Diamand<luke<at>  diamand.org>  writes:
>
>> In adding the test case for labels I also found and fixed a few other
>> small bugs in the label handling:
>>
>>   - labels missing a description or "EOT" in their text cause problems;
>>   - labels without an owner cause problems.
>>
>> I also noticed, but did not fix, that you can't have more than one label
>> per commit (the others are silently dropped) and the documentation for
>> branch import could be improved. I've added a (failing) test case for
>> the multiple label problem.
>
> Hi Luke,
>
> Seeing that you have some experience using labels, could I kindly ask you to
> include some description of it in git-p4.txt?

OK, if you can help me understand what's going on...

The label-detection bug that I've described, on further investigation, 
looks to be a fundamental limitation.

With perforce, I can check out the head revision, and then tag just a 
single file. If I then check out on that tag, I get just that one file.

I think I can't do that with git; certainly fast-import can't do it.

So the code in git-p4 that is checking the file vs label counts (git-p4 
around line 1496) is actually trying to say "this label can't be 
imported into git".

If my understanding is correct, I can then fix my test and update the 
docs and the code to explain this.

As an aside, git-p4.txt currently has quite good information on the 
config variables, but nothing on the command line variables. Possibly 
that should be fixed.

Cheers!
Luke

Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests

[Vitor Antunes]

On Wed, Nov 30, 2011 at 7:14 PM, Luke Diamand <luke@diamand.org> wrote:
> OK, if you can help me understand what's going on...
>
> The label-detection bug that I've described, on further investigation, looks
> to be a fundamental limitation.
>
> With perforce, I can check out the head revision, and then tag just a single
> file. If I then check out on that tag, I get just that one file.
>
> I think I can't do that with git; certainly fast-import can't do it.
>
> So the code in git-p4 that is checking the file vs label counts (git-p4
> around line 1496) is actually trying to say "this label can't be imported
> into git".
>
> If my understanding is correct, I can then fix my test and update the docs
> and the code to explain this.
>
> As an aside, git-p4.txt currently has quite good information on the config
> variables, but nothing on the command line variables. Possibly that should
> be fixed.

Hey Luke,

I did not have any success in the few times I tried to import labels.
Since you were updating some of that code I was hoping you could
extend git-p4.txt such that I would be able to understand it. This to
say: I can't help you much with my current expertise level on how
labels are implemented. Will need to look into that later.

-- 
Vitor Antunes

Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests

[Pete Wyckoff]

luke@diamand.org wrote on Wed, 30 Nov 2011 19:14 +0000:
> On 30/11/11 14:55, Vitor Antunes wrote:
> >Luke Diamand<luke<at>  diamand.org>  writes:
> >
> >>In adding the test case for labels I also found and fixed a few other
> >>small bugs in the label handling:
> >>
> >>  - labels missing a description or "EOT" in their text cause problems;
> >>  - labels without an owner cause problems.
> >>
> >>I also noticed, but did not fix, that you can't have more than one label
> >>per commit (the others are silently dropped) and the documentation for
> >>branch import could be improved. I've added a (failing) test case for
> >>the multiple label problem.

I was hanging onto your v1, and made a comment on the v1's 3/4
that perhaps you missed.  Also acked the entire thing.  I can
resend if my mailer ate it.

Don't expect Junio to pick it up until after 1.7.8 goes out.

> >Hi Luke,
> >
> >Seeing that you have some experience using labels, could I kindly ask you to
> >include some description of it in git-p4.txt?
> 
> OK, if you can help me understand what's going on...
> 
> The label-detection bug that I've described, on further
> investigation, looks to be a fundamental limitation.
> 
> With perforce, I can check out the head revision, and then tag just
> a single file. If I then check out on that tag, I get just that one
> file.
> 
> I think I can't do that with git; certainly fast-import can't do it.

This is another fundamental disconnect between p4 and git.
Reading

http://www.perforce.com/perforce/doc.current/manuals/p4guide/07_labels.html

it is clear that labels are supposed to be used exactly where
tags cannot:  to specify a collection of files as they existed
at _different_ points in the commit history.

Thus I think supporting labels is kind of pointless.  But in the
restricted use case that perforce docs tell us not to do, namely
using labels to identify change numbers, git can reflect that
with tags.

> So the code in git-p4 that is checking the file vs label counts
> (git-p4 around line 1496) is actually trying to say "this label
> can't be imported into git".
> 
> If my understanding is correct, I can then fix my test and update
> the docs and the code to explain this.

Yeah.  It's just a big restriction on how labels get imported.
A better error message and some docs would be useful.

> As an aside, git-p4.txt currently has quite good information on the
> config variables, but nothing on the command line variables.
> Possibly that should be fixed.

Recently, I wrote an asciidoc-style document for git-p4, and
tried to find all the options on all the commands.  There's a lot
more than I ever knew about.  :)  I'll take another pass
through it then send it out for review.  Maybe we can get rid
of the old git-p4.txt then and work on improving a more
structured document.

		-- Pete

Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests

[Pete Wyckoff]

P.S.  Since you're respinning anyway to change the label code,
can you stick the 'branch with shell char' test from t9801 in
with t9803?  It feels more appropriate there than with the branch
tests.  And avoids collision with some Vitor code that will get
added eventually.

		-- Pete

Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests

[Vitor Antunes]

On Wed, Nov 30, 2011 at 11:00 PM, Pete Wyckoff <pw@padd.com> wrote:
> And avoids collision with some Vitor code that will get
> added eventually.

I'm starting to doubt I will ever be able to overcome the fast-import
limitation on not allowing branch delesetion. Sure, the code I wrote was
garbage! But they seem to be very relunctant on the concept of deleting
branches on the fly.
Did you ever take a look at the patch I sent? Maybe you could help me
shape it up a bit.

-- 
Vitor Antunes

Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests

[Pete Wyckoff]

vitor.hda@gmail.com wrote on Thu, 01 Dec 2011 00:37 +0000:
> On Wed, Nov 30, 2011 at 11:00 PM, Pete Wyckoff <pw@padd.com> wrote:
> > And avoids collision with some Vitor code that will get
> > added eventually.
> 
> I'm starting to doubt I will ever be able to overcome the fast-import
> limitation on not allowing branch delesetion. Sure, the code I wrote was
> garbage! But they seem to be very relunctant on the concept of deleting
> branches on the fly.
> Did you ever take a look at the patch I sent? Maybe you could help me
> shape it up a bit.

I don't think we necessarily need branch deletion inside the
fast-import.  Can you go back and look at my mail from August and
see if that approach is doable?  Just make a single commit on a
throwaway branch with no parent, checkpoint, then do diff-tree
for each potential parent until you find a match.  Do the commit
for real where it goes.  As git-p4 exits, we'll delete the branch
ref of the test commit.

If this works, we can see if fast-import can be taught to
generate a tree object without a commit to save the need for a
temporary branch.

		-- Pete

Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests

[Luke Diamand]

On 30/11/11 23:00, Pete Wyckoff wrote:
> P.S.  Since you're respinning anyway to change the label code,
> can you stick the 'branch with shell char' test from t9801 in
> with t9803?  It feels more appropriate there than with the branch
> tests.  And avoids collision with some Vitor code that will get
> added eventually.
>

OK!

Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests

[Vitor Antunes]

On Wed, Nov 30, 2011 at 10:58 PM, Pete Wyckoff <pw@padd.com> wrote:
> This is another fundamental disconnect between p4 and git.
> Reading
>
> http://www.perforce.com/perforce/doc.current/manuals/p4guide/07_labels.html
>
> it is clear that labels are supposed to be used exactly where
> tags cannot:  to specify a collection of files as they existed
> at _different_ points in the commit history.

Check the "Use Tag Fixup Branches" section in fast-import manual, it
might help on this. The basic concept is to create a special branch
that puts all files in the same state the P4 label would put them and
then tag it in git.

Tried to use this for my branch stuff, but with no success.

> Thus I think supporting labels is kind of pointless.  But in the
> restricted use case that perforce docs tell us not to do, namely
> using labels to identify change numbers, git can reflect that
> with tags.

I still use labels as simple tags. Telling that we should use
changelists instead of labels is the same as saying that we should use
IP addresses instead of host names. It works, but I doubt you will
ever remember it unless you write it down somewhere.

-- 
Vitor Antunes

Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests

[Pete Wyckoff]

vitor.hda@gmail.com wrote on Thu, 01 Dec 2011 00:33 +0000:
> On Wed, Nov 30, 2011 at 10:58 PM, Pete Wyckoff <pw@padd.com> wrote:
> > This is another fundamental disconnect between p4 and git.
> > Reading
> >
> > http://www.perforce.com/perforce/doc.current/manuals/p4guide/07_labels.html
> >
> > it is clear that labels are supposed to be used exactly where
> > tags cannot:  to specify a collection of files as they existed
> > at _different_ points in the commit history.
> 
> Check the "Use Tag Fixup Branches" section in fast-import manual, it
> might help on this. The basic concept is to create a special branch
> that puts all files in the same state the P4 label would put them and
> then tag it in git.
> 
> Tried to use this for my branch stuff, but with no success.

Interesting, thanks.  One could certainly construct any arbitrary
tree to represent the tag.  But it would be a truly evil merge of
possibly many commits.

> > Thus I think supporting labels is kind of pointless.  But in the
> > restricted use case that perforce docs tell us not to do, namely
> > using labels to identify change numbers, git can reflect that
> > with tags.
> 
> I still use labels as simple tags. Telling that we should use
> changelists instead of labels is the same as saying that we should use
> IP addresses instead of host names. It works, but I doubt you will
> ever remember it unless you write it down somewhere.

I see your point.  P4 labels are the only way that they support
tagging, apparently.  I'm okay with leaving label support in
git-p4.  And it will be nice if Luke makes it behave a bit
better.  But doing heroics to emulate cross-commit tags feels
like a lot of work, and the wrong direction.

		-- Pete

Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests

[Vitor Antunes]

On Dec 1, 2011 4:03 AM, "Pete Wyckoff" <pw@padd.com> wrote:
> I see your point.  P4 labels are the only way that they support
> tagging, apparently.  I'm okay with leaving label support in
> git-p4.  And it will be nice if Luke makes it behave a bit
> better.  But doing heroics to emulate cross-commit tags feels
> like a lot of work, and the wrong direction.

Agreed. Lets keep it simple.

-- 
Vitor Antunes

Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests

[Luke Diamand]

On 01/12/11 21:59, Vitor Antunes wrote:
> On Dec 1, 2011 4:03 AM, "Pete Wyckoff"<pw@padd.com>  wrote:
>> I see your point.  P4 labels are the only way that they support
>> tagging, apparently.  I'm okay with leaving label support in
>> git-p4.  And it will be nice if Luke makes it behave a bit
>> better.  But doing heroics to emulate cross-commit tags feels
>> like a lot of work, and the wrong direction.
>
> Agreed. Lets keep it simple.
>

I think I'm going to have to go away and do a bit more work on this. The 
existing label code is still quite buggy (or my understanding is 
broken). Either way I'd rather get it to the point where it actually 
works and passes all its tests.

The two issues I'm seeing are:

- two p4 labels covering the same set of files; only one of them gets 
imported.

- if you have a p4 label on a subset of files then it gets dropped 
(which is fine) but so do most of the other labels (as far as I can tell).

I think if this could be made to work it would actually be really useful 
though.

(Pete - I've found your previous email; not sure why I didn't see it 
before. I'll roll that change in with what I'm doing).


Regards!
Luke