Re: [new] devel/libfixposix

Hi Ingo,

I'm sorry for the delay. I started looking into it, but then I was
distracted by something and forgot to reply, please apologies.

Ingo Schwarze <schwarze@usta.de> writes:

> Hi Omar,
>
> Omar Polo wrote on Fri, Dec 03, 2021 at 08:00:03PM +0100:
>
>> Woops, I identified a correct issue in unistd.c, but the fix was wrong.
>> Updated tarball attached and upstream notified :)
>>
>> (update: upstream has accepted one of the two patches)
>>
>> Also enabling the built-in tests this time, I haven't noticed them the
>> first time because without --enable-tests `make test' was successful and
>> listed 0 tests available. (The mkstemp test fails because why check
>> what malloc returns?)
>
> Failing to check return values is bad, too, but not the root cause of
> the failure. That test is attempting to allocate (2^24 - 1) * 15 =
> 251658225 bytes of memory in a single malloc(3) call, about 240 MB,
> because on OpenBSD, TMP_MAX is 0x7fffffff. Running that particular
> test makes no sense until upstream fixes it.
>
> The test src/tests/buildinfo.bats is also broken. It assumes that,
> if you have a file "./utility" that start with the first line
> "#!/bin/sh" and a file "./argument" that starts with the first
> line "#!./utility" and then run "./argument", that the kernel
> would run "#!/bin/sh ./utility ./argument". But our kernel
> runs "#!/bin/sh ./argument" instead.

thanks for figuring this out! the mkstemp actually passes with a lower
NTEMPFILES, but it's pointless anyway as all it does is testing our
mkostemp(3), but for the buildinfo test I was a bit clueless (I haven't
throwed at it too much time neither to be honest)

> The buildinfo.bats issue is more or less unfixable because on the
> one hand, the automake infrastructure in Makefile.am and the
> script config/aux/tap-driver.sh require that the TESTS make
> variable must contain the name of the test *data* file, but then
> turn around and tell the kernel to *execute* that data file.
> To make that portable, upstream needs to completely restructure
> that maze of indirections and abstractions.
>
> On top of that, the tests require gmake(1) and bash(1) while
> nothing in the actual software requires those.
>
> So half the tests (2 out of 4) are broken and completely non-portable.
> The "select" test is completely trivial and certainly does not test
> select(2) in any meaningful way. So what remains as potentially
> working and non-trivial is one single test, the "spawn" test. I don't
> think that is worth depending on gmake and bash and all the trouble
> i described above. Consequently, i suggest to just forget about the
> tests until upstream gets their shit together and makes sure they
> are portable, work, and test some non-trivial features.
>
> I mean, come on, this is supposed to be a portability library -
> but all the same, the test suite is thoroughly Linux-only...

I agree on everything. It all started with a small library to allow a
common lisp extension for x11/stumpwm to work, but more I look into the
project itself the less I want to have it on my machine. I'm sorry for
wasting your time too on this.

> [...]
> * Add CONFIGURE_ENV += ac_cv_prog_GIT=
> That achieves the same as patch-config_m4_custom_checks_m4 ,
> but in a much simpler way. Do forget to remove that patch
> if you agree with this change.

(thanks for remembering me of the ac_cv_* variables. I always forget
that it's possible to override the checks that way)

> * Add a comment explaining why the test suite makes no sense
> just yet, with an incomplete list of changes that would be
> needed to enable it, all commented out, such that we don't
> forget what we already found out.
>
> * Add a comment about the autotools versions.
> I think someone should talk to upstream and find out
> which versions this software is actually designed for.
>
> Feel free to use part or all of this in any way you want.
> I don't want to commit it, but you might. :-)

I agree on everything :)

I'll make sure to report upstream every issue mentioned and work with
them until they're resolved, but for the port itself I'm less and less
sure I want to add it, at least in the current state.

Thanks,

Omar Polo

> Yours,
> Ingo
>
>
> # $OpenBSD$
>
> COMMENT = thin wrapper over libc functions and macros
>
> GH_ACCOUNT = sionescu
> GH_PROJECT = libfixposix
> GH_TAGNAME = v0.4.3
>
> SHARED_LIBS += fixposix 0.0 # 5.0
>
> CATEGORIES = devel
>
> MAINTAINER = Omar Polo <op@omarpolo.com>
>
> # Boost SL 1.0
> PERMIT_PACKAGE = Yes
>
> WANTLIB += pthread
>
> SEPARATE_BUILD = Yes
> CONFIGURE_STYLE = autoreconf
> CONFIGURE_ENV += ac_cv_prog_GIT=
>
> # The test suite is almost trivial, quite non-portable,
> # and utterly broken, so leave it disabled for now.
> # There are only four tests: two are broken and one is trivial.
> # The test suite requires gmake and bash.
> # The presence of the check(1) utility is tested by ./configure
> # unless the following magical incantation is used:
> #CONFIGURE_ARGS += --enable-tests CHECK_CFLAGS=' ' CHECK_LIBS=' '
> #TEST_DEPENDS = devel/check shells/bash
> #USE_GMAKE = Yes
>
> # These versions are not quite right, they are obviously too new
> # and throw multiple warnings. But the versions recommended by
> # upstream in README.md are too old and don't work at all...
> AUTOCONF_VERSION = 2.71
> AUTOMAKE_VERSION = 1.16
>
> .include <bsd.port.mk>