search

erlware / relx

Sane, simple release creation for Erlang
http://erlware.github.io/relx
Apache License 2.0
697 stars 232 forks source link

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

Closed venimus closed 5 years ago

venimus commented 5 years ago

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 https://github.com/aeternity/relx/pull/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: https://github.com/achadwick/styrene/issues/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

tsloughter commented 5 years ago

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

tsloughter commented 5 years ago

And actually, @venimus do you have any thoughts on this other open PR https://github.com/erlware/relx/pull/719

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

venimus commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

Great, thanks!

tolbrino commented 5 years ago

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