nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
107.92k stars 29.75k forks source link

Windows GitHub Actions changes required #55929

Open StefanStojanovic opened 2 days ago

StefanStojanovic commented 2 days ago

Version

No response

Platform

No response

Subsystem

No response

What steps will reproduce the bug?

Running vcbuild.bat

How often does it reproduce? Is there a required condition?

Always with VS 17.12.

What is the expected behavior? Why is that the expected behavior?

I expect Node.js to compile.

What do you see instead?

Compilation fails.

Additional information

I've already opened an issue to track this and reported it to Microsoft. In addition to that, I'll open a PR similar to https://github.com/nodejs/node/pull/53863 marking the v17.12 as unsupported so people get informed.

In addition to everything already mentioned we should disable GitHub Actions for Windows: https://github.com/nodejs/node/actions/workflows/build-windows.yml, https://github.com/nodejs/node/actions/workflows/coverage-windows.yml and https://github.com/nodejs/node/actions/workflows/test-windows.yml as they will fail constantly (eg. https://github.com/nodejs/node/pull/55270) because GitHub Runners use VS v17.12 now.

P.S. Maybe we can even remove test-windows.yml as it wasn't run recently?

cc @nodejs/platform-windows @nodejs/build

richardlau commented 2 days ago

In addition to everything already mentioned we should disable GitHub Actions for Windows: https://github.com/nodejs/node/actions/workflows/build-windows.yml, https://github.com/nodejs/node/actions/workflows/coverage-windows.yml and https://github.com/nodejs/node/actions/workflows/test-windows.yml as they will fail constantly (eg. https://github.com/nodejs/node/pull/55270) because GitHub Runners use VS v17.12 now.

P.S. Maybe we can even remove test-windows.yml as it wasn't run recently?

Do we know which versions of Node.js are broken with VS v17.12?

For example, both

passed for the v20.18.1 proposal with VS v17.12.

FWIW https://github.com/nodejs/node/actions/workflows/build-windows.yml was removed from main by https://github.com/nodejs/node/pull/54662 which was backported to v22.x-staging.

test-windows.yml never landed -- it was proposed in https://github.com/nodejs/node/pull/50519 (to run tests instead of just building) but closed without merging.

targos commented 8 hours ago

I've disabled the workflows