setup: don't set default pgdata blindly

* postgresql-setup.in (service_configuration): Move the setting of
pgdata into sysvinit-only branch.
* NEWS: Document fixed bug.
* configure.ac: Bump version to 3.2.

3 files changed, 35 insertions, 8 deletions

diff --git a/NEWS b/NEWS
index 20240df..42267d2 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,15 @@
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
+Bugfixes in 3.2 version
+
+* Fix regression of setting the default pgdata variable for non-default
+  unit names.  This broke --new-systemd-unit option and --initdb, because
+  the postgresql-setup logic tried to create the data directory on the
+  same place as the _default_ unit stores data (instead of explicitly
+  asking for --datadir option).
+
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
 New in 3.1 version
 
 * When doing --upgrade, postgresql-setup now guards against upgrading when
diff --git a/configure.ac b/configure.ac
index 737e26f..7e7ae38 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,5 +1,5 @@
 # Use the MAJ.MIN[~SUFF].  Note that X.X > X.X~SUFF!
-AC_INIT([postgresql-setup], [3.1], [praiskup@redhat.com])
+AC_INIT([postgresql-setup], [3.2], [praiskup@redhat.com])
 AC_CONFIG_AUX_DIR(auxdir)
 config_aux_dir=auxdir
 AC_SUBST([config_aux_dir])
diff --git a/postgresql-setup.in b/postgresql-setup.in
index fd1bef2..db2acb3 100644
--- a/postgresql-setup.in
+++ b/postgresql-setup.in
@@ -450,27 +450,44 @@ service_configuration()
     local unit_pgdata=
     local envfile_pgport=
     local envfile_pgdata=
+
+    # 'mode' is 'initdb' or 'upgrade'.  Basically, if called with mode=initdb, we
+    # parse configuration of the current (maybe already configured) service.
+    # When run with mode=upgrade, we try to parse the configuration of the old
+    # PostgreSQL configuration that we try to upgrade from.
+
     local mode="$1" datavar="$2" portvar="$3" service="$4"
 
     debug "running service_configuration() for $mode"
 
-    # Well, until the bug #1139148 is not resolved somehow, we need to stay ugly
-    # and parse Environment= and EnvironmentFile= statements.
     local service="$service"
     test upgrade = "$mode" && service="$option_upgradefrom"
 
-    # Sysvinit has the default PGDATA configured inside the script, no
-    # additional configuration must exist.  Don't set the default for upgrade,
-    # however.
-    test initdb = "$mode" && set_var "$datavar" "@PGDATADIR@"
-
     if test "@WANT_SYSVINIT@" = 1; then
+        # Sysvinit has the default PGDATA (for default unit name only)
+        # configured directly in the initscript, so no additional configuration
+        # must exist.  Set the default value of pgdata here to match whats in
+        # initscript for the cases when no additional configuration file exists.
+        # This is done to avoid parsing of whole initscript (for the real value)
+        # and mainly to not fail in the logic following 'service_configuration'
+        # call, where we usually want to error that pgdata is not defined..
+        # Don't set the default pgdata for upgrade case, however, as we must
+        # upgrade only from already properly configured & working stack (missing
+        # pgdata here is a good reason to die later).
+        test initdb = "$mode" && test "$service" = "@NAME_SERVICE@" \
+            && set_var "$datavar" "@PGDATADIR@"
         handle_envfile "@initscriptsconfdir@/$service"
     else
+        # We ship two service files, @SERVICE_NAME@.service and
+        # @SERVICE_NAME@@.service.  The former has PGDATA set by default
+        # similarly to sysvinit case.
         handle_service_env "$service"
         handle_service_envfiles "$option_mode" "$service"
     fi
 
+    # EnvironmentFile beats Environment configuration in systemd.  In sysvinit
+    # there is no "unit_pgdata".  So make sure the envfile_gpdata is used later
+    # than unit_pgdata.
     test -n "$unit_pgdata"      && set_var "$datavar" "$unit_pgdata"
     test -n "$envfile_pgdata"   && set_var "$datavar" "$envfile_pgdata"