detect npm-path properly in cygwin (fixes windows build).

[fromkeith]

Brief description of the changes [REQUIRED]

make build now works in cygwin

What browsers and operating systems have you tested these changes on? [REQUIRED]

Chrome

Are all automated tests passing? [REQUIRED]

Yes

Is this pull request against develop or some other non-master branch? [REQUIRED]

Develop.

Testing

• make
• make build

[feryardiant]

Thanks for addressing my issue, unfortunately I can't test it yet because I'm on Linux and had completely remove my windows but I'll try to test it on vm tonight (GMT+7) and back to you soon.

[rnicholus]

This doesn't seem to break in non-Windows environments, but it seems the OS environment variable is non-standard. Why not use uname instead? http://stackoverflow.com/a/17072017/486979

[feryardiant]

Actualy I have tried using $OSTYPE to detect my system. Sadly, it seems not working when I use it inside script but when I simply echo $OSTYPE from my terminal, it print cygwin. I have no idea afterward.

[rnicholus]

Looks like uname -s produces a value on every flavor of Windows that at least includes the string _NT.

[fromkeith]

I can change it to uname if you like. Found the function 'findstring' which will let us see if CYGWIN is in the uname... Then we just use that as the condition for the switch.

eg.

platform = $(shell uname -s)
ifeq ($(findstring CYGWIN,$(platform)),CYGWIN)
	npm-bin = $(shell cygpath -u $(shell npm bin))
else
	npm-bin = $(shell npm bin)
endif

[rnicholus]

"CYGWIN" isn't necessarily going to be in the output of uname -s.

[fromkeith]

What I get is CYGWIN_NT-10.0

[rnicholus]

on Windows 7, that may be true. On Windows 10, I'm seeing something a bit different. The common string in all versions of Windows seems to be _NT.

[rnicholus]

I don't personally intend to take time to setup cygwin, node, git, etc on a Windows VM to verify this. Can anyone else on a windows box verify that the build now works in Windows, at least under cygwin?

[rnicholus]

Has anyone else on a windows box verified that the build now works in Windows, at least under cygwin?

[singhjusraj]

I can confirm that with the changes from this PR, build seems to be working nicely in windows under cygwin

[rnicholus]

Thanks for the confirmation. Merging this into develop and planning to make this part of 5.14.0. Initially it will be part of 5.14.0-beta3 along with #1719.