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

Windows: node.cmd shortcut instead of full-blown node.exe #871

Closed
am11 opened this issue Feb 17, 2015 · 16 comments
Closed

Windows: node.cmd shortcut instead of full-blown node.exe #871

am11 opened this issue Feb 17, 2015 · 16 comments
Labels
windows Issues and PRs related to the Windows platform.

Comments

@am11
Copy link

am11 commented Feb 17, 2015

In io.js dist, node.exe is provided as a separate executable which is confusing and causes problems if your script is relying on process.execPath and your intention is to run iojs via node alias (npm task etc.).

For Linux, io.js provides a (4bytes) symlink called node which points to iojs in bin/ dir. On Windows we can also create a symlink, and resolve to real executable (like this) but since Windows symbolic links are not sharable FS objects, and they actually reflect the real path if you try to move, copy or package it to a zip. Meaning you cannot copy/paste Windows symlink itself (like shortcut (.lnk) files), it will copy the original file instead.

Having said that, here is a proposed solution:

Inspired by nvmw, which downloads only iojs.exe and then makes a copy of alias-node.cmd to node.cmd in iojs version installation directory like this: nvmw.bat#L170, where alias-node.cmd has the canned content:

:: Created by nvmw, please don't edit manually. 
@IF EXIST "%~dp0\iojs.exe" ( 
   "%~dp0\iojs.exe" %* 
) ELSE ( 
   iojs %* 
) 

Please add this node.cmd in io.js distro in place of node.exe.

Related: appveyor/ci#139.

@piscisaureus
Copy link
Contributor

I agree that this would be a nice, however the issue is that child_process.spawn() can't run .cmd files. This breaks for programs that try to spawn('node').

Using a batch file would also make it impossible to obtain the pid of "node", which makes IPC stdio streams nonfunctional.

Also note that on windows node.exe and iojs.exe are hard linked together. If a symlink was used it would work as expected (except libuv/libuv#206), but symlinks can't be created on windows 2003, and it requires administrator privileges and elevation on other windows versions.

@am11
Copy link
Author

am11 commented Feb 17, 2015

@piscisaureus, thank you for the clarification.

Aside, I am sorry but I am not sure how else to test spawn('node') issue you described. Please take a look at this outcome:

:: this is cmd on Windows 8.1
$ nvmw install iojs-v1.2.0
...

$ nvmw use iojs-v1.2.0
Now using iojs v1.2.0

$ iojs -v
v1.2.0

$ iojs

> var child_process = require('child_process');
> child_process.spawn('c:/users/adeel/.nvmw/iojs/v1.2.0/node.cmd', ['-p', 'process.execPath']).stdout.on('data', function(e){console.log('data: ' + e)})
...
...
> data: c:\Users\Adeel\.nvmw\iojs\v1.2.0\iojs.exe
(^C again to quit)
>
$ node -v
v1.2.0

$ node

> var child_process = require('child_process');
> child_process.spawn('c:/users/adeel/.nvmw/iojs/v1.2.0/node.cmd', ['-p', 'process.execPath']).stdout.on('data', function(e){console.log('data: ' + e)})
...
...
> data: c:\Users\Adeel\.nvmw\iojs\v1.2.0\iojs.exe

Apparently, spawn has no issue executing that .cmd file.

@piscisaureus
Copy link
Contributor

Apparently, spawn has no issue executing that .cmd file.

😲 I just tried your test case and, I can confirm it works. But it shouldn't work!
And the inability to spawn .cmd/.bat files has been on my long-term wishlist for many years.

According to msdn, "to run a batch file, you must start the command interpreter; set lpApplicationName to cmd.exe and set lpCommandLine to the following arguments: /c plus the name of the batch file."

There is no way iojs is prepending 'cmd /c' when spawn() is used.

Am I dreaming? Is it the drugs?
Can someone confirm that this works on windows 7 too?

@piscisaureus
Copy link
Contributor

I think I figured out what happened.

Windows CreateProcess() has been able to run .bat and .cmd files since windows 2000.
However, when I tested it, I provided a full path to the batch file to CreateProcess, but didn't specify a PATH environment variable to be passed to the child process. Subsequently CreateProcess would be unable to locate cmd.exe so it failed.

A relatively recent security fix (in response to CVE-2014-0315) hard-coded the full path to cmd.exe into CreateProcess, which makes it work.

There are still significant issues with running .bat files.

  • It is not possible to reliably forward arguments through batch files.
  • Depending on the structure of the argument list, cmd.exe may not be able to locate the batch file at all, even if a full path is specified.
var spawnSync = require('child_process').spawnSync;

spawnSync('node.bat', ["-p", "'hello'"], { stdio: 'inherit' });
// works
// prints hello as expected.

spawnSync('c:\\users\\bert belder\\node.bat', ["-p", "'hello'"], { stdio: 'inherit' });
// works
// prints hello as expected

spawnSync('c:\\users\\bert belder\\node.bat', ['-p', '"hello"'], { stdio: 'inherit' });
// unexpected error:
// 'c:\users\bert' is not recognized as an internal or external command,
// operable program or batch file.

spawnSync('node.bat', ["-p", "'^'"], { stdio: 'inherit' });
// arguments are mangled
// prints nothing, while '^' was expected

spawnSync('node.bat', ["-p", '"^^"'], { stdio: 'inherit' });
// arguments are mangled
// prints nothing, while '^' was expected

spawnSync('node.bat', ["-p", '"^^^^"'], { stdio: 'inherit' });
// arguments are mangled
// prints '^', while '^^^^' was expected

spawnSync('node.bat', ["-p", '2', '>', '3'], { stdio: 'inherit' });
// arguments are mangled
// expected to print 'false'
// what actually happens is a file named '3' is created.
  • The fact that iojs can't see the PID of the iojs.exe child when it is spawned via an intermediary batch file also makes it unsuitable. This is arguably an unreasonable restriction in iojs, but not one that is easy to overcome.

@am11
Copy link
Author

am11 commented Feb 18, 2015

I do not know why but I am not getting this error: "arguments are mangled" even after moving c:/users/adeel/.nvmw/iojs/v1.2.0/node.cmd to c:/users/adeel/.nvmw/iojs/v1.2.0/node.bat and running the ditto commands.

The last line created file named 3 in the cwd (with type 3 outputting 2).

Perhaps, I have something in the path which is remedying it from happening. Here is the path output:

PATH=c:\Users\Adeel\.nvmw\;c:\Users\Adeel\.nvmw\iojs\v1.2.0;C:\ProgramData\Oracle\Java\javapath;C:\Program Files (x86)\P
ython\;C:\Program Files (x86)\Intel\iCLS Client\;C:\Program Files\Intel\iCLS Client\;C:\windows\system32;C:\windows;C:\w
indows\System32\Wbem;C:\windows\System32\WindowsPowerShell\v1.0\;C:\Program Files\Intel\Intel(R) Management Engine Compo
nents\DAL;C:\Program Files\Intel\Intel(R) Management Engine Components\IPT;C:\Program Files (x86)\Intel\Intel(R) Managem
ent Engine Components\DAL;C:\Program Files (x86)\Intel\Intel(R) Management Engine Components\IPT;C:\Program Files (x86)\
ATI Technologies\ATI.ACE\Core-Static;C:\Program Files\Lenovo\Bluetooth Software\;C:\Program Files\Lenovo\Bluetooth Softw
are\syswow64;C:\Program Files (x86)\Windows Kits\8.1\Windows Performance Toolkit\;C:\Program Files\Microsoft SQL Server\
110\Tools\Binn\;C:\Users\Adeel\AppData\Local\GitHub\PortableGit_054f2e797ebafd44a30203088cd3d58663c627ef\cmd;C:\Program
Files (x86)\Bitvise SSH Client;C:\Program Files\Microsoft SQL Server\120\Tools\Binn\;C:\Program Files (x86)\WinMerge;C:\
Program Files\Microsoft\Web Platform Installer\;C:\wamp\bin\php\php5.5.12;C:\Program Files\Microsoft SQL Server\120\DTS\
Binn\;C:\Program Files\Microsoft SQL Server\Client SDK\ODBC\110\Tools\Binn\;C:\Program Files (x86)\Microsoft SQL Server\
120\Tools\Binn\;C:\Program Files (x86)\Microsoft SQL Server\120\Tools\Binn\ManagementStudio\;C:\Program Files (x86)\Micr
osoft SQL Server\120\DTS\Binn\;C:\Program Files (x86)\Microsoft SDKs\TypeScript\1.0\;C:\HashiCorp\Vagrant\bin;C:\Program
 Files (x86)\Microsoft SDKs\TypeScript\1.1\;C:\Program Files (x86)\nodejs\;c:\users\adeel\.nvmw;C:\path\to\php;c:\path\t
o\php\pear;C:\Users\Adeel\AppData\Roaming\npm

@piscisaureus
Copy link
Contributor

I do not know why but I am not getting this error: "arguments are mangled"

  • Try moving the batch file to a path with spaces in it.
  • I didn't mean to say that you'd see a message "arguments are mangled". What I meant is that the arguments don't come out on the other side as you put them in.

@am11
Copy link
Author

am11 commented Feb 18, 2015

My bad. I thought that was the actual output. 😄

@sam-github
Copy link
Contributor

Instead of installing node as a symlink to iojs.exe, why don't we simply install the binary twice, with two different names?

It avoids all the downsides of symlinks, and preserves all the upsides of having iojs available under both names. On unix, symlinks (and hard links) work well for this kind of thing, I'm wondering if we went too far down the "do it like unix on windows" path.

@piscisaureus
Copy link
Contributor

Instead of installing node as a symlink to iojs.exe, why don't we simply install the binary twice, with two different names

Right now the two files are hard linked. It saves some space - probably so little that it's unnoticeable. Other than that, it should behave as if there were two copies of the same file.

@sam-github
Copy link
Contributor

I'm a bit lost then as to what the problem is, and pretty nervous about replacing with a node.cmd. Does the OP not like that there are two copies of the same file?

@am11 what problem are you trying to solve?

@am11
Copy link
Author

am11 commented Feb 19, 2015

@sam-github, as noted earlier, the issue is described here: appveyor/ci#139.

The AppVeyorCI for Windows is using node.exe provided by io.js distribution in addition to iojs.exe, as you guys are recommending.

As you can see there, it took some hit and trial to narrow down the issue to this:

  • Running iojs.exe via node prefix breaks node-sass binary compatibility (build using pangyp). while running the same with iojs prefix works fine.
  • Naturally, we cannot replace node with iojs for AppVeyor CI, as it will fail the other two node jobs (v0.10 and v0.12).
  • However if we have node.cmd instead of node.exe, it does not present the problem.

Since node.cmd for Windows is a no-go, we will probably have to somehow inject this node.cmd into their iojs install folder and delete the node.exe (if it is even a permitted action) to successfully run io.js job on AppVeyor.

@sam-github
Copy link
Contributor

Sorry, not really understanding the problem.

Naturally, we cannot replace node with iojs for AppVeyor CI, as it will fail the other two node jobs (v0.10 and v0.12).

Not following at all. You clearly already have two packages that contain a node.exe (v0.10 and v0.12), why is a third package (v1.x) a problem?

Threading through the link, it all seems to stem from the assumption that io.js and node are so different, they can be distinguished based on executable name:

appveyor/ci#139 (comment)

Why do that? Use version. 1.x is io.js, 0.10 and 0.12 are node... personally, I'd call them both "node". By the time joyent/node gets to 1.x in a couple years, iojs and it will have merged, and if not, then deal with the runtime distinction in a couple years, don't cause yourself this pain now.

Or wait a while, and use node -p process.release.iojs or whatever gets hacked into there if node-gyp decides not to use the version number.

@sam-github
Copy link
Contributor

@am11
Copy link
Author

am11 commented Feb 19, 2015

@sam-github, we need to distinguish the runtimes to name our binaries for many-runtimes, various-platforms matrix, and publish on a separate repo: https://github.com/sass/node-sass-binaries/. On install, we reconstruct the name and download the matching binary from that repo.

I started with this PR: rvagg/archived-pangyp#1 and I do agree that our weird runtime detection is not the ideal way, but that is not causing of this issue; because even if we hardcode return {name:'iojs'} there, it throws the same exception Module did not self-register with node.exe (v1.2.0, in iojs directory). This is the only case which breaks.

@am11
Copy link
Author

am11 commented Feb 26, 2015

Generally, this is what I have narrowed it down to:

If you built the binary like this:

# this is PS
iojs node_modules/pangyp/bin/node-gyp configure  # optional
iojs node_modules/pangyp/bin/node-gyp build

It will execute perfectly with iojs prefix, but not with the same versioned node.exe (that is bundled with io.js) in io.js bin folder.

Also related: sass/node-sass#708 (comment)

@am11
Copy link
Author

am11 commented Mar 1, 2015

This turned out to be the same issue as #751.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

No branches or pull requests

4 participants