-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat(electron): ✨ call checkConnectivity
as a library
#1555
Conversation
0284750
to
f39bf04
Compare
|
||
|
||
const NATIVE_LIB_PATH_BY_PLATFORM = new Map<string, string[]>([ | ||
['linux', ['prebuilds', 'linux-x64', 'libtun2socks.so']], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the prebuilt binaries, it would be better if we could fetch from the release, otherwise we will get complaints about the binaries in the source code.
Perhaps we can fetch on demand at build time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Also once we publish it to npm it won't cause any problems because it is downloaded to the node_modules folder.
@@ -0,0 +1,96 @@ | |||
// Copyright 2023 The Outline Authors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should either live under src/
or in outline-go-tun2socks
.
Code in third_party should be minimal. Putting in outline-go-tun2socks may make sense, since it aligns with what we do on mobile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but then we need to publish it to npm first (maybe as a private package), otherwise our client cannot reference this package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to publish. You can npm install from github if you put in outline-go-tun2socks. But src/ works too
Close in favor of #2263 |
In this PR, I enabled the native library call feature for electron app. I used
checkConnectivity
to do the experiment this time. So now we don't need to create a new OS process and parse the exit code.Related PR: Jigsaw-Code/outline-go-tun2socks#103