Skip to content

Broken on Windows since interface-datastore use of os-specific path separators #144

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

Closed
Expyron opened this issue Oct 5, 2017 · 5 comments
Labels
exp/expert Having worked on the specific codebase is important help wanted Seeking public contribution on this issue P2 Medium: Good to have, but can wait until someone steps up

Comments

@Expyron
Copy link

Expyron commented Oct 5, 2017

In commit d7ec65a, interface-datastore was changed to use os-specific path separators.

However, js-ipfs-repo is still hardcoded to use a slash (here, maybe other places). This throws an error here.

The specifications of interface-datastore should be clarified as to whether we should always use slashes or os-specific path separators, and either interface-datastore or js-ipfs-repo fixed in accordance with the specifications.

I can submit a pull-request depending on the decision.

@daviddias daviddias added status/ready Ready to be worked exp/expert Having worked on the specific codebase is important help wanted Seeking public contribution on this issue P2 Medium: Good to have, but can wait until someone steps up labels Oct 13, 2017
@daviddias
Copy link
Member

Ref: ipfs/js-ipfs#1017

@Expyron
Copy link
Author

Expyron commented Oct 19, 2017

After some more checking, while this is easy to fix for the blockstore by simply normalizing the path, there is also the same issue in the datastore.

A datastore created on Windows will contain keys with backslashes. This means datastores created on Linux cannot be read on Windows, and conversely. Interop tests are failing on Windows because the test repo has (obviously) been created with slashes.

@richardschneider
Copy link
Contributor

The datastore should be os-agnostic. All keys should use the same path separator of /. Only file storage implementations should worry about the os styled path conventions.

@pvh has raised ipfs/interface-datastore#11 which is a step in the correct direction.

@richardschneider
Copy link
Contributor

I'm getting the following error when running the tests on windows

 1) IPFS Repo Tests on on Node.js default "before all" hook:
: The filename, directory name, or volume label syntax is incorrect.s-ipfs-repo\test\test-repo-for-1509616898591\datastore/MANIFEST-000052

      at c:\Users\Owner\Documents\GitHub\js-ipfs-repo\node_modules\levelup\lib\levelup.js:117:34
      at c:\Users\Owner\Documents\GitHub\js-ipfs-repo\node_modules\leveldown\node_modules\abstract-leveldown\abstract-leveldown.js:39:16

Looks like levelup needs some TLC for windows.

@daviddias
Copy link
Member

Let's see how it behaves with the new datastores! => Track here: #146

@ghost ghost removed the status/ready Ready to be worked label Nov 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exp/expert Having worked on the specific codebase is important help wanted Seeking public contribution on this issue P2 Medium: Good to have, but can wait until someone steps up
Projects
None yet
Development

No branches or pull requests

3 participants