361413 - Build succeeds even when XPTC stubs/invocation stuff are not implemented.

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[David Woodhouse]

User-Agent:       Mozilla/5.0 (X11; U; Linux ppc64; en-GB; rv:1.8.0.8) Gecko/20061107 Fedora/1.5.0.8-1.fc6 Firefox/1.5.0.8
Build Identifier: Mozilla/5.0 (X11; U; Linux ppc64; en-GB; rv:1.8.0.8) Gecko/20061107 Fedora/1.5.0.8-1.fc6 Firefox/1.5.0.8

When building on an unknown platform where XPTC stuff is not implemented, the build should fail. Yet it succeeded.

This (along with other packaging bugs) led to Fedora Core 6 actually shipping with a totally non-functional "firefox" on PPC64 hardware, when it should have been using the perfectly functional PPC32 version (since most of the rest of userspace on PPC64 hardware is PPC32 anyway; in particular the Java and RealPlayer plugins).

mozilla/xpcom/reflect/xptcall/src/md/unix/Makefile.in has this:
# 
# The default is this buildable, but non-functioning code.
#
CPPSRCS         := xptcinvoke_unsupported.cpp xptcstubs_unsupported.cpp


That's a very silly default. If it's going to be broken, it would be better for the build to break. Especially in 'official branding' mode.

It's especially broken in the Fedora package, since we redirect all output to /dev/null -- so even the NS_ASSERTION(0,"XPTC_InvokeByIndex called on unsupported platform") doesn't help because it's lost.

We shouldn't 

Reproducible: Always

Steps to Reproduce:
1. Attempt to build for ppc64


Actual Results:  
Build "succeeds" and generates non-functional firefox.

Expected Results:  
Build should fail.

I actually have a fix for this -- I've implemented the stubs for PPC64. I'll file that in a separate bug.

Comment 1

[David Woodhouse]

Patch to provide ppc64 support is in bug 361415. That's a separate issue though -- this is for the fact that _without_ it, the build should have _failed_.

Comment 2

[Benjamin Smedberg]

Attachment: $(error) on unrecognized platforms, rev. 1

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

fwiw the NS_ASSERTION wouldn't show in non-debug builds anyway.

Comment 4

[timeless]

Comment on attachment 246180 [details] [diff] [review]
$(error) on unrecognized platforms, rev. 1

empirical evidence says this doesn't work.

conclusion $() must be early evaluation and you want late.

Comment 5

[jag (Peter Annema)]

Try it with = instead of :=, so |CPPSRCS = $(...)|.

The := means (loosely translated) resolve variables and assign now, whereas = will resolve variables at the last moment. See http://www.gnu.org/software/make/manual/make.html#Flavors

Comment 6

[Benjamin Smedberg]

Bah, I meant = (and ended up fixing it sometime after having attached this patch). FIXED