-
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
build(client/electron): build Windows with MinGW #1952
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.
We need to update the CI script as well. The build jobs are failing.
Also suggest to update the PR title to build(client/electron): ...
, here is a list of all conventional commit types, we can either to build
or ci
.
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.
Comments addressed
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 exiciting! We can now finally cross-compile Windows binaries natively! Before merging the code, we need to consider some behavior changes carefully (for example, changing from 32-bit to 64-bit).
|
||
android: $(BUILDDIR)/android/tun2socks.aar | ||
|
||
$(BUILDDIR)/android/tun2socks.aar: $(GOMOBILE) | ||
mkdir -p "$(BUILDDIR)/android" | ||
$(ANDROID_BUILD_CMD) -o "$@" $(IMPORT_PATH)/$(ROOT_PKG)/outline/tun2socks $(IMPORT_PATH)/$(ROOT_PKG)/outline/shadowsocks | ||
# Don't strip Android debug symbols so we can upload them to crash reporting tools. |
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.
I think we are still stripping the debug symbols (-w
)? Should this be a TODO
?
Also we need to consider whether we should include debug symbols, that increase the binary size in a significant amount.
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.
-s is what disables the symbol table.
-w is disable DWARF generation
Ideally we would output tun2socks with debug info, and the client build would remove them. Is that possible?
But that's out of scope for this PR
|
||
$(WINDOWS_BUILDDIR)/tun2socks.exe: | ||
mkdir -p "$(@D)" | ||
GOOS=windows GOARCH=amd64 CGO_ENABLED=1 CC="x86_64-w64-mingw32-gcc" go build --ldflags=--extldflags=$(XGO_LDFLAGS) -o "$@" $(ELECTRON_PKG) |
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.
Note that this is updating from a 32-bit (386
) binary to 64-bit (amd64
), which might break some old OS users (like 32-bit Windows 7 / 8 / 10).
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.
As discussed, let's move forward with 64-bit
|
||
$(WINDOWS_BUILDDIR)/tun2socks.exe: | ||
mkdir -p "$(@D)" | ||
GOOS=windows GOARCH=amd64 CGO_ENABLED=1 CC="x86_64-w64-mingw32-gcc" go build --ldflags=--extldflags=$(XGO_LDFLAGS) -o "$@" $(ELECTRON_PKG) |
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.
Maybe not in this PR: our CI is not checking the build file (it is building directly from native Windows), we may need to add some x-compile CI jobs in the future.
src/electron/README.md
Outdated
@@ -30,6 +29,26 @@ npm run action electron/start [windows|linux] | |||
|
|||
### Windows | |||
|
|||
To build for Windows on a macOS, you need to install minGW v11.0.1. |
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.
To build for Windows on a macOS, you need to install minGW v11.0.1. | |
To build for Windows on a macOS, you need to install [MinGW-w64](https://www.mingw-w64.org/) v11.0.1. |
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.
Done
src/electron/README.md
Outdated
@@ -30,6 +29,26 @@ npm run action electron/start [windows|linux] | |||
|
|||
### Windows | |||
|
|||
To build for Windows on a macOS, you need to install minGW v11.0.1. | |||
|
|||
With Homebrew (how to ensure consistent version?): |
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.
Will MacPorts allow you downloading an old version? Actually homebrew is not the official release channel, MacPorts is: https://www.mingw-w64.org/downloads/
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, updated.
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.
Another look?
|
||
android: $(BUILDDIR)/android/tun2socks.aar | ||
|
||
$(BUILDDIR)/android/tun2socks.aar: $(GOMOBILE) | ||
mkdir -p "$(BUILDDIR)/android" | ||
$(ANDROID_BUILD_CMD) -o "$@" $(IMPORT_PATH)/$(ROOT_PKG)/outline/tun2socks $(IMPORT_PATH)/$(ROOT_PKG)/outline/shadowsocks | ||
# Don't strip Android debug symbols so we can upload them to crash reporting tools. |
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.
-s is what disables the symbol table.
-w is disable DWARF generation
Ideally we would output tun2socks with debug info, and the client build would remove them. Is that possible?
But that's out of scope for this PR
|
||
$(WINDOWS_BUILDDIR)/tun2socks.exe: | ||
mkdir -p "$(@D)" | ||
GOOS=windows GOARCH=amd64 CGO_ENABLED=1 CC="x86_64-w64-mingw32-gcc" go build --ldflags=--extldflags=$(XGO_LDFLAGS) -o "$@" $(ELECTRON_PKG) |
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.
As discussed, let's move forward with 64-bit
src/electron/README.md
Outdated
@@ -30,6 +29,26 @@ npm run action electron/start [windows|linux] | |||
|
|||
### Windows | |||
|
|||
To build for Windows on a macOS, you need to install minGW v11.0.1. |
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.
Done
src/electron/README.md
Outdated
@@ -30,6 +29,26 @@ npm run action electron/start [windows|linux] | |||
|
|||
### Windows | |||
|
|||
To build for Windows on a macOS, you need to install minGW v11.0.1. | |||
|
|||
With Homebrew (how to ensure consistent version?): |
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, updated.
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.
LGTM now, thanks!
This works on both Linux and Windows.
I was even able to run tun2socks with Wine!
/cc @amircybersec