Closed Bug 322450 Opened 19 years ago Closed 19 years ago

TARGET_XPCOM_ABI not set in solaris builds


(Firefox Build System :: General, defect)

Not set


(Not tracked)



(Reporter: leon.sha, Assigned: leon.sha)




(Keywords: fixed1.8.0.5, fixed1.8.1)


(4 files)
on Solaris this is never set correctly.
Issuing the following command in the javascript console:
There should be some result there.
Attached patch PatchSplinter Review
Assignee: nobody → leon.sha
Attachment #207595 - Flags: review?
Attachment #207595 - Flags: review? → review?(cls)
Comment on attachment 207595 [details] [diff] [review]

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 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
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.
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:

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

Yes, excellent! Please provide this as a -u8 patch to so that I can review and land it.
here is the proposed fix to (genereated as diff -u
Comment on attachment 207620 [details] [diff] [review] 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 if there aren't any regressions on trunk.
Closed: 19 years ago
Resolution: --- → FIXED
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.
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
*** 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] 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)
This is most definitly not material, but you may want to try
Comment on attachment 207620 [details] [diff] [review] patch

moved to addressed by axel
Attachment #207620 - Flags: approval1.8.0.3? → approval1.8.0.4?
Comment on attachment 207620 [details] [diff] [review] 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+

pls. help check the patch in 1.8.1
Comment on attachment 207620 [details] [diff] [review] 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

how about
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, 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)

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] 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-
Attached patch 180 branch patchSplinter Review
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).
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.