Edgewall Software

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#12896 closed defect (fixed)

GIT control files missing error is raised if directory of git repository is not readable

Reported by: Jun Omae Owned by: Jun Omae
Priority: normal Milestone: 1.0.17
Component: plugin/git Version: 1.0.15
Severity: normal Keywords:
Cc: Branch:
Release Notes:

Improve validations of a git repository.

API Changes:
Internal Changes:


This behavior confuses a user and isn't helpful. I think we should raise Make sure the Git repository is readable error rather than the error.

In particular, we should use os.listdir() before os.path.exists(). os.path.exists() with non-readable directory returns False without any exception.

Attachments (0)

Change History (6)

comment:1 by Jun Omae, 3 years ago

Proposed changes in [8610a12ce/jomae.git] (jomae.git@t12896).

2017-08-22 12:30:15,846 Trac[git_fs] ERROR: GitError: Make sure the Git repository '/var/git/libgit2' is readable: [Errno 13] Permission denied: '/var/git/libgit2'
2017-08-22 12:30:15,847 Trac[git_fs] ERROR: GitError: Make sure the Git repository '/var/git/empty' is readable: [Errno 13] Permission denied: u'/var/git/empty/HEAD'
2017-08-22 12:30:15,847 Trac[PyGIT] DEBUG: Missing Git control file 'HEAD' in '/var/git/pygit2'
2017-08-22 12:30:15,847 Trac[PyGIT] ERROR: GIT control files missing in '/var/git/pygit2'

comment:2 by Ryan J Ollos, 3 years ago

This looks good. See also #12797 (r15924).

Considering comment:4:ticket:12797 and the possible exceptions that lead to re-raising (path) does not appear to be a git repository, we might want to change the message:

  • tracopt/versioncontrol/git/PyGIT.py

    diff --git a/tracopt/versioncontrol/git/PyGIT.py b/tracopt/versioncontrol/git/PyGIT.py
    index 26a52a28b..d1ce2d9af 100644
    a b class Storage(object):  
    404404            dot_git_dir = os.path.join(git_dir, '.git')
    405405            try:
    406406                os.listdir(dot_git_dir)
    407             except EnvironmentError, e:
     407            except EnvironmentError:
    408408                missing = True
    409409            else:
    410410                if self._control_files_exist(dot_git_dir):
  • tracopt/versioncontrol/git/git_fs.py

    diff --git a/tracopt/versioncontrol/git/git_fs.py b/tracopt/versioncontrol/git/git_fs.py
    index 031f68c2b..c5eb64db5 100644
    a b class GitRepository(Repository):  
    493493        except PyGIT.GitError, e:
    494494            log.error(exception_to_unicode(e))
    495495            raise InvalidRepository(
    496                 _("%(path)s does not appear to be a Git repository.",
    497                   path=path))
     496                _('"%(name)s" is not readable or not a Git repository.',
     497                  name=params.get('name') or '(default)'))
    499499        Repository.__init__(self, 'git:' + path, self.params, log)
    500500        self._cached_git_id = str(self.id)

comment:3 by Jun Omae, 3 years ago

Owner: set to Jun Omae
Status: newassigned

That makes sense. Revised jomae.git@t12896 with your suggested changes and created jomae.git@t12896+1.2 and jomae.git@t12896+trunk.

comment:4 by Ryan J Ollos, 3 years ago

Thanks, changes look good to me.

comment:5 by Jun Omae, 3 years ago

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

Committed in [16283] and merged in [16284-16285].

comment:6 by Ryan J Ollos, 3 years ago


Milestone renamed

Modify Ticket

Change Properties
Set your email in Preferences
as closed The owner will remain Jun Omae.
The resolution will be deleted. Next status will be 'reopened'.
to 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.