From c2d1b3056d20d3878cbec04b52c48f8cc376e801 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Mon, 24 Nov 2025 07:35:02 +1300 Subject: [PATCH v2 3/3] Drop resultmap mechanism from pg_regress. This was used to tolerate historical per-platform broken strtof() implementations, but those are now gone. Retain only the "automatic" variation mechanism for alternative expected files. Discussion: https://postgr.es/m/CA%2BhUKGJ0J-H8C51HK8pEGdbD7tVuut5igkRhFt0byWm_CezeoQ%40mail.gmail.com --- doc/src/sgml/regress.sgml | 41 +----- src/test/regress/GNUmakefile | 3 +- src/test/regress/meson.build | 4 +- src/test/regress/pg_regress.c | 244 ------------------------------- src/tools/pgindent/typedefs.list | 1 - 5 files changed, 6 insertions(+), 287 deletions(-) diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml index 160f38bef9e..5ae7f4b01e2 100644 --- a/doc/src/sgml/regress.sgml +++ b/doc/src/sgml/regress.sgml @@ -750,43 +750,13 @@ diff results/random.out expected/random.out Since some of the tests inherently produce environment-dependent - results, we have provided ways to specify alternate expected + results, we have provided a way to specify alternate expected result files. Each regression test can have several comparison files - showing possible results on different platforms. There are two - independent mechanisms for determining which comparison file is used - for each test. + showing possible results on different platforms. - The first mechanism allows comparison files to be selected for - specific platforms. There is a mapping file, - src/test/regress/resultmap, that defines - which comparison file to use for each platform. - To eliminate bogus test failures for a particular platform, - you first choose or make a variant result file, and then add a line to the - resultmap file. - - - - Each line in the mapping file is of the form - -testname:output:platformpattern=comparisonfilename - - The test name is just the name of the particular regression test - module. The output value indicates which output file to check. For the - standard regression tests, this is always out. The - value corresponds to the file extension of the output file. - The platform pattern is a pattern in the style of the Unix - tool expr (that is, a regular expression with an implicit - ^ anchor at the start). It is matched against the - platform name as printed by config.guess. - The comparison file name is the base name of the substitute result - comparison file. - - - - The second selection mechanism for variant comparison files is - much more automatic: it simply uses the best match among + The selection mechanism uses the best match among several supplied comparison files. The regression test driver script considers both the standard comparison file for a test, testname.out, and variant files named @@ -794,10 +764,7 @@ testname:output:platformpattern=comparisonfilename (where the digit is any single digit 0-9). If any such file is an exact match, the test is considered to pass; otherwise, the one that generates - the shortest diff is used to create the failure report. (If - resultmap includes an entry for the particular - test, then the base testname is the substitute - name given in resultmap.) + the shortest diff is used to create the failure report. diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile index ef2bddf42ca..f718c8030e5 100644 --- a/src/test/regress/GNUmakefile +++ b/src/test/regress/GNUmakefile @@ -74,8 +74,7 @@ $(OBJS): | submake-libpgport submake-generated-headers regress_data_files = \ $(wildcard $(srcdir)/sql/*.sql) \ $(wildcard $(srcdir)/expected/*.out) \ - $(wildcard $(srcdir)/data/*.data) \ - $(srcdir)/parallel_schedule $(srcdir)/resultmap + $(wildcard $(srcdir)/data/*.data) install-tests: all install install-lib installdirs-tests $(MAKE) -C $(top_builddir)/contrib/spi install diff --git a/src/test/regress/meson.build b/src/test/regress/meson.build index 1da9e9462a9..651556c8d57 100644 --- a/src/test/regress/meson.build +++ b/src/test/regress/meson.build @@ -8,9 +8,7 @@ regress_sources = pg_regress_c + files( 'pg_regress_main.c' ) -# Need make up something roughly like x86_64-pc-mingw64. resultmap matches on -# patterns like ".*-.*-mingw.*". We probably can do better, but for now just -# replace 'gcc' with 'mingw' on windows. +# Need make up something roughly like x86_64-pc-mingw64. host_tuple_cc = cc.get_id() if host_system == 'windows' and host_tuple_cc == 'gcc' host_tuple_cc = 'mingw' diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index efc41fca2ba..4e447d4867a 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -37,15 +37,6 @@ #include "pg_regress.h" #include "portability/instr_time.h" -/* for resultmap we need a list of pairs of strings */ -typedef struct _resultmap -{ - char *test; - char *type; - char *resultfile; - struct _resultmap *next; -} _resultmap; - /* * Values obtained from Makefile. */ @@ -132,8 +123,6 @@ static char socklock[MAXPGPATH]; static StringInfo failed_tests = NULL; static bool in_note = false; -static _resultmap *resultmap = NULL; - static PID_TYPE postmaster_pid = INVALID_PID; static bool postmaster_running = false; @@ -530,192 +519,6 @@ make_temp_sockdir(void) return temp_sockdir; } -/* - * Check whether string matches pattern - * - * In the original shell script, this function was implemented using expr(1), - * which provides basic regular expressions restricted to match starting at - * the string start (in conventional regex terms, there's an implicit "^" - * at the start of the pattern --- but no implicit "$" at the end). - * - * For now, we only support "." and ".*" as non-literal metacharacters, - * because that's all that anyone has found use for in resultmap. This - * code could be extended if more functionality is needed. - */ -static bool -string_matches_pattern(const char *str, const char *pattern) -{ - while (*str && *pattern) - { - if (*pattern == '.' && pattern[1] == '*') - { - pattern += 2; - /* Trailing .* matches everything. */ - if (*pattern == '\0') - return true; - - /* - * Otherwise, scan for a text position at which we can match the - * rest of the pattern. - */ - while (*str) - { - /* - * Optimization to prevent most recursion: don't recurse - * unless first pattern char might match this text char. - */ - if (*str == *pattern || *pattern == '.') - { - if (string_matches_pattern(str, pattern)) - return true; - } - - str++; - } - - /* - * End of text with no match. - */ - return false; - } - else if (*pattern != '.' && *str != *pattern) - { - /* - * Not the single-character wildcard and no explicit match? Then - * time to quit... - */ - return false; - } - - str++; - pattern++; - } - - if (*pattern == '\0') - return true; /* end of pattern, so declare match */ - - /* End of input string. Do we have matching pattern remaining? */ - while (*pattern == '.' && pattern[1] == '*') - pattern += 2; - if (*pattern == '\0') - return true; /* end of pattern, so declare match */ - - return false; -} - -/* - * Scan resultmap file to find which platform-specific expected files to use. - * - * The format of each line of the file is - * testname/hostplatformpattern=substitutefile - * where the hostplatformpattern is evaluated per the rules of expr(1), - * namely, it is a standard regular expression with an implicit ^ at the start. - * (We currently support only a very limited subset of regular expressions, - * see string_matches_pattern() above.) What hostplatformpattern will be - * matched against is the config.guess output. (In the shell-script version, - * we also provided an indication of whether gcc or another compiler was in - * use, but that facility isn't used anymore.) - */ -static void -load_resultmap(void) -{ - char buf[MAXPGPATH]; - FILE *f; - - /* scan the file ... */ - snprintf(buf, sizeof(buf), "%s/resultmap", inputdir); - f = fopen(buf, "r"); - if (!f) - { - /* OK if it doesn't exist, else complain */ - if (errno == ENOENT) - return; - bail("could not open file \"%s\" for reading: %m", buf); - } - - while (fgets(buf, sizeof(buf), f)) - { - char *platform; - char *file_type; - char *expected; - int i; - - /* strip trailing whitespace, especially the newline */ - i = strlen(buf); - while (i > 0 && isspace((unsigned char) buf[i - 1])) - buf[--i] = '\0'; - - /* parse out the line fields */ - file_type = strchr(buf, ':'); - if (!file_type) - { - bail("incorrectly formatted resultmap entry: %s", buf); - } - *file_type++ = '\0'; - - platform = strchr(file_type, ':'); - if (!platform) - { - bail("incorrectly formatted resultmap entry: %s", buf); - } - *platform++ = '\0'; - expected = strchr(platform, '='); - if (!expected) - { - bail("incorrectly formatted resultmap entry: %s", buf); - } - *expected++ = '\0'; - - /* - * if it's for current platform, save it in resultmap list. Note: by - * adding at the front of the list, we ensure that in ambiguous cases, - * the last match in the resultmap file is used. This mimics the - * behavior of the old shell script. - */ - if (string_matches_pattern(host_platform, platform)) - { - _resultmap *entry = pg_malloc(sizeof(_resultmap)); - - entry->test = pg_strdup(buf); - entry->type = pg_strdup(file_type); - entry->resultfile = pg_strdup(expected); - entry->next = resultmap; - resultmap = entry; - } - } - fclose(f); -} - -/* - * Check in resultmap if we should be looking at a different file - */ -static -const char * -get_expectfile(const char *testname, const char *file) -{ - char *file_type; - _resultmap *rm; - - /* - * Determine the file type from the file name. This is just what is - * following the last dot in the file name. - */ - if (!file || !(file_type = strrchr(file, '.'))) - return NULL; - - file_type++; - - for (rm = resultmap; rm != NULL; rm = rm->next) - { - if (strcmp(testname, rm->test) == 0 && strcmp(file_type, rm->type) == 0) - { - return rm->resultfile; - } - } - - return NULL; -} - /* * Prepare environment variables for running regression tests */ @@ -917,8 +720,6 @@ initialize_environment(void) if (!pghost && !pgport) note("using postmaster on Unix socket, default port"); } - - load_resultmap(); } #ifdef ENABLE_SSPI @@ -1414,26 +1215,8 @@ results_differ(const char *testname, const char *resultsfile, const char *defaul int best_line_count; int i; int l; - const char *platform_expectfile; - - /* - * We can pass either the resultsfile or the expectfile, they should have - * the same type (filename.type) anyway. - */ - platform_expectfile = get_expectfile(testname, resultsfile); strlcpy(expectfile, default_expectfile, sizeof(expectfile)); - if (platform_expectfile) - { - /* - * Replace everything after the last slash in expectfile with what the - * platform_expectfile contains. - */ - char *p = strrchr(expectfile, '/'); - - if (p) - strcpy(++p, platform_expectfile); - } /* Name to use for temporary diff file */ snprintf(diff, sizeof(diff), "%s.diff", resultsfile); @@ -1489,33 +1272,6 @@ results_differ(const char *testname, const char *resultsfile, const char *defaul free(alt_expectfile); } - /* - * fall back on the canonical results file if we haven't tried it yet and - * haven't found a complete match yet. - */ - - if (platform_expectfile) - { - snprintf(cmd, sizeof(cmd), - "diff %s \"%s\" \"%s\" > \"%s\"", - basic_diff_opts, default_expectfile, resultsfile, diff); - - if (run_diff(cmd, diff) == 0) - { - /* No diff = no changes = good */ - unlink(diff); - return false; - } - - l = file_line_count(diff); - if (l < best_line_count) - { - /* This diff was a better match than the last one */ - best_line_count = l; - strlcpy(best_expect_file, default_expectfile, sizeof(best_expect_file)); - } - } - /* * Use the best comparison file to generate the "pretty" diff, which we * append to the diffs summary file. diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 27a4d131897..013018b6435 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -3416,7 +3416,6 @@ __time64_t _dev_t _ino_t _locale_t -_resultmap _stringlist access_vector_t acquireLocksOnSubLinks_context -- 2.51.2