Vendor import and integration for ldns-1.8.3

[khorben]

Dear FreeBSD developers,
While preparing the update of OpenSSL to version 3 in base, I figured some bugfixes upstream for ldns haven't made their way into the system yet. I have therefore prepared this vendor import for base, for which I also created a contrib/ldns/include/internal.h header as a workaround for build errors (internal function prototypes defined in .c files, corresponding functions unused in their original location). Another change had to do with the renaming of a function in the API (ldns_serial_arithmitics_gmtime_r vs ldns_serial_arithmetics_gmtime_r).
Since des@ has done the previous vendor imports for ldns, it'd probably also be nice to involve him for review before merging.
Thanks!
Sponsored by the FreeBSD Foundation

[emaste]

Cirrus didn't like it:

--- all_subdir_usr.bin/drill ---
ld.lld: error: undefined symbol: ldns_edns_option_list_new
>>> referenced by drill.c:465 (/tmp/cirrus-ci-build/contrib/ldns/drill/drill.c:465)
>>>               drill.o:(main)

ld.lld: error: undefined symbol: ldns_edns_new_from_data
>>> referenced by drill.c:468 (/tmp/cirrus-ci-build/contrib/ldns/drill/drill.c:468)
>>>               drill.o:(main)

ld.lld: error: undefined symbol: ldns_edns_option_list_push
>>> referenced by drill.c:475 (/tmp/cirrus-ci-build/contrib/ldns/drill/drill.c:475)
>>>               drill.o:(main)

[emaste]

CC @dag-erling

[khorben]

Cirrus didn't like it:

--- all_subdir_usr.bin/drill ---
ld.lld: error: undefined symbol: ldns_edns_option_list_new
>>> referenced by drill.c:465 (/tmp/cirrus-ci-build/contrib/ldns/drill/drill.c:465)
>>>               drill.o:(main)

ld.lld: error: undefined symbol: ldns_edns_new_from_data
>>> referenced by drill.c:468 (/tmp/cirrus-ci-build/contrib/ldns/drill/drill.c:468)
>>>               drill.o:(main)
[...]

Yes I forgot to add a new file, edns.c, into the build for libldns; I have pushed a fix for that.

[dag-erling]

FWIW NLNet are very receptive to patches so I would encourage you to fix any build issues you encounter properly and submit the patches upstream rather than just work around them.

[khorben]

FWIW NLNet are very receptive to patches so I would encourage you to fix any build issues you encounter properly and submit the patches upstream rather than just work around them.

Hi des, in this case the fixes are coming from ldns and probably need to be imported to FreeBSD. (see https://github.com/NLnetLabs/ldns/blob/173fbf303518d060e0d2bdc0bbd1830c0ec8f21d/Changelog#L65, https://github.com/NLnetLabs/ldns/blob/173fbf303518d060e0d2bdc0bbd1830c0ec8f21d/Changelog#L89)
Are you happy with the vendor import merge here?

[dag-erling]

I'm talking about what you described as workarounds for build issues.

[khorben]

I'm talking about what you described as workarounds for build issues.

One workaround is because of an existing fix upstream that doesn't play well with FreeBSD's toolchain and compilation settings, but will probably not make sense upstream (the renaming of a function).

Another question - since I am rebasing again on main - would be about drill/ldns.c, where FreeBSD apparently has a special case DRILL_REVERSE: that is not upstream; should we keep it or remove it? (you imported it in c6342fe2e90510d8d2296423f2ca92818a7b3d18 with 1.7.0).

[dag-erling]

No, that's upstream code.

[khorben]

No, that's upstream code.

I think I now understand what happened: it was imported with 1.6.13 (where it was upstream), modified with 1.7.0 (possibly with differences from upstream?) but then upstream removed it in 1.7.1. That would explain the conflict; in any case it looks safe to remove this code, which would match upstream. I could not find any references to this in src.git.

[khorben]

I think the workaround about missing prototypes for internal API calls is worth submitting upstream; I am currently breaking it down into a separate commit here. It should be brought as a single commit together with the vendor import in FreeBSD though.

[khorben]

I think the workaround about missing prototypes for internal API calls is worth submitting upstream; I am currently breaking it down into a separate commit here. It should be brought as a single commit together with the vendor import in FreeBSD though.

I managed to combine the two workarounds, into what I believe should be a single fix worth submitting upstream.

[khorben]

I managed to combine the two workarounds, into what I believe should be a single fix worth submitting upstream.

This is now done in NLnetLabs/ldns#214.

[dag-erling]

See https://reviews.freebsd.org/D40226

[khorben]

The underlying issue for the GCC 12 compilation fix was submitted upstream at NLnetLabs/ldns#215.

[khorben]

@dag-erling I have largely simplified the diff with your help from NLnetLabs/ldns#214; thanks!

[dag-erling]

There are several other issues with this PR, please see the Phabricator review I posted which is only missing the gcc fix.

[khorben]

There are several other issues with this PR, please see the Phabricator review I posted which is only missing the gcc fix.

I read through the Phabricator review but I don't see anything else than your comment "Remove man pages.", for which I can't find the corresponding changes. What's still missing? (I'm new to Phabricator)

[dag-erling]

My version:

• does not add hundreds of unnecessary files
• builds drill correctly
• updates freebsd-configure.sh

[emaste]

@dag-erling updated ldns in the base system in 5afab0e