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

zlonast · 2025-05-22T16:23:54Z

Will close issues #9801 #4435
Main idea #9801 (comment)

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.

gccProgram till need to be separated (C/C++) for versions below 9.4.

Template Α: This PR modifies behaviour or interface

Include the following checklist in your PR:

Patches conform to the coding conventions.
Any changes that could be relevant to users have been recorded in the changelog.
- 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.
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 · 2025-05-22T17:54:50Z

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

zlonast · 2025-05-22T18:12:29Z

~~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 #7072

zlonast · 2025-05-24T17:04:23Z

@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)

ulysses4ever

Great start, thank you! Below are some inline questions.

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.

It's not very easy to add flags to sources files due to backward compatibility

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).

changelog.d/pr-10969.md

ulysses4ever · 2025-05-24T17:54:12Z

cabal-testsuite/PackageTests/FFI/ForeignOptsCapi/README.md

@@ -0,0 +1,3 @@
+# ForeignOptsCapi
+
+This test case asserts that cabal passes `cc-options` to the C compiler when use `foreign import capi`.


But the patch does something to other *-options too. Is it possible to test this change w.r.t. other *-options?

I need to think about it, but I don't know yet. Maybe I need to read other issues to see if anything worked.

They worked as long as we manually transferred them to the source files, but they should also work if we pass them on to everyone. Since no errors were found, and the linking errors with the transfer of ccProgram are related to ghc, I don't know what else to add. 🤔

Could you, at least, check that the test suite has tests for all the other-language options (ASM, JS, CXX, CMM, LD)? And if some of these options don't have tests, could you add something simple for them? I agree that it's not fun to do this exercise, but I think it's a big change and I want at least a minimal guarantee that we're not breaking things badly.

That's a really good opinion, I'll check it out and come back with the results.

cc-options - cabal-testsuite/PackageTests/FFI/ForeignOptsC/README.md
cxx-options - cabal-testsuite/PackageTests/FFI/ForeignOptsCxx/README.md
jspp-options - cabal-testsuite/PackageTests/JS/JSPPOptions/README.md
asm-options - none
cmm-options - cabal-testsuite/PackageTests/Cmm/CmmSources/README.md
ld-options - ParserTests
cpp-options - ParserTests
-pgmc - cabal-testsuite/PackageTests/ShowBuildInfo/Complex
-O1 and -O2 for C++ and C and Asm - cabal-testsuite/PackageTests/ShowBuildInfo/Complex

if it still reproduces we should start tests for asm-options and -pgmc/-pgma #6534

geekosaur

Same concerns as @ulysses4ever raised.

Cabal/src/Distribution/Simple/GHC/Internal.hs

phadej · 2025-05-24T19:39:44Z

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.

Mikolaj · 2025-06-05T17:28:42Z

@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 · 2025-06-05T17:28:50Z

@mergify rebase

All resource options now use base options. Add test for FFI/ForeignOptsCapi. Update test ShowBuildInfo/Complex. Add ghcOptCcProgram to componentGhcOptions.

mergify · 2025-06-05T17:29:01Z

rebase

✅ Branch has been successfully rebased

ulysses4ever · 2025-06-06T03:34:32Z

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

zlonast · 2025-06-06T06:52:13Z

@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 #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 · 2025-06-07T08:15:08Z

@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 · 2025-06-07T11:14:27Z

@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 · 2025-06-07T11:17:03Z

@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 · 2025-06-07T12:55:49Z

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 · 2025-06-07T15:40:58Z

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

zlonast · 2025-06-07T16:10:46Z

@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

So, what about possible option duplication? Are we sure we don't pass things twice? Even on older GHCs?

ulysses4ever · 2025-06-07T18:31:49Z

Cabal/src/Distribution/Simple/GHC.hs

-          , ghcOptExtra = hcOptions GHC libBi ++ hcSharedOptions GHC libBi
+          , ghcOptExtra = hcSharedOptions GHC libBi


@zlonast could you comment on this and similar changes below: why is hcOptions not needed anymore?

This is a duplicate of options from componentGhcOptions

vanillaArgs = (Internal.componentGhcOptions verbosity lbi libBi clbi (componentBuildDir lbi clbi)) `mappend` mempty { ghcOptMode = toFlag GhcModeAbiHash , ghcOptInputModules = toNubListR $ exposedModules lib } sharedArgs = vanillaArgs `mappend` mempty { ghcOptDynLinkMode = toFlag GhcDynamicOnly , ghcOptFPic = toFlag True , ghcOptHiSuffix = toFlag "dyn_hi" , ghcOptObjSuffix = toFlag "dyn_o" , ghcOptExtra = hcOptions GHC libBi ++ hcSharedOptions GHC libBi }

ulysses4ever · 2025-06-07T18:34:45Z

Cabal/src/Distribution/Simple/GHC/Build/ExtraSources.hs

+        (Internal.sourcesGhcOptions verbosity lbi bi clbi odir filename)
+          { -- C++ compiler options: GHC >= 8.10 requires -optcxx, older requires -optc
+            -- we want to be able to support cxx-options and cc-options separately
+            ghcOptCxxOptions = Internal.separateGhcOptions (mkVersion [8, 10]) (compiler lbi) (Internal.defaultGhcOptCxxOptions lbi bi)


This is probably silly but I'll ask anyway: why do we touch a cxx-related field in a function that's called buildCSources? Looks like only C-related options should be touched.

I did read the comment above, which probably explains it, but I didn't understand how it's related to my conccern.

hmm, I guess I should somehow refer to the tests cabal-testsuite/PackageTests/FFI/ForeignOptsC

#include "clib.h" #ifndef __TESTOPT_C__ #error "Did not get required __TESTOPT_C__ from cc-options" #endif #ifdef __TESTOPT_CXX__ #error "Got unexpected __TESTOPT_CXX__ from cxx-options" #endif int meaning_of_life_c() { return __TESTOPT_C__; }

the test is just for this

executable foreign-opts-c-exe main-is: Main.hs build-depends: base default-language: Haskell2010 include-dirs: cbits c-sources: cbits/clib.c -- The following options are shared in all ForeignOpts cases: cc-options: -D__TESTOPT_C__=11 cxx-options: -D__TESTOPT_CXX__=22

ulysses4ever · 2025-06-07T18:35:06Z

Cabal/src/Distribution/Simple/GHC/Build/ExtraSources.hs

+          { -- C++ compiler options: GHC >= 8.10 requires -optcxx, older requires -optc
+            -- we want to be able to support cxx-options and cc-options separately
+            ghcOptCxxOptions = Internal.separateGhcOptions (mkVersion [8, 10]) (compiler lbi) (Internal.defaultGhcOptCxxOptions lbi bi)
+          , -- there are problems with linking with versions lower than 9.4, that's why we need this duplication


what "duplication"? With the code for buildCxxOptions below? Please, mention it

I need to change the description, then the line is needed to re-assign ghcOptCcProgram

ulysses4ever · 2025-06-07T18:38:20Z

Cabal/src/Distribution/Simple/GHC/Build/ExtraSources.hs

+            ghcOptCcOptions = Internal.separateGhcOptions (mkVersion [8, 10]) (compiler lbi) (Internal.defaultGhcOptCcOptions lbi bi)
+          , -- there are problems with linking with versions lower than 9.4, that's why we need this duplication
+            ghcOptCcProgram = Internal.defaultGhcOptCcProgram lbi


Same: C-options touched inside a Cxx-function. If this is related to the optcxx vs optc mishmash pre 8.10, I need some sort of simple example to understand how what's done here (and above) helps with that mishmash.

I should somehow refer to the tests cabal-testsuite/PackageTests/FFI/ForeignOptsCxx

ulysses4ever · 2025-06-07T18:39:55Z

Cabal/src/Distribution/Simple/GHC/Build/ExtraSources.hs

                    }
              profSharedSrcOpts =
                vanillaSrcOpts
                  `mappend` mempty
                    { ghcOptProfilingMode = toFlag True
-                    , ghcOptFPic = toFlag True


The change to fpic business is worrying, it should be loudly documented (including the changelog, probably).

This is a duplicate of the vanillaSrcOpts

vanillaSrcOpts = -- -fPIC is used in case you are using the repl -- of a dynamically linked GHC baseSrcOpts{ghcOptFPic = toFlag True}

ulysses4ever · 2025-06-07T18:43:25Z

Cabal/src/Distribution/Simple/GHCJS.hs

+              ( Internal.sourcesGhcOptions verbosity lbi bnfo clbi tmpDir filename
+                  <> mempty
+                    { ghcOptCcOptions =


( Internal.sourcesGhcOptions verbosity lbi bnfo clbi tmpDir filename <> mempty { ghcOptCcOptions =

is it not the same as

Suggested change

( Internal.sourcesGhcOptions verbosity lbi bnfo clbi tmpDir filename

<> mempty

{ ghcOptCcOptions =

( (Internal.sourcesGhcOptions verbosity lbi bnfo clbi tmpDir filename)

{ ghcOptCcOptions =

? (I hope I'm not making up this syntax!)

Yes, you are right, it should be done the same for GHC and GHCJS, it should be done without mempty, because it is a replacement, not an addition.

ulysses4ever · 2025-06-07T18:46:09Z

changelog.d/pr-10969.md

This should have significant: true. I have a slight suspicion that we'll get a fallout after this change, so I want it to be extra visible.

ulysses4ever · 2025-06-07T18:49:43Z

cabal-testsuite/PackageTests/FFI/ForeignOptsCapi/README.md

@@ -0,0 +1,3 @@
+# ForeignOptsCapi
+
+This test case asserts that cabal passes `cc-options` to the C compiler when use `foreign import capi`.


Could you, at least, check that the test suite has tests for all the other-language options (ASM, JS, CXX, CMM, LD)? And if some of these options don't have tests, could you add something simple for them? I agree that it's not fun to do this exercise, but I think it's a big change and I want at least a minimal guarantee that we're not breaking things badly.

ulysses4ever · 2025-06-07T19:03:01Z

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.

zlonast force-pushed the options-invoking-ghc branch 2 times, most recently from 1bd501e to c3e68be Compare May 22, 2025 17:02

zlonast marked this pull request as ready for review May 22, 2025 17:52

zlonast added this to the Considered for 3.16 milestone May 22, 2025

zlonast force-pushed the options-invoking-ghc branch 10 times, most recently from a439429 to 1b8e5bb Compare May 24, 2025 15:33

zlonast requested review from phadej, ulysses4ever and mpickering May 24, 2025 17:05

ulysses4ever reviewed May 24, 2025

View reviewed changes

ulysses4ever added the attention: needs-review label May 24, 2025

This was linked to issues May 24, 2025

cc-options do not get passed to GHC #4435

Open

Cabal doesn't pass cc-options to GHC when compiling ordinary Haskell sources #9801

Open

geekosaur requested changes May 24, 2025

View reviewed changes

phadej reviewed May 24, 2025

View reviewed changes

Cabal/src/Distribution/Simple/GHC/Internal.hs Outdated Show resolved Hide resolved

zlonast force-pushed the options-invoking-ghc branch 3 times, most recently from 0b74992 to 37417aa Compare May 28, 2025 16:26

zlonast force-pushed the options-invoking-ghc branch 3 times, most recently from ccbf2ce to 75993c3 Compare June 1, 2025 17:16

Merging all resource flags.

22d1f58

All resource options now use base options. Add test for FFI/ForeignOptsCapi. Update test ShowBuildInfo/Complex. Add ghcOptCcProgram to componentGhcOptions.

Mikolaj force-pushed the options-invoking-ghc branch from 75993c3 to 22d1f58 Compare June 5, 2025 17:29

zlonast force-pushed the options-invoking-ghc branch 5 times, most recently from 8abdd39 to 9d73a34 Compare June 7, 2025 07:59

fix for windows ci :(

3b8a89e

zlonast force-pushed the options-invoking-ghc branch from 9d73a34 to 3b8a89e Compare June 7, 2025 08:57

zlonast requested a review from phadej June 7, 2025 10:01

ulysses4ever reviewed Jun 7, 2025

View reviewed changes

zlonast changed the title ~~Cabal doesn't pass *-options to GHC when compiling ordinary Haskell sources~~ Pass *-options and -pgmc gcc to GHC when compiling ordinary Haskell sources Jun 10, 2025

zlonast added 2 commits June 10, 2025 12:01

move PackageTests/Cmm* to PackageTests/Cmm/*

0fcb121

add more comments and change changelog.d/pr-10969.md

ce977c6

zlonast force-pushed the options-invoking-ghc branch from 5bc91c4 to ce977c6 Compare June 10, 2025 09:40

		@@ -0,0 +1,3 @@
		# ForeignOptsCapi

		This test case asserts that cabal passes `cc-options` to the C compiler when use `foreign import capi`.

		, ghcOptExtra = hcOptions GHC libBi ++ hcSharedOptions GHC libBi
		, ghcOptExtra = hcSharedOptions GHC libBi

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

Are you sure you want to change the base?

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

Uh oh!

Conversation

zlonast commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zlonast commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zlonast commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zlonast commented May 24, 2025

Uh oh!

ulysses4ever left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geekosaur left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

phadej commented May 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mikolaj commented Jun 5, 2025

Uh oh!

Mikolaj commented Jun 5, 2025

Uh oh!

mergify bot commented Jun 5, 2025

✅ Branch has been successfully rebased

Uh oh!

ulysses4ever commented Jun 6, 2025

Uh oh!

zlonast commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zlonast commented Jun 7, 2025

Uh oh!

Mikolaj commented Jun 7, 2025

Uh oh!

zlonast commented Jun 7, 2025

Uh oh!

Mikolaj commented Jun 7, 2025

Uh oh!

ulysses4ever commented Jun 7, 2025

Uh oh!

zlonast commented Jun 7, 2025

Uh oh!

ulysses4ever left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

zlonast commented May 22, 2025 •

edited

Loading

zlonast commented May 22, 2025 •

edited

Loading

zlonast commented May 22, 2025 •

edited

Loading

phadej commented May 24, 2025 •

edited

Loading

zlonast commented Jun 6, 2025 •

edited

Loading