last-note sometimes points to a non existing mark

[felipec]

This is a continuation from issue #4.

Here's a script that reproduces the issue:

#!/bin/sh

rm -rf test_hg test_git

(
hg init test_hg
cd test_hg
echo A >> content
hg add content
hg commit -m A
)

(
git clone hg::test_hg test_git
cd test_git

echo B >> content
git commit -a -m B
git push
git fetch
)

(
cd test_hg
hg strip tip
hg log
)

(
cd test_git
git log
git fetch
git commit --amend --message="B"
git reflog expire --expire=now --all
git gc --prune=now
git fetch
git fetch
)

[felipec]

So I've found the initial issue: Git dies while parsing the marks file that contains the missing Git object, and after that, it truncates the marks file.

This patch fixes the issue:

--- a/fast-import.c
+++ b/fast-import.c
@@ -1855,8 +1855,10 @@ static void read_marks(void)
                e = find_object(sha1);
                if (!e) {
                        enum object_type type = sha1_object_info(sha1, NULL);
-                       if (type < 0)
-                               die("object not found: %s", sha1_to_hex(sha1));
+                       if (type < 0) {
+                               warning("object not found: %s", sha1_to_hex(sha1));
+                               continue;
+                       }
                        e = insert_object(sha1);
                        e->type = type;
                        e->pack_id = MAX_PACK_ID;

[felipec]

Another issue is that my git marks-check script is stripping notes marks, which might or might not have affected people. I've fixed it now.

I've also updated it to detect missing Git objects. So in theory if you always run marks-check before doing a git fetch you wouldn't have any problems, because it will fix marks-git before git fast-import has a chance of crashing.

git fast-export has a similar issue, but at least it doesn't truncate the marks file.

[felipec]

I've sent the initial patch upstream. However, that's not going to be enough.

[felipec]

Here's a hack that can be applied directly in git remote-hg to make sure the last_note is always correct, even if git fast-import crashes.

--- a/git-remote-hg
+++ b/git-remote-hg
@@ -164,6 +164,16 @@ class Marks:
                 'last-note': self.last_note }

     def store(self):
+        found = False
+        path = os.path.join(dirname, 'marks-git')
+        with open(path, "r") as f:
+            for line in f:
+                if line.startswith(':%u ' % self.last_note):
+                    found = True
+                    break
+        if not found:
+            warn("Last note note found")
+            self.last_note = None
         json.dump(self.dict(), open(self.path, 'w'))

     def __str__(self):

[fgb]

Thanks for looking into this. Are you going to apply the above patch to git-remote-hg in case upstream takes a while to apply your fix?

[felipec]

Probably not. I'm focusing on updating my own fork of Git, where everything should work fine.

After that I will write test cases that exploit the corner cases we have found, and some of the checks of git marks-check integrated directly into git remote-hg, but enabled only optionally. Again--later.

[fgb]

Sounds good. I'll patch my local copy of git-remote-hg for the time being, then. Thanks!

[mnauw]

Ouch. This "fix" sets last_note to None, which is a sure fire way to reproduce issue #58 (wiping out the current notes hierarchy upon a subsequent fetch). The commits fixing that issue avoid the need for last_note altogether (and make git-remote-hg more independent of what may or may not be a problem in Git).

[felipec]

Thanks for looking into this. Are you going to apply the above patch to git-remote-hg in case upstream takes a while to apply your fix?

Well there was one fix actually applied in Git v2.9. At least the test above doesn't fail for me any more.