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

[bugfix] windows incompatibilty #376

Merged
merged 1 commit into from
Aug 21, 2017

Conversation

SylTi
Copy link
Contributor

@SylTi SylTi commented Aug 17, 2017

Currently running 'truffle test' or any other truffle cmd on windows open the truffle.js file. Renaming it solve that issue.

@frangio
Copy link
Contributor

frangio commented Aug 19, 2017

Thanks for contributing @SylTi!

Do you know if there are other issues besides this with regards to Windows compatibility?

@Neurone
Copy link
Contributor

Neurone commented Aug 19, 2017

This is not releated to Windows in general: if you use Power Shell or other terminals (i.e. the one integrated into Visual Studio Code) the problem does not appear. It happens with cmd.exe because the command line takes into consideration the PATHEXT enviroment variable.

For example, a common PATHEXT value at system level is:

PATHEXT=.COM;.EXE;.BAT;.CMD;.VBS;.VBE;.JS;.JSE;.WSF;.WSH;.MSC

This tells Windows to consider files with those extensions as executables. When you run the truffle command inside your project folder, truffle.js takes the precedence and it starts. This means that, if you assigned for example a default text editor to .js file, when you type truffle inside the project folder the truffle.js file is opened inside the default text editor.

To get rid of this particular problem, you can override the default PATHEXT system environment variable with a user environment variable, removing the .JS extension from the admitted values.

IMHO it's better to use a safer PATHEXT in any case. For example, I use this:

PATHEXT=.COM;.EXE;.BAT;.CMD;.MSC

@SylTi
Copy link
Contributor Author

SylTi commented Aug 19, 2017

@frangio obviously the script to run the test/coverage etc are not working. they could be migrated to an npm script instead.
@Neurone cmd is still the default on windows (and old versions don't even have PowerShell). For example, PhpStorm uses it.
You might be right to change that env var, but I think OpenZeppelin should work out of the box.

@frangio
Copy link
Contributor

frangio commented Aug 21, 2017

Yes I agree that the change is harmless enough that we can merge it.

@frangio frangio merged commit 86beb5b into OpenZeppelin:master Aug 21, 2017
@frangio
Copy link
Contributor

frangio commented Aug 21, 2017

Actually, the change broke the coverage analysis. I'm going to have to revert it and open an issue until sc-forks/solidity-coverage#93 is fixed.

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.

None yet

3 participants