Windows, build-runfiles: Implement SymlinkResolver #6014
Conversation
meteorcloudy
force-pushed
the
symlink
branch
3 times, most recently
from
cffef36
to
25d7951
Compare
|
What's the motivation for this PR? |
|
This is for implementing build-runfiles on Windows. It's used to check if a link already points to the correct target. If so, we don't need to recreate the symlink, because creating symlink is slow on Windows. I tested Just filed #6019 to track the progress. |
src/main/cpp/util/file_windows.cc
Outdated
| FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, | ||
| NULL, | ||
| OPEN_EXISTING, | ||
| FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT, |
There was a problem hiding this comment.
FILE_FLAG_BACKUP_SEMANTICS is required when and only when opening a directory or junction. When opening a file symlink, you must not use FILE_FLAG_BACKUP_SEMANTICS. I suggest trying to open it with FILE_ATTRIBUTE_NORMAL | FILE_FLAG_OPEN_REPARSE_POINT in case this CreateFileW call fails.
Furthermore, in order to minimize risk of filesystem races I suggest attempting to open the file without trying to call GetFileAttributesW. The benefit: if the path does not exist, CreateFileW fails and GetLastError() will return a corresponding error code, but if the path does exist, we have an open handle to it and no other process can delete or modify the file until we release this handle. (The OS will delay actual deletion until all handles are closed.)
And in case you do need the attributes after opening the file, you can use GetFileInformationByHandle.
There was a problem hiding this comment.
Good suggestions! Thanks!
Provide a ReadSymlinkW function for resolving symlink on Windows. Also made IsDirectoryW available. Change-Id: I5588c06ee768de12b949f2aa19e3acb84282bb4d
Change-Id: Ic9e71c79ef1c761f94f046cdbf7c61b75cddd746
|
Addressed the comment, please take a look again~ |
src/main/cpp/util/file_windows.cc
Outdated
| OPEN_EXISTING, | ||
| FILE_ATTRIBUTE_NORMAL | FILE_FLAG_OPEN_REPARSE_POINT, | ||
| NULL)); | ||
| if (!handle.IsValid()) { |
There was a problem hiding this comment.
In my experience, CreateFileW(..., FILE_ATTRIBUTE_NORMAL | ...) fails for directories (and junctions, which are also directories), therefore this failure may just mean that "path" refers to a directory.
There was a problem hiding this comment.
Then I think we can call GetFileAttributesW first to check if the path refers to a reparse point or not. If it does, we can just use FILE_FLAG_OPEN_REPARSE_POINT, otherwise, we skip and return the original path. WDYT?
There was a problem hiding this comment.
And we can tell from the attribute if the path is a directory or not, if it is, we set FILE_FLAG_BACKUP_SEMANTICS
There was a problem hiding this comment.
Yes, I think calling GetFileAttributeW then CreateFileW (with FILE_ATTRIBUTE_NORMAL or FILE_FLAG_BACKUP_SEMANTICS, depending on the attributes) is as good as calling CreateFileW first with FILE_FLAG_BACKUP_SEMANTICS then with FILE_ATTRIBUTE_NORMAL -- both approaches achieve the same goal and both are susceptible to file system races (i.e. when the file is modified between the two function calls).
There was a problem hiding this comment.
OK, it's done. But I think we don't need FILE_ATTRIBUTE_NORMAL for file symlink because in the document it says This attribute is valid only if used alone.
https://docs.microsoft.com/en-us/windows/desktop/api/fileapi/nf-fileapi-createfilea
Change-Id: I2b289c9b9701b28421afe4f81e858d678918c16b
Provide a ReadSymlinkW function for resolving symlink on Windows.
Also made IsDirectoryW available.