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

poetry 0.12.16 (new formula) #41055

Closed
wants to merge 1 commit into from
Closed

poetry 0.12.16 (new formula) #41055

wants to merge 1 commit into from

Conversation

zengxs
Copy link
Contributor

@zengxs zengxs commented Jun 18, 2019

poetry is a python package management tool.


  • Have you followed the guidelines for contributing?
  • 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 <formula>)?

@fxcoudert fxcoudert added the new formula PR adds a new formula to Homebrew/homebrew-core label Jun 18, 2019
@nullus
Copy link

nullus commented Jun 19, 2019

I've got a slightly different take on a formula for Poetry, that replicates the offical install method a bit more closely, but installs globally (rather than in $HOME). This means it should work against pyenv/system Python.

Am I right in assuming that this approach runs against Homebrew installed Python 3 only?

I haven't got a branch/PR up yet, but I would be able to with a few hours. I initially intended to contact the author of Poetry to find out whether this method would be accepted/endorsed.

What's the right approach here?

@zengxs
Copy link
Contributor Author

zengxs commented Jun 19, 2019

@nullus Hello, Do you use the official installer get-poetry.py to install it?
I'm not sure which way is better, I think we can contact the author of poetry to find a final solution.

@SMillerDev
Copy link
Member

@nullus Hello, Do you use the official installer get-poetry.py to install it?
I'm not sure which way is better, I think we can contact the author of poetry to find a final solution.

If that downloads all the files you resourced without checking hashes then your way is better.

@nullus
Copy link

nullus commented Jun 19, 2019

Created PR at #41127 for review.

@nullus Hello, Do you use the official installer get-poetry.py to install it?
I'm not sure which way is better, I think we can contact the author of poetry to find a final solution.

The formula I created is based on the workings of get-poetry.py, but it installs in a system path. This is not officially recommended.

The drawbacks with this method are:

  1. It requires analysis of get-poetry.py on updates, to replicate changes in process;
  2. The flexibility to operate with different Python versions means system Python (2.7, EoL Jan 2020) will be used by default; and
  3. No __pycache__/.pyc created for users other than admin/installer. I use multiple accounts when sharing systems for personal use/work.

@zengxs I agree completely, the ideal solution is one Poetry's creator (@sdispater) would be happy to promote.

@SMillerDev
Copy link
Member

@zengxs I agree completely, the ideal solution is one Poetry's creator (@sdispater) would be happy to promote.

The ideal solution would be something that would be the best fit for Homebrew. It'd be nice to have a blessing from upstream but it's not a requirement and probably not the person who's going to maintain it either.

@zengxs
Copy link
Contributor Author

zengxs commented Jun 19, 2019

The third way, maybe we can install poetry from official bundle.

But I am not sure it fit for Homebrew. @nullus @SMillerDev

A simple formula for that:

class Poetry < Formula
  desc "Python dependency management and packaging made easy."
  homepage "https://poetry.eustace.io"
  url "https://github.com/sdispater/poetry/releases/download/0.12.16/poetry-0.12.16-darwin.tar.gz"
  sha256 "63f30bc5ef15333f4c638dd82e036fa2425f48b6e3ba0fec515b8f77d63b86c6"

  def install
    ENV.prepend_create_path "PYTHONPATH", libexec/"lib/python/site-packages"
    (libexec/"lib/python/site-packages/poetry").install Dir["*"]

    (libexec/"bin/poetry").write <<~EOS
      #!/bin/bash
      python -m poetry "$@"
    EOS
    chmod 0755, libexec/"bin/poetry"

    bin.install libexec/"bin/poetry"

    bin.env_script_all_files(libexec/"bin", :PYTHONPATH => ENV["PYTHONPATH"])
  end

  test do
    ENV["LC_ALL"] = "en_US.UTF-8"
    assert_match "Created package", shell_output("#{bin}/poetry new homebrew")
    cd testpath/"homebrew" do
      system "#{bin}/poetry", "config", "settings.virtualenvs.in-project", "true"
      system "#{bin}/poetry", "add", "bcrypt"
    end

    assert_predicate testpath/"homebrew/pyproject.toml", :exist?
    assert_predicate testpath/"homebrew/poetry.lock", :exist?
    assert_match "bcrypt", (testpath/"homebrew/pyproject.toml").read
  end
end

@nullus
Copy link

nullus commented Jun 19, 2019

The third way, maybe we can install poetry from official bundle.

That's how my Formula works, see #41127. Except that I'm also sourcing bin/poetry from get-poetry.py.

@SMillerDev
Copy link
Member

poetry-0.12.16-darwin.tar.gz

Anything that's OS specific binaries is not a good option for homebrew-core. See https://docs.brew.sh/Acceptable-Formulae#we-dont-like-binary-formulae

@nullus
Copy link

nullus commented Jun 20, 2019

Fair point, I considered a cask submission but also misread the FAQ section:

The app is both open-source and CLI-only (i.e. it only uses the binary artifact). In that case, and in the spirit of deduplication, submit it first to Homebrew/core as a formula that builds from source.

Hence the "binary" approach.

I'm looking at the build process for Poetry now. Building the release tarball requires multiple versions of Python: 2.7, 3.4, 3.5, 3.6, and 3.7. I haven't found documentation for the process, but I've got it building locally with Pyenv. I'm assuming I'd have to leverage Pyenv to build, as Homebrew doesn't provide multiple Python 3.x. Is that the case? Any idea if this has been done for any existing Formulas?

@SMillerDev
Copy link
Member

It shouldn't use pyenv as it should be made for the homebrew python versions. I'd say it should do the same as the release tarballs but only for the latest python (and latest python 2 if needed).

@nullus
Copy link

nullus commented Jun 20, 2019

I'm only suggesting pyenv as build dependency, not for install.

To build an equivalent tarball from source, the current version requires the set (as above) of Python versions at build time. It vendorises the dependencies under /py<major><minor>/ paths, which are resolved at runtime to support commonly used Python versions.

The build script could be modified to make them optional, but then you'd lose the ability to operate with arbitrary pyenv versions, which sort of defeats the purpose.

The optimal user experience with this tool is realised by installing via binary bundle (to $HOME), or something closely resembling it. Would it make sense to take it to homebrew-cask for that reason?

@wasdee
Copy link
Contributor

wasdee commented Jul 20, 2019

From

I've got a slightly different take on a formula for Poetry, that replicates the offical install method a bit more closely, but installs globally (rather than in $HOME). This means it should work against pyenv/system Python.

Am I right in assuming that this approach runs against Homebrew installed Python 3 only?

I haven't got a branch/PR up yet, but I would be able to with a few hours. I initially intended to contact the author of Poetry to find out whether this method would be accepted/endorsed.

What's the right approach here?

I don't quite understand that

we should install poetry install globally because it will work against pyenv/system Python.

How could it happen exactly?

@nullus
Copy link

nullus commented Aug 28, 2019

we should install poetry install globally because it will work against pyenv/system Python.

How could it happen exactly?

I posted earlier that:

[Poetry] vendorises the dependencies under /py/ paths, which are resolved at runtime to support commonly used Python versions.

The Poetry build requires all supported Python versions (2.7, 3.5, 3.6, 3.7) in order to do that. So it needs more than just Homebrew Python installed. Also, the Poetry executable itself will use python in the path, so defaults to the system python.

However, given Apple is deprecating Python from the base system in Catalina, and Python 2.7 is officially EOL in 2020, it makes sense that this formula should be used to support Poetry in Homebrew Python 3.

I'll look at building a separate cask for the binary distribution because it can compliment this formula, for anyone who uses pyenv extensively.

@zengxs
Copy link
Contributor Author

zengxs commented Aug 28, 2019

@nullus You can see https://github.com/zengxs/homebrew-tap/blob/master/Formula/poetry.rb

Change line 15 to python -m poetry $@

This formula should support pyenv (and must be used with pyenv) and all Python version (2.7, 3.4, 3.5, 3.6, 3.7), but I have not tested it.
Hope this will help you.

@oliverandrich
Copy link
Contributor

Does this formula solve the problem with pyenv documented in this issue? python-poetry/poetry#571

Otherwise it would be a good idea to wait for poetry 1.0 or switch over to the way via "pip install" like it is used for flake8 and so on. Then poetry would be encapsulated into its own virtualenv.

@wasdee
Copy link
Contributor

wasdee commented Oct 18, 2019

Since python-poetry/poetry#794 8 days ago, The installation allows the POETRY_HOME environment variable to be passed during installation to change the default installation directory of ~/.poetry.

@chenrui333
Copy link
Member

Any update on the PR?

@SMillerDev
Copy link
Member

27 days ago, this is stale. If anyone wants to continue the work, please make a new PR

@SMillerDev SMillerDev closed this Nov 14, 2019
@lock lock bot added the outdated PR was locked due to age label Jan 2, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Jan 2, 2020
@chenrui333
Copy link
Member

Just FYI, I have merged the poetry 1.0.0 PR, thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
new formula PR adds a new formula to Homebrew/homebrew-core outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants