Last Comment Bug 643692 - Implement GYP generator generating Mozilla Makefiles
: Implement GYP generator generating Mozilla Makefiles
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Ted Mielczarek [:ted.mielczarek]
:
:
Mentors:
Depends on: 715645 732474 732492 737398 800847
Blocks: camera 633348 641630 688187 699646 757637
  Show dependency treegraph
 
Reported: 2011-03-22 03:08 PDT by Benoit Jacob [:bjacob] (mostly away)
Modified: 2012-10-14 01:25 PDT (History)
16 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
implement Makefile.in backend for GYP (10.06 KB, patch)
2011-12-22 06:15 PST, Ted Mielczarek [:ted.mielczarek]
no flags Details | Diff | Splinter Review
implement Makefile.in backend for GYP (10.64 KB, patch)
2011-12-22 13:26 PST, Ted Mielczarek [:ted.mielczarek]
no flags Details | Diff | Splinter Review
implement Makefile.in backend for GYP (12.48 KB, patch)
2011-12-29 10:32 PST, Ted Mielczarek [:ted.mielczarek]
no flags Details | Diff | Splinter Review
implement Makefile.in backend for GYP (12.64 KB, patch)
2012-01-05 13:19 PST, Ted Mielczarek [:ted.mielczarek]
no flags Details | Diff | Splinter Review

Description Benoit Jacob [:bjacob] (mostly away) 2011-03-22 03:08:24 PDT
GYP is Chromium's own build system generator:

   http://code.google.com/p/gyp/

We use source code from a few Google projects that use GYP as their build-system: at least ANGLE and (afaik) Breakpad. We are currently manually porting these to our own Makefiles, which is tedious and potentially has to be partly redone whenever we sync with upstream.

Also, for part of ANGLE, we are currently using GYP-generated VS project files on Windows, which is causing some trouble: bug 641630, bug 633348.

Having a GYP generator generating Mozilla Makefiles should be very useful for these reasons.
Comment 1 Randell Jesup [:jesup] 2011-10-04 22:21:55 PDT
Now WebRTC as well (with hundreds of .gyp files...)
Comment 2 Ted Mielczarek [:ted.mielczarek] 2011-12-01 04:35:07 PST
Breakpad's gyp files only cover the Windows client, so it wouldn't be totally useful there.

For WebRTC, I'm first trying to see if we can just make the gyp-generated Makefiles work on Windows. If that turns out to not be feasible then this is probably the tack I'll take.
Comment 3 Richard Newman [:rnewman] 2011-12-10 01:45:34 PST
Relevant to gps's work on making our build system suck less.
Comment 4 Ted Mielczarek [:ted.mielczarek] 2011-12-12 07:31:14 PST
I don't think it's particularly relevant, TBH. The likelihood of us using GYP for Mozilla at large is virtually nil.
Comment 5 Ted Mielczarek [:ted.mielczarek] 2011-12-22 06:15:59 PST
Created attachment 583770 [details] [diff] [review]
implement Makefile.in backend for GYP

This still isn't finished, but it's getting closer. It's an awful awful mess, so I apologize if anyone actually reads this patch.

The basic concept here is to:
a) Generate one Makefile per gyp target (because our build system only supports one target per-Makefile.in), putting them fairly close to the gyp file they were generated from. I generate a subdir per-target, named "gypfile_targetname".
b) Generate a common.mk that gets included by every Makefile.in, because GYP files set things like DEFINES and INCLUDES per-target-type (Release vs. Debug), so that has to be chosen at build-time, so this just encapsulates that logic.
c) Generate a top-level Makefile.in that simply uses DIRS to build all the other targets.

This doesn't produce a working WebRTC build yet, but it does compile some things. I have a few more things to fix, the known ones include:
* Adding extra libs to the link line
* Checking that build order of targets is correct wrt dependencies
* Proper paths to source files (I think object files are winding up in the srcdir right now)
* Handling some other GYP bits, like actions/rules/copies
* Mac support (ObjC, might need to pull settings from the XCodeSettings)
* Test building on Windows
Comment 6 Ted Mielczarek [:ted.mielczarek] 2011-12-22 06:19:07 PST
Sample output (for webrtc.gyp) can be seen in attachment 583771 [details] [diff] [review] on bug 688187.
Comment 7 Gregory Szorc [:gps] 2011-12-22 10:59:18 PST
Looking good!

Would it make sense to put this code under build/ instead of /media/webrtc/? IMO the files constituting the build system are fragmented enough as it is. I'd like to see consolidation of all the code logic (especially the Python bits) in a common directory so we can treat them as a Python package and not have so much PYTHONPATH hackiness.

ensure_directory_exists(path) is not thread safe. You probably want the same construct used in bug 629668:

    try:
        os.makedirs(dir)
    except OSError, ex:
        if ex.errno != errno.EEXIST:
            raise ex
        # else directory already exists. silently ignore and continue

Unless of course there is no way this can be called from multiple threads.

Also, you use underscore for that method but CamelCase elsewhere. Underscore is the standard Python naming convention. But it looks like GYP uses CamelCase. Hmmm. Same goes for getdepth(). Be consistent.

Can you convert the Makefile-producing methods into generators? This loosely couples the act of generating the Makefiles from the GYP-specific side-effect of writing out a file. Personally, I would like this because my alternate build system could consume the Makefile as a "stream" and pipe it directly into my parsing logic without involving any filesystem overhead. It also makes it easier to test the API since, again, you don't involve the filesystem. If the GYP->Makefile generation happens at check-in time, not after checkout, then I don't care as much.

On a higher-level, GYP has access to the global state of everything, no? If so, it would be awesome if you could produce a single Makefile from all the GYP files in one go. Of course, this would require adding adding support to rules.mk or circumventing the rules.mk logic. Those are probably beyond scope, but I'm throwing it out there just in case. Either way, my magical build system could consume your generated Makefiles since they conform to the existing convention, so the end result would be the same (if I can deliver).
Comment 8 Ted Mielczarek [:ted.mielczarek] 2011-12-22 13:26:53 PST
Created attachment 583919 [details] [diff] [review]
implement Makefile.in backend for GYP

I fixed a couple of the things off that list:
* Extra libs now make it to the link line (although it's still failing to link the first test program it gets to, need to figure that out)
* Paths to source files have been sorted out
* I punted on actions/rules/copies, they don't seem to be used in webrtc except for building the protobuf compiler, which I hear is optional
Comment 9 Ted Mielczarek [:ted.mielczarek] 2011-12-22 13:30:33 PST
If you'd like to try this out, the patch in bug 688187 was generated with the following command after applying this patch:

luser@cuatro:~/build/alder/media/webrtc/trunk$ ./build/gyp_chromium --debug=all --format=mozmake --depth=. -D build_with_mozilla=1 -G relative_srcdir=media/webrtc/trunk webrtc.gyp
Comment 10 Benoit Jacob [:bjacob] (mostly away) 2011-12-22 14:05:58 PST
Very excited to see this! If you want a big and useful GYP testcase, see
http://hg.mozilla.org/mozilla-central/file/c5b90ea7e475/gfx/angle/src/build_angle.gyp
Comment 11 Mike Hommey [:glandium] 2011-12-23 00:00:00 PST
(In reply to Gregory Szorc [:gps] from comment #7)
> Would it make sense to put this code under build/ instead of /media/webrtc/?

It very much would, since we already have at least ANGLE that also uses GYP and would benefit from this.
Comment 12 Ted Mielczarek [:ted.mielczarek] 2011-12-23 12:59:55 PST
Unfortunately, this code has to live inside of the gyp folder, since gyp loads its generators by module name. Also, apparently the trunk version of gyp can't build the WebRTC project, so I had to use the copy of gyp that was bundled with the WebRTC source.
Comment 13 Ted Mielczarek [:ted.mielczarek] 2011-12-29 10:32:56 PST
Created attachment 584795 [details] [diff] [review]
implement Makefile.in backend for GYP

This fixes most of the previous issues, I'm able to generate a set of Makefiles and build WebRTC on Linux. I need to test this on Mac and Windows next.
Comment 14 Ted Mielczarek [:ted.mielczarek] 2012-01-05 13:19:14 PST
Created attachment 586192 [details] [diff] [review]
implement Makefile.in backend for GYP

Some slight revisions here. I changed things to generate Makefiles directly,
instead of Makefile.ins in the srcdir. I'll attach a patch in bug 688187
showing how I'm doing this for WebRTC.
Comment 15 Ted Mielczarek [:ted.mielczarek] 2012-01-19 12:32:35 PST
I landed an updated version of this on alder:
http://hg.mozilla.org/projects/alder/rev/1d1d10574a84

I'll get post-facto review from someone once I get things working properly on Windows. (It's close there, but not quite yet.)
Comment 16 Randell Jesup [:jesup] 2012-03-02 05:55:53 PST
Ted landed a version that works on windows as well.  Yeah ted!

However, it has a few issues with the Win32 build servers
1) the use of the 'with' statement, which is not in Python 2.5 by default (see attachment , which has been landed on alder to fix this)
2) with 1) fixed, you find that the json module for python isn't on the WIn32 build servers:

Updating projects from gyp files...
*** Fix above errors and then restart with               "make -f client.mk build"
make[2]: Leaving directory `/e/builds/moz2_slave/a-w32/build'
make[1]: Leaving directory `/e/builds/moz2_slave/a-w32/build'
Traceback (most recent call last):
  File "e:/builds/moz2_slave/a-w32/build/media/webrtc/trunk/build/gyp_chromium", line 171, in <module>
    sys.exit(gyp.main(args))
  File "e:\builds\moz2_slave\a-w32\build\media\webrtc\trunk\tools\gyp\pylib\gyp\__init__.py", line 471, in main
    options.circular_check)
  File "e:\builds\moz2_slave\a-w32\build\media\webrtc\trunk\tools\gyp\pylib\gyp\__init__.py", line 72, in Load
    generator = __import__(generator_name, globals(), locals(), generator_name)
  File "e:\builds\moz2_slave\a-w32\build\media\webrtc\trunk\tools\gyp\pylib\gyp\generator\mozmake.py", line 7, in <module>
    import json
ImportError: No module named json
configure: error: failed to generate WebRTC Makefiles
Comment 17 Randell Jesup [:jesup] 2012-03-02 06:50:34 PST
For json, we can either update python to 2.6 or later on Linux and Windows build servers, or we can remove the dependency on the json module from mozmake.py, or we could install simplejson on the build servers and alternatively import that:

try:
    import json
except ImportError:
    import simplejson as json
Comment 18 Ben Hearsum (:bhearsum) 2012-03-02 09:57:07 PST
(In reply to Randell Jesup [:jesup] from comment #16)
> 1) the use of the 'with' statement, which is not in Python 2.5 by default
> (see attachment , which has been landed on alder to fix this)

You should be able to use 'from __future__ import with_statement' to fix this.

> 2) with 1) fixed, you find that the json module for python isn't on the
> WIn32 build servers:

JP just pinged me about this. This is something we can fix, he's going to file a bug and I'll have a look on Monday.
Comment 19 Randell Jesup [:jesup] 2012-03-02 10:11:42 PST
(In reply to Ben Hearsum [:bhearsum] from comment #18)
> (In reply to Randell Jesup [:jesup] from comment #16)
> > 1) the use of the 'with' statement, which is not in Python 2.5 by default
> > (see attachment , which has been landed on alder to fix this)
> 
> You should be able to use 'from __future__ import with_statement' to fix
> this.

That's exactly what the patch I've applied to alder does. :-)

> > 2) with 1) fixed, you find that the json module for python isn't on the
> > WIn32 build servers:
> 
> JP just pinged me about this. This is something we can fix, he's going to
> file a bug and I'll have a look on Monday

The Linux build servers are also Python 2.5 (without json).  As indicated in comment 17, I'm good with either updating python (yeah!) or installing simplejson (if it isn't there; I haven't checked yet) and using the snippet above.

Mac seems ok.
Comment 20 Randell Jesup [:jesup] 2012-03-12 06:06:04 PDT
just to note, turns out json was included but not needed; so i removed the json include from alder
Comment 21 Bas Schouten (:bas.schouten) 2012-03-13 08:42:21 PDT
I was looking at alder, and it appears when I touch certain headers, the correct stuff isn't rebuild (in this case sink_filter_windows.h). When I touch the source file (sink_filter_windows.cc) stuff rebuilds correctly.
Comment 22 Randell Jesup [:jesup] 2012-03-13 22:34:00 PDT
Note known bug - it doesn't rebuild Makefiles if a gyp file changes
Comment 23 Ted Mielczarek [:ted.mielczarek] 2012-04-05 07:19:40 PDT
I landed a couple of follow-ups here:
http://hg.mozilla.org/projects/alder/rev/3c906d1866b7 - Fix GNU make incompatible paths on Windows
http://hg.mozilla.org/projects/alder/rev/3bfb70703e95 - Add rules to regenerate Makefiles when GYP files change

I'm going to call this project FIXED now. These changes should get review before we land alder into mozilla-central, but this isn't blocking any WebRTC work anymore.

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