Add a config option to override gcc path #2264
Conversation
This allows one to specify with-gcc: in stack.yaml.
| @@ -279,6 +280,12 @@ configOptsParser hide0 = | |||
| <> help "Extra directories to check for libraries" | |||
| <> hide | |||
| ))) | |||
| <*> optionalFirst (textOption | |||
| ( long "with-ghc" | |||
There was a problem hiding this comment.
That should probably be "with-gcc" – not sure if there would be an advantage in using configMonoidOverrideGccPathName directly…
There was a problem hiding this comment.
Not sure what you mean by configMonoidOverrideGccPathName?
There was a problem hiding this comment.
In Stack.Types.Config you define configMonoidOverrideGccPathName = "with-gcc", so you could potentially reuse that definition instead of duplicating the literal.
There was a problem hiding this comment.
I just cargo culted that - I'm happy to make the change, although I think it should be done categorically, as it's "not the way it's currently done".
Thoughts?
There was a problem hiding this comment.
I think it should be done categorically, as it's "not the way it's currently done".
Good point. You can certainly just use a literal for this PR.
| @@ -840,6 +842,8 @@ data ConfigMonoid = | |||
| -- ^ See: 'configExtraIncludeDirs' | |||
| ,configMonoidExtraLibDirs :: !(Set Text) | |||
| -- ^ See: 'configExtraLibDirs' | |||
| , configMonoidOverrideGccPath :: !(First Text) | |||
There was a problem hiding this comment.
Personally, I'd prefer a First (Path Abs File) here.
The main advantage I'd expect from that, would be better error messages than those that we currently relay from cabal or ghc.
I also think that this should be done "categorically", so for configMonoidExtraIncludeDirs, configMonoidExtraLibDirs, configMonoidWorkDir and configMonoidLocalBinPath too. (Not necessarily in this PR though)
Options.Applicative.Builder.eitherReader would probably be a useful helper function for this.
There was a problem hiding this comment.
I have opened #2267 with regard to the overly permissive types.
Given that it's a rather tangential point, feel free to change configMonoidOverrideGccPath to First (Path Abs File) or not.
There was a problem hiding this comment.
I just had a quick play with this - there's a slight problem with there being no FromJSON instance for Path Abs File etc. I'd rather fix that as part of #2267 (as it is a little tangential). I'm happy to pick that one up as well once this is done though :).
There was a problem hiding this comment.
I've already started working on #2267 so you may want to choose a different issue next. ;)
|
Can you also add a paragraph to I think this will be good to go then (if @mgsloan agrees :)). |
|
LGTM! Thanks for working on this, @khanage! |
|
I'm sorry to see this PR languishing for so long. @mgsloan: Given that I've already reviewed this, are you ok with me simply merging this? Alternatively, would anyone else be interested in giving this PR a second look? |
|
I'm fine with this. @sjakobi, go ahead and merge when ready. |
|
Are dependencies also built with the specified gcc? I have one project where I need to build a dependency with a special gcc version, I just tried to add the 'with-gcc' option in the projects stack.yaml, it did not work though. The dependency does not have a stack.yaml at the moment, so I could not add the option there. |
They certainly should. Did you make sure that the dependency was actually rebuilt? If so, please open an issue and include the output with |
|
@sjakobi I have opened an issue (see reference above) |
This allows one to specify
with-gcc:in stack.yaml.Happy for any feedback - I just picked this up as it's marked with newcomer!