pyupio / dparse

A parser for Python dependency files
MIT License
61 stars 23 forks source link

Remove setuptools.. #4

Closed jayfk closed 7 years ago

jayfk commented 7 years ago

.. and move to https://github.com/pypa/packaging

.. or pip's parser

pombredanne commented 7 years ago

My hunch is that you may have conflicts because you have two use cases:

  1. as a library dparse (or pyup before that) uses the features of setuptools, packaging, pip to access and parse deps
  2. as an installable package, dparse needs to have its own proper setup.py and be installable.

I do not think that the two can ever be resolved sanely:

This --using packaging tools not only for packaging, but also as libraries for package introspection-- is a very special use case (and for instance there is no API in pip as far as I know).

IMHO the only sane way is to vendor and bundle the tools to be used at runtime and use a standard setup.py for installation.

A few examples:

jayfk commented 7 years ago

Yep. Vendoring is horrible, but probably the only sane way to go for now. I have never done that, PRs welcome!

pombredanne commented 7 years ago

@jayfk tell me which exact package(s) version you depend on for dparse to work

jayfk commented 7 years ago

It's setuptools==26.1.1, on GitHub at https://github.com/pypa/setuptools/tree/e4ba41ec7f54370a030fb09f9f0a334396f38489.

Setuptools comes with it's own vendored things, take a look at: https://github.com/pypa/setuptools/tree/e4ba41ec7f54370a030fb09f9f0a334396f38489/pkg_resources/_vendor

jayvdb commented 7 years ago

packaging is much simpler to vendor than setuptools.

jayfk commented 7 years ago

Are you talking about the packaging package on PyPi, or the packaging module that's part of setuptools?

pombredanne commented 7 years ago

@jayvdb you wrote:

packaging is much simpler to vendor than setuptools.

do you care to elaborate a bit?

jayvdb commented 7 years ago

Please use https://pypi.python.org/pypi/packaging / https://github.com/pypa/packaging

setuptools vendors packaging, and it is not wise to be using packaging indirectly via setuptools (c.f. https://github.com/pypa/setuptools/commits/master/pkg_resources/_vendor/packaging). Please depend on packaging directly.

jayfk commented 7 years ago

Sadly, the standalone packaging behaves differently than the one that's vendored with setuptools. At least for the given release.

pombredanne commented 7 years ago

@jayvdb I reckon that it may not be wise, but if this can work for now that may be good enough until some future refactoring of the dparse internals. The case here is to provide at least for now some minimally invasive way for dparse to continue working as it is now (in its capacity of analyzing deps and package data) without having a hard declared dep on a specific version of setuptools.... and therefore have a setup.py for dparse that ends being some standard, non-specific setup.

pombredanne commented 7 years ago

Now there is a possibly issue with a vendoring of setuptools .... this monkey patching: https://github.com/pypa/setuptools/blob/1eec02038d28506a42bc45d14ef2d54b136cc8bc/setuptools/__init__.py#L160 I wonder how a vendored _setuptools would interact with setuptools proper....

pombredanne commented 7 years ago

I verified that setuptools 26.1.1 vendors packaging 16.7

https://github.com/pypa/setuptools/tree/v26.1.1/pkg_resources/_vendor/packaging and https://github.com/pypa/packaging/tree/16.7

@jayfk ATM the parser uses pkg_resources here https://github.com/pyupio/dparse/blob/a6b3be7155b913e757e0160e633af3371102315c/dparse/parser.py#L21

... e.g. https://github.com/pypa/setuptools/blob/v26.1.1/pkg_resources/__init__.py#L2810 and the Requirement packaging subclass just below .... How far off are these from the base packaging code behavior?

jayvdb commented 7 years ago

I was hoping this mess of multiple incompatible parsers was over, but maybe it isnt.

https://gsnedders.github.io/python-marker-test/results.html was the last audit done of a specific feature (see https://github.com/gsnedders/python-marker-test/ ), done as part of https://github.com/pypa/packaging/issues/72 .

If there are other incompatibilities (or regressions), they should be raised in packaging .

jayfk commented 7 years ago

How far off are these from the base packaging code behavior?

Let's try it out: https://travis-ci.org/pyupio/dparse/builds/254792139?utm_source=github_status&utm_medium=notification

Commit is at https://github.com/pyupio/dparse/commit/63b939c46d2c9c70e906c3e4475c9a6797004c0a

pombredanne commented 7 years ago

FWIW I checked the pkg_resources code for requirements parsing and there is no way to sanely extracts this bit alone as this carries a truckload of other code.

jayfk commented 7 years ago

Okay, this is looking good. packaging, here we come!