haskell / win32

Haskell support for the Win32 API
http://hackage.haskell.org/package/Win32
Other
97 stars 63 forks source link

Untie Win32 from bytestring #166

Closed Bodigrim closed 3 years ago

Bodigrim commented 3 years ago

I am working on refactoring bytestring test suite. It is currently organized as two packages. There is a bytestring package, which reside in a top-level folder, and a bytestring-tests package in tests sub-folder with the following configuration:

test-suite {foo,bar,baz}
  hs-source-dirs:   . ..
  main-is:          {Foo,Bar,Baz}.hs
  other-modules:    Data.ByteString
                    Data.ByteString.Char8
                    Data.ByteString.Internal
                    Data.ByteString.Lazy
                    Data.ByteString.Lazy.Char8
                    Data.ByteString.Lazy.Internal
                    Data.ByteString.Short
                    Data.ByteString.Short.Internal
                    ...many more...
  build-depends:    base, ghc-prim, deepseq, random,
                    tasty, tasty-quickcheck, QuickCheck

So tests do not depend on bytestring package explicitly, but rather just happen to share source files. This is unfortunate for many reasons. For example, each test component recompiles all modules of bytestring anew. Tooling such as HLS cannot figure out which source file belongs to what. But most importantly this way we do not actually test bytestring package, because there are no guarantees that the configuration in bytestring.cabal (GHC flags, C sources, etc.) is the same.

An obvious solution is to abandon the hack with hs-source-dirs: .. and add bytestring as a proper build-depends of bytestring-tests package. This still does not work in a satisfactory manner, because now any change to bytestring causes rebuilding of random, directory, QuickCheck, process, optparse-applicative, tasty, tasty-quickcheck (all of them depend on bytestring transitively) and, as you can imagine, it takes obnoxiously long for any sort of interactive development.

Ideally I would like to eliminate bytestring as a transitive dependency of the test suite. Not only it would enable us to run tests quicker, but also the very reason for having a separate package bytestring-tests would disappear.

This idea proved to be fruitful for Linux and MacOS builds: I patched optparse-applicative and tasty ecosystem so that they no longer depend on bytestring, when compiled with right flags. Unfortunately, there is a nasty complication on Windows. The reason is that several packages (e. g., ansi-terminal) depend on Win32, which depends on bytestring, which makes it a circular dependency.


I was initially writing this to ask about a possibility to make bytestring an optional dependency of Win32, guarding relevant calls by a Cabal flag. But actually I found out that there is only a single spot of Win32, which involves bytestring:

https://github.com/haskell/win32/blob/6775f8ccfd118f76f69e8b447e59f8f81e31a514/System/Win32/FileMapping.hsc#L56-L60

Moreover, this function is unused: neither Hackage nor GHC refer to it. The code itself is prone to be broken in future, because it relies on an internal API (there are actually ideas to back ByteString by ByteArray# instead of ForeignPtr).

How do you feel about removing mapFileBs (and bytestring dependency) in the future release of Win32?

Not only this would resolve my problem and facilitate smooth testing of bytestring, but it would genuinely untie two boot packages from one another.

Mistuke commented 3 years ago

Hi,

Moreover, this function is unused: neither Hackage nor GHC refer to it.

Well there are plenty of projects not on Hackage though :)

The code itself is prone to be broken in future, because it relies on an internal API (there are actually ideas to back ByteString by ByteArray# instead of ForeignPtr).

Interesting, Wouldn't this represent a regression for code that read from an I/O source. Currently the I/O manager can feed bytestring efficiently from a native pointer that's maintained from the Buffers inside GHC. By forcing it to use a ByteArray# that would require moving memory from the C side to Haskell would it not?

It would seem to me that bytestring would always require a way to create a binary representation from an unmanaged pointer...

That said I have no problem removing this function as it's functionality questionably shouldn't be in Win32 anyway which is meant to just be a low level interface to platform APIs. It would however in the grand scheme of things be very unfortunate not to be able to use bytestring...

Bodigrim commented 3 years ago

Interesting, Wouldn't this represent a regression for code that read from an I/O source.

Practically speaking, this particular change is not gonna happen any time soon. There is an ongoing discussion at https://github.com/haskell/bytestring/issues/193, feel free to chime in. My point was more about relying on internal API in general.

That said I have no problem removing this function as it's functionality questionably shouldn't be in Win32 anyway which is meant to just be a low level interface to platform APIs.

Would you like me to raise a PR?

Mistuke commented 3 years ago

Would you like me to raise a PR?

If you would that would be grand :) Thanks!