Implement ISystemInfoProvider in git connector and database connection classes

[Ryan J Ollos]

It was shown in comment:13:ticket:11565 that there are some places in the code in which the Environment.systeminfo list is directly appended to. It might be better if every component providing systeminfo data did so through ISystemInfoProvider.

The database version info was added in [4420]. The ISystemInfoProvider interface was added in [9242]. In comment:6:ticket:8908 it was noted why the database connector classes still append directly to systeminfo. Care will be given to that issue when preparing the patch.

[Ryan J Ollos]

Some proposed changes in log:rjollos.git:t11587. The changes don't result in a great simplification, but they do narrow the responsibilities of get_connection and by not appending directly to Environment.systeminfo the consistent pattern of providing info through ISystemInfoProvider is used everywhere.

I'm still considering the change in [6ff3cd7f/rjollos.git]. If the change is accepted then SubversionConnector should be modified to behave in the same way.

[Jun Omae]

Some proposed changes in log:rjollos.git:t11587.

If MySQLdb is not installed and mysql://... is used, the following NameError is raised.

$ PYTHONPATH=. ~/venv/py26-1.1/bin/python trac/admin/console.py /dev/shm/t11587 initenv t11587 mysql://tracuser:password@localhost/t11587
Creating and Initializing Project
Initenv for '/dev/shm/t11587' failed.
Failed to create environment.
global name 'MySQLdb' is not defined
Traceback (most recent call last):
  File "trac/admin/console.py", line 458, in do_initenv
    options=options)
...
  File "/home/jun66j5/src/tracdev/git/trac/db/mysql_backend.py", line 89, in __init__
    self._mysqldb_version = get_pkginfo(MySQLdb).get('version',
NameError: global name 'MySQLdb' is not defined

We should check has_mysqldb before use of get_pkginfo(MySQLdb)…

trac/db/mysql_backend.py

@@ -86 +86 @@
 
     def __init__(self):
         self._mysql_version = None
-        self._mysqldb_version = get_pkginfo(MySQLdb).get('version',
+        self._mysqldb_version = has_mysqldb and \
+                                get_pkginfo(MySQLdb).get('version',
                                                          MySQLdb.__version__)
         self.error = None
         self.required = False

[Ryan J Ollos]

Revised changes in log:rjollos.git:t11587.2. SubversionConnector is now a required component when a subversion repository is configured, and Subversion is only shown on the About page if there is a subversion repository configured. This matches the behavior implemented for the GitConnector in the proposed changes, and for the MercurialConnector after the following patch is applied:

tracext/hg/backend.py

@@ -381 +381 @@
             m = re.match(r'(\d+)\.(\d+)(?:\.(\d+))?', self._version or '')
             if m:
                 self._version_info = tuple([int(n or 0) for n in m.groups()])
+        self.required = False
 
     def _setup_ui(self, hgrc_path):
         # Starting with Mercurial 1.3 we can probably do simply:
@@ -418 +419 @@
     # ISystemInfoProvider methods
 
     def get_system_info(self):
-        if self._version is not None:
+        if self.required:
             yield 'Mercurial', self._version
 
     # IRepositoryConnector methods
@@ -437 +438 @@
             self._setup_ui(self.hgrc)
         repos = MercurialRepository(dir, params, self.log, self)
         repos.version_info = self._version_info
+        self.required = True
         return repos
 
     # IWikiSyntaxProvider methods

[Ryan J Ollos]

Replying to jomae:

We should check has_mysqldb before use of get_pkginfo(MySQLdb)…

I added similar checks in PostgreSQLConnector and SQLiteConnector constructors. I've experimented with some test cases for ImportError of database bindings [b93c3cec/rjollos.git], but that's for future consideration in utilizing mock.

Trac changes committed to trunk in [12820].

[Ryan J Ollos]

Change to MercurialPlugin committed to 1.0 branch in [635936bdbe09/mercurial-plugin]. An additional minor change in [12823].

[Ryan J Ollos]

[12820#file5] and [12820#file7] partially reverted in [14293] (#12194).