Skip to content
This repository has been archived by the owner on Aug 11, 2021. It is now read-only.

fstream ignores directories with long paths on windows #30

Open
sdarnell opened this issue Nov 9, 2014 · 2 comments · May be fixed by #42
Open

fstream ignores directories with long paths on windows #30

sdarnell opened this issue Nov 9, 2014 · 2 comments · May be fixed by #42

Comments

@sdarnell
Copy link

sdarnell commented Nov 9, 2014

When piping fstream.Reader() into tar.Pack() fstream ignores long pathnames on windows.

The calling code looks like this, but the problem seems to be on fstream side of the pipe, because I added a filter to log entries as they go by and nothing comes out.

  return fstream.Reader({ path: dirPath, type: 'Directory' }).pipe(
    tar.Pack({ noProprietary: true })).pipe(zlib.createGzip());

To be more precise, I think it is directories that get ignored.

Looking in lib/reader.js@104 and there is some code (legacy?) which tries to convert paths into the UNC form \\?\... when the paths hit 260 characters.

Firstly I think this code is probably redundant now as my understanding is that libuv handles the mapping to long names.

But what seems to be happening is that the code is called twice (not sure quite why), and performs the conversion twice - incorrectly. So I end up with invalid paths starting \\?\\\_\c:\
The underscore comes from what was previously a ?

My guess is that the whole win32 if at line 104 should be removed.

@sdarnell
Copy link
Author

sdarnell commented Nov 9, 2014

There's similar code in writer.js, and I suspect that if these bits are removed, much of the _path stuff can probably removed again too (this code was added in November 2011).

avital added a commit to meteor/fstream that referenced this issue Mar 24, 2015
3e5d171 (from November 2011) introduced this code to fix treatment
of long file paths in Windows. Then, one week later, this commit
fixed long file paths directly in Node's `fs` library:
nodejs/node-v0.x-archive@1f16a7b

So, the code currently in fstream actually /breaks/ long file
paths in Windows.

This fixes it, but keeps an additional mysterious line introduced
by that original commit that I don't understand.

Fixes npm#30. Thanks to @sdarnell for identifying the problem.
@avital avital linked a pull request Mar 24, 2015 that will close this issue
@avital
Copy link

avital commented Mar 24, 2015

Pull request in #42. Thanks @sdarnell for the find!

benjamn pushed a commit to meteor/fstream that referenced this issue Apr 27, 2016
3e5d171 (from November 2011) introduced this code to fix treatment
of long file paths in Windows. Then, one week later, this commit
fixed long file paths directly in Node's `fs` library:
nodejs/node-v0.x-archive@1f16a7b

So, the code currently in fstream actually /breaks/ long file
paths in Windows.

This fixes it, but keeps an additional mysterious line introduced
by that original commit that I don't understand.

Fixes npm#30. Thanks to @sdarnell for identifying the problem.
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 a pull request may close this issue.

2 participants