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

RESOLVED FIXED

Status

()

Core
XPCOM
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: David Woodhouse, Assigned: Benjamin Smedberg)

Tracking

Trunk
PowerPC
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

12 years ago
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.
(Reporter)

Comment 1

12 years ago
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_.
(Assignee)

Updated

12 years ago
Status: UNCONFIRMED → NEW
Component: Build Config → XPCOM
Ever confirmed: true
Product: Firefox → Core
QA Contact: build.config → xpcom
Version: unspecified → Trunk
(Assignee)

Comment 2

12 years ago
Created attachment 246180 [details] [diff] [review]
$(error) on unrecognized platforms, rev. 1
Assignee: nobody → benjamin
Status: NEW → ASSIGNED
Attachment #246180 - Flags: review?(timeless)
fwiw the NS_ASSERTION wouldn't show in non-debug builds anyway.

Updated

12 years ago
Attachment #246180 - Flags: review?(timeless) → review+

Comment 4

12 years ago
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.
Attachment #246180 - Flags: review+ → review-

Comment 5

12 years ago
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
(Assignee)

Comment 6

12 years ago
Bah, I meant = (and ended up fixing it sometime after having attached this patch). FIXED
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.