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

tzinfo patch broken on windows #150

Open
1 task done
apmorton opened this issue Aug 20, 2024 · 3 comments
Open
1 task done

tzinfo patch broken on windows #150

apmorton opened this issue Aug 20, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@apmorton
Copy link

Solution to issue cannot be found in the documentation.

  • I checked the documentation.

Issue

The tzinfo patch (https://github.com/conda-forge/ctng-compilers-feedstock/blob/main/recipe/patches/0003-patch-zoneinfo_dir_override-to-point-to-our-tzdata.patch) calls GetModuleFileNameW and memcpy's the result into a char*.

This is an incorrect conversion from wchar_t to char that results in the patch incorrectly returning only the first character (which happens to be the drive letter).

Installed packages

N/A

Environment info

N/A
@apmorton apmorton added the bug Something isn't working label Aug 20, 2024
@h-vetinari
Copy link
Member

Thanks for the report. Could you open a PR that fixes it, or suggest how the implementation should look like? If you have a package/feedstock that exercises this code path, we could also add it as a downstream test here in some form.

@apmorton
Copy link
Author

I discovered this while working on: https://github.com/conda-forge/howardhinnant_date-feedstock/blob/main/recipe/patches/0001-Locate-zoneinfo-based-on-install-prefix.patch

The "fix" I applied in that patch was to call GetModuleFileNameA. That seemed appropriate in that library since it didn't really look like any of the downstream code would correctly handle wchar paths anyway, but that may not be an appropriate fix for the STL.

I'm also not sure of the context in which libstdc++ is used on win32 in conda-forge?

@h-vetinari
Copy link
Member

I'm also not sure of the context in which libstdc++ is used on win32 in conda-forge?

We just introduced tzdb support (#142), and also the mingw-stack is pretty new (libstcxx is not our default C++ stdlib on windows; in fact, it's not ABI-compatible with the STL, so we have to be very careful to use it).

That said, if it's just renaming the function, then that's a very easy fix; you can even rename it directly in the patch (+ increment the build number).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants