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
Unable to create file on windows #61
Comments
Can you run the test suite and let me know what comes back? Unfortunately I don't have a windows box handy at the moment. |
This is the output when my coworker ran it on their machine:
I am sure though this is another problem. This is with the v0.10.1 branch. |
I was able to identify the problem; there was an error creating the directories in the FileSystem driver when the name exceeded the file path limit for windows. I was able to resolve it for myself by switching the hash function for Stash\Utilities::normalizeKeys to crc32. With this the names are short enough to work. Note: CRC32 should be good enough unless someone tries to deliberately subvert it. |
Would it be an acceptable fix to default the hash function to CRC32 on win32? Either that or I was going to make it configurable. I will only be using it on windows for testing so it should be fine. |
Configurable works far better. CRC is not designed to be a hashing function, it's an error checking function, so unlike md5 (or any hash function) it wasn't designed to avoid collisions. While I haven't done any testing myself, I would be concerned about errors from key collisions. |
I personally would not use for anything beyond simple testing or development. Off the top of my head I cannot think of another algorithm that is shorter than md5 but which should give some minor assurances against collisions. |
see http://stackoverflow.com/questions/4567089/hash-function-that-produces-short-hashes for ideas... |
I'm going to close this issue out now, as the PR was accepted and seems to resolve this. |
mmm, pull request was merged, but no alternative to md5 was provided/recommended/mentioned, right? |
You could do a PR on more code coverage (I'd really appreciate it, in fact). If not I'll take a look at expanding it myself. I'm planning on issuing updated documentation (hosted at http://stash.tedivm.com) and will include that comment. I've made an issue over there to track it ( tedivm/stash-docs#8 ). Any other thoughts? I'm reopening the ticket to keep the conversation going. |
Sorry for not sending any pull request so far, I'm just overloaded (all the issues I'm opening come from a customers project I'm working on). Will do as soon as I have 30 free seconds |
For the moment the code uses crc32. Probably not a safe choice for production, although it might be I have not fully tested it. It is good enough for development, which was my concern at the moment. |
I think this issue is resolved, but if anyone has a problem please feel free to reload it. |
There is a use case where this problem is not resolved: Right now we have an exception on windows systems thrown when the path is too long. But I discovered the problem on a linux system: CentOS on a vm (virtualbox / vagrant on windows host with shared drive) The vagrant synced folders are tied to windows limitations. |
@xgathos in my own experience, Vagrant synced folders have always been problematic: slow to use, prone to problems when dealing with symlinks, etc. I have always been much better off with storing the codebase within the VM itself and sharing it to the windows host via Samba (only downside: you need the VM to be on to access the files) |
Stash v0.10.1 on windows fails with the following error.
My best guess is because it is not using the correct path directory separator.
The text was updated successfully, but these errors were encountered: