Skip to content
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

Update to branch-heads/64 #79

Closed
wants to merge 4 commits into from
Closed

Conversation

uumaro
Copy link
Collaborator

@uumaro uumaro commented Mar 29, 2018

I'm not sure if you want to take this yet or not. In working on cross-compiling support go-webrtc for Windows, I needed to upgrade to branch-heads/64 for the new cross-compiling support.

I'm basing my Windows changes on this branch and it works so far, producing a webrtc.lib as well as continuing to be able to build snowflake for mac and linux.

The substantive part of the change is 371aaf6; the other commits are just generated headers and libraries. Those are for your reference, and don't necessarily have to be merged as-is. I expect you'll want to generate those yourself anyway. I don't have the hardware to generate libwebrtc-darwin-amd64-magic.a.

David Fifield added 3 commits March 29, 2018 14:29
Currently at commit 88f5d9180eae78a6162cccd78850ff416eb82483.

The biggest change is that header files are no longer in a webrtc/
subdirectory. Here is the upstream change:
https://webrtc.googlesource.com/src/+/bb547203bfebcc478b263c4e9ca173c6fd5a0c5d

The signatures for PeerConnectionObserver and
CreatePeerConnectionFactory are slightly changed. I took the examples of
CreateBuiltinAudioEncoderFactory and CreateBuiltinAudioDecoderFactory
from
https://chromium.googlesource.com/external/webrtc/+/32df86ee0e1d892223f6915cd39ab9cb026d9695

This version of WebRTC seems to prefer to link with a bundled copy of
libc++ (libcxx) by default. We set use_custom_libcxx=false in order to
link with libstdc++ like before.

The build automatically used the c++11 ABI, such that I got link errors
(e.g. "undefined reference to `webrtc::JsepSessionDescription::JsepSessionDescription(std::string const&)'").
So remove the manual setting of _GLIBCXX_USE_CXX11_ABI=0 in .pc files.
This is from a build.sh of linux-amd64. build.sh has a "git clean"
command that wiped out the automatically copied files, so I re-ran the
copy loop manually. I omitted the build, buildtools, third_party, and
tools directories.
@asicerik asicerik mentioned this pull request Apr 1, 2018
@arlolra
Copy link
Collaborator

arlolra commented Apr 3, 2018

Hmm, I would expect the Linux part of this to be passing in CI, no?
https://travis-ci.org/keroserene/go-webrtc/jobs/360054090

In order to get the macOS build working, I neeeded 99f5639 but the magic is now in 9be426d

For the include/s, I did 344fe2a, which adjust the .gitignore

@uumaro
Copy link
Collaborator Author

uumaro commented Apr 3, 2018

Oh thanks—I didn't think about CI. All the undefined references seem to reference things like std::string, std::__cxx11::basic_string, std::__cxx11::basic_ostringstream, so it looks like the has to do with the use/non-use of the C++11 ABI. I removed -D_GLIBCXX_USE_CXX11_ABI=0 from the .pc files because my build.sh-built libwebrtc-linux-amd64-magic.a automatically used the C++11 ABI. Perhaps the default is different in the CI environment. I can try to build new magic without the C++11 ABI, so it's more compatible with what was there before. (We have long been overriding -D_GLIBCXX_USE_CXX11_ABI=1 in the Tor Browser build anyway, because it rebuilds the magic and happens to use the C++11 ABI (incompatible with the prepackaged magic).)

Regarding 99f5639, I found the same -DWEBRTC_MAC=1 change was necessary in the Tor Browser build: uumaro/tor-browser-build@11a5d67#diff-0. There, I speculatively also defined -DWEBRTC_LINUX=1 because I noticed that symbol was also used, though I don't know what its effect is.

Thanks for the .gitignore on include/. I noticed that there were seemingly too many subdirectories, but I didn't know that the correct ones had already been enumerated.

In order to do this, I had to hack
	defines += [ "_GLIBCXX_USE_CXX11_ABI=0" ]
into build/config/BUILD.gn.
@arlolra
Copy link
Collaborator

arlolra commented Apr 3, 2018

There, I speculatively also defined -DWEBRTC_LINUX=1 because I noticed that symbol was also used, though I don't know what its effect is.

Ok, I added similar in e45d138

I removed -D_GLIBCXX_USE_CXX11_ABI=0 from the .pc files because my build.sh-built libwebrtc-linux-amd64-magic.a automatically used the C++11 ABI.

I reverted that part since we're still providing a library here with the old ABI.

I briefly tried setting -D_GLIBCXX_USE_CXX11_ABI=1 in CI but couldn't get it to work. I think the toolchain is built with --disable-libstdcxx-dual-abi so that definition is moot.

See mapbox/mason#157 (comment)

@arlolra
Copy link
Collaborator

arlolra commented Apr 3, 2018

I briefly tried setting -D_GLIBCXX_USE_CXX11_ABI=1 in CI but couldn't get it to work. I think the toolchain is built with --disable-libstdcxx-dual-abi so that definition is moot.

I opened #81 for figuring this out.

The bulk of this is now merged though. Thanks!

@arlolra arlolra closed this Apr 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants