958234 - xptcall build broken on powerpc after bug 932178

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Comment 1

[Landry Breuil (:gaston)]

When bug 932178 moved ASFILES to moz.build in xpcom/reflect/xptcall/src/md/unix, it only did the supported archs & platforms.

Build now breaks on OpenBSD/powerpc, because the xptc{invoke,stubs}_asm_ppc_openbsd.s files contain c++-like comments, which the regular gcc compiler doesnt like, since -x assembler-with-cpp is not passed anymore.

There are actually 6 .S files (the one that were migrated) and 33 .s files for 'exotic' archs & platforms. Out of those files, it seems only the ppc/openbsd files still use c++-like comments (ie //) - they were fixed to build with -x assembler-with-cpp in bug 650749, and it that same bug comments were changed from # to // (now, i dont really remember why...)

I cant speak for other oses (solaris, aix, netbsd, rhapsody (!?)) for which files were not migrated to moz.build (as i dont know if the build fails for them), but i'd like to get the trunk build clean again on openbsd/ppc before 28 migrates to beta - for that i see two possible fixes:
- move back to c-like comments, ie /* */ for the header and # for the comments at the end of line - tested working here fine.
- rename the two files to .S, which would magically fix the issue ? (to test)

opinions ?

Comment 2

[Nathan Froyd [:froydnj]]

(In reply to Landry Breuil (:gaston) from comment #1)
> - rename the two files to .S, which would magically fix the issue ? (to test)

This one.

Comment 3

[Landry Breuil (:gaston)]

I'll let my current build with first option finish to ensure it actually runs before testing this second option, as after rereading bug 650749, it was done to fix a crash at startup... of course, i'll also prefer the renaming.

Comment 4

[Landry Breuil (:gaston)]

with the comments changed, m-c built & ran fine. Now building with .s files renamed to .S (and changed accordingly in moz.build) - the build went past the asm files fine, i'll just wait for it to finish & run it tonight

Comment 5

[Landry Breuil (:gaston)]

Attachment: Rename xptc{invoke,stubs}_asm_ppc_openbsd.s to .S

2nd option actually works, since i'm attaching the patch from it :)
Mozilla/5.0 (X11; OpenBSD macppc; rv:29.0) Gecko/20100101 Firefox/29.0

It will probably need uplift to aurora too...

Comment 6

[Nathan Froyd [:froydnj]]

Comment on attachment 8358470 [details] [diff] [review]
Rename xptc{invoke,stubs}_asm_ppc_openbsd.s to .S

Since you're the OpenBSD guy, r+ on changing the sparc{,64} cases too, while you're at it, to maybe save yourself some time in the future.

Comment 7

[Landry Breuil (:gaston)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8945b2a33ba9

sparc64 builds fine without touching it (i suppose because of the style of comments in it) so i'm not daring changing those :)

Building an aurora tree on ppc to check if it needs backporting..

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8945b2a33ba9

Comment 9

[Landry Breuil (:gaston)]

Comment on attachment 8358470 [details] [diff] [review]
Rename xptc{invoke,stubs}_asm_ppc_openbsd.s to .S

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 932178
User impact if declined: failure to build on openbsd/powerpc
Testing completed (on m-c, etc.): WFM on m-c
Risk to taking this patch (and alternatives if risky): NPOTB

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5ff8dfbb1bab

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out in https://hg.mozilla.org/releases/mozilla-aurora/rev/5ff8dfbb1bab along with the other three commits from Ryan's uplift because one of the four caused windows m5 test failures: https://tbpl.mozilla.org/php/getParsedLog.php?id=32842361&tree=Mozilla-Aurora

Comment 12

[Ryan VanderMeulen [:RyanVM]]

There's no way this was at fault for that.
https://hg.mozilla.org/releases/mozilla-aurora/rev/c417a2f38c79