anishathalye / dotbot

A tool that bootstraps your dotfiles ⚡️
MIT License
7.04k stars 291 forks source link

Fix Windows symlinks #308

Closed kurtmckee closed 2 years ago

kurtmckee commented 2 years ago

On Windows, os.readlink(path) returns UNC paths that start with \\?\.

The UNC prefix can simply be removed when running on Windows.

Fixes #307

anishathalye commented 2 years ago

Thank you for the pull request! By the way, did you get the unit tests running on Windows? What did you need to do to make that happen?

kurtmckee commented 2 years ago

My pleasure!

I was able to get the unit tests running with Windows as the host -- it still ultimately runs in a VirtualBox VM that Vagrant set up. (And THANK YOU for excellent instructions for recreating the test environment! It allowed me to make changes and avoid breaking functionality in Linux!)

I looked at how I could make the tests actually run under Windows, but there were several challenges:

  1. Test isolation It might be possible to redesign the tests to run in temp directories, rather than relying on true filesystem isolation. I didn't look closely at that yet, though.
  2. Testing across Python versions I'm familiar with a tool called tox that does a fantastic job of setting up testing environments for different Python versions, matrices of dependencies, plugins, etc etc etc. Tox would be a good way to test everything locally instead of relying just on CI. I strongly recommend it, always.

If you're interested, I can take a look at updating the tests so they can run on Windows. I do want you to bless the attempt before I try, though. :grinning:

anishathalye commented 2 years ago

Re this pull request, I will try to review in a couple days, need to test on a Windows machine which adds a bit more overhead.


I would love for someone to get the tests working on Windows. I'm a long-time macOS/Linux user and don't know much about the current state of things on Windows. Last time I checked, there was some weird behavior related to symlinks. Does seem like a good amount of work to figure this out, though. Likely requires modifying Dotbot code, test code, and CI config.

I don't care quite as much about getting the Windows tests running locally / figuring out isolation etc (though I'd be happy to merge a PR if someone adds this). I care more about getting Windows tests running in CI. The isolation and testing across Python versions becomes much easier there (basically zero effort).

I tried modifying the CI config to switch to Windows, but that alone wasn't enough to get it to work. My half-finished attempt is here, in case that's a helpful starting point: https://github.com/anishathalye/x/tree/windows (in particular, see https://github.com/anishathalye/x/blob/windows/.github/workflows/build.yml). Some example output here: https://github.com/anishathalye/x/runs/5982348889?check_suite_focus=true

kurtmckee commented 2 years ago

Windows used to require admin privileges to create symlinks, but that requirement is dropped if Developer Mode is enabled (or if group policy is modified, I think). Also, Python calls the create-symlink API (whatever it's called) with the correct arguments as of...2019, maybe? Hopefully those changes reduce the weirdness.

I'll try to get the tests working in Windows. I mentioned running the tests locally because I typically target locally-runnable tests, then make CI just run the same test suite (though with a full matrix of OS's). Local and CI testing are both valuable. :+1:

kurtmckee commented 2 years ago

Closing because this work is superseded by the work associated with #309.