306334 - XULRunner debug doesn't link on mac

Status: VERIFIED FIXED

(Toolkit Graveyard :: XULRunner, defect, P1)

Description

[Benjamin Smedberg]

XULRunner debug doesn't link on mac because of the use of PR_Assert in
pldhash.c... replace this usage with NS_ASSERTION, which will link.

Comment 1

[Mike Shaver (:shaver emeritus)]

Is this because we're using a non-debug NSPR with a debug XPCOM?  Is that a
supported configuration?  (Or are we finding the Apple-shipped NSPR and being
confused by that?)

There are other PR_ASSERT references in the tree, like in the XML content sink,
libjar, etc: http://lxr.mozilla.org/mozilla/search?string=PR_ASSERT .  Those are
probably easy enough to fix if we decide that this is what we want to do, but
I'm not clear on why this is a problem for XULRunner but not, say, Firefox.

Finally, pldhash.c is generated from jsdhash.c, so you'll need to change
js/src/plify_jsdhash.sed and regenerate in order to do this, should it indeed be
the right thing.

Comment 2

[Benjamin Smedberg]

Attachment: s/JS_ASSERT/NS_ASSERTION/

Comment 3

[Benjamin Smedberg]

No, this is because the XPCOM glue cannot depend on NSPR.

Comment 4

[Mike Shaver (:shaver emeritus)]

Comment on attachment 194187 [details] [diff] [review]
s/JS_ASSERT/NS_ASSERTION/

OK, I guess that's fair.  plify_jsdhash is brendan's baby, though; -> him.

Comment 5

[Brendan Eich [:brendan]]

Comment on attachment 194187 [details] [diff] [review]
s/JS_ASSERT/NS_ASSERTION/

Cool -- but you might even match PR_ASSERT(0) and turn that into NS_NOTREACHED
or some such.

/be

Comment 6

[Benjamin Smedberg]

Bah, this doesn't actually work: nsDebug.h is needed, which is a C++ header.
Slightly more involved patch coming up.

Comment 7

[Benjamin Smedberg]

Attachment: Use NS_ASSERTION, and make nsDebug.h use C symbols instead of C++

Comment 8

[Brendan Eich [:brendan]]

Comment on attachment 194311 [details] [diff] [review]
Use NS_ASSERTION,  and make nsDebug.h use C symbols instead of C++

r=me, looks fine (only thought is to include XPCOM in the prefix, in addition
to or instead of the Glue or even the NSGlue in NSGlue_ -- but xpcom probably
has a consistent prefixing scheme in place; ignore me if this follows that).

/be

Comment 9

[Darin Fisher]

Comment on attachment 194311 [details] [diff] [review]
Use NS_ASSERTION,  and make nsDebug.h use C symbols instead of C++

r=darin

Comment 10

[Benjamin Smedberg]

Comment on attachment 194311 [details] [diff] [review]
Use NS_ASSERTION,  and make nsDebug.h use C symbols instead of C++

Landed on trunk, this is very low risk.

Comment 11

[Brendan Eich [:brendan]]

Comment on attachment 194311 [details] [diff] [review]
Use NS_ASSERTION,  and make nsDebug.h use C symbols instead of C++

If it's very low risk (and I agree it is), why not for b4?

/be

Comment 12

[Benjamin Smedberg]

Extra bustage fix: we didn't want NS_HIDDEN_() in the nsDebug.h declarations,
only NS_COM_GLUE (the two are incompatible).

Comment 13

[Benjamin Smedberg]

Fixed on 1.8 branch.