Closed Bug 835551 Opened 9 years ago Closed 9 years ago
Build failure due to missing define of UINT32
Building gjs/gnome-shell against an esr17 snapshot I get the following build failure. make all-am make: Entering directory `/usr/local/src/jhbuild-checkout/source/gjs' CXX libgjs_la-jsapi-private.lo In file included from /opt/gnome/include/js/jsprvtd.h:28:0, from /opt/gnome/include/js/jsdbgapi.h:14, from ./gjs/compat.h:36, from gjs/jsapi-util.h:31, from gjs/jsapi-private.cpp:30: /opt/gnome/include/js/HashTable.h: In static member function 'static void js::detail::HashTable<T, HashPolicy, AllocPolicy>::staticAsserts()': /opt/gnome/include/js/HashTable.h:293:9: error: 'UINT32_MAX' was not declared in this scope make: *** [libgjs_la-jsapi-private.lo] Error 1 make: Leaving directory `/usr/local/src/jhbuild-checkout/source/gjs' make: *** [all] Error 2 As a hacky workaround, I just added a "#define UINT32_MAX 4294967295U" to HashTable.h, which gets things building, but I figure there is probably a better way to fix this?
SpiderMonkey's build processes include a command-line -D__STDC_LIMIT_MACROS (well, I think it might be -include fileThatIncludesThatDefine.h, but whatever) so that these macros are always available. If we're using those macros in headers now -- and it looks like we are -- we may have silently started requiring the same of our users. (Hmm, and I guess we're requiring the same thing for -Wno-c++0x-extensions wrt using the Attributes.h macros like MOZ_OVERRIDE and such.) I'm not sure what the exact right solution, to fix everyone, and to accommodate our adding more such auto-#defines like this, would be. Maybe you have some ideas. In the meantime, adding -D__STDC_LIMIT_MACROS to your compile lines should fix things here.
Status: UNCONFIRMED → NEW
Ever confirmed: true
-D__STDC_LIMIT_MACROS is defined in js-confdefs.h, however this header is not installed, additionally it contains a lot of autogenerated defines, that wouldnt want to be installed into a public/internal header. Waldo suggested on IRC to split out the stuff that is required for an embedded build, into a separate header.
Just talked to ted in-person about this, sounds like he's good with this plan of having two separate -includes for the configury stuff and for the constant #defines.
I think this'll do the trick for JS. It seems to compile a browser far enough that I don't think it's buggy on that front, and shell builds seem equally good. Interestingly I realized Mozilla-as-embedder is weird yet again, in that the only reason this stuff works for us is that our mozilla-config.h just *happens* to define the same things that RequiredDefines.h defines. I'm not sure of a way around this. But it doesn't matter for this immediate bug, or for the present.
And of course with that patch, you'd add -include js/RequiredDefines.h instead, and you'd pick up other #define requirements we'll grow in the future (like __STDC_FORMAT_MACROS at some point soon).
Comment on attachment 711138 [details] [diff] [review] Patch Review of attachment 711138 [details] [diff] [review]: ----------------------------------------------------------------- sure, that works for me. I added the -include via the package config file (js.pc.in), which works fine for gjs.
Attachment #711138 - Flags: review?(darkxst) → review+
Comment on attachment 711138 [details] [diff] [review] Patch >+#ifndef js_RequiredDefines_h___ >+#define js_RequiredDefines_h___ Don't use consecutive underscores in a new header. (I'm inclined to file a bug to cleanup these spec violations...)
(In reply to Masatoshi Kimura [:emk] from comment #7) > Don't use consecutive underscores in a new header. > (I'm inclined to file a bug to cleanup these spec violations...) Fair enough. It does happen to be the existing style, for better or worse. And yes, file the bug.
Attachment #711138 - Flags: review?(ted) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/38e92a7b5bf4 This is one of the patches we'll want to backport for a 17-based release, Sean. We'll also need to tell embedders to add js/RequiredDefines.h to their command lines, although the plan is for pkgconfig or whatever to spit it out as part of js-cflags somehow, so hopefully many won't need to take special steps here. The consecutive-underscores thing I left for the followup bug, at least partly because I thought I'd changed it in the patch already when I pushed, but now I look back and see I didn't change it. Shouldn't be any real harm, probably, and the followup bug can handle it easily enough.
Target Milestone: --- → mozilla21
Sean, see comment 9.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #9) > This is one of the patches we'll want to backport for a 17-based release, > Sean. We'll also need to tell embedders to add js/RequiredDefines.h to > their command lines, although the plan is for pkgconfig or whatever to spit > it out as part of js-cflags somehow, so hopefully many won't need to take > special steps here. Just for note, the pkgconfig changes are being tracked as part of Bug 812265, which is in a state of convoluted mess at the moment.
You need to log in before you can comment on or make changes to this bug.