# [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests 17 messages from 2011-11-30 to 2011-12-04. Participants: Luke Diamand, Vitor Antunes, Pete Wyckoff. Thread: https://gitlist.dev/t/29053 ## Luke Diamand, 2011-11-30 09:03 Subject: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests Message-ID: <1322643817-13051-1-git-send-email-luke@diamand.org> URL: https://gitlist.dev/e/1322643817-13051-1-git-send-email-luke%40diamand.org ``` 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 ``` ## Luke Diamand, 2011-11-30 09:03 Subject: [PATCHv2 1/4] git-p4: handle p4 branches and labels containing shell chars Message-ID: <1322643817-13051-2-git-send-email-luke@diamand.org> URL: https://gitlist.dev/e/1322643817-13051-2-git-send-email-luke%40diamand.org In-Reply-To: <1322643817-13051-1-git-send-email-luke@diamand.org> ``` 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 --- 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 ``` ## Luke Diamand, 2011-11-30 09:03 Subject: [PATCHv2 2/4] git-p4: cope with labels with empty descriptions Message-ID: <1322643817-13051-3-git-send-email-luke@diamand.org> URL: https://gitlist.dev/e/1322643817-13051-3-git-send-email-luke%40diamand.org In-Reply-To: <1322643817-13051-1-git-send-email-luke@diamand.org> ``` 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 --- 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 %s %s" % (owner, epoch, self.tz) self.gitStream.write("tagger %s\n" % tagger) - self.gitStream.write("data < URL: https://gitlist.dev/e/1322643817-13051-4-git-send-email-luke%40diamand.org In-Reply-To: <1322643817-13051-1-git-send-email-luke@diamand.org> ``` In p4, the Owner field is optional. If it is missing, construct something sensible rather than crashing. Signed-off-by: Luke Diamand --- 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 %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 %s %s" % (owner, epoch, self.tz) -- 1.7.8.rc1.209.geac91.dirty ``` ## Luke Diamand, 2011-11-30 09:03 Subject: [PATCHv2 4/4] git-p4: add test for p4 labels Message-ID: <1322643817-13051-5-git-send-email-luke@diamand.org> URL: https://gitlist.dev/e/1322643817-13051-5-git-send-email-luke%40diamand.org In-Reply-To: <1322643817-13051-1-git-send-email-luke@diamand.org> ``` 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 --- 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 ``` ## Vitor Antunes, 2011-11-30 14:55 Subject: Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests Message-ID: URL: https://gitlist.dev/e/loom.20111130T155409-599%40post.gmane.org In-Reply-To: <1322643817-13051-1-git-send-email-luke@diamand.org> ``` Luke Diamand 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 ``` ## Luke Diamand, 2011-11-30 19:14 Subject: Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests Message-ID: <4ED6809A.9020703@diamand.org> URL: https://gitlist.dev/e/4ED6809A.9020703%40diamand.org In-Reply-To: ``` On 30/11/11 14:55, Vitor Antunes wrote: > Luke Diamand 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 ``` ## Vitor Antunes, 2011-11-30 19:44 Subject: Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests Message-ID: URL: https://gitlist.dev/e/CAOpHH-XQdvdPtj%3DbCpyFVOLOCPf6%2BYnEUvk2pLh4sVp0XfvLTw%40mail.gmail.com In-Reply-To: <4ED6809A.9020703@diamand.org> ``` On Wed, Nov 30, 2011 at 7:14 PM, Luke Diamand 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 ``` ## Pete Wyckoff, 2011-11-30 22:58 Subject: Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests Message-ID: <20111130225813.GA11544@arf.padd.com> URL: https://gitlist.dev/e/20111130225813.GA11544%40arf.padd.com In-Reply-To: <4ED6809A.9020703@diamand.org> ``` luke@diamand.org wrote on Wed, 30 Nov 2011 19:14 +0000: > On 30/11/11 14:55, Vitor Antunes wrote: > >Luke Diamand 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 ``` ## Pete Wyckoff, 2011-11-30 23:00 Subject: Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests Message-ID: <20111130230007.GA11598@arf.padd.com> URL: https://gitlist.dev/e/20111130230007.GA11598%40arf.padd.com In-Reply-To: <20111130225813.GA11544@arf.padd.com> ``` 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 ``` ## Vitor Antunes, 2011-12-01 00:33 Subject: Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests Message-ID: URL: https://gitlist.dev/e/CAOpHH-U3PF0v7EPqnO0FNxNKh%2BuF1GH%3DcnA_MA09SaQTxo0cDA%40mail.gmail.com In-Reply-To: <20111130225813.GA11544@arf.padd.com> ``` On Wed, Nov 30, 2011 at 10:58 PM, Pete Wyckoff 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 ``` ## Vitor Antunes, 2011-12-01 00:37 Subject: Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests Message-ID: URL: https://gitlist.dev/e/CAOpHH-U6NxRSioRZg9_%2Bf146vVR%2BS1hWsVbRmHz%2Bvsqtz%2BvXiA%40mail.gmail.com In-Reply-To: <20111130230007.GA11598@arf.padd.com> ``` On Wed, Nov 30, 2011 at 11:00 PM, Pete Wyckoff 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 ``` ## Pete Wyckoff, 2011-12-01 04:02 Subject: Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests Message-ID: <20111201040259.GA22189@arf.padd.com> URL: https://gitlist.dev/e/20111201040259.GA22189%40arf.padd.com In-Reply-To: ``` vitor.hda@gmail.com wrote on Thu, 01 Dec 2011 00:33 +0000: > On Wed, Nov 30, 2011 at 10:58 PM, Pete Wyckoff 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 ``` ## Luke Diamand, 2011-12-01 08:31 Subject: Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests Message-ID: <4ED73B67.6080706@diamand.org> URL: https://gitlist.dev/e/4ED73B67.6080706%40diamand.org In-Reply-To: <20111130230007.GA11598@arf.padd.com> ``` 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! ``` ## Vitor Antunes, 2011-12-01 21:59 Subject: Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests Message-ID: URL: https://gitlist.dev/e/CAOpHH-XL5OGpnihEgqnXqUUFsMxXn2wSdLadegnC1epg44vs8A%40mail.gmail.com In-Reply-To: ``` On Dec 1, 2011 4:03 AM, "Pete Wyckoff" 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 ``` ## Luke Diamand, 2011-12-02 08:49 Subject: Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests Message-ID: <4ED89115.6040309@diamand.org> URL: https://gitlist.dev/e/4ED89115.6040309%40diamand.org In-Reply-To: ``` On 01/12/11 21:59, Vitor Antunes wrote: > On Dec 1, 2011 4:03 AM, "Pete Wyckoff" 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 ``` ## Pete Wyckoff, 2011-12-04 16:07 Subject: Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests Message-ID: <20111204160739.GA13549@arf.padd.com> URL: https://gitlist.dev/e/20111204160739.GA13549%40arf.padd.com In-Reply-To: ``` vitor.hda@gmail.com wrote on Thu, 01 Dec 2011 00:37 +0000: > On Wed, Nov 30, 2011 at 11:00 PM, Pete Wyckoff 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 ```