Make kerberos use the regular sqlite
ClosedPublic
Actions

Authored by bapt on May 4 2015, 8:40 PM.

Details

Reviewers

bjk
stas
imp

Commits

rS283160: Make kerberos use the same sqlite libraries as other sqlite consumer.

Summary

Use sqlite instead of the bundled one for kerberos

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

bapt updated this revision to Diff 5189.May 4 2015, 8:40 PM

bapt retitled this revision from to Make kerberos use the regular sqlite.

bapt updated this object.

bapt edited the test plan for this revision. (Show Details)

bapt added a reviewer: imp.

Herald added a subscriber: imp. · View Herald TranscriptMay 4 2015, 8:40 PM

delphij added reviewers: bjk, stas.May 15 2015, 4:16 PM

delphij added a subscriber: delphij.

I believe the heimdal architecture is such that this swap should "just work", but should probably think about it a little bit more before accepting the revision.

bjk added inline comments.May 19 2015, 2:13 AM

Makefile.inc1
1805 ↗	(On Diff #5189)	This seems like it requires a bit more thought -- lib/libsqlite3/Makefile already has LIBADD+=pthread; it's not clear that this extra (conditional!) dependency is correct.
1821 ↗	(On Diff #5189)	Is there harm from unconditionally adding libsqlite3 to prebuild_libs? In any case, I do not think that "_libsqlite3" is the appropriate name for a variable conditional on MK_KERBEROS.

bapt added inline comments.May 19 2015, 9:09 AM

Makefile.inc1
1805 ↗	(On Diff #5189)	I have followed the exact same syntax that was used for libheimsqlite which also had the dependency on pthread defined on its Makefile. iirc those declaration are to avoid races when building the "prebuild" libs
1821 ↗	(On Diff #5189)	Just trying to be consistent with _libcom_err 2 lines below.

This looks good to my eye, but I'm surprised that sqlite3 isn't a private library.

In D2443#48131, @imp wrote:

This looks good to my eye, but I'm surprised that sqlite3 isn't a private library.

It is a private library

Looks good, thanks!

Makefile.inc1
1805 ↗	(On Diff #5189)	bapt is right, these lines in Makefile.inc1 are there to provide correct ordering. LIBADD does not affect build ordering, so it might end up trying to link agains libthr before it's actually available.
1821 ↗	(On Diff #5189)	As an alternative we can just introduce MK_SQLITE3 and make MK_KERBEROS set it to "yes". Not sure if it worth it. Is there anything else that uses sqlite3 at the moment?

This revision is now accepted and ready to land.May 19 2015, 11:54 PM

bapt added inline comments.May 20 2015, 8:02 AM

Makefile.inc1
1821 ↗	(On Diff #5189)	Currently the users for sqlite3 are: mandoc, kerberos and svnlite as soon as I switch makewhatis to mandoc version then there will be another consumer for prebuilt sqlite3