Edgewall Software
Modify

Opened 8 years ago

Closed 6 years ago

#10605 closed defect (fixed)

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

Reported by: Peter Suter Owned by: Jun Omae
Priority: low Milestone: 1.0.2
Component: plugin/git Version:
Severity: normal Keywords:
Cc: Branch:
Release Notes:

Raise NoSuchChangeset from GitNode.__init__ for invalid revision even if path is '/'

API Changes:

Description

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?

Attachments (0)

Change History (8)

comment:1 by Peter Suter, 8 years ago

Type: enhancementdefect

comment:2 by Christian Boos, 7 years ago

Milestone: next-dev-1.1.x
Priority: normallow

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

comment:3 by Christian Boos, 7 years ago

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

comment:4 by Jun Omae, 6 years ago

Milestone: next-dev-1.1.x1.0.2
Owner: set to Jun Omae
Status: newassigned

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

    diff --git a/tracopt/versioncontrol/git/git_fs.py b/tracopt/versioncontrol/git/git_fs.py
    index f7165d4..d5d0c43 100644
    a b class GitNode(Node):  
    549549        self.fs_sha = None # points to either tree or blobs
    550550        self.fs_perm = None
    551551        self.fs_size = None
    552         rev = rev and str(rev) or 'HEAD'
     552        rev = unicode(rev) if rev else 'HEAD'
     553        try:
     554            rev = repos.normalize_rev(rev)
     555        except NoSuchChangeset:
     556            raise NoSuchNode(path, rev)
    553557
    554558        kind = Node.DIRECTORY
    555559        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

comment:5 by Jun Omae, 6 years ago

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

comment:6 by Jun Omae, 6 years ago

Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

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

comment:7 by Jun Omae, 6 years ago

Resolution: fixed
Status: closedreopened

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

comment:8 by Jun Omae, 6 years ago

Release Notes: modified (diff)
Resolution: fixed
Status: reopenedclosed

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

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Jun Omae.
The resolution will be deleted. Next status will be 'reopened'.
to as closed The owner will be changed from Jun Omae to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.