API: Expose Access to Compiler Warnings #3079
Labels
Comments
|
Closely related to #2862 |
|
Damn! I did search issues before posting. It might still be useful to get the warnings as a sass_value list so that we can format them as we like, etc. Merely dumping the output from stderr to a combined string is less flexible. |
|
Would it be useful to re-use the |
xmo-odoo
added a commit
to odoo-dev/odoo
that referenced
this issue
May 19, 2021
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Issue:
Looking at
error_handling.cpp, all deprecation warnings are currently hard-coded to print to STDERR.Could this be changed so that implementors have access to these warnings, like we do for actual errors? STDERR is only useful if libsass is running in a Terminal environment, which it isn't in my case.
Proposal:
If a custom function to handle
@warnstatements is registered with Libsass, call that function and pass the deprecation warning just as you would for a normal@warnstatement. If there is no custom handler for@warnregistered, then fallback to printing the warnings to STDERR.To allow implementors to differentiate between user warnings and compiler warnings, we could add one more property-getter such as
sass_warning_get_origin(). Or an alternate name likesass_warning_is_user_generated(), etc.Complexity
I do see a few variants of the deprecation handlers in
error_handling.cpp, but it looks like all of them have the same information as a normal@warn: filename, line, column, message.Version:
Libsass 3.6.3
The text was updated successfully, but these errors were encountered: