-
Notifications
You must be signed in to change notification settings - Fork 108
Fix for v8 w/ node 12 + Switch to github actions. + Fix deprecated callback + Provide default value for spellchecker_use_hunspell #130
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Hal Gentz <[email protected]>
Build for older and newer version of v8
Catch up with atom/node-spellchecker.
Fixees gentz
Replaces #128 |
commit 0c83612 Merge: 2c0d135 5cbdece Author: Freya Gentz <[email protected]> Date: Thu Dec 19 18:04:18 2019 -0700 Merge pull request #4 from goddessfreya/Add-Github-actions Add Github actions commit 5cbdece Author: Freya Gentz <[email protected]> Date: Thu Dec 19 18:01:01 2019 -0700 Delete appveyor.yml commit 4ddcbd3 Author: Freya Gentz <[email protected]> Date: Thu Dec 19 18:00:49 2019 -0700 Delete .travis.yml commit c6f6c63 Author: Freya Gentz <[email protected]> Date: Thu Dec 19 17:57:45 2019 -0700 Update nodejs.yml commit 84bccbd Author: Freya Gentz <[email protected]> Date: Thu Dec 19 17:55:23 2019 -0700 Update nodejs.yml commit f6720ea Author: Freya Gentz <[email protected]> Date: Thu Dec 19 17:50:59 2019 -0700 Update nodejs.yml commit 514a3f9 Author: Freya Gentz <[email protected]> Date: Thu Dec 19 17:31:31 2019 -0700 Add Github actions Signed-off-by: Freya Gentz <[email protected]>
commit 84b6708 Merge: fef5593 1a0243e Author: Freya Gentz <[email protected]> Date: Thu Dec 19 19:43:16 2019 -0700 Merge pull request #6 from goddessfreya/goddessfreya-patch-1 Add clang testing commit 1a0243e Author: Freya Gentz <[email protected]> Date: Thu Dec 19 19:37:34 2019 -0700 Update nodejs.yml commit 18e838f Author: Freya Gentz <[email protected]> Date: Thu Dec 19 19:35:27 2019 -0700 Delete package-lock.json commit d17cf27 Author: Freya Gentz <[email protected]> Date: Thu Dec 19 19:22:57 2019 -0700 Update nodejs.yml commit 81c5ae6 Author: Freya Gentz <[email protected]> Date: Thu Dec 19 19:12:42 2019 -0700 Update nodejs.yml commit 2db0f18 Author: Freya Gentz <[email protected]> Date: Thu Dec 19 18:56:33 2019 -0700 Update nodejs.yml commit 855c3c1 Author: Freya Gentz <[email protected]> Date: Thu Dec 19 18:49:38 2019 -0700 Update nodejs.yml commit f80be0a Author: Freya Gentz <[email protected]> Date: Thu Dec 19 18:38:49 2019 -0700 Update nodejs.yml commit 3c52d66 Author: Freya Gentz <[email protected]> Date: Thu Dec 19 18:35:04 2019 -0700 Update nodejs.yml commit 5fe3493 Author: Freya Gentz <[email protected]> Date: Thu Dec 19 18:34:31 2019 -0700 Update nodejs.yml commit 8c41771 Author: Freya Gentz <[email protected]> Date: Thu Dec 19 18:34:05 2019 -0700 Update nodejs.yml commit 5bd03fe Author: Freya Gentz <[email protected]> Date: Thu Dec 19 18:33:49 2019 -0700 Update nodejs.yml commit aab4c50 Author: Freya Gentz <[email protected]> Date: Thu Dec 19 18:33:35 2019 -0700 Update nodejs.yml commit 0e6f86f Author: Freya Gentz <[email protected]> Date: Thu Dec 19 18:30:31 2019 -0700 Update nodejs.yml commit fef5593 Merge: 0c83612 e99e477 Author: Freya Gentz <[email protected]> Date: Thu Dec 19 18:16:48 2019 -0700 Merge pull request #5 from abetomo/fix_deprecated_callback Fix deprecated callback commit 0c83612 Merge: 2c0d135 5cbdece Author: Freya Gentz <[email protected]> Date: Thu Dec 19 18:04:18 2019 -0700 Merge pull request #4 from goddessfreya/Add-Github-actions Add Github actions commit 5cbdece Author: Freya Gentz <[email protected]> Date: Thu Dec 19 18:01:01 2019 -0700 Delete appveyor.yml commit 4ddcbd3 Author: Freya Gentz <[email protected]> Date: Thu Dec 19 18:00:49 2019 -0700 Delete .travis.yml commit c6f6c63 Author: Freya Gentz <[email protected]> Date: Thu Dec 19 17:57:45 2019 -0700 Update nodejs.yml commit 84bccbd Author: Freya Gentz <[email protected]> Date: Thu Dec 19 17:55:23 2019 -0700 Update nodejs.yml commit f6720ea Author: Freya Gentz <[email protected]> Date: Thu Dec 19 17:50:59 2019 -0700 Update nodejs.yml commit 514a3f9 Author: Freya Gentz <[email protected]> Date: Thu Dec 19 17:31:31 2019 -0700 Add Github actions commit e99e477 Author: abetomo <[email protected]> Date: Thu Jul 5 10:25:35 2018 +0900 Fix deprecated callback The following warning. ``` ../src/worker.cc: In member function βvirtual void CheckSpellingWorker::HandleOKCallback()β: ../src/worker.cc:44:25: warning: βv8::Local<v8::Value> Nan::Callback::Call(int, v8::Local<v8::Value>*) constβ is deprecated [-Wdeprecated-declarations] callback->Call(2, argv); ``` Signed-off-by: Freya Gentz <[email protected]>
and add "freebsd" case. Co-authored-by: Alexander Krotov <[email protected]>
Also merged in the following PRs:
Thank you folks for your contributions. |
@goddessfreya FYI, the versin with original fix 613ff91 is compilable with @electron 8.0.0-beta.5 (node 12.13.0) and 9.0.0-nightly.20191225 (node 12.14.0) but the HEAD / 98e4377 version is not. rebuild using 8.0.0-beta.5 and 98e4377
|
Unfortunately, I tried my best and could not reproduce this build failure. Running the following:
Results in All the v12.x CIs also did not fail at building spellchecker: Can you provide me steps for reproducing this build failure? |
Being located in the module directory execute (
https://electronjs.org/docs/tutorial/using-native-node-modules |
@vladimiry I got it to build with electron Could you submit a PR against https://github.com/goddessfreya/node-spellchecker that resolved your build issues? |
I'm not sure what you mean by this. If you run Archlinux this is how to reproduce the issue:
Then if you rewind to 613ff91 (corresponds to the #128 PR you posted before) and run |
Following those instructions, I get the following, which is not the error reported by you: https://gist.github.com/goddessfreya/90babde3d51401c51ef9e3d717bd5f05 That lead me to believe it was a configuration error. Because of that, I decided to follow this section of the Arch packing guidelines to get it to build against the system's electron: https://wiki.archlinux.org/index.php/Electron_package_guidelines#Building_compiled_extensions_against_the_system_electron With some fiddling of the provided instructions, this succeeded: https://gist.github.com/goddessfreya/904f5624d0db419f25aa54d3142c718f As you may note, the system electron is version 7.1.7. As I mentioned, after fiddling with electron's PKGBUILD for ~50 minutes (45 minutes of which were waiting for electron to build), I could not get the version 8 alpha to build. |
@goddessfreya looks like node-gyp failed to find expected Python on your system. In my case, this is how the beginning of the command output looks like (you don't have last two lines listed here and PythonFinder raises the error below in your log):
So you need to install |
I most certainly have python installed, I remember building it myself for unrelated reasons:
Neither adding |
@goddessfreya so you have custom python3, maybe then try installing
|
That doesn't worth either: https://gist.github.com/goddessfreya/5a4c360ce4c748968df1c19d1680658c |
My bad, I noticed only now that python finder is a JS script and the exception is related to semver, so try installing it |
@vladimiry That package is already installed. |
Alright, so I applied this patch to the system's node-gyp installation: diff --git a/find-python.js b/usr/lib/node_modules/node-gyp/lib/find-python.js
index 9d918a8..43bf859 100644
--- a/find-python.js
+++ b/usr/lib/node_modules/node-gyp/lib/find-python.js
@@ -226,7 +226,7 @@ PythonFinder.prototype = {
}
this.addLog(`- version is "${version}"`)
- const range = semver.Range(this.semverRange)
+ const range = new semver.Range(this.semverRange)
var valid = false
try {
valid = range.test(version) Now, when I run the command, I get the following: $ HOME=~/.electron-gyp node-gyp rebuild --target=8.0.0-beta.5 --arch=x64 --dist- url=https://electronjs.org/headers
gyp info it worked if it ends with ok
gyp info using node-gyp@6.0.1
gyp info using node@13.5.0 | linux | x64
gyp info find Python using Python version 3.8.1 found at "/usr/bin/python3"
gyp http GET url=https://electronjs.org/headers/v8.0.0-beta.5/node-v8.0.0-beta.5.tar.gz
gyp WARN install got an error, rolling back install
gyp ERR! configure error
gyp ERR! stack Error: Invalid URI "url=https://electronjs.org/headers/v8.0.0-beta.5/node-v8.0.0-beta.5.tar.gz"
gyp ERR! stack at Request.init (/usr/lib/node_modules/node-gyp/node_modules/request/request.js:273:31)
gyp ERR! stack at new Request (/usr/lib/node_modules/node-gyp/node_modules/request/request.js:127:8)
gyp ERR! stack at request (/usr/lib/node_modules/node-gyp/node_modules/request/index.js:53:10)
gyp ERR! stack at download (/usr/lib/node_modules/node-gyp/lib/install.js:426:13)
gyp ERR! stack at /usr/lib/node_modules/node-gyp/lib/install.js:162:19
gyp ERR! stack at /usr/lib/node_modules/node-gyp/node_modules/mkdirp/index.js:30:20
gyp ERR! stack at FSReqCallback.oncomplete (fs.js:153:23)
gyp ERR! System Linux 5.4.6.a-1-hardened
gyp ERR! command "/usr/bin/node" "/usr/bin/node-gyp" "rebuild" "--target=8.0.0-beta.5" "--arch=x64" "--dist-" "url=https://electronjs.org/headers"
gyp ERR! cwd /home/gentz/Documents/node-spellchecker
gyp ERR! node -v v13.5.0
gyp ERR! node-gyp -v v6.0.1
gyp ERR! not ok |
Replace |
|
Ah, nice to know it was already reported. |
Signed-off-by: Freya Gentz <[email protected]>
@vladimiry Thanks for the help getting electron to work. I've pushed the fix: 37cebc4 |
Very nice, can confirm compilation got working with both 8.0.0-beta.5 and 9.0.0-nightly.20191225. |
Will this be merged into a new release soon? |
It works with electron v10 too by the way. |
Will the fixes be merged anytime, or the package is not supported anymore? We want to move to electron@9, and currently have an issue with building the app. It fails on compiling spellchecker native deps. |
I've pinged the maintainers by email to get an update on this, or if I can step in and help maintain. |
@goddessfreya any chance you could merge in #69 into your PR? |
You can use https://github.com/Wulf/nodehun in the meanwhile. |
v11 is also fine, just tried 11.0.0-beta.3. |
βπ Hello there! Welcome. Please follow the steps below to tell us about your contribution.