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

Fixed RangeError: Maximum call stack size exceeded for Windows #407

Closed
wants to merge 3 commits into from

Conversation

royling
Copy link

@royling royling commented Nov 15, 2013

The same issue was posted and discussed in google maillist, but not fixed.
Root cause: file name on Windows is case insensitive.

The same issue was posted in google maillist, but not fixed.
https://groups.google.com/forum/#!msg/yeoman-dev/TveRxapwM0U/KEvzulKkQ6oJ.
The root cause is that file name on Windows is case insensitive.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) when pulling 8a8dc98 on RoyLING:patch-1 into c77412f on yeoman:master.

@addyosmani
Copy link
Member

In our experience most of the problems here come down to insufficient permissions (at least, the issue linked to on the list had this as a core problem). Are you running into this when logged in as an admin?

@royling
Copy link
Author

royling commented Nov 16, 2013

@addyosmani Initially the problem was encountered when running any yo command, I googled the problem and found the related discussion in the googlegroup maillist (closed now, so I cannot reopen the discussion by replying to it). There the final solution was to do the things in Git bash client instead of windows cmd / powershell (I didn't try this yet). But I think this is only a workaround, not solution (that's why I made the change and sent the pull request), so I dived into the code and did some debugging, figured out that the basedir and path.resolve('/') were actually refer to the same path, but 2 string values in different case (in my case, basedir is in lower case (ie. c:/), while the other is in upper case), then it falled into the infinite recursions, resulted to the RangeError.
I don't think this problem has anything to do with the permission. On windows, it's not allowed to create 2 files/folders with names only in different case (e.g. mkdir tmp vs. mkdir Tmp), unlike linux.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) when pulling 05fe8a5 on RoyLING:patch-1 into c77412f on yeoman:master.

@SBoudrias
Copy link
Member

This is weird because I use windows a lot and I never got this bug.

Is it possible in Linux to create two directory with different cases? e.g. mkdir Hey && mkdir hey - it is not possible on mac.

I wouldn't mind adding a check, but the proposed solution is really not expressive.

@royling
Copy link
Author

royling commented Nov 16, 2013

@SBoudrias It's possible to have 2 dirs only with different cases in Linux...
Are you running yo command in windows powershell or cmd? I got the error even when running yo --help.

@sindresorhus
Copy link
Member

Let's fix this. We don't want to care about case-sensitive paths.

@SBoudrias
Copy link
Member

Agreed, just compare both path to lower case without any OS check. And squash your commits together after.

@royling
Copy link
Author

royling commented Nov 17, 2013

I did run yo cmd in node v0.11.x and yesterday I learned that yeoman is recommended to work on node stable version (0.10). Is this true? Anyway, I will downgrade node and see if the problem still exists or not.

@royling
Copy link
Author

royling commented Nov 17, 2013

Update: With node v0.10.22, the error was not encountered, yo works well. So for now, I think we could discard the change.

@royling royling closed this Nov 17, 2013
@royling royling deleted the patch-1 branch January 31, 2015 01:35
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.

None yet

5 participants