Support for the meson build system #142

dagle · 2023-02-10T13:41:56Z

This add support for the meson build system which supports multiple operating systems among things out of the box. This requires #141 to be merged, because gnome.generate_gir() treats warnings as errors.

dagle · 2023-02-10T14:04:21Z

This pr does not contain building documentation, which will be included in a future pr. My reasoning for this is that I updated the documentation to build using the new gi-docgen instead of gtk-doc.

This pr uses a 1.0 version of meson. I don't think I use anything later than 0.7 but I don't know what versions different systems ship with.

coverage works but reports false atm since I don't know if there is a portable way to check for coverage packages / support.

profiling support isn't included in this pr.

eli-schwartz · 2023-02-12T05:49:11Z

meson.build

+
+add_global_arguments('-DHAVE_CONFIG_H=1', language : 'c')
+
+iconv = dependency('iconv', required : false, method: get_option('iconv'))


Is there a particular reason you need to expose an option for the method used? Usually people add options to select whether to enable the functionality at all...

By the way, marking it as required: false seems wrong. The configure.ac considers it mandatory, and the source code unconditionally tries to use it. All you're doing here is adding it to the list of deps -- you don't check whether it's available and set different preprocessor defines to e.g. avoid using it if it doesn't exist.

Yeah, required: false is wrong. It was something left over. Removed it.

The reason for the get_option is to be able to specify what version of iconv to use if you have multiple installed. This is something configure.ac included. Maybe it's considered bad practice or there is a better way to do it in now?

I have no idea what the original intention was, but the general rule of thumb is that you can either get the standalone gnu one (always an external library) or rely on the platform one (which may be a builtin or an external library, and for glibc it's also the same underlying codebase as the gnu one).

Which one you use isn't much of an option, though, because they both require the same iconv.h header and if you have both available then the gnu one overrides the header search path. It also overrides the symbol names, so if you don't perform a link verification test using the header (the dependency() provided by meson does this for you) you may end up compiling against the default header which is the gnu one, but linking to an incompatible BSD or musl one that is built into your libc.

The only choice you have really is to either:

use the one picked up by auto

return an error or simply disable iconv support if you didn't get the one you wanted

modify CFLAGS and LDFLAGS before running meson, to prioritize a custom copy in the location you prefer

You're effectively offering an opt-in for option 2 (return a fatal error). But I traditionally wonder: does it actually matter which iconv you find?

I have no idea what the original intention was, but the general rule of thumb is that you can either get the standalone gnu one (always an external library) or rely on the platform one (which may be a builtin or an external library, and for glibc it's also the same underlying codebase as the gnu one).

This was the original intention, yes.

eli-schwartz · 2023-02-12T05:56:18Z

meson.build

+if cc.has_header('getopt.h')
+  if cc.has_function('getopt_long')
+    conf_data.set('HAVE_GETOPT_H', 1)
+  endif
+endif


Why check both has_header and has_function? You could do both at the same time by checking either:

cc.has_function('getopt_long', prefix: '#include <getopt.h>')

or, to do a compile test but not a link test,

cc.has_header_symbol('getopt.h', 'getopt_long')

This saves on one fork+exec to the compiler, saving a bit of time and making the configure stage a bit more responsive, and is also more accurate as a side bonus.

True. Fixed

eli-schwartz · 2023-02-12T06:10:56Z

meson.build

+if get_option('have_iconv_detect_h')
+  conf_data.set('HAVE_ICONV_DETECT_H', 1)
+else
+  # not cross-compiler secure
+  root = include_directories('.')
+  has_detect = cc.run(iconv_detect, name: 'iconv-detect', 
+    include_directories: root).returncode() == 0
+  if has_detect
+    conf_data.set('HAVE_ICONV_DETECT_H', 1)
+  endif
+endif


Meson can absolutely handle this with cross compilation, you just have to define an exe_wrapper (usually qemu-user). If you don't, meson errors out with "Can not run test applications in this cross environment." which... I suppose could be enhanced to mention exe_wrapper, hmm.

But this is a pretty awkward way to do things here. You're running a test program in the source directory and emitting unclean output into the source directory instead of the build directory -- this violates Meson's guarantee of out-of-source build directories, potentially messing up situations such as read-only mounted source directories or having multiple build directories, one native one cross compiled, which need different generated headers.

Yeah. Adding an error if we can't run.

In the long run I think it might be better to not pollute the system. The reason for me to "port the awkwardness" over is to not disrupt the autotools build which can require this file to exist.

As someone who cross-compiles daily, remember to include a fallback for cross and can't run by checking for a property in the cross file.

To be fair, this code only runs in the else branch of a get_option()...

eli-schwartz · 2023-02-12T06:16:26Z

meson.build

+foreach hd : [
+    { 'm': 'HAVE_DLFCN_H',            'v': 'dlfcn.h', },
+    { 'm': 'HAVE_INTTYPES_H',         'v': 'inttypes.h', },
+    { 'm': 'HAVE_NETDB_H',            'v': 'netdb.h', },
+    { 'm': 'HAVE_STDINT_H',           'v': 'stdint.h', },
+    { 'm': 'HAVE_WINDOWS_H',          'v': 'windows.h'},
+    { 'm': 'HAVE_SYS_MMAN_H',         'v': 'sys/mman.h', },
+    { 'm': 'HAVE_SYS_PARAM_H',        'v': 'sys/param.h', },
+  ]
+  if cc.has_header(hd['v'])
+    conf_data.set10(hd['m'], true)
+  endif
+endforeach


This could be a flat list of headers. e.g. if you use Meson's tools/ac_converter.h, it suggests the following:

cc = meson.get_compiler('c') cdata = configuration_data() check_headers = [ 'dlfcn.h', 'getopt.h', 'iconv/detect.h', 'inttypes.h', 'netdb.h', 'poll.h', 'stdint.h', 'stdio.h', 'stdlib.h', 'strings.h', 'string.h', 'sys/mman.h', 'sys/param.h', 'sys/stat.h', 'sys/types.h', 'time.h', 'unistd.h', 'winsock2.h', ] foreach h : check_headers if cc.has_header(h) cdata.set('HAVE_' + h.underscorify().to_upper(), 1) endif endforeach check_functions = [ ['HAVE_FSYNC', 'fsync', '#include<unistd.h>'], ['HAVE_GETADDRINFO', 'getaddrinfo', '#include<netdb.h>'], # check token ['HAVE_GETDOMAINNAME'] # check token ['HAVE_GETHOSTNAME'] ['HAVE_GETPAGESIZE', 'getpagesize', '#include<unistd.h>'], ['HAVE_MMAP', 'mmap', '#include<sys/mman.h>'], # check token ['HAVE_MSYNC'] # check token ['HAVE_MUNMAP'] ['HAVE_POLL', 'poll', '#include<poll.h>'], ['HAVE_SELECT', 'select', '#include<sys/select.h>'], # check token ['HAVE_UTSNAME_DOMAINNAME'] ] foreach f : check_functions if cc.has_function(f.get(1), prefix : f.get(2)) cdata.set(f.get(0), 1) endif endforeach cdata.set('SIZEOF_OFF_T', cc.sizeof('off t')) cdata.set('SIZEOF_SIZE_T', cc.sizeof('size_t')) cdata.set('SIZEOF_SSIZE_T', cc.sizeof('ssize_t')) cdata.set('SIZEOF_TIME_T', cc.sizeof('time t')) configure_file(input : 'config.h.meson', output : 'config.h', configuration : cdata)

Note the use of:

cdata.set('HAVE_' + h.underscorify().to_upper(), 1)

Cool, never heard of that tool. Fixed

eli-schwartz · 2023-02-12T06:22:19Z

gmime/meson.build

+  requires: LIBS,
+  # libraries_private: EXTRA_LIBS,


LIBS is a list of strings. Consider using the returned dependency() objects as those automatically do "the right thing" (including enforce the same version constraints you used when looking up the dependency to begin with).

eli-schwartz · 2023-02-12T06:29:40Z

gmime/meson.build

+gen_table = executable('gen-table', 'gen-table.c')
+charset_map = executable('charset-map', 'charset-map.c', dependencies : deps, 
+  include_directories: inc_conf)


Since cross compilation came up... you can mark an executable as native: true to make it be compiled with the build machine compiler instead of the cross-compiler, although if you need an exe_wrapper anyway that might not matter so much.

eli-schwartz · 2023-02-12T06:35:06Z

meson.build

+]
+
+gmime_prefix = get_option('prefix')
+gmime_datadir = join_paths(gmime_prefix, get_option('datadir'))


This doesn't appear to ever be used, nor can I find anything in the source code or autotools that it would correspond to.

Ah. Those are used for the docs, which are not included in this PR. Removing.

jstedfast · 2024-06-14T13:41:00Z

First, sorry for taking so long to get around to this, I'm just so busy these days and have multiple projects that I maintain in my limited free time.

I also have no knowledge of the meson build system or what GNOME is doing these days.

That said, I just had the most awful experience trying to build gmime on macOS (the closest thing I have to a suitable for gmime development environment) which is what made me think of this PR that I'd been meaning to get around to.

I just installed meson from brew on my macOS machine and cloned this branch.

How do I test this?

eli-schwartz · 2024-06-14T13:47:23Z

tl;dr

# replaces ./configure
meson setup builddir/

# replaces /configure --enable-foo
meson setup builddir/ -Dfoo=enabled

# replaces ./configure --help
meson configure

# replaces make
ninja

jstedfast · 2024-06-14T13:58:57Z

[jestedfa@localhost gmime]$ meson setup builddir && cd builddir
The Meson build system
Version: 1.4.1
Source dir: /Users/jestedfa/src/gmime
Build dir: /Users/jestedfa/src/gmime/builddir
Build type: native build
Project name: GMime
Project version: 3.2.13
C compiler for the host machine: cc (clang 15.0.0 "Apple clang version 15.0.0 (clang-1500.3.9.4)")
C linker for the host machine: cc ld64 1053.12
Host machine cpu family: aarch64
Host machine cpu: aarch64
Run-time dependency iconv found: YES
Found pkg-config: YES (/opt/homebrew/bin/pkg-config) 0.29.2
Run-time dependency libidn2 found: YES 2.3.7
Run-time dependency gpgme found: YES 1.23.2
Run-time dependency glib-2.0 found: YES 2.80.3
Run-time dependency gobject-2.0 found: YES 2.80.3
Run-time dependency gio-2.0 found: YES 2.80.3
Run-time dependency zlib found: YES 1.2.11
Header "getopt.h" has symbol "getopt_long" : YES 
Checking if "iconv-detect" runs: DID NOT COMPILE
Has header "dlfcn.h" : YES 
Has header "getopt.h" : YES 
Has header "inttypes.h" : YES 
Has header "netdb.h" : YES 
Has header "poll.h" : YES 
Has header "stdint.h" : YES 
Has header "stdio.h" : YES 
Has header "stdlib.h" : YES 
Has header "strings.h" : YES 
Has header "string.h" : YES 
Has header "sys/mman.h" : YES 
Has header "sys/param.h" : YES 
Has header "sys/stat.h" : YES 
Has header "sys/types.h" : YES 
Has header "time.h" : YES 
Has header "unistd.h" : YES 
Has header "winsock2.h" : NO 
Checking whether type "struct utsname" has member "domainname" : NO 
Checking for function "fsync" : YES 
Checking for function "getaddrinfo" : YES 
Checking for function "getdomainname" : YES 
Checking for function "gethostname" : YES 
Checking for function "getpagesize" : YES 
Checking for function "mmap" : YES 
Checking for function "msync" : YES 
Checking for function "munmap" : YES 
Checking for function "poll" : YES 
Checking for function "select" : YES 
Configuring config.h using configuration
Configuring gmime-version.h using configuration

gmime/meson.build:188:0: ERROR: File gmime-version.h does not exist.

A full log can be found at /Users/jestedfa/src/gmime/builddir/meson-logs/meson-log.txt

jstedfast · 2024-06-14T14:00:25Z

Looks like the meson setup logic assumes gmime-version.h exists from the onset, but it is/was autogenerated by the old configure script.

Presumably meson needs to auto-generate it as well.

jstedfast · 2024-06-14T14:03:09Z

Hmmm, looks like it should be doing that...

configure_file(input: 'gmime-version_meson.h.in',
  output: 'gmime-version.h',
  configuration : version_data
)

jstedfast · 2024-06-14T14:07:19Z

Okay, I'm guessing that it's because libgmime_headers contains gmime-version.h

I wonder if maybe inc_conf needs to include the meson build directory so that gmime-version.h can be picked up from there?

This add support for the meson build system which supports multiple operating systems among things out of the box. This requires jstedfast#141 to be merged, because gnome.generate_gir() treats warnings as errors.

This allows meson setup to work (because it auto-generates gmime-version.h, meaning it doesn't exist when meson is setting up the build).

jstedfast · 2024-06-14T14:32:14Z

Rebased this on top of the latest gmime to fix the gir errors that I was getting when trying to meson compile

jstedfast · 2024-06-14T14:39:20Z

Checking if "iconv-detect" runs: DID NOT COMPILE is a bit concerning.

Logs:

Code:
 #include <iconv.h>

int main() {
    iconv_open("","");
}
-----------
Command line: `cc /Users/jestedfa/src/gmime/builddir/meson-private/tmp4tsmc6zs/testfile.c -o /Users/jestedfa/src/gmime/builddir/meson-private/tmp4tsmc6zs/output.exe -O0 -Werror=implicit-function-declaration` -> 1
stderr:
Undefined symbols for architecture arm64:
  "_iconv_open", referenced from:
      _main in testfile-4a9b3c.o
ld: symbol(s) not found for architecture arm64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
-----------
Running compile:
Working directory:  /Users/jestedfa/src/gmime/builddir/meson-private/tmphax2pj_h
Code:
 
        #ifdef __has_include
         #if !__has_include("iconv.h")
          #error "Header 'iconv.h' could not be found"
         #endif
        #else
         #include <iconv.h>
        #endif
-----------
Command line: `cc /Users/jestedfa/src/gmime/builddir/meson-private/tmphax2pj_h/testfile.c -E -P -P -O0 -Werror=implicit-function-declaration` -> 0
Running compile:
Working directory:  /Users/jestedfa/src/gmime/builddir/meson-private/tmplrfdf0sy
Code:
 int main(void) { return 0; }

-----------
Command line: `cc /Users/jestedfa/src/gmime/builddir/meson-private/tmplrfdf0sy/testfile.c -o /Users/jestedfa/src/gmime/builddir/meson-private/tmplrfdf0sy/output.exe -O0 -Werror=implicit-function-declaration -liconv -Wl,-undefined,dynamic_lookup` -> 0
Run-time dependency iconv found: YES

…onv-detect

jstedfast · 2024-06-14T14:55:42Z

Okay, fixed that.

TODO:

need to be able to generate releases (e.g. gmime-{version}.tar.gz or tar.xz or whatever)
need to be able to generate documentation (@dagle - you mentioned above that that you planned to do?)

eli-schwartz · 2024-06-14T14:57:52Z

Releases can be done with meson dist -C builddir/ (equivalent to make distcheck).

eli-schwartz suggested changes Feb 12, 2023

View reviewed changes

dagle and others added 6 commits June 14, 2024 10:26

Support for the meson build system

cdf49ea

This add support for the meson build system which supports multiple operating systems among things out of the box. This requires jstedfast#141 to be merged, because gnome.generate_gir() treats warnings as errors.

Check if we can run the detect function

87ac7be

Fixes meson suggestion by eli

e0da278

Remove gmime-version.h from libgmime_headers

b7e9ec1

This allows meson setup to work (because it auto-generates gmime-version.h, meaning it doesn't exist when meson is setting up the build).

Removed gmime-version_meson.h.in (use gmime-version.h.in instead)

e206215

Updated version to 3.2.14

de59634

jstedfast force-pushed the meson_merge branch from 1f637e3 to de59634 Compare June 14, 2024 14:31

Make sure to include the iconv dependency when trying to build/run ic…

9a7458c

…onv-detect


		add_global_arguments('-DHAVE_CONFIG_H=1', language : 'c')

		iconv = dependency('iconv', required : false, method: get_option('iconv'))

Support for the meson build system #142

Are you sure you want to change the base?

Support for the meson build system #142

Uh oh!

Conversation

dagle commented Feb 10, 2023

Uh oh!

dagle commented Feb 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jstedfast commented Jun 14, 2024

Uh oh!

eli-schwartz commented Jun 14, 2024

Uh oh!

jstedfast commented Jun 14, 2024

Uh oh!

jstedfast commented Jun 14, 2024

Uh oh!

jstedfast commented Jun 14, 2024

Uh oh!

jstedfast commented Jun 14, 2024

Uh oh!

jstedfast commented Jun 14, 2024

Uh oh!

jstedfast commented Jun 14, 2024

Uh oh!

jstedfast commented Jun 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eli-schwartz commented Jun 14, 2024

Uh oh!

Uh oh!

dagle commented Feb 10, 2023 •

edited

Loading

jstedfast commented Jun 14, 2024 •

edited

Loading