Find python package version via pkg_resources instead of pip-internal method
The pyversion.py
script is called from DunePythonFindPackage.cmake
and tries to find a package by its name, returning the extracted version number. Currently, it does so by checking the content of pip.get_installed_distributions()
.
However, in recent pip versions, pip.get_installed_distributions
method was moved to pip._internal
, leading to an AttributeError
in the call to pyversion
with packages that did not supply the __version__
variable.
Consequently, for those packages, DunePythonFindPackage
will always evaluate to false.
What does this MR do?
This MR restores the behaviour of the pyversion.py
script to look for the version of a package in other places then only the version variable.
It does so via pkg_resources.working_set
, which offers access to the same information on installed distributions and is the recommended way. (Note, that it is strongly suggested to not use the pip._internal
interface.)
Also, it removes the defunct implementation via the pip interface.
Open questions
Can it be assumed that pkg_resources
is installed?
The get-pip.py
docs suggest that it is installed unless explicitly suppressed by passing a flag, but I am not too familiar with the exact setup of the virtual environment...
Merge request reports
Activity
Currently, the build seems to time out on the
Debian:10 gcc:c++17
job. Seems unrelated to the changes implemented here, though?/builds/yunus.sevinchan/dune-common/dune/common/simd/test.hh: In static member function 'static T Dune::Simd::UnitTest::prvalue(T) [with T = Vc_1::SimdArray<double, 8>]': /builds/yunus.sevinchan/dune-common/dune/common/simd/test.hh:142:16: note: The ABI for passing parameters with 64-byte alignment has changed in GCC 4.6 static T prvalue(T t)
The note is unrelated to the timeout. I actually prepared a patch for the CI system in joe/dune-docker!1 to make it possible to skip expensive tests, but apparently, I did not prod @ansgar properly.
Nothing you need to do at the moment.
If you change the MR you might hit the same problem again though. In that case the best thing you can do is hit retry. If that does not succeed after a few tries, prod me again. (Or prod Dominic/Steffen directly, if my guess is right and you're from Heidelberg.)
Just to make this clear: I'm not going to merge this because I feel unqualified to judge this. I'm assuming you're in contact with someone else about merging.
If you change the MR you might hit the same problem again though. In that case the best thing you can do is hit retry. If that does not succeed after a few tries, prod me again.
Alright, thanks! :)
(Or prod Dominic/Steffen directly, if my guess is right and you're from Heidelberg.)
It is. Updated my profile now so that that's clear.
I'm assuming you're in contact with someone else about merging.
Not yet, but will do.
@dominic: Would you be the right person to prod here?
I'm working in the Roth Group at the IUP (together with Lukas Riedel) and we came to use DUNE more extensively the last months. The issue that this MR aims to solve originated from that work.
Sorry for not introducing myself earlier or getting in touch with anyone! I thought I'll just propose this MR and see what happens. :)assigned to @dominic
So, I had a look and also stumbled over the same github issue. I think we should follow their strong hint of not using stuff from
pip._internal
. I think it is safe to assumepkg_resources
presence here - you get it alongside setuptools, which should be present. If it breaks, we can still formulate it as a hard requirement. Otherwise, this is a valid concern and I will merge it once you have updated it.I meant reimplementation with the method proposed in this post
Ah, ok. That's already done here. :)
I'm only using a dict instead of a list; that way, we have the name of the packages as keys and can more easily check for the presence of a certain package:
# Generate a dict of distribution information with project names as keys dist_info = {d.project_name: d for d in pkg_resources.working_set} # Check if package is available at all if modstr in dist_info: # ... can now check for the version
The issue of the runner timing out should be mitigated (!502 (merged), merged into master). Although this MR won't benefit from that, unless you merge master into it, or rebase it on top of master.
Edited by Jö Fahlkeadded 37 commits
-
e07f397e...ec9401e6 - 36 commits from branch
core:master
- 467c9420 - Merge branch 'master' into feature/do-not-use-pip-internals-for-pyversion
-
e07f397e...ec9401e6 - 36 commits from branch
I meant reimplementation with the method proposed in this post
@dominic So would you insist on that specific implementation? Or is there something else wrong with the current state of this MR? Just let me know if there is anything else to do :)
Yunus, I am sorry for my handling of this merge request. I originally misread something (lack of sleep/concentration while at conference) and then I went on leave for a month without looking into this. In the mean time @smuething discovered and fixed the same bug in !540 (merged), but not this merge request. Long story short, it should work on master and 2.6 now.