[FileSystem] verbose error when failing to create symlink in windows due to no administrator-rights#4577
Closed
ErikTrapman wants to merge 2 commits intosymfony:masterfrom
Closed
Conversation
…due to no administrator-rights
Member
There was a problem hiding this comment.
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
Author
There was a problem hiding this comment.
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?
Member
There was a problem hiding this comment.
Better error messages for the end user are always a good idea. But changing the way you implement it would be better indeed
Author
There was a problem hiding this comment.
Ok, thnx, i'll come up with another implementation later.
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 or disable the security policy, though that hasn't worked well for me). |
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. |
Member
|
Can you send a PR on the |
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. |
Author
|
Pull request placed in https://github.com/romainneutron/symfony/pulls |
craigmarvelley
pushed a commit
to craigmarvelley/symfony
that referenced
this pull request
Nov 26, 2013
Commits ------- a20fc68 Merge pull request symfony#1 from SamsonIT/FilesystemExceptions 8eca661 [FileSystem] explains possible failure of symlink creation in windows b1f8744 Add Changelog BC Break note 24eb396 [Filesystem] Added few new behaviors: Discussion ---------- [Filesystem] Consistence and enhancements for Filesystem Bug fix: no Feature addition: yes Backwards compatibility break: **yes** Symfony2 tests pass: yes Fixes the following tickets: None License of the code: MIT This 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 method The 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:42Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1367000) (merged 83cdd622 into 1e15f21). --------------------------------------------------------------------------- by fabpot at 2012-05-20T02:40:28Z To be consistent, we should throw exception whenever some operation fails. --------------------------------------------------------------------------- by romainneutron at 2012-05-20T21:10:23Z I 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 methods Add options for touch Add recursive option for chmod --------------------------------------------------------------------------- by travisbot at 2012-05-20T21:11:47Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1383619) (merged a4d1eeb8 into 1407f11). --------------------------------------------------------------------------- by travisbot at 2012-05-22T10:49:06Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1399027) (merged 7e14b6bd into 517ae43). --------------------------------------------------------------------------- by travisbot at 2012-05-22T10:58:10Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1399083) (merged 71852653 into 517ae43). --------------------------------------------------------------------------- by travisbot at 2012-05-22T11:18:44Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1399194) (merged 7645bad3 into 517ae43). --------------------------------------------------------------------------- by travisbot at 2012-05-23T18:21:47Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1414091) (merged b049d5b1 into 517ae43). --------------------------------------------------------------------------- by travisbot at 2012-05-23T18:26:19Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1414123) (merged 34903466 into 517ae43). --------------------------------------------------------------------------- by travisbot at 2012-05-29T16:07:26Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1467173) (merged b1d1eb2e into adf07f1). --------------------------------------------------------------------------- by travisbot at 2012-05-29T16:19:38Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1467261) (merged 42015ffa into adf07f1). --------------------------------------------------------------------------- by romainneutron at 2012-06-01T14:30:45Z Any 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:20Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1565540) (merged 8f55ddb6 into f8e68e5). --------------------------------------------------------------------------- by travisbot at 2012-06-08T11:41:45Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1566247) (merged 880312b6 into f8e68e5). --------------------------------------------------------------------------- by romainneutron at 2012-06-09T11:42:24Z I've added documentation to the Filesystem component : symfony/symfony-docs#1439 --------------------------------------------------------------------------- by travisbot at 2012-06-09T16:47:20Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1577754) (merged 5647ad41 into f8a09db). --------------------------------------------------------------------------- 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:44Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1611591) (merged c8b86c68 into c07e916). --------------------------------------------------------------------------- by fabpot at 2012-06-16T16:40:50Z You need to add a note about the BC breaks in the CHANGELOG file. --------------------------------------------------------------------------- by fabpot at 2012-06-16T16:43:20Z Also, 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:20Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1645757) (merged 925a8234 into 0b8b76b). --------------------------------------------------------------------------- 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:34Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1645981) (merged 634d6fb9 into 0b8b76b). --------------------------------------------------------------------------- by romainneutron at 2012-06-18T11:08:43Z As 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:58Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1646006) (merged 2f65945a into 0b8b76b).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.