[Bug]: Cygwin build fails due to use of Windows-only functions in _tkagg.cpp #22997

DWesl · 2022-05-06T22:08:44Z

Bug summary

When the build gets to https://github.com/matplotlib/matplotlib/blob/main/src/_tkagg.cpp#L262-L273 on Cygwin, the build fails with a few goto crosses initialization warnings, which are easy to fix (closed by #23051), and two error: ‘PyErr_SetFromWindowsErr’ was not declared in this scope, which are less easy to fix.

Code for reproduction

pip install matplotlib

Actual outcome

...
lots of compiler command lines and some warnings (`array subscript has type 'char'`)
...
          gcc ...  -I/usr/include/python3.8 -c src/_tkagg.cpp -o build/temp.cygwin-3.3.4-x86_64-3.8/matplotlib.backends._tkagg/src/_tkagg.o -fvisibility=hidden -flto
          src/_tkagg.cpp: In function ‘void load_tkinter_funcs()’:
          src/_tkagg.cpp:263:9: error: ‘PyErr_SetFromWindowsErr’ was not declared in this scope
            263 |         PyErr_SetFromWindowsErr(0);
                |         ^~~~~~~~~~~~~~~~~~~~~~~
          src/_tkagg.cpp:271:9: error: ‘PyErr_SetFromWindowsErr’ was not declared in this scope
            271 |         PyErr_SetFromWindowsErr(0);
                |         ^~~~~~~~~~~~~~~~~~~~~~~
          src/_tkagg.cpp:283:1: error: jump to label ‘exit’
            283 | exit:
                | ^~~~
          src/_tkagg.cpp:272:14: note:   from here
            272 |         goto exit;
                |              ^~~~
          src/_tkagg.cpp:274:26: note:   crosses initialization of ‘bool tk_ok’
            274 |     bool tcl_ok = false, tk_ok = false;
                |                          ^~~~~
          src/_tkagg.cpp:274:10: note:   crosses initialization of ‘bool tcl_ok’
            274 |     bool tcl_ok = false, tk_ok = false;
                |          ^~~~~~
          src/_tkagg.cpp:283:1: error: jump to label ‘exit’
            283 | exit:
                | ^~~~
          src/_tkagg.cpp:268:14: note:   from here
            268 |         goto exit;
                |              ^~~~
          src/_tkagg.cpp:274:26: note:   crosses initialization of ‘bool tk_ok’
            274 |     bool tcl_ok = false, tk_ok = false;
                |                          ^~~~~
          src/_tkagg.cpp:274:10: note:   crosses initialization of ‘bool tcl_ok’
            274 |     bool tcl_ok = false, tk_ok = false;
                |          ^~~~~~
          src/_tkagg.cpp:283:1: error: jump to label ‘exit’
            283 | exit:
                | ^~~~
          src/_tkagg.cpp:264:14: note:   from here
            264 |         goto exit;
                |              ^~~~
          src/_tkagg.cpp:274:26: note:   crosses initialization of ‘bool tk_ok’
            274 |     bool tcl_ok = false, tk_ok = false;
                |                          ^~~~~
          src/_tkagg.cpp:274:10: note:   crosses initialization of ‘bool tcl_ok’
            274 |     bool tcl_ok = false, tk_ok = false;
                |          ^~~~~~
          error: command '/usr/bin/gcc' failed with exit code 1
...
pip reminds me this is not pip's fault

Expected outcome

...
Successfully installed collected packages: matplotlib

Additional information

Cygwin 3.3.4
GCC 11.2.0
Python 3.8.12

I've installed matplotlib in earlier versions. #17415 rather suggests that Cygwin builds worked at that time, and probably made using the default backend somewhat easier. #22445 seems to be where the PyErr_SetFromWindowsErr calls came in.

I've attempted to create a dummy PyErr_SetFromWindowsErr on Cygwin, but, given the number of failures in the test suite, I probably didn't do it right.

Operating system

Cygwin 3.3.4 on Windows 10

Matplotlib Version

3.5.2

Matplotlib Backend

TkAgg

Python version

3.8.12

Jupyter version

N/A

Installation

pip

The text was updated successfully, but these errors were encountered:

tacaswell · 2022-05-06T22:40:10Z

This was last touched in 9d325ae

I suspect we "just" need some better gating around cygwin?

tacaswell · 2022-05-06T22:45:13Z

Actions here:

#ifndef protect the windows only functions and replace them with PyErr_SetString (or similar)
sort out how to get at least 1 cygwin build running on our CI.

DWesl · 2022-05-06T22:46:45Z

That looks like a good summary. Where should the Cygwin CI run end up? .github/workflows/tests.yml? nightlies.yml or a new file are the other likely spots.

tacaswell · 2022-05-06T22:57:55Z

If it can be done on GHA, add it to .github/workflows/tests.yml (we also have appveyor and azure setup if those are easier to work with). I would very much prefer to not pick up yet another CI service (YACS .... that would be a good name of a new CI service ;) ).

I assume you found this already, but we have

matplotlib/src/_tkagg.cpp

Lines 15 to 27 in 7bb3ff5

    
           #ifdef _WIN32 
        
           #define WIN32_DLL 
        
           #endif 
        
           #ifdef __CYGWIN__ 
        
           /* 
        
            * Unfortunately cygwin's libdl inherits restrictions from the underlying 
        
            * Windows OS, at least currently. Therefore, a symbol may be loaded from a 
        
            * module by dlsym() only if it is really located in the given modile, 
        
            * dependencies are not included. So we have to use native WinAPI on Cygwin 
        
            * also. 
        
            */ 
        
           #define WIN32_DLL 
        
           #endif

at the top so we intentionally conflating the two.

DWesl · 2022-05-06T23:00:48Z

I can do CI on GHA. I'm less sure about the fixing-my-problem part.

Yes. That's from #17415; basically Cygwin inherits Windows's dynamic library semantics, so treating them the same (for related issues) generally solves more problems than it causes.

anntzer · 2022-05-07T11:45:16Z

I guess you need something like

diff --git i/src/_tkagg.cpp w/src/_tkagg.cpp
index 5a5616bea5..c54463dca7 100644
--- i/src/_tkagg.cpp
+++ w/src/_tkagg.cpp
@@ -24,6 +24,8 @@
  * also.
  */
 #define WIN32_DLL
+#define PyErr_SetFromWindowsErr(ierr) \
+    PyErr_SetString(PyExc_OSError, "Call to EnumProcessModules failed")
 #endif
 
 #ifdef WIN32_DLL

(not the cleanest solution, but inlining all the implementation of PyErr_SetFromWindowsErr from cpython's errors.c seems a bit overkill; I would also suggest opening an issue on the cpython tracker to make PyErr_SetFromWindowsErr (and related functions) available on cygwin too -- from a first look (I may well be mistaken), it's just an oversight on their side?)

DWesl · 2022-05-10T19:44:04Z

Would a function like:

static inline PyObject *PyErr_SetFromWindowsErr(int ierr) {
    return PyErr_SetString(PyExc_OSError, "Call to EnumProcessModules failed");
}

also work? Compiler error reporting around preprocessor tokens has gotten better recently, but reporting around functions is often better still.

On a related note, should I make a separate PR for these fixes, since trying to make get a Cygwin CI working in #22999 is still fighting me?

anntzer · 2022-05-18T11:13:48Z

I guess so? (x2)

DWesl · 2022-05-18T12:49:37Z

The barebones function based on the macro proposed earlier has been proposed as PR #23066.

tacaswell · 2022-05-18T19:26:40Z

Sorry my request for CI sent you down a rabbit hole! Thank you for your work and patience
.

DWesl · 2022-05-18T19:37:57Z

I've done a few Cygwin CI setups, so I thought that would be simpler. I wasn't expecting that updating the Libtool configuration of FreeType would fail.

oscargus added the OS: Microsoft label May 6, 2022

tacaswell added the Good first issue Open a pull request against these issues if there are no active ones! label May 6, 2022

tacaswell added this to the v3.5.3 milestone May 6, 2022

DWesl mentioned this issue May 7, 2022

CI: Add a Cygwin run to GHA CI. #22999

Merged

6 tasks

DWesl mentioned this issue May 18, 2022

BLD: Define PyErr_SetFromWindowsErr on Cygwin. #23066

Merged

6 tasks

anntzer closed this as completed in #23066 Jul 26, 2022

javidcf mentioned this issue Nov 25, 2022

Support arbitrary number of loaded modules on Windows python-pillow/Pillow#6761

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Bug]: Cygwin build fails due to use of Windows-only functions in _tkagg.cpp #22997

[Bug]: Cygwin build fails due to use of Windows-only functions in _tkagg.cpp #22997

DWesl commented May 6, 2022 •

edited

Loading

tacaswell commented May 6, 2022

tacaswell commented May 6, 2022

DWesl commented May 6, 2022 •

edited

Loading

tacaswell commented May 6, 2022

DWesl commented May 6, 2022 •

edited

Loading

anntzer commented May 7, 2022

DWesl commented May 10, 2022 •

edited

Loading

anntzer commented May 18, 2022

DWesl commented May 18, 2022

tacaswell commented May 18, 2022

DWesl commented May 18, 2022

[Bug]: Cygwin build fails due to use of Windows-only functions in _tkagg.cpp #22997

[Bug]: Cygwin build fails due to use of Windows-only functions in _tkagg.cpp #22997

Comments

DWesl commented May 6, 2022 • edited Loading

Bug summary

Code for reproduction

Actual outcome

Expected outcome

Additional information

Operating system

Matplotlib Version

Matplotlib Backend

Python version

Jupyter version

Installation

tacaswell commented May 6, 2022

tacaswell commented May 6, 2022

DWesl commented May 6, 2022 • edited Loading

tacaswell commented May 6, 2022

DWesl commented May 6, 2022 • edited Loading

anntzer commented May 7, 2022

DWesl commented May 10, 2022 • edited Loading

anntzer commented May 18, 2022

DWesl commented May 18, 2022

tacaswell commented May 18, 2022

DWesl commented May 18, 2022

DWesl commented May 6, 2022 •

edited

Loading

DWesl commented May 6, 2022 •

edited

Loading

DWesl commented May 6, 2022 •

edited

Loading

DWesl commented May 10, 2022 •

edited

Loading