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

Stack shouldn't try and use a 8.0.1 resolver when the base bound is for an older version. #2502

Closed
Gurkenglas opened this issue Aug 16, 2016 · 6 comments
Assignees

Comments

@Gurkenglas
Copy link
Contributor

@Gurkenglas Gurkenglas commented Aug 16, 2016

http://ircbrowse.net/browse/haskell?id=23226153&timestamp=1471369545#t1471369545

Steps to reproduce what led up to the line linked above: Have this terminal session, then this conversation leading up to that line.

@Blaisorblade
Copy link
Collaborator

@Blaisorblade Blaisorblade commented Aug 16, 2016

Thanks for the ticket. But while the output is a bit confusing, stack's heuristics still end up with sensible guesses. The bigger problem is that after Trying with packages from lts-5.18 as hard constraints... (and failing), it errors out with Could not parse cabal-install errors:—otherwise it'd probably keep trying farther. What looks unusual in the errors is the .EXE below:

>>>> Cabal errors begin
cabal.EXE: Could not resolve dependencies:

Regarding the original report, below's an analysis, but I'm more interested in the parsing errors above.


To summarize, that package says base >=4.8 && <4.9 (here), but stack tries various resolvers including ones for the wrong GHC, not 7.10. It tries both nightly (too new) and lts-0, 1, 2 (which have GHC 7.8).

AFAICS it even tries a few sensible resolvers which fail for other reasons:

mpickering: The problem is that the package requires 7.10.3 but newer versions of http-conduit than in the last lts to use 7.10.3

I agreed it could be nice to be smarter. (And maybe this should even be priority 2).

Making it perfect might be nontrivial, but this would be a heuristic enhancement so we can focus on typical constraints. We'd need a mapping from base major versions to GHC versions (e.g. 4.8->7.10), which is easy enough to hardcode for the supported versions assuming simple constraints—though we probably should have it in some external DB.

However, that might be overkill, since the info is in the snapshots themselves. We already detect incompatible resolvers (and explain the conflict); we might want to hide failures when base doesn't match.

@Blaisorblade
Copy link
Collaborator

@Blaisorblade Blaisorblade commented Aug 16, 2016

According to the conversation, what stack should have tried (and didn't) was stack init --resolver=ghc-7.10.3.

But I've tried this locally and stack came up with a plan! Looking more closely at the source, the problem might be Windows-specific, but not due to .EXE. I think T.isSuffixOf "(user goal)" fails because parseCabalErrors doesn't remove \r with stripCR, hence the lines end in \r 😢 . That's because Windows ends lines with \r\n and lines splits on \n. The real bug, though, is opening the pipe in binary mode rather than text mode (which would translate \r\n to \n)—that's where \r should be vanquished.

Below's what I get on OS X—basically, a config based on lts-5. @Gurkenglas Can I assume you would you be happy with that result?


The stack.yaml I get is essentially:

resolver: lts-5.18
packages:
- '.'
# Dependency packages to be pulled from upstream that are not in the resolver
# (e.g., acme-missiles-0.3)
extra-deps:
- conduit-extra-1.1.13.2
- http-client-0.5.1
- http-client-tls-0.3.0
- http-conduit-2.2.0

The log of init --solver ends with:

Trying with packages from lts-5.18 as hard constraints...
Attempt failed.

>>>> Cabal errors begin
Warning: cannot determine version of /usr/bin/strip :
""
cabal: Could not resolve dependencies:
trying: currency-convert-0.2.1.0 (user goal)
next goal: http-conduit (dependency of currency-convert-0.2.1.0)
rejecting: http-conduit-2.2.0, 2.1.11 (global constraint requires ==2.1.10.1)
rejecting: http-conduit-2.1.10.1 (conflict: currency-convert =>
http-conduit>=2.2 && <2.3)
rejecting: http-conduit-2.1.10, 2.1.9, 2.1.8, 2.1.7.2, 2.1.7.1, 2.1.7, 2.1.6,
2.1.5.1, 2.1.5, 2.1.4.7, 2.1.4.5, 2.1.4.4, 2.1.4.3, 2.1.4.2, 2.1.4.1, 2.1.4,
2.1.3, 2.1.2.3, 2.1.2.2, 2.1.2.1, 2.1.2, 2.1.1, 2.1.0, 2.0.0.10, 2.0.0.9,
2.0.0.8, 2.0.0.7, 2.0.0.6, 2.0.0.5, 2.0.0.4, 2.0.0.3, 2.0.0.2, 2.0.0.1, 2.0.0,
1.9.6, 1.9.5.3, 1.9.5.2, 1.9.5.1, 1.9.5, 1.9.4.5, 1.9.4.4, 1.9.4.3, 1.9.4.2,
1.9.4.1, 1.9.4, 1.9.3, 1.9.2.2, 1.9.2.1, 1.9.2, 1.9.1, 1.9.0, 1.8.9, 1.8.8,
1.8.7.1, 1.8.7, 1.8.6.3, 1.8.6.2, 1.8.6.1, 1.8.6, 1.8.5.2, 1.8.5.1, 1.8.5,
1.8.4.5, 1.8.4.4, 1.8.4.3, 1.8.4.2, 1.8.4.1, 1.8.4, 1.8.3, 1.8.2.1, 1.8.2,
1.8.1, 1.8.0, 1.7.0, 1.6.1.2, 1.6.1.1, 1.6.1, 1.6.0.4, 1.6.0.3, 1.6.0.2,
1.6.0.1, 1.6.0, 1.5.0.3, 1.5.0.2, 1.5.0.1, 1.5.0, 1.4.1.10, 1.4.1.9, 1.4.1.8,
1.4.1.7, 1.4.1.6, 1.4.1.5, 1.4.1.4, 1.4.1.3, 1.4.1.2, 1.4.1.1, 1.4.1, 1.4.0.2,
1.4.0.1, 1.4.0, 1.3.0.1, 1.3.0, 1.2.6, 1.2.5, 1.2.4, 1.2.3, 1.2.2, 1.2.1,
1.2.0, 1.1.2.2, 1.1.1, 1.1.0.1, 1.1.0, 1.0.0.1, 1.0.0 (global constraint
requires ==2.1.10.1)
Dependency tree exhaustively searched.
<<<< Cabal errors end

Retrying with packages from lts-5.18 as preferences...
Successfully determined a build plan with 4 external dependencies.
Initialising configuration using resolver: lts-5.18
Total number of user packages considered: 1
Warning! 4 external dependencies were added.
Writing configuration to file: stack.yaml
All done.
Blaisorblade added a commit that referenced this issue Aug 17, 2016
Noticed on #2502. Basically, `isSuffixOf "suffix" "stuff suffix\r"` is
spuriously false, so we should better strip `\r`.

I've not yet verified that `\r` is actually there; I infer it from the
presence of `stripCR` and the parse failure. Time to test this.
@Blaisorblade Blaisorblade self-assigned this Aug 17, 2016
Blaisorblade added a commit that referenced this issue Aug 17, 2016
As shown in #2502, `cabal init --solver` can fail with `Could not parse
cabal-install errors` on Windows.  The problem boils down to the fact that
we test `isSuffixOf "suffix" "stuff suffix\r"`, which is false, so we
should better strip `\r` before calling `isSuffixOf`.

I've verified that `\r` is actually there only indirectly; I inferred it
from the presence of `stripCR` and the parse failure, and I've recently
seen the behavior of lines on strings with Windows line ends.

Testing confirms this PR fixes the bug—the original procedure fails with
the parent commit and works with this one.
@Blaisorblade
Copy link
Collaborator

@Blaisorblade Blaisorblade commented Aug 17, 2016

Regarding the original request, stack in fact figures the GHC mismatch and says:

* Rejected nightly-2016-08-16
    ghc-8.0.1 cannot be used for these packages:
        - currency-convert
    base version 4.9.0.0 found
        - currency-convert requires >=4.8 && <4.9

The only annoyance is that it must download the build plan, and that's slow the first time, but it's cached afterwards.

The underlying other bug is fixed in #2508.

@Gurkenglas
Copy link
Contributor Author

@Gurkenglas Gurkenglas commented Aug 17, 2016

@Gurkenglas Can I assume you would you be happy with that result?

I'm not sure what I need, but if that makes it build then my guess is that I would.

@Blaisorblade
Copy link
Collaborator

@Blaisorblade Blaisorblade commented Aug 17, 2016

Good to hear! #2508 fixes the problem for me. You can try out the branch, or try with stack upgrade --git as soon as that PR is merged.

@Gurkenglas
Copy link
Contributor Author

@Gurkenglas Gurkenglas commented Aug 18, 2016

Works.

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

Successfully merging a pull request may close this issue.

2 participants
You can’t perform that action at this time.