[elbe-devel] [PATCH v1 2/2] RPCAPTCache now duplicate file descriptor

[Olivier Dion]

On 2019-06-17T13:50:50+0200, Torben Hohn wrote:

> On Tue, May 28, 2019 at 02:43:09PM +0200, dion at linutronix.de wrote:
> > From: Olivier Dion <dion at linutronix.de>
> >
> > Instead of opening a log file, we shall pass now a logging
> > object to 'get_rpcaptcache'.  The worker will automatically
> > redirect both of its outputs to the logging's file
> > descriptor if any.  If the logging's file descriptor is
> > already closed, the worker will open the corresponding log
> > file in append mode with 0 buffering.

> this patch is already very useful, and it works.

This patch is now obsolete.  We should focus on the 'Sane logging' patch
instead that incorporate the same changes for RPCAPTCache and more.

> However, i think we talked about the problems with the order of the
> messages.

> It also requires, that a log object is passed.
> All messages from rpcaptcache should go through the logging framework.

> That means, you really have to create a pipe, and start a thread, which
> reads it out, and log every line read, to the logger.

> you probably need to use duct-typing to add the necessary code around
> all methods of the proxy object, that you obtain.

> this might give you a hint:
> https://stackoverflow.com/questions/14384792/how-to-wrap-python-instance-methods-at-runtime

> dont do it in RPCAptCache.__init__() because you want to modify the
> proxy object, not the manager, or the RPCAptCache object in the forked
> process.


> >
> > Signed-off-by: Olivier Dion <dion at linutronix.de>
> > ---
> >  elbepack/cdroms.py      |  6 +++---
> >  elbepack/elbeproject.py |  2 +-
> >  elbepack/finetuning.py  |  2 +-
> >  elbepack/rpcaptcache.py | 33 ++++++++++++++++++++++++---------
> >  4 files changed, 29 insertions(+), 14 deletions(-)
> >
> > diff --git a/elbepack/cdroms.py b/elbepack/cdroms.py
> > index 3f2ecc21..3ef765e5 100644
> > --- a/elbepack/cdroms.py
> > +++ b/elbepack/cdroms.py
> > @@ -49,7 +49,7 @@ def mk_source_cdrom(
> >                          cdrom_size,
> >                          mirror)
> >
> > -    cache = get_rpcaptcache(rfs, "aptcache.log", arch)
> > +    cache = get_rpcaptcache(rfs, log, arch)
> >      cache.update()
> >      pkglist = cache.get_installed_pkgs()
> >
> > @@ -150,7 +150,7 @@ def mk_binary_cdrom(
> >                                 target_repo_path, log, cdrom_size, mirror)
> >
> >      if xml is not None:
> > -        cache = get_rpcaptcache(rfs, "aptcache.log", arch)
> > +        cache = get_rpcaptcache(rfs, log, arch)
> >          for p in xml.node("debootstrappkgs"):
> >              pkg = XMLPackage(p, arch)
> >              try:
> > @@ -179,7 +179,7 @@ def mk_binary_cdrom(
> >                      pkg.installed_version +
> >                      " missing name or version")
> >
> > -    cache = get_rpcaptcache(rfs, "aptcache.log", arch)
> > +    cache = get_rpcaptcache(rfs, log, arch)
> >      pkglist = cache.get_installed_pkgs()
> >      for pkg in pkglist:
> >          try:
> > diff --git a/elbepack/elbeproject.py b/elbepack/elbeproject.py
> > index 073bfb15..1040d281 100644
> > --- a/elbepack/elbeproject.py
> > +++ b/elbepack/elbeproject.py
> > @@ -809,7 +809,7 @@ class ElbeProject (object):
> >          if env.rpcaptcache is None:
> >              env.rpcaptcache = get_rpcaptcache(
> >                  env.rfs,
> > -                self.log.fp.name,
> > +                self.log,
> >                  arch,
> >                  self.rpcaptcache_notifier,
> >                  norecommend,
> > diff --git a/elbepack/finetuning.py b/elbepack/finetuning.py
> > index 97342862..1e710fa7 100644
> > --- a/elbepack/finetuning.py
> > +++ b/elbepack/finetuning.py
> > @@ -445,7 +445,7 @@ class UpdatedAction(FinetuningAction):
> >
> >          buildenv.rfs.mkdir_p('/tmp/pkgs')
> >          with buildenv:
> > -            cache = get_rpcaptcache(buildenv.rfs, "updated-repo.log", arch)
> > +            cache = get_rpcaptcache(buildenv.rfs, log, arch)
> >
> >              pkglist = cache.get_installed_pkgs()
> >              for pkg in pkglist:
> > diff --git a/elbepack/rpcaptcache.py b/elbepack/rpcaptcache.py
> > index 5f35d419..bcd7c538 100644
> > --- a/elbepack/rpcaptcache.py
> > +++ b/elbepack/rpcaptcache.py
> > @@ -28,6 +28,27 @@ class MyMan(BaseManager):
> >          BaseManager.register(cls.typeid, cls)
> >          return cls
> >
> > +    @staticmethod
> > +    def redirect_outputs(out):
> > +        # This won't work for Python3.  We can't have 0 buffering in
> > +        # write mode .. only in binary.

> buffering = 1 is line buffering, i believe. That this would be ok.
> Please add more information, why buffering needs to be setup this
> way.

> mmm... maybe you just copied it from RPCAPTCache.__init__()

> > +        buffering = 0
> > +        sys.stdout = os.fdopen(sys.stdout.fileno(), "w", buffering)
> > +        sys.stderr = os.fdopen(sys.stderr.fileno(), "w", buffering)
> > +        os.dup2(out, sys.stdout.fileno())
> > +        os.dup2(out, sys.stderr.fileno())
> > +
> > +    def start(self, log):
> > +        try:
> > +            fd = log.fp.fileno()
> > +        except ValueError:
> > +            # It's very important to do this on _2 lines_.  Otherwise
> > +            # Python will GC the file and we would have a bad file
> > +            # descriptor!
> > +            f = open(log.fname, "a", buffering=0)
> > +            fd = f.fileno()
> > +        super(MyMan, self).start(MyMan.redirect_outputs, [fd])
> > +
> >
> >  class InChRootObject(object):
> >      def __init__(self, rfs):
> > @@ -44,18 +65,12 @@ class RPCAPTCache(InChRootObject):
> >      def __init__(
> >              self,
> >              rfs,
> > -            log,
> >              arch,
> >              notifier=None,
> >              norecommend=False,
> >              noauth=True):
> >
> >          # pylint: disable=too-many-arguments
> > -
> > -        sys.stdout = open(log, 'a', buffering=0)
> > -        sys.stderr = open(log, 'a', buffering=0)
> > -        self.logfile = open(log, 'a', buffering=0)
> > -
> >          InChRootObject.__init__(self, rfs)
> >
> >          self.notifier = notifier
> > @@ -177,7 +192,7 @@ class RPCAPTCache(InChRootObject):
> >          os.environ["DEBIAN_FRONTEND"] = "noninteractive"
> >          os.environ["DEBONF_NONINTERACTIVE_SEEN"] = "true"
> >          self.cache.commit(ElbeAcquireProgress(),
> > -                          ElbeInstallProgress(fileno=self.logfile.fileno()))
> > +                          ElbeInstallProgress(fileno=sys.stdout.fileno()))
> >          self.cache.open(progress=ElbeOpProgress())
> >
> >      def clear(self):
> > @@ -297,11 +312,11 @@ def get_rpcaptcache(
> >      # pylint: disable=too-many-arguments
> >
> >      mm = MyMan()
> > -    mm.start()
> > +    mm.start(log)
> >
> >      # Disable false positive, because pylint can not
> >      # see the creation of MyMan.RPCAPTCache by
> >      # MyMan.register()
> >      #
> >      # pylint: disable=no-member
> > -    return mm.RPCAPTCache(rfs, log, arch, notifier, norecommend, noauth)
> > +    return mm.RPCAPTCache(rfs, arch, notifier, norecommend, noauth)
> > --
> > 2.11.0
> >
> >
> > _______________________________________________
> > elbe-devel mailing list
> > elbe-devel at linutronix.de
> > https://lists.linutronix.de/mailman/listinfo/elbe-devel

> --
> Torben Hohn
> Linutronix GmbH | Bahnhofstrasse 3 | D-88690 Uhldingen-Mühlhofen
> Phone: +49 7556 25 999 18; Fax.: +49 7556 25 999 99

> Hinweise zum Datenschutz finden Sie hier (Informations on data privacy
> can be found here): https://linutronix.de/kontakt/Datenschutz.php

> Linutronix GmbH | Firmensitz (Registered Office): Uhldingen-Mühlhofen |
> Registergericht (Registration Court): Amtsgericht Freiburg i.Br., HRB700
> 806 | Geschäftsführer (Managing Directors): Heinz Egger, Thomas Gleixner


--
Olivier Dion
Linutronix GmbH | Bahnhofstrasse 3 | D-88690 Uhldingen-Mühlhofen