GNOME Bugzilla – Bug 781525
Fix meson introspection builds, in particular Visual Studio
Last modified: 2018-01-30 13:38:05 UTC
Hi, Since some modules, along with GStreamer, are beginning to use Meson as the configuration/build system, the introspection scripts need to be updated a bit so that Visual Studio builds using Meson (at least Ninja) can continue to work. The following updates are needed: -Update dumper.py to filter out the '-lm' and '--extra-library=m' arguments, as the math functions are in the CRT .lib/DLL, and there is no m.lib on Visual Studio. -Update ccompiler.py to prepend the paths passed in by -L to the LIB and PATH envvars during the time g-ir-scanner is run, so that we can link to the .lib that we just built, and ensure that we can load the .dll we just built when running the introspection dumper program (which *will* definitely look for that DLL when run!) I will attach patches shortly for this. With blessings, thank you!
Created attachment 350114 [details] [review] giscanner/dumper.py: Filter out m.lib on Visual Studio builds Hi, This is the patch for giscanner/dumper.py so that we filter -lm or --extra-library=m that is passed in to the g-ir-scanner command line when we build for Visual Studio...
Created attachment 350115 [details] [review] giscanner/ccompiler.py: Prepend -L paths to LIB and PATH Hi, This prepends the -L paths passed into g-ir-scanner to LIB and PATH so that the linker (as well as dumpbin, which is used to resolve the .lib to the corresponding DLL that it links to) and the dumper program (which *will* look for the DLL that was just built) will look for these paths first, so that we ensure the .lib/.dll that we just built will be found first. This is necessary as, unlike the NMake Makefiles that we use to built the introspection files, other build systems such as Meson use -L to pass into the paths that the linker should look into first. With blessings, thank you!
Review of attachment 350115 [details] [review]: ::: giscanner/ccompiler.py @@ +141,3 @@ self.compiler.add_library(library) + + if self.check_is_msvc(): I actually link this should be done unconditionally. See bug 780177 for a similar issue happening on Linux, and I personally verified that the dumper bin will prefer the installed library in certain environments. @@ +146,3 @@ + build_lib_path.append(library_path) + + os.environ['LIB'] = ';'.join(build_lib_path) + ';' + \ This should be done using the path separator on the platform, e.g. os.environ['LIB'] = os.pathsep.join(build_lib_path) + os.pathsep + os.environ['LIB'] It could also use `LD_LIBRARY_PATH` on non-Windows environments to minimize the differences.
Created attachment 350186 [details] [review] giscanner/ccompiler.py: Look for libraries in -L paths first for non-libtool builds Hi Emmanuele, This is the updated patch to cover the cases on non-Visual Studio, where we prepend PKG_CONFIG_PATH as well, in addition to prepending LIB/PATH on Visual Studio. This also moves the part where we drop m.lib just before linking by distutils for Visual Studio builds into ccompiler.py so that we can be very sure that distutils won't try to link to m.lib when the dumper program is being linked, and also drop m.lib from the list of libraries that the build will attempt to resolve. With blessings, thank you!
Created attachment 350199 [details] [review] giscanner/ccompiler.py: Look in -L paths first on non-libtool We want to look first in our build paths, and then in the paths indicated by the pkg-config files, and then the paths preset in the system, so that we can ensure that we load the library that we just built before loading any of the same libraries that we might have built previously. Previously, we used NMake Makefiles to define LIB and PATH before we proceeded with building the introspection files, so that the linker will pick up the freshly-built binaries first and the dumper program will do likewise. However, in other build systems (e.g. Meson), we do not have direct control over these; and it was also reported that on non-Visual Studio builds that these paths (build paths, pkg-config paths) should come first so that the freshly-built libraries are likewise picked up first during the process. Also, for Visual Studio builds, just before we link using distutils and before we try to resolve the libraries to find the DLL the .gir/.typelib link to, filter out m.lib as there is no such thing on Visual Studio since the math functions are included in the compiler's CRT .lib/.dll.
I've updated the patch in attachment 350186 [details] [review] in order to make it work with Meson on Linux; with this chunk: if not self.check_is_msvc(): for library_path in libpaths: - args.append('-L' + library_path) then the dumper won't be able to fine the just built library.
Review of attachment 350199 [details] [review]: ::: giscanner/ccompiler.py @@ +135,3 @@ + + for envvar in libpath_env: + build_lib_path.append(library_path.replace('/','\\')) For me, this fails if e.g. LD_LIBRARY_PATH is not set in the environment. Would recommend this: if envvar in os.environ: os.environ[envvar] = os.pathsep.join(build_lib_path + [os.environ[envvar]]) else: os.environ[envvar] = os.pathsep.join(build_lib_path) (Note: I'm not a gobject-introspection reviewer)
I tested attachment 350199 [details] [review] on FreeBSD and it did fix the 'failed to find symbol' error printed by the dumper program when building gst-plugins-base with meson. Setting LD_LIBRARY_PATH is required because FreeBSD patched clang to pass --enable-new-dtags to the linker by default, causing the linker to set -rpath arguments as RUNPATH, which got overrided by LD_LIBRARY_PATH set by JHBuild. When using autotools, this problem was avoided by using libtool to link the dumper program and run the wrapper script generated by libtool. When using cmake, this problem was resolved by prepending paths listed in -L arguments to LD_LIBRARY_PATH in cmake scripts. If we can get the problem fixed in g-ir-scanner, cmake scripts used by evolution-data-server and webkit can be simplified.
Created attachment 350267 [details] [review] giscanner/ccompiler.py: Look in -L paths first on non-libtool We want to look first in our build paths, and then in the paths indicated by the pkg-config files, and then the paths preset in the system, so that we can ensure that we load the library that we just built before loading any of the same libraries that we might have built previously. Previously, we used NMake Makefiles to define LIB and PATH before we proceeded with building the introspection files, so that the linker will pick up the freshly-built binaries first and the dumper program will do likewise. However, in other build systems (e.g. Meson), we do not have direct control over these; and it was also reported that on non-Visual Studio builds that these paths (build paths, pkg-config paths) should come first so that the freshly-built libraries are likewise picked up first during the process. Also, for Visual Studio builds, just before we link using distutils and before we try to resolve the libraries to find the DLL the .gir/.typelib link to, filter out m.lib as there is no such thing on Visual Studio since the math functions are included in the compiler's CRT .lib/.dll.
Updated attachment 350199 [details] [review] to include the feedback in comment 7. Thanks for testing on FreeBSD! If the build succeeds on Windows, I think we can merge this in master.
Created attachment 350287 [details] [review] giscanner/ccompiler.py: Look in -L paths first on non-libtool Hi, It turns out that the lib resolution should only be done when using g-ir-scanner on Python 2.7.x, so this is the new patch for it. I might need to check otherwise why Meson builds using g-ir-scanner on Python 2.7.x puts the libraries it needs into the .gir's that are generated, but it shouldn't matter much as the DLLs put in there are all required whichever-way. I also moved checking for m.lib on Visual Studio in get[internal|external]_link_flags, as that is what puts things through to distutils to link the dumper program. With blessings, thank you!
So, testing it once again, and I cannot build introspection data with or without the patch, when using Meson — even though I did manage to successfully last week. To be brutally honest, the whole distutils set up has been an unmitigated disaster; the only reason why I think it may be salvaged is because it actually makes things build on Windows — otherwise I would push for reverting it.
Hi Emmanuele, I think I have to accept the fact that distutils turns out to be not all that well, mostly. Sorry for the trouble caused. However, I should point out that the distutils move was not to support Windows as Windows support was actually added before 728313, which was the bug report opened to use distutils, and the subsequent 753428. Note that the original intent of 728313 is to let g-ir-scanner on Visual Studio builds not needing to depend on a GCC installation to preprocess[1], and to improve the compiler detection process on Windows[2], but later grew to use distutils. So I think what I can try to do: -Continue to use distutils to detect the compiler that is being used (which is the CC envvar on Linux/MinGW/MSYS/MSYS64, and retain cl.exe/link.exe for the compiler and linker commands on Visual Studio). -One of these: --Retain the dumper *compiling*/proprocessing process using distutils, but assemble our own linker command line for all purposes, since the compiling and preproessing process seems to be okay. -or- --Revert the preprocessing, compiling and linking part by assembling our own command lines. These are my take, hopefully in a IMHO manner :) With blessings, thank you! [1]: https://git.gnome.org/browse/gobject-introspection/commit/?id=639841e46bc32ad92d1f040bc0de3150ffd3b603 [2]: https://git.gnome.org/browse/gobject-introspection/commit/?id=0638f9f130fe080f945d07cde5cc01835a80553a
(In reply to Fan, Chun-wei from comment #13) > Hi Emmanuele, > > I think I have to accept the fact that distutils turns out to be not all > that well, mostly. Sorry for the trouble caused. No need to apologise: I was way too harsh. The idea was good, but Python's distutils, and our own requirement to stick with Python 2.7 because of enterprise-y OSes, are kicking us down. Additionally, distutils seems to be completely oblivious to any stability, as it's just meant to be used for building Python modules on the fly. To be fair, the whole thing of compiling a small binary to extract things like properties and signals is a nightmare, and we should have required people to document the properties and signals instead. > So I think what I can try to do: > -Continue to use distutils to detect the compiler that is being used > (which is the CC envvar on Linux/MinGW/MSYS/MSYS64, and retain > cl.exe/link.exe > for the compiler and linker commands on Visual Studio). There are various other issues; it seems distutils also places the object file in a weird directory that copies the whole path under the build directory, which means you can get things like: tmp-introspectXXXXXX/home/user/gnome/sources/gtk+/_build/gtk/tmp-introspectXXXXXX/Gtk-4.0.o which will likely messu up the linker paths. > -One of these: > --Retain the dumper *compiling*/proprocessing process using distutils, but > assemble our own linker command line for all purposes, since the compiling > and preproessing process seems to be okay. I think linking is the problem, at this point, so it would be better to deal with this in a portable and safe way that we control.
Created attachment 350360 [details] [review] Windows: scannerparser.y: Really remove the temp .h file generated during gi_source_scanner_parse_macros() Hi, On the course to fix the distutils linking problems, I found that the temp folder is being clogged with the temp .h files that is generated during gi_source_scanner_parse_macros() (which is annoying :|). It turns out that we are calling g_unlink() on the temp .h files really early for some reason, and so the g_unlink() call on Windows actually failed because we did not close the FD that is associated with the temp .h file. Fix this by saving the FD from g_file_open_tmp(), do everything that we need from fmacros, then call g_close() on the FD and then finally call g_unlink() on the temp .h file, which will then succeed on Windows. With blessings, thank you!
Created attachment 350361 [details] [review] Windows: scannerparser.y: Really remove the temp .h file generated during gi_source_scanner_parse_macros() (take ii) Hi, Somehow we need to use fclose() on fmacros, not the FD that we get from g_file_open_tmp(), otherwise the FD may become invalid along the way, causing crashes on later MSVCRT DLLs. With blessings, thank you!
Created attachment 350362 [details] [review] giscanner: Fix output paths of temp source and object files Hi, (In reply to Emmanuele Bassi (:ebassi) from comment #14) > There are various other issues; it seems distutils also places the object > file in a weird directory that copies the whole path under the build > directory, which means you can get things like: > > > tmp-introspectXXXXXX/home/user/gnome/sources/gtk+/_build/gtk/tmp- > introspectXXXXXX/Gtk-4.0.o This updates sourcescanner.py to output the source file for preprocessing in GWD as well as outputting the .obj/.o file that is generated during the compilation of the dumper program in the proper location. It turned out that distutils insisted on using the full path to the source files that it is compiling (sans the root directory (i.e. <drive_letter> on Windows cmd.exe or / on other environments), which was causing the issue that Emmanuele mentioned. With this patch the .o/.obj file will end up in /home/user/gnome/sources/gtk+/_build/gtk/tmp-introspectXXXXXX/Gtk-4.0.o for Emmanuele's case, for instance. With blessings, thank you!
Hi, This is the patch to dumper.py and ccompiler.py so that we go back to assembling the linker command line so that we can build the introspection files, which will honor the -L flags passed into the g-ir-scanner in the proper order. Notable changes from the former implementation before 728313 and 753428: -We continue to use distutils to help detect our compiler, and it is still used to preprocess and compile the dumper program. For our case, distutils query for the CC envvar unless we are building under Visual Studio, and so we know in a better way which compiler is being used. We can then use the proper linker executable based on what compiler is detected by distutils. -The -L flags are transformed into -libpath: flags for Visual Studio builds, so that these paths are looked for for the libs first, like it is done on the other builds. -On Visual Studio, we skip linking to and resolving m.lib, because the math functions reside in the CRT .lib/.dll, and so there is no such thing as m.lib -We populate or prepend LIB/PATH (Visual Studio) or LD_LIBRARY_PATH (otherwise) so that the freshly-built library can be found and loaded during the introspection build process, namely during library resolution and when the dumper program is run. -Visual Studio buiilds will not require a GCC installation. This patch was successfully tested on g-i 'make -j8 distcheck', and I was able to successfully build graphene and gstreamer (main library) master branches as parts of a quick test using Meson with the introspection files successfully built, and from the linker command lines the -L paths are properly placed, on Linux. I also successfully built gstreamer 1.10 on Windows/MSVC using Ninja with these--Python 2.7.x builds[1] result in many other libraries (DLLs) besides the library that we are introspecting, but I think this is more of a Meson problem where --extra-library should be used in place of -l, which is already done on Python 3.x. [1]: On Windows it is always possible to build _giscanner.pyd against Python 2.7, so by putting a Python 2.7.x installation in the PATH before the Python 3.4+ installation (which is required by Meson itself), we can build introspection files using Python 2.7 when _giscanner.pyd is linked against Python 2.7.x. With blessings, thank you!
Created attachment 350389 [details] [review] giscanner: Go back to assmebling our linker command line instead of using distutils to link the dumper program (Sorry, I forgot to attach the patch...)
Review of attachment 350389 [details] [review]: ::: giscanner/dumper.py @@ +104,2 @@ else: + self._linker_cmd = [os.environ.get('CC', 'cc')] This doesn't work if CC environment variable includes not only command name but also arguments, such as 'clang -std=c11': g-ir-scanner: link: "clang -std=c11" -o /home/lantw44/gnome/build/gst-plugins-base/tmp-introspectoqqd6822/GstVideo-1.0 -L/home/lantw44/gnome/devinstall/lib -L/usr/local/lib /home/lantw44/gnome/build/gst-plugins-base/tmp-introspectoqqd6822/GstVideo-1.0.o -L. -Wl,-rpath,. -Wl,--no-as-needed -lgstvideo-1.0 -lgstbase-1.0 -lgstreamer-1.0 -lgobject-2.0 -lglib-2.0 -lintl -lm -lorc-0.4 -L/home/lantw44/gnome/build/gst-plugins-base/gst-libs/gst/video -Wl,-rpath=/home/lantw44/gnome/build/gst-plugins-base/gst-libs/gst/video -L/home/lantw44/gnome/devinstall/lib -Wl,-rpath=/home/lantw44/gnome/devinstall/lib -L/usr/local/lib -Wl,-rpath=/usr/local/lib -L/home/lantw44/gnome/devinstall/lib -lgio-2.0 -lgobject-2.0 -Wl,--export-dynamic -lgmodule-2.0 -pthread -lglib-2.0 -lintl Traceback (most recent call last):
+ Trace 237389
sys.exit(scanner_main(sys.argv))
shlibs = create_binary(transformer, options, args)
gdump_parser.get_error_quark_functions())
return dc.run()
self._link(bin_path, introspection_obj)
subprocess.check_call(args)
retcode = call(*popenargs, **kwargs)
with Popen(*popenargs, **kwargs) as p:
restore_signals, start_new_session)
raise child_exception_type(errno_num, err_msg)
@@ +244,3 @@ args.append(cflag) + for ldflag in shlex.split(os.environ.get('LDFLAGS', '')): + args.append(ldflag) Moving LDFLAGS here seems to cause it to be added too early. My LDFLAGS is '-L/home/lantw44/gnome/devinstall/lib -L/usr/local/lib': g-ir-scanner: link: clang -o /home/lantw44/gnome/build/gst-plugins-base/tmp-introspectywkvg6_f/GstVideo-1.0 -L/home/lantw44/gnome/devinstall/lib -L/usr/local/lib /home/lantw44/gnome/build/gst-plugins-base/tmp-introspectywkvg6_f/GstVideo-1.0.o -L. -Wl,-rpath,. -Wl,--no-as-needed -lgstvideo-1.0 -lgstbase-1.0 -lgstreamer-1.0 -lgobject-2.0 -lglib-2.0 -lintl -lm -lorc-0.4 -L/home/lantw44/gnome/build/gst-plugins-base/gst-libs/gst/video -Wl,-rpath=/home/lantw44/gnome/build/gst-plugins-base/gst-libs/gst/video -L/home/lantw44/gnome/devinstall/lib -Wl,-rpath=/home/lantw44/gnome/devinstall/lib -L/usr/local/lib -Wl,-rpath=/usr/local/lib -L/home/lantw44/gnome/devinstall/lib -lgio-2.0 -lgobject-2.0 -Wl,--export-dynamic -lgmodule-2.0 -pthread -lglib-2.0 -lintl /home/lantw44/gnome/build/gst-plugins-base/tmp-introspectywkvg6_f/GstVideo-1.0.o:(.data+0x100): undefined reference to `gst_video_field_order_get_type' /home/lantw44/gnome/build/gst-plugins-base/tmp-introspectywkvg6_f/GstVideo-1.0.o:(.data+0x130): undefined reference to `gst_video_orientation_method_get_type' /home/lantw44/gnome/build/gst-plugins-base/tmp-introspectywkvg6_f/GstVideo-1.0.o:(.data+0x170): undefined reference to `gst_video_time_code_get_type' /home/lantw44/gnome/build/gst-plugins-base/tmp-introspectywkvg6_f/GstVideo-1.0.o:(.data+0x178): undefined reference to `gst_video_time_code_interval_get_type' /home/lantw44/gnome/build/gst-plugins-base/tmp-introspectywkvg6_f/GstVideo-1.0.o:(.data+0x1a0): undefined reference to `gst_video_time_code_meta_api_get_type' /home/lantw44/gnome/build/gst-plugins-base/tmp-introspectywkvg6_f/GstVideo-1.0.o:(.data+0x1c0): undefined reference to `gst_video_direction_get_type' clang: error: linker command failed with exit code 1 (use -v to see invocation) linking of temporary binary failed: Command '['clang', '-o', '/home/lantw44/gnome/build/gst-plugins-base/tmp-introspectywkvg6_f/GstVideo-1.0', '-L/home/lantw44/gnome/devinstall/lib', '-L/usr/local/lib', '/home/lantw44/gnome/build/gst-plugins-base/tmp-introspectywkvg6_f/GstVideo-1.0.o', '-L.', '-Wl,-rpath,.', '-Wl,--no-as-needed', '-lgstvideo-1.0', '-lgstbase-1.0', '-lgstreamer-1.0', '-lgobject-2.0', '-lglib-2.0', '-lintl', '-lm', '-lorc-0.4', '-L/home/lantw44/gnome/build/gst-plugins-base/gst-libs/gst/video', '-Wl,-rpath=/home/lantw44/gnome/build/gst-plugins-base/gst-libs/gst/video', '-L/home/lantw44/gnome/devinstall/lib', '-Wl,-rpath=/home/lantw44/gnome/devinstall/lib', '-L/usr/local/lib', '-Wl,-rpath=/usr/local/lib', '-L/home/lantw44/gnome/devinstall/lib', '-lgio-2.0', '-lgobject-2.0', '-Wl,--export-dynamic', '-lgmodule-2.0', '-pthread', '-lglib-2.0', '-lintl']' returned non-zero exit status 1
Created attachment 350420 [details] [review] giscanner: Go back to assmebling our linker command line instead of using distutils to link the dumper program (take ii) Hi Ting-wei, (In reply to Ting-Wei Lan from comment #20) > This doesn't work if CC environment variable includes not only command name > but also arguments, such as 'clang -std=c11': Ah, I forgot that the string acquired from CC should be split(). This should work now. I also accidentally removed the fix where LDFLAGS is being processed too early-this should restore what it was before this patch. I also cleaned things up a little bit. With blessings, thank you!
With giscanner patches I was able to build gst-plugins-base introspection.
But failed to build evolution-data-server introspection: Traceback (most recent call last):
+ Trace 237400
self._linker_cmd = shlex.split(os.environ.get('CC', 'cc')) fixes problem.
Created attachment 350615 [details] [review] giscanner: Go back to assmebling our linker command line instead of using distutils to link the dumper program (take iii) Hi Albert, I was not able to reproduce your problem--how did you set CC? Anyways, I incorporated your suggestion in this new patch, as it did not cause issues for the other things here. With blessings, thank you!
Confirmed that attachment 350615 [details] [review] lets me build GTK+ with Meson, complete with introspection.
Review of attachment 350361 [details] [review]: Looks good.
Review of attachment 350362 [details] [review]: Hopefully distutils won't break the semantics of output_dir in a later version, considering that distutils is both undocumented *and* deprecated. In any case, this looks good, and works correctly.
Review of attachment 350615 [details] [review]: Looks generally good, minus a style issue. I'd wait until more people test it on different platforms before saying to push it to master. ::: giscanner/ccompiler.py @@ +137,3 @@ + if self.check_is_msvc(): + # Note that Visual Studio builds do not use libtool! + if (library != 'm'): No need to use parentheses, here.
Hi Emmanuele, Thanks! I pushed the patches as follows: -Attachment 350361 [details]: e65915c -Attachment 350362 [details]: 6b703b3 (In reply to Emmanuele Bassi (:ebassi) from comment #29) > Review of attachment 350615 [details] [review] [review]: > > Looks generally good, minus a style issue. > > I'd wait until more people test it on different platforms before saying to > push it to master. Agreed, I think this is a good idea. I will fix the style issue if this patch eventually gets pushed, or if a new patch is needed. With blessings, thank you!
Attachment 350615 [details] fixes 'failed to find symbol' error for gst-plugins-base on FreeBSD, and running ninja under truss (FreeBSD equivalent of strace) shows that LD_LIBRARY_PATH is correctly set. I haven't tested other modules because there are other meson issues that prevent gst-plugins-base from being successfully built.
Thanks for testing!
Comment on attachment 350615 [details] [review] giscanner: Go back to assmebling our linker command line instead of using distutils to link the dumper program (take iii) Let's get it in.
Review of attachment 350615 [details] [review]: Hi Emmanuele, Thanks! I pushed the patch as 5d4cd25. I will leave this bug open for a while in case of any further issues. --- Hi, Thanks to all who provided feedback on this issue, otherwise I could not have seen them! --- Hi Ting-wei, 金多蝦!又再麻煩挺瑋大了，祝當兵一切順利平安~ --- With blessings, thank you! --- Hi,
*** Bug 780177 has been marked as a duplicate of this bug. ***
Created attachment 352002 [details] [review] scanner: Fix non-libtool linker flags on Darwin Darwin's linker doesn't like "-rpath=path"; instead pass "-rpath path", which works on more linkers than the version with the = sign does. Regressed in commit 5d4cd25292b8ed2c7a821ebe19fc5ab5d447db1a.
Quick patch for a regression on macOS.
Review of attachment 352002 [details] [review]: LGTM.
Comment on attachment 352002 [details] [review] scanner: Fix non-libtool linker flags on Darwin Attachment 352002 [details] pushed as 3a140df - scanner: Fix non-libtool linker flags on Darwin Leaving bug open for now, per previous comments.
Bug has been left open long enough.