Skip to content

Fix installing Poetry for Python3#1494

Closed
hongquan wants to merge 8 commits into
python-poetry:masterfrom
hongquan:fix/python3-install
Closed

Fix installing Poetry for Python3#1494
hongquan wants to merge 8 commits into
python-poetry:masterfrom
hongquan:fix/python3-install

Conversation

@hongquan
Copy link
Copy Markdown

Previously, running get-poetry.py with Python 3 didn't give you ability to invoke Poetry with Python 3. This PR is to fix that.

The PR also update installation documentation, giving more examples.

Pull Request Check List

  • Added tests for changed code.
  • Updated documentation for changed code.

Comment thread README.md Outdated
Co-Authored-By: Aliaksei Urbanski <mimworkmail@gmail.com>
Jamim
Jamim previously approved these changes Oct 26, 2019
@Jamim
Copy link
Copy Markdown
Contributor

Jamim commented Oct 30, 2019

Similar PR: #1042.

@Jamim Jamim mentioned this pull request Oct 30, 2019
Comment thread get-poetry.py


BIN = """#!/usr/bin/env python
BIN = """#!/usr/bin/env {python_exe}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I would simple replace BIN with f"#!{sys.executable}". What is the purpose of using env ? Why use project python to run poetry ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I don't know if replacing BIN with f"#!{sys.executable}" could break the behavior.

Currently, poetry install correctly install the Python packages to pre-selected virtual env. If change to f"#!{sys.executable}", I wonder if that behaviour can still be kept.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

> /home/kamal/.local/pipx/venvs/poetry/lib/python3.7/site-packages/poetry/console/commands/install.py(52)handle()
-> extras = []
(Pdb) l
 47  	        installer = Installer(
 48  	            self.io, self.env, self.poetry.package, self.poetry.locker, self.poetry.pool
 49  	        )
 50  	        import pdb;pdb.set_trace()
 51  	
 52  ->	        extras = []
 53  	        for extra in self.option("extras"):
 54  	            if " " in extra:
 55  	                extras += [e.strip() for e in extra.split(" ")]
 56  	            else:
 57  	                extras.append(extra)
(Pdb) self.env
VirtualEnv("/home/kamal/python/demo/.venv")
(Pdb) import sys
(Pdb) sys.executable
'/home/kamal/.local/pipx/venvs/poetry/bin/python'

As we can see above, the installer already running with correct venv context, independent of sys.executable.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can confirm that this small delta fixes the script:

--- get-poetry.py       2020-03-25 17:06:28.451064345 -0400
+++ get-poetry.py.new   2020-03-25 17:04:19.746420009 -0400
@@ -197,7 +197,7 @@
 POETRY_LIB_BACKUP = os.path.join(POETRY_HOME, "lib-backup")
 
 
-BIN = """#!/usr/bin/env python
+BIN = f"""#!{sys.executable}
 # -*- coding: utf-8 -*-
 import glob
 import sys

along with invoking the script with python3, on Debian Buster (10.x).

Copy link
Copy Markdown
Author

@hongquan hongquan Mar 26, 2020

Choose a reason for hiding this comment

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

If so, I opt for f"#!{sys.executable}". The only issue is that f-string can only work with Python 3.6+.

I wonder if @sdispater want to maintain compatibility with Python 3.5. This is the default Python version of Debian Stretch (9).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Right, in my parallel PR I moved back to a .format() approach, see #2222 .

Happy to close mine if this one has traction, but mine's passing all the tests right now.

Comment thread README.md
curl -sSL https://raw.githubusercontent.com/sdispater/poetry/master/get-poetry.py | python
```

or, to install Poetry with Python 3:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Probably, this should be the first option, as Python 2 is unsupported.

Comment thread README.md
@@ -27,6 +35,12 @@ If you want to install prerelease versions, you can do so by passing `--preview`
python get-poetry.py --preview
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It's absolutely not clear from the docs, where to find this get-poetry.py, because curl (or wget) doesn't save it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@NickVolynkin

wget https://raw.githubusercontent.com/python-poetry/poetry/master/get-poetry.py

(Replace curl -sSL with just wget)

will save the install script to file.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@hongquan Sure, it will. But documentation just supposes that the file is already there. It can confuse readers.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@NickVolynkin Thanks. I will update that detail.

@NickVolynkin
Copy link
Copy Markdown

This PR is just great and I'd love to see it merged. Is there anything I could help with, like testing on different systems and Python versions?

@finswimmer finswimmer requested a review from a team March 27, 2020 14:21
@finswimmer
Copy link
Copy Markdown
Member

Thanks a lot for your contribution. 👍

In the meantime #2547 was created and merged. So I will close this one here.

fin swimmer

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 1, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants