Opened 7 weeks ago

Closed 3 weeks ago

Last modified 3 weeks ago

#28205 closed defect (fixed)

linking against other libwebrtc binaries errors out on missing symbols

Reported by: eighthave Owned by: eighthave
Priority: Medium Milestone:
Component: Obfuscation/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)

0001-Start-refactoring-out-a-client-and-library.patch (12.3 KB) - added by arlolra 3 weeks ago.

Download all attachments as: .zip

Change History (57)

comment:1 Changed 7 weeks ago by arlolra

HEAD of go-webrtc is at 88f5d9180eae78a6162cccd78850ff416eb82483 in webrtc, which is around branch-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, circa branch-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

comment:2 Changed 6 weeks ago by eighthave

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 Changed 6 weeks ago by eighthave

Ok, I think this problem lies in the missing #cgo android pkg-config: setup.

comment:4 Changed 6 weeks ago by arlolra

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 in reply to:  3 Changed 6 weeks ago by arlolra

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 6 weeks ago by dcf

Keywords: android added

comment:7 Changed 5 weeks ago by eighthave

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 5 weeks ago by eighthave

Owner: set to eighthave
Status: newaccepted

comment:9 Changed 5 weeks ago by dcf

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 Changed 5 weeks ago by eighthave

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 Changed 5 weeks ago by eighthave

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 in reply to:  10 Changed 5 weeks ago by dcf

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 in CGO_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 in reply to:  11 Changed 5 weeks ago by dcf

Replying to eighthave:

And confirmed your suspicion:

I expected to see arm64 here, not amd64?

comment:14 Changed 5 weeks ago by eighthave

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 5 weeks ago by eighthave

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 Changed 4 weeks ago by eighthave

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 Changed 4 weeks ago by 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" ]
                      ^-------------------------------

comment:18 Changed 4 weeks ago by eighthave

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 4 weeks ago by eighthave

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 4 weeks ago by eighthave

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 Changed 4 weeks ago by eighthave

@dcf @arlolra any idea about the asan linking errors? I'm just shooting in the dark here.

comment:22 Changed 4 weeks ago by ahf

Cc: ahf added

comment:23 in reply to:  16 Changed 4 weeks ago by dcf

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 in reply to:  21 Changed 4 weeks ago by dcf

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 in reply to:  17 Changed 4 weeks ago by dcf

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 in reply to:  17 Changed 4 weeks ago by arlolra

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 4 weeks ago by arlolra

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 4 weeks ago by eighthave

@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 Changed 4 weeks ago by eighthave

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)
Last edited 4 weeks ago by eighthave (previous) (diff)

comment:30 in reply to:  29 ; Changed 4 weeks ago by 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

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 Changed 4 weeks ago by eighthave

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 in reply to:  30 ; Changed 3 weeks ago by 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

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 in reply to:  31 ; Changed 3 weeks ago by dcf

Replying to eighthave:

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?

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 3 weeks ago by eighthave

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 in reply to:  33 Changed 3 weeks ago by n8fr8

Replying to dcf:

Replying to eighthave:

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?

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 Changed 3 weeks ago by eighthave

@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:

https://gitlab.com/eighthave/snowflake/-/jobs/122992936

comment:37 in reply to:  32 Changed 3 weeks ago by n8fr8

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 3 weeks ago by eighthave

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 3 weeks ago by eighthave

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 in reply to:  36 Changed 3 weeks ago by arlolra

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 in reply to:  32 Changed 3 weeks ago by arlolra

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.

comment:42 Changed 3 weeks ago by arlolra

Status: acceptedneeds_review

Attached is a rudimentary patch along those lines.

comment:43 Changed 3 weeks ago by dcf

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 3 weeks ago by eighthave

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 3 weeks ago by eighthave

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 Changed 3 weeks ago by eighthave

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)

https://gitlab.com/eighthave/snowflake/-/jobs/123509344

comment:47 Changed 3 weeks ago by eighthave

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 in reply to:  46 ; Changed 3 weeks ago by arlolra

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 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)

https://gitlab.com/eighthave/snowflake/-/jobs/123509344

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 3 weeks ago by 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

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 in reply to:  48 Changed 3 weeks ago by dcf

Replying to arlolra:

Replying to eighthave:

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.

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.

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 3 weeks ago by eighthave

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 3 weeks ago by eighthave

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 3 weeks ago by eighthave

Resolution: fixed
Status: needs_reviewclosed

comment:54 in reply to:  43 ; Changed 3 weeks ago by arlolra

Replying to dcf:

arlolra's patch is fine with me, I was about to suggest something similar.

Should we merge that then?

comment:55 in reply to:  54 ; Changed 3 weeks ago by dcf

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.

comment:56 in reply to:  55 Changed 3 weeks ago by arlolra

Replying to dcf:

Yes, please do.

Done

Note: See TracTickets for help on using tickets.