Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

chore: Add Node 15 support #2983

Merged
merged 2 commits into from
Oct 27, 2020
Merged

chore: Add Node 15 support #2983

merged 2 commits into from
Oct 27, 2020

Conversation

nschonni
Copy link
Contributor

@nschonni nschonni commented Sep 29, 2020

This should fail till the actual v15 of Node comes out.
Platform compiler requirements based off of nodejs/build#2423

@nschonni nschonni marked this pull request as ready for review October 20, 2020 04:42
@nschonni nschonni mentioned this pull request Oct 20, 2020
4 tasks
@nschonni
Copy link
Contributor Author

@xzyfer any idea why the binding isn't being picked up in 15?

> mocha test/{*,**/**}.js


Error: ENOENT: no such file or directory, scandir '/home/runner/work/node-sass/node-sass/vendor'
    at Object.readdirSync (node:fs:1025:3)
    at Object.getInstalledBinaries (/home/runner/work/node-sass/node-sass/lib/extensions.js:134:13)
    at foundBinariesList (/home/runner/work/node-sass/node-sass/lib/errors.js:20:15)
    at foundBinaries (/home/runner/work/node-sass/node-sass/lib/errors.js:15:5)
    at Object.module.exports.missingBinary (/home/runner/work/node-sass/node-sass/lib/errors.js:45:5)
    at module.exports (/home/runner/work/node-sass/node-sass/lib/binding.js:15:30)
    at Object.<anonymous> (/home/runner/work/node-sass/node-sass/lib/index.js:13:35)
    at Module._compile (node:internal/modules/cjs/loader:1083:30)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1112:10)
    at Module.load (node:internal/modules/cjs/loader:948:32)
    at Function.Module._load (node:internal/modules/cjs/loader:789:14)
    at Module.require (node:internal/modules/cjs/loader:972:19)
    at require (node:internal/modules/cjs/helpers:88:18)
    at Object.<anonymous> (/home/runner/work/node-sass/node-sass/test/api.js:10:10)
    at Module._compile (node:internal/modules/cjs/loader:1083:30)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1112:10)
    at Module.load (node:internal/modules/cjs/loader:948:32)
    at Function.Module._load (node:internal/modules/cjs/loader:789:14)
    at Module.require (node:internal/modules/cjs/loader:972:19)
    at require (node:internal/modules/cjs/helpers:88:18)
    at Object.exports.requireOrImport (/home/runner/work/node-sass/node-sass/node_modules/mocha/lib/esm-utils.js:20:12)
    at Object.exports.loadFilesAsync (/home/runner/work/node-sass/node-sass/node_modules/mocha/lib/esm-utils.js:33:34)
    at Mocha.loadFilesAsync (/home/runner/work/node-sass/node-sass/node_modules/mocha/lib/mocha.js:431:19)
    at singleRun (/home/runner/work/node-sass/node-sass/node_modules/mocha/lib/cli/run-helpers.js:125:15)
    at exports.runMocha (/home/runner/work/node-sass/node-sass/node_modules/mocha/lib/cli/run-helpers.js:190:10)
    at Object.exports.handler (/home/runner/work/node-sass/node-sass/node_modules/mocha/lib/cli/run.js:362:11)
    at /home/runner/work/node-sass/node-sass/node_modules/yargs/lib/command.js:241:49

Happening in Actions an AppVeyor, so it doesn't seem CI specific

@xzyfer
Copy link
Contributor

xzyfer commented Oct 21, 2020

Looks like a duplicate node-sass has crept into the path. Maybe there were changes in this space recently.

-/home/runner/work/node-sass/node-sass/vendor
+/home/runner/work/node-sass/vendor

@nschonni
Copy link
Contributor Author

Odd, it's building to that folder too

gyp info ok 
Installed to /home/runner/work/node-sass/node-sass/vendor/linux-x64-88/binding.node

@nschonni
Copy link
Contributor Author

OK, I tried it locally in WSL after installing 15 through NVM. It seems like after the build, the binding file isn't copied to the the folder like the name suggests.

It has the line at the end of the build ln -f "Release/obj.target/binding.node" "Release/binding.node" 2>/dev/null || (rm -rf "Release/binding.node" && cp -af "Release/obj.target/binding.node" "Release/binding.node"), but it looks like there are no links created. There is a "vendor" folder while its building, but disappears at the end.

@nschonni
Copy link
Contributor Author

Tried cherry-picking the mkdirp bump over, and even tried moving it all sync, but no luck yet. Maybe something node-gyp related since it seems to be failing to ln/copy the built file up to build/Release/binding.node

@nschonni
Copy link
Contributor Author

@xzyfer Success! turns out that only in-publish script was the issue and was triggering the prepublish job. Since we don't need to support NPM 4 anymore, I stripped it out and it's passing now.
Only remaining thing is the Alpine binaries. Not sure if you want to cut the release and wait for them to catch up or not

@xzyfer
Copy link
Contributor

xzyfer commented Oct 22, 2020

Awesome!
I can cut the alpine binaries locally tonight in the worst case.

@nschonni
Copy link
Contributor Author

OK, i'm working on the upstream issues blocking the Node Alpine release, but I don't think it will be resolved in the next few hours

@xzyfer
Copy link
Contributor

xzyfer commented Oct 22, 2020 via email

@nschonni
Copy link
Contributor Author

@xzyfer Node Apline is out. Reset the job, but a bunch of the CLI tests are failing

@nschonni
Copy link
Contributor Author

@xzyfer should we just land this and see if you can build the Alpine binaries locally?

@xzyfer
Copy link
Contributor

xzyfer commented Oct 26, 2020 via email

@nschonni
Copy link
Contributor Author

We could always change the Linux build to use a different distro than Ubuntu, if you think it would offer better compatibility

@xzyfer
Copy link
Contributor

xzyfer commented Oct 27, 2020

I'm going to merge this to cut the release. Lets see how the new binaries perform given the limited Node release support.

@xzyfer xzyfer merged commit 0648b5a into sass:master Oct 27, 2020
@xzyfer
Copy link
Contributor

xzyfer commented Oct 27, 2020

Took a swing at a draft release https://github.com/sass/node-sass/releases/tag/v5.0.0.

What's the process to kick off artifacts uploads?

@nschonni nschonni deleted the node-15 branch October 27, 2020 16:36
@nschonni
Copy link
Contributor Author

They're all on the build for the merge to master EX: the Artifacts download on https://github.com/sass/node-sass/runs/1312916049, and for each of the other platform jobs. Looks like you need to click on one of the individual version jobs before that menu shows up :/

@xzyfer
Copy link
Contributor

xzyfer commented Oct 27, 2020 via email

@nschonni
Copy link
Contributor Author

No worries, I'll see if I can collect the bindings and add them to the draft release

@niemyjski
Copy link

Can someone please post an update when npm has the artifacts. Currently getting No matching version found for....

@nschonni nschonni mentioned this pull request Oct 27, 2020
@xzyfer
Copy link
Contributor

xzyfer commented Oct 31, 2020

Sorry for the delay I've been moving house. I noticed you added the Windows binaries. I've added the Node 15 Alpine binary, removed the prelease status from the release, and published the npm package.

@nschonni
Copy link
Contributor Author

@xzyfer I only added x64 Windows binaries, I couldn't get the x86 ones from AppVeyor

@xzyfer
Copy link
Contributor

xzyfer commented Oct 31, 2020

Kicked off an appveyor build for the windows binaries. They should be added to the release automatically.

https://ci.appveyor.com/project/sass/node-sass/builds/36067507

@nschonni
Copy link
Contributor Author

Yup, look like that worked

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

Successfully merging this pull request may close these issues.

3 participants