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 <[email protected]> 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 <[email protected]> > > # 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>
