Edgewall Software

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#11098 closed defect (fixed)

env.get_version() returns the wrong value if row is missing from system table

Reported by: ethan.jucovy@… Owned by: ethan.jucovy@…
Priority: high Milestone: 1.0.2
Component: general Version: 1.0dev
Severity: normal Keywords:
Cc: Branch:
Release Notes:
API Changes:

Fix Environment.get_version regression from 0.12.x, effectively return False for databases older than Trac 0.11.


The docstring for Environment.get_version in trac/env.py states:

Return the current version of the database. If the optional argument initial is set to True, the version of the database used at the time of creation will be returned. In practice, for database created before 0.11, this will return False which is "older" than any db version number.

This is actually not correct. For databases that lack an "initial_database_version" row in their system table, this function appears to return [] (an empty list) which is interpreted as newer than any db version number, at least on my version of cpython 2.6.5:

>>> False > 0
>>> [] > 0

The function will also return [] (instead of False) when it is called with initial=False against a database whose "database_version" entry has not yet been populated.

(This situation should never occur in normal usage of Trac, but I ran into it today while trying to manually create a new Trac database for an environment that already existed — I ran DatabaseManager(env).init_db() but neglected to then populate the tables with data.)

This then causes unrecoverable (and strange!) errors when running trac-admin commands, because the EnvironmentSetup component's environment_needs_upgrade function will raise the following error:

        elif dbver > db_default.db_version:
            raise TracError(_('Database newer than Trac version'))

The function returns [] because its final line is: return rows and int(rows[0][0]). When there are no matching rows found from the database query, rows will be an empty list. This is considered False for boolean purposes, so the and statement terminates early; evaluates to the value of rows; and returns the empty list rows.

Attachments (1)

get_version.patch (2.2 KB ) - added by ethan.jucovy@… 6 years ago.

Download all attachments as: .zip

Change History (6)

by ethan.jucovy@…, 6 years ago

Attachment: get_version.patch added

comment:1 by ethan.jucovy@…, 6 years ago

attachment:get_version.patch corrects the function so that it returns False, as advertised, if the relevant "database_version" row is missing from the database. The patch also includes a test case that demonstrates the current failure by creating a Trac environment without filling the database with any of the default data.

in reply to:  description comment:2 by Christian Boos, 6 years ago

Keywords: verify added

Replying to ethan.jucovy@…:

[…] If the optional argument initial is set to True, the version of the database used at the time of creation will be returned.

Oh, that's bad… This is probably worth a fix on 0.12-stable too if it happens there as well.

comment:3 by Christian Boos, 6 years ago

Milestone: 0.12.6
Priority: normalhigh

comment:4 by Christian Boos, 6 years ago

API Changes: modified (diff)
Keywords: verify removed
Resolution: fixed
Status: newclosed
Version: 1.0dev

Fix committed in r11740.

There was no issue in 0.12.x as explained in the commit message, this bug was introduced while porting to the new DB API.

r11741 fixed another careless part of the porting.

comment:5 by Christian Boos, 6 years ago

Owner: set to ethan.jucovy@…

Modify Ticket

Change Properties
Set your email in Preferences
as closed The owner will remain ethan.jucovy@….
The resolution will be deleted.
to The owner will be changed from ethan.jucovy@… 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.