Skip to content

postgresql: cleanup postgis#54396

Merged
danbst merged 8 commits intoNixOS:masterfrom
danbst:postgis
Jan 26, 2019
Merged

postgresql: cleanup postgis#54396
danbst merged 8 commits intoNixOS:masterfrom
danbst:postgis

Conversation

@danbst
Copy link
Contributor

@danbst danbst commented Jan 21, 2019

Motivation for this change
  • remove unneeded code
  • make sure GDAL doesn't use wrong PG version
  • pin poppler to an older version, to prevent future GDAL<->poppler API breaks (on poppler updates)
  • split 35Mb of docs into separate output

This is rebased and depends on #54319

cc @ingenieroariel @thoughtpolice

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Extracts some useful parts of NixOS#38698,
in particular, it's vision that postgresql plugins should be namespaced.

Original approach had several problems:
- not gonna happen in forseeable future
- did lots of deprecations
- was all-in-one solution, which is hard to sell to nixpkgs
- even that we have postgresqlPackages now, we can't do arbitrary overrides
  to postgresql and plugins. Several required functions were not exported

Here I've fixed all of those problems:
- deprecates nothing (though plugins were moved now into `aliases.nix`)
- this doesn't touch NixOS at all, and doesn't break anything
- hashes for plugins and PGs are not changed (I hope)
- no extra fixes to pg itself
- default PG is not changed
- plugins and PGs are extensible

Last one is the most interesting thing, because it introduces novel way
to manage `XXX withPackages` problem. It is novel, but has got lots of
inspiration from existing approaches:
- python, so we have now `postgresql.pkgs.*`, `postgresql_11.pkgs.*`
  which all contain plugins compiled with correct PG.
- python, so we have now `postgresql.withPackages` as well
- in contrast to python, there are no `postgresql11Packages`, only
  `postgresql_11.pkgs`
- NixOS#44196, so plugins are referenced starting at self-fixpoint.
  This allows override/add plugins with mere `//` in overlay. This works for
  both `postgresqlPackages` (overrides are applied to all postgresql_xx.pkgs)
  and `postgresql_xx.pkgs` (overrides are specific to this postgresql) sets
- I've made it compatible with proposed mergeable overlays (NixOS#54266)
  however this PR doesn't depend on it
- last, but not least, `postgresql/default.nix` is now an overlay! This
  replaces previous `callPackages` approach with a modern, extensible concept.
Previous approach turned out to be awful. It was impossible to perform
deep override.

This time I didn't invent bycicles and used overlays for subpackages.

Big change is that now overriding `postgresqlPackages` doesn't override
all other package sets. But `postgresqlPackages` are now also available
as an overlay! So you can get that, reorganize whatever you want and then
attach to some postgresql.
Another part of NixOS#38698, though I did cleanup even more.
Moving docs to separate output should save another 30MB.

I did pin poppler to 0.61 just to be sure GDAL doesn't break again next
time poppler changes internal APIs.
@danbst danbst requested a review from thoughtpolice as a code owner January 21, 2019 01:26
@GrahamcOfBorg GrahamcOfBorg added the 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS label Jan 21, 2019
@ingenieroariel
Copy link
Member

I tested it and it works just as master does (but using postgresql 11). The incantation is a bit ugly (mostly based on stolen code):

let
  pkgs = import (fetchTarball "https://github.com/danbst/nixpkgs/archive/afb270f60a8b9721741538d8fb563a73302cd39d.tar.gz") {};
  stdenv = pkgs.stdenv;
  pg = (pg:
      pkgs.buildEnv {
        name = "postgresql-and-plugins-${(builtins.parseDrvName pg.name).version}";
        paths = [ pg pg.lib (pkgs.postgresqlPackages.postgis.override { postgresql = pg; }) ];
        buildInputs = [ pkgs.makeWrapper ];
        postBuild = ''
          mkdir -p $out/bin
          rm $out/bin/{pg_config,postgres,pg_ctl}
          cp --target-directory=$out/bin ${pg}/bin/{postgres,pg_config,pg_ctl}
          wrapProgram $out/bin/postgres --set NIX_PGLIBDIR $out/lib
        '';
      }
    );
in stdenv.mkDerivation rec {
    name = "postgistest";
    buildInputs = with pkgs; [ (pg postgresql_11) ];
    shellHook =
  ''
    export PGDATA=`pwd`/data
    initdb --no-locale --encoding=UTF-8
    pg_ctl -D $PGDATA -l "$PGDATA/server.log" start
    sleep 2
    createdb postgistest
    psql postgistest -c "CREATE EXTENSION postgis;"
    psql postgistest -c "CREATE EXTENSION postgis_topology;"
    psql postgistest -c "SELECT ST_GeomFromText('POINT(1 1)');"
  '';
}

@danbst
Copy link
Contributor Author

danbst commented Jan 21, 2019

@ingenieroariel much thanks for your test! BTW, with this PR you can simplify expression down to:

    let
      pkgs = import (fetchTarball "https://github.com/danbst/nixpkgs/archive/afb270f60a8b9721741538d8fb563a73302cd39d.tar.gz") {};
      pg = pkgs.postgresql_11.withPackages (ps: [   # ps stands for "plugins" or "packages"
          ps.postgis
      ]);
    in pkgs.mkShell rec {
        name = "postgistest";
        buildInputs = [ pg ];
        shellHook = ''
            export PGDATA=$PWD/data
            pg_ctl -D $PGDATA initdb -w
            pg_ctl -D $PGDATA -l "$PGDATA/server.log" start -w
            createdb postgistest
            psql postgistest -c "CREATE EXTENSION postgis;"
            psql postgistest -c "CREATE EXTENSION postgis_topology;"
            psql postgistest -c "SELECT ST_GeomFromText('POINT(1 1)');"
        '';
    };

@GrahamcOfBorg GrahamcOfBorg added 8.has: clean-up This PR removes packages or removes other cruft 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Jan 21, 2019
@nixos-discourse
Copy link

This pull request has been mentioned on Nix community. There might be relevant details there:

https://discourse.nixos.org/t/heads-up-upcoming-changes-to-the-postgresql-infrastructure-and-nixos-module/297/5

@GrahamcOfBorg GrahamcOfBorg added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. and removed 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Jan 26, 2019
@danbst danbst merged commit 4fb8bc8 into NixOS:master Jan 26, 2019
@danbst danbst deleted the postgis branch January 26, 2019 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: clean-up This PR removes packages or removes other cruft 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants