On 11/04/18 08:27, Frediano Ziglio wrote: >> +# >> +# global C defines >> +# >> +spice_server_global_cflags = [#'-std=c99', # fails compiling >> spice-common/bitops.h >> + '-fvisibility=hidden', >> + '-DSPICE_SERVER_INTERNAL', >> + '-DHAVE_CONFIG_H', >> + '-Wall', >> + '-Wextra', >> + #'-Werror', >> + '-Wno-sign-compare', >> + '-Wno-unused-parameter'] >> + > > about this, autoconf spend a lot of time looking at a lot of warning > options, how can be done in Meson? > It can be done with compiler.get_supported_arguments() http://mesonbuild.com/Reference-manual.html#compiler-object BUT, at least from what I understood by looking at the m4 file which does all those checks, it basically adds warning suppression, is it really interesting for us to shut up all those warnings? In my point of view we should have fewer warning supressions, not more, as they can give hints about bugs which we do not know about yet. For instance, -Wno-sign-compare messages might be related to integer overflows, but I tried to build without -Wno-sign-compare, and there are way too many places to fix. Also, it seems that file was copied out from some GL source code, another reason I wonder if this really belongs here. > On overall I think we should have a transition period where we > move from a system to the other trying to have the new one > feature equivalent. For instance rpm spec still uses autoconf stuff. > Sure, the rpm bits are missing. I think the best approach is to have a new meson specific spec file, and then on the removal patch rename it back to the original name. >> +foreach arg : spice_server_global_cflags >> + add_global_arguments(arg, language : 'c') >> +endforeach >> + >> +# other global vars >> +compiler = meson.get_compiler('c') >> +spice_server_config_data = configuration_data() >> +spice_server_config_data.set_quoted('VERSION', meson.project_version()) >> +spice_protocol_min_version='0.12.14' >> +spice_server_include = [include_directories('.')] >> +spice_server_libs = [] >> +spice_server_deps = [] >> +spice_server_link_args = [] >> +spice_server_requires = '' >> + >> +# >> +# Spice common subproject >> +# >> +spice_common_options = >> ['protocol-version=@0@'.format(spice_protocol_min_version), >> + 'generate-code=server'] >> + >> +spice_common = subproject('spice-common', default_options : >> spice_common_options) >> +spice_server_config_data.merge_from(spice_common.get_variable('spice_common_config_data')) >> +spice_server_include += spice_common.get_variable('spice_common_include') >> + >> +foreach dep : ['spice_common_deps', 'spice_common_server_dep'] >> + spice_server_deps += [spice_common.get_variable(dep)] >> +endforeach >> + >> +foreach lib : ['spice_common_lib', 'spice_common_server_lib'] >> + spice_server_libs += spice_common.get_variable(lib) >> +endforeach >> + >> +# >> +# check for system headers >> +# >> +headers = [['sys/time.h', 'HAVE_SYS_TIME_H'], >> + ['execinfo.h', 'HAVE_EXECINFO_H'], >> + ['linux/sockios.h', 'HAVE_LINUX_SOCKIOS_H'], >> + ['pthread_np.h', 'HAVE_PTHREAD_NP_H']] >> + > > seems quite manual having to specify both header name and > macro name, can that be automated like in configure.ac? > How much Python Meson is? > Not much Python, basically the syntax is equivalent, but it is not turing complete, so you can not have functions and reuse in other places. The closest approach to that is using the foreach statement. http://mesonbuild.com/Contributing.html#turing-completeness We would be required to have a patch in meson itself to make things done automatically as done in AC_CHECK_HEADER. And by the way, I noticed many of those macros in config.h are not even used by our code, would be a good time to revisit those as well. >> +# >> +# Non-mandatory/optional dependencies >> +# >> + >> +# gstreamer >> +spice_server_has_gstreamer = false >> +spice_server_gst_version = get_option('gstreamer') >> +if spice_server_gst_version != 'no' >> + gst_deps = ['gstreamer', 'gstreamer-base', 'gstreamer-app', >> 'gstreamer-video'] >> + foreach dep : gst_deps >> + dep = '@0@-@1@'.format(dep, spice_server_gst_version) >> + spice_server_deps += dependency(dep) >> + endforeach >> + spice_server_deps += dependency('orc-0.4') >> + >> + gst_def = 'HAVE_GSTREAMER' >> + if spice_server_gst_version == '1.0' >> + gst_def = 'HAVE_GSTREAMER_1_0' >> + endif >> + >> + spice_server_config_data.set(gst_def, '1') >> + spice_server_has_gstreamer = true >> +endif >> + >> +# lz4 >> +spice_server_has_lz4 = false >> +if get_option('lz4') >> + lz4_dep = dependency('liblz4', required : false, version : '>= 129') >> + if not lz4_dep.found() >> + lz4_dep = dependency('liblz4', version : '>= 1.7.3') >> + endif >> + >> + spice_server_deps += lz4_dep >> + spice_server_config_data.set('USE_LZ4', '1') >> + spice_server_has_lz4 = true >> +endif >> + >> +# sasl >> +spice_server_has_sasl = false >> +if get_option('sasl') >> + spice_server_deps += dependency('libsasl2') > > are these "dependency" using pkg-config? What about packages not > using pkg-config? > Yes, they are implicitly using pkg-config. You can check for libraries with find_library(), programs with find_program() and header files with compiler.find_header(). Take a look at librt and libm cases for instance. The good thing is that the returned object is used the same way in both cases. >> + >> +option('alignment-checks', >> + type : 'boolean', >> + value : false, >> + description : 'Enable runtime checks for cast alignment >> (default=false)') >> + > > why this is not in spice-common? > 'alignment-checks' option is defined here and in spice-common as well. As a subproject, spice-common will inherit it automatically from the super project, with the 'yield' keyword. This is one of the reasons we require meson version 0.45. https://gitlab.com/etrunko/spice-common/blob/meson/meson_options.txt#L4 >> +if spice_server_has_smartcard == true >> + spice_server_sources += ['smartcard.c', >> + 'smartcard.h', >> + 'smartcard-channel-client.c', >> + 'smartcard-channel-client.h'] > > for coherence I'd use a syntax like > > spice_server_sources += [ > 'smartcard.c', > 'smartcard.h', > 'smartcard-channel-client.c', > 'smartcard-channel-client.h', > ] > > (just space changes) > > is possible to do something like > > spice_server_sources += ''' > smartcard.c > smartcard.h > smartcard-channel-client.c > smartcard-channel-client.h > '''.split() > > ?? > I think it is possible yes, but I would go with the first option. >> diff --git a/subprojects/spice-common.wrap b/subprojects/spice-common.wrap >> new file mode 100644 >> index 00000000..6117c631 >> --- /dev/null >> +++ b/subprojects/spice-common.wrap >> @@ -0,0 +1,4 @@ >> +[wrap-git] >> +directory=spice-common >> +url=https://gitlab.com/etrunko/spice-common.git >> +revision=meson > > is possible to have relative URL? So for instance you could > clone spice-server and spice-common and avoid having to change this file? > Something like > > url=../spice-common.git > I don't think so, the subprojects folder is hardcoded and cannot be changed. Teuf proposed an auxiliary script that could be used by both autotools and meson but moving the git submodule under subprojects/. https://paste.fedoraproject.org/paste/HceHsiV0cjLKg3S54X-gnQ Regards, Eduardo. -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etrunko@xxxxxxxxxx _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel