Pass *-options and -pgmc gcc to GHC when compiling ordinary Haskell sources

[zlonast]

Will close issues https://github.com/haskell/cabal/issues/9801 https://github.com/haskell/cabal/issues/4435 Main idea https://github.com/haskell/cabal/issues/9801#issuecomment-2884201996

cc-options, cxx-options, jspp-options, asm-options, cmm-options, ld-options, cpp-options, gccProgram(-pgmc) should be always passed when invoking GHC, similarly as ghc-options should be always used when invoking ghc - regardless of what is the intention of a particular GHC-call. GHC might use or not use the options, Cabal cannot know and should not guess.

cc-options and cxx-options still need to be separated (C/C++) for versions below 8.10. https://gitlab.haskell.org/ghc/ghc/-/issues/16477

gccProgram till need to be separated (C/C++) for versions below 9.4. I'm not sure what fixed this behavior in 9.4

---

Template Α: This PR modifies behaviour or interface

Include the following checklist in your PR:

• [x] Patches conform to the coding conventions.
• [x] Any changes that could be relevant to users have been recorded in the changelog.
  • [x] Is the change significant? If so, remember to add significance: significant in the changelog file.
• [ ] The documentation has been updated, if necessary.
• [ ] Manual QA notes have been included.
• [x] Tests have been added. (Ask for help if you don’t know how to write them! Ask for an exemption if tests are too complex for too little coverage!)

[zlonast]

I probably need to think about tests, maybe something started working besides CApiFFI 🤔

[zlonast]

~~Ok, this only works for things newer than 8.8~~ ~~gcc: fatal error: cannot specify ‘-o’ with ‘-c’, ‘-S’ or ‘-E’ with multiple files~~

Ok, got it, we just need to separate the flags -optc and -optcxx Pass -optcxx for GHC >= 8.10 https://github.com/haskell/cabal/pull/7072

[zlonast]

@ulysses4ever @phadej It's not very easy to add flags to sources files due to backward compatibility, so for now I suggest considering the pull request as is ¯\_(ツ)_/¯ (Anyway, this solved the problem with macros in header files)

[phadej]

did you check that this doesn't lead to duplicated options when cabal calls GHC?

Yes, it looks like so.

I'd say the problem is rather that GhcOptions is not used uniformly. There is e.g. componentCcGhcOptions which does set ghcOptCcOptions. Why these don't show up when compiling files CApiFFI headers?

IMHO, it's wrong that there are separate componentAsmGhcOptions, componentJsGhcOptions, componentGhcOptions. There should be just one componentGhcOptions with all the options, and then it's up to GHC which ones are relevant, and which aren't.

EDIT: There might be situations where ghcOptMode or something like that is different, and input files "obviously". But the bulk of options should be the same. Starting with verbosity. I'd imagine there is only single line in the codebase which does ghcOptVerbosity = toFlag (min verbosity normal). Currently there are six copies of that. Cannot be right. I suspect there are many subtle inconsistencies hiding because of that.

EDIT: So AFAICT, this is not the correct fix, not what I'd imagine seeing.

[zlonast]

Basically I just merged most of the options into one base one and added the separation in 8.10. Now buildHaskellModules, buildAllExtraSources, linkOrLoadComponent have the same base buildOpts.

[zlonast]

One general question: did you check that this doesn't lead to duplicated options when cabal calls GHC? I assume other *-options are passed today sometimes. So, sometimes we'll get those options passed as today and through your change too. Is that right? It seems undesirable.

@ulysses4ever Yes, you're right, now I've fixed it.

Can you be more specific? Do you mean it's hard to test on older GHCs? This shouldn't be a problem because the tests can run for specific versions of GHC (e.g. starting from 8.10).

@phadej correctly noted that compilerCompatVersion should have been added.

I'd say the problem is rather that GhcOptions is not used uniformly. There is e.g. componentCcGhcOptions which does set ghcOptCcOptions. Why these don't show up when compiling files CApiFFI headers?

Because they were only transferred to buildAllExtraSources files.

IMHO, it's wrong that there are separate componentAsmGhcOptions, componentJsGhcOptions, componentGhcOptions. There should be just one componentGhcOptions with all the options, and then it's up to GHC which ones are relevant, and which aren't.

I think you're right, now I did it this way.

EDIT: There might be situations where ghcOptMode or something like that is different, and input files "obviously". But the bulk of options should be the same.

Yes, I separated them into sourcesGhcOptions and componentGhcOptions.

Currently there are six copies of that. Cannot be right. I suspect there are many subtle inconsistencies hiding because of that.

Looks like I'll have to think about how to break the cabal :)

EDIT: So AFAICT, this is not the correct fix, not what I'd imagine seeing.

I hope you like the new version better.

[Mikolaj]

@zlonast: so are the CI failures expected? Some at least look intermittent (Internet is aging), so let me take the liberty of rebasing on master where some tests are marked flaky. Maybe it helps.

Regardless, if anybody would like to help move this PR forward, in particular answer the last question about testing, that would be very helpful. Thank you!

[Mikolaj]

@mergify rebase

[mergify[bot]]

rebase

✅ Branch has been successfully rebased

[ulysses4ever]

Bootstrap failure is unrelated: I've seen it several times today alone. The Windows failure does seem relevant.

[zlonast]

@zlonast: so are the CI failures expected? Some at least look intermittent (Internet is aging), so let me take the liberty of rebasing on master where some tests are marked flaky. Maybe it helps.

Regardless, if anybody would like to help move this PR forward, in particular answer the last question about testing, that would be very helpful. Thank you!

~~Yes, thanks, now I'm thinking of fixing the test for Windows (ignoring the different paths to the -pgmc file) and porting the options from Bazel for static libs (or as an alternative, not passing the --with-gcc flag for versions below 9.4.~~

~~https://github.com/tweag/rules_haskell/blob/3322610caba2ce8f17ec010b2f76a366a06f97c4/haskell/cabal.bzl#L321-L329~~

As I understand it, it would be right to solve this https://github.com/haskell/cabal/issues/4042 first, while I am not sure what solution should be proposed for versions below 9.4.

anyone interested can look at what will happen if you pass the flag for versions below 9.4

https://github.com/haskell/cabal/actions/runs/15363053137/job/43232541759?pr=10969#step:17:952

[zlonast]

@Mikolaj can we add GCCPATH: ${{ steps.setup-haskell.outputs.gcc-exe }}? I'm not very good at writing tests where the path for -pgmc appears

[Mikolaj]

@Mikolaj can we add GCCPATH: ${{ steps.setup-haskell.outputs.gcc-exe }}? I'm not very good at writing tests where the path for -pgmc appears

@zlonast: do you mean to cabal CI? What does it do?

[zlonast]

@Mikolaj can we add GCCPATH: ${{ steps.setup-haskell.outputs.gcc-exe }}? I'm not very good at writing tests where the path for -pgmc appears

@zlonast: do you mean to cabal CI? What does it do?

It's like GHCPATH just for gcc or clang, now I wrote 5 tests because of different paths in different windows versions :(

[Mikolaj]

Sounds good to me. Please go ahead. But just in case and let me ping @geekosaur, @ulysses4ever and @mpickering about this change and the review of the whole PR.

[ulysses4ever]

@zlonast can you comment which suggestions were implemented and give an overview of what's done currently?

[zlonast]

@zlonast can you comment which suggestions were implemented and give an overview of what's done currently?

In this pr I added: cc-options, cxx-options, jspp-options, asm-options, cmm-options, ld-options, cpp-options and ccProgram to componentGhcOptions.

Removed all componentCcGhcOptions, componentCxxGhcOptions, componentAsmGhcOptions, componentJsGhcOptions, componentCmmGhcOptions.

These options still need to be separated for older versions. cc-options and cxx-options need separateGhcOptions for versions below 8.10.

The suggestion that gcc(ccProgram) can always be passed is good, but this is not entirely true for versions below 9.4. Because when passing ghc implicitly, it added the correct flags itself. (probably it is possible to read it somehow, but I lost, see the message above)

I also took out the common part of the linking options. componentGhcOptions and linkGhcOptions

Written a test for capi. Written tests for ShowBuildInfo (there are 5 of them now, but if we add GCCPATH, we can make 2).

[ulysses4ever]

Also, great summary, thanks! Could you also change the PR title and description to better reflect the current state. For instance, the title shouldn't say what Cabal doesn't do, instead it should say what PR does.

[mpickering]

What is the testing strategy for this patch? This is some of the most sensitive code in cabal, so I am nervous that many lines are changed in this patch.

[mpickering]

I compiled stackage with this branch and GHC-9.10.2 and all the packages compiled.

[ulysses4ever]

@zlonast could you post a little summary on the state of this patch? Do you believe you addressed all Matt's concerns? I see one unresolved item here but maybe it's addressed elsewhere? If you're done, please ping Matt for re-reviewing.

Rebasing on master is also advisable.

There's still a chance we could get this into 3.16 but it's getting tight...

[zlonast]

@mpickering I tried to correct your comments. True, I don't know how to run stackage build. (But I would be glad if you told me how you did it) (for now it seems scary to try to install all the packages) Also, I don't know how to comment on "problems with linking", since I'm not sure how exactly it was fixed.

[geekosaur]

Perhaps https://github.com/haskell/clc-stackage would be of help here?

[zlonast]

Perhaps https://github.com/haskell/clc-stackage would be of help here?

Yes thanks.

I compiled stackage with this branch and GHC-9.10.2 and all the packages compiled.

I'll be honest, I have one error. And because the library has no flags for mac os.

https://hackage.haskell.org/package/clock-0.8.4/docs/System-Clock.html#t:Clock

Constructors	Comments
MonotonicRaw	(since Linux 2.6.28, macOS 10.12)
MonotonicCoarse	(since Linux 2.6.32; Linux-specific)

Error with om-time package on my mac os:

Resolving dependencies...
Build profile: -w ghc-9.10.2 -O0
In order, the following will be built (use -v for more details):
 - om-time-0.3.1.1 (lib) (requires build)
 - generated-0.1.0.0 (lib) (configuration changed)
Starting     om-time-0.3.1.1 (lib)
Building     om-time-0.3.1.1 (lib)

Failed to build om-time-0.3.1.1.
Build log (
/Users/iliabaryshnikov/.cabal/logs/ghc-9.10.2/m-tm-0.3.1.1-ffda054d.log ):
Warning: this is a debug build of cabal-install with assertions enabled.
Configuring library for om-time-0.3.1.1...
Warning: this is a debug build of cabal-install with assertions enabled.
Preprocessing library for om-time-0.3.1.1...
Building library for om-time-0.3.1.1...
[1 of 1] Compiling OM.Time          ( src/OM/Time.hs, dist/build/OM/Time.o, dist/build/OM/Time.dyn_o )
src/OM/Time.hs:56:27: error: [GHC-76037]
    Not in scope: data constructor ‘Clock.MonotonicCoarse’
    NB: the module ‘System.Clock’ does not export ‘MonotonicCoarse’.
    Suggested fix:
      Perhaps use ‘Clock.MonotonicRaw’ (imported from System.Clock)
   |
56 |   getTime = Clock.getTime Clock.MonotonicCoarse
   |                           ^^^^^^^^^^^^^^^^^^^^^

[zlonast]

I compiled stackage with this branch and GHC-9.10.2 and all the packages compiled.

Ok, now I have it on Linux too. :)

[phadej]

I realised that we should only pass -pgmc gcc to GHC when user explicitly passed --with-gcc to cabal-install / Cabal. GHC has its own C compiler (in ghc --info) and uses it, and we probably shouldn't "overwrite" it if not explicitly asked.

In fact, if we need to run gcc explicitly ourselves with GHC (which I think we should not need), then we should ask for C compiler command from GHC.

If this insight makes something in this PR easier, go for it; if not, ignore for time being.

[Mikolaj]

@zlonast: anything blocking this except for the second review?

@mpickering: could you please have another look now?

[zlonast]

@Mikolaj thanks for pushing the discussions :)

Maybe it's worth merging this first and trying to explore what's going on in this area.

I would suggest making PRs on:

1. asm-options (No tests)
2. ld-options (Only ParserTests)
3. cpp-options (Only ParserTests)
4. --pgm* (No tests)
5. --with-gcc (No tests) (I think it's worth checking whether everything works correctly next to --with-gcc and how --pgmc affects it, or simply called testing ghc work)
6. Well, and add GCCPATH by analogy with GHCPATH, to reduce single tests into one
7. I would also like to discuss why .h should be added to extra-source-files and is not passed from include-dirs, extra-libraries, c-sources, cxx-sources or etc

[zlonast]

Perhaps we should think about what link-related testing strategy we would like to choose before accepting prs like this 🤔

[mpickering]

I realised that we should only pass -pgmc gcc to GHC when user explicitly passed --with-gcc to cabal-install / Cabal. GHC has its own C compiler (in ghc --info) and uses it, and we probably shouldn't "overwrite" it if not explicitly asked.

In fact, if we need to run gcc explicitly ourselves with GHC (which I think we should not need), then we should ask for C compiler command from GHC.

If this insight makes something in this PR easier, go for it; if not, ignore for time being.

Cabal discovers which C compiler to use by default by querying ghc, so it should make no difference to explicitly pass -pgmc.