Skip to content
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 Windows support #26

Merged
merged 1 commit into from
May 22, 2019
Merged

Improve Windows support #26

merged 1 commit into from
May 22, 2019

Conversation

zb140
Copy link
Contributor

@zb140 zb140 commented May 21, 2019

Fixes #25. More discussion is available there.

Notable changes:

  • Contans(): fs.Stat can return a Windows-specific errno that should be ignored (ERROR_CANT_RESOLVE_FILENAME)
  • vfst: On Windows, symlinks should not be created before the things they point to exist (because links to directories are different from links to files, and the difference must be explicitly specified when calling CreateSymbolicLink). Since maps can't be guaranteed to iterate in any particular order, add support for passing a Builder a list of any of the types already supported, so that order is guaranteed.
  • contains_test: Path names on Windows may not be longer than ~260 characters (some tools and APIs support 32K chars, but not all), so on that platform the long_filename tests are expected to fail.
  • MkdirAll(): When Windows is asked to make a directory containing a path component that exists but is not a directory, the Windows APIs return (the equivalent of) ENOENT rather than ENOTDIR. This causes MkdirAll() to attempt to recursively create the path, which will fail because it already exists. This is detected correctly but MkdirAll was swallowing the error, causing the test to detect no error when one was expected.

@twpayne
Copy link
Owner

twpayne commented May 21, 2019

Thanks very much for this! The build failure is due to a misconfiguration of Travis/coveralls, which should be fixed in #27. Please can you rebase this PR on the latest master to include the fix.

  * Contans(): fs.Stat can return a Windows-specific errno that should be ignored (ERROR_CANT_RESOLVE_FILENAME)
  * vfst: On Windows, symlinks should not be created before the things they point to exist (because links to directories are different from links to files, and the difference must be explicitly specified when calling CreateSymbolicLink).  Since maps can't be guaranteed to iterate in any particular order, add support for passing a Builder a list of any of the types already supported, so that order is guaranteed.
  * contains_test: Path names on Windows may not be longer than ~260 characters (some tools and APIs support 32K chars, but not all), so on that platform the long_filename tests are expected to fail.
  * MkdirAll(): When Windows is asked to make a directory containing a path component that exists but is not a directory, the Windows APIs return (the equivalent of) ENOENT rather than ENOTDIR.  This causes MkdirAll() to attempt to recursively create the path, which will fail because it already exists.  This is detected correctly but MkdirAll was swallowing the error, causing the test to detect no error when one was expected.
@zb140
Copy link
Contributor Author

zb140 commented May 22, 2019

Sure thing! I'm happy to fix any issues that may come up.

@twpayne twpayne merged commit bd3d47c into twpayne:master May 22, 2019
@twpayne
Copy link
Owner

twpayne commented May 22, 2019

Many thanks for the contribution! I'll tweak a couple of very minor details and then tag a new version.

@twpayne twpayne mentioned this pull request May 22, 2019
@zb140 zb140 deleted the windows branch June 30, 2019 03:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve Windows support
2 participants