8000 [Process] Fix BC break: "exec" should remain an internal "detail" by nicolas-grekas · Pull Request #22487 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Process] Fix BC break: "exec" should remain an internal "detail" #22487

New issue 8000

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
Apr 25, 2017
Merged

[Process] Fix BC break: "exec" should remain an internal "detail" #22487

merged 1 commit into from
Apr 25, 2017

Conversation

nicolas-grekas
Copy link
Member
Q A
Branch? 3.3
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #22476
License MIT
Doc PR -

@nicolas-grekas nicolas-grekas added this to the 3.3 milestone Apr 20, 2017
@stof
Copy link
Member
stof commented Apr 20, 2017

this still does not fix the BC break when the executed code already used exec

@nicolas-grekas
Copy link
Member Author

accepting an array is new since 3.3, so this cannot happen

@stof
Copy link
Member
stof commented Apr 20, 2017

oh, it is indeed only for arrays

@stof
Copy link
Member
stof commented Apr 20, 2017 •
edited
Loading

@nicolas-grekas it can happen when using ProcessBuilder, as this one uses the new API under the hood, which is precisely why melody was impacted btw

@nicolas-grekas
Copy link
Member Author

I don't see what you mean, can you post a reproducer that fails with the current patch?

@stof
Copy link
Member
stof commented Apr 21, 2017

ProcessBuilder can be used to create processes using exec already today.
And in 3.3, a second exec will be added on them, due to using an array.

Btw, I think you also broke the setPrefix feature of the ProcessBuilder, as adding it in the array will then make Process escape it

@nicolas-grekas
Copy link
Member Author

I'm really sorry, but either you're wrong, or I totally miss the point.
the prefix in 2.7/3.2 is escaped. See
https://github.com/symfony/symfony/blob/2.7/src/Symfony/Component/Process/ProcessBuilder.php#L270

Please provide me some code sample to help me get your point if it's still valid...

@stof
Copy link
Member
stof commented Apr 25, 2017

ah indeed, I forgot it is.

@fabpot
Copy link
Member
fabpot commented Apr 25, 2017

Thank you @nicolas-grekas.

@fabpot fabpot merged commit eedcece into symfony:master Apr 25, 2017
fabpot added a commit that referenced this pull request Apr 25, 2017
@fabpot
bug #22487 [Process] Fix BC break: "exec" should remain an internal "…
63dca04 
…detail" (nicolas-grekas)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[Process] Fix BC break: "exec" should remain an internal "detail"

| Q             | A
| ------------- | ---
| Branch?       | 3.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #22476
| License       | MIT
| Doc PR        | -

Commits
-------

eedcece [Process] Fix BC break: "exec" should remain an internal "detail"
@nicolas-grekas nicolas-grekas deleted the process-fix branch April 25, 2017 17:43
@stephanvierkant stephanvierkant mentioned this pull request May 26, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Bug Process Ready Status: Needs Review
Projects
None yet
Milestone
3.3
Development

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