Hi 👋
I can't tell you how nice it is to have this package for instructional material. Concerns about what version of node people have are basically gone now. Thank you!
It looks like appveyor can't install node this way though. Here's the relevant output from the first broken build when I added node as a dep to my testing workshop:
> node@8.9.4 preinstall C:\projects\testing-workshop\node_modules\node
20> node installArchSpecificPackage
21
22npm ERR! code EBADPLATFORM
23npm ERR! notsup Unsupported platform for node-win-x86@8.9.4: wanted {"os":"win32","arch":"x86"} (current: {"os":"win32","arch":"ia32"})
24npm ERR! notsup Valid OS: win32
25npm ERR! notsup Valid Arch: x86
26npm ERR! notsup Actual OS: win32
27npm ERR! notsup Actual Arch: ia32
What's interesting is because the arch is ia32 it should hit this logic:
https://github.com/aredridel/node-bin-setup/blob/ad56be2b72108028fc058c07665035e34f25c0e9/index.js#L10
And the arch variable should be set to x86 (which I believe it is because that's what it tries to install based on the error message). What's surprising to me is that the above line of code seems to indicate that this ia32 should be supported by the x86 package, but it's not.
I'm fairly certain that all that needs to happen is node-win-x86 needs to have its package.json updated to support ia32. It appears that we just need to update this to be an array of the supported archs:
Which I think could be done by updating this line:
|
const arch = os == 'win' && cpu == 'ia32' ? 'x86' : cpu |
To be this instead:
const arch = os == 'win' && cpu == 'ia32' ? ['x86', 'ia32'] : cpu
If that's right, then I'll go ahead and open up a PR. Though I'm not sure how to backport support to older versions as the version appears to be tied to the version of node 🤔
Hi 👋
I can't tell you how nice it is to have this package for instructional material. Concerns about what version of node people have are basically gone now. Thank you!
It looks like appveyor can't install
nodethis way though. Here's the relevant output from the first broken build when I addednodeas a dep to my testing workshop:What's interesting is because the arch is
ia32it should hit this logic:https://github.com/aredridel/node-bin-setup/blob/ad56be2b72108028fc058c07665035e34f25c0e9/index.js#L10
And the
archvariable should be set tox86(which I believe it is because that's what it tries to install based on the error message). What's surprising to me is that the above line of code seems to indicate that thisia32should be supported by thex86package, but it's not.I'm fairly certain that all that needs to happen is
node-win-x86needs to have itspackage.jsonupdated to supportia32. It appears that we just need to update this to be an array of the supported archs:node-bin-gen/index.js
Line 72 in f957051
Which I think could be done by updating this line:
node-bin-gen/index.js
Line 51 in f957051
To be this instead:
If that's right, then I'll go ahead and open up a PR. Though I'm not sure how to backport support to older versions as the version appears to be tied to the version of node 🤔