Skip to content

Conversation

hasufell
Copy link
Member

@hasufell hasufell commented Sep 8, 2025

No description provided.

@hasufell hasufell force-pushed the release-ci branch 19 times, most recently from 1370fdb to c61c1ef Compare September 9, 2025 13:50
@hasufell hasufell requested a review from angerman as a code owner September 10, 2025 06:37
@hasufell hasufell force-pushed the release-ci branch 4 times, most recently from c304c1c to 4d0a366 Compare September 11, 2025 08:31
Copy link

@angerman angerman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good so far. We might want to extract parts.

AS_IF(
[test "$CABAL_FLAG_libm" = 1],
[AC_DEFINE([HAVE_LIBM], [1], [Define to 1 if you need to link with libm])])
AC_SEARCH_LIBS([exp2], [m])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll want to extract this (libm) into a separate PR, which we can also upstream.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines +183 to +186
# force skip uniques if not in a git checkout
forceSkipUniquesTest = not inside_git_repo()
config.skip_uniques_test = args.skip_uniques_test or forceSkipUniquesTest

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should extract this as a separate upstreamable PR as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see a point in creating a PR for this in stable-haskell. I however opened a PR upstream: https://gitlab.haskell.org/ghc/ghc/-/merge_requests/14940


$(TOP)/ghc-config/ghc-config : $(TOP)/ghc-config/ghc-config.hs
"$(TEST_HC)" --make -o $@ $<
"$(TEST_HC)" $(EXTRA_HC_OPTS) --make -o $@ $<

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fundamentally correct. I'm still a bit surprised this wasn't already part of the test-suite?

@hsyl20 any thoughts on this?


$(TOP)/ghc-config/ghc-config : $(TOP)/ghc-config/ghc-config.hs
"$(TEST_HC)" --make -o $@ $<
"$(TEST_HC)" $(EXTRA_HC_OPTS) --make -o $@ $<

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd want to extract this EXTRA_HC_OPTS part into a separate PR as well for upstreaming.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

findMergeObjs :: ProgOpt -> Cc -> CcLink -> Nm -> M MergeObjs
findMergeObjs progOpt cc ccLink nm = checking "for linker for merging objects" $ do
prog <- findProgram "linker for merging objects" progOpt ["ld.gold", "ld"]
prog <- findProgram "linker for merging objects" progOpt ["ld"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should ultimately also end up being a seperate PR for upstreaming.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines 131 to 143
if os(freebsd)
package rts
ghc-options: "-optc-DProjectVersion=\"914\""
ghc-options: "-optc-DRtsWay=\"v\""
ghc-options: "-optc-DHostPlatform=\"x86_64-portbld-freebsd\""
ghc-options: "-optc-DHostArch=\"x86_64\""
ghc-options: "-optc-DHostOS=\"freebsd\""
ghc-options: "-optc-DHostVendor=\"unknown\""
ghc-options: "-optc-DBuildPlatform=\"FIXME\""
ghc-options: "-optc-DBuildArch=\"FIXME\""
ghc-options: "-optc-DBuildOS=\"FIXME\""
ghc-options: "-optc-DBuildVendor=\"FIXME\""
ghc-options: "-optc-DGhcUnregisterised=\"FIXME\""
ghc-options: "-optc-DTablesNextToCode=\"FIXME\""
ghc-options: "-optc-DFS_NAMESPACE=rts"
flags: +tables-next-to-code

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should be extracted into a separate PR: "FreeBSD support".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't see why.

Comment on lines 102 to 104
CABAL ?= cabal
SED ?= sed

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These seem sensible. We should probably extract this change? Seems already like a PR we could merge?

@hasufell hasufell force-pushed the release-ci branch 2 times, most recently from 33a48ef to 79c7344 Compare October 7, 2025 12:02
@hasufell
Copy link
Member Author

hasufell commented Oct 7, 2025

Fixes #26434

In detail, this does a number of things:
* Makes GHC aware of 'extra-libraries-static' (this changes the package
  database format).
* Adds a switch '-static-external' that will honour 'extra-libraries-static'
  to link external system dependencies statically.
* Adds a new field to settings/targets: "ld supports verbatim namespace".
  This field is used by '-static-external' to conditionally use '-l:foo.a'
  syntax during linking, which is more robust than trying to find the
  absolute path to an archive on our own.
* Adds a switch '-fully-static' that is meant as a high-level interface
  for e.g. cabal. This also honours 'extra-libraries-static'.

This also attempts to clean up the confusion around library search directories.
At the moment, we have 3 types of directories in the package database
format:
* library-dirs
* library-dirs-static
* dynamic-library-dirs

However, we only have two types of linking: dynamic or static. Given the
existing logic in 'mungeDynLibFields', this patch assumes that
'library-dirs' is really just nothing but a fallback and always
prefers the more specific variants if they exist and are non-empty.

Conceptually, we should be ok with even just one search dirs variant.
Haskell libraries are named differently depending on whether they're
static or dynamic, so GHC can conveniently pick the right one depending
on the linking needs. That means we don't really need to play tricks
with search paths to convince the compiler to do linking as we want it.
For system C libraries, the convention has been anyway to place static and
dynamic libs next to each other, so we need to deal with that issue
anyway and it is outside of our control. But this is out of the scope
of this patch.

This patch is backwards compatible with cabal. Cabal should however
be patched to use the new '-fully-static' switch.
"Executable" seems more appropriate.
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.

2 participants