Page MenuHomeFreeBSD

Make kerberos use the regular sqlite
ClosedPublic

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

Details

Summary

Use sqlite instead of the bundled one for kerberos

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; 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.
delphij added a subscriber: delphij.
bjk edited edge metadata.May 15 2015, 5:35 PM

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.

imp edited edge metadata.May 19 2015, 9:05 PM

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

bapt added a comment.May 19 2015, 9:26 PM
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

stas accepted this revision.May 19 2015, 11:54 PM
stas edited edge metadata.

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

bjk accepted this revision.May 20 2015, 3:17 PM
bjk edited edge metadata.
bjk added inline comments.
Makefile.inc1
1821 ↗(On Diff #5189)

I will accept this revision as-is, since it works, but would also be fine with unconditionally adding sqlite3 to prebuild_libs

This revision was automatically updated to reflect the committed changes.