#28205 closed defect (fixed)
linking against other libwebrtc binaries errors out on missing symbols
Reported by: | eighthave | Owned by: | eighthave |
---|---|---|---|
Priority: | Medium | Milestone: | |
Component: | Circumvention/Snowflake | Version: | |
Severity: | Major | Keywords: | android |
Cc: | dcf, arlolra, ahf | Actual Points: | |
Parent ID: | Points: | ||
Reviewer: | Sponsor: |
Description
I have a build where it builds libwebrtc from the chromium tree from scratch, but it seems to be missing symbols that snowflake needs. My guess is that the symbols change often in libwebrtc given there is no release ever, so the go-webrtc build needs to be pinned against a very specific version of libwebrtc.
Here's the build where I run go-webrtc's ./build.sh script:
https://gitlab.com/eighthave/snowflake/-/jobs/112538917
$ go get ./... # github.com/keroserene/go-webrtc /tmp/go-build263677836/b053/_x008.o: In function `CGO_DeserializeSDP': /go/src/github.com/keroserene/go-webrtc/peerconnection.cc:379: undefined reference to `webrtc::JsepSessionDescription::JsepSessionDescription(std::string const&)' /go/src/github.com/keroserene/go-webrtc/peerconnection.cc:382: undefined reference to `webrtc::SdpDeserialize(std::string const&, webrtc::JsepSessionDescription*, webrtc::SdpParseError*)' /tmp/go-build263677836/b053/_x008.o: In function `CGO_AddIceCandidate': /go/src/github.com/keroserene/go-webrtc/peerconnection.cc:421: undefined reference to `webrtc::CreateIceCandidate(std::string const&, int, std::string const&, webrtc::SdpParseError*)' /tmp/go-build263677836/b053/_x008.o: In function `Peer::Initialize()': /go/src/github.com/keroserene/go-webrtc/peerconnection.cc:57: undefined reference to `rtc::Thread::SetName(std::string const&, void const*)' /go/src/github.com/keroserene/go-webrtc/peerconnection.cc:58: undefined reference to `rtc::Thread::SetName(std::string const&, void const*)' /tmp/go-build263677836/b053/_x008.o: In function `rtc::ArrayView<bool, -4711l>::ArrayView<bool>(bool*, unsigned long)': /go/src/github.com/keroserene/go-webrtc/./include/api/array_view.h:157: undefined reference to `rtc::FatalMessage::FatalMessage(char const*, int, std::string*)' /tmp/go-build263677836/b053/_x008.o: In function `rtc::ArrayView<int, -4711l>::ArrayView<int>(int*, unsigned long)': /go/src/github.com/keroserene/go-webrtc/./include/api/array_view.h:157: undefined reference to `rtc::FatalMessage::FatalMessage(char const*, int, std::string*)'
Then I ran a build against the official google libwebrtc binaries for Android, and that is missing many more symbols:
https://gitlab.com/eighthave/snowflake/-/jobs/112339118
# github.com/keroserene/go-webrtc peerconnection.cc:379: error: undefined reference to 'webrtc::JsepSessionDescription::JsepSessionDescription(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&)' peerconnection.cc:382: error: undefined reference to 'webrtc::SdpDeserialize(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&, webrtc::JsepSessionDescription*, webrtc::SdpParseError*)' peerconnection.cc:420: error: undefined reference to 'webrtc::CreateIceCandidate(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&, int, std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&, webrtc::SdpParseError*)' peerconnection.cc:55: error: undefined reference to 'rtc::Thread::Thread()' peerconnection.cc:56: error: undefined reference to 'rtc::Thread::Thread()' peerconnection.cc:57: error: undefined reference to 'rtc::Thread::SetName(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&, void const*)' peerconnection.cc:58: error: undefined reference to 'rtc::Thread::SetName(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&, void const*)' peerconnection.cc:59: error: undefined reference to 'rtc::Thread::Start(rtc::Runnable*)' peerconnection.cc:60: error: undefined reference to 'rtc::Thread::Start(rtc::Runnable*)' peerconnection.cc:62: error: undefined reference to 'FakeAudioCaptureModule::Create()' peerconnection.cc:67: error: undefined reference to 'webrtc::CreateBuiltinAudioEncoderFactory()' peerconnection.cc:68: error: undefined reference to 'webrtc::CreateBuiltinAudioDecoderFactory()' /go/pkg/gomobile/ndk-toolchains/arm/bin/../lib/gcc/arm-linux-androideabi/4.9.x/../../../../include/c++/4.9.x/string:0: error: undefined reference to 'webrtc::MediaConstraintsInterface::kEnableDtlsSrtp' /go/pkg/gomobile/ndk-toolchains/arm/bin/../lib/gcc/arm-linux-androideabi/4.9.x/../../../../include/c++/4.9.x/string:0: error: undefined reference to 'webrtc::MediaConstraintsInterface::kEnableDtlsSrtp' ./include/api/peerconnectioninterface.h:1289: error: undefined reference to 'webrtc::CreatePeerConnectionFactory(rtc::Thread*, rtc::Thread*, rtc::Thread*, webrtc::AudioDeviceModule*, rtc::scoped_refptr<webrtc::AudioEncoderFactory>, rtc::scoped_refptr<webrtc::AudioDecoderFactory>, cricket::WebRtcVideoEncoderFactory*, cricket::WebRtcVideoDecoderFactory*)' ./include/rtc_base/copyonwritebuffer.h:49: error: undefined reference to 'rtc::CopyOnWriteBuffer::CopyOnWriteBuffer(unsigned int, unsigned int)' ./include/api/datachannelinterface.h:65: error: undefined reference to 'rtc::CopyOnWriteBuffer::CopyOnWriteBuffer(rtc::CopyOnWriteBuffer const&)' ./include/api/datachannelinterface.h:63: error: undefined reference to 'rtc::CopyOnWriteBuffer::~CopyOnWriteBuffer()' datachannel.cc:20: error: undefined reference to 'rtc::CopyOnWriteBuffer::~CopyOnWriteBuffer()' ./include/rtc_base/copyonwritebuffer.h:53: error: undefined reference to 'rtc::CopyOnWriteBuffer::~CopyOnWriteBuffer()' ./include/api/datachannelinterface.h:63: error: undefined reference to 'rtc::CopyOnWriteBuffer::~CopyOnWriteBuffer()' datachannel.cc:20: error: undefined reference to 'rtc::CopyOnWriteBuffer::~CopyOnWriteBuffer()' ./include/rtc_base/copyonwritebuffer.h:49: error: undefined reference to 'rtc::CopyOnWriteBuffer::CopyOnWriteBuffer(unsigned int, unsigned int)' ./include/api/datachannelinterface.h:65: error: undefined reference to 'rtc::CopyOnWriteBuffer::CopyOnWriteBuffer(rtc::CopyOnWriteBuffer const&)' ./include/rtc_base/copyonwritebuffer.h:53: error: undefined reference to 'rtc::CopyOnWriteBuffer::~CopyOnWriteBuffer()' ./include/rtc_base/buffer.h:126: error: undefined reference to 'rtc::FatalMessage::FatalMessage(char const*, int)' ./include/rtc_base/buffer.h:126: error: undefined reference to 'rtc::FatalMessage::~FatalMessage()' ./include/rtc_base/buffer.h:126: error: undefined reference to 'rtc::FatalMessage::~FatalMessage()' ./include/rtc_base/buffer.h:126: error: undefined reference to 'rtc::FatalMessage::~FatalMessage()' ./include/rtc_base/copyonwritebuffer.h:94: error: undefined reference to 'rtc::FatalMessage::FatalMessage(char const*, int)' ./include/rtc_base/copyonwritebuffer.h:94: error: undefined reference to 'rtc::FatalMessage::~FatalMessage()' ./include/rtc_base/buffer.h:141: error: undefined reference to 'rtc::FatalMessage::FatalMessage(char const*, int)' ./include/rtc_base/copyonwritebuffer.h:102: error: undefined reference to 'rtc::FatalMessage::FatalMessage(char const*, int)' clang70++: error: linker command failed with exit code 1 (use -v to see invocation) gomobile: go build -v -buildmode=c-shared -o=/tmp/gomobile-work-129978917/android/src/main/jniLibs/armeabi-v7a/libgojni.so gobind failed: exit status 2
Child Tickets
Attachments (1)
Change History (57)
comment:1 Changed 14 months ago by
comment:2 Changed 13 months ago by
I understand that all the versions need to be lined up, the question is what lines up with what? ./build.sh
at least specifies what it needs with COMMIT=
, or is that not always current in go-webrtc? Looking at the last successful travis build, the only change to code in snowflake.git is:
+ } else { + log.SetOutput(ioutil.Discard)
So I forced ./build.sh
to use c279861207c5b15fc51069e96595782350e0ac12. Building with go-webrtc@c279861207c5b15fc51069e96595782350e0ac12 fails in a new and "interesting" way:
https://gitlab.com/eighthave/snowflake/-/jobs/113223099
$ go get ./... # github.com/keroserene/go-webrtc ctestenums.cc:2:41: fatal error: api/peerconnectioninterface.h: No such file or directory #include "api/peerconnectioninterface.h" ^ compilation terminated.
Sorry I'm not more useful troubleshooting this, I know zero Go. I do know building C up and down, but the C parts seem totally opaque and hidden behind the Go tricks here.
comment:3 follow-up: 5 Changed 13 months ago by
Ok, I think this problem lies in the missing #cgo android pkg-config:
setup.
comment:4 Changed 13 months ago by
There's an inflection point around,
https://github.com/keroserene/go-webrtc/commit/1ca4087bc01b9a6726a68c72ac541b03a91c4c7b
So, things that will likely work together,
webrtc c279861207c5b15fc51069e96595782350e0ac12 go-webrtc 52280a1ecdfbfc0557a962ff832dbf5632122a35 snowflake HEAD
webrtc 88f5d9180eae78a6162cccd78850ff416eb82483 go-webrtc HEAD snowflake HEAD
comment:5 Changed 13 months ago by
Ok, I think this problem lies in the missing
#cgo android pkg-config:
setup.
Yeah, you probably need something like #cgo linux,arm64 pkg-config: webrtc-linux-arm64.pc
.
See https://github.com/keroserene/go-webrtc/commit/9dfd137319e5846f61cddb2d441d2b8e92ce063c
comment:6 Changed 13 months ago by
Keywords: | android added |
---|
comment:7 Changed 13 months ago by
Ok, little bits of progress to convey. I now have libwebrtc repeatably building in gitlab-ci. It probably downloads 10+ gigs of stuff, but it finishes in well under an hour, so it fits fine within the gitlab-ci limits. I think I also have the pkgconfig stuff sorted out. Based on the missing symbols, it looks pretty close, since there are only a few:
https://gitlab.com/eighthave/go-webrtc/-/jobs/115105095
$ go vet . # _/builds/eighthave/go-webrtc /tmp/go-build887473965/b001/_x008.o: In function `CGO_DeserializeSDP': ./peerconnection.cc:379: undefined reference to `webrtc::JsepSessionDescription::JsepSessionDescription(std::string const&)' ./peerconnection.cc:382: undefined reference to `webrtc::SdpDeserialize(std::string const&, webrtc::JsepSessionDescription*, webrtc::SdpParseError*)' /tmp/go-build887473965/b001/_x008.o: In function `CGO_AddIceCandidate': ./peerconnection.cc:421: undefined reference to `webrtc::CreateIceCandidate(std::string const&, int, std::string const&, webrtc::SdpParseError*)' /tmp/go-build887473965/b001/_x008.o: In function `Peer::Initialize()': ./peerconnection.cc:57: undefined reference to `rtc::Thread::SetName(std::string const&, void const*)' ./peerconnection.cc:58: undefined reference to `rtc::Thread::SetName(std::string const&, void const*)' /tmp/go-build887473965/b001/_x008.o: In function `rtc::ArrayView<bool, -4711l>::ArrayView<bool>(bool*, unsigned long)': ././include/api/array_view.h:157: undefined reference to `rtc::FatalMessage::FatalMessage(char const*, int, std::string*)' /tmp/go-build887473965/b001/_x008.o: In function `rtc::ArrayView<int, -4711l>::ArrayView<int>(int*, unsigned long)': ././include/api/array_view.h:157: undefined reference to `rtc::FatalMessage::FatalMessage(char const*, int, std::string*)' collect2: error: ld returned 1 exit status
This is based on:
webrtc 88f5d9180eae78a6162cccd78850ff416eb82483 go-webrtc a1272c08ab1d5ca154c6794ddc5f73d2e576fe1b (current HEAD)
comment:8 Changed 13 months ago by
Owner: | set to eighthave |
---|---|
Status: | new → accepted |
comment:9 Changed 13 months ago by
I'll bet those missing symbols are caused by an ABI mismatch. Notice how all of them include std::string
in the method signature. Chances are, the library you built uses std::__cxx11::basic_string
instead of std::string
. You can check with nm --demangle libwebrtc.a | grep CreateIceCandidate
.
The fix may be to add -D_GLIBCXX_USE_CXX11_ABI=1
to CGO_CXXFLAGS
when building snowflake. For example, see comment:14:ticket:27827.
comment:10 follow-up: 12 Changed 13 months ago by
Kind of ironic that I'm working on this, since I know neither Go nor C++. But I do know C, autotools, and even pkg-config a little bit.
So I think this is starting to make sense to me. The Android .pc file that I wrote already includes -D_GLIBCXX_USE_CXX11_ABI=0
, so I guess I should change that to 1 also? Does this still need to also be set in CGO_CXXFLAGS
? Should this be building with gcc? I think gomobile defaults to clang.
comment:11 follow-up: 13 Changed 13 months ago by
And confirmed your suspicion:
$ nm --demangle libwebrtc-linux-amd64-magic.a| grep CreateIceCandidate U webrtc::CreateIceCandidate(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, webrtc::SdpParseError*) 0000000000000000 T webrtc::CreateIceCandidate(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, webrtc::SdpParseError*)
comment:12 Changed 13 months ago by
Replying to eighthave:
So I think this is starting to make sense to me. The Android .pc file that I wrote already includes
-D_GLIBCXX_USE_CXX11_ABI=0
, so I guess I should change that to 1 also? Does this still need to also be set inCGO_CXXFLAGS
? Should this be building with gcc? I think gomobile defaults to clang.
If you put -D_GLIBCXX_USE_CXX_ABI=1
in CGO_CXXFLAGS
, it will override the setting in the .pc file (with a harmless warning about redefinition).
It's better to leave -D_GLIBCXX_USE_CXX11_ABI=0
in the .pc file for now, because we're currently keeping the pre-built library at that ABI version. In the Tor Browser build, though, we're overriding it to 1
.
Cf. https://github.com/keroserene/go-webrtc/commit/a1272c08ab1d5ca154c6794ddc5f73d2e576fe1b.
I think it should work with either GCC or Clang. I think currently in the Tor Browser build, we're using GCC for linux and Clang for mac, at least if I'm interpreting this and this correctly.
comment:13 Changed 13 months ago by
comment:14 Changed 13 months ago by
I'm first just trying to get a native build repeatable here, e.g. a Debian/amd64 binary building on Debian/amd64. That's where its failing. Once I get that working, then I'll move on to gomobile.
comment:15 Changed 13 months ago by
looks like that did the trick for the GNU/Linux build:
https://gitlab.com/eighthave/go-webrtc/-/jobs/117246103
Wish me luck on the Android stuff!
comment:16 follow-up: 23 Changed 13 months ago by
Ok, baby steps here. I can now repeatably build libwebrtc-linux-amd64-magic.a:
https://gitlab.com/eighthave/go-webrtc/-/jobs/118730515
And libwebrtc-linux-arm-magic.a:
https://gitlab.com/eighthave/go-webrtc/-/jobs/118738490
I've built libwebrtc-android-arm-magic.a but its not yet repeatable:
https://gitlab.com/eighthave/go-webrtc/-/jobs/118746061
The whole shebang builds and the tests pass, but only for go 1.11:
https://gitlab.com/eighthave/go-webrtc/-/jobs/118746063
For some bizarre reason, the rest bail out with "go get: no install location for directory /builds/eighthave/go-webrtc outside GOPATH":
https://gitlab.com/eighthave/go-webrtc/pipelines/36080763
comment:17 follow-ups: 25 26 Changed 13 months ago by
Anyone know anything about gn? I guess it is something from ninja or deploy_tools. This is stumping me:
https://gitlab.com/eighthave/go-webrtc/-/jobs/119428248
+ gn gen out/Release '--args=target_os="android" target_cpu="arm" is_debug=false use_custom_libcxx=false' ERROR at //build/config/android/internal_rules.gni:122:23: Can't load input file. deps += [ "${_target_label}__build_config" ] ^-------------------------------
comment:18 Changed 13 months ago by
And there is one last weird linking issue which seems to be related to something C++, but I can't figure out the missing library:
https://gitlab.com/eighthave/snowflake/-/jobs/119448897
# github.com/keroserene/go-webrtc ../../base/allocator/allocator_shim_default_dispatch_to_linker_wrapped_symbols.cc:36: error: undefined reference to '__real_malloc' ../../base/allocator/allocator_shim_default_dispatch_to_linker_wrapped_symbols.cc:43: error: undefined reference to '__real_calloc' ../../base/allocator/allocator_shim_default_dispatch_to_linker_wrapped_symbols.cc:57: error: undefined reference to '__real_memalign' ../../base/allocator/allocator_shim_default_dispatch_to_linker_wrapped_symbols.cc:50: error: undefined reference to '__real_realloc' ../../base/allocator/allocator_shim_default_dispatch_to_linker_wrapped_symbols.cc:61: error: undefined reference to '__real_free' clang70++: error: linker command failed with exit code 1 (use -v to see invocation)
comment:19 Changed 13 months ago by
Looks like it is related to this from .travis.yml:
if [[ "$OS" == "linux" ]]; then CGO_CFLAGS="-fsanitize=address" CGO_LDFLAGS="-fsanitize=address -fuse-ld=gold" go test -v .; fi
comment:20 Changed 13 months ago by
This is really quite the adventure in linking. -fsanitize=address
now gives me lots of ASAN linking errors:
https://gitlab.com/eighthave/snowflake/-/jobs/119524452
/tmp/go-build532015574/b051/_x001.o:_cgo_export.c:function asan.module_ctor: error: undefined reference to '__asan_init' /tmp/go-build532015574/b051/_x001.o:_cgo_export.c:function asan.module_ctor: error: undefined reference to '__asan_version_mismatch_check_v9' /tmp/go-build532015574/b051/_x002.o:cgo.cgo2.c:function asan.module_ctor: error: undefined reference to '__asan_init' /tmp/go-build532015574/b051/_x002.o:cgo.cgo2.c:function asan.module_ctor: error: undefined reference to '__asan_version_mismatch_check_v9' ...
comment:21 follow-up: 24 Changed 13 months ago by
@dcf @arlolra any idea about the asan linking errors? I'm just shooting in the dark here.
comment:22 Changed 13 months ago by
Cc: | ahf added |
---|
comment:23 Changed 13 months ago by
Replying to eighthave:
The whole shebang builds and the tests pass, but only for go 1.11:
https://gitlab.com/eighthave/go-webrtc/-/jobs/118746063
For some bizarre reason, the rest bail out with "go get: no install location for directory /builds/eighthave/go-webrtc outside GOPATH":
https://gitlab.com/eighthave/go-webrtc/pipelines/36080763
This error is not really a problem: "no install location for directory /builds/eighthave/go-webrtc outside GOPATH". You can ignore it. It just means that your checkout of go-webrtc isn't under GOPATH; i.e., you did git clone https://github.com/keroserene/go-webrtc
rather than go get github.com/keroserene/go-webrtc
initially. That's actually how I usually do it too. But to make the error message go away, you can use go get
to download the go-webrtc repo.
Go 1.11 took steps away from using a centralized GOPATH and introduced modules. That's probably why you don't get the error under 1.11.
The rbm Tor Browser build actually doesn't use go get
at all--we manually git clone
the main repo and all its dependencies and put them in the same place under GOPATH that go get
would have put them. We do that because before the new modules system, there wasn't a way to download a specific commit hash for each dependency.
comment:24 Changed 13 months ago by
Replying to eighthave:
@dcf @arlolra any idea about the asan linking errors? I'm just shooting in the dark here.
So I personally don't use Travis and I don't know what the .travis.yml is about. But I see from its commit history that it's been modified many times to make adjustments to compiler and asan settings. So feel free to make any changes in .travis.yml that you need. For example see #43 and #16150 that disabled asan for non-linux platforms.
comment:25 Changed 13 months ago by
Replying to eighthave:
Anyone know anything about gn? I guess it is something from ninja or deploy_tools. This is stumping me:
Right, gn processes .gn and .gni files and outputs .ninja file. It's not actually part of depot_tools (depot_tools just contains a shim that calls the real gn binary), rather it's part of the webrtc source code, under src/tools/gn.
https://gitlab.com/eighthave/go-webrtc/-/jobs/119428248
+ gn gen out/Release '--args=target_os="android" target_cpu="arm" is_debug=false use_custom_libcxx=false' ERROR at //build/config/android/internal_rules.gni:122:23: Can't load input file. deps += [ "${_target_label}__build_config" ] ^-------------------------------
I haven't seen this error before. Maybe try omitting target_cpu="arm"
? I only mention that because the gn cross compiles documentation gives as an example
gn gen out/Default --args='target_os="android"'
and says: "(We don't have to specify target_cpu because of the conditionals mentioned above)." It seems unlikely that this will help, but I don't know.
You can try using the gn print command to see what it's trying to access. Try:
print("${_target_label}__build_config")
comment:26 Changed 13 months ago by
Replying to eighthave:
Anyone know anything about gn? I guess it is something from ninja or deploy_tools. This is stumping me:
https://gitlab.com/eighthave/go-webrtc/-/jobs/119428248
+ gn gen out/Release '--args=target_os="android" target_cpu="arm" is_debug=false use_custom_libcxx=false' ERROR at //build/config/android/internal_rules.gni:122:23: Can't load input file. deps += [ "${_target_label}__build_config" ] ^-------------------------------
Maybe the more pressing part of that error is,
Unable to load: /builds/eighthave/go-webrtc/third_party/webrtc/src/third_party/findbugs/BUILD.gn I also checked in the secondary tree for: /builds/eighthave/go-webrtc/third_party/webrtc/src/build/secondary/third_party/findbugs/BUILD.gn + exit 1 $ export ANDROID_HOME=$WEBRTC_SRC/third_party/android_tools/sdk $ ls -lR $ANDROID_HOME ls: cannot access '/builds/eighthave/go-webrtc/third_party/webrtc/src/third_party/android_tools/sdk': No such file or directory
It looks like you're missing some dependencies. My .gclient_entries
has some lines like,
'src/third_party/android_tools': 'https://chromium.googlesource.com/android_tools.git@9914c5704717424998c69e837be3631914d787cc', 'src/third_party/android_tools/ndk': 'https://chromium.googlesource.com/android_ndk.git@aabf5c8f4b1ce4269be4791b469e27b15d93a3f2',
I think those are there because my .gclient
file has target_os = ['android', 'linux']
. I may have manually edited that and sync'd, I don't remember, but there should be some command line flag to gclient to specify that. That should pull them in and unblock. Let me know if you need help figuring out how that work.
While I'm here, are you doing this? ./build/linux/sysroot_scripts/install-sysroot.py --arch=arm
From https://github.com/keroserene/go-webrtc/blob/master/build.sh#L74-L79
comment:27 Changed 13 months ago by
there should be some command line flag to gclient to specify that.
Or, not.
After running, gclient config --name src https://chromium.googlesource.com/external/webrtc
You'll want to echo "target_os = ['android']" >> .gclient
And then do the sync.
comment:28 Changed 13 months ago by
@arlolra the GOPATH issues are now fixed using symlinks. GitLab CI puts you into a git clone of the current project, I just symlinked that folder into the GOPATH.
@dcf I've looked at the .travis.yml history and the build that gave that last asan linking error is already running the same flags as go-webrtc#43: CGO_CFLAGS="-fsanitize=address" CGO_LDFLAGS="-fsanitize=address"
@arlolra thanks for pointing out the echo "target_os = ['android']" >> .gclient
thing again, I had messed that step up. I think I fixed it now.
comment:29 follow-up: 30 Changed 13 months ago by
Some progess. The asan linking errors seem to be because I forgot to export
the CGO_LDFLAGS var. Now its moved on to a new an exciting error that is totally opaque to me:
https://gitlab.com/eighthave/snowflake/-/jobs/120594108
/tmp/gomobile-work-486042020/src/gobind/go_snowflakeclientmain.go:979:22: cannot use (*proxysnowflakeclient_SnowflakeCollector)(_param_snowflakes_ref) (type *proxysnowflakeclient_SnowflakeCollector) as type snowflakeclient.SnowflakeCollector in assignment: *proxysnowflakeclient_SnowflakeCollector does not implement snowflakeclient.SnowflakeCollector (missing Melted method)
comment:30 follow-up: 32 Changed 13 months ago by
Now its moved on to a new an exciting error that is totally opaque to me:
Here're some similar issues that came up,
https://github.com/golang/go/issues/27297
https://github.com/golang/go/issues/24882
The second one seems to hint that <-chan struct{}
is an unsupported type to export,
https://godoc.org/golang.org/x/mobile/cmd/gobind#hdr-Type_restrictions
comment:31 follow-up: 33 Changed 13 months ago by
I renamed package main
to package snowflakeclient
since gomobile didn't want to work with the main package. That's a hack we've used in all the Go PTs in Orbot. I guess it is causing trouble here, is there some special meaning to package main
?
comment:32 follow-ups: 37 41 Changed 13 months ago by
Replying to arlolra:
Now its moved on to a new an exciting error that is totally opaque to me:
Here're some similar issues that came up,
https://github.com/golang/go/issues/27297
https://github.com/golang/go/issues/24882
27297 is about packaging v2ray -- that's another circumvention program :)
The second one seems to hint that
<-chan struct{}
is an unsupported type to export,
https://godoc.org/golang.org/x/mobile/cmd/gobind#hdr-Type_restrictions
So I guess this means we need changes to go-webrtc to remove such incompatible type signatures from the public interface?
https://github.com/golang/go/issues/24882#issuecomment-427263354 says "Solved my issue by writing a custom facade library which then provided an API which did not produce gomobile bind errors..." rather than changing go-webrtc, it may be possible to make a wrapper layer that provides a gomobile-compatible interface on the outside and speaks to go-webrtc using <-chan struct{}
on the inside.
comment:33 follow-up: 35 Changed 13 months ago by
Replying to eighthave:
I renamed
package main
topackage snowflakeclient
since gomobile didn't want to work with the main package. That's a hack we've used in all the Go PTs in Orbot. I guess it is causing trouble here, is there some special meaning topackage main
?
I'm looking at this gomobile sample application and it uses package main
, so I guess the renaming is necessary because of something in Orbot. But that seems unrelated to any error message you've posted.
Yes, main has special meaning, it's the package that gets run when you execute the program. Changing the package of sub-programs from main
makes sense when you want to combine them into one meta-program, which you may be doing to avoid having duplicate copies of the runtime, like in #13770.
comment:34 Changed 13 months ago by
I guess what I meant by special meaning of package main
is that it seems to prevent the code from being built into a shared library, since that's what gomobile bind
does. Here's what it looks like when doing gomobile bind
with the package main
stuff intact:
https://gitlab.com/eighthave/snowflake/-/jobs/122976281
gomobile: binding 'main' package () is not supported
As for the <-chan struct()
stuff, I have no idea, since my knowledge of Go is rudementary.
comment:35 Changed 13 months ago by
Replying to dcf:
Replying to eighthave:
I renamed
package main
topackage snowflakeclient
since gomobile didn't want to work with the main package. That's a hack we've used in all the Go PTs in Orbot. I guess it is causing trouble here, is there some special meaning topackage main
?
I'm looking at this gomobile sample application and it uses
package main
, so I guess the renaming is necessary because of something in Orbot. But that seems unrelated to any error message you've posted.
You can use gomobile to build full apps. With those main() is allowed. In our case we are using the "bind" build command to create a shared library. main() is not allowed here.
comment:36 follow-up: 40 Changed 13 months ago by
@dcf we can't build the application with Go, e.g. gomobile build
, since we need a component that runs either as an executable or a shared library. That can then be integrated with Orbot, Tor Browser for Android, etc.
I just realized, perhaps this wasn't clear before, that the current issues are when running gomobile bind
for snowflake, not go-webrtc.
Regarding package main
, I think the snowflake code should isolate the usage of package main
to only the file where it is required. That makes the rest of the code much easier to reuse for Android.
In any case, I'm trying to get it building with gomobile build
to see if there are other issues. So far so good, looks like I just need to sort out the paths to the static libbray binary I built:
comment:37 Changed 13 months ago by
Replying to dcf:
Replying to arlolra:
Now its moved on to a new an exciting error that is totally opaque to me:
Here're some similar issues that came up,
https://github.com/golang/go/issues/27297
https://github.com/golang/go/issues/24882
27297 is about packaging v2ray -- that's another circumvention program :)
The second one seems to hint that
<-chan struct{}
is an unsupported type to export,
https://godoc.org/golang.org/x/mobile/cmd/gobind#hdr-Type_restrictions
We shouldn't be running "gomobile bind" on the webrtc library it self. That should all be building and linking within goland land. The "bind command is for the final primary Go class "snowflake.go" that is going to be exposed as the shared library interface.
I think the main() function here needs to be changed for something else:
https://gitlab.com/eighthave/snowflake/blob/master/client/snowflake.go#L125
So I guess this means we need changes to go-webrtc to remove such incompatible type signatures from the public interface?
I don't think so, again, because aren't binding() against that library, we are linking it into Snowflake. What we public generate the JNI bind interface for is snowflake.go
comment:38 Changed 13 months ago by
Ok, here comes something whacktastic! gomobile build
fails because there isn't the Android "main" present (gomobile: github.com/keroserene/snowflake/client does not import "golang.org/x/mobile/app"
). BUT! It creates /tmp/gomobile-work-246996193/lib/armeabi-v7a/libclient.so. So I just copied that out, maybe it'll be usable:
https://gitlab.com/eighthave/snowflake/-/jobs/123047258
See the attached files for the binary. You can download that build product from the gitlab job also.
comment:39 Changed 13 months ago by
actually, nevermind, the compressed file is 13MB, so too large to attach to Trac. Get it from the gitlab job:
https://gitlab.com/eighthave/snowflake/-/jobs/123053147/artifacts/download
comment:40 Changed 13 months ago by
Replying to dcf:
So I guess this means we need changes to go-webrtc to remove such incompatible type signatures from the public interface?
The interface is in snowflake/client
, not go-webrtc
https://gitweb.torproject.org/pluggable-transports/snowflake.git/tree/client/interfaces.go#n42
Replying to eighthave:
I just realized, perhaps this wasn't clear before, that the current issues are when running
gomobile bind
for snowflake, not go-webrtc.
That would be why.
comment:41 Changed 13 months ago by
Replying to dcf:
So I guess this means we need changes to go-webrtc to remove such incompatible type signatures from the public interface?
https://github.com/golang/go/issues/24882#issuecomment-427263354 says "Solved my issue by writing a custom facade library which then provided an API which did not produce gomobile bind errors..." rather than changing go-webrtc, it may be possible to make a wrapper layer that provides a gomobile-compatible interface on the outside and speaks to go-webrtc using
<-chan struct{}
on the inside.
We probably need to refactor client/
into something like,
client/lib/ client/cli/ client/mobile/
where both cli/
and mobile/
import functionality from lib/
but cli/
has a package main
and builds an executable, and mobile/
exposes a compatible api for that need.
We can probably do away with a cli/
and leave that code at the top level of client/
for backwards compatibility, I'm just suggesting that for clarity.
Changed 13 months ago by
Attachment: | 0001-Start-refactoring-out-a-client-and-library.patch added |
---|
comment:42 Changed 13 months ago by
Status: | accepted → needs_review |
---|
Attached is a rudimentary patch along those lines.
comment:43 follow-up: 54 Changed 13 months ago by
arlolra's patch is fine with me, I was about to suggest something similar. eighthave, does it work for you? (Doing the main
package renaming you were already doing, that is; we could additionally split out a gomobile-specific package as comment:41 suggests.)
comment:44 Changed 13 months ago by
If there is a proper library with an API, then my guess is that we will not need client/mobile/
at all. Perhaps @n8fr8 can weigh in there.
About the patch, could a more descriptive name be used instead of package lib
? I'm not sure the rules of Go namespaces, we will need to avoid name conflicts since ultimately, we want to bundle as many PTs together into a single shared library, which then can be dynamically loaded/started/stopped from tor daemon, Orbot, in Android apps, etc. Here are the build attempts:
- Building a shared library with
gomobile bind
https://gitlab.com/eighthave/snowflake/-/jobs/123363079
- Building an Android app with
gomobile build
, which fails, then extracting the shared lib:
https://gitlab.com/eighthave/snowflake/-/jobs/123362365
Both of these jobs fail on:
$ go get ./... package git.torproject.org/pluggable-transports/snowflake.git/client/lib: cannot find package "git.torproject.org/pluggable-transports/snowflake.git/client/lib" in any of: /usr/local/go/src/git.torproject.org/pluggable-transports/snowflake.git/client/lib (from $GOROOT) /go/src/git.torproject.org/pluggable-transports/snowflake.git/client/lib (from $GOPATH)
comment:45 Changed 13 months ago by
FYI, perhaps it would be easier to test builds if you fork my project on gitlab.com, then you'd inherit the gitlab-ci setup, and it would run with the default runners.
comment:46 follow-up: 48 Changed 13 months ago by
Ok, I figured out the go get
issue, it seems sometimes that go get
decides that it needs to have .git
at the end of the path, and other times not. So I used symlinks to provide both options. Then I only built .../client/lib
, not the whole client. gomobile bind
gets pretty far before dying with:
/tmp/gomobile-work-032904235/src/gobind/go_libmain.go:1052:17: cannot use (*proxylib_SocksConnector)(_param_socks_ref) (type *proxylib_SocksConnector) as type lib.SocksConnector in assignment: *proxylib_SocksConnector does not implement lib.SocksConnector (missing Grant method) /tmp/gomobile-work-032904235/src/gobind/go_libmain.go:1061:22: cannot use (*proxylib_SnowflakeCollector)(_param_snowflakes_ref) (type *proxylib_SnowflakeCollector) as type lib.SnowflakeCollector in assignment: *proxylib_SnowflakeCollector does not implement lib.SnowflakeCollector (missing Melted method)
comment:47 Changed 13 months ago by
gomobile build
with the patch doesn't give any specific errors, but then doesn't even product the libclient.so like before.
https://gitlab.com/eighthave/snowflake/-/jobs/123571414
comment:48 follow-up: 50 Changed 13 months ago by
Replying to eighthave:
About the patch, could a more descriptive name be used instead of
package lib
? I'm not sure the rules of Go namespaces, we will need to avoid name conflicts since ultimately, we want to bundle as many PTs together into a single shared library, which then can be dynamically loaded/started/stopped from tor daemon, Orbot, in Android apps, etc.
See https://golang.org/ref/spec#Import_declarations
In the top level client/snowflake.go
, it's imported as,
sf "git.torproject.org/pluggable-transports/snowflake.git/client/lib"
and used like sf.SnowflakeCollector
, etc.
Replying to eighthave:
Ok, I figured out the
go get
issue, it seems sometimes thatgo get
decides that it needs to have.git
at the end of the path, and other times not. So I used symlinks to provide both options. Then I only built.../client/lib
, not the whole client.gomobile bind
gets pretty far before dying with:
/tmp/gomobile-work-032904235/src/gobind/go_libmain.go:1052:17: cannot use (*proxylib_SocksConnector)(_param_socks_ref) (type *proxylib_SocksConnector) as type lib.SocksConnector in assignment: *proxylib_SocksConnector does not implement lib.SocksConnector (missing Grant method) /tmp/gomobile-work-032904235/src/gobind/go_libmain.go:1061:22: cannot use (*proxylib_SnowflakeCollector)(_param_snowflakes_ref) (type *proxylib_SnowflakeCollector) as type lib.SnowflakeCollector in assignment: *proxylib_SnowflakeCollector does not implement lib.SnowflakeCollector (missing Melted method)
These are the same issues as before with unsupported export types. What needs to be done is a client/mobile/
that imports from client/lib/
but only exposes a few methods that your app would need. (On that note, since the problematic methods are now in lib/
you can probably build the top level client/
without issue, but that probably doesn't help when you want to integrate the functionality into your app. It's just for show.)
comment:49 Changed 13 months ago by
It is not possible to build the shared library, because of package main
:
$ gomobile bind -v -x -target=android/arm github.com/keroserene/snowflake/client gomobile: binding 'main' package () is not supported
Looks like building the Android app is working, except for:
gomobile: github.com/keroserene/snowflake/client does not import "golang.org/x/mobile/app"
https://gitlab.com/eighthave/snowflake/-/jobs/123682282
I'll try implementing golang.org/x/mobile/app
tomorrow, unless someone else wants to try it.
comment:50 Changed 13 months ago by
Replying to arlolra:
Replying to eighthave:
Ok, I figured out the
go get
issue, it seems sometimes thatgo get
decides that it needs to have.git
at the end of the path, and other times not. So I used symlinks to provide both options. Then I only built.../client/lib
, not the whole client.
These are the same issues as before with unsupported export types. What needs to be done is a
client/mobile/
that imports fromclient/lib/
but only exposes a few methods that your app would need.
Right--you want to build client
, not client/lib
. The point of making a separate client/lib
package was not to expose an external interface; rather it was to hide some of client's internal interfaces, specifically the ones that gomobile doesn't like.
Replying to eighthave:
It is not possible to build the shared library, because of
package main
:
$ gomobile bind -v -x -target=android/arm github.com/keroserene/snowflake/client gomobile: binding 'main' package () is not supported
Please do what you were already planning to do and do for other transports; i.e., rename the main
package to something else. We can also talk about adding a new, gomobile-specific package to the source tree to make it easier for you, but first I'd like to ensure that it works using the procedure you have used for other transports.
Looks like building the Android app is working, except for:
gomobile: github.com/keroserene/snowflake/client does not import "golang.org/x/mobile/app"I'll try implementing golang.org/x/mobile/app tomorrow, unless someone else wants to try it.
Yes, please try it; and if it works that's something we can consider adopting into the source tree.
comment:51 Changed 13 months ago by
So now I see the whole picture! By only changing the package name of client/snowflake.go
, I can now build it with gomobile bind
! That is then more or less the same kind of thing that we used before to integrate other PTs into the Android Pluggable Transport library. It is fully building now, and producing the .aar file we need:
https://gitlab.com/eighthave/snowflake/-/jobs/124110588
FYI, I set up a GitLab CI project to run gitlab-ci pipelines for the canonical repo (https://git.torproject.org/pluggable-transports/snowflake.git). That will happily run in parallel with the TravisCI stuff that's there. Since Travis doesn't let you run arbitrary docker images, it is more limited in what you can do with it. My upcoming .gitlab-ci.yml update, as seen in progress here, includes running the builds and tests against various versions of Go/Debian/Ubuntu. That should help smooth packaging and deployment.
You can see that here:
https://gitlab.com/torproject/snowflake/pipelines
comment:52 Changed 13 months ago by
As for implementing golang.org/x/mobile/app
, that now seems only necessary for a test suite built in here. That would be great to have, but our funding goal is getting snowflake running on Android first.
comment:53 Changed 13 months ago by
Resolution: | → fixed |
---|---|
Status: | needs_review → closed |
comment:54 follow-up: 55 Changed 13 months ago by
Replying to dcf:
arlolra's patch is fine with me, I was about to suggest something similar.
Should we merge that then?
comment:55 follow-up: 56 Changed 13 months ago by
Replying to arlolra:
Replying to dcf:
arlolra's patch is fine with me, I was about to suggest something similar.
Should we merge that then?
Yes, please do.
As I see it, we can consider this a rearrangement of client's internals; i.e., we're not committing to maintain a new interface for external callers. If we were, then I would be much more concerned with how we name and structure things. But we retain the freedom to refactor things in the future, keeping in mind the gomobile concerns.
HEAD of go-webrtc is at
88f5d9180eae78a6162cccd78850ff416eb82483
in webrtc, which is aroundbranch-heads/64
https://github.com/keroserene/go-webrtc/blob/master/build.sh#L12
which is what the HEAD of snowflake matches
But in tor-browser-build it's still on
c279861207c5b15fc51069e96595782350e0ac12
, circabranch-heads/58
https://gitweb.torproject.org/builders/tor-browser-build.git/tree/projects/webrtc/config
https://github.com/keroserene/go-webrtc/blob/52280a1ecdfbfc0557a962ff832dbf5632122a35/build.sh#L12
which wants an older snowflake commit
You need to line up your webrtc, go-webrtc, snowflake