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 CI is broken for tests, prebuilt binaries #278

Open
dhritzkiv opened this issue Mar 2, 2024 · 4 comments
Open

Windows CI is broken for tests, prebuilt binaries #278

dhritzkiv opened this issue Mar 2, 2024 · 4 comments

Comments

@dhritzkiv
Copy link
Member

The broken CI on Windows prevents us from testing the library on Windows, and also prevents us from offering prebuilt binaries for each release.

The Windows CI configuration on Appveyor has always been flaky, often failing on every other build. However, as of Node 18/20 or Visual Studio 2019, several parts of the CI setup have conspired to prevent successful builds. It's unclear which of the following issues are having the greatest impact

  • npm config set msvs_version 2019 --global no longer works, as npm complains that it's not a valid setting. This setting was used by node-gyp to find the correct build toolchain on Windows.
  • GYP_MSVS_VERSION, which some have purported was the equivalent to the above, seems to have no effect.
  • node-gyp isn't able to determine the path of the installed toolchain, even when setting the VSINSTALL variable
  • trying to run call "C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Auxiliary\Build\vcvars64.bat" fails as the Visual Studio path is not available (despite following the appveyor recommendations).

I don't have a Windows machine handy to diagnose the problems, and pushing small trial-and-error changes to Appveyor is unproductive, so I have currently given up on trying to fix the CI on Windows in appveyor.

Either:

  • I try any suggestions from the community for fixing the build
  • We migrate to GitHub Actions for CI tasks.
    • Starting with Windows, and then replacing Travis for Linux/macOS as necessary.
    • This comes with its own time and effort investment that I don't personally have to offer at this time. Help is very welcome here.
@dhritzkiv
Copy link
Member Author

Mostly solved by #280 and #264

Still to do:

  • set up prebuilds in GitHub Actions
  • remove Travis CI and Appveyor config files
  • add mention of GitHub Actions in README under CI section

@cclauss
Copy link

cclauss commented Mar 27, 2024

@lukekarrys The question about npm config set msvs_version 2019 no longer working come up a lot in my travels.

I believe that npm (around v10?) reduced its config vocabulary and msvs_version was removed. Is there a changelog or release notes that describes this change?

@lukekarrys
Copy link

lukekarrys commented Apr 15, 2024

@cclauss This breaking change was made in npm v9 to not accept config npm didn't know about. I'm not sure if we had a good upgrade guide for this but I found it listed in one of the npm@9 prereleases https://docs.npmjs.com/cli/v9/using-npm/changelog#%EF%B8%8F-breaking-changes and the v9 release notes on GitHub https://github.com/npm/cli/releases/tag/v9.0.0

In the node-gyp docs, we updated them to recommend setting env vars which should work instead: https://github.com/nodejs/node-gyp?tab=readme-ov-file#environment-variables

I think the same thing would work in this case as well. Do you have an idea of better places we can document this?

@lukekarrys
Copy link

I suppose the best thing might be to gather the list of configs that node-gyp knows about and special case those with a message to set them via env vars instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants