fix #121 - Githubissues

raszi / node-tmp

Temporary file and directory creator for node.js

MIT License

736 stars 92 forks source link

fix #121 #159

Closed silkentrance closed 5 years ago

silkentrance commented 6 years ago

interactive apps running on windows require more work for SIGINT as it does not support signal and sending it a CTRL-C signal requires a console on Windoze, which is not what tmp is about we might add additional tests for this, but, frankly, I am sick of the Windoze platform and will not be implementing any more work arounds or even dedicated tests for it
SIGTERM cannot be handled in any way, so the temporary objects will remain
SIGINT is now handled correctly even with jest sandboxing in place (OSX/Linux only, on Windoze, the tests will be skipped)
for this to work, we also need to drop support for node 6 as it will not work on Linux, whereas it will work just fine on OSX for that same version, it seems as if signal handling on node 6/Linux is not working as one would expect it to

silkentrance commented 6 years ago

@raszi we definitely need linting rules in place, so you do not have to this :grin:

raszi commented 6 years ago

We do have linting rules. We did not have a task for it, but here is a PR for that in #163.

silkentrance commented 6 years ago

rebased to master

silkentrance commented 6 years ago

Tests require a lot more work. I am on it but it will take some time as I am currently working on multiple other things as well.

silkentrance commented 6 years ago

appveyor is broken, all tests have passed and still they report failing builds...

silkentrance commented 5 years ago

It seems as if there is some process still running, causing a timeout for the builds on appveyor. Need to investigate this using a windows machine SIGH.

silkentrance commented 5 years ago

This seems to be a long standing issue

https://github.com/nodejs/node/issues/12378

silkentrance commented 5 years ago

Regardless of the fact that the tests are all off, not to mention that they are all shize. Need to fix these ASAP.

signal parameter is never passed, need to investitate why.

silkentrance commented 5 years ago

Heck, I need a windoze machine for this finally to be resolved.

silkentrance commented 5 years ago

The fix does not work on windows. In fact, appveyor builds will timeout after 1hr. sigh more work to be done to fix this for this darn platform.

I need to be able to cancel build jobs on appveyor #185.

silkentrance commented 5 years ago

hell, even with the latest changes, the SIGINT listener will not be called on windows...

silkentrance commented 5 years ago

Signal handling on the Windoze platform is not an option. Support is available for OSX/Linux only, starting with node > 6. Node 6 on Linux has issues that will prevent the signal SIGINT listener from being called.

silkentrance commented 5 years ago

I am giving up on making this work on Windoze using standard node provided IPC/process signalling. The platform, while it claims to be compatible to POSIX standards, plainly sucks. The associated tests will be skipped on that platform.

silkentrance commented 5 years ago

First of all, this is a nice PR @silkentrance, thank you very much!

My only problem is that this whole graceful cleanup was just a small feature of this library and now we are having hard time supporting it. I believe we should consider dropping it in the near future.

You are welcome. Thank you very much.

Yeah, the graceful cleanup stuff is getting out of hand :)

And it also comes with the safe installation of the listeners, something that is required for this to work flawlessly with the jest testing framework.

I will look into this and see whether it can be simplified. Maybe we can externalise parts of this by exposing the garbage collector?