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.
