Last Comment Bug 322450 - TARGET_XPCOM_ABI not set in solaris builds
: TARGET_XPCOM_ABI not set in solaris builds
Status: RESOLVED FIXED
: fixed1.8.0.5, fixed1.8.1
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: x86 SunOS
: -- normal (vote)
: ---
Assigned To: Leon Sha
:
Mentors:
http://developer.mozilla.org/en/docs/...
: 334270 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-01-05 01:10 PST by Leon Sha
Modified: 2006-06-17 10:39 PDT (History)
7 users (show)
dveditz: blocking1.8.0.5-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (2.85 KB, patch)
2006-01-05 01:26 PST, Leon Sha
benjamin: review-
Details | Diff | Splinter Review
patch to configure (1.03 KB, patch)
2006-01-05 07:22 PST, Jean-Christophe Collet
no flags Details | Diff | Splinter Review
configure.in patch (807 bytes, patch)
2006-01-05 07:46 PST, Jean-Christophe Collet
benjamin: review+
benjamin: approval‑branch‑1.8.1+
dveditz: approval1.8.0.4-
dveditz: approval1.8.0.5-
Details | Diff | Splinter Review
180 branch patch (1.60 KB, patch)
2006-06-13 07:11 PDT, Benjamin Smedberg [:bsmedberg]
dveditz: approval1.8.0.5+
Details | Diff | Splinter Review

Description Leon Sha 2006-01-05 01:10:18 PST
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.
Comment 1 Leon Sha 2006-01-05 01:26:29 PST
Created attachment 207595 [details] [diff] [review]
Patch
Comment 2 Benjamin Smedberg [:bsmedberg] 2006-01-05 05:22:46 PST
Comment on attachment 207595 [details] [diff] [review]
Patch

We do not want to set TARGET_XPCOM_ABI unless target-compiler-abi is set.
Comment 3 Jean-Christophe Collet 2006-01-05 05:54:02 PST
(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 Ilya Konstantinov 2006-01-05 06:20:41 PST
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.
Comment 5 Benjamin Smedberg [:bsmedberg] 2006-01-05 06:24:01 PST
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.
Comment 6 Jean-Christophe Collet 2006-01-05 07:22:27 PST
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 7 Benjamin Smedberg [:bsmedberg] 2006-01-05 07:27:12 PST
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.
Comment 8 Jean-Christophe Collet 2006-01-05 07:46:42 PST
Created attachment 207620 [details] [diff] [review]
configure.in patch

here is the proposed fix to configure.in (genereated as diff -u
Comment 9 Benjamin Smedberg [:bsmedberg] 2006-01-05 08:02:53 PST
Comment on attachment 207620 [details] [diff] [review]
configure.in patch

r=me, I'll land this.
Comment 10 Benjamin Smedberg [:bsmedberg] 2006-01-05 08:10:34 PST
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.
Comment 11 Ilya Konstantinov 2006-01-07 18:55:53 PST
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 Ilya Konstantinov 2006-01-07 19:23:49 PST
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.
Comment 13 Benjamin Smedberg [:bsmedberg] 2006-01-08 06:25:03 PST
No, the first build this is likely to get approved for is ffox/tbird 1.5.0.2
Comment 14 Phil Ringnalda (:philor) 2006-04-16 20:49:12 PDT
*** Bug 334270 has been marked as a duplicate of this bug. ***
Comment 15 Phil Ringnalda (:philor) 2006-04-16 20:53:09 PDT
bsmedberg, were you still planning on landing this on branches, including the s/CPU_ARCH_TEST/OS_TEST/ you caught on checkin?
Comment 16 Nian Liu(n/a in a long time) 2006-04-26 01:38:50 PDT
Comment on attachment 207620 [details] [diff] [review]
configure.in patch

sun handicap concerning extension team really need that. thanks a lot
Comment 17 Axel Hecht 2006-04-26 01:59:53 PDT
This is most definitly not 1.8.0.3 material, but you may want to try
1.8.0.4.
Comment 18 Nian Liu(n/a in a long time) 2006-04-26 02:02:52 PDT
Comment on attachment 207620 [details] [diff] [review]
configure.in patch

moved to 1.8.0.4 addressed by axel
Comment 19 Daniel Veditz [:dveditz] 2006-04-26 12:10:20 PDT
Comment on attachment 207620 [details] [diff] [review]
configure.in patch

Needs 1.8.1 approval and landing first.
Comment 20 Nian Liu(n/a in a long time) 2006-04-27 20:26:14 PDT
Benjamin,

pls. help check the patch in 1.8.1
Comment 21 Daniel Veditz [:dveditz] 2006-04-28 11:45:57 PDT
Comment on attachment 207620 [details] [diff] [review]
configure.in patch

Too late in this cycle, please try early for next release.
Comment 22 Benjamin Smedberg [:bsmedberg] 2006-05-02 10:35:54 PDT
Fixed on 1.8 branch.
Comment 23 Nian Liu(n/a in a long time) 2006-05-21 20:13:50 PDT
Benjamin,

how about 1.8.0.5?
Comment 24 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2006-06-08 17:39:50 PDT
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?
Comment 25 Benjamin Smedberg [:bsmedberg] 2006-06-09 04:15:38 PDT
dbaron, are you thinking of bug 335143?
Comment 26 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2006-06-09 09:36:22 PDT
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.
Comment 27 Daniel Veditz [:dveditz] 2006-06-09 15:02:28 PDT
Sounds like we need a new patch for the 1.8.0 branch?
Comment 28 Daniel Veditz [:dveditz] 2006-06-12 11:35:11 PDT
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
Comment 29 Daniel Veditz [:dveditz] 2006-06-12 11:39:25 PDT
This isn't really a blocking bug. Would approve a branch patch.
Comment 30 Benjamin Smedberg [:bsmedberg] 2006-06-13 07:11:19 PDT
Created attachment 225422 [details] [diff] [review]
180 branch patch
Comment 31 Daniel Veditz [:dveditz] 2006-06-14 14:59:18 PDT
Comment on attachment 225422 [details] [diff] [review]
180 branch patch

approved for 1.8.0 branch, a=dveditz for drivers
Comment 32 Benjamin Smedberg [:bsmedberg] 2006-06-15 06:32:38 PDT
Fixed on MOZILLA_1_8_0_BRANCH
Comment 33 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2006-06-17 10:39:55 PDT
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).

Note You need to log in before you can comment on or make changes to this bug.