Bug 75938 - split wtmp code to separate file, fix on FreeBSD
Summary: split wtmp code to separate file, fix on FreeBSD
Status: RESOLVED FIXED
Alias: None
Product: accountsservice
Classification: Unclassified
Component: general (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Matthias Clasen
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-09 02:32 UTC by Allison Lortie (desrt)
Modified: 2014-03-19 15:54 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments
[PATCH 1/4] daemon: split wtmp code into separate file (19.36 KB, patch)
2014-03-09 02:33 UTC, Allison Lortie (desrt)
Details | Splinter Review
[PATCH 2/4] wtmp: split out the platform-specific bits (1.72 KB, patch)
2014-03-09 02:33 UTC, Allison Lortie (desrt)
Details | Splinter Review
[PATCH 3/4] wtmp: stop using private _PATH_WTMPX (2.11 KB, patch)
2014-03-09 02:33 UTC, Allison Lortie (desrt)
Details | Splinter Review
[PATCH 4/4] wtmp: unbreak the build on FreeBSD (798 bytes, patch)
2014-03-09 02:34 UTC, Allison Lortie (desrt)
Details | Splinter Review

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.