Bug 41747 - Patches: make accountsservice build on FreeBSD
Summary: Patches: make accountsservice build on FreeBSD
Status: RESOLVED FIXED
Alias: None
Product: accountsservice
Classification: Unclassified
Component: general (show other bugs)
Version: unspecified
Hardware: Other FreeBSD
: medium normal
Assignee: Matthias Clasen
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-13 07:52 UTC by Ed Schouten
Modified: 2014-03-15 03:28 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments
[PATCH 1/4] Make use of utmpx optional and buildable on FreeBSD. (2.27 KB, patch)
2011-10-13 07:53 UTC, Ed Schouten
Details | Splinter Review
[PATCH 2/4] Make use of <shadow.h> optional. (2.19 KB, patch)
2011-10-13 07:54 UTC, Ed Schouten
Details | Splinter Review
[PATCH 3/4] Use <sys/wait.h>. (724 bytes, patch)
2011-10-13 07:54 UTC, Ed Schouten
Details | Splinter Review
[PATCH 4/4] Don't use the non-standard fgetpwent() if not available. (2.49 KB, patch)
2011-10-13 07:54 UTC, Ed Schouten
Details | Splinter Review
daemon: emulate fgetpwent() if we don't have it (4.97 KB, patch)
2014-03-13 23:00 UTC, Allison Lortie (desrt)
Details | Splinter Review
daemon: emulate fgetpwent() if we don't have it (4.64 KB, patch)
2014-03-14 16:28 UTC, Allison Lortie (desrt)
Details | Splinter Review

Description Ed Schouten 2011-10-13 07:52:39 UTC
It seems we need some patches to get accountsservice to build on FreeBSD. I have sent them to Matthias Clasen and Ray Strode personally, but they are rather busy at the moment, so I'm throwing them into Bugzilla. :-)
Comment 1 Ed Schouten 2011-10-13 07:53:40 UTC
Created attachment 52295 [details] [review]
[PATCH 1/4] Make use of utmpx optional and buildable on FreeBSD.
Comment 2 Ed Schouten 2011-10-13 07:54:03 UTC
Created attachment 52296 [details] [review]
[PATCH 2/4] Make use of <shadow.h> optional.
Comment 3 Ed Schouten 2011-10-13 07:54:28 UTC
Created attachment 52297 [details] [review]
[PATCH 3/4] Use <sys/wait.h>.
Comment 4 Ed Schouten 2011-10-13 07:54:44 UTC
Created attachment 52298 [details] [review]
[PATCH 4/4] Don't use the non-standard fgetpwent() if not available.
Comment 5 Matthias Clasen 2011-10-17 12:34:26 UTC
These look all fine to me, committed.
Comment 6 Ray Strode [halfline] 2012-06-12 08:12:06 UTC
Hey, I had to revert PATCH 4/4.  getpwent isn't right, we really only want local users.

I think in order for freebsd to work, we're going to need to get local users in some freebsd specific way (or potentially fallback to parsing /etc/passwd manually)
Comment 7 Ed Schouten 2012-06-12 08:26:08 UTC
Hi Ray,

(In reply to comment #6)
> Hey, I had to revert PATCH 4/4.  getpwent isn't right, we really only want
> local users.
> 
> I think in order for freebsd to work, we're going to need to get local users in
> some freebsd specific way (or potentially fallback to parsing /etc/passwd
> manually)

I'm not aware of any proper API for this. Applications normally shouldn't try to make a distinction between a local and a non-local user. I guess you'd better send an email to questions@FreeBSD.org or hackers@FreeBSD.org to get a good answer, but here are some random notes I can give you:

- Don't parse /etc/passwd. On FreeBSD, /etc/passwd only exists for compatibility for broken scripts that really have to parse a plain-text file.
- The easiest thing to do, is to open /etc/pwd.db. This file is a Berkeley DB (see dbopen(3)).

I don't have an internet connection at home right now (recently moved abroad, still waiting to get connected), so unfortunately I can't do a lot of open source hacking right now. Sorry!
Comment 8 Allison Lortie (desrt) 2014-03-13 23:00:42 UTC
Created attachment 95754 [details] [review]
daemon: emulate fgetpwent() if we don't have it

We use fgetpwent directly on /etc/passwd in order to ensure we only get
a list of local users (and not ones from the network directory service).
Unfortunately, this function is not commonly found on non-GNU systems.

Provide our own implementation of fgetpwent if the operating system does
not provide it.
Comment 9 Stef Walter 2014-03-14 13:26:57 UTC
Comment on attachment 52297 [details] [review]
[PATCH 3/4] Use <sys/wait.h>.

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

This patch looks good.
Comment 10 Stef Walter 2014-03-14 13:28:08 UTC
(In reply to comment #9)
> This patch looks good.

Er, wrong window...
Comment 11 Matthias Clasen 2014-03-14 14:28:30 UTC
Patch looks good to me too, fwiw.
Comment 12 Ray Strode [halfline] 2014-03-14 14:52:37 UTC
No real comments on the implementation. If it works, it's probably good enough!  I would just name it fgetpwent though, rather than add the as_ namespace in front.  Aside from the fact that none of the other daemon code has an as_ prefix, I think i'd like this to be a "tranparently fix the platform" type thing (kind of like glib does for printf functions)
Comment 13 Allison Lortie (desrt) 2014-03-14 16:28:19 UTC
Created attachment 95816 [details] [review]
daemon: emulate fgetpwent() if we don't have it

Changes as requested.  Please apply whichever one you prefer.
Comment 14 Allison Lortie (desrt) 2014-03-15 03:28:36 UTC
Thanks for the merge!


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.