Skip to content

Add alternative versions for PyPy3#96305

Closed
tonybaloney wants to merge 6 commits into
Homebrew:masterfrom
tonybaloney:pypy3-updates
Closed

Add alternative versions for PyPy3#96305
tonybaloney wants to merge 6 commits into
Homebrew:masterfrom
tonybaloney:pypy3-updates

Conversation

@tonybaloney

@tonybaloney tonybaloney commented Mar 5, 2022

Copy link
Copy Markdown

The PyPy3 formula is PyPy3.7.

There are major versions 3.8 and 3.9. These are not compatible with 3.7, so they should be optional versions (just like Python 3.7, 3.8 etc)

This PR adds 3.8 and 3.9

  • Have you followed the guidelines for contributing?
  • Have you ensured that your commits follow the commit style guide?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

@BrewTestBot BrewTestBot added automerge-skip `brew pr-automerge` will skip this pull request legacy Relates to a versioned @ formula new formula PR adds a new formula to Homebrew/homebrew-core no ARM bottle Formula has no ARM bottle labels Mar 5, 2022
@henryiii

henryiii commented Mar 5, 2022

Copy link
Copy Markdown
Contributor

Please see #88123 (comment) . The @ is not supposed to be used for different flavors of a formula, but only for "old" versions. If there was a PyPy 8 release, then there could be a pypy3.9@7, for example.

@tonybaloney

Copy link
Copy Markdown
Author

@henryiii thanks. PyPy's versions is a bit confusing

PyPy refers to the Python 2.7 compatible interpreter
PyPy3 refers to the Python 3+ compatible interpreter.

Within PyPy3 there are then releases to match compatibility with the equivalent Python versions

  • 3.7
  • 3.8
  • 3.9

They also run an internal versioning scheme, which is currently v7.3.8. https://doc.pypy.org/en/latest/release-v7.3.8.html

So for the v7.3.8 release there are builds for Python 3.7, 3.8 and 3.9

pypy3.9-v7.3.8-src.tar.bz2
pypy3.8-v7.3.8-src.tar.bz2
pypy3.7-v7.3.8-src.tar.bz2

I don't think general users would know or care whether they installed v7. or v8 of PyPy, since they should be compatible within the version of Python they're targeting.

So, I would expect
brew install python@3.9
To be equivalent to
brew install pypy@3.9

That in mind, and after reading the linked issue. I think the following makes sense.

  • pypy@2.7 - the current pypy recipe
  • pypy@3.7 - the current pypy3
  • pypy@3.8 - added pypy3 recipe (in this PR but I'll rename)
  • pypy as the pypy3@3.9 recipe
  • pypy3 -> pypy as an alias (is this just a symbolic link?)

Please let me know what you think

@carlocab

carlocab commented Mar 6, 2022

Copy link
Copy Markdown
Member

I don't think general users would know or care whether they installed v7. or v8 of PyPy, since they should be compatible within the version of Python they're targeting.

I disagree. If users don't care, why should we ever update the existing pypy and pypy3 formulae?

In any case, even if users don't care, that's not a reason to misuse the @-scheme for identifying versions of the formula.

@tonybaloney

Copy link
Copy Markdown
Author

I don't think general users would know or care whether they installed v7. or v8 of PyPy, since they should be compatible within the version of Python they're targeting.

I disagree. If users don't care, why should we ever update the existing pypy and pypy3 formulae?

In any case, even if users don't care, that's not a reason to misuse the @-scheme for identifying versions of the formula.

Ok. That makes sense.

I'm not clear on the alternative solution though.

Pypy currently installs 2.7-7.3.8 and pypy3 installs 3.7-7.3.8

The latest version of pypy is 3.9-7.3.8

Anyone wanting to run Python >3.7 on pypy can't do so using either recipe today.

@henryiii

henryiii commented Mar 6, 2022

Copy link
Copy Markdown
Contributor

I think the alternate was just to label the formula "pypy3.8", and if there's ever a need for an "old" version, it would be "pypy3.8@7", etc. Though I think the last PR got stuck because the author didn't know how to add a dot to a formula name. Possibly "DOT", like "AT"? Not sure either, though.

@henryiii

henryiii commented Mar 6, 2022

Copy link
Copy Markdown
Contributor

The latest version of PyPy is 7.3.8, and it comes in four flavors, 2.7, 3.7, 3.8, and 3.9 (beta). There will be a 7.3.9 in about a week, FYI, due to an ABI mistake with 3.7 I think. Or maybe 3.8, or both, I've forgotten which.

@carlocab

carlocab commented Mar 6, 2022

Copy link
Copy Markdown
Member

A pypy3.8 formula should just be named pypy3.8.rb with the class declaration

class Pypy38 < Formula

@tonybaloney

Copy link
Copy Markdown
Author

A pypy3.8 formula should just be named pypy3.8.rb with the class declaration

class Pypy38 < Formula

Got it. I'll put that together then. Is there a way of sharing code between recipes since they're pretty much identical except for the URL, version and SHA

@gromgit

gromgit commented Mar 6, 2022

Copy link
Copy Markdown
Contributor

Is there a way of sharing code between recipes since they're pretty much identical except for the URL, version and SHA

Not that I know of, and it's far better that each formula stand alone. Otherwise, when a specific version (probably the latest) requires changes to its install block, you'd have to break up the formulae anyway.

@carlocab

carlocab commented Mar 6, 2022

Copy link
Copy Markdown
Member
class Pypy38 < Pypy3

might work. That said, things like the livecheck and bottle blocks should be different. And we probably also actually want slightly different install methods so that these formulae don't conflict with each other. This will probably also require slightly different test blocks and post_install methods as a consequence.

@gromgit

gromgit commented Mar 6, 2022

Copy link
Copy Markdown
Contributor
class Pypy38 < Pypy3

Does this trigger a rebuild of pypy38 if pypy3 changes blocks that are not overridden in the child formula? If not, that would be a good reason not to inherit formula definitions this way.

@carlocab

carlocab commented Mar 6, 2022

Copy link
Copy Markdown
Member
class Pypy38 < Pypy3

Does this trigger a rebuild of pypy38 if pypy3 changes blocks that are not overridden in the child formula? If not, that would be a good reason not to inherit formula definitions this way.

Actually, it probably does something worse: pypy38 will inherit pypy3's revisions without corresponding rebuilds.

@BrewTestBot BrewTestBot removed the legacy Relates to a versioned @ formula label Mar 7, 2022
@tonybaloney

Copy link
Copy Markdown
Author

Please can you review this PR again? I've made adjustments based on the discussion naming plan

@tonybaloney

Copy link
Copy Markdown
Author

please hold, seeing there are conflicts if you try and install all 3

@tonybaloney

Copy link
Copy Markdown
Author

Alright, can install all 3 recipes side-by-side. The 3.9 branch has different build artefacts so it was a bit confusing.

Please review

@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions Bot added the stale No recent activity label Mar 31, 2022
@github-actions github-actions Bot closed this Apr 7, 2022
@tonybaloney

Copy link
Copy Markdown
Author

This isn't stale

@henryiii

henryiii commented Apr 7, 2022

Copy link
Copy Markdown
Contributor

Agreed. :) Though the PyPy version has likely bumped over the last week. @carlocab perhaps?

@tonybaloney

Copy link
Copy Markdown
Author

Can the bot un-close the PR?

@github-actions github-actions Bot added the outdated PR was locked due to age label May 11, 2022
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators May 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

automerge-skip `brew pr-automerge` will skip this pull request new formula PR adds a new formula to Homebrew/homebrew-core no ARM bottle Formula has no ARM bottle outdated PR was locked due to age stale No recent activity

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants