Details

Reviewers

pkubaj
linimon
alfredo
swills

Group Reviewers

portmgr
kde
gecko
O5: Ports Framework	(Owns No Changed Paths)

Commits

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

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

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

mikael created this revision.Oct 15 2019, 3:44 PM

Herald added 1 blocking reviewer(s): portmgr. · View Herald TranscriptOct 15 2019, 3:44 PM

Herald added a subscriber: mat. · View Herald Transcript

LGTM

alfredo added a child revision: D21588: [PowerPC64] fix www/node build and runtime when using ELFv2 ABI.Oct 15 2019, 4:35 PM

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

mikael mentioned this in D20792: lang/rust: add powerpc64 ELFv2 support.Oct 15 2019, 5:49 PM

pkubaj added inline comments.Oct 16 2019, 6:03 PM

Mk/Uses/cargo.mk
77	PPC_ABI is set only on powerpc64, so there's no need to check for powerpc64 here.
Mk/Uses/qt-dist.mk
175	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
326–328	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

alfredo added inline comments.Oct 17 2019, 11:56 PM

Mk/Uses/cargo.mk
77	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

pkubaj added a comment.Oct 18 2019, 7:18 AM

This comment was removed by pkubaj.

Mk/Uses/cargo.mk
77	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).

address comments

IMO ok.

alfredo accepted this revision.Oct 21 2019, 1:42 PM

alfredo mentioned this in D22100: llvm-devel on powerpc64: add configure option to select ELFv2 ABI.Oct 21 2019, 10:07 PM

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

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.

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	This needs to be cached, see usage of _EXPORTED_VARS.

tobik mentioned this in D22240: lang/rust: Update to 1.39.0.Nov 5 2019, 5:23 AM

address comment

This revision now requires review to proceed.Nov 5 2019, 1:34 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

address comment

mat added inline comments.Nov 7 2019, 7:59 AM

Mk/bsd.port.mk
1129–1135	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.