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

Cabal 3.8 expects pkg-config --libs --static to always work. #8455

Open
hamishmack opened this issue Sep 5, 2022 · 29 comments
Open

Cabal 3.8 expects pkg-config --libs --static to always work. #8455

hamishmack opened this issue Sep 5, 2022 · 29 comments

Comments

@hamishmack
Copy link
Collaborator

See bf32602 and input-output-hk/haskell.nix#1642

I think it is unreasonable for cabal to expect pkg-config --libs --static to work when only dynamic libraries may be installed on a system.

To Reproduce

$ cabal --version
cabal-install version 3.9
compiled using version 3.9.0.0 of the Cabal library
$ ghc --version
The Glorious Glasgow Haskell Compilation System, version 8.10.7
$ cabal v2-install haskell-gi-base -v3
...

In setup configure for haskell-gi-base you will see it has both:

Running: SOME_PATH/pkg-config --libs gobject-2.0 glib-2.0
Running: SOME_PATH/pkg-config --libs --static gobject-2.0 glib-2.0

On nix you can run something like this to see the second one fail:

nix-shell -E 'let pkgs = import <nixpkgs> {}; in pkgs.mkShell { buildInputs = [ pkgs.gtk4.dev pkgs.pkgconfig ]; }' --run 'cabal v2-install haskell-gi-base -v3'

Expected behavior
Perhaps cabal could store the error and replay it when the linker arguments are actually needed.

@Mikolaj
Copy link
Member

Mikolaj commented Sep 6, 2022

@hamishmack: thank you for the report. Did it work fine with cabal 3.6.2 (I assume it's failing with 3.8.1?)?

@hamishmack
Copy link
Collaborator Author

Yes, 3.6.2 worked fine (I think because it does not include bf32602)

hamishmack added a commit to input-output-hk/haskell.nix that referenced this issue Sep 13, 2022
A bug was fixed in cabal 3.8 (haskell/cabal#6771) that haskell.nix relied on for `pkg-config` in support.  To work around this we added a dummy `pkg-config` that `cabal configure` now uses when haskell.nix runs it `cabal configure` internally.  That returns version information for all the `pkg-config` packages in `lib/pkgconfig-nixpkgs-map.nix`.  This mapping has also been expanded based on the results of searching nixpkgs for `*.pc` files.

This update also includes workarounds for:

haskell/cabal#8352
haskell/cabal#8370
haskell/cabal#8455
@gbaz
Copy link
Collaborator

gbaz commented Sep 24, 2022

associated ticket for the original change: #7536

cc @nh2 @alexbiehl

@nh2
Copy link
Member

nh2 commented Sep 24, 2022

The issue description doesn't say what the error message is. Is it like this?

% nix-shell -E 'let pkgs = import <nixpkgs> {}; in pkgs.mkShell { buildInputs = with pkgs; [ gtk4.dev pkgconfig haskellPackages.cabal-install ghc ]; }' --run 'pkg-config --libs --static gobject-2.0 glib-2.0'

Package libpcre was not found in the pkg-config search path.
Perhaps you should add the directory containing `libpcre.pc'
to the PKG_CONFIG_PATH environment variable
Package 'libpcre', required by 'glib-2.0', not found

An alternative to

Perhaps cabal could store the error and replay it when the linker arguments are actually needed.

would be to do what I did before this commit, conditionally passing the flag on configure with ["--static" | fullyStatic]:

bf32602#diff-fff9e57c2727835086925d886619deb3fafd33a06e1af760d60d485ae07cec68L1647

Edit: This alternative cannot work, see #8455 (comment)

@sternenseemann
Copy link

I can reproduce this as well and it is probably going to break a lot of packages in nixpkgs with no nice way of fixing them. When we have a Haskell package pkg, that depends on liba which in turn depends on libb, we'd just place liba's pc files in PKG_CONFIG_PATH. This means that --libs --static will fail, since pkg-config would lookup liba's (private) dependencies for this (in the dynamic case this is unnecessary thanks to dynamic linking).

When we actually link the (system) libs statically, we propagate liba's dependencies, so that they'd be available. We don't do it in the dynamic case, because it is not necessary.

Accomodating Cabal here is hard, since it pertains how packaging of non-Haskell packages is done, so I'd prefer it if we could get rid of this requirement.

@Mikolaj
Copy link
Member

Mikolaj commented Oct 31, 2022

@hamishmack: could you respond to #8455 (comment)? Thank you.

@sternenseemann: I lack context, so I'd need more explanation of your proposal. Could you elaborate on "if we could get rid of this requirement"? What requirement? In which code the change would be done?

@sternenseemann
Copy link

sternenseemann commented Jan 25, 2023

@sternenseemann: I lack context, so I'd need more explanation of your proposal. Could you elaborate on "if we could get rid of this requirement"? What requirement? In which code the change would be done?

The requirement in Cabal, i.e. that it runs pkg-config --libs --static even if nothing will be linked statically against those libraries. I can't really comment on the change I propose, because I don't know why this was introduced in Cabal in the first place.

The problem for nixpkgs is that from the cabal file we only know the libraries the package depends on directly and we want to avoid blindly adding all packages that package depends on to PKG_CONFIG_PATH to avoid packaging errors like accidentally introduced dependencies.

As said, the propagation of the transitive dependency closure is only necessary when we are actually producing fully statically linked executables (when we do, we propagate), but normally our executables are linked statically only against Haskell libs and dynamically against system libraries (including pkgconfig-depends) – which is the default I think.

@sternenseemann
Copy link

Also affects Cabal-3.9.0.0 as shipped by GHC 9.6.1-alpha2.

@gbaz
Copy link
Collaborator

gbaz commented Feb 4, 2023

I think we should follow the suggestion from @nh2 and only conditionally pass the static flag when the build is requested to be static. After inspecting the code for a bit, I think there is no downside here.

@Mikolaj
Copy link
Member

Mikolaj commented Feb 9, 2023

Let's do that.

@Mikolaj
Copy link
Member

Mikolaj commented Feb 9, 2023

@nh2: could you perhaps revert that one bit? That would be very helpful and we'd have it in time for cabal 3.10.

@Mikolaj
Copy link
Member

Mikolaj commented Feb 14, 2023

@nh2: ping?

@nh2
Copy link
Member

nh2 commented Feb 15, 2023

Hey,

I'm currently on business travels for the next 2.5 weeks, so I won't have time to look into that / test it thoroughly until after that.

@Mikolaj
Copy link
Member

Mikolaj commented Feb 25, 2023

@nh2: thank you for letting us know. Lots of success in your travels!

I'm afraid we can't wait with cabal 3.10.1.0 any longer, but we'd gladly incorporate a fix for the regression into 3.10.2.0, if that aligns with your plans.

@Mikolaj
Copy link
Member

Mikolaj commented Mar 22, 2023

@nh2; gentle ping?

@andreasabel andreasabel added the re: pkg-config Concerning pkg-config and pkgconfig-depends constraints label May 4, 2023
@Mikolaj
Copy link
Member

Mikolaj commented May 18, 2023

@nh2; gentle ping again?

@Mikolaj
Copy link
Member

Mikolaj commented Jun 17, 2023

@nh2; gentle ping again?

@nh2
Copy link
Member

nh2 commented Jul 12, 2023

I have looked into this now.

Unfortunately my suggestion to re-introduce ["--static" | fullyStatic] is wrong.

The way @alexbiehl did it with

      ldflags <- pkgconfig ("--libs"   : pkgs)
      ldflags_static <- pkgconfig ("--libs"   : "--static" : pkgs)

is generally right:

The way this feature works is (added docs):

Added fields extra-libraries-static and extra-lib-dirs-static to allow Haskell libraries to remember linker flags needed for fully static linking of system libraries into executables.
The existing field pkgconfig-depends can used to append the relevant output of pkg-config --libs --static to these new fields automatically.

This means that at the time a Haskell library gets compiled, pkg-config packages get turned into .a files.

pkg-config --static is NOT invoked when the final Haskell executable is built (this would require remembering pkgconfig-depends pkg-config package names across Haskell libraries, instead of library .a names which are remembered in extra-libraries-static).

That implies that pkg-config --static must be invoked unconditionally when building a library. Conditioning it on fullyStatic (which is turned on by --enable-executable-static does not make sense, because that flag is never on when building a library.)

Concrete example

Consider the brotli Haskell library.

  • Its .cabal file specifies pkgconfig-depends: libbrotlienc, libbrotlidec.
  • When it is built, pkg-config --libs --static libbrotlidec libbrotlienc is invoked.
    This outputs
    -lbrotlidec -lbrotlienc -lbrotlicommon
    
    Note the appearance of -lbrotlicommon, which is a pkg-config dependency needed for static linking.
  • brotlicommon must be added to extra-libraries-static, otherwise statically building a Haskell executable that has build-depends: brotli will fail with a linker error.

@nh2
Copy link
Member

nh2 commented Jul 12, 2023

Answering #8455 (comment):

I don't know why this was introduced in Cabal in the first place.

@sternenseemann The above should answer that question. At the time you build a Haskell library, you do not know whether that library will eventually be used for building a dynamic executable, a static executable, or maybe even both static and dynamic executables.

One could argue that for many Nix use cases, this can be known / propagated through since in Nix you specify your top-level goal (e.g. pkgsStatic.haskellPackages.darcs). But that is not the case for non-Nix use cases, which build bottom-up instead of having top-down information propagation (for example if you are building in Alpine Linux or its Docker container).

@nh2
Copy link
Member

nh2 commented Jul 13, 2023

Solution approaches

  • Allow errors in pkg-config --libs --static; in the error case, keep extra-libraries-static empty.
    • Pro: Would fix the errors for normal users wo don't want static linking.
    • Pro: Would fix the issue for Nix, because there you'd see the eventual static executable's linker error, change your derivations to make sure that the pkg-config dependencies are propagates all the way down, and rebuild.
      • Con: Rebuild quite a lot, because you notice the error only at the end of the dependency chain.
    • Con: Makes a bad UX for non-Nix users (generally users with non-hermetic build environments) that want to build statically:
      • They build a Haskell library such as brotli, it does not find the static library names, the library gets installed anyway.
      • The linker error appears.
      • The user thinks: "Ah, I just have to e.g. apt install libbrotli-dev to get the .a files", does that.
      • The linker error keeps appearing, because the Haskell library is not rebuilt when system libraries change, and has "remembered" the state the system was in when it was installed.
      • The user has to know to issue cabal install --reinstall brotli to fix it.
  • Remember/Propagate pkgconfig-depends across Haskell packages instead of extra-libraries-static library names.
    Better: When building a static Haskell executable with --enable-executable-static would just construct a pkg-config --libs --static invocation to which as arguments are given all the pkgconfig-depends of all recursive Haskell dependency libraries.

For anybody who wants to give a shot at the pkgconfig-depends solution, I should point out that the fields added in #7536 are still valuable, as extra-libraries-static still has use cases in the same way as extra-libraries does for system libraries that do not provide pkg-config .pc files.

@nh2
Copy link
Member

nh2 commented Jul 13, 2023

I have edited the above message for lining out a simpler approach.

@gbaz
Copy link
Collaborator

gbaz commented Jul 21, 2023

@sternenseemann I see from the above linked ticket that you're open to patching cabal. What do you think about working on the "simpler" approach outlined by @nh2 ?

@sternenseemann
Copy link

@gbaz The cons nh2 outlines also affect us, i.e. interactive usage of cabal-install with a bunch of haskell libraries in a nix-shell are a non-hermetic environment. Those edge cases are probably rare, but I'd opt for a patch that is upstreamable in any case.

I am not sure that the easy fix would be so quick to figure out and test that it'd be worth investigating just for the sake of Nix.

@gbaz
Copy link
Collaborator

gbaz commented Jul 25, 2023

@sternenseemann sorry, to be clear, I don't mean the "easy" approach (the first of the two). I mean the "better" but also "simpler" approach which is the edited version of the second approach: "When building a static Haskell executable with --enable-executable-static would just construct a pkg-config --libs --static invocation to which as arguments are given all the pkgconfig-depends of all recursive Haskell dependency libraries. "

@sternenseemann
Copy link

@gbaz That would work for us. pkg-config --libs --static should definitely work in the context of the executable we are trying to link statically (be it at link time or when configuring the executable's package).

@gbaz
Copy link
Collaborator

gbaz commented Jul 25, 2023

Great, do you think you would be interested in picking it up?

AndersonTorres pushed a commit to NixOS/nixpkgs that referenced this issue Aug 17, 2023
Cabal 3.8 has the same requirements as pkgsStatic even when linking
dynamically, so this patch will be useful for compiling
haskellPackages.libarchive.

haskell/cabal#8455
@Mikolaj
Copy link
Member

Mikolaj commented Nov 4, 2023

How are things going? Is this still a valid approach? If so, would anybody like to get it set up? Thank you!

@nh2
Copy link
Member

nh2 commented Nov 6, 2023

I am currently swamped with tasks, so it is unlikely for me to be able to pick it up this year.

@Mikolaj
Copy link
Member

Mikolaj commented Apr 8, 2024

@nh2: Hi, how are you? Maybe this year? :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants