Open
Conversation
… API in libspud.c
… environment variables in testharness
stephankramer
added a commit
to FluidityProject/spud
that referenced
this pull request
Oct 24, 2022
Summary is that distutils has been deprecated for a while and will be removed in 3.12. setuptools is its replacement. However its use as an installation mechanism, e.g. *calling* setup.py install with CLI arguments is also being deprecated - with no good alternative (yet) being available. Since the whole world still depends on it I don't expect the latter to go away any time soon though, but it does seem to no longer care about fixing things for that frontend which is what now cause prefix arguments to not work as expected (installing things in /usr/local/ whilst specifying --prefix=/usr), which breaks the building of Debian packages. What Debian/Ubuntu seem to have done is patch setuptools to make it keep on behaving as they want, but only if you provide --install-layout=deb. So now providing that argument for all calls of "python setup.py install" in the Debian build. This would have also worked in combination with distutils, but taking this opportunity to upgrade to setuptools (since we'll have to soon anyway). These are changes by @Patol75, taken from FluidityProject/fluidity#351.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #346
The main idea of this PR is to account for the deprecation and future removal of the Python package
distutils. In doing so, I have encountered quite a few aspects that deserved maintenance, so I have tried to account for them as well. I describe all the changes below.Changes in
Makefile.inand in thepythonfolder reflect how building and installation of thefluidityPython package are now handled throughsetuptools,buildandpip. Newpyproject.tomlandsetup.cfgfiles replacesetup.pyin thepythondirectory. These files describe the building process tobuild, which then produces both a.tar.gzarchive and a.whlwheel, stored in/python/dist, that can be processed throughpip, yielding a local installation of thefluiditypackage. As a result, it is no longer required to updatePYTHONPATHto have access tofluidity,fluidity.diagnostics,fluidity_tools,GFD_basisChange_tools.pyandvtktools, as reflected intools/testharness.py.Additionally, in
Makefile.in, I have replaced all instances ofpython3with@PYTHON@, which is sourced from the configuration stage (see below). Thepython_buildtarget is now properly handled and executed, andpython_cleanhas been updated. I have also updated the output names of the testing scripts in line with changes to the CI (see below). Finally, theinstalltarget does not executedefaultandfltoolstargets anymore, which seemed quite repetitive. Similarly, thefltoolstarget now directly executes thefldecomptarget to avoid repetition.I have tried to simplify as much as I could the Python section of the
configurefile. I found that this section was quite confusing because it retained many compatibility aspects that are either deprecated or not supported anymore. The new Python section is located here. Additionally, I have removed the--enable-pythoncommand-line argument that I believe no one uses.Regarding the CI, I have tried to simplify the Dockerfiles.
python3-venvandbuildare required for the package installation changes stated above. Other packages are handled through the virtual environment. I have removed the Groovy files, as Groovy is not supported anymore. For the.yaml, I have pushed some changes in line with the 'testharnessClean' branch, which I am hoping to go back to when the more recent PRs are merged. These changes do not dramatically alter the way the CI operates, but I believe they bring some more clarity. Better handling of the longtests will be achieved through the 'testharnessClean' branch.Within
/libspud, few directories use asetup.pyfile. I have kept these files, albeit with calls tosetuptoolsinstead ofdistutils, as not alldistutilsbehaviours can currently be handled throughpyproject.tomlandsetup.cfgfiles (see the note here). I have also accounted for changes in the NumPy C API within/libspud/python/libspud.c.Finally, quite a few C++ scripts in
/toolsused the functionchdirwithout having importedunistd. I have corrected that.CI failures correspond to
spud-setnot being found at runtime (spud-setis located at/libspud/bin). I had already seen this behaviour when I was working on thetestharnessre-write, as highlighted here. I am puzzled at how the CI in themainbranch currently findsspud-setin the first place.