Hi, On 2023-02-19 11:13:38 -0500, Tom Lane wrote: > Andrew Dunstan <and...@dunslane.net> writes: > > On 2023-02-19 Su 02:25, Peter Eisentraut wrote: > >> On 18.02.23 21:26, Andres Freund wrote: > >>> My inclination is to move TEMP_CONFIG support from the Makefile to > >>> pg_regress.c. That way it's consistent across the build tools and isn't > >>> duplicated. > > >> I'm having a hard time understanding what TEMP_CONFIG is for. > > > It's used by the buildfarm to add the extra config settings from its > > configuration file. > > I have also used it manually to inject configuration changes into > TAP tests, for instance running them with debug_discard_caches = 1.
Similar. Explicitly turning on fsync, changing the log level to debug etc. > It's quite handy, but I agree the lack of documentation is bad. We have some minimal documentation for EXTRA_REGRESS_OPTS, but imo that's a bit of a different use case, as it adds actual commandline options for pg_regress (and thus doesn't work for tap tests). Seems we'd need a section in regress.sgml documenting the various environment variables? > It looks to me like pg_regress already does implement this; that > is, the Makefiles convert TEMP_CONFIG into a --temp-config switch > to pg_[isolation_]regress. So if we made pg_regress responsible > for examining the envvar directly, very little new code would be > needed. It's very little, indeed - the patch upthread ends up with: 4 files changed, 11 insertions(+), 16 deletions(-) > (Maybe net negative code if we remove the command line > switch, but I'm not sure if we should.) I don't think we should - we use it for various regression tests, to specify a config file they should load (shared_preload_libraries, wal_level, etc). The way I implemented it now is that TEMP_CONFIG is added earlier in the resulting config file, than the contents of the file explicitly specified on the commandline. > What we'd lose is the ability to write make TEMP_CONFIG=foo check but I > wouldn't miss that. Having a uniform rule that TEMP_CONFIG is an > environment variable and nothing else seems good. If we were concerned about it we could just add an export of TEMP_CONFIG to src/Makefile.global.in Greetings, Andres Freund