[PATCH] make postgresql find its plugins

  • Done
  • quality assurance status badge
Details
2 participants
  • Gábor Boskovits
  • Julien Lepiller
Owner
unassigned
Submitted by
Julien Lepiller
Severity
normal
J
J
Julien Lepiller wrote on 28 Jul 2018 17:58
(address . guix-patches@gnu.org)
20180728175815.23b6e60d@lepiller.eu
Hi,

attached is a patch to make postgresql find its plugins. Actually, it
only makes it unable to follow symlinks, so we can provide a union of
postgresql and plugins to the postgresql service. I also attached an
example system configuration that shows how this works. Do you think
this is the right solution? The patch was taken from nixos and updated
for the current version of postgresql.

Note that the VM will have troubles loading the extensions with only
its 256MB memory by default. You should add more ram to it.

Inside the VM, you can test with:

psql -U postgres
Toggle quote (5 lines)
> create database postgistest;
> \connect postgistest;
> create extension postgis;
> create extension postgis_topology;

no error, you now have an empty spatial database :)

An extension would be to have a procedure to build the union of
packages (postgresql and extensions) called by the postgresql service.
we would have an "extension" field that would contain a list of
packages that contain extensions for postgresql and the service would
build and use the union of postgresql and these extensions. WDYT?
From 74b24f0e0323174156d3407a83833a5a72b0174a Mon Sep 17 00:00:00 2001
From: Julien Lepiller <julien@lepiller.eu>
Date: Sat, 28 Jul 2018 17:38:38 +0200
Subject: [PATCH] gnu: postgresql: Fix finding extensions.

* gnu/packages/patches/postgresql-disable-resolve_symlinks.patch: New
file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/databases.scm (postgresql)[source]: Use it.
---
gnu/local.mk | 1 +
gnu/packages/databases.scm | 5 ++--
.../postgresql-disable-resolve_symlinks.patch | 25 +++++++++++++++++++
3 files changed, 29 insertions(+), 2 deletions(-)
create mode 100644 gnu/packages/patches/postgresql-disable-resolve_symlinks.patch

Toggle diff (68 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index 612304bad..f98cf86e6 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1037,6 +1037,7 @@ dist_patch_DATA = \
%D%/packages/patches/polkit-drop-test.patch \
%D%/packages/patches/portaudio-audacity-compat.patch \
%D%/packages/patches/portmidi-modular-build.patch \
+ %D%/packages/patches/postgresql-disable-resolve_symlinks.patch\
%D%/packages/patches/potrace-tests.patch \
%D%/packages/patches/procmail-ambiguous-getline-debian.patch \
%D%/packages/patches/procmail-CVE-2014-3618.patch \
diff --git a/gnu/packages/databases.scm b/gnu/packages/databases.scm
index 382c74cfd..562b66fde 100644
--- a/gnu/packages/databases.scm
+++ b/gnu/packages/databases.scm
@@ -18,7 +18,7 @@
;;; Copyright © 2016 Andy Patterson <ajpatter@uwaterloo.ca>
;;; Copyright © 2016 Danny Milosavljevic <dannym+a@scratchpost.org>
;;; Copyright © 2016, 2017, 2018 Marius Bakke <mbakke@fastmail.com>
-;;; Copyright © 2017 Julien Lepiller <julien@lepiller.eu>
+;;; Copyright © 2017, 2018 Julien Lepiller <julien@lepiller.eu>
;;; Copyright © 2017 Thomas Danckaert <post@thomasdanckaert.be>
;;; Copyright © 2017 Jelle Licht <jlicht@fsfe.org>
;;; Copyright © 2017 Adriano Peluso <catonano@gmail.com>
@@ -713,7 +713,8 @@ as a drop-in replacement of MySQL.")
version "/postgresql-" version ".tar.bz2"))
(sha256
(base32
- "0j000bcs9w8wrllg8m7j1lxsd3n2x0yzkack5p35cmxx20iq2q0v"))))
+ "0j000bcs9w8wrllg8m7j1lxsd3n2x0yzkack5p35cmxx20iq2q0v"))
+ (patches (search-patches "postgresql-disable-resolve_symlinks.patch"))))
(build-system gnu-build-system)
(arguments
`(#:configure-flags '("--with-uuid=e2fs")
diff --git a/gnu/packages/patches/postgresql-disable-resolve_symlinks.patch b/gnu/packages/patches/postgresql-disable-resolve_symlinks.patch
new file mode 100644
index 000000000..7874c37b9
--- /dev/null
+++ b/gnu/packages/patches/postgresql-disable-resolve_symlinks.patch
@@ -0,0 +1,25 @@
+From 223c82d1d6ed1f29f26307249827ff679e09c780 Mon Sep 17 00:00:00 2001
+From: Julien Lepiller <julien@lepiller.eu>
+Date: Sat, 28 Jul 2018 12:22:12 +0200
+Subject: [PATCH] disable resolve_symlink
+
+---
+ src/common/exec.c | 2 ++
+ 1 file changed, 2 insertions(+)
+
+diff --git a/src/common/exec.c b/src/common/exec.c
+index 878fc29..6b3e283 100644
+--- a/src/common/exec.c
++++ b/src/common/exec.c
+@@ -218,6 +218,8 @@ find_my_exec(const char *argv0, char *retpath)
+ static int
+ resolve_symlinks(char *path)
+ {
++ // On GuixSD we *want* stuff relative to symlinks.
++ return 0;
+ #ifdef HAVE_READLINK
+ struct stat buf;
+ char orig_wd[MAXPGPATH],
+--
+2.18.0
+
--
2.18.0
;; This is an operating system configuration template ;; for a "bare bones" setup, with no X11 display server. (use-modules (gnu) (guix packages) (guix build-system trivial)) (use-service-modules databases networking ssh) (use-package-modules databases geo screen ssh) (operating-system (host-name "komputilo") (timezone "Europe/Berlin") (locale "en_US.utf8") ;; Boot in "legacy" BIOS mode, assuming /dev/sdX is the ;; target hard disk, and "my-root" is the label of the target ;; root file system. (bootloader (bootloader-configuration (bootloader grub-bootloader) (target "/dev/sdX"))) (file-systems (cons (file-system (device (file-system-label "my-root")) (mount-point "/") (type "ext4")) %base-file-systems)) ;; This is where user accounts are specified. The "root" ;; account is implicit, and is initially created with the ;; empty password. (users (cons (user-account (name "alice") (comment "Bob's sister") (group "users") ;; Adding the account to the "wheel" group ;; makes it a sudoer. Adding it to "audio" ;; and "video" allows the user to play sound ;; and access the webcam. (supplementary-groups '("wheel" "audio" "video")) (home-directory "/home/alice")) %base-user-accounts)) ;; Globally-installed packages. (packages (cons* screen openssh postgresql %base-packages)) ;; Add services to the baseline: a DHCP client and ;; an SSH server. (services (cons* (dhcp-client-service) (postgresql-service #:postgresql (package (name "postgresql") (version (package-version postgresql)) (source #f) (build-system trivial-build-system) (arguments `(#:modules ((guix build utils)) #:builder (begin (use-modules (guix build utils) (srfi srfi-26)) (let* ((postgresql (assoc-ref %build-inputs "postgresql")) (postgis (assoc-ref %build-inputs "postgis")) (out (assoc-ref %outputs "out"))) (with-directory-excursion postgresql (for-each (lambda (file) (mkdir-p (string-append out "/" (dirname file))) (symlink (string-append postgresql "/" file) (string-append out "/" file))) (find-files "." ".*"))) (with-directory-excursion postgis (for-each (lambda (file) (mkdir-p (string-append out "/" (dirname file))) (symlink (string-append postgis "/" file) (string-append out "/" file))) (find-files "." ".*"))))))) (inputs `(("postgresql" ,postgresql) ("postgis" ,postgis))) (home-page "") (synopsis "") (description "") (license (package-license postgresql)))) (service openssh-service-type (openssh-configuration (port-number 2222))) %base-services)))
G
G
Gábor Boskovits wrote on 14 Sep 2018 17:29
CAE4v=pjrYxftcbHjEPgeFX1b-UT3wvG9t-MevKNHatGEqJNZuw@mail.gmail.com
This looks good to me.

The extensions field is a good idea.

I also believe that after adding an extensions field it would be easier to
document this.

Do you think that it might be possible to list these extension packages
somehow, or even stop them being directly installable, noting to use the
extensions field in your service definition? Would that make sense?
Attachment: file
J
J
Julien Lepiller wrote on 14 Sep 2018 22:03
(address . 32297@debbugs.gnu.org)
20180914220353.538a9c9f@lepiller.eu
Le Fri, 14 Sep 2018 17:29:19 +0200,
Gábor Boskovits <boskovits@gmail.com> a écrit :

Toggle quote (7 lines)
> This looks good to me.
>
> The extensions field is a good idea.
>
> I also believe that after adding an extensions field it would be
> easier to document this.

Here is a new series of 2 patches: the first one is unchanged (only
rebased to master) and the second one introduces the extensions field
and documents it. I added an example in the documentation.

Toggle quote (6 lines)
>
> Do you think that it might be possible to list these extension
> packages somehow, or even stop them being directly installable,
> noting to use the extensions field in your service definition? Would
> that make sense?

Extension packages still have to be visible for users to list them in
the new field, so it seems hard to hide them at the same time :) I
agree it would make sense to be able to list all such extensions, and
so is listing extensions to other packages. Maybe we can think of a
generic mechanism for that?
From db5d8bd717f84956a083c6dd7c772433c089da81 Mon Sep 17 00:00:00 2001
From: Julien Lepiller <julien@lepiller.eu>
Date: Sat, 28 Jul 2018 17:38:38 +0200
Subject: [PATCH 1/2] gnu: postgresql: Fix finding extensions.

* gnu/packages/patches/postgresql-disable-resolve_symlinks.patch: New
file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/databases.scm (postgresql)[source]: Use it.
---
gnu/local.mk | 1 +
gnu/packages/databases.scm | 5 ++--
.../postgresql-disable-resolve_symlinks.patch | 25 +++++++++++++++++++
3 files changed, 29 insertions(+), 2 deletions(-)
create mode 100644 gnu/packages/patches/postgresql-disable-resolve_symlinks.patch

Toggle diff (68 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index 7b230cb6f..010aa76e5 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1053,6 +1053,7 @@ dist_patch_DATA = \
%D%/packages/patches/plotutils-libpng-jmpbuf.patch \
%D%/packages/patches/portaudio-audacity-compat.patch \
%D%/packages/patches/portmidi-modular-build.patch \
+ %D%/packages/patches/postgresql-disable-resolve_symlinks.patch \
%D%/packages/patches/potrace-tests.patch \
%D%/packages/patches/procmail-ambiguous-getline-debian.patch \
%D%/packages/patches/procmail-CVE-2014-3618.patch \
diff --git a/gnu/packages/databases.scm b/gnu/packages/databases.scm
index 9db892a0b..722099732 100644
--- a/gnu/packages/databases.scm
+++ b/gnu/packages/databases.scm
@@ -18,7 +18,7 @@
;;; Copyright © 2016 Andy Patterson <ajpatter@uwaterloo.ca>
;;; Copyright © 2016 Danny Milosavljevic <dannym+a@scratchpost.org>
;;; Copyright © 2016, 2017, 2018 Marius Bakke <mbakke@fastmail.com>
-;;; Copyright © 2017 Julien Lepiller <julien@lepiller.eu>
+;;; Copyright © 2017, 2018 Julien Lepiller <julien@lepiller.eu>
;;; Copyright © 2017 Thomas Danckaert <post@thomasdanckaert.be>
;;; Copyright © 2017 Jelle Licht <jlicht@fsfe.org>
;;; Copyright © 2017 Adriano Peluso <catonano@gmail.com>
@@ -812,7 +812,8 @@ as a drop-in replacement of MySQL.")
version "/postgresql-" version ".tar.bz2"))
(sha256
(base32
- "04a07jkvc5s6zgh6jr78149kcjmsxclizsqabjw44ld4j5n633kc"))))
+ "04a07jkvc5s6zgh6jr78149kcjmsxclizsqabjw44ld4j5n633kc"))
+ (patches (search-patches "postgresql-disable-resolve_symlinks.patch"))))
(build-system gnu-build-system)
(arguments
`(#:configure-flags '("--with-uuid=e2fs")
diff --git a/gnu/packages/patches/postgresql-disable-resolve_symlinks.patch b/gnu/packages/patches/postgresql-disable-resolve_symlinks.patch
new file mode 100644
index 000000000..97ef6928f
--- /dev/null
+++ b/gnu/packages/patches/postgresql-disable-resolve_symlinks.patch
@@ -0,0 +1,25 @@
+From 223c82d1d6ed1f29f26307249827ff679e09c780 Mon Sep 17 00:00:00 2001
+From: Julien Lepiller <julien@lepiller.eu>
+Date: Sat, 28 Jul 2018 12:22:12 +0200
+Subject: [PATCH] disable resolve_symlink
+
+---
+ src/common/exec.c | 2 ++
+ 1 file changed, 2 insertions(+)
+
+diff --git a/src/common/exec.c b/src/common/exec.c
+index 878fc29..6b3e283 100644
+--- a/src/common/exec.c
++++ b/src/common/exec.c
+@@ -218,6 +218,8 @@ find_my_exec(const char *argv0, char *retpath)
+ static int
+ resolve_symlinks(char *path)
+ {
++ // On GuixSD we *want* stuff relative to symlinks.
++ return 0;
+ #ifdef HAVE_READLINK
+ struct stat buf;
+ char orig_wd[MAXPGPATH],
+--
+2.18.0
+
--
2.18.0
J
J
Julien Lepiller wrote on 14 Sep 2018 22:14
Re: [bug#32297] [PATCH] make postgresql find its plugins
(address . 32297@debbugs.gnu.org)
20180914221407.286de0e6@lepiller.eu
Le Fri, 14 Sep 2018 22:03:53 +0200,
Julien Lepiller <julien@lepiller.eu> a écrit :

Toggle quote (26 lines)
> Le Fri, 14 Sep 2018 17:29:19 +0200,
> Gábor Boskovits <boskovits@gmail.com> a écrit :
>
> > This looks good to me.
> >
> > The extensions field is a good idea.
> >
> > I also believe that after adding an extensions field it would be
> > easier to document this.
>
> Here is a new series of 2 patches: the first one is unchanged (only
> rebased to master) and the second one introduces the extensions field
> and documents it. I added an example in the documentation.
>
> >
> > Do you think that it might be possible to list these extension
> > packages somehow, or even stop them being directly installable,
> > noting to use the extensions field in your service definition? Would
> > that make sense?
>
> Extension packages still have to be visible for users to list them in
> the new field, so it seems hard to hide them at the same time :) I
> agree it would make sense to be able to list all such extensions, and
> so is listing extensions to other packages. Maybe we can think of a
> generic mechanism for that?

Whoops, that last patch wasn't complete :)
J
J
Julien Lepiller wrote on 14 Sep 2018 22:49
(address . 32297@debbugs.gnu.org)
20180914224936.1d33b9aa@lepiller.eu
Le Fri, 14 Sep 2018 22:14:07 +0200,
Julien Lepiller <julien@lepiller.eu> a écrit :

Toggle quote (31 lines)
> Le Fri, 14 Sep 2018 22:03:53 +0200,
> Julien Lepiller <julien@lepiller.eu> a écrit :
>
> > Le Fri, 14 Sep 2018 17:29:19 +0200,
> > Gábor Boskovits <boskovits@gmail.com> a écrit :
> >
> > > This looks good to me.
> > >
> > > The extensions field is a good idea.
> > >
> > > I also believe that after adding an extensions field it would be
> > > easier to document this.
> >
> > Here is a new series of 2 patches: the first one is unchanged (only
> > rebased to master) and the second one introduces the extensions
> > field and documents it. I added an example in the documentation.
> >
> > >
> > > Do you think that it might be possible to list these extension
> > > packages somehow, or even stop them being directly installable,
> > > noting to use the extensions field in your service definition?
> > > Would that make sense?
> >
> > Extension packages still have to be visible for users to list them
> > in the new field, so it seems hard to hide them at the same time :)
> > I agree it would make sense to be able to list all such extensions,
> > and so is listing extensions to other packages. Maybe we can think
> > of a generic mechanism for that?
>
> Whoops, that last patch wasn't complete :)

I must be really tired, I spotted more mistakes after trying to run
guix system on a config with postgis.
G
G
Gábor Boskovits wrote on 24 Sep 2018 13:23
make postgresql find its plugins
CAE4v=ph+q1TsgcA9Vq6G1=6BbqyGjJETcKq7Go3pG0xxLmi=TQ@mail.gmail.com
It works fine for me.

Some things I noticed while checking this:
1. postgresql contrib extensions should not be listed, and are working even
without the patches. (This includes for example hstore, dblink,...
full list)
Did not test if it is problem if one adds these to the config though.
2. I had postgis actually missing, due to a missing use module, but there
was no complaint, system reconfigure went just fine, but creating extension
postgis did not work. Can we add some way to check if the list supplied is
sane?
3. contrib extensions also work fine with the patches applied.
4. I did not test if it works if we have more than one extension, but it
looks good.
If we have any other extension packaged, we could take a look.
5. Do you think we should add a test?
6. I got a warning that the first patch introduces whitespace errors on
current master. Is that ok?
Attachment: file
J
J
Julien Lepiller wrote on 4 Oct 2018 22:36
(address . 32297-done@debbugs.gnu.org)
20181004223647.1b413458@lepiller.eu
Le Mon, 24 Sep 2018 13:23:38 +0200,
Gábor Boskovits <boskovits@gmail.com> a écrit :

Toggle quote (20 lines)
> It works fine for me.
>
> Some things I noticed while checking this:
> 1. postgresql contrib extensions should not be listed, and are
> working even without the patches. (This includes for example hstore,
> dblink,... see
> https://www.postgresql.org/docs/current/static/contrib.html for the
> full list) Did not test if it is problem if one adds these to the
> config though. 2. I had postgis actually missing, due to a missing
> use module, but there was no complaint, system reconfigure went just
> fine, but creating extension postgis did not work. Can we add some
> way to check if the list supplied is sane?
> 3. contrib extensions also work fine with the patches applied.
> 4. I did not test if it works if we have more than one extension, but
> it looks good.
> If we have any other extension packaged, we could take a look.
> 5. Do you think we should add a test?
> 6. I got a warning that the first patch introduces whitespace errors
> on current master. Is that ok?

Pushed on staging since the changes to postgresql requires a lot of
rebuilds.

The whitespace error is located in parts of the patch I can't change.
I've also added a sentence to the manual to make it clear that you
should list packages and not extensions themselves, and that you don't
need to add contrib extensions in the field. Otherwise, I think I've
addressed all your comments :)

Thank you for your review!
Closed
?