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#configurat... [4] http://www.haskell.org/cabal/users-guide/installing-packages.html#controllin... [5] http://learnyouahaskell.com/input-and-output#hello-world [6] https://www.fpcomplete.com/school/pick-of-the-week/bytestring-bits-and-piece... [7] http://www.haskell.org/hoogle/ [8] https://gitweb.torproject.org/tordnsel.git