Fails to return indirect folders and files on windows 7 #5

Closed
tommedema opened this Issue Dec 18, 2011 · 19 comments
@tommedema

On windows 7, findit only returns files immediately in the initial folder. It does not go through sub folders etc.

@tommedema

Actually, the following example does not return anything on windows 7, node 0.6.6.

var finder = require('findit').find(__dirname);

finder.on('directory', function (dir, stat) {
    console.log(dir + '/');
});

finder.on('file', function (file, stat) {
    console.log(file);
});

finder.on('link', function (link, stat) {
    console.log(link);
});
@tommedema

When I change the first line to:

var findit  = require('findit'),
    path    = require('path'),
    finder  = findit.find(path.resolve(__dirname));

I do get the first file in my folder: bootstrap.js. However, I don't get any folders nor the second file in this folder: test.txt.

@tommedema

After debugging I found that fs.Stats.inode always returns 0. Thus, iNodeSeen always returned true after the first file. I removed the inode checker and added a file checker instead, and it works now:

var fs = require('fs');
var path = require('path');
var EventEmitter = require('events').EventEmitter;
var Seq = require('seq');

function createFileChecker() {
    var files = {};
    return function fileSeen(file) {
        if (files[file]) {
            return true;
        } else {
            files[file] = true;
            return false;
        }
    }
}

exports = module.exports = find;
exports.find = find;
function find (base, options, cb) {
    cb = arguments[arguments.length - 1];
    if (typeof(cb) !== 'function') {
        cb = undefined;
    }
    var em = new EventEmitter;
    var fileSeen = createFileChecker();

    function finder (dir, f) {
        Seq()
            .seq(fs.readdir, dir, Seq)
            .flatten()
            .seqEach(function (file) {
                var p = path.join(dir, file);
                fs.lstat(p, this.into(p));
            })
            .seq(function () {
                this(null, Object.keys(this.vars));
            })
            .flatten()
            .seqEach(function (file) {
                var stat = this.vars[file];
                if (cb) cb(file, stat);

                if (fileSeen(file)) {
                    // already seen this file, skip it
                    this(null);
                }
                else {
                    em.emit('path', file, stat);

                    if (stat.isSymbolicLink()) {
                        em.emit('link', file, stat);
                        if (options && options.follow_symlinks) {
                          path.exists(file, function(exists) {
                            if (exists) {
                              fs.readlink(file, function(err, resolvedPath) {
                                if (err) {
                                  em.emit('error', err);
                                } else {
                                  finder(path.resolve(path.dir(file), resolvedPath));
                                }
                              });
                            }
                          });
                        } else {
                          this(null);
                        }
                    }
                    else if (stat.isDirectory()) {
                        em.emit('directory', file, stat);
                        finder(file, this);
                    }
                    else {
                        em.emit('file', file, stat);
                        this(null);
                    }
                }
            })
            .seq(f.bind({}, null))
            .catch(em.emit.bind(em, 'error'))
        ;
    }

    fs.lstat(base, function (err, s) {
        if (err) {
            em.emit('error', err);
        }
        if (s.isDirectory()) {
            finder(base, em.emit.bind(em, 'end'));
        }
        else if (s.isSymbolicLink()) {
            if (cb) cb(base, s);
            em.emit('link', base, s);
            em.emit('end');
        }
        else {
            if (cb) cb(base, s);
            em.emit('file', base, s);
            em.emit('end');
        }
    });

    return em;
};

exports.findSync = function findSync(dir, options, callback) {
    cb = arguments[arguments.length - 1];
    if (typeof(cb) !== 'function') {
        cb = undefined;
    }
    var inodeSeen = createInodeChecker();
    var files = [];
    var fileQueue = [];
    var processFile = function processFile(file) {
        var stat = fs.lstatSync(file);
        if (inodeSeen(stat.ino)) {
            return;
        }
        files.push(file);
        cb && cb(file, stat)
        if (stat.isDirectory()) {
            fs.readdirSync(file).forEach(function(f) { fileQueue.push(path.join(file, f)); });
        } else if (stat.isSymbolicLink()) {
            if (options && options.follow_symlinks && path.existsSync(file)) {
                fileQueue.push(fs.realpathSync(file));
            }
        }
    };
    /* we don't include the starting directory unless it is a file */
    var stat = fs.lstatSync(dir);
    if (stat.isDirectory()) {
        fs.readdirSync(dir).forEach(function(f) { fileQueue.push(path.join(dir, f)); });
    } else {
        fileQueue.push(dir);
    }
    while (fileQueue.length > 0) {
        processFile(fileQueue.shift());
    }
    return files;
};

exports.find.sync = exports.findSync;

However, I am not sure if a file check is enough to avoid circular references.

@svelez

This will be a hard one to fix... firstly because the path-based solution won't work a symbolic link pointing to an ancestral directory will make it possible to traverse down the path indefinately.... that's what the inode-based code is supposed to be doing. The trouble is that windows provides no "inode" data on a file through it's *stat functions, and therefore node can't either... looking at nodes source it seems they might use some third party for some of this functionality, but I am not sure. I did find this:

http://msdn.microsoft.com/en-us/library/windows/desktop/aa364226%28v=vs.85%29.aspx

and it seems the fileID COULD possibly be used to implement this check as long as the path stayed on the same file system (does the inode solution work across FSs on unix?), but I don't know how that can be wedged in to the node api, and we would have to manage to get it in there in the first place.

For the short-term, it may be easiest to just disable the cycle check on windows, retaining it for other OSes.... not many people use symlinks on windows anyhow. Yet.

@marisks

I created issue on node.js about this: nodejs/node-v0.x-archive#2670

Also schakko suggests solution for it on stackoverflow: http://stackoverflow.com/questions/9062248/why-jasmine-node-doesnt-find-my-spec-files

function createInodeChecker() {
    var inodes = {};
    return function inodeSeen(inode) {
        if (inode == 0) {
            return false;
        }

        if (inodes[inode]) {
            return true;
        } else {
            inodes[inode] = true;
            return false;
        }
    }
}

I think that this solution could be quite good - Windows version would ignore checking, but Unix systems still use it.

@liammagee

I also had this issue, and can confirm marisks' linked solution does fix the problem on Windows.

@soldair

the inode check has to use device id to work on linux 100% as well. different devices can have the same inode numbers

@tommedema

Created this issue 3 months ago, I guess this project is no longer maintained?

@soldair

I created a package with almost the same API as this. It should work on windows I would really like to know of it solves your problem. Its faster and can scan larger trees than this without crashing. github.com/soldair/node-walkdir
npm install walkdir

I'm actively maintaining it so bug reports are welcome.

@jmortensen

I still think this is an issue that is affecting the jasmine-node module: Windows 7 64, node 0.7.7, jasmine-node 1.0.22. I've tried the work around that marisks posted but I still can't get jasmine-node to find any tests to run and it uses findit

@mpareja

There are still a bunch of packages that depend on findit. Can we (pretty please!) at least get a Windows specific workaround so all downstream packages aren't broken on windows?

@svelez

judging from the [lack of] response, it seems no amount of pretty-pleasing will get this done... at least promptly. The better course of action may be to start petitioning the dependent projects to switch to the package referenced by @soldair

@svelez svelez referenced this issue in mhevery/jasmine-node May 22, 2012
Closed

Update dependent library that fails on windows. #153

@soldair
@svelez
@jden

Any progress on this issue? Does a fix depend on an update to the fs API in Node on Windows?

@phun-ky

Status on this?

@joshspivey

Is there any news on this they closed the ticket

mhevery/jasmine-node#153

I guess everyone is suggesting to switch to walker because findit doesn't work on windows. Works great on mac but my company is 90% windows so that's not a option for me...

@soldair

walkdir https://npmjs.org/package/walkdir is the same interface. drop in replacement pretty much jasmine-node switched to it and others. its fine =)

@andrewrk

Thanks. IMO @substack should update the readme advising that this module is no longer maintained and pointing to walkdir.

@substack substack closed this Sep 14, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment