Skip to content
This repository has been archived by the owner on Oct 7, 2020. It is now read-only.

Remove cabal check from stack builds #1368

Merged
merged 3 commits into from Sep 5, 2019
Merged

Conversation

ollef
Copy link
Contributor

@ollef ollef commented Aug 24, 2019

Stack should provide its own cabal, so I don't think the system cabal is
relevant here. This worked for me despite the check failing.

Stack should provide its own cabal, so I don't think the system cabal is
relevant here. This worked for me despite the check failing.
@samuelpilz
Copy link
Contributor

Cabal is regarded a runtime dependency of hie and that has nothing to do with the functionality of stack.

I can try to reproduce the issue when installing hie without having a cabal binary. Until then, this pr should not me merged.

@ollef
Copy link
Contributor Author

ollef commented Aug 24, 2019

If so, it seems like it makes more sense to perform that check at runtime, since the installed version of cabal could change after installing hie, and since you shouldn't need it to compile hie with stack.

The problem that I ran into that this fixed for me was that hie reported my system's cabal version, 2.4.0.0, as insufficient when I tried to compile it.

@jneira
Copy link
Member

jneira commented Aug 24, 2019

Mmm, some context: that was added in #1344 to close #1327, if i am not wrong. Before that, installing hie triggered installing globally the cabal version it needed, overwriting the old one.

In #1327 we discussed that maybe we could make hie use a private version of cabal, even downloading it at runtime when needed. But i am afraid that we can't simply remove it without some additional work.

@ollef have you considered upgrade your default cabal to 2.4.1.0?

@ollef
Copy link
Contributor Author

ollef commented Aug 25, 2019

Cheers for providing context. Is cabal necessary for using hie with stack projects? It seems to be working here without the correct version.

Again, I think runtime dependencies, especially when optional, are better checked at runtime.

I could upgrade, yeah.

@samuelpilz
Copy link
Contributor

The hie runtime is very tricky. I work on getting the correct checks that hie is able to perform a self test. However, I had little success so far.

As far as I know, cabal is indeed reqired, even for stack based projects.

@jneira
Copy link
Member

jneira commented Aug 26, 2019

Well, i've tested to open a stack based project removing cabal entirely from path and hie seems to work (open the project without errors), emitting a warning about not finding the cabal executable.
But i've not tested that the main features continue working.

@ollef have you tested that main features continue working with your cabal version? I guess that being a minor version mismatch (2.4.1.0vs 2.4.0.0) they'll probably work but there is no the general case.

Maybe we could add a flag to avoid/force the cabal installation (--install-cabal and --no-install-cabal or something alike), keeping the install by default for now? I would keep the check as a warning (like at runtime) that hie needs cabal (with a specific version) to work in all cases.

@ollef
Copy link
Contributor Author

ollef commented Aug 26, 2019

Ah, interesting, so there's something looking for cabal even in stack based projects. I didn't notice any warnings about my specific version of cabal, however.

As far as I can tell hie works with my cabal version, but I've only tested some of the basics.

A warning at compile-time on encountering the wrong version seems like a sensible approach to me.

@mpickering
Copy link
Collaborator

I have no idea why hie depends on cabal. It seems unecessary.. my branch doesn't depend on it, unless you are already using cabal.

@jneira
Copy link
Member

jneira commented Aug 30, 2019

I've tried stack without cabal in path a little bit more and hie seems to behave identically for stack based projects.
For cabal projects it shows this log:

info: Found Cabal project at: D:\dev\ws\haskell\cabal-test
Using hie version: Version 0.12.0.0, Git revision ff9a5b2e85a0910205cb781bf630503efe6bd227 (2995 commits) x86_64 ghc-8.6.5
info: 'cabal' executable wasn't found, trying next project type
info: Found no other project type, falling back to plain GHC project

I guess that some features will be not present.

So i am for mimic the runtime behavior in the build and keep the warn in the build if cabal is not present or the version is not adequate. At least there would be a flag to avoid the install.

@samuelpilz
Copy link
Contributor

If others have no objection to this, I'm fine with dropping the check and merge this PR.

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

Successfully merging this pull request may close these issues.

None yet

4 participants