Add Library\mingw-w64 to Windows search path
#10643
Conversation
| sys_prefix / "Library" / "bin", | ||
| sys_prefix / "Library" / "lib", | ||
| sys_prefix / "Library" / "mingw-w64", | ||
| sys_prefix / "Library" / "mingw-w64" / "bin", |
There was a problem hiding this comment.
Could you please share under which condition libxgboost is on the bin path? Also, is mingw ever used for compiling the Python package? Asking since mingw has/had a broken Unix socket implementation, hence distributed computing is disabled there.
There was a problem hiding this comment.
Conda-forge uses Visual Studio to build the Python package. See the logs at https://github.com/conda-forge/xgboost-feedstock/runs/27951580159.
@jakirkham I am inclined to exclude this patch from the upstream because the use of mingw-w64 appears to be a convention specific to Conda-forge and not generally applicable to all users.
There was a problem hiding this comment.
If conda forge uses visual studio, maybe we can simply drop the patch from conda forge as well?
There was a problem hiding this comment.
No, Conda-forge needs this patch. Libraries are installed under the mingw-w64 directory. I tried removing it and got errors.
There was a problem hiding this comment.
Yes. The patch is a product of trial and error. I had to add it to get conda-forge/xgboost-feedstock#132 working.
There was a problem hiding this comment.
That's unexpected. Thank you for sharing. I'm fine with merging it then.
There was a problem hiding this comment.
Thank you both! 🙏
Co-authored-by: jakirkham <jakirkham@gmail.com>
3 participants
Fixes #5386
Fixes conda-forge/xgboost-feedstock#51
When searching for
libxgbooston Windows, this addsLibrary\mingw-w64to the search path after other pathsThis currently is done as a patch in conda-forge. Given this is just checking one more location (and after checking the usual ones), though it made sense to go ahead and upstream this one