Created attachment 116332 [details] patch reintroducing _PATH_WTMPX WTMPX_FILENAME is a GNU extension. _PATH_WTMPX is available on NetBSD. As far as I know FreeBSD doesn't have it so it will not introduce problems there. There was a reply introducing problems on NetBSD: https://bugs.freedesktop.org/show_bug.cgi?id=75938
i'm okay with this patch, but would you be willing to move this (and the surrounding) logic to configure.ac. It would be good so that we could reduce the amount of #ifdef goop in the code.
(In reply to Ray Strode [halfline] from comment #1) > i'm okay with this patch, but would you be willing to move this (and the > surrounding) logic to configure.ac. It would be good so that we could > reduce the amount of #ifdef goop in the code. I will work on it.
Created attachment 120471 [details] [review] Patch to define WTMPX_FILENAME on Solaris I came up with this patch for the Solaris port before seeing this bug. It's less #ifdef goop, but not in configure.ac. I used the #define for WTMPX_FILE from the Solaris <utmpx.h>, but Solaris 11 and later also #defines _PATH_WTMPX in <paths.h> if that's more portable.
Thanks. Doing it via configure.ac is overkill and not really doable. We have different defines in a different set of headers. I resigned from doing it. I cannot just detect if I have a certain define in a certain file, as we have them differently. Going this route would make resulting file much worse #ifdef spaghetti I propose to go with a different route and delegate platform specific parts to platform specific files, for every supported platform.
Created attachment 120489 [details] [review] Add OS specific handling for WTMPX Add OS specific handling for WTMPX. (FreeBSD, Linux, NetBSD, Solaris)
I've proposed a patch. Please double check It was tested on NetBSD. I'm unsure about Solaris, whether it should be SunOS. There is also Illumos, SmartOS, OpenIndiana family actively maintained with pkgsrc. I'm looking forward to commit this change for next release.
Hardcoding OS-specific knowledge seems much worse than checking for ifdefs as I did. This seems like the wrong direction to go.
(In reply to Alan Coopersmith from comment #7) > Hardcoding OS-specific knowledge seems much worse than checking for ifdefs > as I did. This seems like the wrong direction to go. Please prepare a better patch to handle: Linux, FreeBSD, NetBSD and SunOS.
Created attachment 120508 [details] [review] Alternative patch using autoconf checks This patch is messier than a solution using #ifdef's in the C file would be, but cleaner than checking uname in configure to figure out which to use.
Thank you. It doesn't work on NetBSD: checking utmpx.h usability... yes checking utmpx.h presence... yes checking for utmpx.h... yes checking for fgetpwent... no checking for setutxdb... no checking whether WTMPX_FILENAME is declared... no checking whether _PATH_WTMPX is declared... no checking for /var/log/utx.log... no configure: error: in `/tmp/pkgsrc-tmp/wip/accountsservice/work/accountsservice-0.6.40': configure: error: Do not know which filename to watch for wtmp changes See `config.log' for more details *** Error code 1 Stop. make[1]: stopped in /usr/pkgsrc/wip/accountsservice *** Error code 1 Stop. There is need to check for utmpxname(3) and look for _PATH_WTMPX in utmpx.h /usr/include/utmpx.h:#define _PATH_WTMPX "/var/log/wtmpx"
Created attachment 120509 [details] [review] Updated patch that includes <utmpx.h> when checking for _PATH_WTMPX
Thank you! Your autotools-fu is very good. This patch worked for me and I pulled it to pkgsrc-wip: https://wip.pkgsrc.org/cgi-bin/gitweb.cgi?p=pkgsrc-wip.git;a=commitdiff;h=681a36ec17b9c099da669abc267e8ed10a6c730b
Comment on attachment 120489 [details] [review] Add OS specific handling for WTMPX Obsoleted by the patch by Alan.
ping!
thanks guys, pushed.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.