Skip to content

module: fix stat with long paths on windows#2011

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.

Sign up for GitHub

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

Jump to bottom
Closed
Fishrock123 wants to merge 1 commit into from
Closed

module: fix stat with long paths on windows #2011

Fishrock123 wants to merge 1 commit into from
+1 −1

Conversation

@Fishrock123
Copy link
Contributor

@Fishrock123Fishrock123 commented Jun 18, 2015

See https://github.com/nodejs/io.js/pull/1920/files#r32701699, this also requires the long path fix from #1991

@Fishrock123Fishrock123 added the module Issues and PRs related to the module subsystem. label Jun 18, 2015
@Fishrock123Fishrock123 mentioned this pull request Jun 18, 2015
Closed
@bnoordhuis
Copy link
Member

bnoordhuis commented Jun 18, 2015

LGTM but a regression test would be nice.

@targos
Copy link
Member

targos commented Jun 18, 2015

this test I added in #1991 may be enough. I can try when I am back home.

@targos
Copy link
Member

targos commented Jun 18, 2015

or start a CI on master ? It will fail if I am right

@ChALkeR
Copy link
Member

ChALkeR commented Jun 18, 2015

I still think that it might be better to wrap internalModuleStat and internalModuleReadFile…

@Fishrock123
Copy link
ContributorAuthor

Fishrock123 commented Jun 18, 2015

CI on master: https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/47/

@targos
Copy link
Member

targos commented Jun 18, 2015

@ChALkeR I agree. We can introduce internal/fs to expose the wrapped functions

@mscdexmscdex added the windows Issues and PRs related to the Windows platform. label Jun 18, 2015
@Fishrock123
Copy link
ContributorAuthor

Fishrock123 commented Jun 18, 2015

CI on master does not appear to catch it?

@ChALkeRChALkeR mentioned this pull request Jun 18, 2015
Closed
@targos
Copy link
Member

targos commented Jun 18, 2015

apparently not 😞

@vkurchatkin
Copy link
Contributor

vkurchatkin commented Jun 18, 2015

I still think that it might be better to wrap internalModuleStat and internalModuleReadFile…

+1

@targostargos mentioned this pull request Jun 18, 2015
Closed
@Fishrock123
Copy link
ContributorAuthor

Fishrock123 commented Jun 19, 2015

#2013

@gr00ndgr00nd mentioned this pull request Feb 6, 2024
Open
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

moduleIssues and PRs related to the module subsystem.windowsIssues and PRs related to the Windows platform.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@Fishrock123@bnoordhuis@targos@ChALkeR@vkurchatkin@mscdex