-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update to network 3.0 with backward compatibility for network <= 2.8 #98
Merged
VictorDenisov
merged 13 commits into
mongodb-haskell:master
from
plow-technologies:network-deprecations
Jun 15, 2019
Merged
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
4477045
Add .stack-work to .gitignore.
scott-fleischman 13f56bb
Use ciphersuite_default instead of ciphersuite_all.
scott-fleischman c03e1ed
Use Control.Monad.Except instead of Control.Monad.Error.
scott-fleischman 5f04dc6
Remove use of conduit Producer and Consumer.
scott-fleischman b094dff
Prefix internal unused fields with underscore.
scott-fleischman 74a4041
Update shadowing warnings.
scott-fleischman 5bb7751
Revert "Remove use of conduit Producer and Consumer."
scott-fleischman 21cf023
Add module Database.MongoDB.Internal.Network.
scott-fleischman 17287b5
Use network's connectTo when available.
scott-fleischman 996d3e1
Use same version check as in cabal file; fix typo in comment.
scott-fleischman 3334d81
Improve network versioning; add to benchmarks.
scott-fleischman f84cc03
Add numeric instances that network's PortNumber has.
scott-fleischman ef1fc38
Remove explicit default of _old-network flag.
scott-fleischman File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
-- | Compatibility layer for network package, including newtype 'PortID' | ||
{-# LANGUAGE CPP, PackageImports #-} | ||
|
||
module Database.MongoDB.Internal.Network (PortID(..), N.HostName, connectTo) where | ||
|
||
import Control.Exception (bracketOnError) | ||
import Network.BSD as BSD | ||
import System.IO (Handle, IOMode(ReadWriteMode)) | ||
|
||
#if !MIN_VERSION_network(2, 8, 0) | ||
import qualified Network as N | ||
#else | ||
import qualified Network.Socket as N | ||
#endif | ||
|
||
newtype PortID = PortNumber N.PortNumber deriving (Show, Eq, Ord) | ||
|
||
-- Taken from network 2.8's connectTo | ||
-- https://github.com/haskell/network/blob/e73f0b96c9da924fe83f3c73488f7e69f712755f/Network.hs#L120-L129 | ||
connectTo :: N.HostName -- Hostname | ||
-> PortID -- Port Identifier | ||
-> IO Handle -- Connected Socket | ||
connectTo hostname (PortNumber port) = do | ||
proto <- BSD.getProtocolNumber "tcp" | ||
bracketOnError | ||
(N.socket N.AF_INET N.Stream proto) | ||
(N.close) -- only done if there's an error | ||
(\sock -> do | ||
he <- BSD.getHostByName hostname | ||
N.connect sock (N.SockAddrInet port (hostAddress he)) | ||
N.socketToHandle sock ReadWriteMode | ||
) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
resolver: lts-9.21 | ||
flags: | ||
mongoDB: | ||
_old-network: true |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
resolver: lts-11.22 | ||
flags: | ||
mongoDB: | ||
_old-network: true |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
resolver: lts-12.26 | ||
flags: | ||
mongoDB: | ||
_old-network: true |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
resolver: lts-13.23 | ||
extra-deps: | ||
- git: [email protected]:hvr/bson.git # https://github.com/mongodb-haskell/bson/pull/18 | ||
commit: 2fc8d04120c0758201762b8e22254aeb6d574f41 | ||
- network-bsd-2.8.1.0 | ||
- network-3.1.0.0 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
resolver: lts-13.23 | ||
flags: | ||
mongoDB: | ||
_old-network: true |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@scott-fleischman Can you clarify for me who and how is supposed to set this flag?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are set using the cabal configuration flags as described here.
There are a couple choices involved here that I basically deferred to the choices made in the bson PR.
_old-network
is pretty cryptic. I just copied it. If flags can contain numbers it might be better to name it something likeuse-network2
or something like that.False
by default but maybe untilnetwork-3.*
gets into stackage, maybe the default should be true? Then when stackage includes network 3 by default in the newest stackage lts then mongoDB could ship a breaking change which changes the flag to true, so it defaults to using network3.I don't believe the
.cabal
files themselves allow logic like conditional statements based on a library dependency version. In our case we would need to include a new library (network-bsd
) if network is>=3.0
, so that might not be possible without configure flags.In the end, I am happy to consider alternatives to the approach taken in this PR with regards to cabal configuration. But as far as I can tell that is going to involve some user choice of which network to use.
You can see how I set them in the stack files, for example:
https://github.com/plow-technologies/mongodb/blob/f84cc035179198ed63a5e247aa7c95f61ce7a295/stack-ghc80.yaml#L2-L4
The flag is omitted here, so it defaults to using the network 3.0+ and network-bsd dependencies.
https://github.com/plow-technologies/mongodb/blob/network-deprecations/stack-ghc86-network3.yaml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe
manual: False
might be useful here though and would work out-of-the-box in both cases (network 2 or network 3). https://www.haskell.org/cabal/users-guide/developing-packages.html#pkg-field-flag-manualThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't seem to work for
stack
if I remove the explicit flag set. I get errors in the stack configure step.cabal
(cabal-install version 2.4.0.0) itself seems ok with it though. I was able to build using ghc 8.2.2 and it picked the old network configurations even though the_old-network
flag defaults toFalse
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So all in all, it seems to me the current approach seems pretty good given the various options I am aware of.
Stack users will have to set the flag explicitly for network <=2.8.
Cabal users should work fine as-is. (I'm not sure how to test against the newer network because I'm not sure how to get cabal to know about the bson PR, which it would need to for network3.)
So with defaulting the flag to false, stack users of network 2.8 will have to explicitly set it to true (or explicitly include network 3). When the stackage lts includes network 3, then can remove the explicit flag from their configuration.
Cabal users should work fine in both cases. When network 3 is available for their configuration (needs bson available that works with network 3), since the
_old-network
flag default to True, it will try the network 3 config first, so it will use that. Else it default to the old network configuration and builds fine. I tested with ghc 8.2.2 and ghc 8.6.5 with cabal 2.4.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's a reference to the conditional options in a cabal file: https://www.haskell.org/cabal/users-guide/developing-packages.html#conditions