moving libaddns top-level and "merging" libcli/dns

[Alexander Bokovoy]

Hi Kai,

On Fri, May 25, 2012 at 10:05 AM, Kai Blin <kai at samba.org> wrote:
>>> I also think that we really, really should try to get rid of libaddns,
>>> and not mark it as "ok to be used project-wide" by moving it to the top
>>> level.
>> As you are doing a lot of DNS related activities, please look around
>> into not only DNS updates or DNS server but also into resolver code.
>> Up until recently there were four different resolver libraries across
>> Samba code base, all replicating each other with minor or major
>> differences in structures, parameters, and ways they call back into
>> system-provided resolver API. Making things available at a central
>> place (libaddns) was an attempt to get duplication down.
>
> That I am well aware of, and that is exactly why I started a canonical
> implementation based around an IDL-generated parser and an async client
> library. However I'm not paid to work on Samba, and need to focus the
> time I have on one feature at a time. But that doesn't meant I can't
> have an opinion on how our DNS code should be developed. libaddns, as
> far as I'm aware, is completely untested, blocking, and hand-marshalled.
Let me know when you analyse changes in those commits in more detail,
what particularly affects your code in Samba 4 in these changes apart
from moving files between subsystems. The only change we had done that
directly affects samba 4 and was required in order to allow building
without Heimdal is switch in source4/libcli/resolve to use code
provided by lib/addns when performing resolution operations. That code
has replaced roughly same code provided by Heimdal's libroken library
which did have same blocking behavior. This is why
source4/libcli/resolve implements forked resolver by itself, making
calls to actual resolution routines that end up calling system
routines in equivalent way.

All other changes were to make dependencies between subsystems untangled.

Also sorry to be harsh, but claiming the code in Samba 3 being
'untested' is a bit of a stretch.

>> I'm not against doing IDL-based DNS code at all. What I'm contesting
>> here is half-completeness, to the level that we even have some
>> structures defined twice with the same name and different content,
>> IDL-based and otherwise. Whether it comes from an old code or recent
>> additions, duplicate symbols and structures are welcoming to
>> confusion, both for developers and compilers/linkers.
>
> libaddns was used in source3 only, and I didn't want to touch that until
> the libcli/dns implementation was more advanced. Don't blame me for the
> fact that source3 has some DNS code duplicated, part of that has been
> around for longer than I have been a project member.
Since we are dealing with a common source tree, there is no virtual
fences. We are responsible together for all the code.

> I'm saying that by putting a library in the top level, it's your job to
> make sure it's clean and doesn't break other stuff that already is in
> the top level.
I'm sorry for making your life complicated by moving the code around,
but is there any specific technical issue?

Could you please point exactly what this change broke in behavior
other than making your rebase harder? Could you please point me to
specific tests that show the broken behavior? Do we have those tests
in selftest/autobuild? Can we have them there?

>> So instead of killing libaddns' "common use", please look into how to
>> get unifying our disorganised resolvers. This does not mean it has to
>> be public library in terms of us exporting its interfaces to 3rd
>> parties, it only means that it should be used across Samba code,
>> without duplicated pieces of it here and there.
>
> Right, that's the plan for libcli/dns, and you're currently busy making
> this harder than necessary. If you don't have time to fix dns client
> side correctly, please don't make life harder for people who want to.
> I'm unclear what a scenario where merging two unrelated subsystems into
> one library is the right solution looks like, but the current one
> clearly isn't one of them.
I'm sorry to hear your claim on code being unrelated. Such claim is
definitely far from actual state of the functionality and
implementation that both subsystems bear.

-- 
/ Alexander Bokovoy