|Summary:||WTMPX_FILENAME is GNU specific|
|Product:||accountsservice||Reporter:||Kamil Rytarowski <n54>|
|Component:||general||Assignee:||Matthias Clasen <mclasen>|
|Status:||RESOLVED FIXED||QA Contact:|
|Priority:||medium||CC:||alan.coopersmith, marius.vollmer, n54, rstrode, stefw|
|i915 platform:||i915 features:|
patch reintroducing _PATH_WTMPX
Patch to define WTMPX_FILENAME on Solaris
Add OS specific handling for WTMPX
Alternative patch using autoconf checks
Updated patch that includes <utmpx.h> when checking for _PATH_WTMPX
Description Kamil Rytarowski 2015-06-06 14:13:24 UTC
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
Comment 1 Ray Strode [halfline] 2015-06-08 12:41:47 UTC
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.
Comment 2 Kamil Rytarowski 2015-06-08 19:33:03 UTC
(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.
Comment 3 Alan Coopersmith 2015-12-12 01:55:27 UTC
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.
Comment 4 Kamil Rytarowski 2015-12-12 02:08:25 UTC
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.
Comment 5 Kamil Rytarowski 2015-12-13 17:24:49 UTC
Created attachment 120489 [details] [review] Add OS specific handling for WTMPX Add OS specific handling for WTMPX. (FreeBSD, Linux, NetBSD, Solaris)
Comment 6 Kamil Rytarowski 2015-12-13 17:30:04 UTC
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.
Comment 7 Alan Coopersmith 2015-12-15 00:45:31 UTC
Hardcoding OS-specific knowledge seems much worse than checking for ifdefs as I did. This seems like the wrong direction to go.
Comment 8 Kamil Rytarowski 2015-12-15 00:54:09 UTC
(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.
Comment 9 Alan Coopersmith 2015-12-15 02:06:23 UTC
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.
Comment 10 Kamil Rytarowski 2015-12-15 02:44:10 UTC
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: 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"
Comment 11 Alan Coopersmith 2015-12-15 02:58:47 UTC
Created attachment 120509 [details] [review] Updated patch that includes <utmpx.h> when checking for _PATH_WTMPX
Comment 12 Kamil Rytarowski 2015-12-15 03:12:14 UTC
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 13 Kamil Rytarowski 2015-12-15 03:44:04 UTC
Comment on attachment 120489 [details] [review] Add OS specific handling for WTMPX Obsoleted by the patch by Alan.
Comment 14 Kamil Rytarowski 2016-01-27 18:32:54 UTC
Comment 15 Ray Strode [halfline] 2016-01-27 18:41:24 UTC
thanks guys, pushed.