GNOME Bugzilla – Bug 773114
meson: fixes for windows build
Last modified: 2016-10-27 11:13:14 UTC
Mark the gstreamer-check-1.0 dependency as `required: false` everywhere so we can build on windows where check isn't yet present. (Note that if building with the subproject/fallback system, fallback and `required: false` don't work together until this meson commit: https://github.com/mesonbuild/meson/commit/08aeac22a98eb17b3857e1885ae8c4b0b47dfeca )
Created attachment 337868 [details] [review] meson: mark gstreamer-check-1.0 as required: false
Created attachment 337869 [details] [review] [PATCH gst-devtools] meson: mark gstreamer-check-1.0 as required: false
Created attachment 337870 [details] [review] [PATCH gst-editing-services] meson: mark gstreamer-check-1.0 as required: false
Created attachment 337871 [details] [review] [PATCH gst-plugins-bad 1/2] meson: mark gstreamer-check-1.0 as required: false
Created attachment 337872 [details] [review] [PATCH gst-plugins-bad 2/2] meson: winscreencap depends on gstvideo
Created attachment 337873 [details] [review] [PATCH gst-plugins-base] meson: mark gstreamer-check-1.0 as required: false
Created attachment 337874 [details] [review] [PATCH gst-plugins-good] meson: mark gstreamer-check-1.0 as required: false
Created attachment 337875 [details] [review] [PATCH gst-plugins-ugly] meson: mark gstreamer-check-1.0 as required: false
Thinking about it, why don't you just move the declaration check into tests/check/meson.build ? Meaning you would not need that commit in meson and also once we get into finally enabling tests on windows it will not consder libgstcheck as optional (which would be incorrect at that point)
I agree with what Thibault said.
Created attachment 338156 [details] [review] [PATCH gst-devtools] meson: move gstreamer-check-1.0 dependency to validate/tests/check
Created attachment 338157 [details] [review] [PATCH gst-plugins-bad 1/4] meson: Remove gstreamer-check-1.0 dependency
Created attachment 338158 [details] [review] [PATCH gst-plugins-bad 2/4] meson: hls: Only build when any crypto_dep is found
Created attachment 338159 [details] [review] [PATCH gst-plugins-bad 4/4] meson: directsound: Add ole32 library dependency
Created attachment 338160 [details] [review] [PATCH gst-plugins-base] meson: move gstreamer-check-1.0 dependency to tests/check
Created attachment 338161 [details] [review] [PATCH gst-plugins-good 1/3] meson: move gstreamer-check-1.0 dependency to tests/check
Created attachment 338162 [details] [review] [PATCH gst-plugins-good 2/3] meson: directsound: Add ole32 library dependency
Created attachment 338163 [details] [review] [PATCH gst-plugins-good 3/3] udpsrc: change windows check from G_OS_WIN32 to G_PLATFORM_WIN32
Created attachment 338164 [details] [review] [PATCH gst-plugins-ugly] meson: move gstreamer-check-1.0 dependency to tests/check
(In reply to Thibault Saunier from comment #9) > Thinking about it, why don't you just move the declaration check into > tests/check/meson.build ? Meaning you would not need that commit in meson > and also once we get into finally enabling tests on windows it will not > consder libgstcheck as optional (which would be incorrect at that point) Sure thing, uploaded new patches with that. Added a few other incidental patches for windows build problems too, though I can move those to another bug if it's getting to be too much.
Review of attachment 338158 [details] [review]: It looks like it should not be mandatory, I guess we are missing some -DSOMETHING or something, not sure what.
Comment on attachment 338160 [details] [review] [PATCH gst-plugins-base] meson: move gstreamer-check-1.0 dependency to tests/check commit e3c7c17b9b0ff8efb81d23e135178a7be7eaeb1e Author: Scott D Phillips <scott.d.phillips@intel.com> Date: Fri Oct 21 00:32:15 2016 -0700 meson: move gstreamer-check-1.0 dependency to tests/check
Comment on attachment 338163 [details] [review] [PATCH gst-plugins-good 3/3] udpsrc: change windows check from G_OS_WIN32 to G_PLATFORM_WIN32 Why using meson would change that? it does not look right to me, but I do not know :)
Base: commit e3c7c17b9b0ff8efb81d23e135178a7be7eaeb1e Author: Scott D Phillips <scott.d.phillips@intel.com> Date: Fri Oct 21 00:32:15 2016 -0700 meson: move gstreamer-check-1.0 dependency to tests/check Good: commit 91e9a9449229830cffceeb45a6d08fa6669c5cc1 Author: Scott D Phillips <scott.d.phillips@intel.com> Date: Fri Oct 21 00:42:54 2016 -0700 meson: directsound: Add ole32 library dependency https://bugzilla.gnome.org/show_bug.cgi?id=773114 commit 46632694662b96fddb848a1f2091a215b28a2d35 Author: Scott D Phillips <scott.d.phillips@intel.com> Date: Fri Oct 21 00:42:18 2016 -0700 meson: move gstreamer-check-1.0 dependency to tests/check https://bugzilla.gnome.org/show_bug.cgi?id=773114 Ugly: commit 0555c09607e548b6c45d029ab31dbb3042195ae3 Author: Scott D Phillips <scott.d.phillips@intel.com> Date: Fri Oct 21 00:47:14 2016 -0700 meson: move gstreamer-check-1.0 dependency to tests/check https://bugzilla.gnome.org/show_bug.cgi?id=773114 Bad: commit 2ea67f0da2d8789427a099cee848b3d5543cbbeb Author: Scott D Phillips <scott.d.phillips@intel.com> Date: Fri Oct 21 00:39:32 2016 -0700 meson: directsound: Add ole32 library dependency https://bugzilla.gnome.org/show_bug.cgi?id=773114 commit 3f93d7c3d3379cfdc15bbd16f16195041f1b66ca Author: Scott D Phillips <scott.d.phillips@intel.com> Date: Mon Oct 17 09:35:41 2016 -0700 meson: winscreencap depends on gstvideo https://bugzilla.gnome.org/show_bug.cgi?id=773114 commit 7ce5fabc82790833378e8febafeb2e809ba697bc Author: Scott D Phillips <scott.d.phillips@intel.com> Date: Fri Oct 21 00:35:09 2016 -0700 meson: Remove gstreamer-check-1.0 dependency It will later be added under tests/check https://bugzilla.gnome.org/show_bug.cgi?id=773114 Devtools: commit e8e51bdad499b38d2acc0216dc124bb82b0bd72b Author: Scott D Phillips <scott.d.phillips@intel.com> Date: Fri Oct 21 00:48:47 2016 -0700 meson: move gstreamer-check-1.0 dependency to validate/tests/check https://bugzilla.gnome.org/show_bug.cgi?id=773114
Re. G_OS_WIN32 and G_PLATFORM_WIN32 - is this under cygwin?
(In reply to Thibault Saunier from comment #21) > Review of attachment 338158 [details] [review] [review]: > > It looks like it should not be mandatory, I guess we are missing some > -DSOMETHING or something, not sure what. This is the same as the autoconf logic if I understand it correctly. From gst-plugins-bad/configure.ac: > dnl Try to find a valid crypto library > PKG_CHECK_MODULES(NETTLE, nettle, [ > AC_DEFINE(HAVE_NETTLE, 1, [Define if nettle is available]) > HAVE_HLS="yes" > ],[ > AM_PATH_LIBGCRYPT([1.2.0], [ > AC_DEFINE(HAVE_LIBGCRYPT, 1, [Define if libgcrypt is available]) > HAVE_HLS="yes" > ],[ > PKG_CHECK_MODULES(OPENSSL, openssl, [ > AC_DEFINE(HAVE_OPENSSL, 1, [Define if openssl is available]) > HAVE_HLS="yes" > ],[ > HAVE_HLS="no"
(In reply to Tim-Philipp Müller from comment #25) > Re. G_OS_WIN32 and G_PLATFORM_WIN32 - is this under cygwin? Under MSYS2, which I think is a cygwin descendent. Is supporting cygwin an anti-goal? The msys2 system can also install a mingw64 glib what has G_OS_WIN32.
No, it's not an anti-goal to support cygwin, I was just trying to figure out why G_OS_UNIX is defined for you on windows. One possibility is cygwin, another is that GLib/meson/GStreamer doesn't detect things properly in msys2 for some reason.
Comment on attachment 338158 [details] [review] [PATCH gst-plugins-bad 2/4] meson: hls: Only build when any crypto_dep is found Had this fixed as well locally, but your solution is nicer.
Created attachment 338312 [details] [review] [PATCH gst-plugins-good] udpsrc: Check for G_PLATFORM_WIN32 for presence of ipi_spec_dest (In reply to Tim-Philipp Müller from comment #28) > No, it's not an anti-goal to support cygwin, I was just trying to figure out > why G_OS_UNIX is defined for you on windows. One possibility is cygwin, > another is that GLib/meson/GStreamer doesn't detect things properly in msys2 > for some reason. Looking at README.win32 in glib, it seems that G_OS_WIN32 means windows without the cygwin layer and G_OS_UNIX + G_WITH_CYGWIN is windows with cygwin. G_PLATFORM_WIN32 is set both with and without cygwin on windows. Cygwin doesn't try to add the ipi_spec_dst field so we need to not use it in both situations
The patches that moved the dependency('gstreamer-check-1.0', ...) calls to the subdirs broke building of tests with Meson git, and broke the build with Meson 0.35.1 (the last stable release). You can only initialize a subproject from the root meson.build file, and doing it from a subdir gives the following error on 0.35.1: Meson encountered an error in file subprojects/gst-plugins-base/tests/check/meson.build, line 1, column 0: Subprojects must be defined at the root directory. Incidentally, this was not visible with Meson git because one of your patches to Meson was causing this error to be ignored, and gstreamer-check was simply never found: Also couldn't find a fallback subproject in subprojects/gstreamer for the dependency gstreamer-check-1.0 I've submitted a PR for this: https://github.com/mesonbuild/meson/pull/950 I'm going to have to revert these commits for now.
(In reply to Nirbheek Chauhan from comment #31) > You can only initialize a subproject from the root meson.build file, and > doing it from a subdir gives the following error on 0.35.1: Hmm, that's quite surprising. Why should subdirectories have anything to do with fallbacks? So should we go back to the first idea where the gstreamer-check-1.0 dependency is required: false at the top level then?
I suspect it has to do with how state is maintained while doing a subproject invocation. Regardless, we need to build against 0.35.1, so we can't do this even if we change that in Meson. I think the simplest option might be to just wrap the dependency() around `if host_machine.system() != 'windows'`.
Ah, quite annoying! I guess checking if running on windows should be good enough though not very elegant.
Created attachment 338423 [details] [review] [PATCH gst-devtools] meson: Don't depend on gstreamer-check-1.0 on windows
Created attachment 338424 [details] [review] [PATCH gst-editing-services] meson: Don't depend on gstreamer-check-1.0 on windows
Created attachment 338425 [details] [review] [PATCH gst-plugins-base] meson: Don't depend on gstreamer-check-1.0 on windows
Created attachment 338426 [details] [review] [PATCH gst-plugins-good] meson: Don't depend on gstreamer-check-1.0 on windows
Created attachment 338427 [details] [review] [PATCH gst-plugins-ugly] meson: Don't depend on gstreamer-check-1.0 on windows
Ok, here's a new round of patches that puts the gstreamer-check-1.0 dependency behind an if host_machine.system() != 'windows'.
I'll take a look at this tomorrow + test it a bit and then we can push it.
(In reply to Nirbheek Chauhan from comment #41) > I'll take a look at this tomorrow + test it a bit and then we can push it. Thanks Nirbheek, I personally tested with meson-master on windows and meson-0.35.1 and meson-master on linux and got what I think are the expected results: working build on windows, gstreamer-check-1.0 and dependents present on linux.
All pushed except the udpsrc patch. It looks correct to me, so I'll push that too if I get another ACK. Note that we might also want to look into updating glib to define G_OS_WIN32 on MSYS2 if it's appropriate since we've been looking at updating Cerbero on Windows to use MSYS2. Could you look into that please, Scott?
(In reply to Nirbheek Chauhan from comment #43) > Note that we might also want to look into updating glib to define G_OS_WIN32 > on MSYS2 if it's appropriate since we've been looking at updating Cerbero on > Windows to use MSYS2. Could you look into that please, Scott? An msys2 setup has a few repositories it can install packages from: > $ pacman -Ss gcc > mingw32/mingw-w64-i686-gcc 6.2.0-2 (mingw-w64-i686-toolchain) > GNU Compiler Collection (C,C++,OpenMP) for MinGW-w64 > mingw64/mingw-w64-x86_64-gcc 6.2.0-2 (mingw-w64-x86_64-toolchain) > GNU Compiler Collection (C,C++,OpenMP) for MinGW-w64 > msys/gcc 5.3.0-3 (msys2-devel) > The GNU Compiler Collection - C and C++ frontends msys/gcc is a cygwin type toolchain that will simulate some unix stuff and if you install msys/glib2 it will have G_OS_UNIX set. mingw64/...-gcc doesn't include that unix emulation layer so mingw64/...-glib will have G_OS_WIN32 set. Really the absence of G_OS_UNIX is more important than the presence of G_OS_WIN32 meaning that some unixy stuff is unavailable. And then G_PLATFORM_WIN32 is set in both msys/glib2 and mingw64/...-glib and windows API calls are available from both corresponding toolchains. I think the cerbero build currently uses the mingw64 toolchain currently, so an msys2 based cerbero would probably just have `pacman -S mingw64/...` and then not need any other changes related to G_OS_WIN32.
Makes sense. We should get this patch in then; do you agree tim?
Yes, pushed, thanks: commit 023744a5776b55a82681e87a8d822ebe9a001716 Author: Scott D Phillips <scott.d.phillips@intel.com> Date: Sun Oct 23 17:23:10 2016 -0700 udpsrc: Check for G_PLATFORM_WIN32 for presence of ipi_spec_dest G_OS_WIN32 is only set when not building with cygwin, but ipi_spec_dest is missing both with and without cygwin. https://bugzilla.gnome.org/show_bug.cgi?id=773114