build pkgconfig db individually when bulk fails #8496

gbaz · 2022-09-28T00:58:48Z

Resolves #8494

We need to check for versions of packages in the pkg-config db, since we have constraints on versions. We also want to be able to use the pkgdb purely, which means we can't push checking for versions to the call-site where we know which packages are required. Further we don't know which packages we need to query until midway in the solver (since that depends on which deps we pick), so we can't first winnow down the list.

So instead I just added a fallback that says if there's a failure to build the versions en masse they get queried one by one. Slower, but reliable.

Any changes that could be relevant to users have been recorded in the changelog.

arrowd · 2022-10-04T08:37:43Z

cabal-install-solver/src/Distribution/Solver/Types/PkgConfigDb.hs

+        case exitCode of
+          ExitSuccess -> (return . pkgConfigDbFromList . zip pkgNames) (lines pkgVersions)
+          -- if there's a single broken pc file the above fails, so we fall back into calling it individually
+          _ -> pkgConfigDbFromList . catMaybes <$> mapM (getIndividualVersion pkgConfig) pkgNames


Why not just always use this code path? Is it that much slower than getting all packages in one call?

Depends on the size of the database :-)

cabal-install-solver/src/Distribution/Solver/Types/PkgConfigDb.hs

andreasabel · 2022-10-06T18:18:21Z

@gbaz : Not sure how to test this PR without too much effort, because I only experience the pkg-config issue on GitHub CI, e.g. here: https://github.com/andreasabel/text-icu/actions/runs/3199349841/jobs/5225002013:

Due to using haskell/actions/setup@v2 (rather than v1), cabal-install-3.8.1.0 is used (over 3.6.2.0).

The PKG_CONFIG_PATH is correctly set, and pkg-config can deliver a version for icu-i18n.

Run export PKG_CONFIG_PATH=/usr/local/opt/icu4c/lib/pkgconfig
$ ls -l /usr/local/opt/icu4c/lib/pkgconfig
total 24
-r--r--r--  1 runner  wheel  1270 Oct  3 00:50 icu-i18n.pc
-r--r--r--  1 runner  wheel  1262 Oct  3 00:50 icu-io.pc
-r--r--r--  1 runner  wheel  1294 Oct  3 00:50 icu-uc.pc
$ pkg-config --modversion icu-i18n
71.1

cabal-install-3.8.1.0 does not succeed to get the version of icu-i18n and thus fails to build text-icu-0.8.0.2:

Error: cabal: Could not resolve dependencies:
[__0] next goal: text-icu (user goal)
[__0] rejecting: text-icu-0.8.0.2 
(conflict: pkg-config package icu-i18n>=62.1, not found in the pkg-config database)

It seems that text-icu-0.8.0.2 (latest version) does not build on the macOS Virtual Environments with haskell/actions/setup@v2 even if PKG_CONFIG_PATH is correctly set, due to cabal 3.8.1.0. (Note however, that the macOS-* Virtual Environments themselves still ship with cabal 3.6.2.0, so maybe the problem hasn't reached full escalation yet.)

However, I cannot reproduce the problem on my own Mac.

I think it is good if this PR is tested on a macOS Virtual Environment to make sure it fixes the problem I describe, and if successful, a patch should be released as cabal-install-3.8.1.1 (or suitable). Of course, the problem is less pressing if there is a feasible workaround, is there one?

gbaz · 2022-10-06T19:08:36Z

The issue you'll see is if any package in pkg-config --list-all fails in its modversion call on this env. Do you have a way to check that?

Alternately, do you have a way to run your test using a branch of cabal-install?

gbaz · 2022-10-06T19:14:49Z

With regards to a workaround:

I note btw that text-icu with its current cabal file will always fail if there is no pkg-config present on the system, or if icu-uc, or icu-i18n are not registered in it.

It would be better to have an auto flag that switches between a build using pkgconfig-depends and a build explicitly enumerating extra-libraries. This is what the pr that started this all was intended to enable :-) #7621

In such a case if the CI busted on pkgconfig-depends, then it will fallback to the explicit extra-libraries etc.

This also makes the package somewhat more resilient for end-users.

andreasabel · 2022-10-06T20:12:25Z

@gbaz:

The issue you'll see is if any package in pkg-config --list-all fails in its modversion call on this env. Do you have a way to check that?

I could port your Haskell code to shell land and add it to my CI script. Should be doable (just my shell skills are always rusty or rather chronically underdeveloped...). Maybe awk is more suited than bash to iterating over the output lines of pkg-config --list-all...

gbaz · 2022-10-06T20:44:04Z

functional programmers do it with xargs

(to be honest i had to google to remember the details)

pkg-config --list-all | cut -f 1 -d ' ' | xargs pkg-config --modversion

andreasabel · 2022-10-07T04:32:42Z

Gotcha: pkg-config --list-all segfaults of virtual environments macOS-11 and macOS-12.

pkg-config --list-all segfaults on macOS-{11,12} actions/runner-images#6364

Unfortunately, this means that cabal 3.8.1.0 is silently broken for pkg-config things on these platforms.

Such (upstream) blunder also suggests we should have some test involving thepkg-config feature in our CI.

P.S.: --list-all could be fragile, here is a report that one broken .pc file makes it crash:

https://gitlab.freedesktop.org/pkg-config/pkg-config/-/issues/36

andreasabel · 2022-10-07T04:53:36Z

cabal-install-solver/src/Distribution/Solver/Types/PkgConfigDb.hs

@@ -65,9 +67,14 @@ readPkgConfigDb verbosity progdb = handle ioErrorHandler $ do
        -- The output of @pkg-config --list-all@ also includes a description
        -- for each package, which we do not need.


As witnessed by

pkg-config --list-all segfaults on macOS-{11,12} actions/runner-images#6364

pkg-config might be present but pkg-config --list-all might segfault. What is the strategy in such situations? (Maybe building a pkg-config database eagerly isn't so great after all, one could fill it lazily by demand in the solver, could one?)

It looks like cabal treats an error in pkg-config --list-all similarly to the inability to find pkg-config, so the solver would only fail in the case where all potential solutions require at least one pkg-config package.

The solver is currently pure, so querying packages on demand would be a big change.

I acknowledge the problem that the solver would not be pure anymore, although it could be made almost pure by placing it into a service monad that only provides a method to resolve a package version via pkg-config (rather than placing it flatly into IO). Something like:

class Monad m => MonadPkgConfig m where pkgConfigModVersion :: PackageName -> m PackageVersion solver :: MonadPkgConfig m => Inputs -> m Output

Lazy querying pkg-config would solve the problem where pkg-config --modversion is available but not pkg-config --list-all.

There are numerous packages out there that unconditionally declare a pkgconfig-depends (not the least, text-icu), and these all break (in certain circumstances) in the current approach with pkgconfig --list-all.

Note also that the contract given to --list-all is minimal, e.g., my man pkg-config tells me:

--list-all List all modules found in the pkg-config path.

This does not specify any particular output format, something that cabal crucially relies on.

In contrast, this is the contract for --modversion:

--modversion Requests that the version information of the libraries specified on the command line be displayed. If pkg-config can find all the libraries on the command line, each library's version string is printed to stdout, one version per line. In this case pkg-config exits successfully. If one or more libraries is unknown, pkg-con- fig exits with a nonzero code, and the contents of stdout are undefined.

Looks rather more specific (thus, binding) to me.

Programming around "an external program is segfaulting" seems like its really pushing it, though I do understand a frustration with images being broken. I would again advise that text-icu put in a flag that varies depending on pkg-config being present and working or not.

Yeah, but I'd like to know whether pkg-config --list-all works at all on macOS-11/12. Unfortunately, I cannot test it directly as I my machine is still on macOS-10. Anyone with a recent macOS willing to investigate thie?

So, upstream came back with an analysis: Homebrew/homebrew-core#112785
Not that the details matter, but the upshot is that one broken package configuration breaks pkg-config --list-all and consequently cabal-3.8.1.0.
So, install imagemagick or highway and suddently your haskell packages fail to build with cabal. Nightmare scenario? Yes!
Lesson to learn: stay away from the pkg-config --list-all path and only query pkg-config for the packages you are interested in.

Another idea is to work around the segfault by not enforcing pkg-config dependencies when pkg-config exists but the --list-all command fails. Not enforcing pkg-config dependencies is the same behavior as before #7621.

I also think that we should merge this PR first, since it already handles one case where a pkg-config command fails.

I think a broken pkg-config is the same situation as a missing one -- a hard fail is fine, because the intended user pattern should be that there is an auto flag in the package that can switch to how to handle the case where pkg-config is unavailable (because non-existent or broken).

I am aware that not all packages have that flag that allows this switching. But we should encourage them to add it!

Due to a bug in the highway brew package, pkg-config --list-all segfaults on mac runners (actions/runner-images#6364). In addition, cabal probes pkg-config --list-all despite not needing it (haskell/cabal#8496). Combined, this created an issue where cabal would fail to create a build plan on macOS GitHub Action runners citing a lack of libsodium/pkg-config. The temporary solution is to reinstall the highway brew package since it has now been patched. The long-term solution is to wait for cabal to change its probing behaviour (tracked above), or wait for GitHub Actions runner environments to update so it comes with the patch preinstalled. (side note: I was pursuing this CI bug for over 4 days, unable to find anything relevant on search engines.... until 30 minutes ago, when I hit the jackpot with the right search terms and arrived at the issues above. In fact, the issue was only solved 5 hours ago, and the last comment in the thread was from 22 minutes before I arrived. I felt quite some strong adrenaline flowing through me when I realised how lucky I was today...)

arrowd · 2022-10-20T13:18:18Z

Just checked that this PR fixes the problem for me.

michaelpj · 2022-10-24T11:06:14Z

Could we log that we're taking the fallback path if we do that? Two reasons:

The user might want to know that they have some broken pkg-conf pkgs
This looks like the sort of thing where maybe the two paths could come apart in obscure circumstances - it would be nice if there was some evidence that cabal is trying something different in the log.

michaelpj · 2022-10-26T16:37:43Z

Also! Thanks for fixing this! I really appreciate that by the time I found the issue there was already a fix 🙇

gbaz · 2022-10-26T16:41:47Z

Added a log on the fallback path, as requested.

Mikolaj · 2022-10-29T14:59:05Z

@gbaz: a kind reminder about the label, if you are ready. :)

Mikolaj · 2022-11-03T09:22:35Z

@gbaz: I'd set the merge_me label for you, but I don't know if you prefer a squash or not.

andreasabel · 2022-11-03T16:44:29Z

Looks like this needs to be squashed, as intermediate commits have CI failures and some are dubbed fixup.

Mikolaj · 2022-11-07T19:21:51Z

@mergify backport 3.8

mergify · 2022-11-07T19:22:34Z

backport 3.8

✅ Backports have been created

#8579 build pkgconfig db individually when bulk fails (backport #8496) has been created for branch 3.8

* build pkgconfig db individually when bulk fails * fix typo * changelog, comments * address comments * revert unnecessary extra failure check * fix changelog Co-authored-by: Gershom Bazerman <gershom@arista.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit 338d060)

build pkgconfig db individually when bulk fails (backport #8496)

* build pkgconfig db individually when bulk fails * fix typo * changelog, comments * address comments * revert unnecessary extra failure check * fix changelog Co-authored-by: Gershom Bazerman <gershom@arista.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

build pkgconfig db individually when bulk fails

dc6e136

georgefst mentioned this pull request Sep 29, 2022

X11-xft builds under Cabal 3.6, but not Cabal 3.8 #8495

Closed

gbaz and others added 2 commits September 30, 2022 14:43

fix typo

51c15d7

Merge branch 'master' into gb/pkgconfig-fallback

57df0b3

arrowd reviewed Oct 4, 2022

View reviewed changes

grayjay approved these changes Oct 6, 2022

View reviewed changes

cabal-install-solver/src/Distribution/Solver/Types/PkgConfigDb.hs Show resolved Hide resolved

cabal-install-solver/src/Distribution/Solver/Types/PkgConfigDb.hs Outdated Show resolved Hide resolved

This was referenced Oct 6, 2022

Bump CI to GHC 9.4; various other CI bumps haskell/text-icu#78

Merged

Make pkg-config dependency more robust haskell/text-icu#79

Open

andreasabel mentioned this pull request Oct 7, 2022

pkg-config --list-all segfaults on macOS-{11,12} actions/runner-images#6364

Closed

11 tasks

andreasabel mentioned this pull request Oct 7, 2022

Alert: switching to v2 may malfunction on macOS due to cabal 3.8.1.0 haskell/actions#119

Closed

andreasabel reviewed Oct 7, 2022

View reviewed changes

changelog, comments

7cb27dd

Mikolaj mentioned this pull request Oct 24, 2022

Cabal 3.8 checks for literally all pkg-config packages in the system #8494

Closed

Mikolaj added the attention: needs-backport 3.8 label Oct 24, 2022

address comments

f0c5e82

revert unnecessary extra failure check

08bfbb6

fix changelog

1d2409f

gbaz added the attention: needs-review label Oct 26, 2022

grayjay approved these changes Oct 26, 2022

View reviewed changes

andreasabel approved these changes Oct 27, 2022

View reviewed changes

gbaz added the squash+merge me Tell Mergify Bot to squash-merge label Nov 3, 2022

mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Nov 5, 2022

Merge branch 'master' into gb/pkgconfig-fallback

c42caf8

mergify bot merged commit 338d060 into master Nov 5, 2022

mergify bot mentioned this pull request Nov 7, 2022

build pkgconfig db individually when bulk fails (backport #8496) #8579

Merged

mergify bot added a commit that referenced this pull request Nov 7, 2022

Merge pull request #8579 from haskell/mergify/bp/3.8/pr-8496

792b257

build pkgconfig db individually when bulk fails (backport #8496)

lschneiderbauer mentioned this pull request Apr 30, 2023

cabal invokes pkg-config on every installed package, so cabal run or cabal install takes 20 minutes on my system #8930

Open

andreasabel added the re: pkg-config Concerning pkg-config and pkgconfig-depends constraints label May 4, 2023

michaelpj mentioned this pull request Jul 18, 2023

Try each pkg-config query separatedly #9134

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build pkgconfig db individually when bulk fails #8496

build pkgconfig db individually when bulk fails #8496

gbaz commented Sep 28, 2022 •

edited

Loading

arrowd Oct 4, 2022

gbaz Oct 21, 2022

andreasabel commented Oct 6, 2022

gbaz commented Oct 6, 2022

gbaz commented Oct 6, 2022

andreasabel commented Oct 6, 2022

gbaz commented Oct 6, 2022

andreasabel commented Oct 7, 2022 •

edited

Loading

andreasabel Oct 7, 2022

grayjay Oct 7, 2022

andreasabel Oct 7, 2022

gbaz Oct 7, 2022

andreasabel Oct 8, 2022

andreasabel Oct 10, 2022

grayjay Oct 20, 2022

gbaz Oct 26, 2022

arrowd commented Oct 20, 2022

michaelpj commented Oct 24, 2022

michaelpj commented Oct 26, 2022

gbaz commented Oct 26, 2022

Mikolaj commented Oct 29, 2022

Mikolaj commented Nov 3, 2022 •

edited

Loading

andreasabel commented Nov 3, 2022

Mikolaj commented Nov 7, 2022

mergify bot commented Nov 7, 2022

		@@ -65,9 +67,14 @@ readPkgConfigDb verbosity progdb = handle ioErrorHandler $ do
		-- The output of @pkg-config --list-all@ also includes a description
		-- for each package, which we do not need.

build pkgconfig db individually when bulk fails #8496

build pkgconfig db individually when bulk fails #8496

Conversation

gbaz commented Sep 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andreasabel commented Oct 6, 2022

gbaz commented Oct 6, 2022

gbaz commented Oct 6, 2022

andreasabel commented Oct 6, 2022

gbaz commented Oct 6, 2022

andreasabel commented Oct 7, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arrowd commented Oct 20, 2022

michaelpj commented Oct 24, 2022

michaelpj commented Oct 26, 2022

gbaz commented Oct 26, 2022

Mikolaj commented Oct 29, 2022

Mikolaj commented Nov 3, 2022 • edited Loading

andreasabel commented Nov 3, 2022

Mikolaj commented Nov 7, 2022

mergify bot commented Nov 7, 2022

✅ Backports have been created

gbaz commented Sep 28, 2022 •

edited

Loading

andreasabel commented Oct 7, 2022 •

edited

Loading

Mikolaj commented Nov 3, 2022 •

edited

Loading