GNOME Bugzilla – Bug 784212
Initial version of a meson based build system
Last modified: 2018-04-17 07:33:35 UTC
See summary
Created attachment 354501 [details] [review] Initial version of a meson based build system Does not build libsoup-gnome and exclude gssapi and ntlm_auth, and also misses other features of the autotools build system. But generates a useable library.
Created attachment 354505 [details] [review] Initial version of a meson based build system Does not build libsoup-gnome and exclude gssapi and ntlm_auth, and also misses other features of the autotools build system. But generates a useable library.
Created attachment 354510 [details] [review] Initial version of a meson based build system Does not build libsoup-gnome and exclude gssapi and ntlm_auth, and also misses other features of the autotools build system. But generates a useable library.
Created attachment 354511 [details] [review] Initial version of a meson based build system Does not build libsoup-gnome and exclude gssapi and ntlm_auth, and also misses other features of the autotools build system. But generates a useable library.
Created attachment 354512 [details] [review] Initial version of a meson based build system Does not build libsoup-gnome and exclude gssapi and ntlm_auth, and also misses other features of the autotools build system. But generates a useable library.
Created attachment 360818 [details] [review] Support for GSSAPI and NTLM
Thanks! libsoup-gnome and the unit tests at least are also missing, and I think after that we should double-check against the latest configure.ac :)
Comment on attachment 360818 [details] [review] Support for GSSAPI and NTLM Your patch is for *.orig files instead of the real ones :) Can you make them a "git format-patch" formatted patch on top of mine? Otherwise the changes look good to me
(In reply to Sebastian Dröge (slomo) from comment #8) > Comment on attachment 360818 [details] [review] [review] > Support for GSSAPI and NTLM > > Your patch is for *.orig files instead of the real ones :) > > Can you make them a "git format-patch" formatted patch on top of mine? > Otherwise the changes look good to me Yeah, it was generated with gendiff.. But I think that we should move the stuff to some upstream branch.. hence: https://git.gnome.org/browse/libsoup/log/?h=wip/meson
(In reply to Sebastian Dröge (slomo) from comment #7) > Thanks! libsoup-gnome and the unit tests at least are also missing, and I > think after that we should double-check against the latest configure.ac :) We were missing the check for glib-networking - now it's implemented in the wip/meson branch.
Hi Fan, would you mind adding support for meson build for win32? I think that you are the most familiar with libsoup and win32. Feel free to commit to wip/meson branch.
Hello Tomas, The wip/meson branch should now contain items that are needed to build libsoup using Visual Studio now, including support for MIT Kerberos (GSSAPI). Just wondering, since soup-gnome essentially requires the GNOME proxy GIO module (which means gsettings-desktop-schemas is required, which is usable on Windows), do you think we want to try to enable that on Windows builds? I need to look again into the NTLM items in libsoup to see whether it really does not work on Windows (since NTLM itself *is* a Windows item). With blessings, and cheers!
(In reply to Fan, Chun-wei from comment #12) > The wip/meson branch should now contain items that are needed to build > libsoup using Visual Studio now, including support for MIT Kerberos (GSSAPI). Thank you Fan! Nice fixes in other parts of the Meson port! > Just wondering, since soup-gnome essentially requires the GNOME proxy GIO > module (which means gsettings-desktop-schemas is required, which is usable > on Windows), do you think we want to try to enable that on Windows builds? I think that Dan should answer this question.
Is there something that we still miss? Or does it make sense to commit it to the master and have both - Autotools and Meson ports in place and once we are confident we will remove the Autotools port..
Claudio, Dan what you think about commit this to master? Or should we wait after 3.28 is out?
Hey, I don't think this patch is correct: https://git.gnome.org/browse/libsoup/commit/?h=wip/meson&id=0514976cdc8081619f5c503aae7acd06b76c5ec5 For msvc I would not expect those flags. You should probably do something like this: https://git.gnome.org/browse/libgit2-glib/tree/meson.build#n48 Apart from that, I would say to merge this. Just ensure that the new files are part of the of EXTRA_DIST of autotools so we get them in the next release since anyway the autotools stuff will not be removed for now.
Iñigo can you please give also a round of review to the meson stuff? Thanks
(In reply to Ignacio Casal Quinteiro (nacho) from comment #16) > Hey, I don't think this patch is correct: > https://git.gnome.org/browse/libsoup/commit/?h=wip/ > meson&id=0514976cdc8081619f5c503aae7acd06b76c5ec5 Thanks for noticing! Should be fixed with https://git.gnome.org/browse/libsoup/commit/?h=wip/meson&id=24e4e44bb4eaa7805947bb5395edc997c20dda8e > Apart from that, I would say to merge this. Just ensure that the new files > are part of the of EXTRA_DIST of autotools so we get them in the next > release since anyway the autotools stuff will not be removed for now. Yes, the meson files are already part of it.
If you do not get any ack today I would say go ahead and push it. It is not like you are changing the default build system anyway yet and like this the meson one can get some testing.
Review of attachment 354512 [details] [review]: It’s missing i18n support. :(
FWIW I think adding a new build system without removing the old one is a bad idea; it makes it much harder to maintain and you inevitably wind up with broken releases (e.g. meson files missing from EXTRA_DIST). And since removing the existing build system this late in the release cycle would be pretty unfriendly, what I would do if I was the maintainer would be to delete Autotools and add meson at the same time, right after creating the next stable branch.
Review of attachment 354512 [details] [review]: Also missing i18n support is bad. :P Good catch Piotr!
(In reply to Michael Catanzaro from comment #22) > Review of attachment 354512 [details] [review] [review]: > > Also missing i18n support is bad. :P Good catch Piotr! It’s been pointed out to me that the attached patch is outdated, and the Meson port in a branch has i18n support.
(In reply to Michael Catanzaro from comment #21) > FWIW I think adding a new build system without removing the old one is a bad > idea; it makes it much harder to maintain and you inevitably wind up with > broken releases (e.g. meson files missing from EXTRA_DIST). I tested this and it should be fine - see https://git.gnome.org/browse/libsoup/commit/?h=wip/meson&id=2d32b6f4ba6197ad27e984c5d3a362eac9baaed7 > And since > removing the existing build system this late in the release cycle would be > pretty unfriendly, what I would do if I was the maintainer would be to > delete Autotools and add meson at the same time, right after creating the > next stable branch. Could be. But that's something what I would like to hear from others as well. Or at least know Claudio's opinion. I got "ack" from Dan (he's fine if Claduio doesn't object)..
I would definitely like to see this in as I could start to use it on Windows.
Without having reviewed the patch I want to say that I'm OK with the move; but it will have to wait until we branch. Now it's too late in the cycle to get this in.
(In reply to Claudio Saavedra from comment #26) > Without having reviewed the patch I want to say that I'm OK with the move; > but it will have to wait until we branch. Now it's too late in the cycle to > get this in. Sounds good. And do you prefer to remove the autotools support at the same time?
Comment on attachment 354512 [details] [review] Initial version of a meson based build system All the work is in https://git.gnome.org/browse/libsoup/log/?h=wip/meson
libsoup isn't really so fast-moving that one really has to keep up with lots of build system churn, is it? Not sure why there is a need to remove autotools build at the same time. I'd say just merge the Meson build now, say it's experimental, and then remove the autotools when everyone's happy with the Meson build. The sooner it's merged, the sooner people are likely to give it a spin and fix up any remaining issues.
FWIW other modules like gdk-pixbuf, glib or glib-openssl have done the same, starting to integrate meson and then the removal can come once you are confident it is ready
I have done a very quick review (I'm a bit busy this week) and I usually make mistakes and I might have overlooked/missed things. These all are suggestions and everything can be discussed. I hope it helps. meson.build file > license : 'LGPL', I always make mistakes with licenses, but shouldn't this be LGPL2? > default_options : ['warning_level=1', 'c_std=c89']) `warning_level` is already `1` by default, so there is no need to add it here. > foreach arg : [ > ] > if cc.has_argument(arg) Since meson 0.43 you have the `get_supported_arguments` helper function which is quite useful here, you probably will need an auxiliary variable though. > glib_dep = [dependency('glib-2.0', version : '>=2.38'), > dependency('gobject-2.0', version : '>=2.38'), > dependency('gio-2.0', version : '>=2.38')] This is fine. However, all the version requirements are the same. Why don't move these to a variable that would help the maintainer/developers change the version number easier? > if not cc.compiles(check_gio_tls_src, name : 'GIO has real TLS support', dependencies : glib_dep) > error('libsoup requires glib-networking or glib-openssl for TLS support') > endif Although this is fine, you can also use `assert` here (be careful with the length of the lines, though). assert(cc.compiles(check_gio_tls_src, name : 'GIO has real TLS support', dependencies : glib_dep), 'libsoup requires glib-networking or glib-openssl for TLS support') > cdata.set('APACHE_HTTPD', '"@0@"'.format(apache_httpd2.path()) You can add quoted strings to the config data file. cdata.set_quoted('APACHE_HTTPD', apache_httpd2.path()) > add_project_arguments('-DHAVE_APACHE=1', language : 'c') > add_project_arguments('-DAPACHE_HTTPD=' + cdata.get('APACHE_HTTPD'), language : 'c') > add_project_arguments('-DHAVE_PHP_XMLRPC=1', language : 'c') > add_project_arguments('-DHAVE_CURL=1', language : 'c') It would be nice to move these to the `config.h` data, which is included by `tests-utils.h`, which is also included by all tests. In the case of `APACHE_HTTPD` you already have added it. It would be nice to unify it. > gssapi_header = cc.has_header('gssapi/gssapi.h', required: false) > if gssapi_header > gssapi_lib = cc.find_library('gssapi@0@'.format(gssapi_lib_type), required: false) > endif > if gssapi_header and gssapi_lib.found() > add_project_link_arguments('gssapi@0@.lib'.format(gssapi_lib_type), language : 'c') > else > error('GSSAPI support requested, but the MIT Keberos 5 headers and libraries are not found') > endif You can change this code for the following, which is a bit shorter: if cc.has_header('gssapi/gssapi.h', required: false) gssapi_lib = cc.find_library('gssapi' + gssapi_lib_type), required: false) assert(gssapi_lib.found(), 'GSSAPI support requested, but the MIT Keberos 5 headers and libraries are not found') add_project_link_arguments('gssapi@0@.lib'.format(gssapi_lib_type), language : 'c') endif I have also changed the first format to just use concatenation. > if krb5_config.found() > krb5_config_output = run_command (krb5_config, '--libs', 'gssapi') > if krb5_config_output.returncode() == 0 > add_project_link_arguments(krb5_config_output.stdout().split(), language : 'c') > else > error('Failed to obtain cflags for GSSAPI from krb5-config') > enable_gssapi = false > endif > > krb5_config_output = run_command (krb5_config, '--cflags', 'gssapi') > if krb5_config_output.returncode() == 0 > add_project_arguments(krb5_config_output.stdout().split(), language : 'c') > else > error('Failed to obtain cflags for GSSAPI from krb5-config') > enable_gssapi = false > endif > else > error('GSSAPI support requested, but krb5-config not found. Please specify its path with -Dkrb5-config=PATH') > enable_gssapi = false > endif You can change this code for the following, which is a bit shorter: assert(krb5_config.found(), 'GSSAPI support requested, but krb5-config not found. Please specify its path with -Dkrb5-config=PATH') krb5_config_output = run_command (krb5_config, '--libs', 'gssapi') assert(krb5_config_output.returncode() == 0, 'Failed to obtain cflags for GSSAPI from krb5-config') add_project_link_arguments(krb5_config_output.stdout().split(), language : 'c') krb5_config_output = run_command (krb5_config, '--cflags', 'gssapi') assert(krb5_config_output.returncode() == 0, 'Failed to obtain cflags for GSSAPI from krb5-config') add_project_arguments(krb5_config_output.stdout().split(), language : 'c') > add_project_arguments('-DLIBSOUP_HAVE_GSSAPI=1', language : 'c') This seems to be used only on libsoup. Why don't you move this to target specific compiler args? > ntlm_auth_option = get_option('ntlm_auth') > ntlm_auth_path = ntlm_auth_option != '' ? ntlm_auth_option : 'ntlm_auth' > ntlm_auth = find_program(ntlm_auth_path, required : false) If the default value of `ntlm_auth` option is `ntlm_auth` value, change it in the meson_options.txt file and you could replace these lines just for this: ntlm_auth = find_program(get_option('ntlm_auth'), required: false) It will then get `ntlm_auth` as default or the user entered value otherwise. > add_project_arguments('-DUSE_NTLM_AUTH=1', language : 'c') > add_project_arguments('-DNTLM_AUTH=' + ntlm_auth.path(), language : 'c') Use on both tests and libsoup directories. Why don't you move this to target specific compiler args? > cdata.set('BUILD_LIBSOUP_GNOME', enable_gnome) Is this define actually used? > cdata.set('PACKAGE_VERSION', '"@0@"'.format(soup_version)) > cdata.set('LOCALEDIR', '"@0@/@1@"'.format(prefix, get_option('localedir'))) > cdata.set('GETTEXT_PACKAGE', '"libsoup-2.4"') You can also use `set_quoted` here, without any double quotes or string formatting. In the case of locale directory, why don't you use `join_paths` without any formatting? In the case of the gettext domain, why not format a string with the project name and the api version name, so it's easier to change it? > pkgconf = configuration_data() > > pkgconf.set('prefix', get_option('prefix')) > pkgconf.set('exec_prefix', '${prefix}') > pkgconf.set('libdir', '${prefix}/@0@'.format(get_option('libdir'))) > pkgconf.set('includedir', '${prefix}/@0@'.format(get_option('includedir'))) > pkgconf.set('VERSION', soup_version) > > pkg_install_dir = '@0@/pkgconfig'.format(get_option('libdir')) > > configure_file(output : 'libsoup-2.4.pc', > input : 'libsoup-2.4.pc.in', > configuration : pkgconf, > install_dir : pkg_install_dir) > > if enable_gnome > configure_file(output : 'libsoup-gnome-2.4.pc', > input : 'libsoup-gnome-2.4.pc.in', > configuration : pkgconf, > install_dir : pkg_install_dir) > endif Although it's not always applicable, it seems to be in this case. Why don't you use meson's `pkg` module here? libsoup/meson.build file > includedir = 'libsoup-@0@/libsoup'.format(apiversion) Why not use something like: includedir = join_paths('@0@-@1@'.format(meson.project_name(), apiversion), meson.project_name()) Anyway I usually create a variable called `api_name` which is a combination of the project name and the api version. Something like: api_name = '@0@-@1@'.format(meson.project_name(), apiversion) It's usually highly useful. > soup_enums = gnome.mkenums('soup-enum-types', > sources : soup_headers, > h_template : 'soup-enum-types.h.template', > c_template : 'soup-enum-types.c.template', > install_header : true, > install_dir : join_paths(get_option('includedir'), includedir)) Why not use a variable for the strings? It would help on modifications. Something like this: enum_types = 'soup-enum-types' soup_enums = gnome.mkenums(enum_types, sources : soup_headers, h_template : enum_types + '.h.template', c_template : 'enum_types + '.c.template', install_header : true, install_dir : join_paths(get_option('includedir'), includedir)) This would help meson detect any mispelled variable, but won't if they are strings. > libsoup_c_args = ['-DHAVE_CONFIG_H'] > libsoup_c_args += ['-DG_LOG_DOMAIN="libsoup"'] > libsoup_c_args += ['-DLIBSOUP_COMPILATION'] Why don't create a full array once instead of applying concatenations?. Something like: libsoup_c_args = [ '-DHAVE_CONFIG_H', '-DG_LOG_DOMAIN="@0@"'.format(meson.project_name()), '-DLIBSOUP_COMPILATION' ] > libsoup = library('soup-@0@'.format(apiversion), soup_sources, > libsoup_gnome = library('soup-gnome-@0@'.format(apiversion), soup_gnome_sources, Be careful about `library` because it can be set as static. > include_directories : [configinc], It's not necessary to include square brackets if there is only one value. > libsoup_dep = declare_dependency(link_with : libsoup, > include_directories : configinc, > sources : soup_enums) You should only include the enums header: sources: soupenum_h. > includedir = 'libsoup-gnome-@0@/libsoup'.format(apiversion) What about this? includedir = '@0@-gnome-@1@/@0@'.format(meson.project_name(), apiversion) > include_directories : [configinc], Square brackets are not necessary. > dependencies : [deps, libsoup_dep]) Although this is fine, it's a bit odd. You are creating a new array, with a previous array (`deps` is already an array). > soup_introspection_sources = [ Why don't you reuse `soup_sources` and `soup_headers` here somehow (they might need some tweaking), so the files are not listed twice? > '--c-include=libsoup/soup.h' Since meson 0.43 you already have the `header` parameter in `generate_gir`. > soup_gir_gen_sources = [gnome.generate_gir(libsoup, There is no need to create an array from `generate_gir's` return, it's already an array (formed by two elements [gir_target, typelib_target]). Be also careful with `libsoup`, because if it's a static library, this won't work. > nsversion : '2.4', `nsversion's` value can be replace by `apiversion`. > export_packages : 'libsoup-2.4', Why don't use `apiname` for `export_packages's` value? ;) > extra_args : gir_args, > dependencies : [deps, libsoup], I haven't figured out how `dependencies` works here, but I don't think `deps` should be included here. `libsoup` is already the first parameter, so there is no need to duplicate it. > includes : ['Gio-2.0'], Square brackets are not necessary. > namespace : 'Soup', > symbol_prefix : 'soup_', > identifier_prefix : 'Soup', Why don't you create a variable for `Soup` and use it here? soup_ns = 'Soup' namespace: soup_ns, symbol_prefix: soup_ns.to_lower + '_', identifier_prefix: soup_ns, What do you think? > include_directories : [configinc], Square brackets are not necessary here. > gnome.generate_vapi('libsoup-2.4', Use apiname for `libsoup-2.4`? > sources : [soup_gir_gen_sources[0][0]], This should be: sources : soup_gir_gen_sources[0] > packages : ['gio-2.0'], Square brackets are not necessary. > '--identifier-prefix=Soup', You are already using `identifier_prefix` parameter so it's not necessary. > '--symbol-prefix=soup', You are already ysing the `symbol_prefix` parameter so it's not necessary. > '--c-include=libsoup/soup-gnome.h', You can use the `header` parameter from meson 0.43. > '--include-uninstalled=@0@/Soup-2.4.gir'.format(meson.current_build_dir()) You can replace this for the following: '--include-uninstalled=' + soup_gir_gen_sources[0].full_path() > gnome.generate_gir(libsoup_gnome, Be careful because `libsoup_gnome` can be a static library. > nsversion : '2.4' Why don't you replace this for `apiversion`? > dependencies : [deps, libsoup, libsoup_gnome, soup_gir_gen_dep], Look above. > includes : ['Gio-2.0'], Square brackets are not necessary. In this case you can also use something like a variable `soup_ns` like above and replace it here. tests/meson.build > test_utils = static_library('test-utils', 'test-utils.c', 'test-utils.h', `test-utils.h` is not necessary here`. You can also use a variable to avoid `test-utils` string duplication as you have done below on the `foreach` loop. > install : false, It's already false by default so it's not necessary. > source_dir : '.') This is not necessary. example/meson.build > get_example = executable('get', 'get.c', > dependencies : [glib_dep, libsoup_dep]) > > simple_httpd_example = executable('simple_httpd', 'simple-httpd.c', > dependencies : [glib_dep, libsoup_dep]) > > simple_proxy_example = executable('simple_proxy', 'simple-proxy.c', > dependencies : [glib_dep, libsoup_dep]) What about something like this? deps = [ glib_dep, libsoup_dep ] examples = [ 'get', 'simple-httpd', 'simple-proxy' ] foreach example: examples executable( example example + '.c', dependencies: deps ) endforeach If the probram must use an underscore, you can also use `underscorify`.
Minor thing, but I think assert() should only be used in unit tests. error() is the right thing to use in normal build scripts. It's makes the meson.build more readable and the user-facing error message will also be less confusing.
(In reply to Tim-Philipp Müller from comment #32) > Minor thing, but I think assert() should only be used in unit tests. > > error() is the right thing to use in normal build scripts. It's makes the > meson.build more readable and the user-facing error message will also be > less confusing. Actually, the major issue with `assert` is that it's not documented. I believe that is easier to read because it's shorter and it goes straight to the point, so I make heavy use of it. meson has also moved from `if-error-endif` to assert[0][1], though it's true that this only happens in its unit tests. However, this is totally subjective and personal, so I can go back to `if-error-endif` if that is the right thing. [0] https://github.com/mesonbuild/meson/commit/f3e20b2570a1779028f22772ebe295c2c21c3a94 [1] https://github.com/mesonbuild/meson/commit/b337145ec6b83aa763a4d21eb0e4b2bf35ec880a
First of all thank you Iñigo for the review! It's nice to see some feedback from someone who's experienced with Meson! I didn't about set_quoted() or join_paths() before.. (In reply to Iñigo Martínez from comment #31) > gssapi_lib = cc.find_library('gssapi' + gssapi_lib_type), required: false) there was an extra closing bracket.. > > add_project_arguments('-DLIBSOUP_HAVE_GSSAPI=1', language : 'c') > > This seems to be used only on libsoup. Why don't you move this to target > specific compiler args? It's also used in examples, so it should stay as a project argument. > > > add_project_arguments('-DUSE_NTLM_AUTH=1', language : 'c') > > add_project_arguments('-DNTLM_AUTH=' + ntlm_auth.path(), language : 'c') > > Use on both tests and libsoup directories. Why don't you move this to target > specific compiler args? As above.. I would avoid duplicating the lines per targets.. > > > cdata.set('BUILD_LIBSOUP_GNOME', enable_gnome) > > Is this define actually used? Probably a leftover - now enable_gnome should be used everywhere.. > > libsoup = library('soup-@0@'.format(apiversion), soup_sources, > > libsoup_gnome = library('soup-gnome-@0@'.format(apiversion), soup_gnome_sources, > > Be careful about `library` because it can be set as static. I thought that I will change it to shared_library(), but I'm not sure, if it's the right thing. Or we could skip introspection and vapi if it libsoup is going to be compiled as static. I probably need a help here to decide it.. > > > soup_introspection_sources = [ > > Why don't you reuse `soup_sources` and `soup_headers` here somehow (they > might need some tweaking), so the files are not listed twice? How obvious :) - actually the only difference actually is missing soup-proxy-resolver.h and soup.h for introspection. > > namespace : 'Soup', > > symbol_prefix : 'soup_', > > identifier_prefix : 'Soup', > > Why don't you create a variable for `Soup` and use it here? > > soup_ns = 'Soup' > > namespace: soup_ns, > symbol_prefix: soup_ns.to_lower + '_', > identifier_prefix: soup_ns, > > What do you think? Nice one! :)
(In reply to Tomas Popela from comment #34) > First of all thank you Iñigo for the review! It's nice to see some feedback > from someone who's experienced with Meson! I didn't about set_quoted() or > join_paths() before.. You are welcome. Don't worry about those, I'm still discovering new things and meson is also changing (and getting better) with new versions :) > (In reply to Iñigo Martínez from comment #31) > > gssapi_lib = cc.find_library('gssapi' + gssapi_lib_type), required: false) > > there was an extra closing bracket.. Sorry for the mistake. Probably I should have tell you my policy when concatenating strings. If there is only one concatenation and the string is going to be added at the beginning or at the end, I just use the `+` operator, otherwise to avoid surrounding a variable with `+` operators something just in the middle I use format. For example: x = str_var + 'foobar' x = 'foobar' + str_var x = 'foo@0@bar'.format(str_var) x = '@0@foobar@1@'.format(str_var1, str_var2) Just in case it helps... > I thought that I will change it to shared_library(), but I'm not sure, if > it's the right thing. Or we could skip introspection and vapi if it libsoup > is going to be compiled as static. I probably need a help here to decide it.. I was in doubt at the time (iirc when porting totem). In this case, `shared_library` seems the best option here. In the future it might be possible to create both types of libraries at the same time: https://github.com/mesonbuild/meson/pull/2711
(In reply to Iñigo Martínez from comment #35) > I was in doubt at the time (iirc when porting totem). In this case, > `shared_library` seems the best option here. Ok, I will change it to shared_library(). It can be changed later of course :).. > In the future it might be possible to create both types of libraries at the > same time: https://github.com/mesonbuild/meson/pull/2711 Saw this one as well, while googling for how identify the static build (the default_library option).
Tim-Philipp Müller: I've created a new issue[0] in meson in order to clarify if the `assert` function should be used or not. An official statement would be nice. I hope it helps :) [0] https://github.com/mesonbuild/meson/issues/3076
now that the release is out, can we get this sorted out?
Claudio are you ok with merging the wip/meson to master?
After we branch, you can squeeze it and merge it. Please expect to maintain this for a while if there are any issues. I have no intention to start fixing meson related issues for a while :)
(In reply to Claudio Saavedra from comment #40) > After we branch, you can squeeze it and merge it. Thanks! > Please expect to maintain > this for a while if there are any issues. I have no intention to start > fixing meson related issues for a while :) I expect that ;)
We have branched. Please notice that the libpsl patch from bug #769650 was merged already, so please make sure that works too.
This kind of means we will have to port libpsl to meson... since it does not have a build system for msvc...
Issue upstream: https://github.com/rockdaboot/libpsl/issues/95
Hi, I took a quick look at it last night, it needs a little bit of header porting for it to build on Windows without Cygwin, but it seems to work alright. I think for Meson it would be more extensive work involved. The easiest way to get IDN support for MSVC is to use ICU. My take at this. With blessings and cheers!
So I decided that I will merge the wip/meson to master. At this point we will have two build systems - Meson and Autotools. Once the MSVC situation will be resolved, then we can remove the Autotools support and only leave the Meson one.
(In reply to Tomas Popela from comment #46) > So I decided that I will merge the wip/meson to master. At this point we > will have two build systems - Meson and Autotools. Once the MSVC situation > will be resolved, then we can remove the Autotools support and only leave > the Meson one. I am not sure I understand how the two are related. You don't *need* to port libpsl to Meson, it's just a nice-to-have. You can build libpsl with Autotools+MinGW and link to it with libsoup while building with Meson+MSVC. The workflow would be identical to how libsoup is built right now with Autotools+MinGW on Windows. As an aside, you can also build libsoup with Meson+MinGW on Windows (or cross-compile it). Porting to Meson has the benefit of being able to build with MSVC and no other drawbacks.
I do not agree with that mixing runtimes is always leading to weird crashes. Anyway I guess at some point I'll have to Port that lib to meson otherwise I will end up with the current version of libsoup forever. Just sad though that having everything ready for the current release this did not get in having the fact that both build systems could be there without issues.
(In reply to Ignacio Casal Quinteiro (nacho) from comment #48) > I do not agree with that mixing runtimes is always leading to weird crashes. To add to this, there is no mixing of runtimes when you use MSVC to link to a library that was built with MinGW. MinGW literally links to msvcrt.dll. It's the same (release) runtime, hence there's no crashes as long as you stay within that. You do have issues when you try to mix C++ code from MSVC and GCC, because of various reasons and none of them apply here. In any case, removing Autotools does not result in any toolchain regressions since you can always build with MinGW on Windows using Meson.
Hi, I think for libps1 www still need to do the aforementioned header porting (change of the included system headers) for it to compile on MinGW, as there are *NIX networking headers that are included, which can be replaced with the winsock2 headers, for example. Sorry, as I am working with other projects, I didn't have the opportunity to open a PR there for this, but I hope to do so asap, with a set of NMake Makefiles at least. Work blessings, and cheers!
(sorry for the noise, www should read we)
Just for the sake of testing this, I tried to build/make a tarball with meson. I got a failure as soon as I started, not sure if I'm doing something wrong or if there are things that need fixing. In any case, I'll keep using autotools for tarballs for the time being. Here's the error: claudio@patanjali:~/git/gnome/libsoup$ meson --prefix=/home/claudio/gnome3 build The Meson build system Version: 0.45.1 Source dir: /home/claudio/git/gnome/libsoup Build dir: /home/claudio/git/gnome/libsoup/build Build type: native build Project name: libsoup Native C compiler: ccache cc (gcc 6.4.0 "cc (Debian 6.4.0-1) 6.4.0 20170704") Appending CFLAGS from environment: '-Wall -g -O0' Appending LDFLAGS from environment: '-L/home/claudio/gnome3/lib ' Build machine cpu family: x86_64 Build machine cpu: x86_64 Compiler for C supports arguments -Wall -Wmissing-include-dirs -Wpointer-arith -Winit-self -Wdeclaration-after-statement -Werror=missing-prototypes -Werror=implicit-function-declaration -Werror=aggregate-return -Werror=format=2 -Wstrict-prototypes -Wno-format-zero-length: YES Found pkg-config: /usr/bin/pkg-config (0.29) Native dependency glib-2.0 found: YES 2.57.0 Native dependency gobject-2.0 found: YES 2.57.0 Native dependency gio-2.0 found: YES 2.57.0 Native dependency sqlite3 found: YES 3.23.1 Native dependency libxml-2.0 found: YES 2.9.4 Native dependency libpsl found: YES 0.20.0 Checking if "GIO has real TLS support" compiles: YES Program httpd2 found: NO Program httpd found: NO Program apache2 found: NO Program apache found: NO Program curl found: YES (/usr/bin/curl) Program krb5-config found: NO meson.build:176:4: ERROR: Assert failed: GSSAPI support requested, but krb5-config not found. Please specify its path with -Dkrb5-config=PATH
GSSAPI is enabled by default so you need a krb5 package that provide /usr/bin/krb5-config. Or we should turn that option off by default.
I think we need to preserve the defaults from autotools. GSSAPI's default value in configure.ac is "auto".
(In reply to Claudio Saavedra from comment #54) > I think we need to preserve the defaults from autotools. GSSAPI's default > value in configure.ac is "auto". The thing is that I really hate auto values. I was bitten by this not so long ago in Fedora. One package got removed from the build root and gstreamer was missing some functionality. But in this case I will fix it so we are consistent..
With meson the auto value are discouraged
(In reply to Ignacio Casal Quinteiro (nacho) from comment #56) > With meson the auto value are discouraged This is not true. "auto" values for options are an anti-pattern only if done incorrectly. Automatic detection and skipping of non-essential dependencies is important for users to be able to build software without going through dependency hell. However, "auto" options are terrible if there's no corresponding mechanism for forcing it to be on (failing the build if the dependency is not found) or off (not search for it at all). In fact, we've been working on infrastructure that nudges people towards such a mechanism: all options would have three values: "auto", "on", or "off": https://github.com/mesonbuild/meson/pull/3376
You have a point. But if we're going to change default dependency needs then we should document it properly and not just make it a side-effect of changing the build system. Are there other dependencies whose default values have been changed similarly?
See the gnome port page: https://wiki.gnome.org/Initiatives/GnomeGoals/MesonPorting Try to avoid automatic feature detection; this makes it harder for distributors and continuous integration systems to identify the dependencies needed to build your project
(In reply to Ignacio Casal Quinteiro (nacho) from comment #59) > See the gnome port page: I would read that as "don't do automatic feature detection as the only way to detect features". Otherwise, it's quite user unfriendly, as we just saw. ... we should also not use assert() to signal errors, please. That's for internal use (in tests) and gives a particularly unfriendly error message.
(In reply to Nirbheek Chauhan from comment #60) > (In reply to Ignacio Casal Quinteiro (nacho) from comment #59) > > See the gnome port page: > > I would read that as "don't do automatic feature detection as the only way > to detect features". Otherwise, it's quite user unfriendly, as we just saw. > > ... we should also not use assert() to signal errors, please. That's for > internal use (in tests) and gives a particularly unfriendly error message. Since you are probably the best person to give suggestions on meson, can you please modify that wiki page so it does not happen to other projects as well?
(In reply to Ignacio Casal Quinteiro (nacho) from comment #61) > Since you are probably the best person to give suggestions on meson, can you > please modify that wiki page so it does not happen to other projects as well? After discussion with ebassi and mcatanzaro: <mcatanzaro> We're just using the port to meson as a convenient time to drop auto dependencies.... So it's a GNOME Goal to drop automatic detection of dependencies. Nothing to do with Meson.
(In reply to Nirbheek Chauhan from comment #60) > ... we should also not use assert() to signal errors, please. That's for > internal use (in tests) and gives a particularly unfriendly error message. I agree with you, and something like `error_if` sounds also better for me[0], but there is still no such feature in meson and meanwhile, assert does the job[1]. It is something that should be easily replaceable in the future. [0] https://github.com/mesonbuild/meson/issues/3076#issuecomment-366652495 [1] https://github.com/jpakkane/mesonarduino/blob/master/meson.build#L7
(In reply to Claudio Saavedra from comment #58) > You have a point. But if we're going to change default dependency needs then > we should document it properly What we could do is that once we will do the Meson only release, then we will make a note about the changed default in NEWS. I really don't see any other option than this.. > Are there other dependencies whose default values > have been changed similarly? Vala support in Autotools is set on auto, in Meson it's disabled GSSAPI support in Autotools is set on auto, in Meson it's enabled NTLM support in Autotools is basically set on auto, in Meson it's disabled - Kerberos developers prefer to disable this by default
(In reply to Tomas Popela from comment #64) > (In reply to Claudio Saavedra from comment #58) > > You have a point. But if we're going to change default dependency needs then > > we should document it properly > > What we could do is that once we will do the Meson only release, then we > will make a note about the changed default in NEWS. I really don't see any > other option than this.. Sounds good to me. > > Are there other dependencies whose default values > > have been changed similarly? > > Vala support in Autotools is set on auto, in Meson it's disabled > GSSAPI support in Autotools is set on auto, in Meson it's enabled > NTLM support in Autotools is basically set on auto, in Meson it's disabled > - Kerberos developers prefer to disable this by default No objections on my side about those values.