Meson fixes

[Kangie]

A collection of fixes for differences to the autotools build as noted by @somiaj.

Happy to drop, amend, squash, or split off any of these commits as directed.

• We could probably drop libs/asprintf.c
• Can't see where SETPGRP_VOID is supposed to come from?
• Thoughts on USE_LIBICONV?

[ThomasAdam]

You can drop this commit; it's covered on Linux by _DEFAULT_SOURCE which we define.

[Kangie]

I have removed HAVE_MKSTEMP however kept the tidyup and still define HAVE_SAFETY_MKSTEMP - As far as I can tell if we're relying on _DEFAULT_SOURCE we're POSIX 2008 which is safer (at least based on the man) down the line we may be able to drop the 'host does not have safe mkstemp' path.

I am happy to copy the code check from autotools if you would prefer that we properly check for all platforms :)

[ThomasAdam]

@Kangie

I'm OK with this.

Drop the commit C89: Assume RETSIGTYPE void and SIGNAL_RETURN return -- I'm aware of the difference; don't want to change it right now.

Re: making xent a default dependency, you'll need to update README.md as well.

The wait3 change -- I had this in a previous commit; looks like it was never cherry-picked over when we were reviewing this. That's frustrating.

As for SETPGRP -- what don't you understand? We're currently not checking for it in meson.build, and we should be. It's currently not setting the process group when we call Exec. This should really be happening.

I'm not sure why we can drop libs/asprintf.c -- on MacOS, you won't find it. Also on other systems as well. We're keeping it.

I don't see anything in here about LIB_ICONV-- what was your question?

[somiaj]

@ThomasAdam, libs/Ficonv.c checks for USE_LIBICONV and _LIBIONCV_H, which seems autotools has an older check for. So the question is does meson also need to do this, or is this just an older check that isn't needed for any modern implementation of libc which seems iconv is native. Though I think we only looked at glibc and musl.

[ThomasAdam]

I'll check later on, but FreeBSD needed this.

[Kangie]

Drop the commit C89: Assume RETSIGTYPE void and SIGNAL_RETURN return -- I'm aware of the difference; don't want to change it right now.

Done. I made it into its own PR, but no need to merge that in the short term (or at all).

Re: making xent a default dependency, you'll need to update README.md as well.

Xext is already listed; the meson build was defective. I haven't checked if it's fatal on autotools - I assume that "automagic" ensures that it's pulled in most of the time.

The wait3 change -- I had this in a previous commit; looks like it was never cherry-picked over when we were reviewing this. That's frustrating.

Sorry, I ended up clobbering a bunch of my commits when I thought I'd been working on my laptop but had actually been on my dev desktop. Thought I'd fixed all the changes but obviously missed that one.

As for SETPGRP -- what don't you understand? We're currently not checking for it in meson.build, and we should be. It's currently not setting the process group when we call Exec. This should really be happening.

We do check for setpgrp and set HAVE_SETPGRP if found.

fvwm3/meson.build

Lines 146 to 147 in 1067a72

	'setpgid': {},
	'setpgrp': {},

I don't see where SETPGRP_VOID comes from (I can't see anything in the commit that added it either). Is there some missing check?

fvwm3/libs/setpgrp.c

Lines 34 to 36 in 1067a72

	# ifdef HAVE_SETPGRP
	# ifdef SETPGRP_VOID
	rc = setpgrp();

This is currently based on the CI PR's branch; if that gets merged the diff should get down to only the changes we are about here.

[ThomasAdam]

@Kangie

I think SETPGRP_VOID comes from Solaris -- so something we can ignore. It's never going to be defined on most platforms that are currently being used.

[Kangie]

Ack. Dropped the CI commit. If you're happy with HAVE_ICONV as-is this is good to merge.

If we need to get closer to current behaviour for BSD we could do something a bit like this:

if get_option('iconv').allowed() #set to auto or enabled
  iconv = dependency('iconv', required: false, method: 'builtin')
  if iconv.found()
    conf.set10('HAVE_ICONV', true)
  else
    iconv = dependency('iconv', required: true, method: 'system')
    conf.set10('HAVE_ICONV', true)
    conf.set10('USE_LIBICONV', true)
  endif
  # add to deps as usual here
endif

[Kangie]

I think SETPGRP_VOID comes from Solaris -- so something we can ignore. It's never going to be defined on most platforms that are currently being used.

Looks like it's the other way around; from the autoconf manual:

Macro: AC_FUNC_SETPGRP

If setpgrp takes no argument (the Posix version), define SETPGRP_VOID. Otherwise, it is the BSD version, which takes two process IDs as arguments. This macro does not check whether setpgrp exists at all; if you need to work in that situation, first call AC_CHECK_FUNC for setpgrp.

This macro is obsolescent, as current systems have a setpgrp whose signature conforms to Posix. New programs need not use this macro.

I'll just define it always if SETPGRP is found?

[ThomasAdam]

@Kangie -- Riiiight. All makes sense now, re: SETPGRP.

This looks good to go. Will merge.