dusty-phillips / gitifyhg

Tools for using git as a client to mercurial repositories
GNU General Public License v3.0
62 stars 17 forks source link

crash upon clone: local variable 'rev' referenced before assignment #13

Closed fingolfin closed 11 years ago

fingolfin commented 11 years ago

I just tried to clone a hg repository upon which gitifyhg crashed. The last lines of the stack trace:

Debug: Read ref listing.
Debug: Remote helper: -> import HEAD
Debug: Remote helper: -> import refs/heads/branches/default
Debug: Remote helper: -> import refs/heads/master
Debug: Remote helper: -> import refs/heads/remote/master
Debug: Remote helper: -> 
Traceback (most recent call last):
...
  File "/sw/lib/python2.7/site-packages/gitifyhg-0.6.3-py2.7.egg/gitifyhg.py", line 483, in process_ref
    output("from :%u" % self.marks.revision_to_mark(rev))
UnboundLocalError: local variable 'rev' referenced before assignment

Indeed, this access to rev there looks suspicious; rev would not be assigned if revs is empty. And looking at the output, what happens is that in my local clone, I have two (outdated) bookmarks, and no branches:

$ hg branches
default                       79:b1dedc28570a
$ hg bookmarks
   master                    24:ec58b58cfd7a
   remote/master             24:ec58b58cfd7a

Now, I couldn't reproduce the crash exactly with a "fresh" repo just made for this purpose. But I think it is sufficiently clear that something is wrong here. I am just not quite sure how to fix it.

dusty-phillips commented 11 years ago

Can you also supply the output of hg heads? I wonder if it didn't fail on an anonymous head and pick the "wrong tip"

fingolfin commented 11 years ago

There is just one head:

$ hg heads
changeset:   79:b1dedc28570a
tag:         tip
user:        Max Horn <max@quendi.de>
date:        Mon Aug 20 23:48:44 2012 +0200
summary:     Correct broken gl-2-p.g files

And the HG repo is checked out at that tip. But as I described above, there are two bookmarks pointing to the same old commit, and gitifyhg errors out precisely in the moment it is asked to export the second of these two bookmarks...

fingolfin commented 11 years ago

The "two bookmark reference the same rev" was a red herring. The true problem is that I have a bookmark named "master", which does not point to tip. This clashes with the built-in hack for the fake "master" bookmark. Here is a script to reproduce the error (should be turned into a test for the test suite, I guess?)

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

git clone gitifyhg::repo_orig repo_clone

I guess this could be used as yet another argument for prefixing hg bookmarks with "bookmarks/"...

dusty-phillips commented 11 years ago

When possible, I like to document TODOs with failing tests. If you convert it to a test (it ought to be really easy if you follow the existing ones as examples, it uses the sh module) and mark it as pytest.xfail and a comment referencing lucky #13, I'll pull it in.

I agree, there appear to be more and more reasons to prefix bookmarks with hg. I created #15 to implement this.

fingolfin commented 11 years ago

I actually much prefer the shell based tests used by git and felipe's remote-hg -- because I can easily share those tests between gitifyhg, felipe's remote-hg and the one by the msysgit folks. Makes it quite handy when comparing what works in which of them. I don't quite see the point in converting ("obfuscating" ?) shell scripts into python code... but that's just me :-).

dusty-phillips commented 11 years ago

The shell tests never worked well for me so I went with what I know. Mostly, though I just like what py.test gives me when things go wrong.

If you don't feel like converting them, I'm sure I'll get around to it eventually, but my focus is always on those issues that are currently ruining my life. My work's repos don't use bookmarks, so this one isn't my highest priority. Once gitifyhg is politely doing everything I need it to, I'll expand to other people's problems, but pull requests are the best way to get things changed.

fingolfin commented 11 years ago

Sure, of course, perfectly understandable. And just to make it clear: I do not expect you to slave away on all my complaints and ideas. I just log them to make them known, and in the hope that somebody (me, you, or a 3rd party) might be sharing the problem or liking the idea, and willing to work on it, one day... I know how it is to work on an open source projects, and users can somebody be terribly annoying, esp. if they have an "but this is a major problem for me, I can't believe you are so lazy and not fixing it right away!" attitude. But I assure you I don't have such crazy expectations ;-).

And this test and the python test format are both simple enough, so I might make a pull request anyway. Just saying I like the shell tests because they make it easy for me to point out to me and others things that work in gitifyhg but not in the others (or vice versa :-).

Thanks for your work, btw!

jedbrown commented 11 years ago

Regarding "sharing" tests using py.test2, I just set a path with a symlink from git-remote-gitifyhg to some other git-remote-hg and run the test suite. I'm also not convinced of the value of py.test "obfuscation". I seem to get traces coming from the test harness more often than something interesting, but the keyword matching is useful.

fingolfin commented 11 years ago

Yeah, I find py.test and sh output quite annoying -- a single failure causes pages upon pages of output, of which usually just a 2-3 lines in the middle are relevant, and these then sometimes even get cut-off due to sh limiting output of errors / exceptions it caught very strictly...

Anyway running the test suite against other remote helpers is not that helpful right now due to the suite relyong on various gitifyhg idiosyncrasies, such as the specific way notes are used...

dusty-phillips commented 11 years ago

run py.test with --tb=short to get slightly more useful tracebacks. That said, the interaction between py.test's stderr capturing, git-remote's capturing of output, and sh's output handling is making py.test less useful than it normally is.

In addition, I'm sure that it would be easier to get gitifyhg included into mainline git if the remote was using the same test suite as git. I'm not sure if this will or should become a goal, but it does make me more open to switching to the shell based test suites that git uses.