soldair / node-walkdir

Walk a directory tree emitting events based on the contents. API compatable with node-findit. Walk a tree of any depth. Fast! Handles permission errors. Stoppable. windows support. Pull requests are awesome. watchers are appreciated.
MIT License
130 stars 22 forks source link

Duplicate inode entries in Windows (NTFS) #22

Closed repko-artem closed 9 years ago

repko-artem commented 9 years ago

There were many cases, when two or even more files in one directory had the same inode. For example: -863777672-21392098230827184 // left_panel_page_step.js -863777672-21392098230827184 // main_menu_step.js These files had different modification and creation times, different sizes, but exact the same inode key. I suggest to include a file name in a key.

soldair commented 9 years ago

this is great. thanks. the problem is that it breaks cycle protection for infinite loops so i cant merge this straight away.

fs.stat didn't even return an inode on windows when i made this originally as far as i recall. If you are certain these files are not just linked to each other we'll need to come up with rules on when to not use inode at all. =/

repko-artem commented 9 years ago

Thank you, soldair. Unfortunately, I can't reproduce or explain this situation. It happens after teamcity checkout and not always. But we have this problem very often (we are using cucumber-js auto-testing framework, that uses walkdir). These files are absolutely different and have different content, size, dates... If I re-save the file, it will get another (unique) inode.

Can you explain why my commit breaks cycle protection for infinite loops?

soldair commented 9 years ago

on files systems that support links you can have a directory that looks like this

/a /a/b->/a

where b is a link that point to the directory that contains it creating a loop

the path name will be /a/b/b/b/b/b/b/b/b/b... to infinity without the check.

there is a test for this im not sure why they are not running at the moment.

soldair commented 9 years ago

could you send me the return value of fs.stat from a file in your environment. perhaps there is more data there we can use.

repko-artem commented 9 years ago

I've tested your example on linux:

fs.mkdirSync("/a");
fs.symlinkSync("/a", "/a/b");
path.basename("/a/b"); // ----> 'b'

So, there's no infinite loop here.

I'll try to collect fs.stat info and then send you as soon as possible.

soldair commented 9 years ago

we can't use basename because its not a unique name in the whole file system. On Jun 25, 2015 2:41 PM, "Artem Repko" notifications@github.com wrote:

I've tested your example on linux:

fs.mkdirSync("/a"); fs.symlinkSync("/a", "/a/b"); path.basename("/a/b"); // ----> 'b'

So, there's no infinite loops here.

I'll try to collect fs.stat info and then send you as soon as possible.

— Reply to this email directly or view it on GitHub https://github.com/soldair/node-walkdir/pull/22#issuecomment-115406351.

repko-artem commented 9 years ago

Yes, it is not. That's why I suggest to include basename in complex key, that contains dev, ino and basename.

soldair commented 9 years ago

ah of course. i think i still see an issue im going to fix the tests real quick so i can actually test. thanks again for the pr!

soldair commented 9 years ago

awesome couldn't make it return anything extra from before the change. sorry to cause so many back and forths for such a good fix. LGTM

soldair commented 9 years ago

npm @0.0.10

repko-artem commented 9 years ago

Thank you, soldair!