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 add git+https:// will fail with CalledProcessError if package name includes illegal file name characters #3392

Closed
3 tasks done
tanj opened this issue Nov 20, 2020 · 11 comments · Fixed by #5428
Closed
3 tasks done
Labels
kind/bug Something isn't working as expected

Comments

@tanj
Copy link

tanj commented Nov 20, 2020

  • I am on the latest Poetry version.
  • I have searched the issues of this repo and believe that this is not a duplicate.
  • If an exception occurs when executing a command, I executed it again in debug mode (-vvv option).
  • Microsoft Windows 10 Pro: Version 10.0.18363 Build 18363:
  • Poetry version: 1.1.4:
  • Link of a Gist with the contents of your pyproject.toml file:

Issue

I was starting a new project and adding some dependencies and came across a bug in the git clone process.

 % poetry add git+https://github.com/tanj/swissarmy.git

Updating dependencies
Resolving dependencies...

Writing lock file

Package operations: 2 installs, 0 updates, 0 removals

  • Installing python-dateutil (2.8.1)
  • Installing swiss army: untility package (0.1 7e67114)

  CalledProcessError

  Command '['git', 'clone', '--recurse-submodules', 'https://github.com/tanj/swissarmy.git', 'C:\\Users\\REDACTED\\AppData\\Local\\pypoetry\\Cache\\virtualenvs\\gkeep-organizer-mzsur4FB-py3.8\\src\\swiss army: untility package']' returned non-zero exit status 128.

  at ~\.poetry\lib\poetry\utils\_compat.py:217 in run
      213│                 process.wait()
      214│                 raise
      215│             retcode = process.poll()
      216│             if check and retcode:
    → 217│                 raise CalledProcessError(
      218│                     retcode, process.args, output=stdout, stderr=stderr
      219│                 )
      220│         finally:
      221│             # None because our context manager __exit__ does not use them.


Failed to add packages, reverting the pyproject.toml file to its original content.

The issue is that my swissarmy package has a : in the package name and that is an illegal character for a windows file path.

The offending code is located at

src_dir = self._env.path / "src" / package.name

I realize that naming my package like that probably is breaking some PEP, but python itself has no issue installing and using the package...

pip freeze output for that package
-e git+https://github.com/tanj/swissarmy.git@7e671142565cb2b2aa04292ec0775c024bf47547#egg=Swiss_Army_Untility_Package

@tanj tanj added kind/bug Something isn't working as expected status/triage This issue needs to be triaged labels Nov 20, 2020
@sinoroc
Copy link

sinoroc commented Nov 20, 2020

My first impression, is that I do not think it is fair to classify this as a bug in poetry. But...

One would need to check exactly what the standard (if any) says for such a case. Maybe indeed poetry is supposed to apply some name normalization somewhere and it does not currently...

@tanj Have you managed to identify the corresponding text of a standard for project names? What is allowed and what is not? Should poetry do some name normalization or should it reject such a project from installation?

python itself has no issue installing

@tanj You mean pip? Or do you mean python setup.py install?

@sinoroc
Copy link

sinoroc commented Nov 20, 2020

The best I could find in a short time is:

The name of the distribution. The name field is the primary identifier for a distribution. A valid name consists only of ASCII letters and numbers, period, underscore and hyphen. It must start and end with a letter or number. Distribution names are limited to those which match the following regex (run with re.IGNORECASE):

^([A-Z0-9]|[A-Z0-9][A-Z0-9._-]*[A-Z0-9])$

-- https://packaging.python.org/specifications/core-metadata/#name

See also: https://www.python.org/dev/peps/pep-0508/#names

And indeed:

$ python -c "import re; print(re.match(r'^([A-Z0-9]|[A-Z0-9][A-Z0-9._-]*[A-Z0-9])$', 'Swiss Army: Untility Package', re.IGNORECASE))"
None

@uda

This comment has been minimized.

@sinoroc
Copy link

sinoroc commented Nov 20, 2020

@uda But there are no illegal characters in the name of the project you are trying to install, are there? Maybe you would need to add your voice to a different ticket. Maybe this one: #3366, or just simply correct your pyproject.toml to read branch = 'main', since it seems to be your actual issue.

@uda
Copy link

uda commented Nov 20, 2020

Oy, thanks @singulared , I didn't even realize that that's the issue...

@tanj
Copy link
Author

tanj commented Nov 20, 2020

python itself has no issue installing

@tanj You mean pip? Or do you mean python setup.py install?

python setup.py install

It appears that when installing in that way setuptools strips the offending characters. I did go looking briefly before posting this bug report but didn't find PEP508 or the page you found.

Maybe I should report this to setuptools? I would think a warning should be issued if they are modifying the package name to fit PEP508.

@sinoroc
Copy link

sinoroc commented Nov 20, 2020

python itself has no issue installing

@tanj You mean pip? Or do you mean python setup.py install?

python setup.py install

OK, thanks for the confirmation.

It appears that when installing in that way setuptools strips the offending characters. I did go looking briefly before posting this bug report but didn't find PEP508 or the page you found.

No worries, it is a labyrinth... :D

Maybe I should report this to setuptools? I would think a warning should be issued if they are modifying the package name to fit PEP508.

I guess, it is worth asking them the question. Yes a warning sounds good to me.

[I do not know if it can be considered a bug at this point. setuptools (and distutils before that) are so old now, that by now it must be considered a feature for some of their users. And removing this "feature" from setuptools would only cause trouble. :D ]

Maybe poetry as well could be more careful here. I do not know if it is possible to check the metadata before creating the directory (with a name containing a :).

@royerk
Copy link

royerk commented Sep 9, 2021

@uda [...] or just simply correct your pyproject.toml to read branch = 'main', since it seems to be your actual issue.

To use poetry add, just like pip the branch can be specified with @branch-name at the end: poetry add git+https://github.com/someone/project.git@main.

@abn
Copy link
Member

abn commented Apr 9, 2022

Can you please try the fix at #5428.

Using pipx

pipx install --suffix=@5428 'poetry @ git+https://github.com/python-poetry/poetry.git@refs/pull/5428/head'
poetry@5428 new foobar
cd foobar
poetry@5428 add git+https://github.com/tanj/swissarmy.git#7e671142565cb2b2aa04292ec0775c024bf47547
ls $(poetry env info -p)/src

Using a container (podman | docker)

podman run --rm -i --entrypoint bash docker.io/python:3.10 <<EOF
set -xe
python -m pip install -q git+https://github.com/python-poetry/poetry.git@refs/pull/5428/head
poetry new foobar
pushd foobar
poetry add git+https://github.com/tanj/swissarmy.git#7e671142565cb2b2aa04292ec0775c024bf47547
ls \$(poetry env info -p)/src
EOF
+ poetry add git+https://github.com/tanj/swissarmy.git#7e671142565cb2b2aa04292ec0775c024bf47547
Creating virtualenv foobar-lWDpn5M1-py3.10 in /root/.cache/pypoetry/virtualenvs

Updating dependencies
Resolving dependencies...

Writing lock file

Package operations: 3 installs, 0 updates, 0 removals

  • Installing six (1.16.0)
  • Installing python-dateutil (2.8.2)
  • Installing swiss army: untility package (0.1 7e67114)
++ poetry env info -p
+ ls /root/.cache/pypoetry/virtualenvs/foobar-lWDpn5M1-py3.10/src
swissarmy

@abn
Copy link
Member

abn commented May 2, 2022

Resolved-by: #5428

@abn abn closed this as completed May 2, 2022
@mkniewallner mkniewallner removed the status/triage This issue needs to be triaged label Jun 11, 2022
Copy link

github-actions bot commented Mar 1, 2024

This issue 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
kind/bug Something isn't working as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants