Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(38)

Issue 2361623004: GN: Change rtc_source_set targets --> rtc_static_library (Closed)

Created:
2 years, 8 months ago by kjellander_webrtc
Modified:
2 years, 7 months ago
CC:
webrtc-reviews_webrtc.org, yujie_mao (webrtc), zhuangzesen_agora.io, tlegrand-webrtc, qiang.lu, peah-webrtc, bjornv1, video-team_agora.io, tterriberry_mozilla.com, fengyue_agora.io, sdk-team_agora.io, minyue-webrtc, mflodman, Andrew MacDonald, zhengzhonghou_agora.io, stefan-webrtc, kwiberg-webrtc, danilchap, henrika_webrtc, audio-team_agora.io, hlundin-webrtc, niklas.enbom, the sun, perkj_webrtc, aluebs-webrtc
Target Ref:
refs/pending/heads/master
Project:
webrtc
Visibility:
Public.

Description

GN: Change rtc_source_set targets --> rtc_static_library This changes most non-test related rtc_source_set targets to be rtc_static_library instead. Targets without any .cc files are excluded. This should bring back the build behavior we used to have with GYP (i.e. same symbols exported in the libjingle_peerconnection.a file, which are used by some downstream projects). After doing an Android build with these changes: $ nm --defined-only -g -C out/Release/lib.unstripped/libjingle_peerconnection_so.so | grep -i createpeerconnectionf 00077c51 T Java_org_webrtc_PeerConnectionFactory_nativeCreatePeerConnectionFactory $ nm --defined-only -g -C out/Release/obj/webrtc/api/libjingle_peerconnection.a | grep -i createpeerconnectionf 00000001 T webrtc::CreatePeerConnectionFactory(rtc::Thread*, rtc::Thread*, rtc::Thread*, webrtc::AudioDeviceModule*, cricket::WebRtcVideoEncoderFactory*, cricket::WebRtcVideoDecoderFactory*) 00000001 T webrtc::CreatePeerConnectionFactory() See https://chromium.googlesource.com/chromium/src/+/master/tools/gn/docs/cookbook.md#Note-on-static-libraries for more details on this. NOTICE: This should be further cleaned up in the future, to reduce binary bloat and unnecessary linking time. Right now it's more important to restore the desired build output though. BUG=webrtc:6410, chromium:630755 Committed: https://crrev.com/b62dbbe985c643cf4ee28e4c73c75bb3ef5e4d54 Cr-Commit-Position: refs/heads/master@{#14364}

Patch Set 1 : Change all rtc_source_set -> rtc_static_library #

Patch Set 2 : Excluded a few targets #

Patch Set 3 : Reverted test related targets #

Total comments: 2

Patch Set 4 : Restored accidental rebase errors. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+91 lines, -91 lines) Patch
M webrtc/BUILD.gn View 1 2 3 chunks +3 lines, -3 lines 0 comments Download
M webrtc/api/BUILD.gn View 1 2 chunks +2 lines, -2 lines 0 comments Download
M webrtc/audio/BUILD.gn View 1 2 1 chunk +1 line, -1 line 0 comments Download
M webrtc/call/BUILD.gn View 1 2 1 chunk +1 line, -1 line 0 comments Download
M webrtc/common_audio/BUILD.gn View 3 chunks +3 lines, -3 lines 0 comments Download
M webrtc/common_video/BUILD.gn View 1 chunk +1 line, -1 line 0 comments Download
M webrtc/examples/BUILD.gn View 1 2 3 4 chunks +4 lines, -4 lines 0 comments Download
M webrtc/libjingle/xmllite/BUILD.gn View 1 chunk +1 line, -1 line 0 comments Download
M webrtc/libjingle/xmpp/BUILD.gn View 1 chunk +1 line, -1 line 0 comments Download
M webrtc/media/BUILD.gn View 1 2 1 chunk +1 line, -1 line 0 comments Download
M webrtc/modules/audio_coding/BUILD.gn View 1 2 19 chunks +19 lines, -19 lines 0 comments Download
M webrtc/modules/audio_conference_mixer/BUILD.gn View 1 chunk +1 line, -1 line 0 comments Download
M webrtc/modules/audio_device/BUILD.gn View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M webrtc/modules/audio_mixer/BUILD.gn View 1 chunk +1 line, -1 line 0 comments Download
M webrtc/modules/audio_processing/BUILD.gn View 1 2 4 chunks +4 lines, -4 lines 0 comments Download
M webrtc/modules/bitrate_controller/BUILD.gn View 1 chunk +1 line, -1 line 0 comments Download
M webrtc/modules/congestion_controller/BUILD.gn View 1 chunk +1 line, -1 line 0 comments Download
M webrtc/modules/desktop_capture/BUILD.gn View 3 chunks +3 lines, -3 lines 0 comments Download
M webrtc/modules/media_file/BUILD.gn View 1 chunk +1 line, -1 line 0 comments Download
M webrtc/modules/pacing/BUILD.gn View 1 chunk +1 line, -1 line 0 comments Download
M webrtc/modules/remote_bitrate_estimator/BUILD.gn View 2 chunks +2 lines, -2 lines 0 comments Download
M webrtc/modules/rtp_rtcp/BUILD.gn View 1 chunk +1 line, -1 line 0 comments Download
M webrtc/modules/utility/BUILD.gn View 1 chunk +1 line, -1 line 0 comments Download
M webrtc/modules/video_capture/BUILD.gn View 3 chunks +3 lines, -3 lines 0 comments Download
M webrtc/modules/video_coding/BUILD.gn View 1 2 7 chunks +7 lines, -7 lines 0 comments Download
M webrtc/modules/video_processing/BUILD.gn View 3 chunks +3 lines, -3 lines 0 comments Download
M webrtc/p2p/BUILD.gn View 2 chunks +2 lines, -2 lines 0 comments Download
M webrtc/pc/BUILD.gn View 1 chunk +1 line, -1 line 0 comments Download
M webrtc/sdk/BUILD.gn View 2 chunks +2 lines, -2 lines 0 comments Download
M webrtc/stats/BUILD.gn View 1 2 1 chunk +1 line, -1 line 0 comments Download
M webrtc/system_wrappers/BUILD.gn View 1 4 chunks +6 lines, -6 lines 0 comments Download
M webrtc/test/BUILD.gn View 1 2 1 chunk +1 line, -1 line 0 comments Download
M webrtc/test/fuzzers/BUILD.gn View 1 chunk +1 line, -1 line 0 comments Download
M webrtc/tools/BUILD.gn View 1 5 chunks +5 lines, -5 lines 0 comments Download
M webrtc/video/BUILD.gn View 1 2 1 chunk +1 line, -1 line 0 comments Download
M webrtc/voice_engine/BUILD.gn View 3 chunks +3 lines, -3 lines 0 comments Download

Messages

Total messages: 36 (24 generated)
the sun
https://codereview.webrtc.org/2361623004/diff/90001/webrtc/modules/audio_device/BUILD.gn File webrtc/modules/audio_device/BUILD.gn (left): https://codereview.webrtc.org/2361623004/diff/90001/webrtc/modules/audio_device/BUILD.gn#oldcode118 webrtc/modules/audio_device/BUILD.gn:118: "android/opensles_recorder.h", intentional?
2 years, 8 months ago (2016-09-22 18:08:11 UTC) #9
kjellander_webrtc
Thanks for spotting the rebase error Fredrik. Do you guys think this will work as ...
2 years, 8 months ago (2016-09-22 20:13:34 UTC) #15
brucedawson
lgtm, and thanks, but if possible please add a reference to chromium bug 630755 to ...
2 years, 8 months ago (2016-09-22 20:22:50 UTC) #18
the sun
lgtm
2 years, 8 months ago (2016-09-23 07:22:58 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/2361623004/150001
2 years, 8 months ago (2016-09-23 07:28:33 UTC) #25
commit-bot: I haz the power
Committed patchset #4 (id:150001)
2 years, 8 months ago (2016-09-23 07:38:56 UTC) #28
commit-bot: I haz the power
Patchset 4 (id:??) landed as https://crrev.com/b62dbbe985c643cf4ee28e4c73c75bb3ef5e4d54 Cr-Commit-Position: refs/heads/master@{#14364}
2 years, 8 months ago (2016-09-23 07:39:05 UTC) #30
kjellander_webrtc
On 2016/09/23 07:39:05, commit-bot: I haz the power wrote: > Patchset 4 (id:??) landed as ...
2 years, 8 months ago (2016-09-23 08:52:38 UTC) #31
Michael Achenbach
Did you compare the .a file sizes to the old ones in gyp? In V8 ...
2 years, 7 months ago (2016-09-29 12:40:36 UTC) #33
brucedawson
On 2016/09/29 12:40:36, machenbach (slow) wrote: > Did you compare the .a file sizes to ...
2 years, 7 months ago (2016-09-29 21:25:18 UTC) #34
kjellander_webrtc
On 2016/09/29 21:25:18, brucedawson wrote: > On 2016/09/29 12:40:36, machenbach (slow) wrote: > > Did ...
2 years, 7 months ago (2016-09-30 06:14:22 UTC) #35
Michael Achenbach
2 years, 7 months ago (2016-09-30 08:07:39 UTC) #36
Message was sent while issue was closed.
See https://mail.google.com/mail/u/0/#label/gn-dev/15574884e2f3f9c8 for
reference. It looks like in gyp we had thin archives by default while in GN the
archives are self contained (and therefore bigger).

Powered by Google App Engine
This is Rietveld 408576698