Skip to content

Comments

postgresql: remove unnecessary store references#322025

Closed
tie wants to merge 1 commit intoNixOS:stagingfrom
tie:postgresql-closure-size
Closed

postgresql: remove unnecessary store references#322025
tie wants to merge 1 commit intoNixOS:stagingfrom
tie:postgresql-closure-size

Conversation

@tie
Copy link
Member

@tie tie commented Jun 23, 2024 •
edited
Loading

Description of changes

This change removes Nix store references from the recorded pg_config build configuration.

Parent PR:

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@tie tie requested a review from wolfgangwalther June 23, 2024 19:51
@tie tie mentioned this pull request Jun 23, 2024
Draft
13 tasks
@tie tie force-pushed the postgresql-closure-size branch from 3ce0fec to f9256aa Compare June 23, 2024 20:01
@ofborg ofborg bot requested review from Ma27, danbst, globin, ivan and thoughtpolice June 23, 2024 21:39
@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 2501-5000 This PR causes many rebuilds on Darwin and should target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Jun 23, 2024
@tie tie force-pushed the postgresql-closure-size branch 3 times, most recently from f7659f4 to 1b7f0bd Compare June 24, 2024 01:53
@tie tie changed the title postgresql: reduce closure size and remove CC references postgresql: remove unnecessary store references Jun 24, 2024
@tie tie force-pushed the postgresql-closure-size branch 2 times, most recently from cc6fdd8 to f9cca4e Compare June 24, 2024 02:26
@wolfgangwalther
Copy link
Contributor

wolfgangwalther commented Jun 24, 2024

Please have a look at #294504, which is almost ready (just some more reviewing + merge). The idea is to allow those references in the -dev output, but disallow them in the default and -lib outputs. Does that already do what you want to achieve?

@tie
postgresql: remove unnecessary store references
9bea780 
This change removes Nix store references from the recorded pg_config
build configuration.
@tie tie force-pushed the postgresql-closure-size branch from f9cca4e to 9bea780 Compare June 24, 2024 07:02
@tie
Copy link
Member Author

tie commented Jun 24, 2024

I think this change is tangential to splitting the outputs. In particular, it nukes all store references in the recorded build information (CONFIGURE_ARGS, lib/pgxs/src/Makefile.global, etc) so that the output does not reference build inputs.

Before (32 references):

$ nix path-info --json /nix/store/6d9r3mfm7sj6lhb7pka8y7y96ksgnann-postgresql-15.7 | jq -r 'map(.references) | .[].[]'
/nix/store/1xbhiff4h76glivjawp967zl0rgsd4sr-coreutils-9.5
/nix/store/2m0lcpblvwak2mgi9cmnbz6bcm50sd9s-lz4-1.9.4-lib
/nix/store/5fk1hr6vlgjj7dv8m1qlaa3pbng68mbj-zlib-1.3.1
/nix/store/5yyfwkhyjz9qqb2r6v1inmbqm2wrlfw6-icu4c-74.2
/nix/store/6d9r3mfm7sj6lhb7pka8y7y96ksgnann-postgresql-15.7
/nix/store/6firbk7lh0v52yrbsz7715bfbgi57a59-libkrb5-1.21.2-dev
/nix/store/6ppzr5xa8r9b4n6a3zk3c6hpzvww68y7-zstd-1.5.6-dev
/nix/store/8hl28dg94qvih7ar6xn30a4lmldsmyj7-glibc-2.39-52-bin
/nix/store/a842wps8k03yhcz7xlhhjm8924y4h8x6-bash-5.2p26
/nix/store/b1fs5lx7bm0h4fkl3rdkis4w19921jc8-readline-8.2p10
/nix/store/cwpim71i292fpflqm1p7j7gww8pmyg7b-ncurses-6.4.20221231-dev
/nix/store/d0lxxvzgjh7inar288d5n09319gfc6cl-lz4-1.9.4-dev
/nix/store/ddrwwwdqgwid9lmbv1kk8v4g8j4bnm5k-glibc-2.39-52
/nix/store/f245blkjjzrq7wwx8v9br96pqzdyw4w9-lz4-1.9.4
/nix/store/gm21vqbnzck67qv1z118drhvsywhmn8d-zstd-1.5.6
/nix/store/gw8ql8kpvnsv3idp4b7wk9gp2s1hc22r-systemd-255.6-dev
/nix/store/ic2z8lv1qrdbl9h6b1bxphysb7d0i8by-systemd-255.6
/nix/store/idc0164wq2k3qyk0yl3d9wjw93my386z-libxml2-2.12.7
/nix/store/j5ipjxsjamsc9fgl4xy9zgxdfbizyfzm-postgresql-15.7-lib
/nix/store/knnn4yb7r90gpl8y6dnfq398xfvmhdmm-gnutar-1.35
/nix/store/mi7jm022mnx408s2l6indrvhj6j1srld-libxml2-2.12.7-dev
/nix/store/ngf6yc2y47wq83gmpy649j0m0wv2946z-icu4c-74.2-dev
/nix/store/qdy43pjaa9gg8yqmk99mhv5wxbyv0kds-openssl-3.0.14
/nix/store/rz1vi6dy3nbhxwbk0h2nk8a1y7a1zc30-openssl-3.0.14-dev
/nix/store/vc85bvphn9izh8mysyn8binn61x4m6df-libkrb5-1.21.2
/nix/store/vq90y2wypq2g4ml59750wf33zwkdkaw7-libxml2-2.12.7-bin
/nix/store/xi3g4h5n8jypchy6f6vdii3nw68a1qgz-libossp-uuid-1.6.2
/nix/store/xs032bkniq85laggi63b9wli8n4s8l84-zlib-1.3.1-dev
/nix/store/xsla8phslc4d33hsfy5kzmgjvsg7jxp8-tzdata-2024a
/nix/store/yb8vgzd03712bkw3lban1ravg7bjilbs-linux-pam-1.6.1
/nix/store/zc0b9hf0ai9vkpni4n76yzs4a9fkdcnb-readline-8.2p10-dev
/nix/store/zgcaymcycvc6hb9xwr44xccgsc25r8jb-zstd-1.5.6-bin

After (17 references):

$ nix path-info --json /nix/store/nn1rqi0ir5pb6gqnxikbrsj0kfzm44hh-postgresql-15.7 | jq -r 'map(.references) | .[].[]'
/nix/store/2m0lcpblvwak2mgi9cmnbz6bcm50sd9s-lz4-1.9.4-lib
/nix/store/5fk1hr6vlgjj7dv8m1qlaa3pbng68mbj-zlib-1.3.1
/nix/store/5yyfwkhyjz9qqb2r6v1inmbqm2wrlfw6-icu4c-74.2
/nix/store/8hl28dg94qvih7ar6xn30a4lmldsmyj7-glibc-2.39-52-bin
/nix/store/a842wps8k03yhcz7xlhhjm8924y4h8x6-bash-5.2p26
/nix/store/b1fs5lx7bm0h4fkl3rdkis4w19921jc8-readline-8.2p10
/nix/store/ddrwwwdqgwid9lmbv1kk8v4g8j4bnm5k-glibc-2.39-52
/nix/store/gm21vqbnzck67qv1z118drhvsywhmn8d-zstd-1.5.6
/nix/store/ic2z8lv1qrdbl9h6b1bxphysb7d0i8by-systemd-255.6
/nix/store/idc0164wq2k3qyk0yl3d9wjw93my386z-libxml2-2.12.7
/nix/store/kzb2w5aaqckkz2p5gv44aibkig0ah67s-postgresql-15.7-lib
/nix/store/nn1rqi0ir5pb6gqnxikbrsj0kfzm44hh-postgresql-15.7
/nix/store/qdy43pjaa9gg8yqmk99mhv5wxbyv0kds-openssl-3.0.14
/nix/store/vc85bvphn9izh8mysyn8binn61x4m6df-libkrb5-1.21.2
/nix/store/xsla8phslc4d33hsfy5kzmgjvsg7jxp8-tzdata-2024a
/nix/store/yb8vgzd03712bkw3lban1ravg7bjilbs-linux-pam-1.6.1

Aside from lib/pgxs/src/Makefile.global (where referenced store paths appear to be unused for PGXS case), these reference are there only to record build information.

For comparison, postgresql-outputs branch at 471c7c7b28432b48d98b639cade4201bf08c8bc5 (28 references):

$ nix path-info --json /nix/store/a8q2wlxxx7is8wywh2dy3fbm3cpc1ls2-postgresql-15.7 | jq -r 'map(.references) | .[].[]'
/nix/store/18rl7amdy28v6rllj8mrly9zczn17bqy-zstd-1.5.6-dev
/nix/store/45xd8flghk6jrqxzs3vl640ksdx5yb9l-zlib-1.3.1-dev
/nix/store/4crjw21g3kzsn7psy54zx27prin5qjca-libkrb5-1.21.2
/nix/store/4gn9bgdq2ixb4yq3zd5q7jdz2gch7n4z-libxml2-2.12.7-dev
/nix/store/5fkdhxk1j6rcjmvxmanwjq4wzh3k0rcb-ncurses-6.4.20221231-dev
/nix/store/73gsms8in3159kvx5aaqskh4w4p16hf4-readline-8.2p10
/nix/store/7k706h1mgrwr5aa28v80jswsr2lm22pv-systemd-255.6-dev
/nix/store/7s6g79arlx3327w551swq5x64api3spd-systemd-255.6
/nix/store/9j6hzvcla24qnfflprmz4gp0z03c6j06-glibc-2.39-52-bin
/nix/store/a8q2wlxxx7is8wywh2dy3fbm3cpc1ls2-postgresql-15.7
/nix/store/bcwg9xchm6xgpbnqsczb5aq113bd17vn-libkrb5-1.21.2-dev
/nix/store/cz97xfxfvh25zl88qpnskmp29c4kxnwr-readline-8.2p10-dev
/nix/store/i9a9iczd3mxx764g7islg41wmwbn8x5h-zstd-1.5.6
/nix/store/jwnvskiqal855xqdxhacbwgzm2gl32gp-openssl-3.0.13-dev
/nix/store/kknpccdva18f940mvzxm0051q0gd6aa0-zlib-1.3.1
/nix/store/kkrkx0lxzx58pb9nrjy3g1l986m33x1n-tzdata-2024a
/nix/store/lby8zhbh8290phiw3xghn77n6xb7q990-libossp-uuid-1.6.2
/nix/store/mhng6mr6dgwm5f5sbdwhj91780yywgqr-bash-5.2p26
/nix/store/nwafrra88p8kmjzwabyniyarnqmilg89-libxml2-2.12.7
/nix/store/ph9gkhnz35c72p96s5aiq1a4x7pvwfk8-lz4-1.9.4
/nix/store/r49hv2aw0swhjh7r1d6wkm2vgw46jkax-glibc-2.39-52
/nix/store/v2lip5yrrxjnrfglcipi2garfwzxz6ak-linux-pam-1.6.1
/nix/store/vpldkwvj3jhf7m22hsayj7wl2i2aivic-postgresql-15.7-lib
/nix/store/wbmr0hr1m3mm0cavjhn3zic9xrhv6mm1-openssl-3.0.13
/nix/store/wkrc5bq9wl84wd4ba2ridp5z3w70nxfr-icu4c-73.2
/nix/store/yy2zivigh81x9c8pa8p0yvwpicbcl437-lz4-1.9.4-dev
/nix/store/zvl7pq1by0drks895jim5q3jlsp4yhag-icu4c-73.2-dev

These dev references are leaking via CONFIGURE_ARGS that this PR sets to the output of nuke-refs.

@wolfgangwalther
Copy link
Contributor

wolfgangwalther commented Jun 24, 2024

I think this change is tangential to splitting the outputs. In particular, it nukes all store references in the recorded build information (CONFIGURE_ARGS, lib/pgxs/src/Makefile.global, etc) so that the output does not reference build inputs.

Right. And my point is that those references should be in the pgxs Makefile. This file is used to build extensions - and building extensions should happen with the same tools used to build PG itself.

Right now it's a problem to have those references, because we don't have split outputs. But once we have the dev output split cleanly, it's totally fine to have them in there - as long as the default and lib outputs are kept clean.

@wolfgangwalther
Copy link
Contributor

wolfgangwalther commented Jun 24, 2024

These dev references are leaking via CONFIGURE_ARGS that this PR sets to the output of nuke-refs.

Hm. I'll need to look into that again. I remember I had something in place to avoid those CONFIGURE_ARGS, but apparently this didn't make the finish line.

@wolfgangwalther wolfgangwalther mentioned this pull request Jun 24, 2024
Merged
13 tasks
@wolfgangwalther
Copy link
Contributor

wolfgangwalther commented Aug 24, 2024

After merging #294504, we now have on staging:

% nix path-info --json /nix/store/wkfq8h812f7xridh5ks45xb5962l4n6m-postgresql-16.4  | jq -r 'map(.references) | .[].[]'
/nix/store/5kcid5as43vc8kvar1w0knh22bgzdyp6-icu4c-74.2
/nix/store/bkvxlipx80z1zbd33w3qvyvnhv0h0k2q-tzdata-2024a
/nix/store/gha2hirja5nwwbhj12cmslrmfvjhj2iv-postgresql-16.4-lib
/nix/store/ib9sy6zprriqwwyi6ihsk0jclqyac9pk-zstd-1.5.6
/nix/store/iyvq4ypsh5rsnc2620kbypbrqyxhjybv-glibc-2.39-52-bin
/nix/store/k9ryh6vf0ffqj2h544hbz6agj6lccfbs-lz4-1.9.4-lib
/nix/store/l3jd89iaa1x31ayln5rqljrnnhlv13n4-libossp-uuid-1.6.2
/nix/store/pvvl9vw8rvnjgn8w1mbc8jh7yxrk4a20-linux-pam-1.6.1
/nix/store/qjdf5m54bfjm38ir198lx3r4gjzynq7q-libxml2-2.13.3
/nix/store/qq6h30pwsjwjgpm5a8g268idsm5rjfw6-openssl-3.0.14
/nix/store/sdfbqgp9ghygc1drk80jvw6z8z2xbwrw-systemd-256.4
/nix/store/sh78727j89acid5dw9xk7nylrb0p0wkj-readline-8.2p10
/nix/store/vpvy79k1qq02p1vyqjk6nb89gwhxqvyb-bash-5.2p32
/nix/store/w5i2qpan9r4814rmsqp0xndcylvj8zii-krb5-1.21.3-lib
/nix/store/wfml6h84mna7sw9f6v7z0l60f4faxk8w-zlib-1.3.1
/nix/store/wkfq8h812f7xridh5ks45xb5962l4n6m-postgresql-16.4
/nix/store/wlffq5p6mxxgfap10sav3ij936jzqm59-glibc-2.39-52

17 references for the default output.

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

Reviewers

@wolfgangwalther wolfgangwalther Awaiting requested review from wolfgangwalther

@Ma27 Ma27 Awaiting requested review from Ma27

@globin globin Awaiting requested review from globin

@thoughtpolice thoughtpolice Awaiting requested review from thoughtpolice

@ivan ivan Awaiting requested review from ivan

@danbst danbst Awaiting requested review from danbst

Assignees

No one assigned

Labels

10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 2501-5000 This PR causes many rebuilds on Darwin and should target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tie @wolfgangwalther