libgit2 / pygit2

Python bindings for libgit2
https://www.pygit2.org/
Other
1.58k stars 382 forks source link

Missing `info.requires_dist` metadata in PyPI JSON API #1128

Closed KSmanis closed 2 years ago

KSmanis commented 2 years ago
$ curl -s https://pypi.org/pypi/pygit2/json | jq '.info.requires_dist'
null

The info.requires_dist field in the PyPI JSON API is null, leading Poetry, which depends on the JSON API, to resolve wrong dependencies [*]. ~Supposedly this field should have been properly populated by recent versions of twine, but I haven't been able to pinpoint the source of this nuisance.~ Apparently the upload order seems to affect the contents of the field. My guess is that the sdist is to blame, because it seems to be always uploaded first according to PyPI timestamps.

[*] Specifically, when the field is null, Poetry downloads the first available wheel (instead of the most suitable one) in order to extract the relevant metadata. This happens to be a non-3.7 Python wheel, thus missing the cached-property dependency, causing, in turn, import pygit2 to fail. This behavior is a known issue in Poetry.

jdavid commented 2 years ago

In the new 1.9.0 release I've uploaded the wheels first, and now the requires_dist field is filled in https://pypi.org/pypi/pygit2/json Does this fix the issue?

KSmanis commented 2 years ago

Thank you, it partially does indeed. Uploading the wheels first is one of the two conditions required for properly populating the field. The other condition is using an environment marker for cached-property so that it is only installed when necessary. https://github.com/libgit2/pygit2/commit/d1cbb5ab4e5b3fca11017de3bacfba81609555ba reverted this exact change, was it causing any trouble?

jdavid commented 2 years ago

The commit's comment says:

In the line:

python setup.py egg_info

Setuptools produces a requirements file with an environment marker syntax that is not supported by pip.

Just tested with latest pip and it's still broken, python setup.py egg_info produces the pygit2.egg-info/requires.txt file with this content:

cffi>=1.4.0

[:python_version < "3.8"]
cached-property

Then running pip install -r pygit2.egg-info/requires.txt produces the error:

pip._vendor.pyparsing.exceptions.ParseException: Expected string_end, found '['  (at char 11), (line:1, col:12)

This is used in the build.sh script. Maybe there's another way. PRs welcome.

KSmanis commented 2 years ago

Is there any minimum version constraint on setuptools? If not, the latest version seems to be working properly:

$ git clone gh:libgit2/pygit2
Cloning into 'pygit2'...
remote: Enumerating objects: 12229, done.
remote: Counting objects: 100% (772/772), done.
remote: Compressing objects: 100% (509/509), done.
remote: Total 12229 (delta 481), reused 502 (delta 262), pack-reused 11457
Receiving objects: 100% (12229/12229), 6.91 MiB | 769.00 KiB/s, done.
Resolving deltas: 100% (8770/8770), done.
$ cd pygit2/
$ docker run -it -u "$(id -u):$(id -g)" -v "$PWD:/usr/src/pygit2" -w /usr/src/pygit2 --rm python:3.7-slim bash
I have no name!@44f09a58083b:/usr/src/pygit2$ python -m venv venv
I have no name!@44f09a58083b:/usr/src/pygit2$ . venv/bin/activate
(venv) I have no name!@44f09a58083b:/usr/src/pygit2$ pip install -U pip setuptools 
WARNING: The directory '/.cache/pip' or its parent directory is not owned or is not writable by the current user. The cache has been disabled. Check the permissions and owner of that directory. If executing pip with sudo, you may want sudo's -H flag.
Collecting pip
  Downloading pip-22.0.3-py3-none-any.whl (2.1 MB)
     |████████████████████████████████| 2.1 MB 1.4 MB/s 
Collecting setuptools
  Downloading setuptools-60.9.3-py3-none-any.whl (1.1 MB)
     |████████████████████████████████| 1.1 MB 1.7 MB/s 
Installing collected packages: pip, setuptools
  Attempting uninstall: pip
    Found existing installation: pip 20.1.1
    Uninstalling pip-20.1.1:
      Successfully uninstalled pip-20.1.1
  Attempting uninstall: setuptools
    Found existing installation: setuptools 47.1.0
    Uninstalling setuptools-47.1.0:
      Successfully uninstalled setuptools-47.1.0
Successfully installed pip-22.0.3 setuptools-60.9.3
(venv) I have no name!@44f09a58083b:/usr/src/pygit2$ python setup.py egg_info
/usr/src/pygit2/venv/lib/python3.7/site-packages/setuptools/installer.py:30: SetuptoolsDeprecationWarning: setuptools.installer is deprecated. Requirements should be satisfied by a PEP 517 installer.
  SetuptoolsDeprecationWarning,
WARNING: The wheel package is not available.
WARNING: The directory '/.cache/pip' or its parent directory is not owned or is not writable by the current user. The cache has been disabled. Check the permissions and owner of that directory. If executing pip with sudo, you should use sudo's -H flag.
WARNING: The wheel package is not available.
WARNING: The directory '/.cache/pip' or its parent directory is not owned or is not writable by the current user. The cache has been disabled. Check the permissions and owner of that directory. If executing pip with sudo, you should use sudo's -H flag.
running egg_info
creating pygit2.egg-info
writing pygit2.egg-info/PKG-INFO
writing dependency_links to pygit2.egg-info/dependency_links.txt
writing requirements to pygit2.egg-info/requires.txt
writing top-level names to pygit2.egg-info/top_level.txt
writing manifest file 'pygit2.egg-info/SOURCES.txt'
reading manifest file 'pygit2.egg-info/SOURCES.txt'
adding license file 'COPYING'
adding license file 'AUTHORS.rst'
writing manifest file 'pygit2.egg-info/SOURCES.txt'
(venv) I have no name!@44f09a58083b:/usr/src/pygit2$ cat pygit2.egg-info/requires.txt
cffi>=1.9.1
cached-property
(venv) I have no name!@44f09a58083b:/usr/src/pygit2$ pip install -r pygit2.egg-info/requires.txt 
WARNING: The directory '/.cache/pip' or its parent directory is not owned or is not writable by the current user. The cache has been disabled. Check the permissions and owner of that directory. If executing pip with sudo, you should use sudo's -H flag.
Collecting cffi>=1.9.1
  Downloading cffi-1.15.0-cp37-cp37m-manylinux_2_12_x86_64.manylinux2010_x86_64.whl (427 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 427.1/427.1 KB 1.0 MB/s eta 0:00:00
Collecting cached-property
  Downloading cached_property-1.5.2-py2.py3-none-any.whl (7.6 kB)
Collecting pycparser
  Downloading pycparser-2.21-py2.py3-none-any.whl (118 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 118.7/118.7 KB 3.3 MB/s eta 0:00:00
Installing collected packages: cached-property, pycparser, cffi
Successfully installed cached-property-1.5.2 cffi-1.15.0 pycparser-2.21

Edit: Of course I forgot to add the environment marker, pardon me, haven't had my coffee yet! I'll keep looking into it.

KSmanis commented 2 years ago

Would it be acceptable if the dependencies were extracted into a requirements.txt file, the contents of which would be assigned to install_requires? Something along these lines:

requirements.txt:

cached-property; python_version < "3.8"
cffi>=1.9.1

setup.py:

with open('requirements.txt') as f:
    install_requires = f.read().splitlines()

build.sh:

# Build pygit2
$PREFIX/bin/pip install -U pip wheel
if [ "$1" = "wheel" ]; then
    shift
    $PREFIX/bin/pip install wheel
    $PREFIX/bin/python setup.py bdist_wheel
    WHEELDIR=dist
else
    # Install Python requirements & build inplace
    $PREFIX/bin/pip install -r requirements.txt
    $PREFIX/bin/python setup.py build_ext --inplace
fi
jdavid commented 2 years ago

Yes, that looks good.