Setuptools needs to be vendored

[adamjstewart]

Both jinja2 and pytest have a setuptools dependency, but we aren't currently vendoring setuptools:

$ grep -R pkg_resources
jinja2/loaders.py:        from pkg_resources import DefaultProvider, ResourceManager, \
_pytest/vendored_packages/pluggy.py:        from pkg_resources import (iter_entry_points, DistributionNotFound,
_pytest/config.py:        import pkg_resources
_pytest/config.py:            for entrypoint in pkg_resources.iter_entry_points('pytest11')
_pytest/assertion/rewrite.py:        self._register_with_pkg_resources()
_pytest/assertion/rewrite.py:    def _register_with_pkg_resources(cls):
_pytest/assertion/rewrite.py:            import pkg_resources
_pytest/assertion/rewrite.py:            pkg_resources.__name__
_pytest/assertion/rewrite.py:        pkg_resources.register_loader_type(cls, pkg_resources.DefaultProvider)
_pytest/outcomes.py:            from pkg_resources import parse_version as pv
_pytest/outcomes.py:                          "pkg_resources to parse version strings." % (modname,),

Currently, when I build python with Spack and use it to try to run the unit tests, I see the following error message:

==> Error: No module named 'pkg_resources'

[adamjstewart]

@alalazo Want to take another stab at vendoring setuptools? Alternatively, we could stop vendoring things like pytest which aren't required to run Spack. We already don't vendor flake8, although I wonder if we should.

[alalazo]

Regardless of setuptools (that is needed by Jinja 2 and thus is a real dependency) I am also wondering since a while if we should avoid vendoring dev dependencies like pytest. That will make it a little bit more difficult to setup development for Spack, but on the other hand we could use up to date packages if employing Python 3.X as an interpreter. For instance, we are currently using pytest@3.2.5 instead of pytest@5.1.2 because of Python 2.6 - while we could setup different virtual environments for development and check that Spack stays compatible with the latest changes.

@tgamblin What do you think about that?

[adamjstewart]

Ping @tgamblin

[healther]

We already don't vendor flake8, although I wonder if we should.

I am also wondering since a while if we should avoid vendoring dev dependencies like pytest.

Why not provide them via spack bootstrap? Probably with a flag

[adamjstewart]

@alalazo I would like to see this in the next release. Vendoring setuptools would allow us to do a lot of fun stuff in the PythonPackage base class. For instance, packages won't need to define an ever-changing list of import_modules, we can run setuptools.find_packages to automatically detect the libraries we want to try to import. Then we can add import tests for every PythonPackage in Spack, reducing a lot of lines of code at the same time. See https://spack.readthedocs.io/en/latest/build_systems/pythonpackage.html#import-tests

[scheibelp]

I'd have to think about whether this preserves compatibility. I don't think jinja2 and pytest are essential dependencies for the normal operation of Spack, although setuptools is, so conflicts between vendored and system setuptools may be a problem.

I definitely like the idea from #9034 (comment) - could packages presume setuptools support and fall back to manually-defined import_modules otherwise?

[adamjstewart]

could packages presume setuptools support and fall back to manually-defined import_modules otherwise?

I definitely could, but if we're planning on not vendoring setuptools, we probably shouldn't vendor jinja2 or pytest either. I'd prefer Spack "Just Works", so I'm in favor of vendoring all 3, maybe even flake8 too while we're at it.

[alalazo]

I don't think jinja2 and pytest are essential dependencies for the normal operation of Spack, although setuptools is, so conflicts between vendored and system setuptools may be a problem.

@scheibelp jinja2 is a real dependency. We use it for core commands exposed to users like spack containerize or generating module files etc. pytest is only used by spack test - but more recent versions are incompatible with our tweaks of argparse so we need to take care of that too if we start not vendoring it.

[scheibelp]

If we can identify one Setuptools version that works for each version of Python currently supported by Spack (i.e., those versions which are unit-tested by the CI), then I'd be less worried. The latest one which appears to satisfy those constraints (i.e. which supports Python 2.6) on PyPI is https://pypi.org/project/setuptools/36.8.0/. If issues have been resolved for later Python versions in later versions of Setuptools, freezing to this version could be problematic.

Once 2.6 support is dropped (i.e. we stop including it in our unit tests), then we could freeze to a much later version of Setuptools.

jinja2 is a real dependency

I agree. But I think there is still a difference in the importance of the jinja2 dependency and the setuptools dependency. Since jinja2 is used for modules/containers, if we have to update that vendored dependency for later versions of Python then we could just say those features aren't available for 2.X versions of python. If Setuptools 36.8.0 stops working with the latest version of Python we want to support in Spack, then we are stuck.

if we're planning on not vendoring setuptools, we probably shouldn't vendor jinja2 or pytest either

To be clear: based on the arguments I have made above in this comment, I disagree.

[tgamblin]

Hey folks -- I took a look at this. We do not use the part of jinja2 that would require pkg_resources. jinja2.loaders.PackageLoader is the offending class. We do not instantiate it anywhere, and neither does jinja2. We use FileSystemLoader. So jinja2 is safe -- we don't need to deal with it.

The only thing we need setuptools for is pytest, and recent versions of pytest do not use pkg_resources anymore. I'm going to dig a bit deeper to see whether we can just eliminate the use of pkg_resources in our (now rather old) pytest version.

[tgamblin]

Ok I looked at pytest. pkg_resources is used only to mark pytest plugins for rewriting, and as part of pytester, which is disabled by default and used only to test pytest with pytest. For rewriting plugins, it looks like they use something else in newer versions (pytest-dev/pytest#5063), but we don't even care about this use case. For testing pytest, well, we don't do that either.

If I just make _mark_plugins_for_rewrite a no-op in _pytest/config.py, things seem to pass. So why don't I just put in a patch to our vendored pytest to get rid of the code that imports pkg_resources?

[alalazo]

So why don't I just put in a patch to our vendored pytest to get rid of the code that imports pkg_resources?

+1 to that