dependencies: find extraframeworks on case-sensitive filesystems #13038
Conversation
reckenrode
force-pushed
the
extraframeworks-casing
branch
from
365a827
to
c11da42
Compare
|
I fixed the mypy lint failure. |
mesonbuild/dependencies/framework.py
Outdated
| p = Path(path) | ||
| lname = name.lower() | ||
| for d in p.glob('*.framework/'): | ||
| if lname == d.name.rsplit('.', 1)[0].lower(): | ||
| return d | ||
| framework_name = d.name.rsplit('.', 1)[0] |
There was a problem hiding this comment.
wouldn't be better to use d.stem here ?
There was a problem hiding this comment.
I implemented your other suggestion, so this change could be dropped completely, but I converted it to stem in my update. Let me know if I should drop it to keep the size of the diff down.
mesonbuild/dependencies/framework.py
Outdated
| @@ -70,12 +77,13 @@ def detect(self, name: str, paths: T.List[str]) -> None: | |||
| self.is_found = True | |||
| return | |||
|
|
|||
| def _get_framework_path(self, path: str, name: str) -> T.Optional[Path]: | |||
| def _get_framework_path(self, path: str, name: str) -> T.Optional[tuple[Path, str]]: | |||
There was a problem hiding this comment.
You should use T.Tuple here, because of older versions of Python
mesonbuild/dependencies/framework.py
Outdated
| @@ -70,12 +77,13 @@ def detect(self, name: str, paths: T.List[str]) -> None: | |||
| self.is_found = True | |||
| return | |||
|
|
|||
| def _get_framework_path(self, path: str, name: str) -> T.Optional[Path]: | |||
| def _get_framework_path(self, path: str, name: str) -> T.Optional[tuple[Path, str]]: | |||
There was a problem hiding this comment.
I think you should either rename the function to _get_framework_path_and_name, or return a single path like d / framework_name and use path.name in the caller
There was a problem hiding this comment.
I ended up using path.stem on the returned path.
bruchar1
added
the
OS:macos
Fixes a test failure on case-sensitive filesystems when a CMake dependency is turned into an Apple framework.
reckenrode
force-pushed
the
extraframeworks-casing
branch
from
c11da42
to
a6fb2c1
Compare
|
ci failures seem irrelevant |
|
Is there anything else needing to be done to move this forward after approval? |
|
We are currently in RC freeze so only regression bugfixes are permitted. |
This PR fixes a test failure on case-sensitive filesystems when a CMake dependency is turned into an Apple framework. I ran into this issue while working on the Darwin stdenv in nixpkgs after Meson was updated to 1.4.0 because my Nix store is installed on a case-sensitive APFS volume.
The extraframeworks search logic was already case insensitive. The fix was to use the match’s actual name instead of the one that was requested. I included a test, though the original test for #12181 also serves as a test for case-sensitive filesystems.
I found #12480 in the issue tracker, but the scope of fixing that seems like a much larger fix and wouldn’t address the issue I encountered building the Darwin stdenv in nixpkgs.