-
-
Notifications
You must be signed in to change notification settings - Fork 335
WIP Fix up the ParseFlags regression test #4740
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
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 <[email protected]>
I added an earlier comment, but don't see it. Appveyor tests failed. There are two Is it worth continuing to wrestle with this and find a "portable enough" approach or do we just leave it broken if you happen to have installed Strawberry Perl (and thus skipped on Github CI)? Feel like the couple of hours I spent chasing my tail on this ended up as a waste. |
So this work is just to make it work with strawberry perl installed which is only on appveyor at this point? |
Strawberry = only github. And I did all this nice stuff and broke appveyor, because it has some ancient pkg-config that doesn't take the helpful option I chose to use. |
Just a thought, is there any chance that installing the I know it adds a pkg-config.exe to the python script directory which is earlier than the Strawberry perl directory (i.e., pkg-config.bat). I haven't gotten around to seeing why exactly the test failed with the python version yet. The changes for windows in this PR might be worth testing with python pkgconf it might be easier than working around strawberry perl. Edit: |
I think it's not worth the effort. If we have some test environments which test this functionality (which realistically is unlikely to be used in the field on windows.. ) then that's good enough. I wouldn't spend effort trying to get it working on windows.. |
Good. I didn't realize pkgconf requires 3.9+ anyway. |
It's just that the environment on the AppVeyor 2017 image is too old for the new approach. We could force an update in the msys environment - though I don't know how to do that, actually. |
Quick google result. Possibly out of luck as VS2017 may be too old. https://www.msys2.org/docs/ci/
|
I did suggest earlier that it's time to ditch the 2017 image there, but got pushback. I'm not sure if it's picking pkg-config from the mingw path or the msys path... or even the cygwin path (I had a debug print reporting this while I worked on the patch, but took it out before pushing). This is the top of the Path reported from that failing run (flipped the slashes for my own sanity):
|
Looks like
|
sigh. of course it says so in the test itself.... thanks. |
Off-topic: Windows runner 2022 and 2025 environment configuration diagnostics [WIP]. I have found the following diagnostic information useful when trying to figure out how the windows runner environments are configured. Still working dumping the powershell modules. The attached archive file contains hand-edited log files printing configuration diagnostics for the windows-2022 and windows-2025 runners. This is a work in progress and may not match the SCons test environment yet exactly. windows-runner-diagnostics-wip01.zip The
The
This started with the VS configuration information. It is possible to export the package configuration file and then use that configuration file to install packages on another machine. I haven't actually attempted to try to re-install yet. Unfortunately the export is done from the visual studio installer GUI and can't be done from the command-line without the vs bootstrapper executable. The bootstrapper executable is downloaded based on the installations detected and the vsconfig file is exported. An alternative method using the VSSetup powershell module appears to find all of the packages but does not seem to include installed extensions (which I don't really care about at the moment). Enjoy. |
The msys2 project has both |
VS2017+ comments. Starting with VS2017, there is no longer a "full" offline installer available (i.e., exe or iso). The vs bootstrapper is required for each "flavor" of install: enterprise, professional, community, buildtools, and express (2017 only). It appears that it is possible to use the vs bootstrapper to create an "offline" installation folder and install from the "offline" installation folder on another machine using the vs bootstrapper without connecting to the internet. The vs bootstrapper links for 2017 are no longer available directly from the MS visual studio pages. However, the direct download links appear to still be working. If one has an interest in installing VS2017 in the future for testing, creating an offline installation should probably be done sooner rather than later. |
no, msys2 installer. that's where pkg-config is coming from. |
? Unclear what this is referencing. |
my previous comment |
Just skip this test in vs2017 and let's move on? |
Temporarily marking this work-in-progress till I get the motivation to partially roll back changes so it covers the known CI situations as well as normal Linux dev. That still won't be perfect (no such thing), but should work for us. |
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.
Initiallly, have taken the test out of the skip list so can see if builds work everywhere other than my environment. If so, I will update with changelog info and other bookkeeping.
Contributor Checklist:
CHANGES.txt
andRELEASE.txt
(and read theREADME.rst
).