dburrows/ blog/ entry/ from-blogspot/ Dear Lazyweb: Mercurial Python API

So, in the midst of trying to convert some Darcs repositories to Mercurial repositories (see here for the reason), I hit an "interesting" bug: tailor processes "rename" patches by deleting the renamed file. A little debugging later, I get to this simulation of what tailor does:

>>> from mercurial import hg,ui
>>> uio = ui.ui(debug=True,verbose=True)
>>> r = hg.repository(ui=uio, path='.', create=True)
>>> f = file('A', 'w')
>>> f.write('Some data\n')
>>> f.close()
>>> r.add('A')
>>> r.commit(text='Add A')
>>> r.copy('A', 'B')
B does not exist!

Huh? Of course B doesn't exist, that's why I want to create it! So, we look at the code in localrepo.py to handle copying...

    def copy(self, source, dest, wlock=None):
        p = self.wjoin(dest)
        if not os.path.exists(p):
            self.ui.warn(_("%s does not exist!\n") % dest)
        elif not os.path.isfile(p):
            self.ui.warn(_("copy failed: %s is not a file\n") % dest)
            if not wlock:
                wlock = self.wlock()
            if self.dirstate.state(dest) == '?':
                self.dirstate.update([dest], "a")
            self.dirstate.copy(source, dest)

Double huh?

  1. Why is this method checking whether the destination exists before it does anything? It appears to me that this is actually necessary; just disabling this test leads to a crash later.
  2. How does mercurial work at all with such huge breakage in its core code? Does /usr/bin/hg use a different implementation of the repository code than mercurial.hg?
  3. Why, in the name of $DEITY, is this a warning and not a fatal exception? One example of what happens when you don't signal to your users that you're ignoring them is this bug: tailor happily goes ahead and removes the original file, safe in the knowledge that mercurial has copied it to the new location. Except, of course, that mercurial hasn't actually copied it. Whoops.

I'm hoping that someone will tell me that it's just too late at night for me and I'm being an idiot. Please tell me this. It would make me very sad if code like that was in my VCS.

:-( <--- sad Daniel

Comment by alexis at 11:43 PM:

  1. localrepository.copy only marks dest as a copy of source. The caller is supposed to do the actual "cp source dest" before calling this function (e.g. using the copyfile function in util.py).

  2. util.copyfile is used by the docopy function in commands.py to copy the bytes.

  3. yes, that function should return some kind of error code.