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

Drop include/HsNetworkConfig.h from the dist #307

Closed
angerman opened this issue Feb 8, 2018 · 13 comments
Closed

Drop include/HsNetworkConfig.h from the dist #307

angerman opened this issue Feb 8, 2018 · 13 comments

Comments

@angerman
Copy link
Contributor

angerman commented Feb 8, 2018

network-2.6.3.3
├── CHANGELOG.md
├── LICENSE
├── Network
│   ├── BSD.hsc
│   ├── Socket
│   │   ├── ByteString
│   │   │   ├── IOVec.hsc
│   │   │   ├── Internal.hs
│   │   │   ├── Lazy
│   │   │   │   ├── Posix.hs
│   │   │   │   └── Windows.hs
│   │   │   ├── Lazy.hs
│   │   │   └── MsgHdr.hsc
│   │   ├── ByteString.hsc
│   │   ├── Internal.hsc
│   │   └── Types.hsc
│   └── Socket.hsc
├── Network.hs
├── README.md
├── Setup.hs
├── cbits
│   ├── HsNet.c
│   ├── ancilData.c
│   ├── asyncAccept.c
│   ├── initWinSock.c
│   └── winSockErr.c
├── config.guess
├── config.sub
├── configure
├── configure.ac
├── examples
│   ├── EchoClient.hs
│   └── EchoServer.hs
├── include
│   ├── HsNet.h
│   ├── HsNetworkConfig.h
│   └── HsNetworkConfig.h.in
├── install-sh
├── network.buildinfo.in
├── network.cabal
└── tests
    ├── Regression.hs
    ├── Simple.hs
    └── doctests.hs

I believe that include/HsNetworkConfig.h should not be part of the distribution. As it is generated by the configure script. However running the configure script in a different folder to keep the source tree clean, will result in two HsNetworkConfig.h files, and at this point the order and interpretation of -I parameters becomes important, which is rather unfortunate.

@angerman angerman mentioned this issue Feb 8, 2018
4 tasks
@kazu-yamamoto
Copy link
Collaborator

@angerman I agree.

@kazu-yamamoto
Copy link
Collaborator

@eborden 2.6.3.3 includes include/HsNetworkConfig.h.
But cabal sdist creates .tar.gz without it.
How did you pack the package?

@hvr
Copy link
Member

hvr commented Feb 8, 2018

btw, you can take a look at how unix does it, to make sure autoconf generated files aren't included in the sdist; take a look at the .cabal file and the .buildinfo usage.

I should probably make sure to have a section in the cabal user's guide to document the pattern for that.

@eborden
Copy link
Collaborator

eborden commented Feb 8, 2018

@kazu-yamamoto Odd, I used cabal sdist.

@hvr
Copy link
Member

hvr commented Feb 8, 2018

The problem is that include/HsNetworkConfig.h is mentioned in install-includes, which is seen by cabal sdist (and we don't yet have the equivalent of autogen-modules for install-includes). The trick that unix uses is to hide the install-includes from sdist for autogenerated files in the .buidinfo file.

PS: The problem I described was already fixed last year via 284ed49

@kazu-yamamoto
Copy link
Collaborator

I'm busy now. I will come back to this issue on 15th Feb.

@kazu-yamamoto
Copy link
Collaborator

I cannot reproduce this. Both cabal-install-1.24 and cabal-install-2.0 do not include include/HsNetworkConfig.h even after cabal configure. So, I cannot see if the . hack solve this issue.

@angerman
Copy link
Contributor Author

Well, I'm happy either way; as long as no file that's supposed to be generated is in the package hackage hands me :-)

@kazu-yamamoto
Copy link
Collaborator

@angerman I need to understand why this happens and fix it so that it does not happen anymore.

@hvr
Copy link
Member

hvr commented Feb 15, 2018

@kazu-yamamoto in which branch of network did you try this? the master branch seems to be fine already; and so does the network-2.7 branch....

PS: I see... this is due to 284ed49 which is already part of all recent branches, and which I cooked up to address @angerman very problem last year... :-)

@hvr
Copy link
Member

hvr commented Feb 15, 2018

@kazu-yamamoto @angerman, I finally understand what happened here:

@angerman reported the issue against network-2.6.3.3 which predates 284ed49 and thus exhibits the packaging issue

However, this is already fixed in Git, and I've found the commit in the branches for network-2.6, network-2.7, and network-3.0 (aka master). So there's nothing to be done for the upcoming release as far as #307 is concerned IMO.

@kazu-yamamoto
Copy link
Collaborator

I was testing this on 2.6.

@kazu-yamamoto
Copy link
Collaborator

However, this is already fixed in Git, and I've found the commit in the branches for network-2.6, network-2.7, and network-3.0 (aka master). So there's nothing to be done for the upcoming release as far as #307 is concerned IMO.

Ah, I understand. Thanks. Let's close.

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