Merge ~kissiel/plainbox:fix-1262898-warns-when-XFG_HOME-RO into plainbox:master

Proposed by Maciej Kisielewski on 2018-01-12
Status: Merged
Approved by: Maciej Kisielewski on 2018-01-12
Approved revision: b677f977b500befffc7dc830d60d9224711dec2d
Merged at revision: ad28eb7e7ccaf1a378b1c38fc26a1150e2c00b84
Proposed branch: ~kissiel/plainbox:fix-1262898-warns-when-XFG_HOME-RO
Merge into: plainbox:master
Diff against target: 66 lines (+14/-8)
2 files modified
plainbox/impl/logging.py (+10/-6)
plainbox/provider_manager.py (+4/-2)
Reviewer Review Type Date Requested Status
Paul Larson 2018-01-12 Approve on 2018-01-12
Review via email: mp+336052@code.launchpad.net

Description of the change

silence EPERM on log_dir when using provider_manager

When plainbox is unable to create log_dir it displays a warning. That logdir is created in XDG_CACHE_HOME which when using debian build systems is not writable. This is not only the case when running plainbox commands, but also when setting up providers.

This patch adds a flag to the logging config that suppresses that warning, and uses it in provider_manager calls.

That warning still pops up when plainbox is called (as in plainbox the command in $PATH). But I'm ignoring it, as calling plainbox directly will soon be disabled.

Fixes: LP:1262898

To test it I created a dir with mode "uog-w" and pointed XDG_CACHE_HOME before running ./manage.py of some provider.

To post a comment you must log in.
Paul Larson (pwlars) wrote :

is provider_manager the only place that hits it? I assume this will be enough to stop it during the normal builds, which appears to be the intent of the bug. +1 assuming that's true. Nice cleanup!

review: Approve
Maciej Kisielewski (kissiel) wrote :

> is provider_manager the only place that hits it? I assume this will be enough
> to stop it during the normal builds, which appears to be the intent of the
> bug. +1 assuming that's true. Nice cleanup!

AFAIK this is the only place outside plainbox that uses facilities from plainbox and is used in building anything.

Sylvain Pineau (sylvain-pineau) wrote :

I think this commit broke one debian patch:

Applying patch /home/buildd/build-RECIPEBRANCHBUILD-1515077/chroot-autobuild/home/buildd/work/tree/recipe/debian/patches/silence-logging-failure
patching file plainbox/impl/logging.py
Hunk #1 FAILED at 94.
1 out of 1 hunk FAILED -- rejects in file plainbox/impl/logging.py
Restoring plainbox/impl/logging.py
Patch /home/buildd/build-RECIPEBRANCHBUILD-1515077/chroot-autobuild/home/buildd/work/tree/recipe/debian/patches/silence-logging-failure does not apply (enforce with -f)
Restoring plainbox/impl/logging.py
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/gitbuildrecipe/main.py", line 164, in main
    package_version.upstream_version, working_basedir)
  File "/usr/lib/python3/dist-packages/gitbuildrecipe/deb_util.py", line 86, in extract_upstream_tarball
    raise NoSuchTag(tag_names[0])
gitbuildrecipe.deb_util.NoSuchTag

I'll try to refresh it to restore our daily builds of plainbox

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/plainbox/impl/logging.py b/plainbox/impl/logging.py
2index 85fe7dd..6e8b2d5 100644
3--- a/plainbox/impl/logging.py
4+++ b/plainbox/impl/logging.py
5@@ -84,20 +84,23 @@ class LoggingHelper:
6 Helper class that manages logging subsystem
7 """
8
9- def setup_logging(self):
10- config_dict = self.DEFAULT_CONFIG
11+ def setup_logging(self, config_dict = dict()):
12 # Ensure that the logging directory exists. This is important
13 # because we're about to open some files there. If it can't be created
14 # we fall back to a console-only config.
15+ if not config_dict:
16+ config_dict = self.DEFAULT_CONFIG
17 if not os.path.exists(self.log_dir):
18 # It seems that exists_ok is flaky
19 try:
20 os.makedirs(self.log_dir, exist_ok=True)
21 except OSError as error:
22- logger.warning(
23- _("Unable to create log directory: %s"), self.log_dir)
24- logger.warning(_("Reason: %s. All logs will go to "
25- "console instead."), error)
26+ if not config_dict.get(
27+ 'silence_eperm_on_logdir_warning', False):
28+ logger.warning(
29+ _("Unable to create log directory: %s"), self.log_dir)
30+ logger.warning(_("Reason: %s. All logs will go to "
31+ "console instead."), error)
32 config_dict = self.DEFAULT_CONSOLE_ONLY_CONFIG
33 # Apply the selected configuration. This overrides anything currently
34 # defined for all of the logging subsystem in this python runtime
35@@ -403,6 +406,7 @@ class LoggingHelper:
36 },
37 "incremental": False,
38 "disable_existing_loggers": True,
39+ "silence_eperm_on_logdir_warning": False,
40 }
41
42 @property
43diff --git a/plainbox/provider_manager.py b/plainbox/provider_manager.py
44index 58611ea..96767ea 100644
45--- a/plainbox/provider_manager.py
46+++ b/plainbox/provider_manager.py
47@@ -44,7 +44,7 @@ from plainbox.i18n import ngettext
48 from plainbox.impl.buildsystems import all_buildsystems
49 from plainbox.impl.commands import ToolBase, CommandBase
50 from plainbox.impl.job import JobDefinition
51-from plainbox.impl.logging import setup_logging
52+from plainbox.impl.logging import setup_logging, LoggingHelper
53 from plainbox.impl.providers.special import get_categories
54 from plainbox.impl.providers.special import get_manifest
55 from plainbox.impl.providers.v1 import InsecureProvider1PlugInCollection
56@@ -1478,7 +1478,9 @@ def setup(**kwargs):
57 gettext_domain:
58 gettext translation domain for job definition strings, optional
59 """
60- setup_logging()
61+ config = LoggingHelper().DEFAULT_CONFIG.copy()
62+ config['silence_eperm_on_logdir_warning'] = True
63+ setup_logging(config)
64 manage_py = inspect.stack()[1][0].f_globals['__file__']
65 location = os.path.dirname(os.path.abspath(manage_py))
66 definition = Provider1Definition()

Subscribers

People subscribed via source and target branches