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.

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

Fix "'findstr' is not recognized as ..." #724

Merged
merged 1 commit into from
May 27, 2019
Merged

Fix "'findstr' is not recognized as ..." #724

merged 1 commit into from
May 27, 2019

Conversation

venimus
Copy link
Contributor

@venimus venimus commented May 27, 2019

Make sure findstr is discoverable

In certain cases findstr could not be found in path (ie command is executed by an external process which does not properly import env).

For instance bundling an app with https://github.com/achadwick/styrene + relx (ref aeternity#4 )

Styrene overrides the default path variable removing the Windows\System32 entry which makes findstr unavailable when script is executed from the launcher aeternity.exe resulting in:

'findstr' is not recognized as an internal or external command,
operable program or batch file.
'findstr' is not recognized as an internal or external command,
operable program or batch file.
'findstr' is not recognized as an internal or external command,
operable program or batch file.
test.cmd exited with status 0.
Press return to close this window.

rel: achadwick/styrene#23

Patch just adds %SystemRoot%\System32 in %PATH% + turn off command echo (some commands miss the @)

Alternative fix could be to replace all occurrences of findstr with %SystemRoot%\System32\findstr

Make sure findstr is discoverable

(cherry picked from commit 8848196)
@tsloughter
Copy link
Member

Thanks. I'm guessing this is fine, but will first try to get someone with windows to take a look :)

@tsloughter
Copy link
Member

And actually, @venimus do you have any thoughts on this other open PR #719

I'd like to get both of these taken care of today so I can publish a new version.

@venimus
Copy link
Contributor Author

venimus commented May 27, 2019

Actually I use the forked version https://github.com/aeternity/relx which already includes the fix and it works properly on windows 10.

https://github.com/aeternity/relx/blob/master/priv/templates/extended_bin_windows#L50

fix is already applied by @tolbrino there

@venimus
Copy link
Contributor Author

venimus commented May 27, 2019

to get more info -> https://ss64.com/nt/for_cmd.html

here in the for cycle there is 'backquote' flag (ie. usebackq) specified to escape the command (i guess because %vm_args% might include quotes and the variable is expanded before passing to findstr ).
Currently the command is wrapped by single quotes which makes the flag useless in the first place. However to properly escape the command back quotes should be used because of %vm_args%. So that fix seems logical to me.

@tsloughter
Copy link
Member

Great, thanks!

@tsloughter tsloughter merged commit ba7be7e into erlware:master May 27, 2019
@tolbrino
Copy link
Contributor

@venimus I seem to have forgotten to create a PR for this. 👍

@venimus venimus deleted the findstr-path branch June 7, 2019 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants