cabal icon indicating copy to clipboard operation
cabal copied to clipboard

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

Open hamishmack opened this issue 2 years ago • 29 comments

See https://github.com/haskell/cabal/commit/bf32602db5770a312fe632b997ff9b7c7e4d0329 and https://github.com/input-output-hk/haskell.nix/issues/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.

hamishmack avatar Sep 05 '22 09:09 hamishmack

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

Mikolaj avatar Sep 06 '22 08:09 Mikolaj

Yes, 3.6.2 worked fine (I think because it does not include https://github.com/haskell/cabal/commit/bf32602db5770a312fe632b997ff9b7c7e4d0329)

hamishmack avatar Sep 11 '22 00:09 hamishmack

associated ticket for the original change: https://github.com/haskell/cabal/pull/7536

cc @nh2 @alexbiehl

gbaz avatar Sep 24 '22 19:09 gbaz

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]:

https://github.com/haskell/cabal/pull/7536/commits/bf32602db5770a312fe632b997ff9b7c7e4d0329#diff-fff9e57c2727835086925d886619deb3fafd33a06e1af760d60d485ae07cec68L1647

Edit: This alternative cannot work, see https://github.com/haskell/cabal/issues/8455#issuecomment-1633349219

nh2 avatar Sep 24 '22 19:09 nh2

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.

sternenseemann avatar Oct 09 '22 14:10 sternenseemann

@hamishmack: could you respond to https://github.com/haskell/cabal/issues/8455#issuecomment-1257052047? 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?

Mikolaj avatar Oct 31 '22 14:10 Mikolaj

@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 avatar Jan 25 '23 15:01 sternenseemann

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

sternenseemann avatar Feb 03 '23 21:02 sternenseemann

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.

gbaz avatar Feb 04 '23 01:02 gbaz

Let's do that.

Mikolaj avatar Feb 09 '23 17:02 Mikolaj

@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 avatar Feb 09 '23 18:02 Mikolaj

@nh2: ping?

Mikolaj avatar Feb 14 '23 19:02 Mikolaj

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.

nh2 avatar Feb 15 '23 06:02 nh2

@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 avatar Feb 25 '23 20:02 Mikolaj

@nh2; gentle ping?

Mikolaj avatar Mar 22 '23 10:03 Mikolaj

@nh2; gentle ping again?

Mikolaj avatar May 18 '23 17:05 Mikolaj

@nh2; gentle ping again?

Mikolaj avatar Jun 17 '23 13:06 Mikolaj

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 avatar Jul 12 '23 23:07 nh2

Answering https://github.com/haskell/cabal/issues/8455#issuecomment-1272560459:

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 avatar Jul 12 '23 23:07 nh2

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.
    • Pro: Fixes all "Con"s of the above approach.
    • ~~Con: The fix will need to be the size of PR #7536. Somebody needs to do that grind.~~ It should just be a pkg-config invocation at the very end.

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 avatar Jul 13 '23 00:07 nh2

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

nh2 avatar Jul 13 '23 00:07 nh2

@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 ?

gbaz avatar Jul 21 '23 17:07 gbaz

@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.

sternenseemann avatar Jul 25 '23 00:07 sternenseemann

@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. "

gbaz avatar Jul 25 '23 00:07 gbaz

@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).

sternenseemann avatar Jul 25 '23 10:07 sternenseemann

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

gbaz avatar Jul 25 '23 23:07 gbaz

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

Mikolaj avatar Nov 04 '23 08:11 Mikolaj

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

nh2 avatar Nov 06 '23 13:11 nh2

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

Mikolaj avatar Apr 08 '24 08:04 Mikolaj