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
Appearance settings

Added deprecation to cwd not existing Fixes #18249 #23708

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

Closed
wants to merge 25 commits into from
Closed

Added deprecation to cwd not existing Fixes #18249 #23708

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
a019543
Added deprecation to cwd not existing
alexbowers Jul 28, 2017
602a7d4
Upgrade Docs updated
alexbowers Jul 28, 2017
7103381
Fixed codestyle issues
alexbowers Jul 28, 2017
56a4a01
Renamed test
alexbowers Jul 28, 2017
f9225dd
Improved tests to check for deprecation properly
alexbowers Jul 28, 2017
4c9f3d0
Check if cwd is a directory, not a file
alexbowers Sep 2, 2017
f44bfd9
Remove unnecessary comment
alexbowers Sep 2, 2017
296cfe6
Improved documenting the deprecation
alexbowers Sep 2, 2017
5918f16
Improve error and make test pass again
alexbowers Sep 2, 2017
1dd14be
Use options properly
alexbowers Sep 2, 2017
06dd2eb
Run the commands without checking exit codes
alexbowers Sep 2, 2017
d684e70
Improve testing
alexbowers Sep 2, 2017
7e0141c
Forgot to put semi colon on
alexbowers Sep 2, 2017
1e4abf6
Fix test
alexbowers Sep 2, 2017
53b2c2e
Fixing coding style
alexbowers Sep 2, 2017
8830131
Windows exception
alexbowers Sep 3, 2017
d9746a2
Removed newline
alexbowers Sep 3, 2017
babbfea
Changed the upgrade doc wording
alexbowers Sep 3, 2017
d57ac4d
Codestyle fix
alexbowers Sep 3, 2017
b93e7ad
Fixes
alexbowers Sep 7, 2017
e8a2702
Behaviour -> behavior
alexbowers Sep 14, 2017
647fdff
New line
alexbowers Sep 14, 2017
29ad993
Handle test that should pass, failing, better
alexbowers Sep 14, 2017
8e9ddd2
Improvement fixes
alexbowers Oct 5, 2017
f46239d
Ran fabbot
alexbowers Oct 5, 2017
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
Prev Previous commit
Next Next commit
Windows exception
  • Loading branch information
@alexbowers
alexbowers committed Sep 14, 2017
commit 8830131eea6500e70d8f56b3cb9c5e8067be107b
4 changes: 4 additions & 0 deletions 4 src/Symfony/Component/Process/Process.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,10 @@ public function start(callable $callback = null/*, array $env = array()*/)
}

if (!is_dir($this->cwd)) {
if ('\\' === DIRECTORY_SEPARATOR) {
throw new RuntimeException("The provided cwd does not exist.");
}

@trigger_error('The provided cwd does not exist. Command is currently ran against getcwd(). This behaviour is deprecated since version 3.4 and will be removed in 4.0.', E_USER_DEPRECATED);
Copy link
Member

Choose a reason for hiding this comment

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

I think we should stick to my previous suggestion: on Windows, we should throw an exception instead of a deprecation, because on Windows, it's already broken, the getcwd fallback does not apply.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

Choose a reason for hiding this comment

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

behavior

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does Symfony follow british or american spelling across the board?

Copy link
Member

Choose a reason for hiding this comment

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

US

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, ill make that change this evening

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

}

Expand Down
9 changes: 4 additions & 5 deletions 9 src/Symfony/Component/Process/Tests/ProcessTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ protected function tearDown()
*/
public function testInvalidCwd()
{
if (strtoupper(substr(PHP_OS, 0, 3)) === 'WIN') {
if ('\\' === DIRECTORY_SEPARATOR) {
$this->markTestSkipped('Windows handles this automatically.');
}

Expand All @@ -67,13 +67,12 @@ public function testInvalidCwd()
}

/**
* @group legacy
* @expectedDeprecation The provided cwd does not exist. Command is currently ran against getcwd(). This behaviour is deprecated since version 3.4 and will be removed in 4.0.
* @expectedException \PHPUnit_Framework_Error_Warning
* @expectedException \Symfony\Component\Process\Exception\RuntimeException
* @expectedExceptionMessage The provided cwd does not exist.
*/
public function testInvalidCwdOnWindows()
{
if (strtoupper(substr(PHP_OS, 0, 3)) !== 'WIN') {
if ('\\' !== DIRECTORY_SEPARATOR) {
$this->markTestSkipped('Unix handles this automatically.');
}

Expand Down
Morty Proxy This is a proxified and sanitized view of the page, visit original site.