-
Notifications
You must be signed in to change notification settings - Fork 36
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
Improve symlink support on Windows #430
Comments
I agree with your proposed solution and caveats. We need to be able to test effectively and fail gracefully. |
We either need to add windows to our CI or consider windows unsupported. I'm going to add the 2.0 milestone to this issue since we need to decide formally if windows will be supported in 2.x. Since we don't have windows in our CI I'm tempted to close this issue unless someone wants to add windows to the CI + take on the increased maintenance workload. Another solution is to point users to the WSL which now is officially supported by Microsoft. |
I was wrong! We do test on Windows, so we should support this. |
@mikemhenry This issue just affects specific features that rely on symlinks, primarily linked views. Windows does support symlinks but it depends on system settings and user privileges. To resolve this issue, we would need to:
This follows my proposal from above:
|
It seems to me that providing access to the data on the file system (hidden behind the job id) through the human-readable linked views is one of the core features. I might be missing something, but as an alternative, have you considered replacing symlinks with simple shortcuts on windows? This will increase the maintenance load though. But may solve the problem of testing symlinks on the CI platform. Another valid proposal - using WSL - removes the problem from the maintainers, but on the user side is even more involved than enabling symlinks. Users, who take this approach will have many other reasons to do it, not just signac. Also, as much as it's fair to point the users to WSL (as a solution to the problem of accessing linked views on Windows), this does not make the package "Windows-compatible". |
@slowglow Thanks for your feedback. Most (all?) of our code contributors use Mac/Linux/WSL, so linked views on Windows have not been requested until now, but we’d be happy to have the feature improvement! Would you like to help contribute a pull request to help solve this? If you’d like to have some guidance or discuss in real time with the project team, we have regular developer meetings and office hours, as well as a Slack workspace. See here for the calendar & links to join: https://docs.signac.io/en/latest/community.html |
@bdice I agree with your proposed steps 1-3 for the development of the v2.0.0 milestone. This would enable the same codebase on different platforms. In the future, linked views implementation as Windows shortcuts to the Expected advantages of such approach are that:
Expected disadvantages are that:
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
There may be a very simple solution: instead of a blank refusal to create a symlink on Windows, try and catch the OSError, then issue a helpful error message. Here's what I did:
After installing follow the tutorial and try to execute the following line:
It will return the hardcoded error message for windows platforms:
This is not entirely true, so let's comment out the error first.
Most likely you will get the error:
The means that windows was not set up to enable the creation of symlinks.
Settings → Privacy & Security → For Developers → Use developer features: Select “Developer mode”. If you use Python >=3.8 that's all there is to it. For Python < 3.8, a few more steps are required. If the user Windows system is setup correctly for the creation of symlinks, just commenting out of the relevant portion of the code should be sufficient to solve this issue. |
@slowglow Thanks for the analysis. Please consider filing a PR if you would like this change to be released in the future. |
Done. Please review, comment and merge. |
@slowglow mentioned in #214:
I proposed that it would be fine to enable this feature if we are able to test it effectively and fail safely on Windows systems that don't support symlinks. Our continuous integration may not have the appropriate privileges to create symlinks, so the tests need to be optional (depending on whether the system is capable of running the tests).
As a short term solution, @slowglow proposes to change the error message to something like "Error: signac cannot create linked views. Symbolic links on Windows are currently not supported by signac."
The text was updated successfully, but these errors were encountered: