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

Add custom-setup stanza #274

Merged
merged 1 commit into from
May 9, 2018
Merged

Add custom-setup stanza #274

merged 1 commit into from
May 9, 2018

Conversation

snoyberg
Copy link
Contributor

@snoyberg snoyberg commented May 9, 2018

The Setup.lhs file is incompatible with newer versions of the Cabal
library. This change adds the appropriate custom-setup stanza to
indicate compatibility with Cabal 1.24.

The Setup.lhs file is incompatible with newer versions of the Cabal
library. This change adds the appropriate custom-setup stanza to
indicate compatibility with Cabal 1.24.
@tonymorris tonymorris merged commit f1d9e4d into system-f:master May 9, 2018
@hvr
Copy link

hvr commented May 9, 2018

Unfortunately this change made sure that the .cabal file is now ill-specified and so will guarantee to fail to build (except maybe for tooling which doesn't implement the custom-setup semantics correctly). I suggest either reverting this commit or doing it properly.

@parsonsmatt
Copy link
Contributor

parsonsmatt commented May 9, 2018

How would it be broken? I just built the project with this commit with cabal new-build and stack init && stack build and both worked fine

@gwils
Copy link
Collaborator

gwils commented May 10, 2018

@parsonsmatt These changes concern the projects/NetworkServer/haskell subdirectory, which is an optional extra project to complete for a student looking to go further. If you cd to there and then run cabal new-build it will fail.

I've opened #275 to fix new-build for that project.
The doctests are broken and I don't have time to fix them yet, so I've raised issue #278

@hvr
Copy link

hvr commented May 10, 2018

I'd also suggest filing a bug against Stack, as it appears that even though the custom-setup stanza was introduced a long time ago w/ Cabal-1.24 current Stack still fails to correctly implement its semantics which as a consequence caused this faulty PR to be filed.

@parsonsmatt
Copy link
Contributor

I'll happily raise an issue on stack's tracker, but I'm unclear on the problem because it built fine for me. I don't know what isn't working (or, maybe, what is working that shouldn't be!) :)

@hvr
Copy link

hvr commented May 11, 2018

@parsonsmatt Cabal's package description design is driven by the "no untracked dependencies" mantra, i.e. the .cabal package description is supposed to specify the build environment (i.e. dependencies) as accurately as possible. See https://www.well-typed.com/blog/2015/07/cabal-setup-deps/ for more details about this.

In the case of custom-setup's setup-depends this means that only packages specified via setup-depends are allowed to be in scope for compiling Setup.hs. So if Stack happens to interpret setup-depends: Cabal >= x && < y merely as a kind of constraint and constructs package environment in scope for compiling Setup.hs which brings into scope more than the lib:Cabal component, then it's unfortunately terribly broken in its implementation of the custom-setup stanza semantics and this non-compliance has the potential to cause serious problems down the road for both stack and cabal users because it harms the goal of achieving reproducibility (as explained in the blog-post linked above: "while it worked for you, it wouldn’t build for me").

@puffnfresh
Copy link
Contributor

Eta was a project which relied on a very similar Stack bug. Sucks.

@parsonsmatt
Copy link
Contributor

Oh, interesting! Should I file a bug on cabal's repo since it worked also, or is it fixed in newer versions?

@hvr
Copy link

hvr commented May 11, 2018

@parsonsmatt If you can reproduce this with cabal new-build then it's worth filing.

@parsonsmatt
Copy link
Contributor

parsonsmatt commented May 11, 2018

Oh, I was looking at the wrong cabal file the entire time 😱 The custom-setup stanza does fail to build with both stack and cabal in the network sub-project.

@hvr
Copy link

hvr commented May 12, 2018

@parsonsmatt I'm confused now... are you saying that snoyberg screwed up by not having even tested his PR before submitting?

@tonymorris
Copy link
Contributor

@gwisl What is the correct fix from here?

@gwils
Copy link
Collaborator

gwils commented May 15, 2018

@tonymorris Rewrite/simplify the Setup.lhs file. I've just opened #280 to discuss.

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

Successfully merging this pull request may close these issues.

7 participants