Tests are broken on various Linux platforms on master / v6 rc

[MylesBorins]

will be digging into this further today, but wanted to give you a heads up

[MylesBorins]

It looks like the breakage has been caused by nodejs/node@b488b19

Node is now relying on a native implementation of realpath that is provided by libuv

One of the changes is the removal of the cache, which is used by glob -- >https://github.com/isaacs/node-glob/blob/master/glob.js#L235

This does not seem to be causing the problem, as running tests with the cache removed is still throwing the same errors. @jasnell is digging into libuv to see if it is an implementation bug

[jasnell]

Turns out to be fairly straight forward.. fs.realpath and fs.realpathSync now defer to libuv for the implementation which now throw errors that the previous implementation did not. Specifically, when the sym link depth reaches a certain point, both fs.readdir and fs.realpath both start complaining. While glob swallows the readdir throw, it doesn't swallow the realpath throw, causing the problem.

Now, we aren't seeing this happen on OSX or other platforms because on those platforms, readdir complains about the symlink depth earlier than realpath, and since glob swallows that error and uses it as an exit criteria (by returning null for the readdir check), the code never gets to the depth that causes realpath to throw.

The fix is fairly simple. In sync.js and glob.js, in the error handling for fs.realpath, check for er.code === 'ELOOP' and set[real] = true.

There is also a secondary issue with glob's use of fs.realpath. In the new implementation, the cache argument has been removed and was replaced with an options argument. This options argument does not have the same semantics as cache. It allows for a single encoding parameter that specifies the character encoding of the returned path. Right now the cache object passed in by glob is ignored completely, but if that object happened to have an encoding parameter on it, it wouldn't be good. It's probably ok to continue passing the cache argument along (it would be treated as if it were an options object in v6) for backwards compatibility. But it's something to keep an eye on.

[isaacs]

Fixed by falling back to the old implementation of fs.realpath on Node 6 when new errors are thrown from the realpath syscall. Using http://npm.im/fs.realpath module.

[MylesBorins]

thanks for taking the time to find a userland solution that works for now, I'll remove node-glob from the flaky list on citgm

I will also be following up internally to help figure out a way to fix this in node proper

[MylesBorins]

Here's a citgm run of the latest publish of node-glob using master of node

https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/308/