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

[Alexander Bokovoy]

On Fri, May 25, 2012 at 7:58 AM, Kai Blin <kai at samba.org> wrote:
> On 2012-05-23 18:47, Alexander Bokovoy wrote:
>
> Hi Alexander,
>
> I just noticed this patchset because it broke some of my rebases..
>
>>        via  744f991 libcli/dns: make 'clidns' private library out of DNS code in WAF build
>
> What is this needed for? I don't understand the reason mentioned in the
> commit message.
>
>>     After consolidating DNS resolver code to lib/addns, there is one piece
>>     that still needs to be moved into a common DNS resolver library: DNS_HOSTS_FILE
>>     subsystem. Unfortunately, direct move would require lib/addns to depend on
>>     libcli/util/{ntstatus.h,werror.h} (provided by errors subsystem).
>>
>>     In addition, moving libcli/dns/* code to lib/addns/ would make conflicting
>>     the dns_tkey_record struct. The conflict comes from source4/dns_server/ and is due
>>     to use of IDL to define the struct. lib/addns/ library also provides its own definition
>>     so we either need to keep them in sync (rewrite code in lib/addns/ a bit) or
>>     depend on generated IDL headers.
>
> A while ago when I initially proposed the internal DNS implementation, I
> got quite some flak from Simo for duplicating this implementation, and
> one of the reasons I mentioned was that this would finally allow us to
> get rid of the multiple implementations of hand-marshalled DNS we
> already have in the tree. Moving libaddns top-level and then bending the
> new implementation around to make things still work is straight against
> the idea behind using the IDL-based async client library.
WAF build is strict on linking the same code twice in resulting
binaries. DNS_HOSTS_FILE subsystem was independently linked in
multiple places that, when their real dependencies were fixed, ended
up linked into the same binaries.

> Also, I'd like to point out that dns_host_file is a terrible, terrible
> hack and is not supposed to go into the LIBCLI_DNS library at all.
>
>>     Thus, making a private library and subsystem clidns is an intermediate step
>>     that allows to buy some time fore refactoring.
>
> Can you please, please discuss refactoring of code that is pretty recent
> with the person who wrote the code in the first place? Already now this
> patch forces me to clean up both my work-in-progress code as well as the
> already committed code to move that dns_host_file hack out of the client
> library.
>
> 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.

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.

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.
-- 
/ Alexander Bokovoy