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

Proposed by Maciej Kisielewski
Status: Merged
Approved by: Maciej Kisielewski
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 Approve
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.
Revision history for this message
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
Revision history for this message
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.

Revision history for this message
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