-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
external_project: workaround for MinGW #13886
Conversation
22873d7
to
0fb6fa0
Compare
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 should really be reported to CPython as a regression.
a6e7e39
to
7ae4d8a
Compare
It seems recent ObjFW update broke an Objective-C++ test case on MinGW.
This pull request cannot be merged until the test case is fixed. |
|
7ae4d8a
to
63b7f8e
Compare
mingw-w64-clang-x86_64-libobjfw seems now fixed. |
63b7f8e
to
4d7de83
Compare
0a5a992
to
9165d0b
Compare
This is a quick fix AFK. Amended commit will be done tonight JST. |
OK thanks. For the commit message of the first commit I'd also recommend a slight reformatting tweak while you are amending anyway:
|
Looks like this change was on purpose: python/cpython#99031 since:
|
This has always been the case. So likely a Python behavior change as well. I'll try to figure out why. |
self.prefix = self.prefix.relative_to(self.prefix.drive) | ||
self.prefix = Path(relpath(self.prefix, self.prefix.drive)) | ||
|
||
# self.prefix is an absolute path, so we cannot append it to another path. | ||
self.rel_prefix = self.prefix.relative_to(self.prefix.root) | ||
self.rel_prefix = Path(relpath(self.prefix, self.prefix.root)) |
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.
Using relative_to(self.prefix.anchor)
should also work in both cases as far as I understand. https://docs.python.org/3/library/pathlib.html#pathlib.PurePath.anchor
As long as prefix is absolute.
ymmv
It's a regression introduced in python/cpython#103179 which makes shutil.which return paths that are not executable, from what I can tell. |
Sigh, as with all things pathlib nothing can be fixed because every bad behavior is intentional to be bad. Hence dropping back to os.path.relpath, oh well... :) |
I've filed a bug python/cpython#127001 though that doesn't help us here I guess. edit, this maybe?: import shutil
import os
windows_exts = ('exe', 'msc', 'com', 'bat', 'cmd')
def which_fixed(cmd, *args, **kwargs):
res = shutil.which(cmd, *args, **kwargs)
# Work around https://github.com/python/cpython/issues/127001
# for the common case at least
if res is not None and not os.path.splitext(res)[1]:
for ext in windows_exts:
res = shutil.which(cmd + '.' + ext, *args, **kwargs)
if res is not None:
break
return res
print(which_fixed("cmd")) |
On Python 3.12, self.prefix.relative_to(self.prefix.drive) no longer works and raises error like: ``` ValueError: 'C:/msys64/mingw64' is not in the subpath of 'C:' ```
In some reason, recent version of MSYS2 is shipped with /usr/bin/cmd which makes the test fail
57e2ab3
to
071135d
Compare
thanks @MihailJP |
This computation of prefix and rel_prefix was re-written in mesonbuild#13886 but it introduced another bug where the leading slash was missing. In addition drive root should have trailing slash, or it would use different path as base of relpath in some cases.
This computation of prefix and rel_prefix was re-written in mesonbuild#13886 but it introduced another bug where the leading slash was missing. In addition drive root should have trailing slash, or it would use different path as base of relpath in some cases.
On Python 3.12, self.prefix.relative_to(self.prefix.drive) no longer works and raises error like:
ValueError: 'C:/msys64/mingw64' is not in the subpath of 'C:'