Skip to content
This repository has been archived by the owner on Sep 1, 2022. It is now read-only.

lodash's infinitely nested dependency graph collides with Windows path limit of 255 chars, fix tests that fail on windows #23

Closed
wants to merge 1 commit into from

Conversation

robrich
Copy link

@robrich robrich commented Jan 10, 2014

No description provided.

…h limit of 255 chars, fix tests that fail on windows
@yocontra
Copy link
Member

i'm still unsure about this...

@sindresorhus
Copy link
Contributor

@jdalton lol :p

@jdalton
Copy link
Contributor

jdalton commented Jan 10, 2014

Not sure where the issue is but some part isn't letting Windows know to use its extended-length path. The issue is bound to be more generic than lodash's dep tree. See http://msdn.microsoft.com/en-us/library/aa365247(VS.85).aspx

To specify an extended-length path, use the "?" prefix. For example, "?\D:\very long path"

@jdalton
Copy link
Contributor

jdalton commented Jan 10, 2014

BTW we have worked to reduce the dep graph of individual modules in the next version bump and we're also deprecating lodash-node in favor of simply requiring lodash like so:

var template = require('lodash/utilities/template');

will be available in v2.5.0.

Also the dep graph isn't anything crazy:

dedup'ed it's just

@yocontra
Copy link
Member

\? windows is insane

@jdalton
Copy link
Contributor

jdalton commented Jan 10, 2014

\? windows is insane

I'm suggesting it's an issue in the module that's handling path resolution not something you should be fixing in your test code.

@sindresorhus
Copy link
Contributor

@jdalton i agree, far from the first time i've seen this though. might already be a ticket in Node on this.

@yocontra
Copy link
Member

@jdalton That would be in node core then - AFAIK we are not trying to resolve the path of lodash.template besides requiring it

@jdalton
Copy link
Contributor

jdalton commented Jan 10, 2014

Some more digging: https://github.com/joyent/node/blob/master/lib/path.js#L476-L504 shows Node has code to handle long paths in Windows.

I just tested this on Windows 7 and didn't run into any issues with Lo-Dash and only tweaked the File tests to avoid the path separator and use path.join. I ran from my desktop and from the root of C:.

@yocontra
Copy link
Member

@jdalton
Copy link
Contributor

jdalton commented Jan 10, 2014

Coo cool, pretty sure this is a non-lodash specific issue, the PR should be modified to simply have better use of path.join without the lodash package swap. I did this locally and all tests pass.

Btw my longest path was ~130 chars, so still half left.

@yocontra
Copy link
Member

@jdalton thanks for the troubleshooting 👍

@robrich
Copy link
Author

robrich commented Jan 10, 2014

Yes, the real issue is outside gulp. But requiring a shallower dependency
graph fixed it. The actual error is team city unable to clean the directory
because git doesn't use the '?' syntax. All else being equal, staying
under 255 chars is usually easier, and this seems a trivial change to
resolve it.

@phated
Copy link
Member

phated commented Jan 15, 2014

What should be done to resolve this? Should the PR be merged or not?

@yocontra
Copy link
Member

Closing this as wontfix

@luochen1990
Copy link

when the dependence vinyl-fs added. this problem occurs again. the path looks like this:
...\NODE_M~1\gulp\NODE_M~1\vinyl-fs\NODE_M~1\GLOB-S~1\NODE_M~1\GLOB2B~1\NODE_M~1\LODASH~1.FIN\NODE_M~1\LODASH~1.CRE\NODE_M~1\LODASH~2._BA\NODE_M~1\LODASH~1.BIN\NODE_M~1\LODASH~1._CR\NODE_M~1\LODASH~1._BA\node_modules\lodash._basecreate\...

@gulpjs gulpjs locked and limited conversation to collaborators Jul 2, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants