I'm open to any suggestions for cleaning this up

Comment on attachment 487696 [details] [diff] [review] patch >diff --git a/toolkit/library/Makefile.in b/toolkit/library/Makefile.in >--- a/toolkit/library/Makefile.in >+++ b/toolkit/library/Makefile.in >@@ -271,6 +271,11 @@ endif > > include $(topsrcdir)/config/rules.mk > >+ifdef MOZ_TREE_FREETYPE >+-lfreetype: >+ make -C ../../modules/freetype2 >+endif >+ > export:: $(RDF_UTIL_SRC_CPPSRCS) $(INTL_UNICHARUTIL_UTIL_CPPSRCS) > $(INSTALL) $^ . > Why is this necessary?

(In reply to comment #2) > Comment on attachment 487696 [details] [diff] [review] > Why is this necessary? I removed modules/freetype2 from tier_platform_dirs because we no longer need to create the Makefile from (the no deleted) Makefile.in. Unfortunately, this also removes the rule for making libfreetype. As I said in comment 0, any suggestions for making this cleaner are welcome.

I believe you can add it to tier_platform_staticdirs, and it will do what you want.

Comment on attachment 487885 [details] [diff] [review] patch Not too thrilled about exporting all those variables; bumping this to ted to see if he has a better idea.

A few concerns here: 1. It feels like a static library is being generated almost coincidentally as a result of a configure test failing due to quirks in our toolchain. Specifying --disable-shared explicitly would be nicer. 2. Freetype's build system uses libtool, which doesn't do -fPIC because shared libs are disabled. Right now, not using -fPIC causes more memory usage due to code that can't be shared between processes. 3. This makes it more difficult to do fakelibs. Fakelibs lets us lay out libxul better (bug 603370) and improve startup time and memory usage (via custom linker tricks).

Comment on attachment 487885 [details] [diff] [review] patch >diff --git a/configure.in b/configure.in >+# Run freetype configure scrip "script". >+ >+if test "$MOZ_TREE_FREETYPE"; then >+ export CFLAGS="$CFLAGS -std=c99" >+ export CPPFLAGS="$CPPFLAGS" >+ export CXXFLAGS="$CXXFLAGS" >+ export LDFLAGS="$LDFLAGS" >+ export CONFIG_FILES="unix-cc.mk:unix-cc.in unix-def.mk:unix-def.in freetype-config freetype2.pc:freetype2.in" >+ ac_configure_args="$ac_configure_args --host=$host" You may want to sanity-check this, since the meaning of --host and --target is different in our configure and every other configure. In our configure --host is the build machine, and --target is the thing you're compiling for, but in every other configure --build is the build machine, --host is the thing you're compiling for, and --target is the platform your binaries will generate code for (if you were building a compiler). >+ AC_OUTPUT_SUBDIRS(modules/freetype2) >+fi I take it freetype's configure script doesn't know how to calculate these values correctly, which is why we have to pass them down from our configure? Does the freetype configure work with MSVC? If not, please make --enable-tree-freetype fail when using MSVC. r=me with those fixes.

Before you check this in, *please* make sure the thing is building with -fPIC.

carrying ted's review

(In reply to comment #10) > Created attachment 489919 [details] [diff] [review] > patch for checkin > > carrying ted's review IIRC CPPFLAGS are for the c preprocessor and CXXFLAGS are for the c++ compiler so the -fPIC in CPPFLAGS isn't necessary, though I guess harmless if it still builds.

found a configure flag for -fPIC, which I like better

pushed http://hg.mozilla.org/mozilla-central/rev/f57bef48f41f