Vendor the OCaml DNS code

djs55 commented 3 years ago

Previously we used 2 libraries:

However:

the upstream ocaml-dns is completely different from version 2.0.0 onwards. It's now based on a DNS library with different goals, prioritising standards compliance; while we prioritise backwards compatibility with old resolvers. If we updated to the new implementation we might expose bugs in old clients which we wouldn't be able to work around easily. It would be interesting to try upgrading later, after we have updated all the other dependencies.
the ocaml-dns-forward library was never used by anyone else. It's really an implementation of vpnkit-specific policy so should live in this repo anyway.

Furthermore

ocaml-dns contains lots of code we don't need and dependencies we don't want (e.g. hashcons). By vendoring it we can avoid the dependencies from the unused code.
probably a lot of ocaml-dns-forward is now unused, since we don't try to use it to talk to split-tunnel DNS servers in VPNs. It will be easier to trim if all the code lives in one place.

This is a required step on the plan to vendor all the OCaml dependencies, to make building easier (and on Windows: to make building possible again)

samoht commented 3 years ago

LGTM

avsm commented 3 years ago

Looks fine. You may want to add a (vendored_dirs) field to cover the dns directory. https://dune.readthedocs.io/en/stable/dune-files.html#vendored-dirs-since-1-11

djs55 commented 3 years ago

@avsm thanks, useful tip!

moby / vpnkit