-
Notifications
You must be signed in to change notification settings - Fork 268
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
refactor: build tun2socks from source within Android Studio #487
Conversation
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 looks pretty good. Just some small changes.
Android/tun2socks/build.gradle
Outdated
|
||
commandLine('go', 'install', | ||
'golang.org/x/mobile/cmd/gomobile@latest', |
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.
If you use @latest
, you will ignore the go.mod, and you won't get consistent versioning.
Use go build
instead, so it's aware of the module: https://github.com/Jigsaw-Code/outline-sdk/tree/main/x/mobileproxy#build-the-go-mobile-binaries-with-go-build
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.
Removing @latest
from the package name would do the work as well. If the @
symbol is omitted, go install
will also read the go.mod
file.
Here is the source code that handles package names containing @
in go install
: https://github.com/golang/go/blob/ad9e6edfddf7c68b7a549ab7b491919f0980889d/src/cmd/go/internal/work/build.go#L690-L693
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.
The code is not very clear if it will install in module mode and use the pinned dependencies.
But the documentation seems to imply it will be installed in module-aware mode. So I guess it's fine to just remove the version suffix.
I still think go build is superior, because you don't need to know the rules of where Go installs things. You can explicitly specify the output location with the -o
flag. This is especially helpful for people not used to Go, since otherwise what happens to the binary is just magic. But it's up to you.
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.
replaced it with go build
since go install
won't install reproducible binaries, either.
outputs.file("${goBuildDir}/gobind") | ||
|
||
commandLine('go', 'build', |
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 is a lot clearer, thanks!
This PR moves all Go code from
go-tun2socks/intra
repository to./Android/tun2socks/intra
. It also updates the import packages fromgithub.com/Jigsaw-Code/outline-go-tun2socks/intra
togithub.com/Jigsaw-Code/Intra/Android/tun2socks/intra
, and copyright headers fromOutline Authors
toJigsaw LLC
.Additionally, it removes the need to depend on a binary file, by defining the
gomobile
bind actions in./Android/tun2socks/build.gradle
. This means that we can now build the entire app completely from source code in Android Studio.