[FIX] base: redirect libsass warnings to logging

libsass is currently hard-coded to emit deprecation warnings to
stderr (sass/libsass#3079), though explicit warnings can be
intercepted (I could not say whether they *are* by the Python bindings
though).

Since libsass 3.5 (released July 2017), extending compound selector is
deprecated ([0][bc], [1][extendcompound]) triggering the following message:

    WARNING on line 6713, column 13 of stdin:
    Compound selectors may no longer be extended.
    Consider `@extend .form-control, :disabled` instead.
    See http://bit.ly/ExtendCompound for details.

This is both basically impossible to see when running "full" logs, and
extremely spammy when only showing "warning"-level logs and running
tours, as each tour will cause at least one assets compilation,
triggering the warning if using a non-ancient libsass. Because it's
emitted directly to stderr, it's also difficult to filter out.

This PR messes arounds with fd redirections (adding a helper for that,
similar to `contextlib.redirect_stderr` but working at the fd level as
that's what we need here) in order to capture the stderr output of
libsass and re-emit it as a logging `warning` if non-empty, to make it
both visible and filter-able.

* Intercepts to a temporary file, as we don't really know how
  much *stuff* libsass may output, currently it's way below PIPE_BUF (to
  say nothing of the actual pipe buffers) but we've no way to know
  that will always be the case, so using a pipe would require a
  separate thread to ensure the pipe output gets read and we don't
  risk clogging up the pipe (and blocking libsass entirely).
* Doesn't bother checking for content with `tell` beforehand, that
  seems unlikely to be a limiting factor: currently we always lseek +
  read, by first checking for content with `tell` we'd have a "best
  case" (libsass emitted no warnings) of lseek but a "worst
  case" (libsass emitted warnings) of lseek + lseek + read.
* Emitting a logging event rather than a `Deprecation` warning, an
  actual deprecation warning might be useful if we were processing the
  actual source files and could provide a precise source location to
  `warn_explicit`, but here we're dealing with a concatenated pile of
  junk (the warning is signaled on line 6713, even the largest
  bootstrap scss file is barely above 1Kloc) so shoving it through
  logging seems sufficient.

SAFETY
------

In multiprocessing context (workers / prefork mode), this should not
be an issue because the local redirection of an fd should not affect
other processes.

In multithreading context, `libsass` does not seem to release the GIL
so it *should not* be a concern, however there are small windows right
before and after calling `libsass.compile` where the
interpreter *could* decide to suspend the current thread.

Python has no API for interpreter-level critical sections of any
sort (as of 3.9), nor does it use a standard stream lock, so there is
no clean way to completely protect against this race condition. We
could try and set the switching interval to a ridiculous value (using
`sys.setswitchinterval`), but that seems riskier than the unlikely
possibility of reassigning a few logging messages to a libsass
warning.

[bc]: https://sass-lang.com/documentation/breaking-changes/extend-compound
[extendcompound]: http://bit.ly/ExtendCompound

Author: xmo-odoo

odoo/addons/base/models/assetsbundle.py

@@ -1,18 +1,18 @@
 # -*- coding: utf-8 -*-
-from collections import OrderedDict
-from datetime import datetime
-from subprocess import Popen, PIPE
 import base64
 import hashlib
 import itertools
 import json
 import logging
 import os
 import re
+import tempfile
 import textwrap
 import uuid
+from collections import OrderedDict
+from datetime import datetime
+from subprocess import Popen, PIPE
 
-import psycopg2
 try:
     import sass as libsass
 except ImportError:
@@ -23,7 +23,7 @@
 from odoo import SUPERUSER_ID
 from odoo.http import request
 from odoo.modules.module import get_resource_path
-from odoo.tools import func, misc, transpile_javascript, is_odoo_module, SourceMapGenerator
+from odoo.tools import func, misc, transpile_javascript, is_odoo_module, SourceMapGenerator, osutil
 from odoo.tools.misc import html_escape as escape
 from odoo.tools.pycompat import to_text
 
@@ -1017,17 +1017,25 @@ def compile(self, source):
         if libsass is None:
             return super(ScssStylesheetAsset, self).compile(source)
 
-        try:
-            return libsass.compile(
-                string=source,
-                include_paths=[
-                    self.bootstrap_path,
-                ],
-                output_style=self.output_style,
-                precision=self.precision,
-            )
-        except libsass.CompileError as e:
-            raise CompileError(e.args[0])
+        with tempfile.TemporaryFile() as newstderr:
+            try:
+                with osutil.redirect_fd(2, redirect_to=newstderr.fileno()):
+                    return libsass.compile(
+                        string=source,
+                        include_paths=[
+                            self.bootstrap_path,
+                        ],
+                        output_style=self.output_style,
+                        precision=self.precision,
+                    )
+            except libsass.CompileError as e:
+                raise CompileError(e.args[0])
+            finally:
+                newstderr.seek(0)
+                warnings = newstderr.read().strip()
+                if warnings:
+                    _logger.getChild('libsass').warning("%s", warnings.decode())
+
 
     def get_command(self):
         try:

odoo/tools/osutil.py

@@ -166,6 +166,31 @@ def close_srv(srv):
         except Exception:
             return False
 
+
+@contextmanager
+def redirect_fd(fd, *, redirect_to):
+    """ Redirects anything written to `fd` to `redirect_to`. Restores `fd` on
+    contextmanager exit.
+
+    Similar to :class:`python:contextlib.redirect_stdout`, but works at the fd
+    level instead of the file-like object level.
+
+    .. warning::
+
+        When applied to global fds (e.g. standard streams), this obviously has
+        global effects.
+
+    :param int fd:
+    :param int redirect_to:
+    """
+    oldfd = os.dup(fd)
+    try:
+        os.dup2(redirect_to, fd)
+        yield
+    finally:
+        os.dup2(oldfd, fd)
+        os.close(oldfd)
+
 if __name__ == '__main__':
     from pprint import pprint as pp
     pp(listdir('../report', True))