Skip to content
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

mingw-w64: Update to 03d8a40f57 #22907

Merged
merged 1 commit into from
Dec 26, 2024
Merged

Conversation

lazka
Copy link
Member

@lazka lazka commented Dec 26, 2024

mingw-w64/mingw-w64@a2d1921...03d8a40

drop the revert since that should be fixed via mingw-w64/mingw-w64@c9eca28

@lazka lazka force-pushed the mingw-w64-03d8a40f57 branch from 438b3d7 to 6aec481 Compare December 26, 2024 11:30
@lazka lazka merged commit 4a8935a into msys2:master Dec 26, 2024
7 checks passed
@rouault
Copy link
Contributor

rouault commented Dec 28, 2024

I strongly believe this update has caused a regression in the GDAL regression test suite.
Last good build in https://github.com/OSGeo/gdal/actions/runs/12505586623/job/34889173672 used git-12.0.0.r446.ga2d19218d-1 and first failed in https://github.com/OSGeo/gdal/actions/runs/12515627580/job/34913475983 uses -git-12.0.0.r458.g03d8a40f5-1
One failure is about long filenames starting with \\?\ (not sure which calls to the Windows API has regressed there)
The other failure is more straightforward. It is about a failure renaming "tmp/yy\u4E2D\u6587.\u4E2D\u6587" on top of itself using _wrename()

@Biswa96
Copy link
Member

Biswa96 commented Dec 29, 2024

Could you provide any minimal sample code to reproduce the issue?

@lazka
Copy link
Member Author

lazka commented Dec 29, 2024

just to confirm, looking at the build logs the only package versions differences are due to this mingw-w64 change, so seems likely.

@rouault
Copy link
Contributor

rouault commented Dec 29, 2024

I figured the root cause. With git-12.0.0.r446.ga2d19218d-1, the below code prints 0x700, whereas with >= git-12.0.0.r458.g03d8a40f5-1, it prints 0x600. The GDAL code base has code paths that require __MSVCRT_VERSION__ >= 0x601 to use some functions of the Win32 API

test.c:

#include <windows.h>
#include <stdio.h>

int main()
{
    printf("0x%X\n", __MSVCRT_VERSION__);
    return 0;
}

old working mingw64 install:

evenr@DESKTOP-J297O1T MINGW64 /c/dev
$ pacman -Q | grep git
mingw-w64-x86_64-crt-git 12.0.0.r369.g0d4221712-2
mingw-w64-x86_64-headers-git 12.0.0.r369.g0d4221712-2
mingw-w64-x86_64-libmangle-git 12.0.0.r369.g0d4221712-1
mingw-w64-x86_64-libwinpthread-git 12.0.0.r369.g0d4221712-1
mingw-w64-x86_64-tools-git 12.0.0.r369.g0d4221712-1
mingw-w64-x86_64-winpthreads-git 12.0.0.r369.g0d4221712-1
mingw-w64-x86_64-winstorecompat-git 12.0.0.r369.g0d4221712-1

evenr@DESKTOP-J297O1T MINGW64 /c/dev
$ gcc test.c -o test.exe

evenr@DESKTOP-J297O1T MINGW64 /c/dev
$ ./test.exe
0x700

latest mingw64 install:

evenr@DESKTOP-J297O1T MINGW64 /c/dev
$ pacman -Q | grep git
mingw-w64-x86_64-crt-git 12.0.0.r459.g63f3f2846-1
mingw-w64-x86_64-headers-git 12.0.0.r459.g63f3f2846-1
mingw-w64-x86_64-libwinpthread-git 12.0.0.r459.g63f3f2846-1
mingw-w64-x86_64-winpthreads-git 12.0.0.r459.g63f3f2846-1

evenr@DESKTOP-J297O1T MINGW64 /c/dev
$ gcc test.c -o test.exe

evenr@DESKTOP-J297O1T MINGW64 /c/dev
$ ./test.exe
0x600

@rouault
Copy link
Contributor

rouault commented Dec 29, 2024

ok, so this seems to be related to commit mingw-w64/mingw-w64@90d3f33
Probably that the build recipe here should use --with-default-msvcrt=msvcr70 to restore the previous behavior ?

@lazka
Copy link
Member Author

lazka commented Dec 29, 2024

ok, so this seems to be related to commit mingw-w64/mingw-w64@90d3f33

Here is the discussion behind it: https://sourceforge.net/p/mingw-w64/mailman/mingw-w64-public/thread/20241018181538.bahgwxwolvx2n6cj%40pali/#msg58829839

Probably that the build recipe here should use --with-default-msvcrt=msvcr70 to restore the previous behavior ?

That would break the mingw-w64 internal checks then, since we are not using msvcr70. We could revert that linked commit temporarily, but removing the checks for __MSVCRT_VERSION__ on your side, as you did in the PR, is probably the best way forward.

rouault added a commit to OSGeo/gdal that referenced this pull request Dec 29, 2024
…ssuming this is always available given how antiquated this is

Workaround issue of msys2/MINGW-packages#22907 (comment)
@rouault
Copy link
Contributor

rouault commented Dec 29, 2024

but removing the checks for __MSVCRT_VERSION__ on your side, as you did in the PR, is probably the best way forward.

ok, I'll stick with that then since it doesn't seem to have obvious inconvenience (if one excepts the fact that current released versions of GDAL would be slightly broken when built against current mingw-w64, until our next release, but I guess that's life)

thanks for your assistance

@lazka
Copy link
Member Author

lazka commented Jan 3, 2025

There is some talk upstream to maybe move the default to 0x06FF instead, to avoid issues like this.

I've also created boostorg/align#21 for boost to remove some old checks as well there.

@ognevny
Copy link
Collaborator

ognevny commented Jan 3, 2025

the repo seems abandoned, latest commit was created 2 years ago :(

boxerab pushed a commit to GrokImageCompression/gdal that referenced this pull request Jan 5, 2025
…ssuming this is always available given how antiquated this is

Workaround issue of msys2/MINGW-packages#22907 (comment)
boxerab pushed a commit to GrokImageCompression/gdal that referenced this pull request Jan 7, 2025
…ssuming this is always available given how antiquated this is

Workaround issue of msys2/MINGW-packages#22907 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants