Closed Bug 688187 Opened 13 years ago Closed 12 years ago

Build Makefiles from WebRTC GYP files

Categories

(Core :: Audio/Video, defect)

Other Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jesup, Assigned: ted)

References

Details

Attachments

(2 files, 20 obsolete files)

10.62 KB, patch
Details | Diff | Splinter Review
486 bytes, patch
ted
: review+
khuey
: feedback+
Details | Diff | Splinter Review
Probably one Makefile for each section
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.
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.
(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.
What is libprotobuf actually being used for?
Looks like the audio_processing module is using it for debug tracing.
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.
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
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.
Attached patch work in progress (obsolete) — Splinter Review
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.
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.
Attached patch work in progress v2 (obsolete) — Splinter Review
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
Attachment #562913 - Attachment is obsolete: true
Comment on attachment 563245 [details] [diff] [review]
work in progress v2

Didn't get this finished today, but updating with work in progress.
Attached patch updated WIP (obsolete) — Splinter Review
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.
Attachment #563245 - Attachment is obsolete: true
Attached file More updated file (WIP) (obsolete) —
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.
Attached patch updated WIP (obsolete) — Splinter Review
This builds some parts of the tree, but I still can't link code using it.
Attachment #563269 - Attachment is obsolete: true
Attached patch current work in progress (obsolete) — Splinter Review
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.
Attached patch updated WIP (obsolete) — Splinter Review
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.
Attachment #563592 - Attachment is obsolete: true
Attachment #564372 - Attachment is obsolete: true
Gets to actual link errors for libxul
Attachment #564470 - Attachment is obsolete: true
Depends on: 643692
Attachment #563346 - Attachment is obsolete: true
Attachment #563598 - Attachment is obsolete: true
Attachment #563622 - Attachment is obsolete: true
Attachment #564483 - Attachment is obsolete: true
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
ac_add_options --enable-webrtc is useless, since it's enabled by default.
You can disable it with --disable-webrtc
to build with the current alder repository, you also need to export OBJDIR.
Attached patch followup (obsolete) — Splinter Review
Adding more header files to EXPORTS
Attached patch patches to make Mac build (obsolete) — Splinter Review
Note: fabrice's patches for camera access (on another bug) fail to build for mac
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.
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!
Yeah, running it manually (or as part of a script) on import/gyp change was my intention.
This set of Makefiles builds successfully for me on Linux. I don't know if the output is correct, but it's a start.
Attachment #583771 - Attachment is obsolete: true
This patch is also necessary to make the build succeed. It just tweaks an include path for the vp8 build.
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.
Attachment #584793 - Attachment is obsolete: true
Attachment #584794 - Attachment is obsolete: true
Attached patch minor WebRTC GYP tweaks (obsolete) — Splinter Review
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...
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
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.)
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.
Assignee: rjesup → ted.mielczarek
Attachment #564904 - Attachment is obsolete: true
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.
Attachment #585507 - Attachment is obsolete: true
Depends on: 718031
Attachment #564763 - Attachment is obsolete: true
Attachment #567607 - Attachment is obsolete: true
Attachment #584994 - Attachment is obsolete: true
Attachment #585508 - Attachment is obsolete: true
Attachment #585509 - Attachment is obsolete: true
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).
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.
Win32 build machines run python 2.5
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.)
Attachment #601494 - Flags: review?(ted.mielczarek)
Attachment #601494 - Flags: review?(khuey)
Attachment #601494 - Flags: review?(khuey) → feedback?(khuey)
Comment on attachment 601494 [details] [diff] [review]
Patch for python 2.5

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

Gross, but thanks.
Attachment #601494 - Flags: review?(ted.mielczarek) → review+
This is building on all our Tier-1 platforms on Tinderbox now (Android is busted, but we'll deal with that separately).
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Summary: Build Makefile.in's for major WebRTC modules → Build Makefiles from WebRTC GYP files
Were the {C,CPP}FLAGS issues solved?
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.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: