Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[FileSystem] verbose error when failing to create symlink in windows due to no administrator-rights#4577

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

ErikTrapman
Copy link

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes

This PR aims to tell a windows-user about the most common pitfall when the creation of a symlink fails: lack of administrator-rights. IMO a valuable addition as this is the first and biggest problem all windows-users face working with symlinks.
This PR clarifies the possible problem by reading the windows-error-code and tells what's most likely causing the failure.

symlink($originDir, $targetDir);
try {
symlink($originDir, $targetDir);
} catch (\ErrorException $e) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

this is wrong as it relies on the fact an error handler is registered (and turns errors into exceptions), which should not be considered as a requirement

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

True. I could change it by adding an @ to the symlink-statement and reading error_get_last on a false result and then throw an exception with the appr. message.
And how do you see the overall value of this addition?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Better error messages for the end user are always a good idea. But changing the way you implement it would be better indeed

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Ok, thnx, i'll come up with another implementation later.

@Seldaek
Copy link
Member

Would be even better if the exception message had a link to some cookbook page explaining how to enable this stuff (run as admin ordisable the security policy, though that hasn't worked well for me).

@ErikTrapman
Copy link
Author

@Seldaek i'd love that too, but there's probably not 1 knowledge base article from Microsoft that covers that for all the different windows versions that are available and upcoming.
And i'm not sure it's up to us to provide information on how to change that, adding general "please consult your local sysadmin" might be as far as we have to go. Not everybody is in the position to change this settings on his own.

@travisbot
Copy link

This pull requestpasses (mergedd0d848f intoc07e916).

@travisbot
Copy link

This pull requestfails (merged5879a54 intoc07e916).

@fabpot
Copy link
Member

Can you send a PR on theromainneutron:FilesystemExceptions branch (see#4330) as he has already worked on such enhancements? Thanks.

@ErikTrapman
Copy link
Author

@fabpot sure, i'm working on sending a pull-request to the branch of romainneutron. If the PR is there i'll close this one.

@ErikTrapman
Copy link
Author

Pull request placed inhttps://github.com/romainneutron/symfony/pulls

craigmarvelley pushed a commit to craigmarvelley/symfony that referenced this pull requestNov 26, 2013
Commits-------a20fc68 Merge pull requestsymfony#1 from SamsonIT/FilesystemExceptions8eca661 [FileSystem] explains possible failure of symlink creation in windowsb1f8744 Add Changelog BC Break note24eb396 [Filesystem] Added few new behaviors:Discussion----------[Filesystem] Consistence and enhancements for FilesystemBug fix: noFeature addition: yesBackwards compatibility break: **yes**Symfony2 tests pass: yesFixes the following tickets: NoneLicense of the code: MITThis PR adds features and introduce a backward compatibility break.features :- whenever an action fails, a \RuntimeException is thrown- add access to the second and third arguments of ``touch`` function- add a recursive option for chmod- add a chown method- add a chgrp methodThe backward compatibility break happens in the mkdir method : Before this PR, a boolean is returned ; true if all directories were created, false otherwise.It now returns nothing.---------------------------------------------------------------------------by travisbot at 2012-05-18T14:26:42ZThis pull request [passes](http://travis-ci.org/symfony/symfony/builds/1367000) (merged 83cdd622 into1e15f21).---------------------------------------------------------------------------by fabpot at 2012-05-20T02:40:28ZTo be consistent, we should throw exception whenever some operation fails.---------------------------------------------------------------------------by romainneutron at 2012-05-20T21:10:23ZI fix the consistency ; mkdir now throws an exception if a directory creation fails.This introduce a BC break, see PR message which has been updated with all features and BC break.Added chgrp and chown methodsAdd options for touchAdd recursive option for chmod---------------------------------------------------------------------------by travisbot at 2012-05-20T21:11:47ZThis pull request [passes](http://travis-ci.org/symfony/symfony/builds/1383619) (merged a4d1eeb8 into1407f11).---------------------------------------------------------------------------by travisbot at 2012-05-22T10:49:06ZThis pull request [passes](http://travis-ci.org/symfony/symfony/builds/1399027) (merged 7e14b6bd into517ae43).---------------------------------------------------------------------------by travisbot at 2012-05-22T10:58:10ZThis pull request [passes](http://travis-ci.org/symfony/symfony/builds/1399083) (merged 71852653 into517ae43).---------------------------------------------------------------------------by travisbot at 2012-05-22T11:18:44ZThis pull request [passes](http://travis-ci.org/symfony/symfony/builds/1399194) (merged 7645bad3 into517ae43).---------------------------------------------------------------------------by travisbot at 2012-05-23T18:21:47ZThis pull request [fails](http://travis-ci.org/symfony/symfony/builds/1414091) (merged b049d5b1 into517ae43).---------------------------------------------------------------------------by travisbot at 2012-05-23T18:26:19ZThis pull request [fails](http://travis-ci.org/symfony/symfony/builds/1414123) (merged 34903466 into517ae43).---------------------------------------------------------------------------by travisbot at 2012-05-29T16:07:26ZThis pull request [passes](http://travis-ci.org/symfony/symfony/builds/1467173) (merged b1d1eb2e intoadf07f1).---------------------------------------------------------------------------by travisbot at 2012-05-29T16:19:38ZThis pull request [passes](http://travis-ci.org/symfony/symfony/builds/1467261) (merged 42015ffa intoadf07f1).---------------------------------------------------------------------------by romainneutron at 2012-06-01T14:30:45ZAny news about this PR ?---------------------------------------------------------------------------by stloyd at 2012-06-08T09:57:39Z@romainneutron You need to [squash](http://www.silverwareconsulting.com/index.cfm/2010/6/6/Using-Git-Rebase-to-Squash-Commits) your commits, and add more proper message in squashed commit i.e.:> [Filesystem]  Added few new behaviors:* whenever an action fails, a `RuntimeException` is thrown* add access to the second and third arguments of `touch()` function* add a recursive option for `chmod()`* add a `chown()` method* add a `chgrp()` method> BC break: `mkdir()` function throw exception in case of failture instead of returning Boolean value.---------------------------------------------------------------------------by romainneutron at 2012-06-08T10:59:55Z@stloyd squash done !---------------------------------------------------------------------------by travisbot at 2012-06-08T11:26:20ZThis pull request [passes](http://travis-ci.org/symfony/symfony/builds/1565540) (merged 8f55ddb6 intof8e68e5).---------------------------------------------------------------------------by travisbot at 2012-06-08T11:41:45ZThis pull request [passes](http://travis-ci.org/symfony/symfony/builds/1566247) (merged 880312b6 intof8e68e5).---------------------------------------------------------------------------by romainneutron at 2012-06-09T11:42:24ZI've added documentation to the Filesystem component :symfony/symfony-docs#1439---------------------------------------------------------------------------by travisbot at 2012-06-09T16:47:20ZThis pull request [passes](http://travis-ci.org/symfony/symfony/builds/1577754) (merged 5647ad41 intof8a09db).---------------------------------------------------------------------------by stloyd at 2012-06-13T14:47:31Z@romainneutron You probably need to rebase your code as some changes were merge into master for `Filesystem`.---------------------------------------------------------------------------by romainneutron at 2012-06-13T15:17:31Z@stloyd rebase OK !by the way, do you have any idea when/if this PR will be merged ?---------------------------------------------------------------------------by travisbot at 2012-06-13T15:20:44ZThis pull request [passes](http://travis-ci.org/symfony/symfony/builds/1611591) (merged c8b86c68 intoc07e916).---------------------------------------------------------------------------by fabpot at 2012-06-16T16:40:50ZYou need to add a note about the BC breaks in the CHANGELOG file.---------------------------------------------------------------------------by fabpot at 2012-06-16T16:43:20ZAlso, instead of using `\RuntimeException`, I would create a custom exception like we have done in other components (an interface + a RuntimeException that implements the interface and extends \RuntimeException). The exception name can be something like `IOException`.---------------------------------------------------------------------------by travisbot at 2012-06-18T10:11:20ZThis pull request [fails](http://travis-ci.org/symfony/symfony/builds/1645757) (merged 925a8234 into0b8b76b).---------------------------------------------------------------------------by stloyd at 2012-06-18T10:14:52Z@fabpot Anything blocking merge of this PR ? (tests are failing because of issue in master, not releted to this PR)---------------------------------------------------------------------------by romainneutron at 2012-06-18T10:29:20Z@fabpot@stloyd the latest push was just a rebase push for PR 4577 (symfony#4577)I'm currently fixing the Exception and changelog things, I'll push very soon---------------------------------------------------------------------------by romainneutron at 2012-06-18T10:44:38Z@fabpot I've added the exception and the exception interface, add the changelog info---------------------------------------------------------------------------by travisbot at 2012-06-18T10:53:34ZThis pull request [fails](http://travis-ci.org/symfony/symfony/builds/1645981) (merged 634d6fb9 into0b8b76b).---------------------------------------------------------------------------by romainneutron at 2012-06-18T11:08:43ZAs reported by@stloyd the PR is failing due to an issue in the master, I re-push and trig the PR build when this issue is solved---------------------------------------------------------------------------by travisbot at 2012-06-18T11:16:58ZThis pull request [fails](http://travis-ci.org/symfony/symfony/builds/1646006) (merged 2f65945a into0b8b76b).
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

5 participants
@ErikTrapman@Seldaek@travisbot@fabpot@stof
[8]ページ先頭
©2009-2025 Movatter.jp