[tor-dev] PRELIMINARY: [PATCH] Adapt to changes in 'GHC.Handle'.

Nikita Karetnikov nikita at karetnikov.org
Fri Jun 21 20:10:08 UTC 2013


> 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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-Flag-Devel-to-tordnsel.cabal-and-update-its-layo.patch
Type: text/x-diff
Size: 7447 bytes
Desc: not available
URL: <http://lists.torproject.org/pipermail/tor-dev/attachments/20130622/3e84acaf/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: <http://lists.torproject.org/pipermail/tor-dev/attachments/20130622/3e84acaf/attachment.sig>


More information about the tor-dev mailing list