Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libgraphqlparser: deprecate as it requires Python 2 #83290

Closed
wants to merge 1 commit into from

Conversation

cho-m
Copy link
Member

@cho-m cho-m commented Aug 15, 2021

  • 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>?

Experimenting with pypy for some Python 2-dependent formulae.

Seemed to build locally.

@cho-m cho-m added in progress Stale bot should stay away linux to homebrew-core Migration of linuxbrew-core to homebrew-core CI-force-linux [DEPRECATED] Don't pass --skip-unbottled-linux to brew test-bot. labels Aug 15, 2021
@cho-m cho-m marked this pull request as draft August 15, 2021 03:37
Comment on lines 19 to 21
on_linux do
depends_on "pypy" => :build # Due to Python 2
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a huge fan of this (but not a very strong opinion).

I actually think we should consider deprecating formulae that still need Python2. Thoughts, @Homebrew/core?

Copy link
Member Author

@cho-m cho-m Aug 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm open to opinions here as I'm mainly experimenting with this.

In terms of Python2 formulae, the most popular ones didn't work on quick attempt, i.e.

  • node@10 - failed with generic message. It is already deprecated, but is remains popular (at least on macOS = 5.5k+ installs/mo). It is also a dependency for kibana(deprecated)/opensearch-dashboards(should update to newer node that builds with Python3).
  • bazaar - it builds, but has Python runtime dependency, with some code that is designed around CPython. It is a dependency for influxdb, which is reasonably popular (macOS = 2.4k+ installs/mo)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe @iMichka and @sjackman have a expressed a strong opposition to supporting Python 2 in any way on Linux, and I completely agree with them on this. I would support deprecating all formulae which still depend on Python 2 as well, unless the upstream team has provided a clear roadmap for when they will transition to Python 3. We are coming up on 2 years since Python 2 EOL, and I think it is unacceptable for projects to depend on an unsupported language like this.

Specifically regarding bazaar, Fedora has replaced it with breezy, a maintained forked that should be drop in compatible: https://fedoraproject.org/wiki/Changes/ReplaceBazaarWithBreezy. We could disable the bazaar formula and make bzr an alias to breezy.

For this particular formula, I think it should be deprecated and we should notify upstream that this has happened. There has a been a pull request open for 3 years to add Python 3 support: graphql/libgraphqlparser#66. It was also noted in the issue that this repo may no longer have an active maintainer: graphql/libgraphqlparser#66.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, let's not add back python 2 support. So much work has been done to get rid of it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed with deprecating these formulae.

@cho-m cho-m force-pushed the libgraphqlparser-linux branch from 73e7895 to 3344a5b Compare August 16, 2021 06:16
@cho-m cho-m changed the title libgraphqlparser: use pypy to build on Linux libgraphqlparser: deprecate as it requires Python 2 Aug 16, 2021
@BrewTestBot BrewTestBot added formula deprecated Formula deprecated macos-only Formula depends on macOS labels Aug 16, 2021
@@ -14,6 +14,8 @@ class Libgraphqlparser < Formula
sha256 cellar: :any, high_sierra: "64779ec3108d9eef789d279abfafa90437c6a76b2ed3973d45979cd1051dc170"
end

deprecate! date: "2020-04-20", because: "requires Python 2 to build"
Copy link
Member Author

@cho-m cho-m Aug 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some dates I was considering:

  • 2020-04-20 was final Python 2 (2.7.18) release date.
  • 2020-01-01 was official Python 2 EOL date.

Could also just use current date or 2021-04-20 as 1+ year after final release.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just go with today, so that it's easier for us to figure out if enough time has passed to disable! it.

@cho-m cho-m removed in progress Stale bot should stay away CI-force-linux [DEPRECATED] Don't pass --skip-unbottled-linux to brew test-bot. labels Aug 16, 2021
@cho-m cho-m marked this pull request as ready for review August 16, 2021 06:19
@cho-m cho-m added CI-syntax-only Change only affects brew syntax, not the install. Only run syntax CI. and removed linux to homebrew-core Migration of linuxbrew-core to homebrew-core labels Aug 16, 2021
@BrewTestBot
Copy link
Member

🤖 A scheduled task has triggered a merge.

@cho-m cho-m deleted the libgraphqlparser-linux branch August 23, 2021 05:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI-syntax-only Change only affects brew syntax, not the install. Only run syntax CI. formula deprecated Formula deprecated macos-only Formula depends on macOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants