cabal icon indicating copy to clipboard operation
cabal copied to clipboard

lib:Cabal - do not use GHC to configure LD.

Open angerman opened this issue 7 months ago • 9 comments

Cabal uses a peculiar c program to check if LD supports and should use -x. To do this, it shells out to GHC to compiler the C file. This however requires that GHC will not bail out, yet cabal does not pass --package-db flags to this GHC invocation, and as such we can run into situations where GHC bails out, especially during GHC bootstrap phases where not all boot packages are available.

We however do not need GHC to compiler a c program, and can rely on the C compiler.

Fundamentally cabal does not allow modelling program dependencies in the program db, as such we must configure gcc first before using it.

We make a small change to lib:Cabal (specifically the GHC module, and it's Internal companion) to allow it to configure gcc first, before trying to configure ld, and thus having gcc in scope while configuring ld. This removes the need for the awkward ghc invocation to compiler the test program.

E.g. the PR only touches documentation or tests, does refactorings, etc.

Include the following checklist in your PR:

  • [ ] Patches conform to the coding conventions.
  • [ ] Is this a PR that fixes CI? If so, it will need to be backported to older cabal release branches (ask maintainers for directions).

angerman avatar May 27 '25 00:05 angerman

Huh. I wonder if this would solve a problem an OI user was having trying to get things to work a few months ago; it was failing around where you described. https://matrix.to/#/!pAORzwKXYKXnnMpVxT:matrix.org/$yryJoFP9O1XxKDT9E93xKjmuWnMLBCqqkDo0EIrT9Zc?via=matrix.org&via=kf8nh.com&via=tchncs.de

geekosaur avatar May 27 '25 00:05 geekosaur

Huh. I wonder if this would solve a problem an OI user was having trying to get things to work a few months ago; it was failing around where you described. https://matrix.to/#/!pAORzwKXYKXnnMpVxT:matrix.org/$yryJoFP9O1XxKDT9E93xKjmuWnMLBCqqkDo0EIrT9Zc?via=matrix.org&via=kf8nh.com&via=tchncs.de

Interesting, sadly most of the pastebins are now unavailable. 🤷‍♂️

angerman avatar May 27 '25 01:05 angerman

So is the user, AFAICT.

geekosaur avatar May 27 '25 01:05 geekosaur

The testsuite-failure is actually quite interesting:

PackageTests/Init/init-without-git.test.hs                                                                        FAIL (0.42s)
$ /usr/local/.ghcup/ghc/9.6.7/bin/runghc-9.6.7 -- '--ghc-arg=-i' '--ghc-arg=-hide-all-packages' '--ghc-arg=-no-user-package-db' '--ghc-arg=-package-db' '--ghc-arg=/home/runner/.cabal/store/ghc-9.6.7/package.db' '--ghc-arg=-package-db' '--ghc-arg=/home/runner/work/cabal/cabal/dist-newstyle-validate-ghc-9.6.7/packagedb/ghc-9.6.7' '--ghc-arg=-package-id' '--ghc-arg=Cabal-3.15.0.0-inplace' '--ghc-arg=-package-id' '--ghc-arg=Cabal-hooks-3.16-inplace' '--ghc-arg=-package-id' '--ghc-arg=Cabal-syntax-3.15.0.0-inplace' '--ghc-arg=-package-id' '--ghc-arg=base-4.18.3.0' '--ghc-arg=-package-id' '--ghc-arg=bytestring-0.11.5.4' '--ghc-arg=-package-id' '--ghc-arg=cabal-testsuite-3-inplace' '--ghc-arg=-package-id' '--ghc-arg=containers-0.6.7' '--ghc-arg=-package-id' '--ghc-arg=directory-1.3.8.5' '--ghc-arg=-package-id' '--ghc-arg=exceptions-0.10.7' '--ghc-arg=-package-id' '--ghc-arg=filepath-1.4.301.0' '--ghc-arg=-package-id' '--ghc-arg=process-1.6.19.0' '--ghc-arg=-package-id' '--ghc-arg=time-1.12.2' '--ghc-arg=-package-id' '--ghc-arg=transformers-0.6.1.0' '--ghc-arg=-package-id' '--ghc-arg=unix-2.8.6.0' PackageTests/Init/init-without-git.test.hs --builddir /home/runner/work/cabal/cabal/dist-newstyle-validate-ghc-9.6.7/build/x86_64-linux/ghc-9.6.7/cabal-testsuite-3 PackageTests/Init/init-without-git.test.hs '--extra-package-db=/home/runner/work/cabal/cabal/testdb/intree/store/ghc-9.6.7/package.db' '--extra-package-db=/home/runner/work/cabal/cabal/testdb/intree/dist-newstyle/packagedb/ghc-9.6.7' --with-cabal /home/runner/work/cabal/cabal/dist-newstyle-validate-ghc-9.6.7/build/x86_64-linux/ghc-9.6.7/cabal-install-3.15.0.0/x/cabal/build/cabal/cabal --with-ghc ghc
stdout:
+ /usr/bin/git ls-files --cached --modified
app/Main.hs
init-backup.out
init-backup.test.hs
init-interactive-empty-folder.out
init-interactive-empty-folder.test.hs
init-interactive-ghc2021.out
init-interactive-ghc2021.test.hs
init-interactive-legacy.out
init-interactive-legacy.test.hs
init-interactive.out
init-interactive.test.hs
init-legacy.out
init-legacy.test.hs
init-without-git.out
init-without-git.test.hs
init.out
init.test.hs
Copying /home/runner/work/cabal/cabal/cabal-testsuite/PackageTests/Init/app/Main.hs to /tmp/cabal-testsuite-123966/app/Main.hs
Copying /home/runner/work/cabal/cabal/cabal-testsuite/PackageTests/Init/init-backup.out to /tmp/cabal-testsuite-123966/init-backup.out
Copying /home/runner/work/cabal/cabal/cabal-testsuite/PackageTests/Init/init-interactive-empty-folder.out to /tmp/cabal-testsuite-123966/init-interactive-empty-folder.out
Copying /home/runner/work/cabal/cabal/cabal-testsuite/PackageTests/Init/init-interactive-ghc2021.out to /tmp/cabal-testsuite-123966/init-interactive-ghc2021.out
Copying /home/runner/work/cabal/cabal/cabal-testsuite/PackageTests/Init/init-interactive-legacy.out to /tmp/cabal-testsuite-123966/init-interactive-legacy.out
Copying /home/runner/work/cabal/cabal/cabal-testsuite/PackageTests/Init/init-interactive.out to /tmp/cabal-testsuite-123966/init-interactive.out
Copying /home/runner/work/cabal/cabal/cabal-testsuite/PackageTests/Init/init-legacy.out to /tmp/cabal-testsuite-123966/init-legacy.out
Copying /home/runner/work/cabal/cabal/cabal-testsuite/PackageTests/Init/init-without-git.out to /tmp/cabal-testsuite-123966/init-without-git.out
Copying /home/runner/work/cabal/cabal/cabal-testsuite/PackageTests/Init/init.out to /tmp/cabal-testsuite-123966/init.out
# cabal init
+ /home/runner/work/cabal/cabal/dist-newstyle-validate-ghc-9.6.7/build/x86_64-linux/ghc-9.6.7/cabal-install-3.15.0.0/x/cabal/build/cabal/cabal init '-vverbose +markoutput +nowrap' -i
Warning: this is a debug build of cabal-install with assertions enabled.
Error: [Cabal-6666]
The program 'gcc' is required but it could not be found.
stderr:
Symlinking /home/runner/.ghcup/bin/ghc <== /tmp/bin-123966/ghc
Symlinking /home/runner/work/cabal/cabal/dist-newstyle-validate-ghc-9.6.7/build/x86_64-linux/ghc-9.6.7/cabal-install-3.15.0.0/x/cabal/build/cabal/cabal <== /tmp/bin-123966/cabal
*** Exception: Command /home/runner/work/cabal/cabal/dist-newstyle-validate-ghc-9.6.7/build/x86_64-linux/ghc-9.6.7/cabal-install-3.15.0.0/x/cabal/build/cabal/cabal init '-vverbose +markoutput +nowrap' -i failed.
Output:
Warning: this is a debug build of cabal-install with assertions enabled.
Error: [Cabal-6666]
The program 'gcc' is required but it could not be found.

*** unexpected failure for PackageTests/Init/init-without-git.test.hs

angerman avatar May 27 '25 02:05 angerman

You could fix the testsuite failure by moving the requireProgram call into the call to configureLd? I had to reason about this for a bit to work out it was going to pass "C compiler link flags" to the gccProg call.

It seems a bit dubious generally that Cabal is calling ld directly, since GHC will always perform linking by calling the C compiler and "ld command" is not defined these days. If you also start looking for something called "ld" on your path, that might not be the same one which GHC is configured to use. (See also #9301) So another way to fix this would be to stop attempting to use ld for recent GHC versions which support the --merge-objs flag. This patch does seem to move away from your idea in that ticket to "always delegate linking to GHC".

mpickering avatar May 29 '25 12:05 mpickering

Is there a ticket number corresponding to this change?

mpickering avatar May 29 '25 12:05 mpickering

Is there a ticket number corresponding to this change?

I don't believe so, although the CONTRIBUTING guidelines state:

A pull request fixes a problem that is described in an issue. Make sure to file an issue before opening a pull request.

https://github.com/haskell/cabal/blob/master/CONTRIBUTING.md#pull-requests--issues

ulysses4ever avatar May 29 '25 13:05 ulysses4ever

You could fix the testsuite failure by moving the requireProgram call into the call to configureLd? I had to reason about this for a bit to work out it was going to pass "C compiler link flags" to the gccProg call.

It seems a bit dubious generally that Cabal is calling ld directly, since GHC will always perform linking by calling the C compiler and "ld command" is not defined these days. If you also start looking for something called "ld" on your path, that might not be the same one which GHC is configured to use. (See also #9301) So another way to fix this would be to stop attempting to use ld for recent GHC versions which support the --merge-objs flag. This patch does seem to move away from your idea in that ticket to "always delegate linking to GHC".

Yes, I still think Cabal should always delegate to GHC, the behavior here also seems deeply wrong though. I'm happy fixing this and then nuking the whole logic outright afterwards (I'm just weary that will entail a lot more issues than expected).

angerman avatar Jun 05 '25 09:06 angerman

I create the issue here: #10974

angerman avatar Jun 05 '25 09:06 angerman