Page MenuHomeFreeBSD

Mk/bsd.port.mk: detect powerpc64 abi
ClosedPublic

Authored by mikael.urankar_gmail.com on Oct 15 2019, 3:44 PM.

Details

Summary

We'll have a new abi on ppc64 soon (ELFv2) which is incompatible with the ELFv1 abi. We need to detect the abi on which we build stuff.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Herald added a subscriber: mat. · View Herald Transcript
tcberner accepted this revision as: kde.Oct 15 2019, 5:23 PM
tcberner added a subscriber: tcberner.

The qt-dist part looks fine to me with my kde@-hat on.

pkubaj added inline comments.Oct 16 2019, 6:03 PM
Mk/Uses/cargo.mk
77 ↗(On Diff #63296)

PPC_ABI is set only on powerpc64, so there's no need to check for powerpc64 here.

Mk/Uses/qt-dist.mk
175 ↗(On Diff #63296)

powerpc and powerpcspe will switch to LLVM at the same time as powerpc64, so there could be one check (probably based either on OSVERSION or existence of /usr/bin/clang) for all powerpc*.

Mk/bsd.gecko.mk
330–332 ↗(On Diff #63296)

This block can be removed, none of gecko ports (thunderbird, firefox, firefox-esr, cliqz) work on elfv1.

pkubaj requested changes to this revision.Oct 16 2019, 6:13 PM
This revision now requires changes to proceed.Oct 16 2019, 6:13 PM
Mk/Uses/cargo.mk
77 ↗(On Diff #63296)

Actually PPC_ABI is not being defined if platform is !powerpc64 and make will complain it's a "malformed line", so $ARCH check is required.

Other solutions could be:

.if defined(PPC_ABI) && ${PPC_ABI} == ELFv1

or (didn't check this)

.if "${PPC_ABI}" == ELFv1
This comment was removed by pkubaj.
Mk/Uses/cargo.mk
77 ↗(On Diff #63296)

Then I think the variant below:

.if defined(PPC_ABI) && ${PPC_ABI} == ELFv1

is the cleanest one (but I'm not in portmgr, so my opinion here actually doesn't matter).

pkubaj accepted this revision.Oct 21 2019, 10:11 AM

IMO ok.

portmgr, since you definitely don't care about this patch (it's opened for a month now) can you remove yourself as a blocker, it blocks people from working, thank you

bdragon added a subscriber: bdragon.Nov 3 2019, 1:02 AM

Note to anyone confused about PPC_ABI being specific to ppc64: The powerpc and powerpcspe abi is NOT changing (only the compiler), as there is no such thing as 32 bit ELFv2 ABI.

ELFv2 is shorthand for The OpenPOWER ELF V2 ABI, described in "64-Bit ELF V2 ABI Specification: Power Architecture". It has a baseline requirement of 64-bit.
ELFv1 is shorthand for The 64-bit IBM PowerPC ELF ABI, described in "64-bit PowerPC ELF Application Binary Supplement 1.9".

The 32 bit ABI was originally described in "System V Application Binary Interface: PowerPC Processor Supplement" from 1995, but the modern version covering everything 32-bit is "The Power Architecture 32-bit ABI", as described in "Power Architecture® 32-bit Application
Binary Interface Supplement 1.0 - Linux® & Embedded" -- this covers both powerpc and powerpcspe.

swills accepted this revision as: portmgr, swills.Nov 4 2019, 7:14 PM

Not a huge fan of the added != but since it's only for powerpc64 and only temporary until there's a version bump for it, I think it's OK. Approved. Please wait 1 day before committing.

This revision is now accepted and ready to land.Nov 4 2019, 7:14 PM
mat added inline comments.Nov 4 2019, 7:57 PM
Mk/bsd.port.mk
1130 ↗(On Diff #63475)

This needs to be cached, see usage of _EXPORTED_VARS.

This revision now requires review to proceed.Nov 5 2019, 1:34 PM
pkubaj requested changes to this revision.Nov 6 2019, 5:03 PM

Mk/Uses/php.mk now also needs to be changed after https://svnweb.freebsd.org/ports?view=revision&revision=516904

This revision now requires changes to proceed.Nov 6 2019, 5:03 PM
mat added inline comments.Nov 7 2019, 7:59 AM
Mk/bsd.port.mk
1129–1135 ↗(On Diff #64034)

I have never actually used _EXPORTED_VARS, but the goal is to not run the command in != if the variable is already set, so you might need to test if the variable already exists, and only add to _EXPORTED_VARS if the arch is powerpc64. See other places where the variable is used.

pkubaj accepted this revision.Nov 7 2019, 12:34 PM
This revision is now accepted and ready to land.Nov 7 2019, 12:34 PM

Hopefully I got this right.

Do I need to put space with multiple .if? it's not very consistent in the file.

This revision now requires review to proceed.Nov 7 2019, 6:24 PM

Is this ready to commit?

This revision is now accepted and ready to land.Fri, Nov 22, 8:16 PM
This revision was automatically updated to reflect the committed changes.
luporl added a subscriber: luporl.Fri, Nov 29, 6:03 PM