antonmedv / fx

Terminal JSON viewer & processor
https://fx.wtf
MIT License
19.08k stars 438 forks source link

fx does not work on Windows #104

Closed cfjedimaster closed 4 years ago

cfjedimaster commented 5 years ago

When testing with Windows 10 Powershell, I get:

Windows PowerShell[26248]: src\node_file.cc:1453: Assertion `(argc) == (5)' failed.
 1: 00007FF71FFBEEE5
 2: 00007FF71FF98CD6
 3: 00007FF71FF98DA1
 4: 00007FF71FF5EFB1
 5: 00007FF72037EE92
 6: 00007FF7203803BD
 7: 00007FF72037F419
 8: 00007FF72037F2FB
 9: 000003581215C6C1
cfjedimaster commented 5 years ago

So more details. Running fx by itself correctly shows the help info. cmd.exe has the same issue (help works, running it on a file reports the same issue). So it may be just Windows in general, and not just Powershell.

111andre111 commented 5 years ago

Hi @antonmedv ,

I had a look into this topic too. There are parallel issues for the same issue: GH-106 , GH-92 and GH-84 Regarding to this comment: https://github.com/antonmedv/fx/issues/92#issuecomment-466907391 I did some digging too:

Prerequsites

Windows 10 v1903 build 18362.239

fx -v 14.0.1 node -v v11.13.0 28 Mar 2019

Investigation

try: upgraded node to version 12.8.1 But still same error was thrown. Now, the situation that type filename | fx . works, shows, that especially with the interactive mode there is an issue. I found the location of the node module like this:

C:\Users\<username>\AppData\Roaming\npm\node_modules\fx\index.js
C:\Users\<username>\AppData\Roaming\npm\node_modules\fx\fx.js

Now the place where the argc = 5 error happens exactly is like you already stated in another issue inside of node itself - I think it is here: https://github.com/nodejs/node/blob/master/src/node_file.cc#L1571 So the command chain starts here: https://github.com/antonmedv/fx/blob/master/index.js#L102 require('./fx')(filename, json) https://github.com/antonmedv/fx/blob/master/fx.js#L11 module.exports = function start(filename, source) https://github.com/antonmedv/fx/blob/master/fx.js#L37 ttyReadStream = tty.ReadStream(cfs.open('conin$', fs.constants.O_RDWR | fs.constants.O_EXCL, 0o666)) So I gave out some values there with console.log and that was the outcome: started here after that line: https://github.com/antonmedv/fx/blob/master/fx.js#L36

console.log(cfs)
{
  access: [Function: access],
  close: [Function: close],
  open: [Function: open],
  openFileHandle: [Function: openFileHandle],
  read: [Function: read],
  fdatasync: [Function: fdatasync],
  fsync: [Function: fsync],
  rename: [Function: rename],
  ftruncate: [Function: ftruncate],
  rmdir: [Function: rmdir],
  mkdir: [Function: mkdir],
  readdir: [Function: readdir],
  internalModuleReadJSON: [Function: internalModuleReadJSON],
  internalModuleStat: [Function: internalModuleStat],
  stat: [Function: stat],
  lstat: [Function: lstat],
  fstat: [Function: fstat],
  link: [Function: link],
  symlink: [Function: symlink],
  readlink: [Function: readlink],
  unlink: [Function: unlink],
  writeBuffer: [Function: writeBuffer],
  writeBuffers: [Function: writeBuffers],
  writeString: [Function: writeString],
  realpath: [Function: realpath],
  copyFile: [Function: copyFile],
  chmod: [Function: chmod],
  fchmod: [Function: fchmod],
  chown: [Function: chown],
  fchown: [Function: fchown],
  lchown: [Function: lchown],
  utimes: [Function: utimes],
  futimes: [Function: futimes],
  mkdtemp: [Function: mkdtemp],
  kFsStatsFieldsNumber: 14,
  statValues: Float64Array [
           2664779574,              33206,
                    1,                  0,
                    0,                  0,
                 4096,  45035996273992660,
                  647,                  1,
        1566059503387,       499162500000,
    1566059503388.367, 1566059503387.0315,
                    0,                  0,
                    0,                  0,
                    0,                  0,
                    0,                  0,
                    0,                  0,
                    0,                  0,
                    0,                  0
  ],
  bigintStatValues: BigUint64Array [
    0n, 0n, 0n, 0n, 0n, 0n, 0n, 0n,
    0n, 0n, 0n, 0n, 0n, 0n, 0n, 0n,
    0n, 0n, 0n, 0n, 0n, 0n, 0n, 0n,
    0n, 0n, 0n, 0n
  ],
  StatWatcher: [Function: StatWatcher],
  FSReqCallback: [Function: FSReqCallback],
  FileHandle: [Function: FileHandle],
  kUsePromises: Symbol(use promises)
}

Then analyzed next line of values: ttyReadStream = tty.ReadStream(cfs.open('conin$', fs.constants.O_RDWR | fs.constants.O_EXCL, 0o666))

console.log(fs.constants.O_RDWR)
  2

next value:

console.log(fs.constants.O_EXCL)
  1024

and next value:

console.log(0o666)
  438

and migrate the values together:

console.log('conin$', fs.constants.O_RDWR | fs.constants.O_EXCL, 0o666))
  conin$ 1026 438

and finally the whole line: So the actual issue is the CONIN$ value under Windows which seems to be an extremely old issue: https://github.com/nodejs/node-v0.x-archive/issues/7300

possible Solution

@Matt-Esch did an awesome work and showed a workaround using userland modules: https://github.com/nodejs/node-v0.x-archive/issues/7300#issuecomment-37837368 Regarding the stackoverflow the issue lies in libuv and _fs.open in connection with CONIN$. This pull request here seems to be interesting, but I don't know if this helps to solve the issue, this is a bit over my knowledge: https://github.com/libuv/libuv/pull/1989 https://github.com/libuv/libuv/commit/b9a0840307843ba0bfa85af95f8ade081c0fed5b

So my question is, is it possible to check out if one of these modules can be integrated into fx so the Windows users can have the sweetness of fx too?

And last but not least, there is a very interesting enhancement request for that issue here and a deep description, that that actual used file descriptors of module fs have several issues: https://github.com/libuv/leps/blob/master/005-windows-handles-not-fd.md

So possibly there is even a possible solution here? https://github.com/libuv/leps/blob/master/005-windows-handles-not-fd.md#transition

111andre111 commented 5 years ago

Oh sorry, I didn't even recognise, that there is a working EXE file going on here: https://github.com/antonmedv/fx/issues/107

antonmedv commented 5 years ago

I think currently the only safe solutions to opening CONIN$ are via native modules. There is no provision in libuv for opening the terminal input, and using _fs.open for opening non-files is unsupported, displaying inconsistent behavior between versions of windows.

antonmedv commented 5 years ago

Is it possible to patch node to support it?

111andre111 commented 5 years ago

@antonmedv Maybe I am wrong, but https://github.com/libuv/libuv/issues/856 I covers think this topic a bit and I would say nodeJS has everything alread on board I'd say.

        nodeJS 12.9.1 -> libuv 1.31.0 https://github.com/nodejs/node/pull/29070
        nodeJS 12.6.0 -> libuv 1.30.1 https://github.com/nodejs/node/pull/28449 https://github.com/nodejs/node/pull/28511  deps/uv/ChangeLog  AC_INIT([libuv], [1.30.1]

So since 12.9.1 is most recent version of libuv integrated.

I understand that the code has to be migrated from a fd to a Windows handle: The difference between the both of them is discussed here a little bit https://stackoverflow.com/questions/7964907/is-handle-similar-to-file-descriptor-in-linux

And I even found this documentation page from Microsoft themselves: https://support.microsoft.com/help/90088/info-createfile-using-conout-or-conin https://docs.microsoft.com/en-us/windows/console/getstdhandle

Here is the piece of code in libuv https://github.com/haskell/process/blob/d17cda7356628f45e7e8a64b59324f84fe2e6e07/cbits/runProcess.c#L680-L684 And I think here may be some code examples for conversion: https://github.com/libuv/libuv/issues/1625 Does that help?

antonmedv commented 5 years ago

Can somebody try to make PR for it? And bump node requirements.

antonmedv commented 4 years ago

Duplicate of #106