GitPlugin: GitNode *must* throw NoSuchNode on invalid revisions for Bitten to work

[Peter Suter]

Created as part of the move of GitPlugin. Tickets originally reported for ​th:GitPlugin: ​th:#9879

Quoting ​dvdkhlng:

When git revisions are lost in a repository (due to rebases or whatever), and the ​Bitten plugin already had build such a revisions, then it will later die with a NoSuchChangeset excption while showing the Timeline.

A very similar probmlem existed for Bitten (​see Bitten ticket 606) with SVN repositories and was ​fixed already. However that fix does not generalize to Git repositories, as ​GitNode.__init_ (called from GitRepository.get_node) does not throw NoSuchNode on invalid revisions if path is '/' (it does throw for all paths != '/').

So Bitten calls repo.get_node with an invalid revision, which succeeds (this part is try/catch protected). And later dies when calling repo.normalize_rev (this part is not protected).

What would be the correct fix? Is it sufficient to pull the ls_tree_info assignment and result check out of the if p: block in the GitNode constructor?

[Christian Boos]

Jun did some normalize_rev enhancements recently, maybe that applies here as well.

[Christian Boos]

That was repos:jomae.git:15468953, probably won't help here.

[Jun Omae]

I just investigated. GitNode.__init__() has inconsistent behavior with/without '/' as path.

>>> from trac.env import Environment
>>> env = Environment('/var/trac/1.0')
>>> repos = env.get_repository('trac.git')
>>> repos
<tracopt.versioncontrol.git.git_fs.GitRepository object at 0x9cfe2ac>
>>> node = repos.get_node('/', 'trunk')
>>> node.rev
'trunk'
>>> node = repos.get_node('/trac', 'trunk')
>>> node.rev
'dd8f1ebd74a89c780c91fc71b3f19d713789f133'
>>> node = repos.get_node('/', 'NOTFOUND')
>>> node = repos.get_node('/trac', 'NOTFOUND')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/jun66j5/venv/trac/1.0.1/lib/python2.5/site-packages/tracopt/versioncontrol/git/git_fs.py", line 417, in get_node
    return GitNode(self, path, rev, self.log, None, historian)
  File "/home/jun66j5/venv/trac/1.0.1/lib/python2.5/site-packages/tracopt/versioncontrol/git/git_fs.py", line 525, in __init__
    raise NoSuchNode(path, rev)
trac.versioncontrol.api.NoSuchNode: No node /trac at revision NOTFOUND

We should verify and normalize the rev argument using normalize_rev(). Also rev can be specified a branch name via get_node(), so that an unicode string.

tracopt/versioncontrol/git/git_fs.py

@@ -549 +549 @@
         self.fs_sha = None # points to either tree or blobs
         self.fs_perm = None
         self.fs_size = None
-        rev = rev and str(rev) or 'HEAD'
+        rev = unicode(rev) if rev else 'HEAD'
+        try:
+            rev = repos.normalize_rev(rev)
+        except NoSuchChangeset:
+            raise NoSuchNode(path, rev)
 
         kind = Node.DIRECTORY
         p = path.strip('/')

After the patch, GitNode.rev has 40-bytes sha string and raises NoSuchNode for non-existent revision. I'll added unit tests for it later.

>>> node = repos.get_node('/', 'trunk')
>>> node.rev
'dd8f1ebd74a89c780c91fc71b3f19d713789f133'
>>> node = repos.get_node('/trac', 'trunk')
>>> node.rev
'dd8f1ebd74a89c780c91fc71b3f19d713789f133'
>>> node = repos.get_node('/', 'NOTFOUND')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "tracopt/versioncontrol/git/git_fs.py", line 449, in get_node
    return GitNode(self, path, rev, self.log, None, historian)
  File "tracopt/versioncontrol/git/git_fs.py", line 556, in __init__
    raise NoSuchNode(path, rev)
trac.versioncontrol.api.NoSuchNode: No node / at revision NOTFOUND
>>> node = repos.get_node('/trac', 'NOTFOUND')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "tracopt/versioncontrol/git/git_fs.py", line 449, in get_node
    return GitNode(self, path, rev, self.log, None, historian)
  File "tracopt/versioncontrol/git/git_fs.py", line 556, in __init__
    raise NoSuchNode(path, rev)
trac.versioncontrol.api.NoSuchNode: No node /trac at revision NOTFOUND

[Jun Omae]

Proposed changes and unit tests in log:jomae.git@t10605.

[Jun Omae]

Committed in [12620] and merged in [12621].

[Jun Omae]

We should raise a NoSuchChangeset if revision argument of get_node() is an invalid revision.

Subversion:

>>> repos = env.get_repository('trac')
>>> repos.youngest_rev
12566
>>> repos.get_node('/', 99999)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/jun66j5/venv/trac/1.0.1/lib/python2.5/site-packages/trac/versioncontrol/cache.py", line 291, in get_node
    return self.repos.get_node(path, self.normalize_rev(rev))
  File "/home/jun66j5/venv/trac/1.0.1/lib/python2.5/site-packages/trac/versioncontrol/cache.py", line 386, in normalize_rev
    raise NoSuchChangeset(rev)
trac.versioncontrol.api.NoSuchChangeset: No changeset 99999 in the repository
>>> repos.get_node('/NOTFOUND', 12566)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/jun66j5/venv/trac/1.0.1/lib/python2.5/site-packages/trac/versioncontrol/cache.py", line 291, in get_node
    return self.repos.get_node(path, self.normalize_rev(rev))
  File "/home/jun66j5/venv/trac/1.0.1/lib/python2.5/site-packages/tracopt/versioncontrol/svn/svn_fs.py", line 481, in get_node
    return SubversionNode(path, rev, self, self.pool)
  File "/home/jun66j5/venv/trac/1.0.1/lib/python2.5/site-packages/tracopt/versioncontrol/svn/svn_fs.py", line 733, in __init__
    raise NoSuchNode(path, rev)
trac.versioncontrol.api.NoSuchNode: No node /NOTFOUND at revision 12566

Git:

>>> repos = env.get_repository('trac.git')
>>> repos.youngest_rev
'dd8f1ebd74a89c780c91fc71b3f19d713789f133'
>>> repos.get_node('/', 'invalid-rev')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "tracopt/versioncontrol/git/git_fs.py", line 449, in get_node
    return GitNode(self, path, rev, self.log, None, historian)
  File "tracopt/versioncontrol/git/git_fs.py", line 556, in __init__
    raise NoSuchNode(path, rev)
trac.versioncontrol.api.NoSuchNode: No node / at revision invalid-rev
>>> repos.get_node('/NOTFOUND', 'dd8f1ebd74a89c780c91fc71b3f19d713789f133')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "tracopt/versioncontrol/git/git_fs.py", line 449, in get_node
    return GitNode(self, path, rev, self.log, None, historian)
  File "tracopt/versioncontrol/git/git_fs.py", line 567, in __init__
    raise NoSuchNode(path, rev)
trac.versioncontrol.api.NoSuchNode: No node /NOTFOUND at revision dd8f1ebd74a89c780c91fc71b3f19d713789f133

[Jun Omae]

Fixed in [12654] and merged in [12655]. Re-closing.