Skip to content

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

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

Merged
merged 1 commit into from
May 27, 2018
Merged

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

merged 1 commit into from
May 27, 2018

Conversation

sanmai
Copy link
Contributor

@sanmai 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);
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. How about now?

@sanmai sanmai changed the title Consider "executable" suffixes first on Windows [Process] Consider "executable" suffixes first on Windows May 19, 2018
@fabpot fabpot changed the base branch from 2.7 to 2.8 May 27, 2018 07:40
@fabpot
Copy link
Member

fabpot commented May 27, 2018

Thank you @sanmai.

@fabpot fabpot merged commit 9372e7a into symfony:2.8 May 27, 2018
fabpot added a commit that referenced this pull request May 27, 2018
…(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 2.7-bat-first branch May 27, 2018 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants