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.

Sign up for GitHub

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

Jump to bottom

Adds symlink to windows #8794

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Adds symlink to windows #8794

Changes from 2 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
61b65e3
Adds symlink to windows
MrcSnm Aug 8, 2023
23a5f64
Merge pull request #1 from MrcSnm/MrcSnm-patch-1
MrcSnm Aug 8, 2023
fed6310
Adds unittests, Ddoc and fixes some formatting problems
MrcSnm Aug 9, 2023
896cf77
Missed space
MrcSnm Aug 9, 2023
8025ef8
Updates documentation and make windows symlink accepts non existing f…
MrcSnm Aug 9, 2023
c67bd93
Updated to use CybarShadow approach
MrcSnm Aug 15, 2023
542af0e
Fix description
MrcSnm Aug 15, 2023
22b09c4
Minor change.
MrcSnm Aug 15, 2023
c68d465
Fixe's ddoc build
MrcSnm Aug 15, 2023
bc44a4b
Fix trailing whitespace
MrcSnm Aug 15, 2023
63894e7
Fixes style
MrcSnm Aug 15, 2023
97e215f
Fix import style
MrcSnm Aug 15, 2023
21cf5b7
Fix style check on cast
MrcSnm Aug 15, 2023
8d6b35d
Fixes style for slices
MrcSnm Aug 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions std/file.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3246,6 +3246,41 @@ if ((isSomeFiniteCharInputRange!RO || isConvertibleToString!RO) &&
}
}

version(Windows)
{
import core.sys.windows.w32api:_WIN32_WINNT;;
MrcSnm marked this conversation as resolved.
Show resolved Hide resolved
static if(_WIN32_WINNT >= 0x600) //WindowsVista or later
Copy link
Member

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

Copy link
Member

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.

Copy link
Member

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.

Copy link
Member

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.

Copy link
Contributor Author

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?

Copy link
Member

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:

https://github.com/reactos/reactos/blob/893715b72267723cb2a40052f4cea8453e2d1eb0/dll/win32/kernel32/kernel32_vista/vista.c#L215

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.

Copy link
Contributor Author

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");

Copy link
Member

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.

{
void symlink(string original, string link)
{
import core.stdc.string:strlen;
import core.sys.windows.winbase;
import core.sys.windows.windef:DWORD, MAX_PATH, LPWSTR;
import std.utf:toUTF16z;
import std.file:FileException;

DWORD typeFlag = 0; //File
if(std.file.isDir(original))
MrcSnm marked this conversation as resolved.
Show resolved Hide resolved
typeFlag = SYMBOLIC_LINK_FLAG_DIRECTORY;
typeFlag|= SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE;
MrcSnm marked this conversation as resolved.
Show resolved Hide resolved

if(link.length > MAX_PATH) link = `\\?\`~link;
if(original.length > MAX_PATH) original = `\\?\`~original;

if(!CreateSymbolicLinkW(link.toUTF16z, original.toUTF16z, typeFlag))
MrcSnm marked this conversation as resolved.
Show resolved Hide resolved
{
LPWSTR strBuffer;
DWORD length = FormatMessageW(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM, null, GetLastError(),0, cast(LPWSTR)&strBuffer, 0, null);
wchar[] str = new wchar[length];
str[] = strBuffer[0..str.length];
LocalFree(strBuffer);
import std.conv;
throw new FileException(original, str.to!string);
}
}
}
}

version (Posix) @safe unittest
{
if (system_directory.exists)
Expand Down
Loading