Collectd with Musl build failure since NUT version 2.8.0

[ffontaine]

collectd fails to link with nut on musl since version 2.8.0 and 4f760ba on:

configure:107607: checking for UPSCONN_t
configure:107607: /usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-1/output-1/host/bin/i586-linux-gcc -c -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64  -Os -g0  -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64  conftest.c >&5
In file included from conftest.c:155:
/usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-1/output-1/host/i586-buildroot-linux-musl/sysroot/usr/include/upsclient.h:102:87: error: unknown type name 'time_t'
  102 | ssize_t upscli_sendline_timeout(UPSCONN_t *ups, const char *buf, size_t buflen, const time_t timeout);
      |                                                                                       ^~~~~~

I don't know what is the proper way to fix this failure, timehead.h can't be included as this is an internal header that depends on config.h.

Full build log: http://autobuild.buildroot.org/results/22b758097e8fb72c68e41329cbc7abc748d81ca6/build-end.log

[jimklimov]

Thanks, this does not sound right indeed. A few weeks back I had some PRs to revise public headers (#1616, #1615) - does the problem still happen with current master-branch NUT?

What header(s) declare time_t on that platform? (at worst a fallback could be declared, but this one is a pretty basic common data type)

[jimklimov]

So, it seems that currently it would still at best include time.h indirectly, and PRs above did not touch this header - fixing that to be sure. Thanks for the report!

(Hm, guess our CI needs some test cases to build stuff against public headers and libs without knowledge of NUT build workspace)

[jimklimov]

As for the build log, I suppose most of the error noise about ups->conn as an int is because of C defaulting:

src/nut.c:40:2: error: #error "Unable to determine the UPS connection type."
   40 | #error "Unable to determine the UPS connection type."
      |  ^~~~~
src/nut.c:46:3: error: unknown type name 'collectd_upsconn_t'
   46 |   collectd_upsconn_t *conn;
      |   ^~~~~~~~~~~~~~~~~~

further due to this:

checking upsclient.h usability... yes
checking upsclient.h presence... yes
checking for upsclient.h... yes
checking for upscli_connect in -lupsclient... yes
checking for upscli_init in -lupsclient... yes
checking for upscli_tryconnect in -lupsclient... yes
checking for UPSCONN_t... no
checking for UPSCONN... no

Not sure why configure decides the header is usable if it then fails to parse/use it, I suppose - if it does not find the UPSCONN_t as usable. But the code build does not seem to fail because of the header syntax (consumer C file includes time.h elsewhere in the chain before upsclient.h?) - e.g. it knows to expect struct <anonymous> * here:

In file included from src/nut.c:33:
/usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-1/output-1/host/i586-buildroot-linux-musl/sysroot/usr/include/upsclient.h:82:13: note: expected 'UPSCONN_t *' {aka 'struct <anonymous> *'} but argument is of type 'int *'
   82 | const char *upscli_strerror(UPSCONN_t *ups);
      |             ^~~~~~~~~~~~~~

In fact, for that build log I do not see mention of time_t at all.

However some other changes to less loose data types, like uint16_t vs. earlier int for the port numbers may need to be fixed in the consumer codebase. At least that much of the change was "sort of" documented:
https://github.com/networkupstools/nut/blob/master/UPGRADING#L51-L59

[jimklimov]

Also FWIW there are some caveats noted in https://musl.libc.org/time64.html and the file is apparently here http://git.musl-libc.org/cgit/musl/tree/include/time.h

[jimklimov]

FWIW, recording a quick reproducer:

nut$ ./ci_build.sh && make DESTDIR=/tmp/.inst install

# cloned from https://github.com/collectd/collectd
collectd# ./build.sh ### generate configure script

collectd$ ./configure --disable-all-plugins --enable-nut \
    CFLAGS="-I/tmp/.inst/usr/local/ups/include/ " \
    CPPFLAGS="-I/tmp/.inst/usr/local/ups/include/ " \
    LDFLAGS="-L/tmp/.inst/usr/local/ups/lib " \
    PKG_CONFIG_PATH="/tmp/.inst/usr/local/ups/lib/pkgconfig"

[jimklimov]

Note: also posted a PR to collectd to fix the int-type discrepancies (actually to detect which types suit the NUT API the consumer is building against, and transparently use those in the consumer codebase).

[jimklimov]

@ffontaine : hopefully fixed on my side; if you can stage a build with NUT and collectd codebases as of PRs above, to see how it goes (if the issues are resolved by these modifications), that would be great.

UPDATE: The NUT change is in master branch now. Your collectd sources may need patch from that collectd/collectd#4043 PR.

[jimklimov]

@ffontaine : side note - at the https://collectd.org/wiki/index.php/Plugin:NUT plugin page, maybe makes sense to point "libupsclient" to https://networkupstools.org/docs/man/upsclient.html and not just site root?

[jimklimov]

@ffontaine : FYI : both current NUT and collectd "master" branch codebases should already include respective proposed fixes.

Please check that they play well together for your packaging effort, and feel free to close this issue if resolved.

And thanks for bringing this up :)

[ffontaine]

I'll wait for the next NUT release to test this as I'm unable to properly run autoreconf on buildroot side.

[jimklimov]

That might take a while, hope to make sense of CPS regression at least, before that. You can ./autogen.sh && ./configure && make dist on an asciidoc- and autoconf-capable system to produce a tarball in the meanwhile. You can configure --with-docs=skip if not caring for man pages and/or lacking asciidoc.

[jimklimov]

Hopefully this can get properly tested after NUT 2.8.1 gets released and propagated to the environment in question.

[jimklimov]

@octo, @ffontaine : NUT v2.8.1 and recently v2.8.2 were released with recipe fixes mentioned above. Did you have a chance to test any of those?

[ffontaine]

Yes, everything is building fine on buildroot since https://git.buildroot.net/buildroot/commit/?id=02c8901791d8d65094ecf56d9773babe74dd1845 where we applied commit cafd779.

I'll close this issue.