8000 [Bridge/PhpUnit] Prefer $_SERVER['argv'] over $argv · Pull Request #25304 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Bridge/PhpUnit] Prefer $_SERVER['argv'] over $argv #25304

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 5 commits into from
Closed

[Bridge/PhpUnit] Prefer $_SERVER['argv'] over $argv #25304

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9f8c89b
Prefer $_SERVER['argv'] over $argv.
ricardodevries Dec 4, 2017
67cc253
Put back some code that was removed for debug purposes.
ricardodevries Dec 4, 2017
3c28b42
Put back the correct variable names.
ricardodevries Dec 4, 2017
8fbf912
Reverting changes and init the variables at the beginning.
ricardodevries Dec 4, 2017
6140de9
Add global variables.
ricardodevries Dec 4, 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
Next Next commit
Prefer $_SERVER['argv'] over $argv.
  • Loading branch information
@ricardodevries
ricardodevries committed Dec 4, 2017
commit 9f8c89bd4daa73fc1f2503525b3616df01bd58cf
7 changes: 3 additions & 4 deletions src/Symfony/Bridge/PhpUnit/bin/simple-phpunit
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ EOPHP
}

$components = array();
$cmd = array_map('escapeshellarg', $argv);
$cmd = array_map('escapeshellarg', $_SERVER['argv']);
$exit = 0;

if (isset($argv[1]) && 'symfony' === $argv[1] && !file_exists('symfony') && file_exists('src/Symfony')) {
Copy link
Member

Choose a reason for hiding this comment

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

if you happen to be running this script in a context where $argv is not defined, then this line will always eval to false, which means it's useless
$argv is expected to be always defined in the current code. A better patch would be to always ensure the vars are defined

Copy link
Author

Choose a reason for hiding this comment

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

So the $cmd should be $argv to be sure it is always defined. That means that this is correct code but leaves $argv still used, not sure where to init the variable $_SERVER['argv'] so that $argv will be intact.

Copy link
Member

Choose a reason for hiding this comment

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

just start the script with:

$argv = isset($_SERVER['argv']) ? $_SERVER['argv'] : array();
$argc = isset($_SERVER['argc']) ? $_SERVER['argc'] : 0;

Copy link
Author

Choose a reason for hiding this comment

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

Ah yea. I was thinking way too deep here. Thank you for your suggestion. I reverted everything in 8fbf912 and added the lines at the beginning.

Expand Down Expand Up @@ -230,9 +230,8 @@ if ($components) {
if (!class_exists('SymfonyBlacklistSimplePhpunit', false)) {
class SymfonyBlacklistSimplePhpunit {}
}
array_splice($argv, 1, 0, array('--colors=always'));
$_SERVER['argv'] = $argv;
$_SERVER['argc'] = ++$argc;
Copy link
Member

Choose a reason for hiding this comment

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

this line should not be removed!

Copy link
Author

Choose a reason for hiding this comment

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

I forgot to put them back while I was debugging. They are back in 67cc253.

array_splice($_SERVER['argv'], 1, 0, array('--colors=always'));
Copy link
Member

Choose a reason for hiding this comment

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

ok, in fact, this change introduces a bug
both $_SERVER['argv'] and $argv must be updated
this breaks the second


include "$PHPUNIT_DIR/phpunit-$PHPUNIT_VERSION/phpunit";
}

Expand Down
0