Appveyor #160
Appveyor #160
Conversation
|
The appveyor build is working for my personal account: https://ci.appveyor.com/project/eatkins/io. Someone with appropriate permissions would have to turn on appveyor for the io project. |
|
@cunei Could you please review the Milli changes in this PR, please? |
| else | ||
| else if (err == ERROR_SHARING_VIOLATION && attempt < MAX_GET_HANDLE_ATTEMPTS) { | ||
| Thread.sleep(1) | ||
| getHandle(lpFileName, dwDesiredAccess, dwShareMode, attempt + 1) |
There was a problem hiding this comment.
There was a problem hiding this comment.
In sbt/sbt#3972 (comment), a user reports that an ERROR_SHARING_VIOLATION still appears in a 1.x snapshot when running under the Windows Subsystem for Linux (with #134 applied); conversely in sbt/sbt#3972 (comment) the patch seems to have resolved the issue.
Access permissions are weird in Windows, and I was unable to reproduce it post-patch. In the context of this pull request, I would start by backporting #134 to 1.1.x, and use the workaround proposed by @eatkins only if that is still a problem in Appveyor afterwards. That would be my suggestion, at least.
| @@ -281,12 +290,16 @@ private object WinMilli extends MilliNative[FILETIME] { | |||
| null) | |||
| if (hFile == INVALID_HANDLE_VALUE) { | |||
| val err = GetLastError() | |||
| if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND) | |||
| if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND || err == ERROR_ACCESS_DENIED) | |||
There was a problem hiding this comment.
Good catch. An alternative could be throwing java.nio.file.AccessDeniedException for ERROR_ACCESS_DENIED, but java.io.FileNotFoundException was the usual response before nio, so I'm ok with that. I'd just use a different string for the error message in that case, for clarity, like:
if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND)
throw new FileNotFoundException("Not found: " + lpFileName)
else if (err == ERROR_ACCESS_DENIED)
throw new FileNotFoundException("Access denied: " + lpFileName)
else
|
@cunei thanks for the suggestions. I'll incorporate them soon and re-push. |
This will enable us to run tests in a windows environment during CI. I am only running io/test since the formatting and binary compatibility tests are handled by travis and appveyor do not provide as many build agents as travis.
In Windows, reading or writing file attributes requires opening the file via CreateFile(). If only FILE_SHARE_READ is specified when reading the timestamp, the CreateFile() call will fail when the file happens to be already open for writing. This pull request specifies that the file that is being opened has shared read and write access; the shared delete mode (which also implies rename) is also allowed, and in case the subsequent timestamp get/set fails (if the file disappears) the usual exception-to-zero code path will be taken. See sbt/sbt#3972
After adding the .appveyor.yml, I found that my builds would typically fail and usually one of the polling watch service threads died with an IOException with message "CreateFile() failed with error 5." I tracked this down to ERROR_ACCESS_DENIED, which seemed weird since any of the files that were being polled were generated by the test. It turns out that windows will return ERROR_ACCESS_DENIED when the file was in a pending delete state. https://stackoverflow.com/questions/6680491/why-does-windows-return-error-access-denied-when-i-try-to-open-a-delete-pended-f While this feels slightly wrong, it doesn't seem completely unreasonable to consider ERROR_ACCESS_DENIED to be like a FileNotFoundException. In either the deleted or privileged file cases, the file is effectively not findable. An alternative would be to throw a different exception and catch that in getModifiedTimeOrZero.
I was seeing transient failures of this test on windows, which has significantly slower file access apis than linux and osx.
No description provided.