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
diff --git a/plainbox/impl/logging.py b/plainbox/impl/logging.py
index 85fe7dd..6e8b2d5 100644
--- a/plainbox/impl/logging.py
+++ b/plainbox/impl/logging.py
@@ -84,20 +84,23 @@ class LoggingHelper:
84 Helper class that manages logging subsystem84 Helper class that manages logging subsystem
85 """85 """
8686
87 def setup_logging(self):87 def setup_logging(self, config_dict = dict()):
88 config_dict = self.DEFAULT_CONFIG
89 # Ensure that the logging directory exists. This is important88 # Ensure that the logging directory exists. This is important
90 # because we're about to open some files there. If it can't be created89 # because we're about to open some files there. If it can't be created
91 # we fall back to a console-only config.90 # we fall back to a console-only config.
91 if not config_dict:
92 config_dict = self.DEFAULT_CONFIG
92 if not os.path.exists(self.log_dir):93 if not os.path.exists(self.log_dir):
93 # It seems that exists_ok is flaky94 # It seems that exists_ok is flaky
94 try:95 try:
95 os.makedirs(self.log_dir, exist_ok=True)96 os.makedirs(self.log_dir, exist_ok=True)
96 except OSError as error:97 except OSError as error:
97 logger.warning(98 if not config_dict.get(
98 _("Unable to create log directory: %s"), self.log_dir)99 'silence_eperm_on_logdir_warning', False):
99 logger.warning(_("Reason: %s. All logs will go to "100 logger.warning(
100 "console instead."), error)101 _("Unable to create log directory: %s"), self.log_dir)
102 logger.warning(_("Reason: %s. All logs will go to "
103 "console instead."), error)
101 config_dict = self.DEFAULT_CONSOLE_ONLY_CONFIG104 config_dict = self.DEFAULT_CONSOLE_ONLY_CONFIG
102 # Apply the selected configuration. This overrides anything currently105 # Apply the selected configuration. This overrides anything currently
103 # defined for all of the logging subsystem in this python runtime106 # defined for all of the logging subsystem in this python runtime
@@ -403,6 +406,7 @@ class LoggingHelper:
403 },406 },
404 "incremental": False,407 "incremental": False,
405 "disable_existing_loggers": True,408 "disable_existing_loggers": True,
409 "silence_eperm_on_logdir_warning": False,
406 }410 }
407411
408 @property412 @property
diff --git a/plainbox/provider_manager.py b/plainbox/provider_manager.py
index 58611ea..96767ea 100644
--- a/plainbox/provider_manager.py
+++ b/plainbox/provider_manager.py
@@ -44,7 +44,7 @@ from plainbox.i18n import ngettext
44from plainbox.impl.buildsystems import all_buildsystems44from plainbox.impl.buildsystems import all_buildsystems
45from plainbox.impl.commands import ToolBase, CommandBase45from plainbox.impl.commands import ToolBase, CommandBase
46from plainbox.impl.job import JobDefinition46from plainbox.impl.job import JobDefinition
47from plainbox.impl.logging import setup_logging47from plainbox.impl.logging import setup_logging, LoggingHelper
48from plainbox.impl.providers.special import get_categories48from plainbox.impl.providers.special import get_categories
49from plainbox.impl.providers.special import get_manifest49from plainbox.impl.providers.special import get_manifest
50from plainbox.impl.providers.v1 import InsecureProvider1PlugInCollection50from plainbox.impl.providers.v1 import InsecureProvider1PlugInCollection
@@ -1478,7 +1478,9 @@ def setup(**kwargs):
1478 gettext_domain:1478 gettext_domain:
1479 gettext translation domain for job definition strings, optional1479 gettext translation domain for job definition strings, optional
1480 """1480 """
1481 setup_logging()1481 config = LoggingHelper().DEFAULT_CONFIG.copy()
1482 config['silence_eperm_on_logdir_warning'] = True
1483 setup_logging(config)
1482 manage_py = inspect.stack()[1][0].f_globals['__file__']1484 manage_py = inspect.stack()[1][0].f_globals['__file__']
1483 location = os.path.dirname(os.path.abspath(manage_py))1485 location = os.path.dirname(os.path.abspath(manage_py))
1484 definition = Provider1Definition()1486 definition = Provider1Definition()

Subscribers

People subscribed via source and target branches