Opened 6 years ago

Closed 3 years ago

#9419 closed defect (implemented)

update tordnsel to work with debian wheezy ghc libs

Reported by: phobos Owned by:
Priority: Medium Milestone:
Component: Core Tor/TorDNSEL Version:
Severity: Normal Keywords:
Cc: david@…, lunar, tup, arlolra Actual Points:
Parent ID: #9417 Points:
Reviewer: Sponsor:

Description

tordnsel doesn't run on wheezy due to updated ghc libs, see https://lists.torproject.org/pipermail/tor-dev/2013-July/005157.html

We should update the code to work.

Child Tickets

Attachments (2)

p1.patch (238.8 KB) - added by dkm 6 years ago.
fix for wheezy
tordnsel.cabal.diff (1.2 KB) - added by arlolra 6 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 6 years ago by lunar

dkm has produced a version that, according to my tests, works fine on Wheezy. It is available from git://numm.org/tordnsel. I have installed the dependencies using cabal install.

(This time on the right ticket, sorry for the mess.)

comment:2 Changed 6 years ago by dkm

Fixed memory consumption on long runs.

It's being tested for a couple of weeks now, I think it's good to go.

I was probably also commenting on the wrong ticket about this, #9204, so noting it here.

comment:3 Changed 6 years ago by dkm

Cc: david@… added

Changed 6 years ago by dkm

Attachment: p1.patch added

fix for wheezy

comment:4 Changed 6 years ago by dkm

Replying to phobos:

Can you do a set of patches against the canonical tordnsel repo at https://gitweb.torproject.org/tordnsel.git ?

Attached.

Let's get this one out, then I'll post another batch to fix a bunch of deprecation errors left.

once I can merge the changes, we can release a new version, and try it out on our server. can everything be installed via apt-get? We can't do cabal on production machines, only apt-get. If it's not in a debian repo, we cannot install it.

Cabal is haskell's build infrastructure; you use it to compile regardless of whether you invoke the cabal binary or the Setup.hs scriptlet.

re: debian repo:

I added some additional code to cope with relatively old libraries in wheezy (so it builds exactly with libghc-XXX-dev libraries installed with apt), but it will break the build the next time libraries are revbumped.

Cabal can also fetch and compile all the needed haskell dependencies for a project. What's your take on having build-time deps that are not in debian repos, but are fully automatically fulfilled by the build system to produce a debian-packagable and deployable binary?

comment:5 Changed 6 years ago by dkm

Status: newneeds_review

comment:6 Changed 6 years ago by arma

Cc: lunar tup arlolra added

Was Lunar our haskell enthusiast here?

Or perhaps tup can take a quick look over it to see if it's what we want?

Switching tordnsel to wheezy will be nice since then we can upgrade sergii, and make Arlo's check2 deployment easier to maintain.

comment:7 Changed 6 years ago by arlolra

I spun up a VM with wheezy and ran the following,

apt-get install git ghc libghc-mtl-dev libghc-network-dev libghc-hunit-dev libghc-stm-dev libghc-conduit-dev
ghc --version
> The Glorious Glasgow Haskell Compilation System, version 7.4.1
git clone https://git.torproject.org/tordnsel.git
cd tordnsel
wget https://trac.torproject.org/projects/tor/raw-attachment/ticket/9419/p1.patch
git am p1.patch
./Setup.lhs configure
./Setup.lhs build

and, other than a few deprecation warnings, that seems to work.

I'm not sure how to run the test suite. runtests.hs seems to want for foreign function files (.hsc) compiled.

Changed 6 years ago by arlolra

Attachment: tordnsel.cabal.diff added

comment:8 Changed 6 years ago by arlolra

I've attached a patch by Nikita Karetnikov which builds runtests. Continuing with the above, the output is:

vagrant@debian-7:~/tordnsel/dist/build/runtests$ ./runtests
### Failure in: 0:0                        
expected: Just (fromList [("address","10.0.0.1"),("changerootdirectory","/srv/tordnsel/"),("dnslistenaddress","127.0.0.1:53"),("domainname","exitlist-ns.example.com."),("enableactivetesting","True"),("group","tordnsel"),("log","notice file /srv/tordnsel/log/"),("pidfile","/srv/tordnsel/run/tordnsel.pid"),("runasdaemon","True"),("runtimedirectory","/srv/tordnsel/run/"),("soarname","hostmaster.example.com."),("statedirectory","/state"),("testdestinationaddress","10.0.0.1:80,443,110,53,22,5190,6667,9030"),("testlistenaddress","10.0.0.1:80,443,110,53,22,5190,6667,9030"),("torcontroladdress","127.0.0.1:9051"),("torcontrolpassword","password"),("torsocksaddress","127.0.0.1:9050"),("user","tordnsel"),("zoneofauthority","exitlist.example.com.")])
 but got: Just (fromList [("address","10.0.0.1"),("changerootdirectory","/srv/tordnsel"),("dnslistenaddress","127.0.0.1:53"),("domainname","exitlist-ns.example.com."),("enableactivetesting","True"),("group","tordnsel"),("log","notice file /srv/tordnsel/log/"),("pidfile","/srv/tordnsel/run/tordnsel.pid"),("runasdaemon","True"),("runtimedirectory","/srv/tordnsel/run/"),("soarname","hostmaster.example.com."),("statedirectory","/state"),("testdestinationaddress","10.0.0.1:80,443,110,53,22,5190,6667,9030"),("testlistenaddress","10.0.0.1:80,443,110,53,22,5190,6667,9030"),("torcontroladdress","127.0.0.1:9051"),("torcontrolpassword","password"),("torsocksaddress","127.0.0.1:9050"),("user","tordnsel"),("zoneofauthority","exitlist.example.com.")])
Cases: 21  Tried: 21  Errors: 0  Failures: 1

comment:9 Changed 6 years ago by dkm

The difference is in canonicalization of directory paths: the failing first test is trying to verify config parsing and expects a certain trailing slash to be stripped.

The code to perform the stripping does not exist, however, and it wouldn't make any difference if it did.

Which is a symptom of the more basic fact that the tests haven't been kept up to date with the rest of the code, are very sparse to begin with, and not very useful in general. Especially since the proposed patch barely touches parsing.

Which is why I didn't rely on them while fixing the code.

The only way to actually test this is to create a basic config, run the program and use dig or such to establish that the responses are what they should be.

I've been meaning to write a short guide on how to set it up. But that procedure is not changed by the patch in question, so whoever already knows how to set up and run a tordnsel instance could just give it a go and see if it works?

comment:10 in reply to:  9 Changed 5 years ago by isis

Replying to dkm:

I've been meaning to write a short guide on how to set it up. But that procedure is not changed by the patch in question, so whoever already knows how to set up and run a tordnsel instance could just give it a go and see if it works?

I'm not sure that there's anyone left who knows tordnsel well enough to feel comfortable setting it up and accepting a patch, so something like a guide, or some docker/vagrant configs, for setting up a test environment would likely be quite helpful!

FWIW, I've met a person recently who expressed interest in working on tordnsel, but they seemed to be a bit discouraged that this ticket has gone without review for a year or so.

comment:11 Changed 3 years ago by arlolra

Severity: Normal

I merged these patches. Thanks dkm!

comment:12 Changed 3 years ago by arlolra

Resolution: implemented
Status: needs_reviewclosed
Note: See TracTickets for help on using tickets.