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.

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

[Process] Consider "executable" suffixes first on Windows #27303

Merged
merged 1 commit into from May 27, 2018

Conversation

Projects
None yet
5 participants
@sanmai
Contributor

sanmai commented May 18, 2018

Q A
Branch? 2.7
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR n/a

Executable finder should consider "executable" suffixes first on Windows because we basically ignore executability on Windows (on the lines below changed), which leads, for example, to finding usually-non-executable phpunit file first where both phpunit and phpunit.bat are present.

I may miss something here, so please tell me if this makes any sense.

Same change against master: #27301

$this->assertSamePath($target.'.BAT', $result);
unlink($target);

This comment has been minimized.

@xabbuh

xabbuh May 18, 2018

Member

cleaning up the fragments should not only happen when the test succeeds

This comment has been minimized.

@sanmai

sanmai May 18, 2018

Contributor

Good point. How about now?

@sanmai sanmai changed the title from Consider "executable" suffixes first on Windows to [Process] Consider "executable" suffixes first on Windows May 19, 2018

@fabpot

fabpot approved these changes May 27, 2018

@fabpot fabpot changed the base branch from 2.7 to 2.8 May 27, 2018

@fabpot

This comment has been minimized.

Member

fabpot commented May 27, 2018

Thank you @sanmai.

@fabpot fabpot merged commit 9372e7a into symfony:2.8 May 27, 2018

1 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request May 27, 2018

bug #27303 [Process] Consider "executable" suffixes first on Windows …
…(sanmai)

This PR was squashed before being merged into the 2.8 branch (closes #27303).

Discussion
----------

[Process] Consider "executable" suffixes first on Windows

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        | n/a

Executable finder should consider "executable" suffixes first on Windows because we basically ignore executability on Windows (on the lines below changed), which leads, for example, to finding usually-non-executable `phpunit` file first where both `phpunit` and `phpunit.bat` are present.

I may miss something here, so please tell me if this makes any sense.

Same change against master: #27301

Commits
-------

9372e7a [Process] Consider \"executable\" suffixes first on Windows

@sanmai sanmai deleted the sanmai:2.7-bat-first branch May 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment