Date: Mon, 25 Jan 2021 20:12:01 -0300
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Bruce Momjian <bruce@momjian.us>
Cc: Tom Kincaid <tomjohnkincaid@gmail.com>,
	Andres Freund <andres@anarazel.de>,
	Amit Kapila <amit.kapila16@gmail.com>,
	Robert Haas <robertmhaas@gmail.com>,
	Thomas Munro <thomas.munro@gmail.com>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>,
	Stephen Frost <sfrost@snowman.net>,
	Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
Subject: Re: Key management with tests
Message-ID: <20210125231201.GA10108@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20210125191805.GD27081@momjian.us>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

In patch 1,

* The docs are not clear on what happens if --auth-prompt is not given
but an auth prompt is required for the program to work.  Should it exit
with a status other than 0?

* BootStrapKmgr claims it is called by initdb, but that doesn't seem to
be the case.

* Also, BootStrapKmgr is the only one that checks USE_OPENSSL; what if a
with-openssl build inits the datadir, and then a non-openssl runs it?
What if it's the other way around?  I think you'd get a failure in
stat() ...

* ... oh, KMGR_DIR_PID is used but not defined anywhere.  Is it defined
in some later commit?  If so, then I think you've chosen to split the
patch series wrong.


May I suggest to use "git format-patch" to produce the patch files?  When
working with a series like this, trying to do patch handling manually
like you seem to be doing, is much more time-consuming and error prone.
For example, with a branch containing individual commits, you could use 
  git rebase -i origin/master -x "make install check-world"
or similar, so that each commit is built and tested individually.

-- 
Álvaro Herrera       Valdivia, Chile
Al principio era UNIX, y UNIX habló y dijo: "Hello world\n".
No dijo "Hello New Jersey\n", ni "Hello USA\n".