src,win: Replacement of unsupported versions of Windows runtime exit.

[xCykrix]

Replaces Windows 7 EOL runtime program termination (#31954) to an unsupported Windows operating system notice at runtime to notify minimal support for using an unsupported version of Windows.

Tests do not pass due to stderr output, and I am unsure of how to handle this further myself due to my limited knowledge of C++. Node.js did compile and act as expected when supplied -e or an input file.

Discussed originally in #33034.
Fixes #33034

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• commit message follows commit guidelines

[addaleax]

@nodejs/tsc

I’ll put this on the agenda as requested by @bnoordhuis.

[mmarchini]

Tests do not pass due to stderr output, and I am unsure of how to handle this further myself due to my limited knowledge of C++.

I assume that's only the case when running tests on unsupported Windows versions, correct (which we don't do in our CI)? If so it should be fine.

[xCykrix]

Tests do not pass due to stderr output, and I am unsure of how to handle this further myself due to my limited knowledge of C++.

I assume that's only the case when running tests on unsupported Windows versions, correct (which we don't do in our CI)? If so it should be fine.

That should be the case, correct. If the CI does use a later version of Windows, then it should be fine and I will set that once the checks pass as I only have Windows 7 available to test locally.

[tjrgg]

Small ask. Any chance there could be a flag or some other way to mute the warning? I understand what I'm doing and would prefer to keep the warning out of my logs.

[addaleax]

Small ask. Any chance there could be a flag or some other way to mute the warning? I understand what I'm doing and would prefer to keep the warning out of my logs.

That would imply moving the warning to a later point during Node’s startup (InitializeOncePerProcess()), because then we have parsed the CLI options and know whether --no-warnings is passed, but I don’t see a fundamental downside to that. 👍

[xCykrix]

Small ask. Any chance there could be a flag or some other way to mute the warning? I understand what I'm doing and would prefer to keep the warning out of my logs.

That would imply moving the warning to a later point during Node’s startup (InitializeOncePerProcess()), because then we have parsed the CLI options and know whether --no-warnings is passed, but I don’t see a fundamental downside to that.

Could I get an approximate location for that? I haven't had much time to investigate the loading structure of Node.js quite yet, but I am all for that as well 🙂

[addaleax]

@xCykrix Look for InitializeOncePerProcess in node.cc, move the code over to there, and add a check for per_process::cli_options->per_isolate->per_env->no_warnings that would disable printing the warning.

You’ll need to keep that check within #ifdef _WIN32, and probably need to copy/move the windows headers used by node_main.cc over to node.cc, too (again, behind an #ifdef _WIN32 guard). I hope that helps?

[xCykrix]

@addaleax Alright, I got it all put in place. It actually had to be slightly later than that but it definitely pointed me in the right direction! Would a static message be better or a deprecation notice ((node:?) [DEP????] DeprecationWarning: message on deprecation) ? Not sure what is more favored to be in place with something like this. I know that would require editing documentation and such as well if that is the case.

[mcollina]

lgtm

[addaleax]

Not sure what is more favored to be in place with something like this. I know that would require editing documentation and such as well if that is the case.

@xCykrix We do deprecation warnings when we intend to remove API features in the future. I don’t think that applies here.

[tjrgg]

@xCykrix I don't think it needs to be a depreciation warning, but if there's some convention for other errors or warnings, I think that should be used over a static message.

Personally, I'd like to make sure it's clear that the message is coming from Node and not a Node application.

[bzoz]

I'm -1 on this. Node is not tested on Windows 7, neither is libuv. Having Node run on Win7, with only a warning message - which will be ignored by almost anyone anyway - is asking for trouble in the long run.

That said, having a --skip-supported-platform-check switch as @jasnell suggested sounds like a reasonable alternative. We should make sure the users know what they are doing before they run Node on an unsupported platform.

[jasnell]

While I am fine with this change as is, I definitely do think that requiring a --skip-supported-platform-check like command-line flag would be the better option as it makes it explicitly opt-in.

[cjihrig]

Not an objection, just an observation:

One thing about a --skip-supported-platform-check flag is that it would only be enforced on a subset of the platforms that Node supports. I understand that we only have these checks on some platforms anyway, but it still feels janky.

[jasnell]

As @bnoordhuis points out, we're already inconsistent here tho @cjihrig. Unsupported macos are filtered out at build time, Unsupported linux aren't filtered out at all. At the very least, a --skip-supported-platform-check would give us a way we could at least try to implement some consistency moving forward as the same flag could be used if we find a way to consistently detect unsupported linux distributions. That said, putting this behind a --skip-supported-platform-check flag doesn't really make things any more inconsistent than they already are.

[cjihrig]

Yea, I understand all of that. It still feels awkward because our internal inconsistencies are leaking out into documented APIs/flags. Like I said, just an observation.

[gabrielschulhof]

FWIW, FreeBSD 11.2 is no longer supported by their organization, and FreeBSD 11.2 images continue to boot. Whenever one tries to install packages one gets a Big Fat Warning™ indicating that there is no recourse for getting problems fixed.

[mmarchini]

Windows 7 works, for now. It will stop working, how fast remains to be seen. It might stop all at once by some system call that is not available there, or slowly by increased errors and issues (I wouldn't be surprised by something like high amounts of ECONNRESET errors). We should make sure users are aware of the risks when running Node on Windows 7.

I don't see any difference between this and how we handle Linux unsupported platforms. Am I missing something? Is this (blocking the application execution) a common practice on Windows?

[jasnell]

Is this (blocking the application execution) a common practice on Windows?

On Windows, blocking is typically handled at the installation phase.

[xCykrix]

Alright, so after listening to the TSC meeting discussion regarding this. It is seeming that, to my understanding, favor is leaning towards adding a flag that will opt-out to bypass this check instead of just a warning at runtime?

If adding this flag, are we going to allow it to be silenced by --no-warnings per the suggestion from @tjrgg?

@xCykrix I don't think it needs to be a deprecation warning, but if there's some convention for other errors or warnings, I think that should be used over a static message.

Personally, I'd like to make sure it's clear that the message is coming from Node and not a Node application.

I am not at all opposed to either direction and given that some of the core Windows maintaining team has put in favor of adding a flag to specifically opt-out of the unsupported platform check, I myself am not quite sure the direction it should take. I'm still leaning towards a mutable warning at the earliest convenience, but that does come with the risk of it not being able to be displayed when something goes wrong.

Before I do any more work besides what is in place now, I would just like to meet a general consensus so it can be discussed in its near-final state during the next TSC session.

[jasnell]

I would make this independent of --no-warnings. If someone is using the --skip-supported-platform-check (or whatever we call it) flag to explicitly opt out of the check, then the warning should be always printed. We have precedence for that with the node_revert mechanism, which allows uses to explicitly opt out of selected security fixes. When used, a message is printed to the console always, regardless of whether --no-warnings is used.

[tjrgg]

Frankly, I'm not in favor of requiring a flag. The only way I'd support that is if there is a policy change where this is standard and applies to all platforms. I don't mind if Node doesn't support it, but I see no excuse for not allowing it to run at all if it can run and am very disappointed that approach was taken to begin with.

@xCykrix My suggestion only matters if a flag isn't being required and instead just a warning comes up. If a flag is going to be required in order to prevent the application from exiting, which I very much oppose, then the --no-warnings flag makes no difference to me.

[joaocgreis]

This breaks installing many native modules (ref, sleep, ...).

One option might be to use an environment variable to skip the check instead of a flag, but even that can possibly cause issues.

[joaocgreis]

Opened #33176 with an alternative for this using an environment variable.

[xCykrix]

I'll post this in both places so it is seen by everyone (regarding the alternative by joaocgreis above), but if the alternative is what ends up being favored I will gladly let it be accepted over the resolution I have regarding just putting warnings in place. Both solve the issue at hand, but given the nature of this; it needs to be met under an agreement or a vote in the end because it may set future policies for deprecation with Windows. The command-line flag alone is not really viable due to the fact that child processes will not execute correctly, so either a mutable warning or environment variables are about the only options I can see resolving this with minimal breakage of features.

[sam-github]

Tagged TSC agenda, but not clear what should be discussed. It looks like #33176 is accepted?

[xCykrix]

To prevent any future confusion, I'm going to go ahead and close this and my attached original issue as I am in support of #33176 myself, and it seems consensus has been met for it.

[lygstate]

Create a fork that works with win7? by testing libuv and nodejs it under win7 . Maybe that's a reasonable way