We can't build with system lcms anymore and we won't be able to any time soon. filed in response to bug 469556.

Added a patch for this. It builds fine on my local machine - I'm running it through the tryserver now. This is certainly not my area of expertise - I'm mostly just going off of mxr. I'll flag for review once the tryserver build comes out positive.

Comment on attachment 352925 [details] [diff] [review] fix 0.1 tryserver all green. Flagging vlad for review. Vlad, I'm not sure who the appropriate build system people are for the sr. Can you flag them for sr? if your r+ this? I'll be gone for the next 11 days, and after that i'll have 1 day before being gone again until Jan 6th. If this needs to be pushed through soon someone else should take ownership.

Oh, was just thinking -- we rename the lcms symbols, right? To avoid conflicts with system lcms in case something else pulls it in?

Comment on attachment 352925 [details] [diff] [review] fix 0.1 -#if MOZ_NATIVE_LCMS==1 -#define WRAP_LCMS_HEADERS -#endif #ifdef WRAP_LCMS_HEADERS icc34.h lcms.h #endif Just kill those following lines. (And in js/src/config/system-headers as well.) --- a/toolkit/toolkit-makefiles.sh How about you move this block up underneath MAKEFILES_libmar and use the same style for it? Not a big deal, but might as well be consistent. Looks fine otherwise.

vlad - no, I don't think so. What kind of scenario would have it pulled in?

ted - I assume I should kill the lines immediately before as well? (they deal with setting WRAP_LCMS_HEADERS, which it seems like we don't care about). Running the build through the tryserver again just to be safe. I'll land it once it goes green.

Comment on attachment 352925 [details] [diff] [review] fix 0.1 Flagging beltzner for 191 approval. Beltzner - you said we wanted this over email, so it should be a no-brainer.

pushed to 8f347bf50a53. Will land in 191 after baking.

I guess this broke static-analysis tbox

Comment on attachment 352925 [details] [diff] [review] fix 0.1 And this is not the patch which was pushed to m-c

This patch broke non-libxul clobber builds on linux. On x86-64 this manifests as a build bustage... on i686 it's just a text relocation. The problem is this hunk in config/system-headers: --- a/config/system-headers Mon Jan 12 21:47:31 2009 +0100 +++ b/config/system-headers Mon Jan 12 16:20:45 2009 -0800 @@ -1014,18 +1014,6 @@ png.h #if MOZ_NATIVE_ZLIB==1 zlib.h #endif -#if MOZ_ENABLE_LIBXUL!=1 -#if BUILD_STATIC_LIBS!=1 -#define WRAP_LCMS_HEADERS -#endif -#endif -#if MOZ_NATIVE_LCMS==1 -#define WRAP_LCMS_HEADERS -#endif -#ifdef WRAP_LCMS_HEADERS -icc34.h -lcms.h -#endif #ifdef MOZ_ENABLE_STARTUP_NOTIFICATION libsn/sn.h libsn/sn-common.h We should still be wrapping icc34.h and lcms.h in the no-libxul case: this hunk should only have removed the MOZ_NATIVE_LCMS case. I will commit a fix, but please make sure you include it when you port to the branch.

hrgh, given that the committed patch didn't match the reviewed patch, I'm going to back this out

this is the patch that was committed and broke bsmedberg's build. It has the changes ted asked for in the sr+. It looks like those are the problematic ones. I have to head out but I'll re-submit a patch once I get back with the proper fixes.

Comment on attachment 356815 [details] [diff] [review] patch 0.2 - fixes bsmedberg's issue I've added an updated patch that should fix things. Flagging bsmedberg and ted. Bsmedberg - I don't have access to an x86_64 box. Is there some way I can test that this definitely fixes things?

pushed to mc in 71a49097a2fd.

pushed to mozilla-1.9.1 in 94974f18a47c. Resolving as fixed.

Please remember the fixed1.9.1 keyword.

yep - I had a midair collision with you when you added it. Sorry about that.