Skip to content
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

Fix run-script not setting Path correctly on Windows #10700

Merged
merged 2 commits into from Apr 6, 2022
Merged

Fix run-script not setting Path correctly on Windows #10700

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6c7e37d
Fix run-script not setting Path correctly on Windows
xPaw Apr 3, 2022
45ba6e3
Add docs
Seldaek Apr 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/Composer/EventDispatcher/EventDispatcher.php
View file
Open in desktop
Expand Up @@ -571,7 +571,11 @@ protected function popEvent(): ?string
private function ensureBinDirIsInPath(): void
{
$pathEnv = 'PATH';
if (false === Platform::getEnv('PATH') && false !== Platform::getEnv('Path')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm pretty sure the Platform::getEnv helper is used for a good reason and should not be removed :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Platform::getEnv will fallback to getenv which is not case sensitive, making this check moot.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, that's still not right. Env variables may exist in $_SERVER only, and not in putenv.

Copy link
Contributor Author

@xPaw xPaw Apr 4, 2022 •
edited

Choose a reason for hiding this comment

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

Well, this is the code that was used before it was changed to getEnv. I don't really think getenv matters here because on Windows, Path (but not PATH) should exist in $_SERVER. All we're doing is just getting the right capitalization.

Regardless, if you have a better idea on how to fix scripts not running on Windows in 2.3, do suggest it.


// checking if only Path and not PATH is set then we probably need to update the Path env
// on Windows getenv is case-insensitive so we cannot check it via Platform::getEnv and
// we need to check in $_SERVER directly
if (!isset($_SERVER[$pathEnv]) && isset($_SERVER['Path'])) {
$pathEnv = 'Path';
}

Expand Down