Skip to content

Commit

Permalink
fix: catch ELOOP
Browse files Browse the repository at this point in the history
fs.realpath has been optimized to utilize `uv_fs_realpath()`, a new api
in libuv. The libuv api has slightly different behavior. There are two
particular issues that are causing problems for glob

* `ev_fs_realpath()` throwing before `fs.readdir` ELOOP
* The removal of the cache

In the past glob was relying on `fs.readdir` to throw when handling
recursive symbolically linked directories. It is now possible that the
call to libuv can throw before. The behavior is currently different on
osx and linux causing the test suite to fail on all of the flavors of
linux we are running in CI.

This commit adds an extra check for 'ELOOP' and sets appropriately.

The commit includes an extra check to make sure that `real` is truthy
Without that extra check the test suite was reporting garbage data in the results.
This was only happening with the async implementation.

This commit has not done anything regarding the removal of the cache
As long as the cache doesn't include a value called "encoding"
everything will be fine.

There has not been any benchmarking done on this change.

ref: libuv/libuv#531
ref: nodejs/node#3594
  • Loading branch information
Myles Borins committed Jun 6, 2016
1 parent 68f2509 commit 7c5655e
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 0 deletions.
3 changes: 3 additions & 0 deletions glob.js
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,9 @@ Glob.prototype._realpathSet = function (index, cb) {
set[real] = true
else if (er.syscall === 'stat')
set[p] = true
else if (er.code === 'ELOOP') {
if (real) set[real] = true
}
else
self.emit('error', er) // srsly wtf right here

Expand Down
3 changes: 3 additions & 0 deletions sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ GlobSync.prototype._finish = function () {
} catch (er) {
if (er.syscall === 'stat')
set[self._makeAbs(p)] = true
else if (er.code === 'ELOOP') {
if (real) set[real] = true
}
else
throw er
}
Expand Down

0 comments on commit 7c5655e

Please sign in to comment.