Re: pgsql: Move tablespace path re-creation from the makefiles to pg_regres

On Mon, Apr 12, 2021 at 02:25:53PM +0900, Michael Paquier wrote:
> On Fri, Apr 09, 2021 at 08:07:10PM -0700, Noah Misch wrote:
> > "pg_regress --outputdir" is not a great location for a file or directory
> > created by a user other than the user running pg_regress. If one does "make
> > check" and then "make installcheck" against a cluster running as a different
> > user, the rmtree() will fail, assuming typical umask values. An rmtree() at
> > the end of the tablespace test would mostly prevent that, but that can't help
> > in the event of a mid-test crash.
>
> Yeah, I really don't think that we need to worry about multi-user
> scenarios with pg_regress like that though.

Christoph Berg's first message on this thread reported doing that. If
supporting server_user!=pg_regress_user is unwarranted and Christoph Berg
should stop, then already-committed code suffices.

> > I'm not sure we should support installcheck against a server running as a
> > different user. If we should support it, then I'd probably look at letting
> > the caller pass in a server-writable directory. That directory would house
> > the tablespace instead of outputdir doing so.
>
> But, then, we would be back to the pre-13 position where we'd need to
> have something external to pg_regress in charge of cleaning up and
> creating the tablespace path, no?

Correct.

> That's basically what we want to
> avoid with the Makefile rules.

The race that commit 6c788d9 fixed is not inherent to Makefile rules. For
example, you could have instead caused test.sh to issue 'make
installcheck-parallel TABLESPACEDIR="$outputdir"/testtablespace' and have the
makefiles consult $(TABLESPACEDIR) rather than hard-code ./testtablespace.
(That said, I like how 6c788d9 consolidated Windows and non-Windows paths.)

> I can get that it could be interesting
> to be able to pass down a custom path for the test tablespace, but do
> we really have a need for that?

I don't know. I never considered server_user!=pg_regress_user before this
thread, and I don't plan to use it myself. Your latest patch originated to
make that case work, and my last message was reporting that the patch works
for a rather-narrow interpretation of server_user!=pg_regress_user, failing on
variations thereof. That might be fine.