Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pkg-config: Use --libs --static when building with using --enable-executable-static #6935

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

alexbiehl
Copy link
Member

@alexbiehl alexbiehl commented Jun 30, 2020

When --enable-executable-static is enabled Cabal passes --static to pkg-config to obtain linker flags for static linking.


Please 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 (add file to changelog.d directory).
  • The documentation has been updated, if necessary.

Please also shortly describe how you tested your change. Bonus points for added tests!

@alexbiehl alexbiehl force-pushed the alex/pkg-config-static branch 2 times, most recently from 196eed4 to 5263473 Compare June 30, 2020 10:47
@alexbiehl alexbiehl requested review from nh2 and phadej June 30, 2020 10:53
@alexbiehl alexbiehl force-pushed the alex/pkg-config-static branch from 5263473 to 5969a91 Compare June 30, 2020 10:56
Copy link
Collaborator

@phadej phadej left a comment

Choose a reason for hiding this comment

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

This needs a test, or at the very least an issue which describes what this fixes (i.e. some test plan).

Cabal/ChangeLog.md Outdated Show resolved Hide resolved
@alexbiehl alexbiehl force-pushed the alex/pkg-config-static branch from 5969a91 to 23a4daa Compare June 30, 2020 11:48
@phadej
Copy link
Collaborator

phadej commented Jul 7, 2020

@nh2 does this look good? If you say so, I'll merge this without requiring a test case

@nh2
Copy link
Member

nh2 commented Jul 7, 2020

The motion is great, and I want and need this, but I think what's in this PR is not enough to make it work well.

I already did most of this work in the past, please read nh2/static-haskell-nix#62.

Key important part:

At NixCon I found that this requires adding a new field to ghc-pkg's .conf files, like extra-libraries-static, because libraries can't know at the time they invoke pkg-config whether they will eventually be linked statically or not, so we must carry along both sets of flags.

This requires Cabal changes (which I provide in 2 branches, linked from there), and a GHC change (which I got blocked on because of Hadrian bugs).

I think the right way to do this is to fix Hadrian (I already provided a WIP GHC PR, linked, but then ran out of time for now), and then do the GHC side and rebase the Cabal patches.

I'd be happy to advise on those topics if you'd like to do it @alexbiehl because i'm currently too busy to finish it myself.

@alexbiehl
Copy link
Member Author

Great! I lam willing to take this on. It will take a while as I am on vacations though. So feel free to take this up again in the meantime. I will leave a note when I start working on it actively.

@gbaz
Copy link
Collaborator

gbaz commented Aug 11, 2021

@alexbiehl any plans to return to this?

@alexbiehl
Copy link
Member Author

Hi! Thanks for the gentle reminder. I lost track of this but static linking is still near and dear to my heart and would love to get this done. I have put this on my backlog for tue weekend to evaluate and see what’s left to do.

@Mikolaj
Copy link
Member

Mikolaj commented Aug 11, 2021

Oh, great news. In case it matters, here's a probably superficially related newer issue not yet mentioned in pingbacks above: #7236,

@mergify mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Sep 1, 2022
@ulysses4ever ulysses4ever removed the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Sep 3, 2022
@fgaz fgaz added the conflicts label Nov 23, 2022
@andreasabel andreasabel added the re: pkg-config Concerning pkg-config and pkgconfig-depends constraints label May 4, 2023
@Kleidukos
Copy link
Member

@alexbiehl would you like my help with fixing the conflicts of your PR?

@alexbiehl
Copy link
Member Author

@Kleidukos sure! Feel free to to resolve and push!

@Kleidukos Kleidukos self-assigned this Jun 2, 2023
@tchoutri tchoutri force-pushed the alex/pkg-config-static branch from 23a4daa to 52811c0 Compare June 12, 2023 15:06
@Kleidukos Kleidukos force-pushed the alex/pkg-config-static branch 2 times, most recently from 6b3621b to 1adc67a Compare June 12, 2023 15:10
@Kleidukos Kleidukos requested a review from Mikolaj June 13, 2023 16:47
@Kleidukos Kleidukos force-pushed the alex/pkg-config-static branch from 1adc67a to 39ff0f1 Compare June 13, 2023 16:49
@Mikolaj
Copy link
Member

Mikolaj commented Jun 20, 2023

@Kleidukos: I have an impression the actual change got literally lost while formatting. If I'm mistaken, Could you point out what the original change has been? Ideally, it'd be in a commit separate from the formatting changes.

@ulysses4ever
Copy link
Collaborator

I remove needs-review for now because it currently needs some love (see Mikolaj's comment above)..

@Mikolaj Mikolaj removed their request for review August 28, 2023 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
attention: needs-help Help wanted with this issue/PR re: pkg-config Concerning pkg-config and pkgconfig-depends constraints
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants