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

Fix the wheel target for case insensitive file systems #1080

Merged
merged 2 commits into from
Dec 3, 2023
Merged

Fix the wheel target for case insensitive file systems #1080

merged 2 commits into from
Dec 3, 2023

Conversation

ofek
Copy link
Collaborator

@ofek ofek commented Dec 3, 2023

Resolves #1054

@jeanas
Copy link

jeanas commented Dec 3, 2023

Thanks! Let me test it...

Copy link
Contributor

github-actions bot commented Dec 3, 2023

Code Coverage

Package Statements
hatch 96.16% (4864 / 5058)
hatchling 97.65% (3745 / 3835)
tests 99.95% (14707 / 14713)
Summary 98.77% (23316 / 23606)

@staticmethod
def get_raw_fs_path_name(directory: str, name: str) -> str:
normalized = name.casefold()
entries = os.listdir(directory)
Copy link

@jeanas jeanas Dec 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Friendly tip: maybe you want to limit this to platform.system() == "Darwin"? In Pygments, hatchling is ~2× faster to build a wheel than setuptools, and I suspect this is in part because it saves lots of syscalls for listdir() in our huge test/ tree. Now, the way this function is used right now, this is not very likely to matter a great deal because I'd expect the package directory not to have hundreds of files, unlike some test directories, but still...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also applies to Windows, sure I will not do this on Linux! Before I do however, did this fix it for you?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it seems to be working on the macOS machine I have access to.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thank you!

@ofek ofek merged commit 2137d89 into master Dec 3, 2023
38 checks passed
@ofek ofek deleted the b branch December 3, 2023 23:10
github-actions bot pushed a commit that referenced this pull request Dec 3, 2023
* Fix the `wheel` target for case insensitive file systems

* address feedback 2137d89
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.

Simple configuration with capitalized package name fails on macOS
2 participants