haskell-servant / servant

Servat is a Haskell DSL for describing, serving, querying, mocking, documenting web applications and more!
https://docs.servant.dev/
1.83k stars 414 forks source link

Dependency on bytestring-conversion and successively double-conversion causes problems #188

Closed rubenmoor closed 9 years ago

rubenmoor commented 9 years ago

Since Servant 0.4.0 there is a dependency on double-conversion (via bytestring-conversion). The package double-conversion causes problems in windows (but not only there).

Cf. this stack issue and this double-conversion issue

And this comment:

"Also, for the record, double-conversion had been a pain point for a long time, and not just on windows. I had to patch aeson at some point in the past to avoid using it. I wouldn't consider it a good library to be dependent on." (source)

Any clue how big a fork a servant w/o double-conversion would be?

alexbiehl commented 9 years ago

https://github.com/twittner/bytestring-conversion/commit/c8a75f30f3e44a20cbcef4c18c04f971b54e7980

Make sure you use bytestring-conversion 0.3.1. It will use blaze-textual instead of double-conversion on Windows. (And it should fit with servants bounds for bytestring-conversion)

alpmestan commented 9 years ago

I'm not really sure I see what the best option is, but we definitely want the experience to be as smooth as possible on all main platforms, including windows. Let us know what you think servant should do wrt bytestring-conversion etc.

rubenmoor commented 9 years ago

Thanks to @alexbiehl: switch to latest stackage nightly solved my issue.

The servant code is fine, as the dependency is specified with *. Keep it up!

esjmb commented 5 years ago

this is back. Specifically, with lts-13.0, and stack --docker build (with fpco/stack-build as the base image) of a servant project, i get:

Configuring double-conversion-2.0.2.0...
    Cabal-simple_mPHDZzAJ_2.4.0.1_ghc-8.6.3: Missing dependency on a foreign
    library:
    * Missing (or bad) C library: stdc++
    This problem can usually be solved by installing the system package that
    provides this library (you may need the "-dev" version). If the library is
    already installed but in a non-standard location then you can use the flags
    --extra-include-dirs= and --extra-lib-dirs= to specify where it is.If the
    library file does exist, it may contain errors that are caught by the C
    compiler at the preprocessing stage. In this case you can re-run configure
    with the verbosity flag -v3 to see the error messages.

g++ seems to be installed into fpco/stack-build.

alpmestan commented 5 years ago

As before, there isn't much that servant itself can do to help as this is not a package we depend on directly. Please do however feel free to use this ticket to figure out a solution with others (I'm afraid I won't be of much help).

esjmb commented 5 years ago

Hi Alpmestan,

Appreciate that, it looks to me like its an issue with the fpco/stack-build for 13.0, but I think I've noticed something odd about servant-auth-server's dependency on byte string-conversion, which is that the dependency doesn't seem to be actually used in the servant-auth-server codebase? instead it seems that Blaze toByteString alternatives are used.

I tried simply removing the entry in the cabal file for servant-auth-server, and it compiled both native and via --docker under stack with its-13.0. builds fine.

I also did a quick scan for to/fromByteString usage (the functions bytestring-conversion provides) and could only find these.. https://github.com/haskell-servant/servant-auth/search?q=toByteString&unscoped_q=toByteString

.. with the import being Blaze.ByteString.Builder (toByteString) and not Data.ByteString.Conversion...

I've scanned generally for usage of bytestring-conversion functions in the servant code base but didn't find any.

so, I may be wrong of course, but it looks to me that the dependency on bytestring-conversion can be safely eliminated from servant-auth?

Given that double-conversion seems to cause a lot of problems generally, and your only dependency on it is both indirect and via the seemingly unused byte string-conversion, might this be a double-win?

alpmestan commented 5 years ago

It might very well indeed! Do you feel like putting together a PR to servant-auth that removes the dependency? :-)

phadej commented 5 years ago

FWIW, if someone could run weeder on both servant and servant-auth codebases, it would be nice.

esjmb commented 5 years ago

It might very well indeed! Do you feel like putting together a PR to servant-auth that removes the dependency? :-)

i'll do that sure. thanks.

esjmb commented 5 years ago

FWIW, if someone could run weeder on both servant and servant-auth codebases, it would be nice.

here is the output for servant-auth cloned from acc59c0acd903d86852fc41959b0430759af2028 this morning (note that I had to add a few deriving instance ToHttpApiData SetCookie to test code to get it to build with weeder).

= Package servant-auth-server =

== Section library ==
Redundant build-depends entry
* bytestring-conversion
* crypto-api

== Section test:readme ==
Redundant build-depends entry
* markdown-unlit
* transformers

= Package servant-auth-docs =

== Section library ==
Redundant build-depends entry
* text

== Section test:doctests ==
Missing other-modules entry
* Build_doctests
Redundant build-depends entry
* QuickCheck
* servant-auth-docs
* template-haskell

== Section test:spec ==
Redundant build-depends entry
* QuickCheck
* lens
* servant
* servant-auth
* servant-auth-docs
* servant-docs
* text

= Package servant-auth-client =

== Section library ==
Redundant build-depends entry
* text

== Section test:spec ==
Redundant build-depends entry
* transformers

= Package servant-auth-swagger =

== Section test:spec ==
Redundant build-depends entry
* QuickCheck
* text

= Package servant-auth =
No warnings
esjmb commented 5 years ago

FWIW, if someone could run weeder on both servant and servant-auth codebases, it would be nice.

here is the output for servant cloned from a3d335b4363ffc973c4d2592e3ad230ce6847e78 this morning

= Package servant-docs =

== Section test:spec ==
Weeds exported
* Servant.DocsSpec
  - Datatype1
  - Describe
  - TT
  - TT1
  - TT2
  - TestApi1
  - TestTreeM
  - UT
  - UT1
  - UT2
  - compareWith
  - comprehensiveDocs
  - describe
  - golden
  - it
  - runTestTreeM
  - shouldBe
  - shouldContain
  - shouldNotContain

= Package servant-conduit =

== Section library ==
Redundant build-depends entry
* bytestring
* mtl

== Section test:example ==
Redundant build-depends entry
* http-media

= Package servant-client =

== Section library ==
Redundant build-depends entry
* servant
* transformers-compat

== Section test:readme ==
Redundant build-depends entry
* markdown-unlit

== Section test:spec ==
Redundant build-depends entry
* text
* transformers-compat

= Package servant-client-core =

== Section library ==
Redundant build-depends entry
* transformers

= Package servant-pipes =

== Section library ==
Redundant build-depends entry
* bytestring
* mtl

== Section test:example ==
Redundant build-depends entry
* http-media

= Package servant-server =

== Section test:doctests ==
Missing other-modules entry
* Build_doctests
Redundant build-depends entry
* servant-server

== Section test:spec ==
Redundant build-depends entry
* QuickCheck
* transformers-compat
Weeds exported
* Servant.ArbitraryMonadServerSpec
  - CombinedAPI
  - IdentityAPI
  - ReaderAPI
  - combinedAPI
  - combinedReaderServer
  - combinedReaderServer'
  - enterSpec
  - fReader
  - identityAPI
  - readerAPI
  - readerServer
  - readerServer'
* Servant.HoistSpec
  - API
  - App
  - api
  - f
  - server
  - server'
* Servant.Server.StaticFilesSpec
  - Api
  - api
  - app
  - server
  - withStaticFiles
* Servant.Server.StreamingSpec
  - TestAPI
  - Waiter
  - executeRequest
  - newWaiter
  - testAPI
  - timeout
* Servant.Server.UsingContextSpec
  - InjectAPI
  - NamedContextAPI
  - OneEntryAPI
  - OneEntryTwiceAPI
  - WithBirdfaceAPI
  - injectApp
  - namedContextApp
  - oneEntryApp
  - oneEntryTwiceApp
  - spec1
  - spec2
  - spec3
  - spec4
  - testServer
  - withBirdfaceApp
* Servant.ServerSpec
  - AlternativeApi
  - Animal
  - BasicAuthAPI
  - CaptureAllApi
  - CaptureApi
  - GenAuthAPI
  - HeaderApi
  - MiscCombinatorsAPI
  - QueryParamApi
  - RawApi
  - ReqBodyApi
  - ResponseHeadersApi
  - VerbApi
  - alice
  - alternativeApi
  - alternativeServer
  - alternativeSpec
  - basicAuthApi
  - basicAuthContext
  - basicAuthServer
  - basicAuthSpec
  - beholder
  - captureAllApi
  - captureAllServer
  - captureAllSpec
  - captureApi
  - captureServer
  - captureSpec
  - comprehensiveApiContext
  - genAuthApi
  - genAuthContext
  - genAuthServer
  - genAuthSpec
  - headerApi
  - headerSpec
  - jerry
  - miscApi
  - miscCombinatorSpec
  - miscServ
  - qpServer
  - queryParamApi
  - queryParamSpec
  - rawApi
  - rawApplication
  - rawSpec
  - reqBodyApi
  - reqBodySpec
  - responseHeadersServer
  - responseHeadersSpec
  - tweety
  - verbSpec

= Package servant =

== Section library ==
Redundant build-depends entry
* tagged

== Section test:doctests ==
Missing other-modules entry
* Build_doctests
Redundant build-depends entry
* hspec
* servant

== Section test:spec ==
Weeds exported
* Servant.API.ContentTypesSpec
  - JSONorText
  - SomeData
  - ZeroToOne
  - addToAccept
* Servant.API.StreamSpec
  - roundtrip
  - runRenderFrames
  - runUnrenderFrames
* Servant.LinksSpec
  - AllGood
  - LinkableApi
  - NoEndpoint
  - NotALink
  - TestApi
  - WrongContentType
  - WrongMethod
  - WrongPath
  - WrongReturnType
  - apiLink
  - shouldBeLink

= Package servant-machines =

== Section library ==
Redundant build-depends entry
* bytestring
* mtl

== Section test:example ==
Redundant build-depends entry
* http-media

= Package servant-foreign =

== Section test:spec ==
Weeds exported
* Servant.ForeignSpec
  - LangX
  - TestApi
  - camelCaseSpec
  - listFromAPISpec
  - testApi
phadej commented 5 years ago

Good. Weeder output looks like it needs proper attention. I'll take a look (hopefully) this week. A bit of OSS stuff in a queue right now.

Sent from my iPhone

On 3 Jan 2019, at 12.56, Stephen Barrett notifications@github.com wrote:

FWIW, if someone could run weeder on both servant and servant-auth codebases, it would be nice.

here is the output for servant-auth cloned from a3d335b this morning

= Package servant-docs =

== Section test:spec == Weeds exported

  • Servant.DocsSpec
    • Datatype1
    • Describe
    • TT
    • TT1
    • TT2
    • TestApi1
    • TestTreeM
    • UT
    • UT1
    • UT2
    • compareWith
    • comprehensiveDocs
    • describe
    • golden
    • it
    • runTestTreeM
    • shouldBe
    • shouldContain
    • shouldNotContain

= Package servant-conduit =

== Section library == Redundant build-depends entry

  • bytestring
  • mtl

== Section test:example == Redundant build-depends entry

  • http-media

= Package servant-client =

== Section library == Redundant build-depends entry

  • servant
  • transformers-compat

== Section test:readme == Redundant build-depends entry

  • markdown-unlit

== Section test:spec == Redundant build-depends entry

  • text
  • transformers-compat

= Package servant-client-core =

== Section library == Redundant build-depends entry

  • transformers

= Package servant-pipes =

== Section library == Redundant build-depends entry

  • bytestring
  • mtl

== Section test:example == Redundant build-depends entry

  • http-media

= Package servant-server =

== Section test:doctests == Missing other-modules entry

  • Build_doctests Redundant build-depends entry
  • servant-server

== Section test:spec == Redundant build-depends entry

  • QuickCheck
  • transformers-compat Weeds exported
  • Servant.ArbitraryMonadServerSpec
    • CombinedAPI
    • IdentityAPI
    • ReaderAPI
    • combinedAPI
    • combinedReaderServer
    • combinedReaderServer'
    • enterSpec
    • fReader
    • identityAPI
    • readerAPI
    • readerServer
    • readerServer'
  • Servant.HoistSpec
    • API
    • App
    • api
    • f
    • server
    • server'
  • Servant.Server.StaticFilesSpec
    • Api
    • api
    • app
    • server
    • withStaticFiles
  • Servant.Server.StreamingSpec
    • TestAPI
    • Waiter
    • executeRequest
    • newWaiter
    • testAPI
    • timeout
  • Servant.Server.UsingContextSpec
    • InjectAPI
    • NamedContextAPI
    • OneEntryAPI
    • OneEntryTwiceAPI
    • WithBirdfaceAPI
    • injectApp
    • namedContextApp
    • oneEntryApp
    • oneEntryTwiceApp
    • spec1
    • spec2
    • spec3
    • spec4
    • testServer
    • withBirdfaceApp
  • Servant.ServerSpec
    • AlternativeApi
    • Animal
    • BasicAuthAPI
    • CaptureAllApi
    • CaptureApi
    • GenAuthAPI
    • HeaderApi
    • MiscCombinatorsAPI
    • QueryParamApi
    • RawApi
    • ReqBodyApi
    • ResponseHeadersApi
    • VerbApi
    • alice
    • alternativeApi
    • alternativeServer
    • alternativeSpec
    • basicAuthApi
    • basicAuthContext
    • basicAuthServer
    • basicAuthSpec
    • beholder
    • captureAllApi
    • captureAllServer
    • captureAllSpec
    • captureApi
    • captureServer
    • captureSpec
    • comprehensiveApiContext
    • genAuthApi
    • genAuthContext
    • genAuthServer
    • genAuthSpec
    • headerApi
    • headerSpec
    • jerry
    • miscApi
    • miscCombinatorSpec
    • miscServ
    • qpServer
    • queryParamApi
    • queryParamSpec
    • rawApi
    • rawApplication
    • rawSpec
    • reqBodyApi
    • reqBodySpec
    • responseHeadersServer
    • responseHeadersSpec
    • tweety
    • verbSpec

= Package servant =

== Section library == Redundant build-depends entry

  • tagged

== Section test:doctests == Missing other-modules entry

  • Build_doctests Redundant build-depends entry
  • hspec
  • servant

== Section test:spec == Weeds exported

  • Servant.API.ContentTypesSpec
    • JSONorText
    • SomeData
    • ZeroToOne
    • addToAccept
  • Servant.API.StreamSpec
    • roundtrip
    • runRenderFrames
    • runUnrenderFrames
  • Servant.LinksSpec
    • AllGood
    • LinkableApi
    • NoEndpoint
    • NotALink
    • TestApi
    • WrongContentType
    • WrongMethod
    • WrongPath
    • WrongReturnType
    • apiLink
    • shouldBeLink

= Package servant-machines =

== Section library == Redundant build-depends entry

  • bytestring
  • mtl

== Section test:example == Redundant build-depends entry

  • http-media

= Package servant-foreign =

== Section test:spec == Weeds exported

  • Servant.ForeignSpec
    • LangX
    • TestApi
    • camelCaseSpec
    • listFromAPISpec
    • testApi — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.