This PR was merged into the 2.3 branch.
Discussion
----------
[Process] fix some typos and refactor some code
| Q | A
| ------------- | ---
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
Commits
-------
b422613 [Process] fix some typos and refactor some code
This PR was merged into the 2.3 branch.
Discussion
----------
[2.3][Process] Fix escaping on Windows
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| License | MIT
Windows escaping is broken since the last merges.
After digging more on Windows escaping, I realised some things:
- We forbid environment variable expansion by escaping `%APPDATA%` to `^%"APPDATA"^%`
- We explicitly ask for variable expansion at runtime (running the command line with the [`/V:ON`](https://github.com/symfony/symfony/blob/2.3/src/Symfony/Component/Process/Process.php#L235) flag). Running a command containing `!APPDATA!` will be escaped and expanded (our previous rule is easily overriden)
- On platform that are not windows, we use strong escaping that prevents any variable expansion (`$PATH` will be escaped to `'$PATH'` that is not interpreted as the current PATH)
We have three possibilities:
- Keep this behavior as this.
- Prefer a consistent API and use a strong escaping strategy everywhere, but it would result in a BC break (see #8975).
- Allow environment variable expansion and escape `%APPDATA%` to `"%APPDATA%"`
Any thoughts about this ?
Commits
-------
0f65f90 [Process] Fix escaping on Windows
This PR was merged into the 2.3 branch.
Discussion
----------
[2.3][Process] Fix unit tests in sigchild disabled environment
| Q | A
| ------------- | ---
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| License | MIT
We've been a bit fast when merging #10480 and tests were broken in the last update I did.
Commits
-------
5f6ee12 [Process] Fix unit tests in sigchild disabled environment
This PR was merged into the 2.3 branch.
Discussion
----------
[Process] Fixed fatal errors in getOutput and getErrorOutput when process was not started
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #10022
| License | MIT
This PR replaces #9452 and address the latest changes.
Side note : I've not updated `getExitCode`, `getExitCodeText` and `isSuccessful` as they were explicitly tested to return null in case the process is not started or terminated. I think it would be a BC break to do that.
Commits
-------
449fe01 [Process] Trow exceptions in case a Process method is supposed to be called after termination
0ae6858 [Process] fixed fatal errors in getOutput and getErrorOutput when process was not started
This PR was merged into the 2.3 branch.
Discussion
----------
[2.3][Process] Make Process::start non-blocking on Windows platform
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #9755
| License | MIT
This PR is a proposition that solves issue #9755.
Let me sum-up what problems we have on Windows platform:
- We can not use pipes with `proc_open` on Windows because `stream_set_blocking` does not work on this platform, resulting in blocking pipes and a blocking `Process::start` (see https://bugs.php.net/bug.php?id=51800, https://bugs.php.net/bug.php?id=47918 and https://bugs.php.net/bug.php?id=50856).
- We can not use file handles for both `STDOUT` and `STDERR` as it might produce corrupted content (see https://bugs.php.net/bug.php?id=65650).
- We currently use file handles for `STDOUT` and pipe for `STDERR` but it makes `Process::start` blocking.
The solution I propose here is to use native redirections, write `STDERR` / `STDOUT` in temporary files, read these files, finally cleanup.
It works pretty well, all tests pass on Windows. Better : [previous tests that were specific to this platform](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Process/Tests/AbstractProcessTest.php#L720-725) disappear as unix one are now okay :).
The drawback of this is the need of using `taskkill`: When stopping a process (`Process::stop` or in case of a timeout reached) `proc_terminate` does not kill the underlying process properly, only the `cmd` parent. The underlying process run by Process still runs after the `proc_terminate` call, having a lock on the temporary files we're using for getting the output, avoiding PHP to remove them (or any user on system) until the underlying process has finish to run).
So I use the `taskkill` Windows command to terminate the underlying process. This works well but I've to admit it's not pretty. If we do not use this hack, let's face that the underlying process is not stopped.
Last but not least, let's deal with the case we're running on Windows platform with PHP having --enable-sigchild environment (I don't know if it can really happen)
In this case:
- we can not retrieve the `pid`
- we can not `taskkill` the underlying process
- it runs
- locks remain on temporary files, we might not be able to remove them
We can't really deal with this.
Feedback more than welcome
Commits
-------
1f5bf32 [Process] Make Process::start non-blocking on Windows platform
This PR was merged into the 2.3 branch.
Discussion
----------
[server:run] [#10461] Add docs about custom environment
| Q | A
| ------------- | ---
| Fixed tickets | #10461, https://github.com/symfony/symfony-docs/issues/3688
| License | MIT
It's better to document this specific usecase in the command help.
Commits
-------
6871a61 [#10461] Add docs about custom environment
This PR was submitted for the 2.3-dev branch but it was merged into the 2.3 branch instead (closes#10460).
Discussion
----------
Typo fix at UploadedFile.php documentation
Commits
-------
ced3f04 Typo fix at UploadedFile.php documentation
This PR was merged into the 2.3 branch.
Discussion
----------
[2.3][Process] Fix random failures in test suite on TravisCI
| Q | A
| ------------- | ---
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| License | MIT
Commits
-------
783e377 [Process] Avoid failures because of slow IOs
238565e [Process] Avoid failure because of a slow process
173f8c5 [Process] Avoid failure when calling Process::stop in edge cases
This PR was submitted for the 2.3-dev branch but it was merged into the 2.3 branch instead (closes#10436).
Discussion
----------
Update DefaultAuthenticationSuccessHandler.php
[Security] [Http] [Authentication] Fixed class description of DefaultAuthenticatioNSuccessHander
| Q | A
| ------------- | ---
| Fixed tickets | none
| License | MIT
Commits
-------
b458551 Update DefaultAuthenticationSuccessHandler.php
This PR was merged into the 2.3 branch.
Discussion
----------
[2.3][Process] Fix quoted arguments escaping
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| License | MIT
This PR replaces #8972
Commits
-------
de681cb [Process] Add tests on ProcessUtils::escapeArgument
85fb495 [Process] Fix: Arguments including space and quote are not correctly escaped (win)
This PR was merged into the 2.3 branch.
Discussion
----------
[DomCrawler] Fixed incorrect value name conversion in getPhpValues() and getPhpFiles()
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #6908
| License | MIT
This PR replaces #10193
Commits
-------
89c599e [DomCrawler] Add tests for recursive cases of getPhpValues() and getPhpFiles()
e961f57 [DomCrawler] Fixed incorrect value name conversion in getPhpValues() and getPhpFiles()
This PR was submitted for the 2.3-dev branch but it was merged into the 2.3 branch instead (closes#10447).
Discussion
----------
[Config][Loader] Code style fix
| Q | A
| ------------- | ---
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | -
Commits
-------
6d84c04 Minor style fix
This PR was submitted for the 2.3-dev branch but it was merged into the 2.3 branch instead (closes#10423).
Discussion
----------
[Config] XmlUtils::convertDomElementToArray does not handle '0'
`XmlUtils::convertDomElementToArray` does not handle `0` as a value of a text node, and interprets it as `null`.
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | maybe? (if someone is depending on the previous behaviour.)
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | N/A
| License | MIT
Commits
-------
e56ac59 convertDomElementToArray should handle zero values
This PR was merged into the 2.3 branch.
Discussion
----------
adding http:// to server:run output to make it clickable
|Q |A |
|--- |---|
|Bug Fix? |yes |
|New Feature? |n |
|BC Breaks? |n |
|Deprecations?|n |
|Tests Pass? |n |
|Fixed Tickets| #10430 |
|License |MIT|
|Doc PR | no |
Commits
-------
6498518 prefixed http:// to url output on server:run command in order to make it clickable
This PR was merged into the 2.3 branch.
Discussion
----------
[Process] Fixed data in pipe being truncated if not read before process termination
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #9409
| License | MIT
| Doc PR | N/A
This is a repeat of the botched pull request #9630.
Commits
-------
7e51913 Fixed data in pipe being truncated if not read before process termination
This PR was merged into the 2.3 branch.
Discussion
----------
[2.3][Process] Fix#9160 : escaping an argument with a trailing backslash on windows fails
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #9160
| License | MIT
Commits
-------
10e903a [Process] Fix#9160 : escaping an argument with a trailing backslash on windows fails
This PR was merged into the 2.3 branch.
Discussion
----------
[Process] Fix some unit tests that create the process object instead of delegate it to the implementation
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| License | MIT
This is a minor error ; creating the process must be delegated to the implementation
Commits
-------
227b85b [Process] Fix some unit tests that create the process object instead of delegate it to the implementation
This PR was merged into the 2.3 branch.
Discussion
----------
[2.3][Process] Make process tests more accurate on exception messages
| Q | A
| ------------- | ---
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| License | MIT
I need exception message to be checked more accurately in my upcoming PR about windows.
I decided to make a separated PR as the other one will probably take some time before being merged
Commits
-------
1b1768a [Process] Make process tests more accurate on exception messages
This PR was merged into the 2.3 branch.
Discussion
----------
[Process] Fix process status in TTY mode
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| License | MIT
When running a process with TTY mode, status in automatically set to `terminated`
once it's started.
It's wrong for two reasons :
- The status of the process is not yet terminated.
- The exitcode value is never caught
Commits
-------
51c70f8 [Process] Fix process status in TTY mode
This PR was merged into the 2.3 branch.
Discussion
----------
[Process] Fix comparisons against process exitcode
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| License | MIT
Use identical comparison instead of equal. In case the exitcode has not been caught, `Process::isSuccessful` returns true instead of false. It should not happen, but it's much more clean like that.
Commits
-------
d66f63f [Process] Use assertSame instead of assertEquals to avoid comparison against `null`
When running a process with TTY mode, status in automatically set to `terminated`
once it's started.
It's wrong for two reasons :
- The status of the process is not yet terminated.
- The exitcode value is never caught
