Skip to content

Commit 435f51c

Browse files
wolfgangwaltherwolfgangwalther
committed
postgresql: split dev output
This splits a dev output to make the default output not depend on any build dependencies anymore. This also avoids removing references from pgxs' Makefile this way, which should, at least theoretically, be good to build extensions via pgxs, making sure they use the same tooling. ecpg is the "embedded SQL C preprocessor", which is certainly a dev tool. Most important, for closure size anyway, is to move pg_config to the dev output, since it retains paths to all the other outputs. The only thing with references to the dev output remaining is then the postgres binary itself. It contains all the output paths, because it shows those in the pg_config system view. There is no other way than to nuke those references to avoid circular dependencies between outputs - and blowing up closure size again.
1 parent 7797728 commit 435f51c

6 files changed

Lines changed: 161 additions & 26 deletions

File tree

‎nixos/doc/manual/release-notes/rl-2411.section.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,8 @@
298298

299299
- Docker now defaults to 27.x, because version 24.x stopped receiving security updates and bug fixes after [February 1, 2024](https://github.com/moby/moby/pull/46772#discussion_r1686464084).
300300

301+
- `postgresql` was split into default and -dev outputs. To make this work without circular dependencies, the output of the `pg_config` system view has been removed. The `pg_config` binary is provided in the -dev output and still works as expected.
302+
301303
- `keycloak` was updated to version 25, which introduces new hostname related options.
302304
See [Upgrading Guide](https://www.keycloak.org/docs/25.0.1/upgrading/#migrating-to-25-0-0) for instructions.
303305

‎nixos/modules/services/databases/postgresql.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,3 +362,7 @@ postgresql.withJIT.pname
362362
```
363363

364364
evaluates to `"foobar"`.
365+
366+
## Notable differences to upstream
367+
368+
- To avoid circular dependencies between default and -dev outputs, the output of the `pg_config` system view has been removed.

‎pkgs/servers/sql/postgresql/generic.nix‎

Lines changed: 50 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ let
66
, glibc, zlib, readline, openssl, icu, lz4, zstd, systemd, libossp_uuid
77
, pkg-config, libxml2, tzdata, libkrb5, substituteAll, darwin
88
, linux-pam
9+
, removeReferencesTo
910

1011
# This is important to obtain a version of `libpq` that does not depend on systemd.
1112
, systemdSupport ? lib.meta.availableOn stdenv.hostPlatform systemd && !stdenv.hostPlatform.isStatic
@@ -64,10 +65,26 @@ let
6465

6566
hardeningEnable = lib.optionals (!stdenv'.cc.isClang) [ "pie" ];
6667

67-
outputs = [ "out" "lib" "doc" "man" ];
68-
setOutputFlags = false; # $out retains configureFlags :-/
68+
outputs = [ "out" "dev" "doc" "lib" "man" ];
69+
outputChecks.out = {
70+
disallowedReferences = [ "dev" "doc" "man" ];
71+
disallowedRequisites = [
72+
stdenv'.cc
73+
] ++ (
74+
map lib.getDev (builtins.filter (drv: drv ? "dev") finalAttrs.buildInputs)
75+
) ++ lib.optionals jitSupport [
76+
llvmPackages.llvm.out
77+
];
78+
};
6979
outputChecks.lib = {
70-
disallowedReferences = [ "out" "doc" "man" ];
80+
disallowedReferences = [ "out" "dev" "doc" "man" ];
81+
disallowedRequisites = [
82+
stdenv'.cc
83+
] ++ (
84+
map lib.getDev (builtins.filter (drv: drv ? "dev") finalAttrs.buildInputs)
85+
) ++ lib.optionals jitSupport [
86+
llvmPackages.llvm.out
87+
];
7188
};
7289

7390
buildInputs = [
@@ -90,6 +107,7 @@ let
90107
nativeBuildInputs = [
91108
makeWrapper
92109
pkg-config
110+
removeReferencesTo
93111
]
94112
++ lib.optionals jitSupport [ llvmPackages.llvm.dev nukeReferences patchelf ];
95113

@@ -116,7 +134,6 @@ let
116134
"--with-libxml"
117135
"--with-icu"
118136
"--sysconfdir=/etc"
119-
"--libdir=$(lib)/lib"
120137
"--with-system-tzdata=${tzdata}/share/zoneinfo"
121138
"--enable-debug"
122139
(lib.optionalString systemdSupport' "--with-systemd")
@@ -133,6 +150,7 @@ let
133150

134151
patches = [
135152
(if atLeast "16" then ./patches/relative-to-symlinks-16+.patch else ./patches/relative-to-symlinks.patch)
153+
(if atLeast "15" then ./patches/empty-pg-config-view-15+.patch else ./patches/empty-pg-config-view.patch)
136154
./patches/less-is-more.patch
137155
./patches/paths-for-split-outputs.patch
138156
./patches/paths-with-postgresql-suffix.patch
@@ -154,43 +172,52 @@ let
154172

155173
postPatch = ''
156174
substituteInPlace "src/Makefile.global.in" --subst-var out
157-
# Hardcode the path to pgxs so pg_config returns the path in $out
158-
substituteInPlace "src/common/config_info.c" --subst-var out
175+
# Hardcode the path to pgxs so pg_config returns the path in $dev
176+
substituteInPlace "src/common/config_info.c" --subst-var dev
159177
'';
160178

161179
postInstall =
162180
''
163-
moveToOutput "lib/libpgcommon*.a" "$out"
164-
moveToOutput "lib/libpgport*.a" "$out"
165-
166-
# Prevent a retained dependency on gcc-wrapper.
167-
substituteInPlace "$out/lib/pgxs/src/Makefile.global" --replace ${stdenv'.cc}/bin/ld ld
181+
moveToOutput "bin/ecpg" "$dev"
182+
moveToOutput "lib/pgxs" "$dev"
183+
184+
# Pretend pg_config is located in $out/bin to return correct paths, but
185+
# actually have it in -dev to avoid pulling in all other outputs.
186+
moveToOutput "bin/pg_config" "$dev"
187+
# To prevent a "pg_config: could not find own program executable" error, we fake
188+
# pg_config in the default output.
189+
cat << EOF > "$out/bin/pg_config" && chmod +x "$out/bin/pg_config"
190+
#!${stdenv'.shell}
191+
echo The real pg_config can be found in the -dev output.
192+
exit 1
193+
EOF
194+
wrapProgram "$dev/bin/pg_config" --argv0 "$out/bin/pg_config"
195+
196+
# postgres exposes external symbols get_pkginclude_path and similar. Those
197+
# can't be stripped away by --gc-sections/LTO, because they could theoretically
198+
# be used by dynamically loaded modules / extensions. To avoid circular dependencies,
199+
# references to -dev, -doc and -man are removed here. References to -lib must be kept,
200+
# because there is a realistic use-case for extensions to locate the /lib directory to
201+
# load other shared modules.
202+
remove-references-to -t "$dev" -t "$doc" -t "$man" "$out/bin/postgres"
168203
169204
if [ -z "''${dontDisableStatic:-}" ]; then
170205
# Remove static libraries in case dynamic are available.
171-
for i in $out/lib/*.a $lib/lib/*.a; do
206+
for i in $lib/lib/*.a; do
172207
name="$(basename "$i")"
173208
ext="${stdenv'.hostPlatform.extensions.sharedLibrary}"
174209
if [ -e "$lib/lib/''${name%.a}$ext" ] || [ -e "''${i%.a}$ext" ]; then
175210
rm "$i"
176211
fi
177212
done
178213
fi
214+
# The remaining static libraries are libpgcommon.a, libpgport.a and related.
215+
# Those are only used when building e.g. extensions, so go to $dev.
216+
moveToOutput "lib/*.a" "$dev"
179217
'' + lib.optionalString jitSupport ''
180218
# In the case of JIT support, prevent a retained dependency on clang-wrapper
181-
substituteInPlace "$out/lib/pgxs/src/Makefile.global" --replace ${stdenv'.cc}/bin/clang clang
182219
nuke-refs $out/lib/llvmjit_types.bc $(find $out/lib/bitcode -type f)
183220
184-
# Stop out depending on the default output of llvm
185-
substituteInPlace $out/lib/pgxs/src/Makefile.global \
186-
--replace ${llvmPackages.llvm.out}/bin "" \
187-
--replace '$(LLVM_BINPATH)/' ""
188-
189-
# Stop out depending on the -dev output of llvm
190-
substituteInPlace $out/lib/pgxs/src/Makefile.global \
191-
--replace ${llvmPackages.llvm.dev}/bin/llvm-config llvm-config \
192-
--replace -I${llvmPackages.llvm.dev}/include ""
193-
194221
${lib.optionalString (!stdenv'.isDarwin) ''
195222
# Stop lib depending on the -dev output of llvm
196223
rpath=$(patchelf --print-rpath $out/lib/llvmjit.so)
@@ -210,8 +237,6 @@ let
210237
# autodetection doesn't seem to able to find this, but it's there.
211238
checkTarget = "check";
212239

213-
disallowedReferences = [ stdenv'.cc ];
214-
215240
passthru = let
216241
this = self.callPackage generic args;
217242
jitToggle = this.override {

‎pkgs/servers/sql/postgresql/patches/empty-pg-config-view-15+.patch‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
Empty the pg_config system information view. This view keeps references to
2+
several -dev outputs, which we want to avoid to keep closure size down.
3+
4+
The alternative to this patch would be to nuke references across the board,
5+
but this will also affect the output of the pg_config utility. By emptying
6+
the view only, we keep the pg_config binary intact. It resides in the -dev
7+
output, so it's fine to have all those references there.
8+
9+
---
10+
--- a/src/backend/utils/misc/pg_config.c
11+
+++ b/src/backend/utils/misc/pg_config.c
12+
@@ -32,20 +32,5 @@ pg_config(PG_FUNCTION_ARGS)
13+
/* initialize our tuplestore */
14+
InitMaterializedSRF(fcinfo, 0);
15+
16+
- configdata = get_configdata(my_exec_path, &configdata_len);
17+
- for (i = 0; i < configdata_len; i++)
18+
- {
19+
- Datum values[2];
20+
- bool nulls[2];
21+
-
22+
- memset(values, 0, sizeof(values));
23+
- memset(nulls, 0, sizeof(nulls));
24+
-
25+
- values[0] = CStringGetTextDatum(configdata[i].name);
26+
- values[1] = CStringGetTextDatum(configdata[i].setting);
27+
-
28+
- tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc, values, nulls);
29+
- }
30+
-
31+
return (Datum) 0;
32+
}
33+
--- a/src/test/regress/expected/sysviews.out
34+
+++ b/src/test/regress/expected/sysviews.out
35+
@@ -29,7 +29,7 @@ select name, ident, parent, level, total_bytes >= free_bytes
36+
(1 row)
37+
38+
-- At introduction, pg_config had 23 entries; it may grow
39+
-select count(*) > 20 as ok from pg_config;
40+
+select count(*) = 0 as ok from pg_config;
41+
ok
42+
----
43+
t
44+
--- a/src/test/regress/sql/sysviews.sql
45+
+++ b/src/test/regress/sql/sysviews.sql
46+
@@ -18,7 +18,7 @@ select name, ident, parent, level, total_bytes >= free_bytes
47+
from pg_backend_memory_contexts where level = 0;
48+
49+
-- At introduction, pg_config had 23 entries; it may grow
50+
-select count(*) > 20 as ok from pg_config;
51+
+select count(*) = 0 as ok from pg_config;
52+
53+
-- We expect no cursors in this test; see also portals.sql
54+
select count(*) = 0 as ok from pg_cursors;

‎pkgs/servers/sql/postgresql/patches/empty-pg-config-view.patch‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
Empty the pg_config system information view. This view keeps references to
2+
several -dev outputs, which we want to avoid to keep closure size down.
3+
4+
The alternative to this patch would be to nuke references across the board,
5+
but this will also affect the output of the pg_config utility. By emptying
6+
the view only, we keep the pg_config binary intact. It resides in the -dev
7+
output, so it's fine to have all those references there.
8+
9+
---
10+
--- a/src/backend/utils/misc/pg_config.c
11+
+++ b/src/backend/utils/misc/pg_config.c
12+
@@ -69,16 +69,6 @@ pg_config(PG_FUNCTION_ARGS)
13+
/* initialize our tuplestore */
14+
tupstore = tuplestore_begin_heap(true, false, work_mem);
15+
16+
- configdata = get_configdata(my_exec_path, &configdata_len);
17+
- for (i = 0; i < configdata_len; i++)
18+
- {
19+
- values[0] = configdata[i].name;
20+
- values[1] = configdata[i].setting;
21+
-
22+
- tuple = BuildTupleFromCStrings(attinmeta, values);
23+
- tuplestore_puttuple(tupstore, tuple);
24+
- }
25+
-
26+
/*
27+
* no longer need the tuple descriptor reference created by
28+
* TupleDescGetAttInMetadata()
29+
--- a/src/test/regress/expected/sysviews.out
30+
+++ b/src/test/regress/expected/sysviews.out
31+
@@ -20,7 +20,7 @@ select count(*) >= 0 as ok from pg_available_extensions;
32+
(1 row)
33+
34+
-- At introduction, pg_config had 23 entries; it may grow
35+
-select count(*) > 20 as ok from pg_config;
36+
+select count(*) = 0 as ok from pg_config;
37+
ok
38+
----
39+
t
40+
--- a/src/test/regress/sql/sysviews.sql
41+
+++ b/src/test/regress/sql/sysviews.sql
42+
@@ -13,7 +13,7 @@ select count(*) >= 0 as ok from pg_available_extension_versions;
43+
select count(*) >= 0 as ok from pg_available_extensions;
44+
45+
-- At introduction, pg_config had 23 entries; it may grow
46+
-select count(*) > 20 as ok from pg_config;
47+
+select count(*) = 0 as ok from pg_config;
48+
49+
-- We expect no cursors in this test; see also portals.sql
50+
select count(*) = 0 as ok from pg_cursors;

‎pkgs/servers/sql/postgresql/patches/paths-for-split-outputs.patch‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
i++;
55

66
configdata[i].name = pstrdup("PGXS");
7-
+ strlcpy(path, "@out@/lib", sizeof(path));
7+
+ strlcpy(path, "@dev@/lib", sizeof(path));
88
- get_pkglib_path(my_exec_path, path);
99
strlcat(path, "/pgxs/src/makefiles/pgxs.mk", sizeof(path));
1010
cleanup_path(path);

0 commit comments

Comments
 (0)