search

nodejs / gyp-next

A fork of the GYP build system for use in the Node.js projects
BSD 3-Clause "New" or "Revised" License
125 stars 69 forks source link

feat: add VCToolsVersion for msvs #209

Closed tr-takatsuka closed 9 months ago

tr-takatsuka commented 9 months ago

Add the ability to specify the 'MSVC toolset version' for the VisualStudio platform.

This PR created for gyp-next, following the adviced in node-gyp. Thank you. https://github.com/nodejs/node-gyp/pull/2910

Usage example for binding.gyp

'targets': [
 {
   'configurations': {
     'Debug': {
       "msvs_configuration_attributes": {
         "VCToolsVersion": "14.36.32532",

Thank you and best regards.

cclauss commented 9 months ago

@StefanStojanovic @rzhao271 Your reviews, please.

StefanStojanovic commented 9 months ago

I see a problem with this PR, and all future PRs in this repo unrelated to changes directly. The Node.js Windows integration / build-windows workflow runs on windows-2019 which from what I know uses VS2019. However, after this landed in Node.js, it no longer supports building on VS2019 and requires VS2022. Luckily for us, fix should be as simple as changing the line I referenced in a workflow file to windows-2022 or even simpler to windows-latest. We could do this in another PR, land it and after that rebase this one, or directly in this PR. Although first approach would be cleaner I do not have any objections to doing it in here.

tr-takatsuka commented 9 months ago

Thanks for the review.

I think I have understood your points correctly. changed build-windows workflow to "windows-latest". could you review if this change is correct?

StefanStojanovic commented 9 months ago

Thank you @tr-takatsuka. Yes you understood me correctly. @cclauss in this repo I do not have a write access, so can you please approve changes to trigger the workflows?

StefanStojanovic commented 9 months ago

Is there something more that should be done with this PR? I think it can land cc @cclauss