After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 759531 - Fix giscannermodule.c for Windows on Python 3.5+
Fix giscannermodule.c for Windows on Python 3.5+
Status: RESOLVED FIXED
Product: gobject-introspection
Classification: Platform
Component: g-ir-scanner
unspecified
Other Windows
: Normal normal
: ---
Assigned To: gobject-introspection Maintainer(s)
gobject-introspection Maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2015-12-16 09:25 UTC by Fan, Chun-wei
Modified: 2016-01-08 15:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Win32: Fix giscannermodule.c for Python 3.5+ (3.60 KB, patch)
2015-12-16 09:34 UTC, Fan, Chun-wei
none Details | Review
Win32: Fix giscannermodule.c for Python 3.5+ (3.60 KB, patch)
2015-12-16 09:36 UTC, Fan, Chun-wei
committed Details | Review

Description Fan, Chun-wei 2015-12-16 09:25:56 UTC
Hi,

In order to build G-I with different MSVC versions or with MinGW, we need to make use of the correct CRT DLL so that we can pass file descriptors/handles safely between the giscanner C module and Python, without causing g-ir-scanner and the other Python tools in the package to crash.

For Windows, since the official Python 3.5.x installer/binary release (and quite probably for a number of the following 3.x releases) is built with Visual Studio 2015, which means the CRT DLL used by Python 3.5.x is going to be different from the CRT DLLs that we used in Python 3.4.x and earlier, so we need to acquire _get_osfhandle() from the correct CRT DLL.

I will attach a patch in a bit to fix this issue.
Comment 1 Fan, Chun-wei 2015-12-16 09:34:20 UTC
Created attachment 317483 [details] [review]
Win32: Fix giscannermodule.c for Python 3.5+

Hi,

This is the patch for fixing g-i builds on Windows using Python 3.5+, so that builds of the giscanner Python module on Windows will continue to work correctly.

Since Visual Studio 2015, the CRT DLLs are refactored, into the so-called universal CRT, so unlike we are accustomed to, _get_osfhandle() is now in ucrtbase.dll instead of the msvcrxx.dll where we usually acquire that function--
according to Microsoft this would be the CRT DLL that would (hope to) be continued to be used even for later Visual Studio releases.

Note that, as even on Visual Studio 2015 builds of g-i, there is a crash that needs to be investigated for passing the file descriptor/handle directly between the giscanner module and Python, so on Windows we will use _get_osfhandle() for all cases when building against Python 3.5+.

With blessings, thank you!
Comment 2 Fan, Chun-wei 2015-12-16 09:36:32 UTC
Created attachment 317484 [details] [review]
Win32: Fix giscannermodule.c for Python 3.5+

Hmm, had a typo in an important part of the comments, so resending patch :|
Comment 3 Ignacio Casal Quinteiro (nacho) 2016-01-08 11:48:12 UTC
Review of attachment 317484 [details] [review]:

See the comment.

::: giscanner/giscannermodule.c
@@ +472,3 @@
+
+  /* XXX: Somehow we cannot use the FD directly on Python 3.5+ even when
+   *      using Visual Studio 2015, so we currently need to use _get_osfhandle() when

I do not see a call to this function anywhere on the patch. Is this comment wrong?
Comment 4 Fan, Chun-wei 2016-01-08 12:11:36 UTC
Hi Nacho,

Hmm, the comment is correct.  The reason that the call to _osf_gethandle()  is not in the patch is that it is already in the code.  The MSVC 2012 and 2013 builds use this as well, along with MSVC 2010 before we supported Python 3.3/3.4, by default.

With blessings, thank you!
Comment 5 Ignacio Casal Quinteiro (nacho) 2016-01-08 12:12:27 UTC
Comment on attachment 317484 [details] [review]
Win32: Fix giscannermodule.c for Python 3.5+

Looks correct then.
Comment 6 Fan, Chun-wei 2016-01-08 15:54:07 UTC
Hi Nacho,

Thanks for the quick reviews and comments.  I have pushed the patch as 368fc90e, meaning Visual Studio builds for the giscanner Python module will work fully for Python 2.7+ and 3.3+, along with non-Mallard uses of Python 3.2+ (I did not test with Python 3.1.x/3.0.x, since I don't have them and I doubt there would be much support needed for them, and Mako requires either Python 2.7.x or 3.3.x or later).

p.s. I am looking at LGI (g-i for LUA), as an additional testbed for g-i on Windows, and things do look quite alright so far on both x86 and x64 builds (no projects/NMake Makefiles yet, just proof-of-concept builds on it).  It does look to me that the crash on PyGI on x64 Windows so far is a PyGI problem, rather than g-i...

With blessings, thank you!