Skip to content

Conversation

@clarodeus
Copy link
Contributor

In the buildconf and configure batch files, Windows' cscript utility was being
run without the /e:jscript flag. This works on systems that have not had the
default .js file association changed, but if .js has been re-associated to
(say) an IDE, the batch files fail with the error message:

Input Error: There is no script engine for file extension ".js".

This PR fixes the issue by inserting the neccessary flag.

@clarodeus clarodeus changed the title Fixed script errors on Win32 Fix #79146: cscript can fail to run on some systems Jan 21, 2020
Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Ah, good catch. Thanks! I think one further occurance has to be changed as well:

cscript /nologo %PHP_BUILDCONF_PATH%\script\phpize.js %*

@clarodeus
Copy link
Contributor Author

Ah, good catch. Thanks! I think one further occurance has to be changed as well:

cscript /nologo %PHP_BUILDCONF_PATH%\script\phpize.js %*

Thanks for pointing that out! I've added the flag there as well.

@cmb69
Copy link
Member

cmb69 commented Jan 21, 2020

I've added the flag there as well.

Seems that didn't make it in.

Also, I think we should treat this as bug fix (having the .js extension associated with some other enginge is likely not uncommon), and apply the patch to PHP-7.3.

In the buildconf and configure batch files, Windows' cscript utility was being
run without the /e:jscript flag. This works on systems that have not had the
default .js file association changed, but if .js has been re-associated to
(say) an IDE, the batch files fail with the error message:

Input Error: There is no script engine for file extension ".js".
@clarodeus
Copy link
Contributor Author

It seems I did git commit --amend without having staged the changes 🤦

I double-checked this time, and it's definitely in the PR now.

As far as targeting PHP 7.3, would you like me to do the rebase?

Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM now.

No need to rebase, since the patch applies cleanly on PHP-7.3. I'll merge this PR ASAP.

@cmb69
Copy link
Member

cmb69 commented Jan 21, 2020

Applied as 3046e35. Thanks!

@cmb69 cmb69 closed this Jan 21, 2020
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.

2 participants