Building with GN instead of GYP

DanielTBrown commented 8 years ago

Does it make sense to transition the build to using GN instead of GYP? @agouaillard mentioned the issue in his comment on pull request #18.

Alex referenced the Google+ discuss-webrtc PSA by Henrik Kjellander https://groups.google.com/forum/?fromgroups#!topic/discuss-webrtc/QMJBvIn1Y3o (as I write this, updated 12 hours ago), which states that all builds other than iOS now use GN instead of GYP.

Presumably, updating to GN might be a problem for people building older revisions of WebRTC.

Thoughts?

agouaillard commented 8 years ago

I meant this PSA:

https://groups.google.com/forum/?fromgroups#!topic/discuss-webrtc/TB87VP8M4VQ

On Mon, Aug 29, 2016 at 6:30 PM, DanielTBrown notifications@github.com wrote:

Does it make sense to transition the build to using GN instead of GYP? @agouaillard https://github.com/agouaillard mentioned the issue in his comment on pull request #18 https://github.com/vsimon/webrtcbuilds/pull/18.

Alex referenced the Google+ discuss-webrtc PSA by Henrik Kjellander https://groups.google.com/forum/?fromgroups#!topic/ discuss-webrtc/QMJBvIn1Y3o (as I write this, updated 12 hours ago), which states that all builds other than iOS now use GN instead of GYP.

Presumably, updating to GN might be a problem for people building older revisions of WebRTC.

Thoughts?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/vsimon/webrtcbuilds/issues/20, or mute the thread https://github.com/notifications/unsubscribe-auth/AAT1nsZlTJKlz_V0uOzMKaMA5XEfwXEBks5qkwk5gaJpZM4JvpqI .

Alex. Gouaillard, PhD, PhD, MBA

Principal Architect - Citrix, San Francisco

President - CoSMo Software Consulting, Singapore

sg.linkedin.com/agouaillard

-

DanielTBrown commented 8 years ago

@agouaillard: Thank you for the clarification. I looked at the post by Henrick Kjellander, and it appears to me that the current procedure now includes the call to 'webrtc/build/gyp_webrtc.py'. So, I think we are OK.

agouaillard commented 8 years ago

depending on the OS. But here again, I think there is a misunderstanding. I wanted to point at the fact that you are supposed to run "gclient runhooks" still.

extract from my blog post from last year:

VI.gclient runhooks

The hooks in this case come from the DEPS file.

check_root_dir.py # just checks if you re still using “trunk” or not. Backward compatibility.
sync_chromium.py # the one you re gonna learn to hate. Takes 40mn from Mountain View, CA with 89MB DL.
setup_links.py # see below
download_from_google_storage # get the test files
src/webrtc/build/gyp_webrtc.py # generate the build files – see below

even if the last step has been removed, you still have everything else that is being run. You can check in the DEPS file, at the botto what is run today, to see what you would be missing.

On Tue, Aug 30, 2016 at 6:08 PM, DanielTBrown notifications@github.com wrote:

@agouaillard https://github.com/agouaillard: Thank you for the clarification. I looked at the post by Henrick Kjellander, and it appears to me that the current procedure now includes the call to 'webrtc/build/gyp_webrtc.py'. So, I think we are OK.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/vsimon/webrtcbuilds/issues/20#issuecomment-243492130, or mute the thread https://github.com/notifications/unsubscribe-auth/AAT1nn5ii3hKw_sWChZRTk1kFsH4bT7Kks5qlFWXgaJpZM4JvpqI .

Alex. Gouaillard, PhD, PhD, MBA

Principal Architect - Citrix, San Francisco

President - CoSMo Software Consulting, Singapore

sg.linkedin.com/agouaillard

-

DanielTBrown commented 8 years ago

From my experience over the past few days, WebRTC for Microsoft Windows 64-bit must be built using GN with Microsoft Visual Studio 2015. I still have the following problems, if anyone has any suggestions:

The build result .lib files are scattered in various directories, so the call to the library utility to build the output webrtc_full library does not work.

I am building revision 52 (just to have a stable reference for my work), but I encountered similar problems with the trunk. Here are the issues I ran into:

Building with Microsoft Visual Studio 2013 does not work, because the build reports C++ compiler error C1083 with the text 'cannot open compiler generated file'. The best explanation I can find is that there is a known problem with multiple simultaneous compilations on a multi-core computer, due to one of the compiler components having a threading problem. The suggested solution I found was to compile on only a single core at a time, which makes an already slow build process much slower. When I switched to using Microsoft Visual Studio 2015, the problem went away.
Building with GYP resulted in a link conflict when generating executables, because at least one component was incorrectly built for x86 32-bit instead of x64 64-bit. Building with GN eliminated the problem.

So, once I have the full library building correctly (please give me any suggestions as to how to do this, when the lib files are in scattered directories), I plan to generate a pull request with the proposed updates. Other updates I would like to include:

Add an option to suppress the 'clean' operation on the downloaded source code, so when rebuilding, the entire source tree is not loaded.
Add an option to build from a specified revision by git tag rather than hash value. Perhaps this works already, I have not tried it. I suspect it does not, and if anyone can guide me to the git commands I need to do this in the script, I would appreciate any assistance.

GN does appear to be substantially faster than GYP, and the handling of build options (x86/x64, debug/release) is much cleaner.

DanielTBrown commented 8 years ago

I want to be clear: I am proposing these changes at the moment only for Microsoft Windows. I can build with no problems on Linux using the older GYP.

vsimon commented 8 years ago

Add an option suppress 'clean' operation. Great suggestion.
As for git tags, did you mean branch names?

I see branch names i.e.

  $ git branch -a
  remotes/branch-heads/52
  remotes/branch-heads/53
  remotes/branch-heads/54

so I don't think this will work already based on the gclient step in the checkout function, gclient sync --force --revision $revision. Perhaps an additional step after the gclient sync step might work git checkout branch-heads/52 where 52 can be the release branch specified via an option.

Sorry I don't have a Windows machine at the moment for your other questions.

DanielTBrown commented 8 years ago

@vsimon Thank you. You are correct, I meant branch names. I have a git command that returns the revision based on the branch name, so solving this problem should be easy.

@agouaillard Thank you for the reference. I think I now have Henrik's instructions implemented precisely.

The other issues have been resolved. I am now testing. If this works, I will clean up the script and submit a pull request.

DanielTBrown commented 8 years ago

The GN-based builds are almost working now. When I combine all the libraries, I'm getting many duplicate symbol errors, such as:

protobuf_lite.lib(arena.obj) : warning LNK4006: "public: __cdecl google::protobuf::Arena::~Arena(void)" (??1Arena@protobuf@google@@QEAA@XZ) already defined i n protobuf_full.lib(arena.obj); second definition ignored

The libraries being combined are (debug build for Microsoft Windows):

src/out/Debug_x64/obj/webrtc/base/rtc_base.lib src/out/Debug_x64/obj/webrtc/base/rtc_base_approved.lib src/out/Debug_x64/obj/webrtc/base/rtc_task_queue.lib src/out/Debug_x64/obj/webrtc/modules/audio_coding/neteq_unittest_proto.lib src/out/Debug_x64/obj/webrtc/modules/audio_processing/audioproc_debug_proto.lib src/out/Debug_x64/obj/webrtc/modules/audio_processing/audioproc_unittest_proto.lib src/out/Debug_x64/obj/webrtc/rtc_event_log_proto.lib src/out/Debug_x64/obj/third_party/boringssl/boringssl.lib src/out/Debug_x64/obj/third_party/boringssl/boringssl_asm.lib src/out/Debug_x64/obj/third_party/expat/expat.lib src/out/Debug_x64/obj/third_party/libjpeg_turbo/libjpeg.lib src/out/Debug_x64/obj/third_party/libjpeg_turbo/simd.lib src/out/Debug_x64/obj/third_party/libjpeg_turbo/simd_asm.lib src/out/Debug_x64/obj/third_party/libsrtp/libsrtp.lib src/out/Debug_x64/obj/third_party/libvpx/libvpx.lib src/out/Debug_x64/obj/third_party/libvpx/libvpx_yasm.lib src/out/Debug_x64/obj/third_party/libyuv/libyuv.lib src/out/Debug_x64/obj/third_party/openmax_dl/dl/dl.lib src/out/Debug_x64/obj/third_party/opus/opus.lib src/out/Debug_x64/obj/third_party/protobuf/protobuf_full.lib src/out/Debug_x64/obj/third_party/protobuf/protobuf_lite.lib src/out/Debug_x64/obj/third_party/protobuf/protoc_lib.lib src/out/Debug_x64/obj/third_party/usrsctp/usrsctp.lib src/out/Debug_x64/obj/third_party/winsdk_samples/winsdk_samples.lib src/out/Debug_x64/obj/third_party/yasm/yasm_utils.lib

Any suggestions?

agouaillard commented 8 years ago

Some libs (base vs base_approved) are duplicate, and if you just grep all of them by extension, that's what you get. for my build scripts, I manually maintain the list of libs. Not Ideal.

On Fri, Sep 2, 2016 at 1:26 AM, DanielTBrown notifications@github.com wrote:

The GN-based builds are almost working now. When I combine all the libraries, I'm getting many duplicate symbol errors, such as:

protobuf_lite.lib(arena.obj) : warning LNK4006: "public: __cdecl google::protobuf::Arena::~Arena(void)" (??1Arena@protobuf@google@@QEAA@XZ) already defined i n protobuf_full.lib(arena.obj); second definition ignored

The libraries being combined are (debug build for Microsoft Windows):

src/out/Debug_x64/obj/webrtc/base/rtc_base.lib src/out/Debug_x64/obj/webrtc/base/rtc_base_approved.lib src/out/Debug_x64/obj/webrtc/base/rtc_task_queue.lib src/out/Debug_x64/obj/webrtc/modules/audio_coding/neteq_unittest_proto.lib src/out/Debug_x64/obj/webrtc/modules/audio_processing/ audioproc_debug_proto.lib src/out/Debug_x64/obj/webrtc/modules/audio_processing/ audioproc_unittest_proto.lib src/out/Debug_x64/obj/webrtc/rtc_event_log_proto.lib src/out/Debug_x64/obj/third_party/boringssl/boringssl.lib src/out/Debug_x64/obj/third_party/boringssl/boringssl_asm.lib src/out/Debug_x64/obj/third_party/expat/expat.lib src/out/Debug_x64/obj/third_party/libjpeg_turbo/libjpeg.lib src/out/Debug_x64/obj/third_party/libjpeg_turbo/simd.lib src/out/Debug_x64/obj/third_party/libjpeg_turbo/simd_asm.lib src/out/Debug_x64/obj/third_party/libsrtp/libsrtp.lib src/out/Debug_x64/obj/third_party/libvpx/libvpx.lib src/out/Debug_x64/obj/third_party/libvpx/libvpx_yasm.lib src/out/Debug_x64/obj/third_party/libyuv/libyuv.lib src/out/Debug_x64/obj/third_party/openmax_dl/dl/dl.lib src/out/Debug_x64/obj/third_party/opus/opus.lib src/out/Debug_x64/obj/third_party/protobuf/protobuf_full.lib src/out/Debug_x64/obj/third_party/protobuf/protobuf_lite.lib src/out/Debug_x64/obj/third_party/protobuf/protoc_lib.lib src/out/Debug_x64/obj/third_party/usrsctp/usrsctp.lib src/out/Debug_x64/obj/third_party/winsdk_samples/winsdk_samples.lib src/out/Debug_x64/obj/third_party/yasm/yasm_utils.lib

Any suggestions?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/vsimon/webrtcbuilds/issues/20#issuecomment-244242466, or mute the thread https://github.com/notifications/unsubscribe-auth/AAT1ngOObwlyl_0AKV0x6TakXumNr385ks5ql184gaJpZM4JvpqI .

Alex. Gouaillard, PhD, PhD, MBA

President - CoSMo Software Consulting, Singapore

sg.linkedin.com/agouaillard

-

DanielTBrown commented 8 years ago

The previous (GYP-based) version of webrtcbuilds grabs all the .lib files, and it works. I think @vsimon found GYP build options that built specifically for standalone without the duplicate libraries. I do not see a corresponding gn --args='...' option for a standalone build.

gn --args='...' options that might be adding unnecessary libraries include:

rtc_include_tests, which is by default 'true'.
include_examples, which is referenced under the description of rtc_sanitize_coverage, but not documented, and may or may not actually exist.

The WebRTC build options are described in the file out/src/webrtc/build/webrtc.gni.

A more accurate set of build options for a specific target is available with the command 'gn args --list, where '' is the gn target directory for a build.

DanielTBrown commented 8 years ago

Thank you @agouaillard and @vsimon for your help. If you want to see my current version (with changes noted below) is in the repository DanielTBrown/webrtcbuilds branch DTB-20160829.

Problems:

The library build does not have an accurate list of libraries to combine.
The current version builds only Microsoft Windows x64 binaries, not Microsoft Windows x32 binaries. The additional targets are easy to add, but I wanted to get everything working first.
On Friday afternoon, 'gclient sync' stopped working for me (also issued manually), and I have not yet worked out why. That will have to wait for Monday. I get the error (paths edited for simplicity):

subprocess.CalledProcessError: Command '['webrtcbuilds\depot_tools\python276_bin\python.exe', 'webrtcbuilds\depot_tools\win_toolchain\get_toolchain_if_necessary.py', '--output-json', 'webrtcbuilds\out\src\chromium\src\build\win_toolchain.json', '95ddda401ec5678f15eeed01d2bee08fcbc5ee97']' returned non-zero exit status 1

The features added in this version are all that I set out to add:

Builds using gn, not gyp (for Microsoft Windows only).
New -b option builds based on the specified branch tag. The script looks up the git revision hash. The user can write './build.sh -b "branch-heads/52"'.
New -g option with '-g sync' performs a 'gclient sync' and builds. '-g sync' does not fetch the source code (other than what is required for the 'gclient sync'). '-g fetch' (the default) performs a 'git fetch' and builds.
On Microsoft Windows, checks for presence of Microsoft Visual Studio 2015, which is now required. I had to remove my option to use Microsoft Visual Studio 2013, because it can no longer be used.

Also:

The build.html file describes the scripts for a script developer. This documentation is very primitive, and may or may not be useful.
In build.sh, the handling of the -g option (the symbol GETTYPE) is not as clean as the other code written by @vsimon. I would like to do better, but I think this would require more refactoring.
In util.sh, each Microsoft Windows platform build is about 6 lines long, with only minor differences between them. This could be refactored.

vsimon commented 8 years ago

For the checkout/sync step, I just tried this and had a really good experience getting the source by removing clean altogther:

diff --git a/build.sh b/build.sh
index 143b56b..99263b2 100755
--- a/build.sh
+++ b/build.sh
@@ -51,7 +51,6 @@ if [ -n "$MSVSVER" ]; then
 fi

 set-platform
-clean $OUTDIR
 check::deps $PLATFORM
 check::depot-tools $PLATFORM $DEPOT_TOOLS_URL $DEPOT_TOOLS_DIR

gclient handles the initial fetch as well as follow-up syncs (even after other source files were "patched"). gclient seems alot better now at syncing than I remembered working with it awhile ago.

This was on 16.04. I wonder if removing clean would work commonly then? If so, obviously clean would then just remove the build artifacts.

vsimon commented 8 years ago

I'm going to try and go down the GN rabbit-hole on 16.04 at least. @DanielTBrown your comments https://github.com/vsimon/webrtcbuilds/issues/20#issuecomment-244280342 https://github.com/vsimon/webrtcbuilds/issues/20#issuecomment-244510035 and branch have been super helpful for catching-up.

DanielTBrown commented 8 years ago

@vsimon Thank you for your comments. I think I see your point about checkout/sync. If I understand correctly, the idea is to update the script to something like:

if depot_tools does not exist, then load the depot_tools. If depot_tools exists, then just update.
if out (or out/src) does not exist, then fetch the source. If out (or out/src) exists, then use gclient to sync

In this way, the 'clean' option is eliminated, and the script becomes simpler. Also, the script always follows the procedure documented in the WebRTC native build instructions.

DanielTBrown commented 8 years ago

@vsimon On Monday I will be working on the Windows build again, so I will make sure to merge any updates from you. You are certainly welcome to use anything in my branch, it was my intention to post a pull request once I had everything working cleanly.

vsimon commented 8 years ago

This is a work-in-progress pull request https://github.com/vsimon/webrtcbuilds/pull/21

Goal was to get the test (compile and run sample app) to pass with the GN-based instructions for building. It finally passed on ubuntu 16.04, going to try osx next, and as well as do some further testing. For the time being, I've left Windows alone.

DanielTBrown commented 8 years ago

I have merged the changes from pull request #21 into the branch DanielTBrown/webrtcbuilds branch DTB-20160829. Under Microsoft Windows there is still a problem, I am working on it. During build of branch-heads/52 the error occurs (paths shortened with ... for readability):

Traceback (most recent call last): File "src/build/landmines.py", line 148, in sys.exit(main()) File "src/build/landmines.py", line 135, in main gyp_environment.SetEnvironment() File "...\out\src\chromium\src\build\gyp_environment.py", line 34, in SetEnvironment vs_toolchain.SetEnvironmentAndGetRuntimeDllDirs() File "...\out\src\chromium\src\build\vs_toolchain.py", line 42, in SetEnvironmentAndGetRuntimeDllDirs Update() File "...\out\src\chromium\src\build\vs_toolchain.py", line 334, in Update subprocess.check_call(get_toolchain_args) File "...\depot_tools\python276_bin\lib\subprocess.py", line 540, in check_call raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['...\depot_tools\python276_bin\python.exe', '...\depot_tools\win_toolchain\get_toolchain_if_necessary.py', '--output-json', '...\out\src\chromium\src\build\win_toolchain.json', '95ddda401ec5678f15eeed01d2bee08fcbc5ee97']' returned non-zero exit status 1 Error: Command '...\depot_tools\python276_bin\python.exe src/build/landmines.py' returned non-zero exit status 1 in ...\out\src\chromium Hook ''...\depot_tools\python276_bin\python.exe' -u src/sync_chromium.py --target-revision 34689ee3d1d8f80d2ea34fadccb2b3e6f422d3a2' took 674.23 secs Error: Command '...\depot_tools\python276_bin\python.exe -u src/sync_chromium.py --target-revision 34689ee3d1d8f80d2ea34fadccb2b3e6f422d3a2' returned non-zero exit status 2 in ...\out

DanielTBrown commented 8 years ago

I have traced the code, if I understand correctly, it appears that gclient is not finding the Microsoft Visual Studio tools.

vsimon commented 8 years ago

I don't think I changed the deps install or checkout process much, unless the absence of the whole-sale clean might have affected it?

The minimal test case now passes on 16.04 and OSX in pull request #21 in both debug and release configs at least. It can probably still use some refactoring and tightening up but so far so good.

DanielTBrown commented 8 years ago

The same problem occurs if I perform the WebRTC native build procedure manually, so the problem has nothing to do with anything you did. The problem seems to have something to do with the detection of Microsoft Visual Studio 2015 deep in the python scripts. Since I do not see the problem reported elsewhere, it must have something to do with my Microsoft Windows 7 development system. I am working on it.

vsimon commented 8 years ago

Gotcha thx @DanielTBrown

DanielTBrown commented 8 years ago

I grabbed the current version of pull request #21, and built under Ubuntu 16.04.1 (new install). The build stopped after compiling the WebRTC source for debug. I then re-ran it, and received the console output below. You will see that the first ninja run completed, but nothing else:

Platform set: linux64 Checking dependencies Checking depot-tools HEAD is now at 0abcd26 Fix typobug in bot_update.py. Building revision: 432950cab292beb8efb88e37024088e9d7297504 Associated revision number: 14143 Checking out WebRTC revision (this will take awhile): 432950cab292beb8efb88e37024088e9d7297504 Syncing projects: 100% (2/2), done.

____ running '/usr/bin/python -c import os,sys;script = os.path.join("trunk","check_rootdir.py"); = os.system("%s %s" % (sys.executable,script)) if os.path.exists(script) else 0' in '/home/dbrown/Work/WebRTCBuilds/out'

____ running '/usr/bin/python -u src/sync_chromium.py --target-revision cf9457edb7c734f509dd12149acd78410c518570' in '/home/dbrown/Work/WebRTCBuilds/out' Chromium already up to date: cf9457edb7c734f509dd12149acd78410c518570

____ running '/usr/bin/python src/setup_links.py' in '/home/dbrown/Work/WebRTCBuilds/out'

____ running '/usr/bin/python src/build/landmines.py --landmine-scripts src/webrtc/build/get_landmines.py --src-dir src' in '/home/dbrown/Work/WebRTCBuilds/out'

____ running '/usr/bin/python src/third_party/instrumented_libraries/scripts/download_binaries.py' in '/home/dbrown/Work/WebRTCBuilds/out'

____ running 'download_from_google_storage --directory --recursive --num_threads=10 --no_auth --quiet --bucket chromium-webrtc-resources src/resources' in '/home/dbrown/Work/WebRTCBuilds/out' Patching WebRTC source Compiling WebRTC Done. Made 191 targets from 94 files in 125ms ninja: Entering directory `.' ninja: no work to do.

vsimon commented 8 years ago

Thanks for trying.

Does the build exit with error code 1? It must right? Can you add set -x to the top of build.sh to get a trace of every command.

I'm currently hitting one issue still on Ubuntu 16.04 w/gcc during the compile, still working through it. OSX w/clang seems to work.

DanielTBrown commented 8 years ago

I found that the dependencies in the WebRTC/chromium script were not present. While running the script to install them, it locked up installing the Microsoft TrueType fonts. They must be installed manually. While running the manual installer for the fonts, Comcast went down. I suspect the dependencies are the problem, I will document the install process tomorrow, Comcast-permitting.

DanielTBrown commented 8 years ago

For Ubuntu 16.04.1, here is the procedure to load the build dependencies:

Manually install the Microsoft core TrueType fonts, by installing the package ttf-mscorefonts-installer.
In the WebRTC 'src' folder, perform: ./build/install-build-deps.sh

Discussion

The WebRTC native build instructions include a description of how to load the prerequisite software (https://webrtc.org/native-code/development/prerequisite-sw/). The webrtcbuilds procedure 'check::deps' in util.sh script does not appear to install all the build dependencies. It appears necessary to run the script as specified in the WebRTC documentation:

./build/install-build-deps.sh

This path is located in the WebRTC 'src' folder, so the WebRTC sources must be downloaded before calling install-build-deps.sh.

You may want to invoke install-build-deps.sh under sudo, to avoid the script asking for your password when installing:

sudo ./build/install-build-deps.sh

install-build-deps.sh attempts to install the Microsoft TrueType core fonts. For me, the procedure hung in the license acknowledgement screen. I had to manually install the fonts before calling install-build-deps.sh. This makes the full procedure:

Step 1: sudo apt-get install ttf-mscorefonts-installer

This will install the installer. The installer will then run, which will download and install the fonts. Proceed to the next step only after the font installation is complete.

Step 2: In the WebRTC 'src' directory, call ./build/install-build-deps.sh

After this completes, then webrtcbuilds build.sh will execute.

vsimon commented 8 years ago

Hi @DanielTBrown

The current check::deps is based on old instructions which asked you to install a bunch of packages

    packages="curl wget git python python-pip default-jdk g++ ruby
      libnss3-dev libasound2-dev libpulse-dev libjpeg-dev libxv-dev
      libgtk2.0-dev libexpat1-dev libxtst-dev libxss-dev libudev-dev
      libgconf2-dev libgnome-keyring-dev libpci-dev libgl1-mesa-dev lib32stdc++6
      lib32z1"

Having an install build deps script is encouraging. I'll look to integrating your suggestions. In other news, my build problems from pull request 21 on linux/mac have resolved, I have not touched windows yet (I'll open a issue), but I'm inclined to merge.

vsimon / webrtcbuilds