Notes: push and then pull fails

[fingolfin]

Using this simple test script, I can trigger an error in the new notes code:

#!/bin/sh -ex
hg init repo_orig
cd repo_orig
echo a > a
hg add a
hg commit -m a
cd ..

git clone gitifyhg::repo_orig repo_clone

cd repo_clone
echo b > a
git ci -m b a
git push
git pull

The final pull yields this error over here:

   42c68b7..760821e  branches/default -> origin/branches/default
error: refs/notes/hg-origin does not point to a valid object!

[fingolfin]

Indeed, it is not even clear to me that refs/notes/FOO is a valid commit target. Looking at the git test suite, their refs commits always target refs/heads/FOO (specifically, FOO = S or Snotes. Interesting... Perhaps it needs to be added to the refspec at the start? Huh... ? Now I am not sure anymore I even understand how notes are supposed to work. Back to reading...

[jedbrown]

@fingolfin See t9301-fast-import-notes.sh, committing directly to refs/notes/*. I don't know what's going wrong here and haven't figured out what it thinks is an invalid object.

[fingolfin]

You are right of course. Phew. Actually, I could have sworn I saw similar commits in the git tests when I hacked on my own notes PR, but earlier today I somehow overlooked it. Silly.

Anyway, I am wondering if the problem could be that in this pull, the notes commit is the only object in the fast-import stream. Indeed, if I modify the test script to ensure that some other data is pulled, the error goes away:

#!/bin/sh -ex
hg init repo_orig
cd repo_orig
echo a > a
hg add a
hg commit -m a
cd ..

git clone gitifyhg::repo_orig repo_clone

cd repo_clone
echo b > a
git ci -m b a
git push
cd ..

cd repo_orig
hg update
echo c > a
hg commit -m c
cd ..

cd repo_clone
git pull

If you comment out the single line hg commit -m c, the error returns.

[fingolfin]

OK, before I fall over dead-tired, some last remarks: We receive the command "import refs/heads/branches/default" and reply with a commit to refs/notes... I wonder if that is part of the problem?

DEBUG: 'INPUT: import refs/heads/branches/default'
DEBUG: 'OUT: feature done'
DEBUG: 'OUT: feature import-marks=.git/hg/f1421126ba1e51f053fc6dc989558c3804ca8401/marks-git'
DEBUG: 'OUT: feature export-marks=.git/hg/f1421126ba1e51f053fc6dc989558c3804ca8401/marks-git'
DEBUG: 'OUT: feature notes'
DEBUG: 'OUT: reset refs/hg/origin/branches/default'
DEBUG: 'OUT: from :4'
DEBUG: 'OUT: '
DEBUG: 'OUT: commit refs/notes/hg-f1421126ba1e51f053fc6dc989558c3804ca8401'
DEBUG: 'OUT: mark :5'

The only difference with the extra hg commit added is that before the rest, we also output a commit refs/hg/origin/branches/default. But I don't see why it should matter.. but then my brain is already a bit mushy, I should get some sleep now ;-).

[fingolfin]

Ahhh, one last remark: @jedbrown t/t9301-fast-import-notes.sh has this cool trick: from refs/notes/test^0. Could this be the almost trivial answer to our notes update problem? It seems to easy to be true... but... Ah well, I guess we should just try it (and add some test cases for multiple remotes).

[jedbrown]

Yeah, I also noticed that trick this afternoon. I guess since these should never conflict, it may be okay. It worked in my simple test (though I keep the mark to know which revisions have not yet been tagged). I'm not convinced that it's really race-free, however. I doubt fast-import is really taking a lock for every ref update, which I think is the main reason the docs say to always use refspec to write to a private ref.

Good observation about the lone notes commit. I'm somewhat doubtful that failing is really the intended behavior.

Also, implementing fetch rather than import may be part of the answer to our performance woes. I was worried about races, but as long as we only write directly to private refs, it should be okay. Somewhat more code, however, or a dependency on dulwich.