Bug 75938

Summary: split wtmp code to separate file, fix on FreeBSD
Product: accountsservice Reporter: Allison Lortie (desrt) <desrt>
Component: generalAssignee: Matthias Clasen <mclasen>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: medium CC: marius.vollmer, rstrode, stefw
Version: unspecified   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments: [PATCH 1/4] daemon: split wtmp code into separate file
[PATCH 2/4] wtmp: split out the platform-specific bits
[PATCH 3/4] wtmp: stop using private _PATH_WTMPX
[PATCH 4/4] wtmp: unbreak the build on FreeBSD

Description Allison Lortie (desrt) 2014-03-09 02:32:36 UTC
The code for getting the list of a user's last logins is complicated and it lies smack in the middle of daemon.c.

It also currently fails to build on FreeBSD due to an unconditional use of a privately-defined and non-standard _WTMP_PATH (to setup the file change monitor).

Let's split this code out to a separate file and make it build again on FreeBSD.
Comment 1 Allison Lortie (desrt) 2014-03-09 02:33:06 UTC
Created attachment 95389 [details] [review]
[PATCH 1/4] daemon: split wtmp code into separate file
Comment 2 Allison Lortie (desrt) 2014-03-09 02:33:29 UTC
Created attachment 95390 [details] [review]
[PATCH 2/4] wtmp: split out the platform-specific bits
Comment 3 Allison Lortie (desrt) 2014-03-09 02:33:51 UTC
Created attachment 95391 [details] [review]
[PATCH 3/4] wtmp: stop using private _PATH_WTMPX
Comment 4 Allison Lortie (desrt) 2014-03-09 02:34:18 UTC
Created attachment 95392 [details] [review]
[PATCH 4/4] wtmp: unbreak the build on FreeBSD
Comment 5 Stef Walter 2014-03-10 10:18:52 UTC
Comment on attachment 95392 [details] [review]
[PATCH 4/4] wtmp: unbreak the build on FreeBSD

Review of attachment 95392 [details] [review]:
-----------------------------------------------------------------

::: src/wtmp-helper.c
@@ +219,3 @@
>  #else
>  #error Do not know which filename to watch for wtmp changes
>  #endif

It seems like this sorta thing should go into configure logic. Discovering missing dependencies and incompatibilities half-way through the build isn't the best.

We can AC_DEFINE_UNQUOTED(WTMPX_FILENAME ...) in configure.
Comment 6 Allison Lortie (desrt) 2014-03-11 22:34:32 UTC
(In reply to comment #5)
> We can AC_DEFINE_UNQUOTED(WTMPX_FILENAME ...) in configure.

The reason I didn't do it this way is that it's a bit more complicated than that.  There is a wtmp filename for purposes of switching utmpx mode (which is what we need to do on Linux) but there is also a wtmp filename for purposes of monitoring.

This also isn't really a matter of differing configurations or missing build dependencies.  This is a "differnet OSes" sort of thing.

imho, it's fine to #error out part way though a build (ideally with a clear indication why) when being built on a platform where your package is not yet supported... This is "initial porter to a new platform" territory -- not "jhbuilder forgot to install sysdeps" sort of thing.
Comment 7 Allison Lortie (desrt) 2014-03-19 15:54:46 UTC
Pushed (after discussion on IRC)

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.