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

Restore compatibility with GHC-7 / old Cabal environments. #747

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@leftaroundabout
Contributor

leftaroundabout commented Sep 5, 2017

Some of the changes done lately make the project compile only with GHC-8, and only when there are no obsolete package-versions installed in the Cabal environment.

Here I adress these issues so IHaskell will also install on an old GHC-7.10 setup.

leftaroundabout added some commits Sep 5, 2017

Adjust dependency lower boundary.
In `haskell-src-exts-1.17`, `Module` was not parameterised yet, so trying
to compile with that version gives

src/IHaskell/Eval/Lint.hs:36:19:
    ‘SrcExts.Module’ is applied to too many type arguments
    In the type ‘SrcExts.Module SrcSpanInfo’
    In the type declaration for ‘ExtsModule’
Restore GHC-7.10 compatibility.
Commit 3f9a422 is only valid for GHC-8, and e97b701 also
made an incorrect change in

+#if MIN_VERSION_ghc(8,0,0)
+getErrMsgDoc = ErrUtils.pprLocErrMsg
+#else
+getErrMsgDoc msg = ErrUtils.errMsgShortString msg $$ ErrUtils.errMsgContext msg
+#endif
@@ -428,10 +457,7 @@ safely state = ghandle handler . ghandle sourceErrorHandler
     sourceErrorHandler :: SourceError -> Interpreter EvalOut
     sourceErrorHandler srcerr = do
       let msgs = bagToList $ srcErrorMessages srcerr
-      errStrs <- forM msgs $ \msg -> do
-                   shortStr <- doc $ errMsgShortDoc msg
-                   contextStr <- doc $ errMsgExtraInfo msg
-                   return $ unlines [shortStr, contextStr]
+      errStrs <- forM msgs $ doc . getErrMsgDoc

I reverted these changes in the !MIN_VERSION_ghc(8,0,0) branches.
@vaibhavsagar

This comment has been minimized.

Collaborator

vaibhavsagar commented Sep 6, 2017

Thanks for this PR! Dropping 7.10 compatibility is intentional as per #716 (comment) and the README. I was actually planning to remove more of the <8.0 stuff because it complicates the codebase and makes adding support for future GHCs harder.

Can you tell me why you want 7.10 support in master when you can use the GHC7 tag?

@leftaroundabout leftaroundabout referenced this pull request Sep 6, 2017

Merged

Support GHC 8 #716

@leftaroundabout

This comment has been minimized.

Contributor

leftaroundabout commented Sep 6, 2017

Well, actually I didn't realise there's a GHC-7 tag... where does the README actually tell me about it?

The reason I want to stay in master is of course to easily keep up-to-date. Rather than switching to a legacy branch, I'd switch the compiler – I'm not overly concerned with long backwards compatibility, but I'm also not a fan of Stack's “everything in realtime” approach. GHC-7.10 isn't, like, Fortran 95 gfortran or something, it's no big deal to keep compatibility with it...

@vaibhavsagar

This comment has been minimized.

Collaborator

vaibhavsagar commented Sep 6, 2017

It's true that 7.10 is not that old, but I'm already running into issues with #735 because the GHC API has noticeably changed between 8.0 and 8.2. I don't see the benefit of keeping backwards compatibility when no new features have been added since the GHC7 tag (other than GHC 8 support). I can update the README to mention the tag, would that be sufficient resolution?

@leftaroundabout leftaroundabout deleted the leftaroundabout:adapt/dependencies branch Sep 6, 2017

leftaroundabout added a commit to leftaroundabout/IHaskell that referenced this pull request Sep 6, 2017

Reflect lack of support for GHC<8 in the dependencies.
As per gibiansky#716, GHC-7 support has at the moment no priority. Use the GHC7 tag
for the last version that retains the original GHC-7.10 support.

https://github.com/gibiansky/IHaskell/releases/tag/GHC7

(However, it is still quite feasible to support GHC-7.10 in master, see
gibiansky#747.)
@leftaroundabout

This comment has been minimized.

Contributor

leftaroundabout commented Sep 6, 2017

Ok, just the lack of support should then also be reflected in the Cabal dependencies. #748

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment