* [PATCHv1 0/3] git-p4: fixing import with labels when commit is not transferred
@ 2015-08-27 7:18 Luke Diamand
2015-08-27 7:18 ` [PATCHv1 1/3] git-p4: failing test for ignoring invalid p4 labels Luke Diamand
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Luke Diamand @ 2015-08-27 7:18 UTC (permalink / raw
To: git; +Cc: marcus.holl, Luke Diamand
Marcus Holl discovered that git-p4 fails to import labels sometimes.
http://permalink.gmane.org/gmane.comp.version-control.git/273034
It turns out there are at least two related problems.
The first is that if git-p4 tries to import a P4 tag which references
a commit that we don't know about at all (e.g. because the P4
changelist was created before the point at which the repo was
cloned) then when it tries to find the commit, the git rev-list
command fails, and git-p4 terminates.
This can be fixed by detecting the problem and ignoring that label.
The second problem is that git-p4 can end up trying to find out
about a commit which is still in the git fast-import stream and not
yet processed. This is fixed by using fast-import "marks" to reference
these commits, which avoids having to query for commits which don't
yet exist. The normal path is used for commits that have not been
imported during this run of the program (and so won't have marks).
A test case is added to demonstrate the problem.
Marcus - if you're able to take a look at this and see if it
fixes your problem that would be very useful.
Thanks,
Luke
Luke Diamand (3):
git-p4: failing test for ignoring invalid p4 labels
git-p4: do not terminate creating tag for unknown commit
git-p4: fix P4 label import for unprocessed commits
git-p4.py | 25 +++++++++++++++--------
t/t9811-git-p4-label-import.sh | 45 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 62 insertions(+), 8 deletions(-)
--
2.5.0.rc0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCHv1 1/3] git-p4: failing test for ignoring invalid p4 labels
2015-08-27 7:18 [PATCHv1 0/3] git-p4: fixing import with labels when commit is not transferred Luke Diamand
@ 2015-08-27 7:18 ` Luke Diamand
2015-08-27 12:53 ` Eric Sunshine
2015-08-27 7:18 ` [PATCHv1 2/3] git-p4: do not terminate creating tag for unknown commit Luke Diamand
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Luke Diamand @ 2015-08-27 7:18 UTC (permalink / raw
To: git; +Cc: marcus.holl, Luke Diamand
When importing a label which references a commit that git-p4 does
not know about, git-p4 should skip it and go on to process other
labels that can be imported.
Instead it crashes when attempting to find the missing commit in
the git history. This test demonstrates the problem.
Signed-off-by: Luke Diamand <luke@diamand.org>
---
t/t9811-git-p4-label-import.sh | 45 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/t/t9811-git-p4-label-import.sh b/t/t9811-git-p4-label-import.sh
index 095238f..f7d5048 100755
--- a/t/t9811-git-p4-label-import.sh
+++ b/t/t9811-git-p4-label-import.sh
@@ -214,6 +214,51 @@ test_expect_success 'use git config to enable import/export of tags' '
)
'
+p4_head_revision() {
+ p4 changes -m 1 "$@" | awk '{print $2}'
+}
+
+# Importing a label that references a P4 commit that has
+# has not been seen. The presence of a label on a commit
+# we haven't seen should not cause git-p4 to fail. It should
+# merely skip that label, and still import other labels.
+test_expect_failure 'importing labels with missing revisions' '
+ test_when_finished cleanup_git &&
+ (
+ rm -fr "$cli" "$git" &&
+ mkdir "$cli" &&
+ P4CLIENT=missing-revision &&
+ client_view "//depot/missing-revision/... //missing-revision/..." &&
+ cd "$cli" &&
+ >f1 && p4 add f1 && p4 submit -d "start" &&
+
+ p4 tag -l TAG_S0 ... &&
+
+ >f2 && p4 add f2 && p4 submit -d "second" &&
+
+ startrev=$(p4_head_revision //depot/missing-revision/...) &&
+
+ >f3 && p4 add f3 && p4 submit -d "third" &&
+
+ p4 edit f2 && date >f2 && p4 submit -d "change" f2 &&
+
+ endrev=$(p4_head_revision //depot/missing-revision/...) &&
+
+ p4 tag -l TAG_S1 ... &&
+
+ # we should skip TAG_S0 since it is before our startpoint,
+ # but pick up TAG_S1.
+
+ git p4 clone --dest="$git" --import-labels -v \
+ //depot/missing-revision/...@$startrev,$endrev &&
+ (
+ cd "$git" &&
+ git rev-parse TAG_S1 &&
+ ! git rev-parse TAG_S0
+ )
+ )
+'
+
test_expect_success 'kill p4d' '
kill_p4d
--
2.5.0.rc0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCHv1 2/3] git-p4: do not terminate creating tag for unknown commit
2015-08-27 7:18 [PATCHv1 0/3] git-p4: fixing import with labels when commit is not transferred Luke Diamand
2015-08-27 7:18 ` [PATCHv1 1/3] git-p4: failing test for ignoring invalid p4 labels Luke Diamand
@ 2015-08-27 7:18 ` Luke Diamand
2015-08-27 7:18 ` [PATCHv1 3/3] git-p4: fix P4 label import for unprocessed commits Luke Diamand
2015-09-14 6:45 ` [PATCHv1 0/3] git-p4: fixing import with labels when commit is not transferred Holl, Marcus
3 siblings, 0 replies; 6+ messages in thread
From: Luke Diamand @ 2015-08-27 7:18 UTC (permalink / raw
To: git; +Cc: marcus.holl, Luke Diamand
If p4 reports a tag for a commit that git-p4 does not know
about (e.g. because it references a P4 changelist that was
imported prior to the point at which the repo was cloned into
git), make sure that the error is correctly caught and handled.
rather than just crashing.
Signed-off-by: Luke Diamand <luke@diamand.org>
---
git-p4.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 073f87b..a62611a 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2494,9 +2494,9 @@ class P4Sync(Command, P4UserMap):
# find the corresponding git commit; take the oldest commit
changelist = int(change['change'])
gitCommit = read_pipe(["git", "rev-list", "--max-count=1",
- "--reverse", ":/\[git-p4:.*change = %d\]" % changelist])
+ "--reverse", ":/\[git-p4:.*change = %d\]" % changelist], ignore_error=True)
if len(gitCommit) == 0:
- print "could not find git commit for changelist %d" % changelist
+ print "importing label %s: could not find git commit for changelist %d" % (name, changelist)
else:
gitCommit = gitCommit.strip()
commitFound = True
--
2.5.0.rc0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCHv1 3/3] git-p4: fix P4 label import for unprocessed commits
2015-08-27 7:18 [PATCHv1 0/3] git-p4: fixing import with labels when commit is not transferred Luke Diamand
2015-08-27 7:18 ` [PATCHv1 1/3] git-p4: failing test for ignoring invalid p4 labels Luke Diamand
2015-08-27 7:18 ` [PATCHv1 2/3] git-p4: do not terminate creating tag for unknown commit Luke Diamand
@ 2015-08-27 7:18 ` Luke Diamand
2015-09-14 6:45 ` [PATCHv1 0/3] git-p4: fixing import with labels when commit is not transferred Holl, Marcus
3 siblings, 0 replies; 6+ messages in thread
From: Luke Diamand @ 2015-08-27 7:18 UTC (permalink / raw
To: git; +Cc: marcus.holl, Luke Diamand
With --detect-labels enabled, git-p4 will try to create tags
using git fast-import by writing a "tag" clause to the
fast-import stream.
If the commit that the tag references has not yet actually
been processed by fast-import, then the tag can't be created
and git-p4 fails to import the P4 label.
Teach git-p4 to use fast-import "marks" when creating tags
which reference commits created during the current run of the
program.
Commits created before the current run are still referenced
in the old way using a normal git commit.
Signed-off-by: Luke Diamand <luke@diamand.org>
---
git-p4.py | 25 +++++++++++++++++--------
t/t9811-git-p4-label-import.sh | 2 +-
2 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index a62611a..2018011 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2322,8 +2322,11 @@ class P4Sync(Command, P4UserMap):
else:
return "%s <a@b>" % userid
- # Stream a p4 tag
def streamTag(self, gitStream, labelName, labelDetails, commit, epoch):
+ """ Stream a p4 tag.
+ commit is either a git commit, or a fast-import mark, ":<p4commit>"
+ """
+
if verbose:
print "writing tag %s for commit %s" % (labelName, commit)
gitStream.write("tag %s\n" % labelName)
@@ -2374,7 +2377,7 @@ class P4Sync(Command, P4UserMap):
self.clientSpecDirs.update_client_spec_path_cache(files)
self.gitStream.write("commit %s\n" % branch)
-# gitStream.write("mark :%s\n" % details["change"])
+ self.gitStream.write("mark :%s\n" % details["change"])
self.committedChanges.add(int(details["change"]))
committer = ""
if author not in self.users:
@@ -2493,13 +2496,19 @@ class P4Sync(Command, P4UserMap):
if change.has_key('change'):
# find the corresponding git commit; take the oldest commit
changelist = int(change['change'])
- gitCommit = read_pipe(["git", "rev-list", "--max-count=1",
- "--reverse", ":/\[git-p4:.*change = %d\]" % changelist], ignore_error=True)
- if len(gitCommit) == 0:
- print "importing label %s: could not find git commit for changelist %d" % (name, changelist)
- else:
- gitCommit = gitCommit.strip()
+ if changelist in self.committedChanges:
+ gitCommit = ":%d" % changelist # use a fast-import mark
commitFound = True
+ else:
+ gitCommit = read_pipe(["git", "rev-list", "--max-count=1",
+ "--reverse", ":/\[git-p4:.*change = %d\]" % changelist], ignore_error=True)
+ if len(gitCommit) == 0:
+ print "importing label %s: could not find git commit for changelist %d" % (name, changelist)
+ else:
+ commitFound = True
+ gitCommit = gitCommit.strip()
+
+ if commitFound:
# Convert from p4 time format
try:
tmwhen = time.strptime(labelDetails['Update'], "%Y/%m/%d %H:%M:%S")
diff --git a/t/t9811-git-p4-label-import.sh b/t/t9811-git-p4-label-import.sh
index f7d5048..f994640 100755
--- a/t/t9811-git-p4-label-import.sh
+++ b/t/t9811-git-p4-label-import.sh
@@ -222,7 +222,7 @@ p4_head_revision() {
# has not been seen. The presence of a label on a commit
# we haven't seen should not cause git-p4 to fail. It should
# merely skip that label, and still import other labels.
-test_expect_failure 'importing labels with missing revisions' '
+test_expect_success 'importing labels with missing revisions' '
test_when_finished cleanup_git &&
(
rm -fr "$cli" "$git" &&
--
2.5.0.rc0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCHv1 1/3] git-p4: failing test for ignoring invalid p4 labels
2015-08-27 7:18 ` [PATCHv1 1/3] git-p4: failing test for ignoring invalid p4 labels Luke Diamand
@ 2015-08-27 12:53 ` Eric Sunshine
0 siblings, 0 replies; 6+ messages in thread
From: Eric Sunshine @ 2015-08-27 12:53 UTC (permalink / raw
To: Luke Diamand; +Cc: Git List, marcus.holl
On Thu, Aug 27, 2015 at 3:18 AM, Luke Diamand <luke@diamand.org> wrote:
> When importing a label which references a commit that git-p4 does
> not know about, git-p4 should skip it and go on to process other
> labels that can be imported.
>
> Instead it crashes when attempting to find the missing commit in
> the git history. This test demonstrates the problem.
>
> Signed-off-by: Luke Diamand <luke@diamand.org>
> ---
> diff --git a/t/t9811-git-p4-label-import.sh b/t/t9811-git-p4-label-import.sh
> index 095238f..f7d5048 100755
> --- a/t/t9811-git-p4-label-import.sh
> +++ b/t/t9811-git-p4-label-import.sh
> @@ -214,6 +214,51 @@ test_expect_success 'use git config to enable import/export of tags' '
> )
> '
>
> +p4_head_revision() {
> + p4 changes -m 1 "$@" | awk '{print $2}'
> +}
> +
> +# Importing a label that references a P4 commit that has
> +# has not been seen. The presence of a label on a commit
s/has has/has/
> +# we haven't seen should not cause git-p4 to fail. It should
> +# merely skip that label, and still import other labels.
> +test_expect_failure 'importing labels with missing revisions' '
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCHv1 0/3] git-p4: fixing import with labels when commit is not transferred
2015-08-27 7:18 [PATCHv1 0/3] git-p4: fixing import with labels when commit is not transferred Luke Diamand
` (2 preceding siblings ...)
2015-08-27 7:18 ` [PATCHv1 3/3] git-p4: fix P4 label import for unprocessed commits Luke Diamand
@ 2015-09-14 6:45 ` Holl, Marcus
3 siblings, 0 replies; 6+ messages in thread
From: Holl, Marcus @ 2015-09-14 6:45 UTC (permalink / raw
To: Luke Diamand, git@vger.kernel.org
Hi Luke,
>Marcus - if you're able to take a look at this and see if it
>fixes your problem that would be very useful.
Fix works for me.
Thanks a lot for fixing this issue.
Best regards,
Marcus
On 27/08/15 09:18, "Luke Diamand" <luke@diamand.org> wrote:
>Marcus Holl discovered that git-p4 fails to import labels sometimes.
>
>http://permalink.gmane.org/gmane.comp.version-control.git/273034
>
>It turns out there are at least two related problems.
>
>The first is that if git-p4 tries to import a P4 tag which references
>a commit that we don't know about at all (e.g. because the P4
>changelist was created before the point at which the repo was
>cloned) then when it tries to find the commit, the git rev-list
>command fails, and git-p4 terminates.
>
>This can be fixed by detecting the problem and ignoring that label.
>
>The second problem is that git-p4 can end up trying to find out
>about a commit which is still in the git fast-import stream and not
>yet processed. This is fixed by using fast-import "marks" to reference
>these commits, which avoids having to query for commits which don't
>yet exist. The normal path is used for commits that have not been
>imported during this run of the program (and so won't have marks).
>
>A test case is added to demonstrate the problem.
>
>Marcus - if you're able to take a look at this and see if it
>fixes your problem that would be very useful.
>
>Thanks,
>Luke
>
>Luke Diamand (3):
> git-p4: failing test for ignoring invalid p4 labels
> git-p4: do not terminate creating tag for unknown commit
> git-p4: fix P4 label import for unprocessed commits
>
> git-p4.py | 25 +++++++++++++++--------
> t/t9811-git-p4-label-import.sh | 45
>++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 62 insertions(+), 8 deletions(-)
>
>--
>2.5.0.rc0
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-09-14 6:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-27 7:18 [PATCHv1 0/3] git-p4: fixing import with labels when commit is not transferred Luke Diamand
2015-08-27 7:18 ` [PATCHv1 1/3] git-p4: failing test for ignoring invalid p4 labels Luke Diamand
2015-08-27 12:53 ` Eric Sunshine
2015-08-27 7:18 ` [PATCHv1 2/3] git-p4: do not terminate creating tag for unknown commit Luke Diamand
2015-08-27 7:18 ` [PATCHv1 3/3] git-p4: fix P4 label import for unprocessed commits Luke Diamand
2015-09-14 6:45 ` [PATCHv1 0/3] git-p4: fixing import with labels when commit is not transferred Holl, Marcus
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).