trac-admin: more descriptive error message on invalid env path

[anatoly techtonik <techtonik@…>]

When wrong path is specified to trac-admin you get the following confusing message:

$ trac-admin /var/trac upgrade
Error: Command not found

[Remy Blank]

Yes, this could be improved.

[coelhudo@…]

I just finish to read the HowToContribute section, then I downloaded and setup the enviroment. Then I decided to fix a bitesized ticket.

I cloned the repository and I ran the "trac-admin" command and I saw a different message:

trac-admin /var/trac upgrade
Error: IOError: [Errno 2] No such file or directory: '/var/trac/VERSION'

This error message need to be improved?

[anatoly techtonik <techtonik@…>]

I see that the error message is improved already, but it can give an additional user-friendly hint.

[Remy Blank]

The message is not ideal, as it references /var/trac/VERSION and not the path entered on the command line. Ideally, we should check for the existence of the environment path and show a message if it doesn't exist, even before trying to open it.

[coelhudo@…]

Improving error message

[coelhudo@…]

Trying to improve this message through a clear message​.

[Christian Boos]

Well, the message is misleading as there's no such environment variable VERSION. We're talking about the file <trac-env>/VERSION here!

[anatoly techtonik <techtonik@…>]

trac/env.py

@@ -364 +364 @@
     def verify(self):
         """Verify that the provided path points to a valid Trac environment
         directory."""
-        with open(os.path.join(self.path, 'VERSION'), 'r') as fd:
-            assert fd.read(26) == 'Trac Environment Version 1'
+
+        try:
+            with open(os.path.join(self.path, 'VERSION'), 'r') as fd:
+                assert fd.read(26) == 'Trac Environment Version 1'
+        except:
+            raise TracError(_('Trac environment is not found on the given path:\n  %(enviroment_path)s.',
+                              enviroment_path = os.path.join(self.path)))
 
     def get_db_cnx(self):
         """Return a database connection from the connection pool

[Remy Blank]

Could you also please replace the use of assert? It becomes a no-op when compiled with -O. And use read() instead of read(26), so that we differentiate between 'Trac Environment Version 1' and 'Trac Environment Version 13'. Actually, the best would be to use trac.util.read_file().

[anatoly techtonik <techtonik@…>]

That's not my patch, bit I can edit it no problem (somebody need to insert from trac.util import read_file:

trac/env.py

@@ -364 +364 @@
     def verify(self):
         """Verify that the provided path points to a valid Trac environment
         directory."""
-        with open(os.path.join(self.path, 'VERSION'), 'r') as fd:
-            assert fd.read(26) == 'Trac Environment Version 1'
+
+        try:
+            if not read_file(os.path.join(self.path, 'VERSION')).startswith('Trac Environment Version 1'):
+                raise TracError(_('Bad VERSION content.'))
+        except:
+            raise TracError(_('Trac environment is not found on the given path:\n  %(enviroment_path)s.',
+                              enviroment_path = self.path))
 
     def get_db_cnx(self):
         """Return a database connection from the connection pool

[Remy Blank]

Fixed in [11073].