Merged
Conversation
github-actions
bot
added
6.topic: python
wolfgangwalther
force-pushed
the
postgresql-libpq
branch
from
d97ea3c to
ca6dc81
Compare
nix-owners
bot
requested review from
K900,
Ma27,
NickCao,
RaghavSood,
SuperSandro2000,
aanderse,
drupol,
figsoda,
globin,
infinisil,
jtojnar,
mmahut,
philiptaron,
talyz and
thoughtpolice
ofborg
bot
added
the
2.status: merge conflict
wolfgangwalther
force-pushed
the
postgresql-libpq
branch
2 times, most recently
from
1eb5366 to
862bd30
Compare
wolfgangwalther
removed
the
2.status: merge conflict
wolfgangwalther
force-pushed
the
postgresql-libpq
branch
from
862bd30 to
10eca45
Compare
Member
|
Apologies, misclicked 😅 |
Contributor
Author
Dependencies come into play whenever we need to (re-)build. For us the most relevant case is the one inside nixpkgs - where we discuss how to speed up merging to master etc. However, there's more: Yes, postgresql builds in pkgsStatic now. But it still carries more dependencies. All of which might need to be rebuild when you're trying to actually cross-compile libpq. The whole cross-compilation thing was brought up repeatedly, e.g. in #61580 (comment).
Essentially most of the discussion in #61580 is exactly about this. I'd summarize this as: many are for a split, few are for no split. Although, to be fair those numbers are certainly without considering the facts that the outputs are now split and pkgsStatic actually builds.... ... that is, it builds on Linux. One example why Try building * which in itself is quite a 🥳 moment for me! |
Resolves NixOS#61580
wolfgangwalther
force-pushed
the
postgresql-libpq
branch
from
448a13d to
546ece5
Compare
Contributor
Author
I squashed that into the main libpq commit. |
Contributor
Author
Addition: The same happens for me on |
Contributor
Author
So was I, when I started my comment above. Finding out about darwin and freebsd and the actual practical consequences, I convinced myself. Thus: Merging! |
Member
|
After sleeping one night over it and reading your comment, I also agree (just ftr). Thanks a lot for your work here! |
Contributor
|
Bisect says 546ece5 |
Contributor
Author
Thanks for the report. Will look into it later today. |
Member
|
The issue is that |
13 tasks
Ma27
added a commit
to Ma27/nixpkgs
that referenced
this pull request
Jan 20, 2025
The `libpq` attribute from `postgresqlVersions` doesn't have a `pkgs` attribute with extensions in it. Filtering for postgresql server packages only fixes the problem. Reported in NixOS#359659 (comment)
Contributor
|
Another eval failure is |
Contributor
Author
|
All of the postgresql nixos tests have the same pattern and should be fixed in #375292. Just merged that - can you check again? |
Contributor
|
Yeah, it's all fixed now. I probably picked a patch too early before it was applied to all places. Thank you! |
Member
|
I have a downstream package (code) that uses
|
Contributor
Author
|
Seems like the We'll essentially need the same as for I originally intended to not ship pg_config in libpq at all and rely entirely on pkg-config instead. But that didn't work out well, too many packages needed pg_config. Odd, that they still worked, given that it returns the wrong LIBDIR.
Ultimately, this means that in your case pkg-config seems not to be used, but pg_config is. I found https://github.com/sgrif/pq-sys?tab=readme-ov-file#building, which mentions that |
Member
Well it does since the rust libs aren't dynamic, so they don't actually need to link the library against the system library. The |
Contributor
Author
|
Ok. I'll prepare a PR to fix pg_config in libpq. |
Member
|
Ah, the problem is that the use of |
13 tasks
Member
|
Another option would be to set one of the environment variables via defaultCrateOverrides. |
3 tasks
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
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.
This creates a separate libpq package as discussed in #61580 (comment).
This brings down the number of rebuilds after updating
postgresqlto about 1.2k darwin and 2.5k linux. Before, they were ~ around 5k, IIRC.Unfortunately, we are still not anywhere low enough to be able to merge
postgresqlupdates directly into master. I think the nr. 1 reason for that ispostgresqlTestHook- and the fact that many packages use a very simple postgresql server in their build dependencies for the check phase.The only way to get the number of rebuilds lower would be to package
postgresqlTestHookas a separate postgresql derivation. The idea would be, that security related issues are irrelevant for the check phase of other packages - and thus updating this separately could easily go through staging with a certain delay. This derivation could also be one that is built with the minimal feature set, making it slimmer overall.I still think that introducing
libpqis a good thing on it's own, thus this PR.Closes #61580 and #191920
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.