From 643b39e9d00106caea9fac2d6908369c06f1a3af Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Fri, 4 Jul 2025 08:52:41 -0600 Subject: [PATCH 1/3] Fix up the ParseFlags regression test This was failing on a GitHub Windows Runner, for obscure reasons. Reworked a bunch to be more general. Dropped the whole dance about getting an ordered dict, since with the current base Python version you get that for free. Signed-off-by: Mats Wichmann --- test/CPPDEFINES/pkg-config.py | 103 +++++++++++++++++++++------------ testing/ci/windows_ci_skip.txt | 1 - 2 files changed, 66 insertions(+), 38 deletions(-) diff --git a/test/CPPDEFINES/pkg-config.py b/test/CPPDEFINES/pkg-config.py index 22fd7bbfa6..1375d62d5a 100644 --- a/test/CPPDEFINES/pkg-config.py +++ b/test/CPPDEFINES/pkg-config.py @@ -25,8 +25,27 @@ """ Verify merging with MergeFlags to CPPPDEFINES with various data types. + +Uses system pkg-config with a stub pc file to generate the data. +Not entirely sure the motivation for this - it would be more "isolated" +to just paste in the data and not depend on pkg-config, but presumably +this test exists because of observed bugs, so leaving alone for now. + +Ran into an interesting problem when we spun up a GitHub Windows Action: +it finds pre-installed Strawberry Perl's pkg-config over the mingw one, +which is also pre-installed. The Strawberry one is written in pure +Perl (after getting past the initial batch file), so requires perl.exe, +which won't be in the SCons env['ENV']['PATH']. We try to detect it, +by somewhat hokey means: if pkg-config's path had "strawberry' in it, +assume we need to add the directory path to our execution env. + +It ended up being more portable to use pkg-config's --with-path than +to set PKG_CONFIG_PATH, because the Windows cmd.exe can't accept +the standard calling style. """ +import pathlib + import TestSCons import TestCmd @@ -34,9 +53,17 @@ pkg_config_path = test.where_is('pkg-config') if not pkg_config_path: - test.skip_test("Could not find 'pkg-config' in system PATH, skipping test.\n") + test.skip_test("Could not find 'pkg-config' in PATH, skipping test.\n") pkg_config_path = pkg_config_path.replace("\\", "/") +# Try to guess if this was Strawberry Perl's pkg-config +if 'strawberry' in pkg_config_path.lower(): + # as_posix() or the pasted paths in SConstruct will have escape problems + # (or need to be raw strings) + strawberry_perl_path = pathlib.PurePath(pkg_config_path).parent.as_posix() +else: + strawberry_perl_path = "" + test.write('bug.pc', """\ prefix=/usr exec_prefix=${prefix} @@ -57,72 +84,74 @@ """) if TestCmd.IS_WINDOWS: - pkg_config_file = 'bug.pc' + pkg_config_file = 'bug' pkg_config_tools = 'mingw' - pkg_config_cl_path = "" else: - pkg_config_file = 'bug' + pkg_config_file = 'bug.pc' pkg_config_tools = 'default' - pkg_config_cl_path = "PKG_CONFIG_PATH=." +pkg_config_cl_path = '--with-path=.' -test.write('SConstruct', """\ +test.write('SConstruct', f"""\ import os import sys -# Python3 dicts dont preserve order. Hence we supply subclass of OrderedDict -# whose __str__ and __repr__ act like a normal dict. -from collections import OrderedDict - -class OrderedPrintingDict(OrderedDict): - def __repr__(self): - return '{' + ', '.join(['%r: %r' % (k, v) for (k, v) in self.items()]) + '}' - - __str__ = __repr__ - - # Because dict-like objects (except dict and UserDict) are not deep copied - # directly when constructing Environment(CPPDEFINES = OrderedPrintingDict(...)) - def __semi_deepcopy__(self): - return self.copy() -""" + """ +DefaultEnvironment(tools=[]) # https://github.com/SCons/scons/issues/2671 # Passing test cases -env_1 = Environment(CPPDEFINES=[('DEBUG', '1'), 'TEST'], tools=['%(pkg_config_tools)s']) +env_1 = Environment( + CPPDEFINES=[('DEBUG', '1'), 'TEST'], + tools=['{pkg_config_tools}'] +) +if '{strawberry_perl_path}': + env_1.AppendENVPath('PATH', '{strawberry_perl_path}') if sys.platform == 'win32': os.environ['PKG_CONFIG_PATH'] = env_1.Dir('.').abspath.replace("\\\\", "/") -env_1.ParseConfig( - '%(pkg_config_cl_path)s "%(pkg_config_path)s" --cflags %(pkg_config_file)s' -) +env_1.ParseConfig('"{pkg_config_path}" {pkg_config_cl_path} --cflags {pkg_config_file}') print(env_1.subst('$_CPPDEFFLAGS')) -env_2 = Environment(CPPDEFINES=[('DEBUG', '1'), 'TEST'], tools=['%(pkg_config_tools)s']) +env_2 = Environment( + CPPDEFINES=[('DEBUG', '1'), 'TEST'], + tools=['{pkg_config_tools}'] +) +if '{strawberry_perl_path}': + env_2.AppendENVPath('PATH', '{strawberry_perl_path}') env_2.MergeFlags('-DSOMETHING -DVARIABLE=2') print(env_2.subst('$_CPPDEFFLAGS')) # Failing test cases env_3 = Environment( - CPPDEFINES=OrderedPrintingDict([('DEBUG', 1), ('TEST', None)]), - tools=['%(pkg_config_tools)s'], -) -env_3.ParseConfig( - '%(pkg_config_cl_path)s "%(pkg_config_path)s" --cflags %(pkg_config_file)s' + CPPDEFINES=dict([('DEBUG', 1), ('TEST', None)]), + tools=['{pkg_config_tools}'], ) +if '{strawberry_perl_path}': + env_3.AppendENVPath('PATH', '{strawberry_perl_path}') +env_3.ParseConfig('"{pkg_config_path}" {pkg_config_cl_path} --cflags {pkg_config_file}') print(env_3.subst('$_CPPDEFFLAGS')) env_4 = Environment( - CPPDEFINES=OrderedPrintingDict([('DEBUG', 1), ('TEST', None)]), - tools=['%(pkg_config_tools)s'], + CPPDEFINES=dict([('DEBUG', 1), ('TEST', None)]), + tools=['{pkg_config_tools}'], ) +if '{strawberry_perl_path}': + env_4.AppendENVPath('PATH', '{strawberry_perl_path}') env_4.MergeFlags('-DSOMETHING -DVARIABLE=2') print(env_4.subst('$_CPPDEFFLAGS')) # https://github.com/SCons/scons/issues/1738 -env_1738_1 = Environment(tools=['%(pkg_config_tools)s']) +# TODO: the Perl pkg-config doesn't work right with both --cflags --libs +# e.g.: pkg-config --with-path=. --libs --cflags bug +# Exepct: -DSOMETHING -DVARIABLE=2 +# Strawberry: (blank) +# We don't have any libs in the stub pc file, so just drop that for now. +env_1738_1 = Environment(tools=['{pkg_config_tools}']) +if '{strawberry_perl_path}': + env_1738_1.AppendENVPath('PATH', '{strawberry_perl_path}') env_1738_1.ParseConfig( - '%(pkg_config_cl_path)s "%(pkg_config_path)s" --cflags --libs %(pkg_config_file)s' + '"{pkg_config_path}" {pkg_config_cl_path} --cflags {pkg_config_file}' ) -env_1738_1.Append(CPPDEFINES={'value': '1'}) +env_1738_1.Append(CPPDEFINES={{'value': '1'}}) print(env_1738_1.subst('$_CPPDEFFLAGS')) -"""%locals() ) +""") expect_print_output="""\ -DDEBUG=1 -DTEST -DSOMETHING -DVARIABLE=2 diff --git a/testing/ci/windows_ci_skip.txt b/testing/ci/windows_ci_skip.txt index 6f96386f5a..73566c3a55 100644 --- a/testing/ci/windows_ci_skip.txt +++ b/testing/ci/windows_ci_skip.txt @@ -1,5 +1,4 @@ # temporarily skip this in GitHub Windows runner -test/CPPDEFINES/pkg-config.py test/packaging/msi/explicit-target.py test/packaging/msi/file-placement.py test/packaging/msi/package.py From d1d134da2ef47a2cca9f53e031c7fcc692e984ba Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Mon, 23 Feb 2026 11:56:25 -0700 Subject: [PATCH 2/3] Use a mocked pkg-config in test Signed-off-by: Mats Wichmann --- test/CPPDEFINES/pkg-config.py | 119 +++++++++------------------------- 1 file changed, 30 insertions(+), 89 deletions(-) diff --git a/test/CPPDEFINES/pkg-config.py b/test/CPPDEFINES/pkg-config.py index 1375d62d5a..68f332fe2c 100644 --- a/test/CPPDEFINES/pkg-config.py +++ b/test/CPPDEFINES/pkg-config.py @@ -26,70 +26,42 @@ """ Verify merging with MergeFlags to CPPPDEFINES with various data types. -Uses system pkg-config with a stub pc file to generate the data. -Not entirely sure the motivation for this - it would be more "isolated" -to just paste in the data and not depend on pkg-config, but presumably -this test exists because of observed bugs, so leaving alone for now. - -Ran into an interesting problem when we spun up a GitHub Windows Action: -it finds pre-installed Strawberry Perl's pkg-config over the mingw one, -which is also pre-installed. The Strawberry one is written in pure -Perl (after getting past the initial batch file), so requires perl.exe, -which won't be in the SCons env['ENV']['PATH']. We try to detect it, -by somewhat hokey means: if pkg-config's path had "strawberry' in it, -assume we need to add the directory path to our execution env. - -It ended up being more portable to use pkg-config's --with-path than -to set PKG_CONFIG_PATH, because the Windows cmd.exe can't accept -the standard calling style. +Now uses a mocked pkg-config to avoid some weird problems, +but is still a "live" test because we do expansion on $_CPPDEFFLAGS, +which needs to have been set up with a compiler tool so have something +to subst into. """ import pathlib +import sys import TestSCons import TestCmd test = TestSCons.TestSCons() +_python_ = TestSCons._python_ -pkg_config_path = test.where_is('pkg-config') -if not pkg_config_path: - test.skip_test("Could not find 'pkg-config' in PATH, skipping test.\n") -pkg_config_path = pkg_config_path.replace("\\", "/") - -# Try to guess if this was Strawberry Perl's pkg-config -if 'strawberry' in pkg_config_path.lower(): - # as_posix() or the pasted paths in SConstruct will have escape problems - # (or need to be raw strings) - strawberry_perl_path = pathlib.PurePath(pkg_config_path).parent.as_posix() -else: - strawberry_perl_path = "" - -test.write('bug.pc', """\ -prefix=/usr -exec_prefix=${prefix} -libdir=${exec_prefix}/lib -includedir=${prefix}/include - -Name: bug -Description: A test case .pc file -Version: 1.2 -Cflags: -DSOMETHING -DVARIABLE=2 +mock_pkg_config = test.workpath('mock-pkg-config.py') +test.write(mock_pkg_config, f"""\ +import sys +if '--cflags' in sys.argv: + print("-DSOMETHING -DVARIABLE=2") +sys.exit(0) """) -test.write('main.c', """\ -int main(int argc, char *argv[]) -{ - return 0; -} -""") +# Since the mock pkg-config is a Python script, use the selected Python to +# run it to avoid execution problems (esp. on Windows) +pc_path = (_python_ + ' ' + mock_pkg_config).replace("\\", "/") +# We need a toolset: if TestCmd.IS_WINDOWS: - pkg_config_file = 'bug' - pkg_config_tools = 'mingw' + pc_tools = 'mingw' else: - pkg_config_file = 'bug.pc' - pkg_config_tools = 'default' -pkg_config_cl_path = '--with-path=.' + pc_tools = 'default' + +# these are no longer really needed, leave so it looks like a "real" pkg-config +pc_file = 'bug' +pc_cl_path = "" test.write('SConstruct', f"""\ import os @@ -98,57 +70,26 @@ DefaultEnvironment(tools=[]) # https://github.com/SCons/scons/issues/2671 # Passing test cases -env_1 = Environment( - CPPDEFINES=[('DEBUG', '1'), 'TEST'], - tools=['{pkg_config_tools}'] -) -if '{strawberry_perl_path}': - env_1.AppendENVPath('PATH', '{strawberry_perl_path}') -if sys.platform == 'win32': - os.environ['PKG_CONFIG_PATH'] = env_1.Dir('.').abspath.replace("\\\\", "/") -env_1.ParseConfig('"{pkg_config_path}" {pkg_config_cl_path} --cflags {pkg_config_file}') +env_1 = Environment(CPPDEFINES=[('DEBUG', '1'), 'TEST'], tools=['{pc_tools}']) +env_1.ParseConfig('{pc_path} {pc_cl_path} --cflags {pc_file}') print(env_1.subst('$_CPPDEFFLAGS')) -env_2 = Environment( - CPPDEFINES=[('DEBUG', '1'), 'TEST'], - tools=['{pkg_config_tools}'] -) -if '{strawberry_perl_path}': - env_2.AppendENVPath('PATH', '{strawberry_perl_path}') +env_2 = Environment(CPPDEFINES=[('DEBUG', '1'), 'TEST'], tools=['{pc_tools}']) env_2.MergeFlags('-DSOMETHING -DVARIABLE=2') print(env_2.subst('$_CPPDEFFLAGS')) # Failing test cases -env_3 = Environment( - CPPDEFINES=dict([('DEBUG', 1), ('TEST', None)]), - tools=['{pkg_config_tools}'], -) -if '{strawberry_perl_path}': - env_3.AppendENVPath('PATH', '{strawberry_perl_path}') -env_3.ParseConfig('"{pkg_config_path}" {pkg_config_cl_path} --cflags {pkg_config_file}') +env_3 = Environment(CPPDEFINES=dict([('DEBUG', 1), ('TEST', None)]), tools=['{pc_tools}']) +env_3.ParseConfig('{pc_path} {pc_cl_path} --cflags {pc_file}') print(env_3.subst('$_CPPDEFFLAGS')) -env_4 = Environment( - CPPDEFINES=dict([('DEBUG', 1), ('TEST', None)]), - tools=['{pkg_config_tools}'], -) -if '{strawberry_perl_path}': - env_4.AppendENVPath('PATH', '{strawberry_perl_path}') +env_4 = Environment(CPPDEFINES=dict([('DEBUG', 1), ('TEST', None)]), tools=['{pc_tools}']) env_4.MergeFlags('-DSOMETHING -DVARIABLE=2') print(env_4.subst('$_CPPDEFFLAGS')) # https://github.com/SCons/scons/issues/1738 -# TODO: the Perl pkg-config doesn't work right with both --cflags --libs -# e.g.: pkg-config --with-path=. --libs --cflags bug -# Exepct: -DSOMETHING -DVARIABLE=2 -# Strawberry: (blank) -# We don't have any libs in the stub pc file, so just drop that for now. -env_1738_1 = Environment(tools=['{pkg_config_tools}']) -if '{strawberry_perl_path}': - env_1738_1.AppendENVPath('PATH', '{strawberry_perl_path}') -env_1738_1.ParseConfig( - '"{pkg_config_path}" {pkg_config_cl_path} --cflags {pkg_config_file}' -) +env_1738_1 = Environment(tools=['{pc_tools}']) +env_1738_1.ParseConfig('{pc_path} {pc_cl_path} --cflags {pc_file}') env_1738_1.Append(CPPDEFINES={{'value': '1'}}) print(env_1738_1.subst('$_CPPDEFFLAGS')) """) From 8afa81a05feb0c9ca24901ec9b80bc4a8d5dbe09 Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Mon, 23 Feb 2026 12:36:30 -0700 Subject: [PATCH 3/3] Add changelog for pkg-config test [skip ci] Signed-off-by: Mats Wichmann --- CHANGES.txt | 1 + RELEASE.txt | 2 ++ 2 files changed, 3 insertions(+) diff --git a/CHANGES.txt b/CHANGES.txt index a6963f268b..066f9bb5ad 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -60,6 +60,7 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER - Additional small tweaks to Environment.py type hints, fold some overly long function signature lines, and some linting-insipired cleanups. - Test framework: tweak module docstrings + - Fix CPPDEFINES test using pkg-config, which was failing on GH Actions. RELEASE 4.10.1 - Sun, 16 Nov 2025 10:51:57 -0700 diff --git a/RELEASE.txt b/RELEASE.txt index 571cc35d17..aca82836c3 100644 --- a/RELEASE.txt +++ b/RELEASE.txt @@ -38,6 +38,8 @@ FIXES - Fix --debug=includes for case of multiple source files. +- Fix CPPDEFINES test using pkg-config, which was failing on GH Actions. + IMPROVEMENTS ------------