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

Respect ghc-options and with-gcc when compiling C sources #5440

Closed
wants to merge 1 commit into from

Conversation

syntheorem
Copy link
Contributor

This fix allows users to override the C/C++ compiler, either by passing
--with-gcc to configure or adding -pgmc to ghc-options in their Cabal
file.

Tested manually with a project using C++ sources that require a more
recent version of clang than the default install (macOS). Inspected the
output of Setup.hs build -v to confirm that the correct arguments are
being passed to GHC when compiling the C++ sources.

Resolves: #4439

  • Patches conform to the coding conventions.
  • Any changes that could be relevant to users have been recorded in the changelog.
  • The documentation has been updated, if necessary.
  • If the change is docs-only, [ci skip] is used to avoid triggering the build bots.

This fix allows users to override the C/C++ compiler, either by passing
--with-gcc to configure or adding -pgmc to ghc-options in their Cabal
file.

Tested manually with a project using C++ sources that require a more
recent version of clang than the default install (macOS). Inspected the
output of `Setup.hs build -v` to confirm that the correct arguments are
being passed to GHC when compiling the C++ sources.

Resolves: haskell#4439
@angerman
Copy link
Collaborator

Shouldn’t this be —with-cc? Didn’t we already respect CC env vars?

@syntheorem
Copy link
Contributor Author

Shouldn’t this be —with-cc?

No, this is using the existing program replacement infrastructure, in which the C compiler is named gcc. So --with-gcc is the already existing option (stack uses it as well).

Didn’t we already respect CC env vars?

Possibly, if GHC itself respects those env vars. The problem is that we were calling GHC to compile C files without explicitly passing -pgmc, so GHC didn't know about any --with-gcc arguments that were passed to Setup configure.

@angerman
Copy link
Collaborator

Fair point on --with-gcc.

What happens if --ghc-options with -pgmc and --with-gcc is provided pointing to different ccs?

@syntheorem
Copy link
Contributor Author

Since --ghc-options are always passed last to GHC, they should override any previous options. So the precedence would be:

  1. --ghc-options on the command line
  2. ghc-options in the .cabal file
  3. --with-gcc

@angerman
Copy link
Collaborator

So I could provide --with-gcc, and end up in a situation where some other value is picked?

Should we rather WARN or even ERROR in that case?

@syntheorem
Copy link
Contributor Author

Sure, but that applies to any argument that Cabal passes to GHC. I think that ghc-options overriding everything is expected behavior.

Although maybe we want to warn upon ever seeing -pgmc in a .cabal file, since it seems questionable to configure it at all from there (because which program to use will depend on the system you're compiling on). Seems like a separate issue should be filed for that though.

@syntheorem
Copy link
Contributor Author

FYI, this is the Travis CI failure, only in the DEBUG_EXPENSIVE_ASSERTIONS build:

No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.

And this is the AppVeyor failure:

Build execution time has reached the maximum allowed time for your plan (60 minutes).

Not sure that these are actionable failures.

@nh2
Copy link
Member

nh2 commented Feb 12, 2020

What's holding this back?

It looks like a simple change.

@ezyang
Copy link
Contributor

ezyang commented Feb 14, 2020

This looks fine. Needs a rebase and merge conflict resolution but o/w ok to go in./

@phadej
Copy link
Collaborator

phadej commented Feb 14, 2020

... and test(s)

@gbaz
Copy link
Collaborator

gbaz commented Sep 6, 2021

@syntheorem sorry this languished. do you care to resolve the conflicts against master so we can proceed with it?

@Mikolaj
Copy link
Member

Mikolaj commented Dec 22, 2021

Hi! Heads up about #7874, which is this PR just rebased by @jberryman and we are desperate to merge it before it bitrots.

We need to add a small test for that or at least test the branch on the examples you have. Please help!

@Mikolaj
Copy link
Member

Mikolaj commented Jan 11, 2022

Please kindly also have a look (and review) at #7900, which provides a test and discusses its result. Last chance to provide feedback before we merge. Thank you!

@Mikolaj
Copy link
Member

Mikolaj commented Jan 12, 2022

@jberryman: thank you for #7900, which is now merged, which ends the saga! Please, everybody, test and reopen if anything doesn't work as expected.

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.

cabal does not respect ghc-options for -pgmc, -pgml etc
8 participants