> On 27 Jul 2019, at 08:42, Peter Eisentraut <peter.eisentr...@2ndquadrant.com> > wrote: > > On 2019-07-25 16:33, Daniel Gustafsson wrote: >> Fair enough, those are both excellent points. I’ve shuffled the code around >> to >> move back the check for exec_path to setup (albeit earlier than before due to >> where we perform directory checking). This does mean that the directory >> checking in the options parsing must learn to cope with missing directories, >> which is a bit unfortunate since it’s already doing a few too many things >> IMHO. >> To ensure dogfooding, I also removed the use of -B in ‘make check’ for >> pg_upgrade, which should bump the coverage. >> >> Also spotted a typo in a pg_upgrade file header in a file touched by this, so >> included it in this thread too as a 0004. > > I have committed 0002, 0003, and 0004. > > The implementation in 0001 (Only allow upgrades by the same exact > version new bindir) has a problem. It compares (new_cluster.bin_version > != PG_VERSION_NUM), but new_cluster.bin_version is actually just the > version of pg_ctl, so this is just comparing the version of pg_upgrade > with the version of pg_ctl, which is not wrong, but doesn't really > achieve the full goal of having all binaries match. > > I think a better structure would be to add a version check for each > validate_exec() so that each program is checked against pg_upgrade. > This should mirror what find_other_exec() in src/common/exec.c does. In > a better world we would use find_other_exec() directly, but then we > can't support -B. Maybe expand find_other_exec() to support this, or > make a private copy for pg_upgrade to support this. (Also, we have two > copies of validate_exec() around. Maybe this could all be unified.)
Turns out I overshot my original estimate of a new 0001 by a hair (by ~530 days or so) but attached is an updated version. This exports validate_exec to reduce duplication, and implements a custom find_other_exec-like function in pg_upgrade to check each binary for the version number. Keeping a local copy of validate_exec is easy to do if it's deemed not worth it to export it. -- Daniel Gustafsson https://vmware.com/
v5-0001-Check-version-of-target-cluster-binaries.patch
Description: Binary data
