Skip to content
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

Document the purpose of Network.Socket.BSD or consider merging #201

Closed
mboes opened this issue Jun 6, 2016 · 12 comments
Closed

Document the purpose of Network.Socket.BSD or consider merging #201

mboes opened this issue Jun 6, 2016 · 12 comments

Comments

@mboes
Copy link
Contributor

mboes commented Jun 6, 2016

The network package is really a combination of four API's:

  1. Network, which is deprecated.
  2. Network.Socket, which is a set of straight up low-level bindings to the basic C socket API.
  3. Network.Socket.ByteString, morally the same thing but bytestring based.
  4. Network.Socket.BSD - I don't know.

The latter provides additional functions not found in Network.Socket, presumably specific to the "BSD" sockets API. But everything is the BSD sockets API. Even Winsock on Windows is just a (relatively minor for our purposes) extension of it. In fact many of the functions exported by Network.BSD are also exported when on the Windows platform. So is the split in two modules of the BSD sockets API a distinction without a difference?

If so, I suggest we merge the two modules, for the sake of clarity. If not, then the purpose of Network.Socket.BSD should be clearly documented. It currently just says "defines Haskell bindings to network programming functionality provided by BSD Unix derivatives", which isn't too helpful given that the same could be said of Network.Socket.

@kazu-yamamoto
Copy link
Collaborator

I'm a new maintainer. So, I don't know the historical reason. If we decide this change, I think that we should ask to library ML if this change is acceptable. In my opinion, the fewer modules, the better. :-)

@kazu-yamamoto
Copy link
Collaborator

Relating to #169 where I proposed to provide Safe modules.

@eborden
Copy link
Collaborator

eborden commented Jun 8, 2016

I agree with @kazu-yamamoto on the stance that the libraries mailing list should be consulted on matters like these. As well any removal should be passed through a proper deprecation cycle**.

On the subject raised by @mboes, I would propose more drastic action (if only as a thought experiment):

  • Network has long been deprecated, it should be removed.
  • If there is no valid case, after research, that Network.Socket.BSD should exist it should be merged and deprecated.
  • Network.Socket's use of String is questionable. The existence of a String based socket implementation is mostly useless from a modern Haskell perspective. It has "training wheels" uses, but any serious networking application is going to utilize ByteString. These portions of the module should be deprecated and either removed or made a temporary alias to a Network.Socket.String module.

Again, these stances are aggressive and pie in the sky. This package is depended on by large swaths of the ecosystem and any change must be slow and thoughtful.

** I do not believe we have an official stance on deprecation. This is something for @kazu-yamamoto and I to consider and document.

@eborden
Copy link
Collaborator

eborden commented Jun 8, 2016

To pile on, documentation in Network.Socket explicitly drive users away from its String implementations:
Do not use the send and recv functions defined in this module in new code, as they incorrectly represent binary data as a Unicode string. As a result, these functions are inefficient and may lead to bugs in the program. Instead use the send and recv functions defined in the ByteString module.

@kazu-yamamoto
Copy link
Collaborator

Yes. To deprecate something, we should prepare documentation and put them deprecated pragma first.

I support the deprecation of Network and Network.Socket.BSD. I agree with @eborden's impression that the String interface in Network.Socket is questionable. However, I would like to keep them as is because the extent of the impact is too large and we should explain that users should use Network.Socket.ByteString always.

@eborden
Copy link
Collaborator

eborden commented Jun 15, 2016

@kazu-yamamoto, I agree that the scope of breakage from removing the string API is too large. Alternatively we could introduce warnings for those functions to further lead people in the right direction.

@winterland1989
Copy link

winterland1989 commented Jul 1, 2016

Excuse me for interrupting, i just come across here after trying to start use network, i almost get lost between these modules:

  • Network provide connectTo which is the most obvious way to start a TCP connection, but it has wrong document Network documentation incorrectly claims handles are block-buffered #173 and you guys suggest Network is deprecated, so should i use it?
  • Network.Socket provide socket which involve lots of details(AF_INET,Stream...), is there any plan to add helpers or should i use network-simple instead?
  • does functions in Network.Socket.BSD works on windows?

Another question maybe not related, is there any necessary to use Handler with Socket(block-buffered or no buffered)? does os provide buffer already? I suggest someone add these knowledge to document.

@winterland1989
Copy link

winterland1989 commented Jul 5, 2016

I get better understanding after reading many other language's net standard library and unix socket materials, now current module structure looks reasonable to me, here's what i think:

  • Network is definitely not deprecated, it provide higher level abstraction, namely connectTo.
  • Although Network.Socket.BSD 's naming is confusing, it's cross platform, and provide uniform bsd style address resolution, it's better leave it be.
  • The String api in Network should be marked deprecated indeed, there's no other usage than keeping compatibility.
  • We really shound add some helpers to Network module, for example:
--  we should be able to get connected 'Socket' and 'SockAddr' easily
connectTo' :: HostName -> PortID -> IO (Socket, SockAddr)
  • Please merge code from package network-socket-options, it provide many useful socket option and cross-platform, and standard library in other languages usually support these operations. We can add these functions into Network.Socket module or make a new Network.Socket.Extra module.

@kazu-yamamoto What's your opinion on these change? do they make sense? should they go through core-library consensus?

@kazu-yamamoto
Copy link
Collaborator

These changes are readable to me.
Would you give us pull requests step by step?
If your pull request include too many commits, I would give up reviewing.

@winterland1989
Copy link

winterland1989 commented Jul 20, 2016

Just like i said #211, many thoughts have changed since i'm getting more familiar with network and unix socket now, i'll try to improve the document situation first, but like unix socket itself, these low level operations are not easy to explain well.

FYI, i have release tcp-streams for more high-level need, i think network has done a good job already.

@kazu-yamamoto
Copy link
Collaborator

@mboes What is your opinion at this moment?

@kazu-yamamoto
Copy link
Collaborator

I would like to close this issue. Please reopen if necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants