> It would be a bit sad if it fell to me to reviewing the haskell code
> alone, since I don't really know Haskell very well.
If so, don't spend too much time on the preliminary patches. I think
that quick review is enough at this point.
> I get that this is required for more warnings, though, and I think
> it's okay to turn it off temporarily.
Yes, '-Werror' converts all warnings into errors [1], but it also
hides useful messages sometimes. '-Werror' is "suitable for
development but not release. [...] The package will break silently
as soon as the next version of ghc adds a new warning, which generally
does happen every major release." [2]
> Is there a conditional build thing where we can get -Werror when
> developing, and turn it off for hackage?
Yep, take a look at [3] and [4]. (I'll ask someone about Hackage.)
What do you think about an attached patch? We can add other flags if
it's needed.
Example ('git pull' and 'git am' were omitted):
# ghc --version
The Glorious Glasgow Haskell Compilation System, version 6.12.1
# cabal --version
cabal-install version 1.16.0.2
using version 1.16.0 of the Cabal library
# ./Setup.lhs configure --user
[...]
Configuring TorDNSEL-0.1.1...
# ./Setup.lhs build
[...]
dist/build/tordnsel/tordnsel-tmp/TorDNSEL/Util.hs:1:11:
Warning: -#include is deprecated: No longer has any effect
[ 4 of 39] Compiling TorDNSEL.Util ( dist/build/tordnsel/tordnsel-tmp/TorDNSEL/Util.hs, dist/build/tordnsel/tordnsel-tmp/TorDNSEL/Util.o )
src/TorDNSEL/Util.hsc:143:23:
Module `GHC.Handle' does not export `fillReadBuffer'
src/TorDNSEL/Util.hsc:143:39:
Module `GHC.Handle' does not export `readCharFromBuffer'
src/TorDNSEL/Util.hsc:145:26:
Module `GHC.IOBase' does not export `Buffer(..)'
# ./Setup.lhs configure --user -f devel
[...]
# ./Setup.lhs build
[...]
dist/build/tordnsel/tordnsel-tmp/TorDNSEL/Main.hs:3:11:
Warning: -#include is deprecated: No longer has any effect
<no location info>:
Failing due to -Werror.
I don't think there is a need to push this patch right now. There
might be other issues (for instance, incorrect build dependencies.)
> Hm. I don't understand how the hgetLineN implementation can work. It
> looks like it reads N bytes unconditionally, then looks for the EOL
> marker in them , and returns everything before the EOL marker... but
> what does it do with everything after the EOL marker? It appears to
> me that if the EOL marker is not right at the end of the N bytes,
> those bytes would get lost.
Right, should it work differently? (Again, I haven't inspected
'hGetLine' yet.)
The current version allows to estimate a worst case scenario because
it reads N bytes unconditionally. But it might be better to look for
the EOL marker first, then check the number of bytes. What do you
think?
> Am I missing something there? Do the extra bytes get put back somehow?
No. Here is an example:
# cat > Main.hs
module Main where
import Data.ByteString
import qualified Data.ByteString.Char8 as B
import System.IO
-- | Read @n@ bytes from @handle@; strip @eol@ (e.g., @'B.pack' "\r\n"@)
-- and everything after it.
hGetLineN :: Handle -> ByteString -> Int -> IO ByteString
hGetLineN handle eol n = do
hSetBuffering handle LineBuffering
bStr <- B.hGet handle n
return $ fst $ B.breakSubstring eol bStr
main = do
handle <- openFile "test.txt" ReadMode
hGetLineN handle (B.pack "\n") 42
# echo -e "foo\nbar\nbaz" > test.txt
# runhaskell Main
"foo"
Does it answer your question?
(FYI: a chapter about I/O [5], an introduction to bytestrings [6], and
a search engine [7].)
>> If you apply both patches (with GHC 7.6.3), the following errors will
>> appear:
>>
>> [ 3 of 39] Compiling TorDNSEL.Compat.Exception ( src/TorDNSEL/Compat/Exception.hs, dist/build/tordnsel/tordnsel-tmp/TorDNSEL/Compat/Exception.o ) [dist/build/autogen/cabal_macros.h changed]
>> [ 4 of 39] Compiling TorDNSEL.System.Timeout ( src/TorDNSEL/System/Timeout.hs, dist/build/tordnsel/tordnsel-tmp/TorDNSEL/System/Timeout.o )
> I don't see the errors there... did they go to stderr or something?
I was a bit sleepy and messed it up. Here is the right output:
src/TorDNSEL/System/Timeout.hs:56:47:
Module `TorDNSEL.Compat.Exception' does not export `throwDynTo'
src/TorDNSEL/System/Timeout.hs:56:59:
Module `TorDNSEL.Compat.Exception' does not export `dynExceptions'
> Seems reasonable. It might also be good to have a publicly git
> repository somewhere where you work on the branch, that can store all
> of the preliminary repositories.
Can I create a new branch here [8]? If so, should I also send patches
and comments to this list, or will it only annoy everyone?
[1] http://www.haskell.org/ghc/docs/7.6.3/html/users_guide/flag-reference.html
[2] http://hackage.haskell.org/trac/hackage/ticket/191
[3] http://www.haskell.org/cabal/users-guide/developing-packages.html#configurations
[4] http://www.haskell.org/cabal/users-guide/installing-packages.html#controlling-flag-assignments
[5] http://learnyouahaskell.com/input-and-output#hello-world
[6] https://www.fpcomplete.com/school/pick-of-the-week/bytestring-bits-and-pieces
[7] http://www.haskell.org/hoogle/
[8] https://gitweb.torproject.org/tordnsel.git
From a54dbe1d21ba44b2ff108182be809c3096d918d7 Mon Sep 17 00:00:00 2001 From: Nikita Karetnikov <nikita@xxxxxxxxxxxxxx> Date: Fri, 21 Jun 2013 19:31:28 +0000 Subject: [PATCH] Add 'Flag Devel' to 'tordnsel.cabal', and update its layout. --- tordnsel.cabal | 159 +++++++++++++++++++++++++++++++------------------------- 1 files changed, 88 insertions(+), 71 deletions(-) diff --git a/tordnsel.cabal b/tordnsel.cabal index 243e026..09a273e 100644 --- a/tordnsel.cabal +++ b/tordnsel.cabal @@ -1,74 +1,91 @@ -Name: TorDNSEL -Version: 0.1.1-dev -Synopsis: provides a DNSBL-style interface for detecting Tor exit nodes -Description: TorDNSEL implements a design for a DNSBL-style interface for - detecting connections exiting the Tor network. This design and its rationale - is described at <https://gitweb.torproject.org/tordnsel.git/blob/HEAD:/doc/torel-design.txt>. -License: PublicDomain -License-File: LICENSE -Homepage: http://p56soo2ibjkx23xo.onion/ -Package-URL: https://archive.torproject.org/tor-package-archive/tordnsel/tordnsel-0.1.1.tar.gz -Author: tup -Maintainer: tup.tuple@xxxxxxxxxxxxxx, lunar@xxxxxxxxxx, andrew@xxxxxxxxxxxxxx -Build-Type: Simple -Build-Depends: base>=2.0, network>=2.0, mtl>=1.0, unix>=1.0, stm>=2.0, - time>=1.0, HUnit>=1.1, binary>=0.4, bytestring>=0.9, array>=0.1, directory>=1.0, - containers>=0.1 -Tested-With: GHC==6.6, GHC==6.8, GHC==6.10, GHC==6.12 -Data-Files: config/tordnsel.conf.sample, contrib/cacti-input.pl, +Name: TorDNSEL +Version: 0.1.1-dev +Cabal-Version: >= 1.2 +Synopsis: DNSBL-style interface for detecting Tor exit nodes +Description: TorDNSEL implements a design for a DNSBL-style interface + for detecting connections exiting the Tor network. This design and + its rationale are described at + <https://gitweb.torproject.org/tordnsel.git/blob/HEAD:/doc/torel-design.txt>. +License: PublicDomain +License-File: LICENSE +Homepage: http://p56soo2ibjkx23xo.onion/ +Package-URL: https://archive.torproject.org/tor-package-archive/tordnsel/tordnsel-0.1.1.tar.gz +Author: tup +Maintainer: tup.tuple@xxxxxxxxxxxxxx, lunar@xxxxxxxxxx, andrew@xxxxxxxxxxxxxx +Build-Type: Simple +Tested-With: GHC==6.6, GHC==6.8, GHC==6.10, GHC==6.12 +Data-Files: config/tordnsel.conf.sample, contrib/cacti-input.pl, contrib/tordnsel-init.d-script.sample, doc/tordnsel.8 -Executable: tordnsel -Main-Is: tordnsel.hs -Other-Modules: TorDNSEL.Config, - TorDNSEL.Config.Internals, - TorDNSEL.Control.Concurrent.Link, - TorDNSEL.Control.Concurrent.Link.Internals, - TorDNSEL.Control.Concurrent.Util, - TorDNSEL.DeepSeq, - TorDNSEL.Directory, - TorDNSEL.Directory.Internals, - TorDNSEL.DistinctQueue, - TorDNSEL.DNS, - TorDNSEL.DNS.Internals, - TorDNSEL.DNS.Server, - TorDNSEL.DNS.Server.Internals, - TorDNSEL.Document, - TorDNSEL.ExitTest.Initiator, - TorDNSEL.ExitTest.Initiator.Internals, - TorDNSEL.ExitTest.Request, - TorDNSEL.ExitTest.Server, - TorDNSEL.ExitTest.Server.Internals, - TorDNSEL.Log, - TorDNSEL.Log.Internals, - TorDNSEL.Main, - TorDNSEL.NetworkState, - TorDNSEL.NetworkState.Internals, - TorDNSEL.NetworkState.Storage, - TorDNSEL.NetworkState.Storage.Internals, - TorDNSEL.NetworkState.Types, - TorDNSEL.Random, - TorDNSEL.Socks, - TorDNSEL.Socks.Internals, - TorDNSEL.Statistics, - TorDNSEL.Statistics.Internals, - TorDNSEL.System.Timeout, - TorDNSEL.TorControl, - TorDNSEL.TorControl.Internals, - TorDNSEL.Util -HS-Source-Dirs: src -Includes: sys/types.h, unistd.h, sysexits.h, netinet/in.h, openssl/rand.h -Extra-Libraries: crypto -GHC-Options: -O2 -Wall -Werror -DVERSION="0.1.1-dev" +Flag Devel + Description: Enable almost all warnings, and make them fatal + Default: False -Executable: runtests -Buildable: False -Main-Is: runtests.hs -Other-Modules: TorDNSEL.Config.Tests, - TorDNSEL.Directory.Tests, - TorDNSEL.DNS.Tests, - TorDNSEL.DNS.Server.Tests -HS-Source-Dirs: src -Includes: netinet/in.h, openssl/rand.h -Extra-Libraries: crypto -GHC-Options: -fasm -Wall -Werror -fno-warn-missing-signatures +Executable tordnsel + Build-Depends: base>=2.0, network>=2.0, mtl>=1.0, unix>=1.0, stm>=2.0, + time>=1.0, binary>=0.4, bytestring>=0.9, array>=0.1, + directory>=1.0, containers>=0.1 + Main-Is: tordnsel.hs + Other-Modules: TorDNSEL.Config, + TorDNSEL.Config.Internals, + TorDNSEL.Control.Concurrent.Link, + TorDNSEL.Control.Concurrent.Link.Internals, + TorDNSEL.Control.Concurrent.Util, + TorDNSEL.DeepSeq, + TorDNSEL.Directory, + TorDNSEL.Directory.Internals, + TorDNSEL.DistinctQueue, + TorDNSEL.DNS, + TorDNSEL.DNS.Internals, + TorDNSEL.DNS.Server, + TorDNSEL.DNS.Server.Internals, + TorDNSEL.Document, + TorDNSEL.ExitTest.Initiator, + TorDNSEL.ExitTest.Initiator.Internals, + TorDNSEL.ExitTest.Request, + TorDNSEL.ExitTest.Server, + TorDNSEL.ExitTest.Server.Internals, + TorDNSEL.Log, + TorDNSEL.Log.Internals, + TorDNSEL.Main, + TorDNSEL.NetworkState, + TorDNSEL.NetworkState.Internals, + TorDNSEL.NetworkState.Storage, + TorDNSEL.NetworkState.Storage.Internals, + TorDNSEL.NetworkState.Types, + TorDNSEL.Random, + TorDNSEL.Socks, + TorDNSEL.Socks.Internals, + TorDNSEL.Statistics, + TorDNSEL.Statistics.Internals, + TorDNSEL.System.Timeout, + TorDNSEL.TorControl, + TorDNSEL.TorControl.Internals, + TorDNSEL.Util + HS-Source-Dirs: src + Includes: sys/types.h, unistd.h, sysexits.h, netinet/in.h, + openssl/rand.h + Extra-Libraries: crypto + cpp-options: -DVERSION="0.1.1-dev" + + if flag(devel) + GHC-Options: -O2 -Wall -Werror + else + GHC-Options: -O2 + +Executable runtests + Build-Depends: HUnit>=1.1 + Buildable: False + Main-Is: runtests.hs + Other-Modules: TorDNSEL.Config.Tests, + TorDNSEL.Directory.Tests, + TorDNSEL.DNS.Tests, + TorDNSEL.DNS.Server.Tests + HS-Source-Dirs: src + Includes: netinet/in.h, openssl/rand.h + Extra-Libraries: crypto + + if flag(devel) + GHC-Options: -fasm -Wall -Werror + else + GHC-Options: -fasm -fno-warn-missing-signatures -- 1.7.5.4
Attachment:
pgpa5w2opXPdU.pgp
Description: PGP signature
_______________________________________________ tor-dev mailing list tor-dev@xxxxxxxxxxxxxxxxxxxx https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-dev