Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace distutils throughout building system #351

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Patol75
Copy link
Contributor

@Patol75 Patol75 commented Jan 25, 2022

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.in and in the python folder reflect how building and installation of the fluidity Python package are now handled through setuptools, build and pip. New pyproject.toml and setup.cfg files replace setup.py in the python directory. These files describe the building process to build, which then produces both a .tar.gz archive and a .whl wheel, stored in /python/dist, that can be processed through pip, yielding a local installation of the fluidity package. As a result, it is no longer required to update PYTHONPATH to have access to fluidity, fluidity.diagnostics, fluidity_tools, GFD_basisChange_tools.py and vtktools, as reflected in tools/testharness.py.

Additionally, in Makefile.in, I have replaced all instances of python3 with @PYTHON@, which is sourced from the configuration stage (see below). The python_build target is now properly handled and executed, and python_clean has been updated. I have also updated the output names of the testing scripts in line with changes to the CI (see below). Finally, the install target does not execute default and fltools targets anymore, which seemed quite repetitive. Similarly, the fltools target now directly executes the fldecomp target to avoid repetition.

I have tried to simplify as much as I could the Python section of the configure file. 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-python command-line argument that I believe no one uses.

Regarding the CI, I have tried to simplify the Dockerfiles. python3-venv and build are 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 a setup.py file. I have kept these files, albeit with calls to setuptools instead of distutils, as not all distutils behaviours can currently be handled through pyproject.toml and setup.cfg files (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 /tools used the function chdir without having imported unistd. I have corrected that.


CI failures correspond to spud-set not being found at runtime (spud-set is located at /libspud/bin). I had already seen this behaviour when I was working on the testharness re-write, as highlighted here. I am puzzled at how the CI in the main branch currently finds spud-set in the first place.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace distutils by setuptools
1 participant