From: YASUOKA Masahiko <yasuoka@openbsd.org>
Subject: Re: develop/m4 fix make test
To: kn@openbsd.org
Cc: tb@openbsd.org, sthen@openbsd.org, ports@openbsd.org
Date: Thu, 22 May 2025 19:27:50 +0900

On Thu, 22 May 2025 09:30:54 +0000
Klemens Nanni <kn@openbsd.org> wrote:
> 22.05.2025 12:23, Theo Buehler пишет:
>> On Thu, May 22, 2025 at 09:06:17AM +0000, Klemens Nanni wrote:
>>> 22.05.2025 11:56, YASUOKA Masahiko пишет:
>>>> Hello,
>>>>
>>>> The diff makes "make test" pass on devel/m4.
>>>>
>>>> ok?
>> 
>> ok tb.
> 
> Either way I'm not objecting;  test fixes are always fine.
> 
> OK kn

Thanks,

>>>>
>>>> Index: Makefile
>>>> ===================================================================
>>>> RCS file: /cvs/ports/devel/m4/Makefile,v
>>>> diff -u -p -r1.41 Makefile
>>>> --- Makefile	22 Apr 2025 18:18:15 -0000	1.41
>>>> +++ Makefile	22 May 2025 08:54:54 -0000
>>>> @@ -4,7 +4,7 @@ DISTNAME=	m4-1.4.19
>>>>  CATEGORIES=	devel
>>>>  SITES=		${SITE_GNU:=m4/}
>>>>  EXTRACT_SUFX=	.tar.xz
>>>> -REVISION=	0
>>>> +REVISION=	1
>>>
>>> Looks like this is not needed since none of the patched files are in PLIST
>>> and package contents otherwise don't change, either.
>> 
>> The m4.info file generated from it changes.
>> 
>>>> +# $HOME must exist for cvs that is used by tests/test-vc-list-files-cvs.sh
>>>> +PORTHOME=      ${WRKDIR}
>>>
>>> Just exist or does it have to be writable?
>> 
>> Does it matter? this fixes a regress test.
>> 
>>>
>>>>  
>>>>  post-install:
>>>>  	${INSTALL_DATA_DIR} ${PREFIX}/share/examples/gm4
>>>> Index: patches/patch-doc_m4_texi
>>>> ===================================================================
>>>> RCS file: /cvs/ports/devel/m4/patches/patch-doc_m4_texi,v
>>>> diff -u -p -r1.4 patch-doc_m4_texi
>>>> --- patches/patch-doc_m4_texi	4 Dec 2024 09:51:29 -0000	1.4
>>>> +++ patches/patch-doc_m4_texi	22 May 2025 08:54:54 -0000
>>>> @@ -1,3 +1,7 @@
>>>> +Redirect stderr to /dev/null, since our /bin/sh prints "Killed" when a
>>>> +subprocess is died by a KILL signal.  Delete that message since that is
>>>
>>> is -> has, but you can probably just omit the " when ... signal." part.

ok, I'll s/is/has/.

>>>> +out of the scope of the test.
>>>> +
>>>
>>> So is this a cosmetic error or does something check stderr and thus fails?
>> 
>> regress runs it.

Yes.  The block basically is to test whether 'kill -9 $$' works for
the following tests.

>>> The former seems uneeded, the latter could be explained more clearly.
>>>
>>>>  Index: doc/m4.texi
>>>>  --- doc/m4.texi.orig
>>>>  +++ doc/m4.texi
>>>> @@ -10,3 +14,12 @@ Index: doc/m4.texi
>>>>   @end direntry
>>>>   
>>>>   @titlepage
>>>> +@@ -6756,7 +6756,7 @@ ifdef(`__unix__', ,
>>>> + ')m4exit(`77')')dnl
>>>> + changequote(`[', `]')
>>>> + @result{}
>>>> +-syscmd([/bin/sh -c 'kill -9 $$'; st=$?; test $st = 137 || test $st = 265])
>>>> ++syscmd([/bin/sh -c 'kill -9 $$'2>/dev/null; st=$?; test $st = 137 || test $st = 265])
>>>
>>> Better redirect inside the single quotes to only silence kill(1) and not the
>>> whole sh(1) process.
>> 
>> but it's the sh command doing -c  that is killed, so this is correct.
> 
> Then I misunderstood.
> 
>> 
>>>
>>>> + @result{}
>>>> + ifelse(sysval, [0], , [errprint([ skipping: shell does not send signal 9
>>>> + ])m4exit([77])])dnl
>>>> Index: patches/patch-tests_test-sys_wait_c
>>>> ===================================================================
>>>> RCS file: patches/patch-tests_test-sys_wait_c
>>>> diff -N patches/patch-tests_test-sys_wait_c
>>>> --- /dev/null	1 Jan 1970 00:00:00 -0000
>>>> +++ patches/patch-tests_test-sys_wait_c	22 May 2025 08:54:54 -0000
>>>> @@ -0,0 +1,13 @@
>>>> +workaround for #include puzzle
>>>
>>> Could you mention what fails and or how?
>>> Otherwise this is just a new puzzle for porters trying to understand the patch.

including <sys/time.h> is in an ad-hoc way.  I was not able to
understood the proper way to compose #include lines in m4 source
code.

The test code just needs to refer <sys/wait.h>.  The compile error was:

In file included from test-sys_wait.c:21:
In file included from ../lib/sys/wait.h:28:
In file included from /usr/include/sys/wait.h:39:
In file included from /usr/include/sys/siginfo.h:131:
In file included from ./sys/time.h:39:
In file included from /usr/include/sys/time.h:38:
In file included from ./sys/select.h:117:
In file included from ../lib/signal.h:52:
In file included from /usr/include/signal.h:38:
/usr/include/sys/signal.h:115:31: error: unknown type name 'siginfo_t'
                void    (*__sa_sigaction)(int, siginfo_t *, void *);

#include <sys/wait.h> causes including <sys/siginfo.h> then including
<signal.h>.  As the error above <signal.h> uses siginfo_t that is
defined in <sys/siginfo.h>.  This is a cycle.

#1: ../lib/sys/wait.h => 
#2: /usr/include/sys/wait.h =>
#3: /usr/include/sys/siginfo.h =>
#4: ./sys/time.h =>
#5: /usr/include/sys/time.h =>
#6: ./sys/select.h =>
#7: ../lib/signal.h =>
#8: /usr/include/signal.h => the error

including <sys/time.h> in advance avoids #4.  Then it breaks the cycle.

>>>
>>>> +
>>>> +Index: tests/test-sys_wait.c
>>>> +--- tests/test-sys_wait.c.orig
>>>> ++++ tests/test-sys_wait.c
>>>> +@@ -18,6 +18,7 @@
>>>> + 
>>>> + #include <config.h>
>>>> + 
>>>> ++#include <sys/time.h>
>>>> + #include <sys/wait.h>
>>>> + 
>>>> + /* Check for existence of required types.  */
>>>>
>>>
> 
>