pathSepS is broken on Windows #11
Conversation
Other parts of IPFS (specifically, for example, libipfs-kad-dht) assume that pathSep is '/'. This code assumes pathSep is determined by the operating system and therefore on Windows will be '\'. This patch is probably not the best solution to the problem (perhaps you should support both common path separators on all platforms?) but allows me to at least keep using libp2p on Windows for the time being.
|
Path separator in datastore is completely separate from filesysystem separator. |
|
One could make the argument that the error is at https://github.com/libp2p/js-libp2p-kad-dht/blob/master/src/utils.js#L42 instead. If paths can originate from places other than the local filesystem perhaps the library should just support all known path separators instead of just the current OS reported one? Like I said, I'm trying to raise the issue more than provide a definitive solution. |
|
@pvh that "path" is used as a key to levelDB database which can be one of backing databases of datastore but keys from datastores have no mapping to filesystem. This is why path separators in datastores should not introduce issues. It might be the same character but it has completely different meaning. cc @diasdavid |
|
I guess I'm confused - as you can see in my sketch of PR above, the path separator is presently determined by what the Perhaps you're suggesting my brutish approach is actually reasonable after all? |
|
@pvh thanks for pushing this. @richardschneider continue this work on #12 and now we are on the final review step on #13 Closing this PR, join the convo on #13 \o/ |
Other parts of IPFS (specifically, for example, libipfs-kad-dht) assume that pathSep is '/'. This code assumes pathSep is determined by the operating system and therefore on Windows will be ''.
This patch is probably not the best solution to the problem (perhaps you should support both common path separators on all platforms?) but allows me to at least keep using libp2p on Windows for the time being.