-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[WIP][FileSystem][Enhancement] Fallback to mklink for symlinks/junctions #18324
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
Conversation
Shouldn't we use mklink instead of junction? mklink is shipped by default since Vista, whereas junction.exe need a manual installation. (see e.g. http://superuser.com/questions/752538/mklink-vs-junction-exe) |
@nicolas-grekas i am also fine with
vs
|
one drawback of using |
Given that mklink is available since Vista, we could IMHO safely state that this feature requires it without excluding anyone. Not a blocker for me. |
Note that this would be a new feature, to be added to master, not 2.3 |
@nicolas-grekas a few questions: Overview
ad 1) introreading your note was quite a setback for me. personally i think the option to create reliable symlinks/junkctions on windows is a bug and not a new feature. hardcopy is really no option. if you have a complete data repo (~5gb) you want to symlink into your var directory the fallback hardlink option is no option. so its everytime a manual creation of symlinks. also it is a problem that this bugfix cannot easily/clean manually be backported to the a symfony version by a user. as the filesystem component is not installed as seperate component but with "symfony/symfony" which replaces "symfony/filesytem" the usual composer way of using a custom fork does not work. see also:
this leads to a
so my current "solution" is quite uncool and leads to a composer warning.
please lets call this a bug and let more symfony versions benefit of it. is there a room for discussion to get it in 2.3 or at least into 2.8? maybe add a feature flag for this, to allow user to enable or disable the bugfix/new feature. ad 2) Accept PR?as i noticed that this PR is quite a lot of work, especially creating tests and work around the ad 3) Need more conceptual guidelines/Supportas i played around with
ad 4)
|
@c33s that's a great analysis thanks! For your first point, I suggest we don't discuss if it's a bug or a feature here but wait for this discussion to decide (and contribute to it meanwhile). What is sure is that if you add any new public or protected methods to the Filesystem class, it will qualify as a feature. So that's our main boundary: no change in any public interface.
there is no need to bother: let's call it and if it fails, fallback to the current way. PHP always executes processes through cmd.exe so the command will almost always be here.
maybe later but I don't think it will provide much benefit (since I expect mklink to almost always work).
directory symlink is the only feature provided by Filesystem->symlink, we're good!
these would be new features, thus on master only
we use appveyor testing for every PR, which runs PHP on Windows already
this is also already tested and done in the existing test suite, and run on appveyor where the required privilege is set
no need, appveyor again
you could add a private isLink method on 2.3, but a public one would need to be on master
if fact, it is used a few times in the component with required workarounds when it returns false but should return true. I don't know if we need a feature full isLink implementation for these cases. Of course, that would be required if the method were made public, but since your target is 2.3, maybe not. You could add the public method on master later, after 2.3 is made to work here with mklink. |
I spent a bit of time playing with junctions+php on Windows. <?php
@mkdir('tata');
echo exec('mklink /J toto tata'), "\n";
echo 'realpath: '.realpath('./toto'), "\n";
echo 'file_exists: '.file_exists('./toto'), "\n";
echo 'is_link: '.@is_link('./toto'), "\n";
echo 'readlink: '.@readlink('./toto'), "\n";
echo 'filetype: '.@filetype('./toto'), "\n";
print_r(array_diff(lstat('./toto'), stat('./tata')));
echo 'unlink: '.@unlink('./toto'), "\n";
echo 'rmdir: '.@rmdir('./toto'), "\n";
echo exec('mklink /J toto tata'), "\n";
@rmdir('tata');
echo 'realpath: '.realpath('./toto'), "\n";
echo 'file_exists: '.file_exists('./toto'), "\n";
echo 'is_link: '.@is_link('./toto'), "\n";
echo 'readlink: '.@readlink('./toto'), "\n";
echo 'filetype: '.@filetype('./toto'), "\n";
print_r(array_diff(lstat('./toto'), stat('./tata')));
echo 'unlink: '.@unlink('./toto'), "\n";
echo 'rmdir: '.@rmdir('./toto'), "\n"; |
@c33s Any feedback? |
@fabpot currently to much work to do, should be able to continue working on this in a few weeks |
@c33s Sorry to be pushy, but do you think you will be able to finish this one soon? If not, we can create an issue referencing this PR and close it so that anyone interested can take over your work. Thanks. |
Closing this PR as there i no activity and it's far from being ready. |
currently creating symlinks with php on windows are a real pain. a console with admin rights is needed
or you have to change the privileges for "Create symbolic links" (more information on that, and some more info to that)
i use "symlinks" (junctions) since 2011, with the great tool junction.exe from sysinternals. this tool simply work.
it can only handle directories and no files, but most of the time this is exactly what you need.
running a console as administrator or changing the group policy privileges is not always possible but putting a simple exe file in a directory which is in a directory refered in th environmental path variable, is often possible and easy.
so i made a small POC change in the filesystem component to allow fallback to junction on windows to ask if such a pull request would be accepted or if it have to live as hack (not that easy to replace the filesystem component in a symfony project)
currently i "overload" the filesystem class with composer:
things todo: