Issue 5828 - Additional paths require settings.ini

cms/sources.py

 # This file is part of the Adblock Plus web scripts,
 # Copyright (C) 2006-present eyeo GmbH
 #
 # Adblock Plus is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as
 # published by the Free Software Foundation.
 #
 # Adblock Plus is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
@@ skipping 52 matching lines @@
 
         parts = page.split('/')
         if parts[-1] == default_page:
             page = '/'.join(parts[:-1])
         if locale:
             path = '/{}/{}'.format(locale, page)
             return locale, urlparse.urlunparse(parsed[0:2] + (path,) + parsed[3:])
         return locale, '/' + page
 
     def read_config(self):
-        try:
-            configdata = self.read_file('settings.ini')[0]
-            config = ConfigParser.SafeConfigParser()
-            config.readfp(StringIO(configdata))
-            return config
-        except IOError:
-            return ConfigParser.SafeConfigParser()

[Vasily Kuznetsov, 2019/01/04 18:46:03]

This will do the trick, however, I'm worried that it makes the error message for
missing top level config file (that still should be required) less helpful. With
these change, what the user will see is:

    ... stackstrace ...
    ConfigParser.NoSectionError: No section: 'general'

whereas before it was:

    ... stackstrace ...
    IOError: [Errno 2] No such file or directory: 'docs/settings.ini'

Of course the old message is not very helpful either, but at least it was more
clear what file to look at to fix the problem. Now this is no longer the case
and more intimate knowledge of CMS would be required to understand what went
wrong.

I'm thinking that it would probably be better to change the code in
`create_source` instead. Something like this:

	-    config = source.read_config()
	     try:
	+        config = source.read_config()
		 ap = config.get('paths', 'additional-paths').strip()
		 additional_paths = filter(None, ap.split())
	-    except (ConfigParser.NoSectionError, ConfigParser.NoOptionError):
	+    except (ConfigParser.NoSectionError, ConfigParser.NoOptionError, IOError):
		 additional_paths = []

What do you think?

[rhowell, 2019/01/08 00:19:25]

Yeah, this seems clearer. Done.

+        configdata = self.read_file('settings.ini')[0]
+        config = ConfigParser.SafeConfigParser()
+        config.readfp(StringIO(configdata))
+        return config
 
     def exec_file(self, filename):
         source, filename = self.read_file(filename)
         code = compile(source, filename, 'exec')
         namespace = {}
         exec code in namespace
         return namespace
 
     #
     # Page helpers
@@ skipping 283 matching lines @@
     If `settings.ini` in the source contains `[paths]` section with an
     `additional-paths` key that contains the list of additional root folders,
     `MultiSource` will be instantiated and its bases will be the original
     source plus an additional source for each additional root folder.
     `MultiSource` looks up files in its base sources in the order they are
     provided, so the files in the additional folders will only be used if the
     original source doesn't contain that file.
     """
     source = FileSource(path)
 
-    config = source.read_config()
     try:
+        config = source.read_config()
         ap = config.get('paths', 'additional-paths').strip()
         additional_paths = filter(None, ap.split())
-    except (ConfigParser.NoSectionError, ConfigParser.NoOptionError):
+    except (ConfigParser.NoSectionError, ConfigParser.NoOptionError, IOError):
         additional_paths = []
 
     if additional_paths:
         additional_sources = [
             create_source(os.path.join(path, p))
             for p in additional_paths
         ]
         source = MultiSource([source] + additional_sources)
 
     if cached:
         for fname in [
             'list_files',
             'list_locales',
             'resolve_link',
             'read_config',
             'read_template',
             'read_locale',
             'read_file',
             'read_include',
             'exec_file',
         ]:
             setattr(source, fname, utils.memoize(getattr(source, fname)))
 
     return source