The default bug view has changed. See this FAQ.

TARGET_XPCOM_ABI not set in solaris builds

RESOLVED FIXED

Status

()

Core
Build Config
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Leon Sha, Assigned: Leon Sha)

Tracking

({fixed1.8.0.5, fixed1.8.1})

Trunk
x86
SunOS
fixed1.8.0.5, fixed1.8.1
Points:
---
Bug Flags:
blocking1.8.0.5 -

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(4 attachments)

(Assignee)

Description

11 years ago
http://developer.mozilla.org/en/docs/TARGET_XPCOM_ABI
on Solaris this is never set correctly.
Issuing the following command in the javascript console:
Components.classes["@mozilla.org/xre/app-info;1"].getService(Components.interfaces.nsIXULRuntime).XPCOMABI
There should be some result there.
(Assignee)

Comment 1

11 years ago
Created attachment 207595 [details] [diff] [review]
Patch
Assignee: nobody → leon.sha
Status: NEW → ASSIGNED
Attachment #207595 - Flags: review?
(Assignee)

Updated

11 years ago
Attachment #207595 - Flags: review? → review?(cls)
Comment on attachment 207595 [details] [diff] [review]
Patch

We do not want to set TARGET_XPCOM_ABI unless target-compiler-abi is set.
Attachment #207595 - Flags: review?(cls) → review-
(In reply to comment #2)
> (From update of attachment 207595 [details] [diff] [review] [edit])
> We do not want to set TARGET_XPCOM_ABI unless target-compiler-abi is set.
> 

Why not? the documentation at http://developer.mozilla.org/en/docs/TARGET_XPCOM_ABI specifically says that TARGET_COMPILER_ABI should be ignored for all but the 5 listed compilers:
"For other compilers, this value is ommited, along with the preceding dash."

Also, it even says: 
"Thunderbird built with the Sun C++ Compiler for the SPARC processor would have XPCOM ABI of sparc"

This is important for extensions that include some native code since Solaris (SunOS) exists in at least two architectures: sparc and x86

Comment 4

11 years ago
I admit having written that piece of documentation (the TARGET_XPCOM_ABI page at MDC) without extensive research :)

At any rate, if we avoid setting TARGET_XPCOM_ABI on Solaris (just because target-compiler-abi isn't set), then me, as an extension developer (whose extension requires a binary component) cannot make a multiplatform XPI with Solaris inside, since I have no way to distinct x86 Solaris, x86-64 Solaris and Sparc Solaris.
The documents were based on the code, not the other way around. Every value should have an CPU-compiler combination. I have updated the doc to make it clear that there should always be both values.

I suggest using

"sparc-sunc" or something similar to identify your CPU/compiler combination.
Created attachment 207615 [details] [diff] [review]
patch to configure

K, sounds fair enough.
Here is a new proposed fix that I just verified.
Note that on Solaris 'uname -m' returns 'i86pc' for x86 and 'sun4u' for sparc, which explains my first set of changes (to set CPU_ARCH).

I tested the changes on Solaris Sparc and Solaris x86, both with Sun C and gcc, and now TARGET_XPCOM_ABI is set to either of these 4 values:
 x86-sunc
 x86-gcc3
 sparc-sunc
 sparc-gcc3

Is this satisfactory?
Comment on attachment 207615 [details] [diff] [review]
patch to configure

Yes, excellent! Please provide this as a -u8 patch to configure.in so that I can review and land it.
Created attachment 207620 [details] [diff] [review]
configure.in patch

here is the proposed fix to configure.in (genereated as diff -u
Comment on attachment 207620 [details] [diff] [review]
configure.in patch

r=me, I'll land this.
Attachment #207620 - Flags: review+
Fixed on trunk. I don't see any reason we can't get this on the 1.8 branch as well, and probably 1.8.0.2 if there aren't any regressions on trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Comment 11

11 years ago
Mike, you might want to check out with whoever at IBM builds Firefox for IBM's platforms to ensure this bug doesn't repeat there.

Comment 12

11 years ago
Any chance of it going into TB1.5 final? I'd really hate us to miss the opportunity brought up with the platform-specific directories feature by missing such a simple thing.
No, the first build this is likely to get approved for is ffox/tbird 1.5.0.2
*** Bug 334270 has been marked as a duplicate of this bug. ***
bsmedberg, were you still planning on landing this on branches, including the s/CPU_ARCH_TEST/OS_TEST/ you caught on checkin?
Comment on attachment 207620 [details] [diff] [review]
configure.in patch

sun handicap concerning extension team really need that. thanks a lot
Attachment #207620 - Flags: approval1.8.0.3?
Attachment #207620 - Flags: approval-branch-1.8.1?(shaver)

Comment 17

11 years ago
This is most definitly not 1.8.0.3 material, but you may want to try
1.8.0.4.
Comment on attachment 207620 [details] [diff] [review]
configure.in patch

moved to 1.8.0.4 addressed by axel
Attachment #207620 - Flags: approval1.8.0.3? → approval1.8.0.4?
Comment on attachment 207620 [details] [diff] [review]
configure.in patch

Needs 1.8.1 approval and landing first.
Attachment #207620 - Flags: approval-branch-1.8.1?(shaver) → approval-branch-1.8.1?(benjamin)
Attachment #207620 - Flags: approval-branch-1.8.1?(benjamin) → approval-branch-1.8.1+
Benjamin,

pls. help check the patch in 1.8.1
Comment on attachment 207620 [details] [diff] [review]
configure.in patch

Too late in this cycle, please try early for next release.
Attachment #207620 - Flags: approval1.8.0.5?
Attachment #207620 - Flags: approval1.8.0.4?
Attachment #207620 - Flags: approval1.8.0.4-
Fixed on 1.8 branch.
Keywords: fixed1.8.1
Benjamin,

how about 1.8.0.5?
Any chance we could get this (i.e., what was checked in, which is a little different from the patch in the bug, IIRC) in for 1.8.0.5, so that we could ship extensions with binary components for platforms like Linux x86_64?
Flags: blocking1.8.0.5?
dbaron, are you thinking of bug 335143?
No, I'm thinking of the change made when this patch was checked in to the trunk and 1.8 branch that's not part of the patch above:

 case "$OS_TEST" in
 [...]
 x86_64 | sparc | ppc | ia64)
-    CPU_ARCH="$CPU_ARCH_TEST"
+    CPU_ARCH="$OS_TEST"
     ;;
 esac

which actually caused $CPU_ARCH to be set on those 4 platforms, which allowed TARGET_XPCOM_ABI to be non-empty in the build system.

Maybe that patch is needed too, though.
Flags: blocking1.8.0.5? → blocking1.8.0.5+
Sounds like we need a new patch for the 1.8.0 branch?
Comment on attachment 207620 [details] [diff] [review]
configure.in patch

minusing for 1.8.0.x, it sounds like we need a new patch for the branches
Attachment #207620 - Flags: approval1.8.0.5? → approval1.8.0.5-
This isn't really a blocking bug. Would approve a branch patch.
Flags: blocking1.8.0.5+ → blocking1.8.0.5-
Created attachment 225422 [details] [diff] [review]
180 branch patch
Attachment #225422 - Flags: approval1.8.0.5?
Comment on attachment 225422 [details] [diff] [review]
180 branch patch

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #225422 - Flags: approval1.8.0.5? → approval1.8.0.5+
Fixed on MOZILLA_1_8_0_BRANCH
Keywords: fixed1.8.0.5
Note that you didn't commit to configure as well, and it's not auto-updated on MOZILLA_1_8_0_BRANCH.  I committed (yesterday) an updated configure (and checked that autoconf-2.13 on my machine matches what is auto-generated on the trunk and the 1.8 branch, and that the diffs to configure were only expected diffs).
You need to log in before you can comment on or make changes to this bug.