-
-
Notifications
You must be signed in to change notification settings - Fork 701
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
Adds symlink to windows #8794
base: master
Are you sure you want to change the base?
Adds symlink to windows #8794
Conversation
Adds symlink to windows
Thanks for your pull request and interest in making D better, @MrcSnm! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + phobos#8794" |
The doc says |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, exists && isDir
is a TOCTOU... but more generally, what happens if I create a symlink pointing at nothing, and then I create a directory at the location that the symlink points at?
Deleting a symlink pointing at a directory also seems differ across platforms - on Windows, it seems that you must use rmdir
instead of remove
.
These differences in semantics and the "developer mode" requirement make me wonder if it is really suitable to have this function with the same name as the POSIX symlink
.
My personal library has had a family of these functions which try to expose some of the underlying semantics. E.g.:
- If we know that the target is a directory, there is a function called
dirLink
which creates a directory junction (not a symlink) on Windows and a regular symlink on POSIX. Junctions don't need special privileges or "developer mode". - If we don't know what the symlink points / will point at, allow the user to provide this information.
- For symlinks pointing to directories, I think the only reason to use symbolic links instead of junctions is that symbolic links can be relative.
So, it's a little hairy and it's not clear to me how best to proceed.
Perhaps, for a start, we could add wrappers for the underlying API, and if we decide that we have compelling arguments to provide a cross-platform interface in the future, we could think about that later?
Here is my implementation (though note that this uses the older, lower-level API):
std/file.d
Outdated
version(Windows) | ||
{ | ||
import core.sys.windows.w32api:_WIN32_WINNT; | ||
static if(_WIN32_WINNT >= 0x600) //WindowsVista or later |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note, it's not possible to change this constant, so this condition will effectively always be true.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It actually is possible. They aren't reserved versions, that means one needs to manually put WindowsVista
or Windows10
to actually make it true.
There is no API to create a junction point, the only way to do that AFAIK is by using executeShell with mklink /J
.
As for the developer mode requirement, it will be thrown an error if an user does not use admin rights, so I guess the naming is not that of a problem.
Symlinks can also be used to network drives while junction points cannot.
The junction or even hardlink approaches IMO are preferable since it will allow it to throw less since they don't require admin rights.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the way, what do you plan to be done?
We could do symlink with extra arguments like isDir? Give me some thoughts.
I have also readed your code but I found it a little hard to reason about, is it done by creating a file and calling DeviceIOControl? The other part looks like some optimizations.
I'm also a little confused into how you used the privileges.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They aren't reserved versions, that means one needs to manually put WindowsVista or Windows10 to actually make it true.
Yes, and then recompile Phobos; and, the only way to add the version is to modify the Phobos build files, so it's not very different from modifying the source anyway.
There is no API to create a junction point, the only way to do that AFAIK is by using executeShell with
mklink /J
.
That seems like a strange thing to say. How would mklink
work if not using the API?
FSCTL_SET_REPARSE_POINT
is the important part of the API here.
As for the developer mode requirement, it will be thrown an error if an user does not use admin rights, so I guess the naming is not that of a problem.
I'm not sure what you mean by this.
Symlinks can also be used to network drives while junction points cannot.
Good point.
The junction or even hardlink approaches IMO are preferable since it will allow it to throw less since they don't require admin rights.
Yes. However, there are still big distinctions, such as that only symlinks can point to files, and I think junctions cannot be relative.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the way, what do you plan to be done?
Well, how about adding something like void windowsSymlink(string target, string path, bool targetIsDir = isDir(target))
? An explicitly different name avoids problems caused by differences in semantics. A separate createJunction
could also be added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have also readed your code but I found it a little hard to reason about, is it done by creating a file and calling DeviceIOControl? The other part looks like some optimizations.
It is a translation to D of some code I found on MSDN. I think it is the "standard" way of doing this, before the CreateSymbolicLink
function was added.
I'm also a little confused into how you used the privileges.
Yeah, that part is some "you have to put this code in your program otherwise it won't work" magic. I'm not sure it is OK to put it in Phobos because it affects the entire process; however, it's possible that CreateSymbolicLink
uses the same method under the hood.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is fine for me having a separate name, at least it will be a lot easier than nowadays since I always had to implement a symlink for windows.
I didn't knew about this implementation you said about creating a junction, I have searched a lot on MSDN and I only found documentation on how Reparse Points
works ,but nothing onto how to explicitly use Win32 API to create a junction, but your code looks like it could do it.
Yes, and then recompile Phobos; and, the only way to add the version is to modify the Phobos build files, so it's not very different from modifying the source anyway.
About that part, I didn't thought about that... Sometimes I forget that having the prototype is very different from implementation, how should we solve that then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't knew about this implementation you said about creating a junction, I have searched a lot on MSDN and I only found documentation on how Reparse Points works ,but nothing onto how to explicitly use Win32 API to create a junction, but your code looks like it could do it.
Well, I know that both symlinks and junctions are different kinds of "reparse points", which is why I think that CreateSymbolicLink
might be a simplified wrapper around FSCTL_SET_REPARSE_POINT
.
The Wine project also seems to think that this is the case: https://bugs.winehq.org/show_bug.cgi?id=27108#c2
And, here is the implementation in React OS, which also uses FSCTL_SET_REPARSE_POINT
under the hood:
About that part, I didn't thought about that... Sometimes I forget that having the prototype is very different from implementation, how should we solve that then?
I think it should not be a problem. D only promises to support those Windows versions which Microsoft currently supports, which is currently Windows 10 and newer, and CreateSymbolicLink
was added in Vista. I am not sure about SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE
, which I think is newer; if older Windows versions ignore that flag, then it's not a problem, but if not, then it could be checked at runtime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I think I'm still a little worried on that case. I doubt a simple PR like the one I'm submitting would be enough to make D being compiled to support Windows Vista instead of Windows XP.
About the windowsSymlink, this is what I'm going with onwards right now.
On the createJunction side, maybe you would want to add like it is on your personal library? I'm really impressed that ReactOS' version is 200 lines. It makes me feel so inclined to just make it executeShell("mklink /J");
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I think I'm still a little worried on that case. I doubt a simple PR like the one I'm submitting would be enough to make D being compiled to support Windows Vista instead of Windows XP.
I would be surprised if D still actually supported Windows XP. It's likely that we're already using newer APIs somewhere.
Anyway, I've just done a small experiment, and tried my implementation of symlink
but without the acquirePrivilege
call. It seems to work, meaning that, all that SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE
seems to do is disable the AdjustTokenPrivileges
call! So, we don't need to use CreateSymbolicLink
or SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE
, we can use the old method and preserve backwards compatibility.
OK, here is a draft of a start: version (StdDdoc)
{
/++
$(BLUE This function is Windows-Only.)
Creates a Windows symbolic link.
"Developer Mode" may need to be first enabled on the local
machine before unprivileged processes are allowed to create
symbolic links.
Note that symbolic links on Windows have slightly different
semantics than those on POSIX. For example, it must be known
at creation time whether the symbolic link points / will point
to a file or directory. To delete a Windows symbolic link,
`rmdir` must be used instead of `remove` if it was created as
pointing at a directory.
Params:
original = The location that is being linked. This is the
target path that's stored in the reparse point. The
path may be relative, in which case it is relative to
the symbolic link's parent directory.
link = The location of the junction to create. A relative
path is relative to the current working directory.
Throws:
$(LREF WindowsException) on error.
+/
// TODO: isDir(original) is not correct when original is a relative path
void createWindowsSymlink(in char[] original, in char[] link, bool originalIsDir = isDir(original));
/++
$(BLUE This function is Windows-Only.)
Creates a Windows filesystem junction.
Junctions are similar to symbolic links, but cannot be
relative and cannot point to files; on the other hand, they
can be created even without special privileges or "Developer
Mode".
Params:
original = The location that is being linked. This is the
target path that's stored in the reparse point. A
relative path is first resolved to an absolute path.
link = The location of the junction to create. A relative
path is relative to the current working directory.
Throws:
$(LREF WindowsException) on error.
+/
void createJunction(in char[] original, in char[] link);
}
else
version (Windows)
{
/// Create an NTFS reparse point (junction or symbolic link).
private void createReparsePoint
(string reparseBufferName, string extraInitialization, string reparseTagName)
(in char[] target, in char[] print, in char[] link)
{
import core.sys.windows.winbase;
import core.sys.windows.windef;
import core.sys.windows.winioctl;
enum SYMLINK_FLAG_RELATIVE = 1;
HANDLE hLink = CreateFileW(
link.toUTF16z(),
GENERIC_READ | GENERIC_WRITE,
0, null,
OPEN_EXISTING,
FILE_FLAG_OPEN_REPARSE_POINT | FILE_FLAG_BACKUP_SEMANTICS,
null);
wenforce(hLink && hLink != INVALID_HANDLE_VALUE, "CreateFileW");
scope(exit) CloseHandle(hLink);
enum pathOffset =
mixin(q{REPARSE_DATA_BUFFER.} ~ reparseBufferName) .offsetof +
mixin(q{REPARSE_DATA_BUFFER.} ~ reparseBufferName)._PathBuffer.offsetof;
auto targetW = target.toUTF16();
auto printW = print .toUTF16();
// Despite MSDN, two NUL-terminating characters are needed, one for each string.
auto pathBufferSize = targetW.length + 1 + printW.length + 1; // in chars
auto buf = new ubyte[pathOffset + pathBufferSize * WCHAR.sizeof];
auto r = cast(REPARSE_DATA_BUFFER*)buf.ptr;
r.ReparseTag = mixin(reparseTagName);
r.ReparseDataLength = to!WORD(buf.length - mixin(q{r.} ~ reparseBufferName).offsetof);
auto pathBuffer = mixin(q{r.} ~ reparseBufferName).PathBuffer;
auto p = pathBuffer;
mixin(q{r.} ~ reparseBufferName).SubstituteNameOffset = to!WORD((p-pathBuffer) * WCHAR.sizeof);
mixin(q{r.} ~ reparseBufferName).SubstituteNameLength = to!WORD(targetW.length * WCHAR.sizeof);
p[0..targetW.length] = targetW;
p += targetW.length;
*p++ = 0;
mixin(q{r.} ~ reparseBufferName).PrintNameOffset = to!WORD((p-pathBuffer) * WCHAR.sizeof);
mixin(q{r.} ~ reparseBufferName).PrintNameLength = to!WORD(printW .length * WCHAR.sizeof);
p[0..printW.length] = printW;
p += printW.length;
*p++ = 0;
assert(p-pathBuffer == pathBufferSize);
mixin(extraInitialization);
DWORD dwRet; // Needed despite MSDN
DeviceIoControl(hLink, FSCTL_SET_REPARSE_POINT, buf.ptr, buf.length.to!DWORD(), null, 0, &dwRet, null).wenforce("DeviceIoControl");
}
void createWindowsSymlink(in char[] original, in char[] link, bool originalIsDir = isDir(original))
{
import core.sys.windows.winnt;
if (originalIsDir)
mkdir(link);
else
write(link, "");
scope (failure)
if (originalIsDir)
rmdir(link);
else
remove(link);
createReparsePoint!(
q{SymbolicLinkReparseBuffer},
q{r.SymbolicLinkReparseBuffer.Flags = link.isAbsolute() ? 0 : SYMLINK_FLAG_RELATIVE;},
q{IO_REPARSE_TAG_SYMLINK}
)(original, original, link);
}
void createJunction(in char[] original, in char[] link)
{
mkdir(link);
scope(failure) rmdir(link);
auto target = `\??\` ~ (cast(string)original).absolutePath((cast(string)link.dirName).absolutePath).buildNormalizedPath;
if (target[$-1] != '\\')
target ~= '\\';
createReparsePoint!(
q{MountPointReparseBuffer},
q{},
q{IO_REPARSE_TAG_MOUNT_POINT}
)(target, null, link);
}
} |
I have changed to use your approach, also added the enum |
Great, thanks!
If |
@atilaneves What do you think about adding these two Windows-only functions for symlinks? |
I don't think OS-specific code like this belongs in Phobos. |
|
Are symlinks OS specific anymore? They have existed in Windows since Vista. Or is this in reference to using OS specific API calls? Because we've essentially promised that we're going to rewrite std.stdio with OS APIs in V3, so I expect the OS API usage to explode higher. |
@LightBender The main problem on symlinking for Windows is that it either required development mode active or the user to be running your program as admin. That being said, there is still Junctions which can be done without neither of those requirements |
Feel free to improve however you see fit. I found it a little lacking to not have Windows support.
I can't understand how to write in phobos way, but the base implementation is there.