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

[Jun Omae]

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.

[Jun Omae]

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'

[Ryan J Ollos]

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

@@ -404 +404 @@
             dot_git_dir = os.path.join(git_dir, '.git')
             try:
                 os.listdir(dot_git_dir)
-            except EnvironmentError, e:
+            except EnvironmentError:
                 missing = True
             else:
                 if self._control_files_exist(dot_git_dir):

tracopt/versioncontrol/git/git_fs.py

@@ -493 +493 @@
         except PyGIT.GitError, e:
             log.error(exception_to_unicode(e))
             raise InvalidRepository(
-                _("%(path)s does not appear to be a Git repository.",
-                  path=path))
+                _('"%(name)s" is not readable or not a Git repository.',
+                  name=params.get('name') or '(default)'))
 
         Repository.__init__(self, 'git:' + path, self.params, log)
         self._cached_git_id = str(self.id)

[Jun Omae]

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

[Ryan J Ollos]

Thanks, changes look good to me.

[Jun Omae]

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

[Ryan J Ollos]

Milestone renamed