Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows paths with ipfs add #1922

Closed
djdv opened this issue Oct 31, 2015 · 5 comments
Closed

Windows paths with ipfs add #1922

djdv opened this issue Oct 31, 2015 · 5 comments

Comments

@djdv
Copy link
Contributor

djdv commented Oct 31, 2015

On Windows, when recursively adding directories with ipfs add -r *target*, ipfs will produce strange trees. The target directory will be walked and added but there is some redundant adding and malformed names being produced, files will have the path they are located at prepended to their filename and be listed in 2 places, the root will contain all files and directories no matter how deep in addition to having files listed (with malformed names) where they should be located, for example a file located at "C:\dir\subdir\file.ext" would live here after adding "C:\dir\subdir/C:\dir\subdir\file.ext"

These visual examples should better exemplify what I mean:
listings
listing 1
listing 1b
listing 2

These bad directories can be viewed at these hashes:
QmYtNSri8H4wstyaEwHe9gWRM1cSADCNNJBNcw3mtQuB2T
QmfPYceFpW2JNJcN8NpyncM7vT8tkasnd7pG5A6xQs7RUd

I'm assuming the problem is related to the path delimiters on Windows being '' and pkg/path or pkg/path/filepath not being used somewhere it needs to be.
There seems to be other instances where this causes issues such as when ipfs add tries to create ~/.ipfs/block if it does not exist, it will return this error:
Error: mkdir C:\Users\Dominic\.ipfs/blocks/1220bfcc: The system cannot find the path specified.

-Edit-
For reference:
OS: Windows 10 x64
Go: 1.5.1
IPFS: a93dbf6

@wscott
Copy link

wscott commented Nov 2, 2015

(from outside observer...) Windows is perfectly happy to use forward
slashes if you hand it a pathname like that. So pathnames should be
normalized to use forward slashes internally for consistency.

On Sat, Oct 31, 2015 at 11:44 AM, Dominic Della Valle <
notifications@github.com> wrote:

On Windows, when recursively adding directories with ipfs add -r target,
ipfs will produce strange trees. The target directory will be walked and
added but there is some redundant adding and malformed names being
produced, files will have the path they are located at prepended to their
filename and be listed in 2 places, the root will contain all files and
directories no matter how deep in addition to having files listed (with
malformed names) where they should be located, for example a file located
at "C:\dir\subdir\file.ext" would live here after adding
"C:\dir\subdir/C:\dir\subdir\file.ext"

These visual examples should better exemplify what I mean:
[image: listings]
https://cloud.githubusercontent.com/assets/13862850/10864379/b37d74ea-7fc2-11e5-946c-8285b4169f31.png
[image: listing 1]
https://cloud.githubusercontent.com/assets/13862850/10864381/b37f54e0-7fc2-11e5-80f9-74c04c5f13d6.PNG
[image: listing 1b]
https://cloud.githubusercontent.com/assets/13862850/10864378/b37bc064-7fc2-11e5-9691-cfebb40ec6fc.PNG
[image: listing 2]
https://cloud.githubusercontent.com/assets/13862850/10864380/b37d6d1a-7fc2-11e5-9766-b242d2af48ce.PNG

These bad directories can be viewed at these hashes:
QmYtNSri8H4wstyaEwHe9gWRM1cSADCNNJBNcw3mtQuB2T
QmfPYceFpW2JNJcN8NpyncM7vT8tkasnd7pG5A6xQs7RUd

I'm assuming the problem is related to the path delimiters on Windows
being '' and pkg/path or pkg/path/filepath not being used somewhere it
needs to be.
There seems to be other instances where this causes issues such as when
ipfs add tries to create ~/.ipfs/block if it does not exist, it will return
this error:
Error: mkdir C:\Users\Dominic.ipfs/blocks/1220bfcc: The system cannot
find the path specified.


Reply to this email directly or view it on GitHub
#1922.

@jbenet
Copy link
Member

jbenet commented Nov 2, 2015

this is a golang thing-- we need to use the filepath package instead of path in more places, and convert the paths on the way out.

@wscott the golang team recommends the above for portability. i dont know that much about how all the windows systems handle paths, etc, but do know things generally improve when we find these cases and adjust them to use filepath.

@jbenet
Copy link
Member

jbenet commented Nov 2, 2015

(but yeah, if windows is supposed to accept forward slashes, then who knows!)

@djdv
Copy link
Contributor Author

djdv commented Nov 2, 2015

Windows will accept forward slashes but even when passing in a relative path with one (ipfs add -r "./Windows path") it and its children will be converted to a backslash path somewhere along the line. It's fine (but probably discouraged) to read and write to mixed slashed paths in Windows but it can cause issues like this one when you need to parse the path itself for other uses.
Things like path.Base will not handle a path like this correctly "a\b" where it will handle "a/b" just fine, mixing both slashes also produces non-desired output. filepath.Base will handle all 3 of those cases correctly when called on Windows.

Using the platform preferred delimiter internally except when producing output that depends on forward slashes seems like it would be a good way to go.

I've created a pull request that fixes the malformed filenames by just changing the package import and call however there may be other places in the project that require this same change for other issues. It may be worth investigating.
#1932 Oops!
#1933

The same directory now produces these hashes with the same arguments
QmVu8MgHzuF7asFYx8VxsXXt1QxPS3GYu3QBx17pWFg9Mr
QmYnEVf8mS3d1yQFMXrgopvaimyQm53tV6x9YEeJCoJXh7

When called with the full path as the argument, the full path is listed (in this case "J:\demo\ipfs\windows directory")
All children of this directory are listed in the root alongside the root itself.

Is this the intended behavior?

@djdv
Copy link
Contributor Author

djdv commented Dec 22, 2015

Should be resolved with #1933.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants