Fixing pg_upgrade's check of available binaries - Mailing list pgsql-hackers

I've been fooling around with creating upgrade-in-place support for the
Fedora/RHEL RPMs.  What I want to have is a separate postgresql-upgrade
RPM containing just the minimum possible set of previous-release files,
together with pg_upgrade itself.  Experimenting with this convinced me
that pg_upgrade is a few bricks shy of a load in its tests for whether
the old and new clusters have the right binaries available:

* it insists on pg_dumpall and psql being present in the old cluster,
though they are not in fact called
* it fails to check for pg_resetxlog, even though it needs it in both
old and new clusters
* it fails to check for pg_config, which it does need in the new
cluster.  It does not however really need it in the old cluster,
because it has no use for the old cluster's --pkglibdir path.

I propose the attached patch to clean these things up.  Any objections?

            regards, tom lane

*** contrib/pg_upgrade/exec.c~    Sat Dec 11 14:05:02 2010
--- contrib/pg_upgrade/exec.c    Tue Dec 28 16:44:26 2010
***************
*** 14,20 ****


  static void check_data_dir(const char *pg_data);
! static void check_bin_dir(ClusterInfo *cluster);
  static int    check_exec(const char *dir, const char *cmdName);
  static const char *validate_exec(const char *path);

--- 14,20 ----


  static void check_data_dir(const char *pg_data);
! static void check_bin_dir(ClusterInfo *cluster, Cluster whichCluster);
  static int    check_exec(const char *dir, const char *cmdName);
  static const char *validate_exec(const char *path);

***************
*** 99,105 ****
      check_ok();

      prep_status("Checking old bin directory (%s)", old_cluster.bindir);
!     check_bin_dir(&old_cluster);
      check_ok();

      prep_status("Checking new data directory (%s)", new_cluster.pgdata);
--- 99,105 ----
      check_ok();

      prep_status("Checking old bin directory (%s)", old_cluster.bindir);
!     check_bin_dir(&old_cluster, CLUSTER_OLD);
      check_ok();

      prep_status("Checking new data directory (%s)", new_cluster.pgdata);
***************
*** 107,113 ****
      check_ok();

      prep_status("Checking new bin directory (%s)", new_cluster.bindir);
!     check_bin_dir(&new_cluster);
      check_ok();
  }

--- 107,113 ----
      check_ok();

      prep_status("Checking new bin directory (%s)", new_cluster.bindir);
!     check_bin_dir(&new_cluster, CLUSTER_NEW);
      check_ok();
  }

***************
*** 158,169 ****
   *    exit().
   */
  static void
! check_bin_dir(ClusterInfo *cluster)
  {
      check_exec(cluster->bindir, "postgres");
-     check_exec(cluster->bindir, "psql");
      check_exec(cluster->bindir, "pg_ctl");
!     check_exec(cluster->bindir, "pg_dumpall");
  }


--- 158,175 ----
   *    exit().
   */
  static void
! check_bin_dir(ClusterInfo *cluster, Cluster whichCluster)
  {
      check_exec(cluster->bindir, "postgres");
      check_exec(cluster->bindir, "pg_ctl");
!     check_exec(cluster->bindir, "pg_resetxlog");
!     if (whichCluster == CLUSTER_NEW)
!     {
!         /* these are only needed in the new cluster */
!         check_exec(cluster->bindir, "pg_config");
!         check_exec(cluster->bindir, "psql");
!         check_exec(cluster->bindir, "pg_dumpall");
!     }
  }


*** contrib/pg_upgrade/option.c~    Wed Dec 15 21:48:52 2010
--- contrib/pg_upgrade/option.c    Tue Dec 28 16:45:24 2010
***************
*** 310,316 ****
  static void
  get_pkglibdirs(void)
  {
!     old_cluster.libpath = get_pkglibdir(old_cluster.bindir);
      new_cluster.libpath = get_pkglibdir(new_cluster.bindir);
  }

--- 310,320 ----
  static void
  get_pkglibdirs(void)
  {
!     /*
!      * we do not need to know the libpath in the old cluster, and might not
!      * have a working pg_config to ask for it anyway.
!      */
!     old_cluster.libpath = NULL;
      new_cluster.libpath = get_pkglibdir(new_cluster.bindir);
  }