Use linker capability detection to improve linker use #9443
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
erikd
changed the title
erikd
requested review from
geekosaur,
Kleidukos,
angerman and
andreabedini
and removed request for
geekosaur and
Kleidukos
erikd
force-pushed
the
erikd/relocatable-flag-2
branch
from
1e3366c to
d424287
Compare
4 tasks
erikd
force-pushed
the
erikd/relocatable-flag-2
branch
2 times, most recently
from
1180003 to
e66f226
Compare
angerman
approved these changes
Nov 15, 2023
geekosaur
requested changes
Nov 15, 2023
erikd
force-pushed
the
erikd/relocatable-flag-2
branch
2 times, most recently
from
609049b to
b7810b9
Compare
|
@geekosaur Changelog entry added. |
geekosaur
approved these changes
Nov 15, 2023
erikd
force-pushed
the
erikd/relocatable-flag-2
branch
2 times, most recently
from
eec42d8 to
1e3528a
Compare
Comment on lines
771
to
779
| let linkerSupportsGhciLibs :: Bool | ||
| linkerSupportsGhciLibs = | ||
| case lookupProgramByName "ld" programDb'' of | ||
| Nothing -> True -- NOTE: This may still fail if the linker does not support -r. | ||
| Just ld -> | ||
| case Map.lookup "Supports relocatable output" $ programProperties ld of | ||
| Just "YES" -> True | ||
| Just "NO" -> False | ||
| _other -> True -- NOTE: This may still fail if the linker does not support -r. |
There was a problem hiding this comment.
Sorry to be a PITA 😂
What this function does is checking whether we had previously detected that linker does not support -r. Am I correct? We use this information to ignore enable-library-for-ghci further down. I think this should be more explicit.
How about we
- Rename the function to not mention ghci but to mention the linker property (relocatable outputs? is it always
-r?) - Change the function to return Maybe Bool (as in Yes/No/I don't know).
- Delegate the
fromMaybe TruetowithGHCiLib_
Does this sound fair?
erikd
force-pushed
the
erikd/relocatable-flag-2
branch
from
1e3528a to
9d8b74b
Compare
mergify
bot
added
the
merge delay passed
Standard GNU `ld` ues `--relocatable` while `ld.gold` uses a `-relocatable` flag (with a single `-`). Code will now detect both versions.
`ldProgram` gets configured in two places, a seemingly default and a GHC specific version. The later needs to be updated so that it first calls the default configuration and then the new GHC version.
The function `comperSupportsGhciLibs` has been renamed to `linkerSupportsGhciLibs` because its about the linker not the compiler. The function `comperSupportsGhciLibs` was using the compiler version as a proxy for whether the linker supports relocatable objects. Now support for relocatable objects is detected by running the linker.
Mikolaj
force-pushed
the
erikd/relocatable-flag-2
branch
from
9d8b74b to
53fc3d3
Compare
|
🎉 🎉 🎉 |
|
@Mikolaj @Kleidukos could this possibly be backported to 3.10.3.0? |
|
@Kleidukos calls the shots re 3.10.3.0, but from what I know, it's 5 past midnight, so the PR would need to be backported by somebody else, which involves solving formatting conflicts. Is there a volunteer? It may be best to apply the changes afresh instead of backporting, but let me try an automatic backport just to confirm the conflicts. |
|
@mergify backport 3.10 |
✅ Backports have been created
|
Kleidukos
added a commit
to Kleidukos/cabal
that referenced
this pull request
Mar 11, 2024
Kleidukos
added a commit
to Kleidukos/cabal
that referenced
this pull request
Mar 11, 2024
Kleidukos
added a commit
to Kleidukos/cabal
that referenced
this pull request
Mar 11, 2024
Kleidukos
added a commit
that referenced
this pull request
Mar 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Cabal currently adds the
-r(relocatable) flag to some invocations ofldProgram, butlld(commonly used on WIndows) does not understand the-rflag. This PR just detects ifldProgramaccepts-rduring theconfigurestep.Hopefully this far simpler implementation will replace #9270