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

Support case-sensitive windows directories #59

Closed

Conversation

Gudahtt
Copy link

@Gudahtt Gudahtt commented Aug 2, 2018

Checks for both upper-case and lower-case file extensions. This is to
support case-sensitive directories on Windows.

Mixed-case file extensions still won't work, but those are rare enough
that I don't think it's worth the performance cost to check those as
well.

Closes #57

Checks for both upper-case and lower-case file extensions. This is to
support case-sensitive directories on Windows.

Mixed-case file extensions still won't work, but those are rare enough
that I don't think it's worth the performance cost to check those as
well.

Closes npm#57
@Gudahtt
Copy link
Author

Gudahtt commented Aug 2, 2018

I didn't add tests for this fix, as I wasn't sure how. The per-directory case-sensitivity flag wasn't added until build 17093, which was fairly recent.

I tested it locally by editing the lines in test/basic.js where fixtures was deleted. I changed them to delete fixtures/foo.sh instead, leaving the directory. Then I created the fixtures directory and set the case-sensitivity flag on it using the command fsutil.exe file setCaseSensitiveInfo test/fixtures enable.

Once doing that, I added tests that would use process.env.PATHEXT (#58 ), and ran them. find when executable > with process.env.PATHEXT > foo was the failing test. It passed after making this change.

@Gudahtt
Copy link
Author

Gudahtt commented Aug 11, 2018

I'm tempted to change this to use fs.readdir instead, but I'm not sure that would be a good idea. It would make this work for mixed-case file extensions, and would probably be faster for small directories. But for large directories (i.e. containing many files), this would be slower and would consume more memory.

@danielgary
Copy link

Are there any plans to merge this PR? This is a hangup on npx and yarn.

caitlinelfring added a commit to get-woke/vscode-woke that referenced this pull request Oct 23, 2020
Needed support for case-sensitive file extensions, in unmerged PR npm/node-which#59
caitlinelfring added a commit to get-woke/vscode-woke that referenced this pull request Oct 23, 2020
Port npm/which to typescript

Needed support for case-sensitive file extensions, in unmerged PR npm/node-which#59
@lukekarrys lukekarrys requested a review from a team as a code owner November 1, 2022 18:36
@wraithgar
Copy link
Member

#100

@wraithgar wraithgar closed this May 1, 2023
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.

Doesn't find executables in case-sensitive directory on Windows
3 participants