Page MenuHomeFreeBSD

lang/pypy and lang/pypy3: Fix rvmprof build on FreeBSD
ClosedPublic

Authored by farrokhi on Jul 21 2018, 4:44 AM.

Details

Summary
lang/pypy and lang/pypy3: Fix rvmprof build on FreeBSD

Originally pypy did not build on FreeBSD due to build failure in rvmprof. In order to build it on FreeBSD (and OpenBSD) you have to disable rvmprof during build process. This is what a previous patch [1][2] did to enable building on FreeBSD. 

This is a patch obtained from upstream [3] that actually solves the rvmprof build problem which enables building pypy/pypy3 on FreeBSD with rvmprof enabled.

[1] https://svnweb.freebsd.org/changeset/ports/473994
[2] https://reviews.freebsd.org/D16138
[3] https://bitbucket.org/pypy/pypy/issues/2853/build-fails-on-freebsd-11x-x64#comment-46813575

Reviewed_by: koobs, ???
Approved by: ???
Differential_Revision: https://reviews.freebsd.org/D16378
Test Plan
  • portlint: ???
  • testport: ??? (poudriere: <versions>, <archs> tested)

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

farrokhi created this revision.Jul 21 2018, 4:44 AM
farrokhi edited the summary of this revision. (Show Details)
farrokhi added a project: Python.
koobs edited the summary of this revision. (Show Details)Jul 22 2018, 10:58 AM
koobs edited the summary of this revision. (Show Details)
koobs added a subscriber: koobs.

Does the change pass QA (poudriere) on supported/expected FreeBSD versions/archs without regression?

lang/pypy/files/patch-rpython_rlib_rvmprof_cintf.py
8 ↗(On Diff #45644)

Is 12 not supported, or is there another reason for that omission?

lang/pypy3/files/patch-rpython_rlib_rvmprof_cintf.py
8 ↗(On Diff #45644)

Is 12 not supported, or is there another reason for that omission?

koobs edited the test plan for this revision. (Show Details)Jul 22 2018, 11:00 AM
farrokhi added inline comments.Jul 23 2018, 5:48 AM
lang/pypy3/files/patch-rpython_rlib_rvmprof_cintf.py
8 ↗(On Diff #45644)

The original patch did not include 12. I am trying a new patch in poudriere against 10,11 and 12 and will also submit to upstream for review. I will update this patch accordingly.

farrokhi updated this revision to Diff 45771.EditedJul 24 2018, 1:23 PM
farrokhi marked 3 inline comments as done.
Update patches that enables bulid on FreeBSD 10+

- There is no need to disable vmprof build on FreeBSD > 10
- Local patches are also committed [1][2] in upstream and will not be needed in future releases
- Build test passed on latest 10-stable, 11-stable and 12-current (amd64)

[1] https://bitbucket.org/pypy/pypy/commits/587bebd53960
[2] https://bitbucket.org/pypy/pypy/commits/24a43b6a4d733b840b7ada958fbb3b129dc28103
farrokhi updated this revision to Diff 45772.Jul 24 2018, 1:26 PM

Remove another unrelated patch that was accidentally slipped in

dbn accepted this revision.Jul 26 2018, 3:50 PM
dbn added a subscriber: dbn.

The comments I mention are just suggestions. Happy for you to commit this patch without those changes.

lang/pypy/Makefile
108 ↗(On Diff #45772)

Since versions of FreeBSD < 10 are not supported, I would just drop this condition.

lang/pypy/bsd.pypy.mk
22 ↗(On Diff #45772)

Since PORTREVISION and DISTVERSION are the same for both instances of pypy maybe move them outside the block (or maybe just DISTVERSION?

This revision is now accepted and ready to land.Jul 26 2018, 3:50 PM
farrokhi updated this revision to Diff 45913.Jul 27 2018, 4:20 PM
farrokhi marked an inline comment as done.

Remove unnecessary version check for FreeBSD < 10

This revision now requires review to proceed.Jul 27 2018, 4:20 PM
farrokhi added inline comments.Jul 27 2018, 4:27 PM
lang/pypy/bsd.pypy.mk
22 ↗(On Diff #45772)

True. However since they always release two different tarballs for pypy and pypy3, there is always a chance that they have different versions. And what if we need bump PORTREVISION for one package, but not for another?
Maybe I am being too conservative here, but I suggest we keep this intact.

miwi accepted this revision.Jul 27 2018, 5:14 PM

LGTM

This revision is now accepted and ready to land.Jul 27 2018, 5:14 PM
dbn added a comment.Jul 28 2018, 5:16 AM

FYI, commit messages require the full path to this review (your draft message [I assume] only includes the review number)

In D16378#349810, @dbn wrote:

FYI, commit messages require the full path to this review (your draft message [I assume] only includes the review number)

Thanks for noting. This is actually how it looks in my commit message:

Differential Revision:  https://reviews.freebsd.org/D16378
farrokhi edited the summary of this revision. (Show Details)Jul 28 2018, 5:25 AM
This revision was automatically updated to reflect the committed changes.