We currently use gyp to build the webrtc signaling code (media/webrtc/signaling/*). With the work in Bug 1336429 to switch from gyp to gn to build webrtc.org code, we should try to find a way of building the signaling code using moz.build. It would be ugly to continue to use gyp here when webrtc.org has switched to gn, and it would be unfortunate to have to write our own gn files for this.

Try job here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=59dbbde5fc7668c34cfa09ab520fb0dd9ac39e9b

I'll review when I'm off PTO

Note: all of these should also be reviewed by a build peer I think

Comment on attachment 8890453 [details] Bug 1382182 - Build peerconnection using moz.build; https://reviewboard.mozilla.org/r/161592/#review168402 ::: media/webrtc/signaling/src/peerconnection/moz.build:21 (Diff revision 1) > +if CONFIG['OS_TARGET'] == 'Linux': > + DEFINES['WEBRTC_LINUX'] = True > + DEFINES['WEBRTC_POSIX'] = True > + DEFINES['HAVE_UINT64_T'] = True > +elif CONFIG['OS_TARGET'] == 'Darwin': > + DEFINES['WEBRTC_MAC'] = True > + DEFINES['WEBRTC_POSIX'] = True > + DEFINES['HAVE_UINT64_T'] = True > +elif CONFIG['OS_TARGET'] == 'WINNT': > + DEFINES['WEBRTC_WIN'] = True > + DEFINES['HAVE_UINT64_T'] = True > + DEFINES['HAVE_WINSOCK2_H'] = True > +else: > + DEFINES['WEBRTC_POSIX'] = True > + DEFINES['HAVE_UINT64_T'] = True I think there are a couple of moz.builds that need this stuff (because they include webrtc stuff, or build it), and if things are added often one is fogotten. perhaps these could be moved into some included file everything shares? dom/media, dom/media/webrtc, dom/media/systemservices, peerconnection (and other signaling/src/* dirs in the future), etc.

Comment on attachment 8890454 [details] Bug 1382182 - Build signaling common using moz.build; https://reviewboard.mozilla.org/r/161594/#review168404

Comment on attachment 8890449 [details] Bug 1382182 - Build jsep using moz.build; https://reviewboard.mozilla.org/r/161584/#review168408 ::: media/webrtc/signaling/src/jsep/moz.build:12 (Diff revision 1) > +if CONFIG['OS_TARGET'] == 'Linux': > + DEFINES['WEBRTC_LINUX'] = True > + DEFINES['WEBRTC_POSIX'] = True > +elif CONFIG['OS_TARGET'] == 'Darwin': > + DEFINES['WEBRTC_MAC'] = True > + DEFINES['WEBRTC_POSIX'] = True > +elif CONFIG['OS_TARGET'] == 'WINNT': > + DEFINES['WEBRTC_WIN'] = True > + See comment on other patch -- share these somehow

Comment on attachment 8890450 [details] Bug 1382182 - Build sdp using moz.build; https://reviewboard.mozilla.org/r/161586/#review168412

Comment on attachment 8890451 [details] Bug 1382182 - Build media-conduit using moz.build; https://reviewboard.mozilla.org/r/161588/#review168414 ::: media/webrtc/signaling/src/media-conduit/moz.build:19 (Diff revision 1) > +if CONFIG['OS_TARGET'] == 'Linux': > + DEFINES['WEBRTC_LINUX'] = True > + DEFINES['WEBRTC_POSIX'] = True > +elif CONFIG['OS_TARGET'] == 'Darwin': > + DEFINES['WEBRTC_MAC'] = True > + DEFINES['WEBRTC_POSIX'] = True > +elif CONFIG['OS_TARGET'] == 'WINNT': > + DEFINES['WEBRTC_WIN'] = True > +else: > + DEFINES['WEBRTC_POSIX'] = True and again

Comment on attachment 8890452 [details] Bug 1382182 - Build mediapipeline using moz.build; https://reviewboard.mozilla.org/r/161590/#review168416 ::: media/webrtc/signaling/src/mediapipeline/moz.build:17 (Diff revision 1) > +if CONFIG['OS_TARGET'] == 'Linux': > + DEFINES['WEBRTC_LINUX'] = True > + DEFINES['WEBRTC_POSIX'] = True > + DEFINES['HAVE_UINT64_T'] = True > +elif CONFIG['OS_TARGET'] == 'Darwin': > + DEFINES['WEBRTC_MAC'] = True > + DEFINES['WEBRTC_POSIX'] = True > + DEFINES['HAVE_UINT64_T'] = True > +elif CONFIG['OS_TARGET'] == 'WINNT': > + DEFINES['WEBRTC_WIN'] = True > + DEFINES['HAVE_UINT64_T'] = True > + DEFINES['HAVE_WINSOCK2_H'] = True > +else: > + DEFINES['WEBRTC_POSIX'] = True > + DEFINES['HAVE_UINT64_T'] = True and one more time

Comment on attachment 8890452 [details] Bug 1382182 - Build mediapipeline using moz.build; https://reviewboard.mozilla.org/r/161590/#review168416 > and one more time This can also be deduplicated e.g., ```python DEFINES['HAVE_UINT64_T'] = True if CONFIG['OS_TARGET'] != 'WINNT': DEFINES['WEBRTC_POSIX'] = True if CONFIG['OS_TARGET'] == 'Linux': DEFINES['WEBRTC_LINUX'] = True elif CONFIG['OS_TARGET'] == 'Darwin': DEFINES['WEBRTC_MAC'] = True elif CONFIG['OS_TARGET'] == 'WINNT': DEFINES['WEBRTC_WIN'] = True DEFINES['HAVE_WINSOCK2_H'] = True ```

When combining common WEBRTC_<PLATFORM> also include existing usage e.g., $ rg -g moz.build WEBRTC_LINUX media/webrtc/signaling/gtest/moz.build 10: DEFINES['WEBRTC_LINUX'] = True media/webrtc/trunk/gtest/moz.build 59: DEFINES['WEBRTC_LINUX'] = True http://searchfox.org/mozilla-central/search?q=WEBRTC_LINUX&path=moz.build

As suggested, I've added a webrtc.mozbuild file that can be included to set the defines and updated the existing uses in tree to make use of it.

Thanks. Ignoring bug 1391716 builds fine on FreeBSD. Tests from /webrtc-landing page appear to work as before.

Mass change P1->P2 to align with new Mozilla triage process

Comment on attachment 8890449 [details] Bug 1382182 - Build jsep using moz.build; https://reviewboard.mozilla.org/r/161584/#review187160 ::: dom/media/systemservices/moz.build:28 (Diff revision 2) > -if CONFIG['OS_TARGET'] == 'WINNT': > +if CONFIG['OS_TARGET'] != 'WINNT': > - DEFINES['WEBRTC_WIN'] = True > -else: > - DEFINES['WEBRTC_POSIX'] = True > # Must match build/gyp.mozbuild: enable_libevent > DEFINES['WEBRTC_BUILD_LIBEVENT'] = True Does this want to move to webrtc.mozbuild as well? ::: media/webrtc/signaling/gtest/moz.build:6 (Diff revision 2) > # -*- Mode: python; indent-tabs-mode: nil; tab-width: 40 -*- > # vim: set filetype=python: > # This Source Code Form is subject to the terms of the Mozilla Public > # License, v. 2.0. If a copy of the MPL was not distributed with this > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > +include('../../webrtc.mozbuild') nit: I think the topsrcdir-absolute path is more readable here. ::: media/webrtc/signaling/moz.build:7 (Diff revision 2) > +# vim: set filetype=python: > +# This Source Code Form is subject to the terms of the Mozilla Public > +# License, v. 2.0. If a copy of the MPL was not distributed with this > +# file, You can obtain one at http://mozilla.org/MPL/2.0/. > +DIRS += [ > + 'src/jsep', You could skip having this file if nothing is actually going to get built here and just list the subdirs directly in media/webrtc/moz.build. ::: media/webrtc/signaling/src/jsep/moz.build:6 (Diff revision 2) > +# -*- Mode: python; indent-tabs-mode: nil; tab-width: 40 -*- > +# vim: set filetype=python: > +# This Source Code Form is subject to the terms of the Mozilla Public > +# License, v. 2.0. If a copy of the MPL was not distributed with this > +# file, You can obtain one at http://mozilla.org/MPL/2.0/. > +include('../../../webrtc.mozbuild') again re: the topsrcdir path.

Comment on attachment 8890450 [details] Bug 1382182 - Build sdp using moz.build; https://reviewboard.mozilla.org/r/161586/#review187162 ::: media/webrtc/signaling/src/sdp/sipcc/moz.build:5 (Diff revision 2) > +# -*- Mode: python; indent-tabs-mode: nil; tab-width: 40 -*- > +# vim: set filetype=python: > +# This Source Code Form is subject to the terms of the Mozilla Public > +# License, v. 2.0. If a copy of the MPL was not distributed with this > +# file, You can obtain one at http://mozilla.org/MPL/2.0/. Can you just merge this moz.build file into the parent one? This code all gets linked into the same place, splitting it into separate moz.build files doesn't really buy you much.

Comment on attachment 8890451 [details] Bug 1382182 - Build media-conduit using moz.build; https://reviewboard.mozilla.org/r/161588/#review187166

Comment on attachment 8890452 [details] Bug 1382182 - Build mediapipeline using moz.build; https://reviewboard.mozilla.org/r/161590/#review187168 ::: media/webrtc/signaling/src/mediapipeline/moz.build:30 (Diff revision 2) > +] > + > +FINAL_LIBRARY = 'xul' > + > +if CONFIG['GNU_CXX']: > + CXXFLAGS += ['-Wno-shadow'] Maybe this wants to wind up in webrtc.mozbuild?

Comment on attachment 8890453 [details] Bug 1382182 - Build peerconnection using moz.build; https://reviewboard.mozilla.org/r/161592/#review187170 ::: media/webrtc/signaling/src/peerconnection/moz.build:23 (Diff revision 2) > + '/media/webrtc/signaling/src/media-conduit', > + '/media/webrtc/signaling/src/mediapipeline', > + '/media/webrtc/trunk', > +] > + > +# Multiple uses of logTag How hard would it be to fix the `logTag` issue? Being able to use `UNIFIED_SOURCES` would speed up builds a bit.

Comment on attachment 8890454 [details] Bug 1382182 - Build signaling common using moz.build; https://reviewboard.mozilla.org/r/161594/#review187172 ::: media/webrtc/signaling/src/common/moz.build:8 (Diff revision 2) > +# This Source Code Form is subject to the terms of the Mozilla Public > +# License, v. 2.0. If a copy of the MPL was not distributed with this > +# file, You can obtain one at http://mozilla.org/MPL/2.0/. > +DIRS += [ > + 'browser_logging', > + 'time_profiling', I would just merge the contents of both of the other moz.build files here into this one.

Comment on attachment 8890453 [details] Bug 1382182 - Build peerconnection using moz.build; https://reviewboard.mozilla.org/r/161592/#review187170 > How hard would it be to fix the `logTag` issue? Being able to use `UNIFIED_SOURCES` would speed up builds a bit. There are few other places with similar problems. I filed Bug 1402334 to follow up on this.

Pushed by dminor@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/b2b754756c4c Build jsep using moz.build; r=ted,jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/addaa13f513c Build sdp using moz.build; r=ted,jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/bf92c7059f79 Build media-conduit using moz.build; r=ted,jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/ce7d7a06cf83 Build mediapipeline using moz.build; r=ted,jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/a3c417b41724 Build peerconnection using moz.build; r=ted,jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/1f0a69b73cd1 Build signaling common using moz.build; r=ted,jesup

https://hg.mozilla.org/mozilla-central/rev/b2b754756c4c https://hg.mozilla.org/mozilla-central/rev/addaa13f513c https://hg.mozilla.org/mozilla-central/rev/bf92c7059f79 https://hg.mozilla.org/mozilla-central/rev/ce7d7a06cf83 https://hg.mozilla.org/mozilla-central/rev/a3c417b41724 https://hg.mozilla.org/mozilla-central/rev/1f0a69b73cd1