Last Comment Bug 688187 - Build Makefiles from WebRTC GYP files
: Build Makefiles from WebRTC GYP files
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: Other Branch
: All All
: -- normal (vote)
: ---
Assigned To: Ted Mielczarek [:ted.mielczarek]
:
Mentors:
Depends on: 643692 718031
Blocks: 688178
  Show dependency treegraph
 
Reported: 2011-09-21 09:30 PDT by Randell Jesup [:jesup]
Modified: 2012-06-29 15:50 PDT (History)
11 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
work in progress (6.68 KB, patch)
2011-09-27 17:13 PDT, Ralph Giles (:rillian) needinfo me
no flags Details | Diff | Splinter Review
work in progress v2 (144.53 KB, patch)
2011-09-28 16:58 PDT, Ralph Giles (:rillian) needinfo me
no flags Details | Diff | Splinter Review
updated WIP (11.30 KB, patch)
2011-09-28 18:52 PDT, [:fabrice] Fabrice Desré
no flags Details | Diff | Splinter Review
More updated file (WIP) (9.05 KB, text/plain)
2011-09-29 02:18 PDT, Randell Jesup [:jesup]
no flags Details
updated WIP (14.38 KB, patch)
2011-09-29 16:34 PDT, [:fabrice] Fabrice Desré
no flags Details | Diff | Splinter Review
current work in progress (119.45 KB, patch)
2011-09-29 16:57 PDT, Ralph Giles (:rillian) needinfo me
no flags Details | Diff | Splinter Review
updated WIP (15.09 KB, patch)
2011-09-29 18:29 PDT, [:fabrice] Fabrice Desré
no flags Details | Diff | Splinter Review
Add makefiles that invoke the native webrtc build system - WIP (46.16 KB, patch)
2011-10-03 15:54 PDT, Randell Jesup [:jesup]
no flags Details | Diff | Splinter Review
patch to use gyp directly and put object files in OBJDIR - WIP (52.11 KB, patch)
2011-10-04 00:04 PDT, Randell Jesup [:jesup]
no flags Details | Diff | Splinter Review
Patch that gets to the link phase and fails - WIP (57.01 KB, patch)
2011-10-04 01:41 PDT, Randell Jesup [:jesup]
no flags Details | Diff | Splinter Review
build with webrtc - works, all-in-one (361.06 KB, patch)
2011-10-05 01:12 PDT, Randell Jesup [:jesup]
no flags Details | Diff | Splinter Review
followup (8.62 KB, patch)
2011-10-05 10:02 PDT, [:fabrice] Fabrice Desré
no flags Details | Diff | Splinter Review
patches to make Mac build (16.80 KB, patch)
2011-10-17 15:42 PDT, Randell Jesup [:jesup]
no flags Details | Diff | Splinter Review
generate Makefile.in build files for WebRTC from GYP files (264.69 KB, patch)
2011-12-22 06:18 PST, Ted Mielczarek [:ted.mielczarek]
no flags Details | Diff | Splinter Review
generate Makefile.in build files for WebRTC from GYP files (218.09 KB, patch)
2011-12-29 10:27 PST, Ted Mielczarek [:ted.mielczarek]
no flags Details | Diff | Splinter Review
tweak include dir for webrtc vp8 build (1.36 KB, patch)
2011-12-29 10:29 PST, Ted Mielczarek [:ted.mielczarek]
no flags Details | Diff | Splinter Review
minor WebRTC GYP tweaks (2.27 KB, patch)
2011-12-30 10:51 PST, Ted Mielczarek [:ted.mielczarek]
no flags Details | Diff | Splinter Review
generate Makefile.in build files for WebRTC from GYP files (260.22 KB, patch)
2012-01-03 12:12 PST, Ted Mielczarek [:ted.mielczarek]
no flags Details | Diff | Splinter Review
workaround visibility problem with system expat (581 bytes, patch)
2012-01-03 12:13 PST, Ted Mielczarek [:ted.mielczarek]
no flags Details | Diff | Splinter Review
Fix webrtc shared_libs.mk to use the paths from the new Makefiles (8.36 KB, patch)
2012-01-03 12:16 PST, Ted Mielczarek [:ted.mielczarek]
no flags Details | Diff | Splinter Review
generate Makefiles for WebRTC from GYP files at configure time (10.62 KB, patch)
2012-01-05 13:25 PST, Ted Mielczarek [:ted.mielczarek]
no flags Details | Diff | Splinter Review
Patch for python 2.5 (486 bytes, patch)
2012-02-28 19:16 PST, Randell Jesup [:jesup]
ted: review+
khuey: feedback+
Details | Diff | Splinter Review

Description Randell Jesup [:jesup] 2011-09-21 09:30:02 PDT
Probably one Makefile for each section
Comment 1 Timothy B. Terriberry (:derf) 2011-09-21 10:11:48 PDT
Ideally we could generate these from the upstream .gyp files directly, to make it easier to track changes from upstream. I have no idea how difficult that would be.
Comment 2 Ralph Giles (:rillian) needinfo me 2011-09-22 15:19:59 PDT
There are also Android.mk files scattered throughout the code. I suspect they're more likely to have version skew than the .gyp files, but they might be easier to scan for lists of files.
Comment 3 Ralph Giles (:rillian) needinfo me 2011-09-22 16:14:34 PDT
(more thinking aloud)

Looking at webrtc/trunk/DEPS, we already have most of the dependencies: libvpx, libjpeg(turbo), yasm, expat; or they're required for the gyp build (like gyp itself) and the testing framework. 

That leaves libprotobuf, jsoncpp, and the patched libjingle as remaining external dependencies, which is a small enough number it may be better to work with the bare webrtc svn repository, which simplilfies things.
Comment 4 Timothy B. Terriberry (:derf) 2011-09-22 16:31:30 PDT
What is libprotobuf actually being used for?
Comment 5 Ralph Giles (:rillian) needinfo me 2011-09-22 17:10:20 PDT
Looks like the audio_processing module is using it for debug tracing.
Comment 6 Randell Jesup [:jesup] 2011-09-22 22:05:38 PDT
Right - and nothing else.  Very tempting to find a way to remove it. I'm tempted for the moment to import without it and to turn off those tests for now.

That leaves jsoncpp and libjingle (which includes things like ICE for some reason.
Comment 10 Randell Jesup [:jesup] 2011-09-23 13:06:53 PDT
Discussion of repo plans moved to bug 688847.  Let this one return to makefiles...  I'll hide the comments I made here to avoid confusion
Comment 11 Ralph Giles (:rillian) needinfo me 2011-09-23 13:07:57 PDT
Certainly we should update the bug here with links to google's rietveld instance when the upstream status for a patch changes. I'd suggest, therefore, that our webrtc patch bugs not be closed until they're accepted or rejected upstream.
Comment 12 Ralph Giles (:rillian) needinfo me 2011-09-27 17:13:40 PDT
Created attachment 562913 [details] [diff] [review]
work in progress

This is just a snapshot, showing how things could look. This is based on (manually) copying the LOCAL_MODULE, LOCAL_SRC_FILES, and LOCAL_C_INCLUDES from media/webrtc/src/common_audio/resampler/main/source/Android.mk

I moved webrtc/trunk/Makefile.in up a level because I'd hoped to flatten the whole thing, more like the libvpx build works. However, there are so many submodules I think it doesn't actually make sense to do that.
Comment 13 Ralph Giles (:rillian) needinfo me 2011-09-27 17:18:52 PDT
Re android.mk vs *.gyp; in theory the gyp files are easier to parse, being json-ish, but there are a lot of cross includes and the build stuff isn't well modularized, so the Android.mk files looked like a cleaner place to start.
Comment 14 Ralph Giles (:rillian) needinfo me 2011-09-28 16:58:42 PDT
Created attachment 563245 [details] [diff] [review]
work in progress v2

The patch includes a rough python script which walks the tree, parses the source and include variables from Android.mk files and generates corresponding Makefile.in files, which are also included in the patch.

Still TODO:

* add DIRS support to write_makefile so the script can update the top-level webrtc/Makefile.in (the current version is manual)
* s/CPPSRCS/CSRCS/ for .c source files
* add some missing module-wide includes, defines, and other flags
* filter out test directories
Comment 15 Ralph Giles (:rillian) needinfo me 2011-09-28 17:00:21 PDT
Comment on attachment 563245 [details] [diff] [review]
work in progress v2

Didn't get this finished today, but updating with work in progress.
Comment 16 [:fabrice] Fabrice Desré 2011-09-28 18:52:07 PDT
Created attachment 563269 [details] [diff] [review]
updated WIP

I made some update to the python script:
* add DIRS support to write_makefile so the script can update the top-level webrtc/Makefile.in
* support for CPPSRCS and CSRCS
* filter out test directories

This still fails to compile probably because we're missing some include.
Comment 17 Randell Jesup [:jesup] 2011-09-29 02:18:58 PDT
Created attachment 563346 [details]
More updated file (WIP)

Note: this is just update_makefiles.py.   Gets pretty far, dies on dependency on protobuf includes it appears.  Evil hacks to even get it this far; the model is a tough one for complex makefiles; I punted on the file with 3 effective makefiles - one main and two tests - and only did the first part.
Comment 18 [:fabrice] Fabrice Desré 2011-09-29 16:34:38 PDT
Created attachment 563592 [details] [diff] [review]
updated WIP

This builds some parts of the tree, but I still can't link code using it.
Comment 19 Ralph Giles (:rillian) needinfo me 2011-09-29 16:57:18 PDT
Created attachment 563598 [details] [diff] [review]
current work in progress

I spent some time merging Randell and Fabrice's changes this afternoon. Just a snapshot of my tree, still based on parsing the Android.mk files. Didn't get any further than Randell last night.
Comment 20 [:fabrice] Fabrice Desré 2011-09-29 18:29:49 PDT
Created attachment 563622 [details] [diff] [review]
updated WIP

Now creating a shared_libs.mk included from layout/build/Makefile.in and popuplated with the libraries we build.

This builds, but I still can't link code from netwerk/protocol/device with webrtc.
Comment 21 Randell Jesup [:jesup] 2011-10-03 15:54:44 PDT
Created attachment 564372 [details] [diff] [review]
Add makefiles that invoke the native webrtc build system - WIP
Comment 22 Randell Jesup [:jesup] 2011-10-04 00:04:16 PDT
Created attachment 564470 [details] [diff] [review]
patch to use gyp directly and put object files in OBJDIR - WIP
Comment 23 Randell Jesup [:jesup] 2011-10-04 01:41:32 PDT
Created attachment 564483 [details] [diff] [review]
Patch that gets to the link phase and fails - WIP

Gets to actual link errors for libxul
Comment 24 Randell Jesup [:jesup] 2011-10-05 01:12:17 PDT
Created attachment 564763 [details] [diff] [review]
build with webrtc - works, all-in-one
Comment 25 Randell Jesup [:jesup] 2011-10-05 01:26:38 PDT
committed:
https://hg.mozilla.org/projects/alder/rev/7b88b3cb72f2

Build with this in .mozconfig:

ac_add_options --enable-webrtc
ac_add_options --with-system-libvpx
ac_add_options --disable-crashreporter
Comment 26 [:fabrice] Fabrice Desré 2011-10-05 08:13:52 PDT
ac_add_options --enable-webrtc is useless, since it's enabled by default.
You can disable it with --disable-webrtc
Comment 27 [:fabrice] Fabrice Desré 2011-10-05 08:21:57 PDT
to build with the current alder repository, you also need to export OBJDIR.
Comment 28 [:fabrice] Fabrice Desré 2011-10-05 10:02:27 PDT
Created attachment 564904 [details] [diff] [review]
followup

Adding more header files to EXPORTS
Comment 29 Randell Jesup [:jesup] 2011-10-17 15:42:24 PDT
Created attachment 567607 [details] [diff] [review]
patches to make Mac build
Comment 30 Randell Jesup [:jesup] 2011-10-17 15:44:27 PDT
Note: fabrice's patches for camera access (on another bug) fail to build for mac
Comment 31 Ted Mielczarek [:ted.mielczarek] 2011-12-22 06:18:25 PST
Created attachment 583771 [details] [diff] [review]
generate Makefile.in build files for WebRTC from GYP files

Here's the current output of my GYP->Makefile.in generator from bug 643692. It doesn't actually build properly yet, but you can see what the output is likely to look like.
Comment 32 Randell Jesup [:jesup] 2011-12-22 07:06:37 PST
Looks good.  I assume we'll run the gyp-to-Makefile.in process when we import a new set of code from google (we'd add a command to do it to media/webrtc/webrtc-update.sh), or manually run it if we change a .gyp file.

Thanks Ted!
Comment 33 Ted Mielczarek [:ted.mielczarek] 2011-12-22 11:22:52 PST
Yeah, running it manually (or as part of a script) on import/gyp change was my intention.
Comment 34 Ted Mielczarek [:ted.mielczarek] 2011-12-29 10:27:39 PST
Created attachment 584793 [details] [diff] [review]
generate Makefile.in build files for WebRTC from GYP files

This set of Makefiles builds successfully for me on Linux. I don't know if the output is correct, but it's a start.
Comment 35 Ted Mielczarek [:ted.mielczarek] 2011-12-29 10:29:00 PST
Created attachment 584794 [details] [diff] [review]
tweak include dir for webrtc vp8 build

This patch is also necessary to make the build succeed. It just tweaks an include path for the vp8 build.
Comment 36 Ted Mielczarek [:ted.mielczarek] 2011-12-30 10:50:27 PST
Comment on attachment 584793 [details] [diff] [review]
generate Makefile.in build files for WebRTC from GYP files

These Makefiles work on Linux, but due to the way GYP functions, I don't think we'll be able to go with this exact approach. We'll probably have to generate the Makefiles from the GYP files at configure-time so that the per-platform bits are correct. The other alternative that derf suggested is to generate three sets of Makefiles (one per-platform), and select the correct platform to be included from a common Makefile at build time. That's possible, but it might require generating each platform's Makefiles on that specific platform, which would make updating the WebRTC snapshot annoying.
Comment 37 Ted Mielczarek [:ted.mielczarek] 2011-12-30 10:51:58 PST
Created attachment 584994 [details] [diff] [review]
minor WebRTC GYP tweaks

Just accumulating another minor tweak to the GYP files.

With these tweaks + the GYP backend from the other bug, I was able to generate Makefiles and build WebRTC successfully on a Mac build. Now on to Windows...
Comment 38 Ted Mielczarek [:ted.mielczarek] 2012-01-03 05:45:31 PST
these makefiles were generated using the following command:
cd media/webrtc/trunk; ./build/gyp_chromium --debug=all --format=mozmake --depth=. -D build_with_mozilla=1 -D enable_protobuf=0 -G relative_srcdir=media/webrtc/trunk webrtc.gyp
Comment 39 Ted Mielczarek [:ted.mielczarek] 2012-01-03 12:12:12 PST
Created attachment 585507 [details] [diff] [review]
generate Makefile.in build files for WebRTC from GYP files

Just a checkpoint.
Comment 40 Ted Mielczarek [:ted.mielczarek] 2012-01-03 12:13:57 PST
Created attachment 585508 [details] [diff] [review]
workaround visibility problem with system expat

This patch works around visibility issues with the system expat that libjingle links to by adding a system-headers-style wrapper header.

In the long run, I'm not sure we want to link to system expat, but we can worry about that later. (Note that libxul is currently transitively linked to expat on my Linux system anyway.)
Comment 41 Ted Mielczarek [:ted.mielczarek] 2012-01-03 12:16:45 PST
Created attachment 585509 [details] [diff] [review]
Fix webrtc shared_libs.mk to use the paths from the new Makefiles

This patch (which is actually from jesup) fixes the paths in shared_libs.mk to match the locations where things get built with the new Makefiles.

With all of my patches in this bug applied on top of alder, I'm able to successfully link libxul.
Comment 42 Ted Mielczarek [:ted.mielczarek] 2012-01-05 13:25:43 PST
Created attachment 586195 [details] [diff] [review]
generate Makefiles for WebRTC from GYP files at configure time

This patch lets us generate the Makefiles from the gyp files directly
into the objdir while running configure. This worked in a clobber
build for me on Linux, I'm running it through a Mac clobber right now
as well. If this works okay on Mac/Linux, then I think we should get
it landed on alder, and I'll continue to work on Windows support.
Comment 43 Ted Mielczarek [:ted.mielczarek] 2012-01-19 12:35:20 PST
Landed a combined version of this on alder:
http://hg.mozilla.org/projects/alder/rev/75ef608c54d2

It's working on Linux, and it was working on Mac (I need to re-test there, I forgot to do so before landing). Windows is close, but we still have just a few small issues to work through. jesup is going to merge mozilla-central into alder to pick up the changes that created the separate "gkmedia" library. We'll change things so that the WebRTC code gets linked into gkmedia, which should solve one of the remaining issues (the libjingle copy of expat having symbol conflicts with Mozilla's built-in expat on Windows).
Comment 44 Ted Mielczarek [:ted.mielczarek] 2012-01-20 14:29:00 PST
I merged mozilla-central in today, and landed a few tweaks:
http://hg.mozilla.org/projects/alder/rev/3a981cf610f7 (link WebRTC code into gkmedias instead of libxul)
http://hg.mozilla.org/projects/alder/rev/84616cd44046 (work around some upstream code on Windows that doesn't actually seem to link)

...and my build succeeds on Windows. I'm not sure if anything works, since I can't get fabrice's patches in bug 692955 to build, but it seems promising.
Comment 45 Randell Jesup [:jesup] 2012-02-28 19:16:36 PST
Created attachment 601494 [details] [diff] [review]
Patch for python 2.5

Win32 build machines run python 2.5
Comment 46 Randell Jesup [:jesup] 2012-02-28 19:33:55 PST
Comment on attachment 601494 [details] [diff] [review]
Patch for python 2.5

Win32 build machines are *still* python 2.5.  Really simply patch.

This patch is for alder, for mozmake.py, which itself has not yet been reviewed, so an rs= may be appropriate.  

(I'm asking since I've told people we'd be doing reviews on alder checkins now, since we now are getting ready to land non-imported code in volume and also have new-to-mozilla external commiters joining the effort.)
Comment 47 Ted Mielczarek [:ted.mielczarek] 2012-03-08 08:10:22 PST
Comment on attachment 601494 [details] [diff] [review]
Patch for python 2.5

Review of attachment 601494 [details] [diff] [review]:
-----------------------------------------------------------------

Gross, but thanks.
Comment 48 Ted Mielczarek [:ted.mielczarek] 2012-04-05 07:21:43 PDT
This is building on all our Tier-1 platforms on Tinderbox now (Android is busted, but we'll deal with that separately).
Comment 49 Mike Hommey [:glandium] 2012-04-05 07:27:10 PDT
Were the {C,CPP}FLAGS issues solved?
Comment 50 Ted Mielczarek [:ted.mielczarek] 2012-04-05 08:07:20 PDT
Yes, jesup patched that in bug 737398 by making the GYPfiles specify essential flags in something like a moz_cflags variable, and ignoring the default flags.

Note You need to log in before you can comment on or make changes to this bug.