-
-
Notifications
You must be signed in to change notification settings - Fork 384
CI(OSGeo4W): Upgrade image to windows-2022 #5562
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
Conversation
|
Merged main to see if #5601 of r.plane changes something to the r.plane failure. And also since some improvements to the stdbool handling. |
|
Note that the brownouts are starting in June 2025, (in three weeks), and the windows-2019 will be gone at in July, after 4 brownouts during June. |
|
I think I found a clue on what's going on. The tests that fail here (like that are using r.mapcalc), are failing with return code 3221225781, and a little google search brings this expanded at the top result:
So something is not finding the DLLs for r.mapcalc in the pytest step. |
|
I think I found it, it worked on a "dirty", mixed, PR on my fork. I'm trying out here just the minimum that worked once reverting piece by piece what wasn't needed. |
|
If this run fails, then it might mean #5717 was needed, it's one of the changes that I tried with but didn't try again without it. |
|
Once this would be ready to merge, we will need to add windows 2022 to the required checks, and after that, disable windows-2019 from the required checks. At this point, PRs won't have the required status check for the "windows-2019", so can't be merged. Then merge this PR. Then, PRs can be updated from main to be able to merge. Question: |
|
After rereading for an uncountable number of times the contents of the runner images of windows-2019 and windows-2022, today the note (present in both pages) that said that msys2 is preinstalled but not in PATH gave me a new idea to explore. I looked in the custom script "build_osgeo4w.sh", the one that diverges from the one used by osgeo4w, and tried changing three lines that used the c:\msys64 path (the location of the pre-installed one), and changed it to the custom location installed by us in the first steps. Then it worked. So, I removed the changes one by one, and only the one remaining here was needed. I then adapted it to use a env var to supply that folder, with the default value being the currently used value. |
wenzeslaus
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me and most of the tests still run.
I can change the required check from 2019 to 2022 for starters.
|
Ok, so you would merge as is? When are you ready for the switch? Would you want to change to not use a matrix strategy for the OS now, or at a later time ? And the same number of tests (files) run and pass, so no change here |
|
The Windows 2022 check is now mandatory and 2019 is not. @echoix I'll let you do the merge. Everyone, all PRs will need an update from main to pass the checks again after this PR is merged. Let's do the matrix change separately. As for timing, with the brownouts, it seems we need to switch now. Sorry, for the delay here. |
|
Update the PRs that need to be merged today. I'll more slowly update ones that are close to be ready later in the next evenings, when there is less CI activity, to not slow down work during the day. |
|
@neteler I think this need to be backported if we continue to commit on 8.4 |
Sure. I just tried but get
Shall I sync that file entirely to G84? |
|
Hmmm, were the patches that handles using a different package list+copying dlls applied in 8.4? If not, it should be only the relevant parts of this PR to apply, not the full workflow |
The windows-2019 image is being removed in less than a month. We need to use windows-2022. The changes needed in our build_osgeo4w.sh script relate to the msys2 paths added to path. They need to use the location defined in the CI step, otherwise, another version already installed in the runner, but normally not on path, is used. On windows-2019, we didn’t see errors of this misconfiguration, but we did for windows-2022. * CI(OSGeo4W): Upgrade image to windows-2022 * mswindows: Update path for dumpbin in mklibs.sh * windows: Use MSYS2_LOCATION env var to create script from templates in build_osgeo4w.sh * CI(OSGeo4W): Add MSYS2_LOCATION env var to build stage
The windows-2019 image is being removed in less than a month. We need to use windows-2022. The changes needed in our build_osgeo4w.sh script relate to the msys2 paths added to path. They need to use the location defined in the CI step, otherwise, another version already installed in the runner, but normally not on path, is used. On windows-2019, we didn’t see errors of this misconfiguration, but we did for windows-2022. * CI(OSGeo4W): Upgrade image to windows-2022 * mswindows: Update path for dumpbin in mklibs.sh * windows: Use MSYS2_LOCATION env var to create script from templates in build_osgeo4w.sh * CI(OSGeo4W): Add MSYS2_LOCATION env var to build stage
Closes #4554
When it will be time to merge this, enable this workflow to be a required check, then merge (as the Windows 2019 run won't have a result). Then other PRs would need to be updated.
There is a problem with r.mapcalc giving different weird return codes., making tests fail after reruns. Something isn't right, and I can't pin it down. I think it's the same reason I didn't create a PR here last year for that.
Help is needed.