GNOME Bugzilla – Bug 789877
manifest: add build api patch to gvfs
Last modified: 2018-04-05 19:44:03 UTC
Created attachment 362924 [details] [review] Add build api patch for gvfs gvfs has recently moved to meson[0] and is going to remove autotools from its source code tree. This patch adds a build api patch for it. [0] https://bugzilla.gnome.org/show_bug.cgi?id=786149
Review of attachment 362924 [details] [review]: ::: patches/gvfs-build-api.patch @@ +101,3 @@ ++enable_installed_tests='' ++enable_man='' ++enable_programs='' As much as I appreciate the completeness, Continuous currently only uses three options: - disable-gphoto2 - disable-documentation - enable-installed-tests so there's no real need to do this... @@ +246,3 @@ ++ ${enable_docs} \ ++ ${enable_man} \ ++ ${enable_introspection} \ ... Especially because you are not using *any* of the options you define above.
Created attachment 362934 [details] [review] Add build api patch for gvfs I'm sorry. I've tried it very quickly and I should have checked and tested it better. I've done it now. I have also changed the options used by gnome-continuous, though I've left the `disable-gphoto2`. The new options allow all the available programs to be built, included those used for development. Thank you for your review.
Review of attachment 362934 [details] [review]: Looks good
Comment on attachment 362934 [details] [review] Add build api patch for gvfs Pushed as 85d3744 - Add build-api patch for gvfs
I have reverted the patch because it makes the build process to fail. gvfs uses `libgcrypt`, which instead of providing a `.pc` file, comes with a tool called `libgcrypt-config` which can be used to get information about library and compiler flags used when building and also its version number. gnome-continuous has failed with the following error: Meson encountered an error in file meson.build, line 264, column 0: Uncomparable version string '--should-not-have-used-/usr/bin/libgcrypt-config'. gvfs does the following to get the version number and compare it: libgcrypt_version = run_command(libgcrypt_config, '--version').stdout().strip() ... have_gcrypt = libgcrypt_version.version_compare('>= 1.2.2') I don't know how to fix this as I would need to analyze the commands output inside gnome-continuous. Is there any way to check `libgcrypts` output without making unnecessary commits?
There's a patch in Continuous that provides a libgcrypt.pc file; sadly, upstream has rejected it, but it could be used downstream. My suggestion would be to defensively check for libgcrypt via pkg-config, in case upstream changes their mind, or if more downstream distributions apply the patch. Basically: libgcrypt_dep = dependency('libgcrypt', required: false) if not libgcrypt_dep.found() # use libgcrypt_config ... endif
(In reply to Emmanuele Bassi (:ebassi) from comment #6) > There's a patch in Continuous that provides a libgcrypt.pc file; sadly, > upstream has rejected it, but it could be used downstream. > > My suggestion would be to defensively check for libgcrypt via pkg-config, in > case upstream changes their mind, or if more downstream distributions apply > the patch. > > Basically: > > libgcrypt_dep = dependency('libgcrypt', required: false) > if not libgcrypt_dep.found() > # use libgcrypt_config > ... > endif Oh! too bad they rejected the `libgcrypt.pc` file. Thank you for your approach as it would be enough if `libgcrypt-config` fallback worked. However this is exactly what is failing. This is the ouput of `--version` option in my system: $ libgcrypt-config --version 1.8.1 The output looks valid enough to be used in meson, so the following has been my approach to get `libgcrypt's` version number and compare it with the version number necessary in gvfs. libgcrypt_version = run_command(libgcrypt_config, '--version').stdout().strip() have_gcrypt = libgcrypt_version.version_compare('>= 1.2.2') This have worked in different computers, but it does not in gnome-continuous. I suspect that this might be due to the `libgcrypt's` output, but I can't fix/work on any workaround without analyzing its output. I may commit a change in gvfs which shows `libgcrypt's` version with `message(libgcrypt_version)`, trigger a build on gnome-continuos and check the output in order to guess what is happening but I don't like to make any testing commits. This is the reason why I was asking if there was any way to check `libgcrypt's' output.
Created attachment 363135 [details] [review] Add build api patch for gvfs Although the problem with `libgcrypt-config` is probably present, gvfs has recently added an option to disable `libgcrypt` support on demand[0]. This new patch uses this new option as a workaround to avoid the issue with `libgcrypt-config`. (I was thinking on doing a local Continuous install[1] to analyze the issue, though I don't have that much free space on my hard drive.) [0] https://git.gnome.org/browse/gvfs/commit/?id=d1189c1b [1] https://wiki.gnome.org/Projects/GnomeContinuous/Building
Created attachment 363341 [details] [review] Add build api patch for gvfs A followup update to the previous patch, which changes the `build-api` patch due to changes in the build options following the new meson porting guidelines to be included in gvfs. The manifest file has also been changed as one of the build options has also adopted a new name.
Created attachment 364646 [details] [review] meson: Check of a libgcrypt pkg-config file Some downstreams, like Yocto, patch their version of libgcrypt to provide a pkg-config file. Since GNOME Continuous uses Yocto as the base for the run time, and as Yocto patches their libgcrypt to provide a pkg-config file instead of using libgcrypt-config, check for libgcrypt using the idiomatic dependency() object first, and then fall back to invoking the libgcrypt-config binary.
Review of attachment 363341 [details] [review]: There's no need to use a build-api wrapper, now that I've added support for Meson. I also wrote a patch for GVFS to check for a pkg-config dependency first, and then fall back to libgcrypt-config, as I outlined in comment 6.
(In reply to Emmanuele Bassi (:ebassi) from comment #11) > Review of attachment 363341 [details] [review] [review]: > > There's no need to use a build-api wrapper, now that I've added support for > Meson. Thank you for your work. I guess that gvfs is compiling fine now, so please, feel free to close this bug. > I also wrote a patch for GVFS to check for a pkg-config dependency first, > and then fall back to libgcrypt-config, as I outlined in comment 6. The patch looks good to me, why don't you propose it for gvfs?
(In reply to Iñigo Martínez from comment #12) > (In reply to Emmanuele Bassi (:ebassi) from comment #11) > > Review of attachment 363341 [details] [review] [review] [review]: > > > > There's no need to use a build-api wrapper, now that I've added support for > > Meson. > > Thank you for your work. I guess that gvfs is compiling fine now, so please, > feel free to close this bug. Hmm, seems not really: http://build.gnome.org/continuous/buildmaster/builds/2017/11/30/29/build/log-gvfs.txt
(In reply to Ondrej Holy from comment #13) > (In reply to Iñigo Martínez from comment #12) > > (In reply to Emmanuele Bassi (:ebassi) from comment #11) > > > Review of attachment 363341 [details] [review] [review] [review] [review]: > > > > > > There's no need to use a build-api wrapper, now that I've added support for > > > Meson. > > > > Thank you for your work. I guess that gvfs is compiling fine now, so please, > > feel free to close this bug. > > Hmm, seems not really: > http://build.gnome.org/continuous/buildmaster/builds/2017/11/30/29/build/log- > gvfs.txt This is weird. All the error messages are produced by missing `gvfsdbus.h` file, which should be created before `libgvfscommon` library is created. Also all the libraries/executables depending on `libgvfscommon` would be able to find the header files, but are not.
Created attachment 364662 [details] [review] build: Missing gvfsdbus header file when building Could you try this patch? I hope that this helps.
No, the patch is wrong, even if your approach is technically right: it's a missing dependency in a highly parallelised build. The problem is that gdbus-codegen generates both the header and source files, and thus gnome.gdbus_codegen() only has one target instead of two. If we depend on the target it means we're going to compile gvfsdbus.c for every sub-module that depends on libgvfscommon. The appropriate fix would be for gnome.gdbus_codegen() to return a target for the header and a target for the source, and then we'd be able to a dependency on the header only — just like we do for generated enumerations. This requires fixing, in order: - gdbus-codegen in GIO - Meson and it would require a GLib version check inside Meson and a bump of the required version of Meson in GVFS. Alternatively, sub-components of GVFS should not include gvfsdbus.h directly, but that's a pretty tall order. For the time being I've forced the GVFS build to use Autotools in Continuous.
Just to comment how this is progressing: - gdbus-codegen changes have been merged in glib[0], so now is possible to generate the header and the source code separately. - meson PR which uses the previous changes to generate different targets[1]. [0] https://git.gnome.org/browse/glib/commit/?id=e4d68c7b3 [1] https://github.com/mesonbuild/meson/pull/2930
Created attachment 370184 [details] [review] manifest: build gvfs with meson Until recent `gdbus-codegen` support is introduced in meeson, a recent commit[0] has been merged with a workaround in gvfs. This should fix all the issues with dependencies. ebassi: does this look good to you? [0] https://git.gnome.org/browse/gvfs/commit/?id=06c28a632d738b0141b619ea1c500d2563a20298
attachment 370184 [details] [review] pushed as 19f3454 - manifest: build gvfs with meson Building gvfs with gnome-continuous has involved several commits. However, it's finally building properly with meson.