8000 [Console] Do not check for "stty" using "exec" if that function is disabled by mvorisek · Pull Request #37466 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Console] Do not check for "stty" using "exec" if that function is disabled #37466

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

Jump to bottom
Merged
merged 1 commit into from
Jul 1, 2020
Merged

[Console] Do not check for "stty" using "exec" if that function is disabled #37466

merged 1 commit into from
Jul 1, 2020

Conversation

mvorisek
Copy link
Contributor
@mvorisek mvorisek commented Jul 1, 2020
Q A
Branch? 3.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets no
License MIT
Doc PR no

fixes PHP-CS-Fixer/PHP-CS-Fixer#5030

@mvorisek mvorisek mentioned this pull request Jul 1, 2020
Closed
@mvorisek
Copy link
Contributor Author
mvorisek commented Jul 1, 2020

Please verify AppVeyor tests and if ok, please merge asap.

@nicolas-grekas
Copy link
Member

Is this really enough? other parts of the component use shell_exec, and I'd suspect exec and shell_exec to be disabled at the same time.

I'm not sure it makes much sense also to run console commands without exec: if you can run a Symfony command, it means you can run any command, isn't it?

@mvorisek
Copy link
Contributor Author
mvorisek commented Jul 1, 2020

I'm not sure it makes much sense also to run console commands without exec: if you can run a Symfony command, it means you can run any command, isn't it?

Some users have these functions disabled for cli (like for web where it is very usual). Console library should not throw if exec is not available - because stty can not be called anyway then.

Example of usecase: PHP-CS-Fixer/PHP-CS-Fixer#5030

CS fixer and any library that does not need to start another process does not need exec or similar functions.

Is this really enough? other parts of the component use shell_exec, and I'd suspect exec and shell_exec to be disabled at the same time.

Yes, only this one.

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Jul 1, 2020
nicolas-grekas
nicolas-grekas approved these changes Jul 1, 2020
View reviewed changes
Copy link
Member
@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

I checked the source: all calls to shell_exec() in Console have an hasSttyAvailable() check before, that's why it's fine to check only for exec.

@nicolas-grekas nicolas-grekas changed the title Do not check for "stty" using "exec" if that function is disabled [Console] Do not check for "stty" using "exec" if that function is disabled Jul 1, 2020
@nicolas-grekas
Copy link
Member

Thank you @mvorisek.

@nicolas-grekas nicolas-grekas merged commit 9f6acd5 into symfony:3.4 Jul 1, 2020
@mvorisek mvorisek deleted the patch-1 branch July 1, 2020 18:24
@Gappa Gappa mentioned this pull request Mar 9, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

@chalasr chalasr chalasr approved these changes

Assignees
No one assigned
Labels
Bug Status: Reviewed
Projects
None yet
Milestone
3.4
Development

Successfully merging this pull request may close these issues.
4 participants
@mvorisek @nicolas-grekas @chalasr @carsonbot
0