python overhaul branch
Use shebang from dunecontrol
Merge request reports
Activity
added 1 commit
- c2314af7 - need to add required python packages to dune.module to have them downloaded for the CI
Can I ask why we need a dune module inside a dune module here? Isn't it possible to "export" two Python modules in the same way, one dune.testtools and one dune.testtooltests?
It seems like the structure of Dune modules is getting more and more complicated on top of an anyhow complex module structure which makes it difficult to understand and maintain.
Probably possible. Since I don't use this and nobody tried to get this to work with the new Python bindings, I'm at the moment just trying to get something to work. Otherwise the sphinx docu on the website doesn't build anymore. If someone else wants to take this over I'm be completely happy with that as well. The other option is then to revert some of my changes while makeing sure it still works. Another problem is that I don't know why this exactly worked previously. The make python_install is called in the builddir but the 'python' folder was not made available in the builddir as far as I could figure out. As I said, this is the best I could do at the moment to at least get things to build again (well I'm not quite finished yet the website is still not building).
In
python/wrapper
are Python scripts used by users of dune-testtools to write system tests. Internally, the scripts use functionality from the Python packagedune.testtools
and particularly from thedune.testtools.wrapper
module. I think it's a common structure that scripts are not part of the Python package. The scripts are also installed with setup.py as scripts.Edited by Timo Kochif they are not installed you need to specify the full path to wherever the dune-testtools source hangs around. But that would only be in a debugging setup and even there you could
pip install -e
?Also for the scripts to work it's required that
dune.testtools
is installed, or at least the PATH and PYTHONPATH need to be adjusted in a way that it would work.What is the issue exactly?
Edited by Timo Koch@tkoch I don't have super much time for this right now, but here is the answer to the module in module question: With the changes in dune-common (at least with the parts that I implemented in spring, I was not following further development), Python packages are installed into the virtualenv, but this is not done at configure (=cmake) time, but at build time instead! For dune-testtools, that means that from a downstream modules perspective, nothing changed. The only actual change is for dune-testtools's internal testing as we cannot use (or test) the functionality of dune-testtools at configure time anymore. That is why I originally went with the module in module approach. If scripts are currently not installed into the virtualenv although marked for installation in
setup.py
, then that is a bug.Thanks for the explanation. Makes sense now. But that means if I run
dunecontrol configure
with freshly checked out modules, it will still fail? Because I haven't installed dune.testtools before running CMake on the test module?So basically the only safe way is to install dune.testtools before running CMake. Then it also wouldn't matter if it's a sub/downstream-module or not.
Edited by Timo KochI don't believe anything has changed in that respect. Doninic's last change to the overhaul branch in dune-testtools was
Here are the issues I could identify with this version:
- Apparently, the switch to the new version of namespace packages hadn't been done at this point so the init.py needs to be removed from the python/dune folder and
- The actual pip install is done without 'index' access because otherwise 'make' in the builddir would fail without internet access which I found unexceptable. So the required packages need to be put into dune.module so that the wheels are build during configure time (with internet access, w/o the python package is just deactivated).
Those changes are easy to make
Now what I'm completely unclear about is how the scirpts in dune-testtools/python are available to be instaled into the venv. I don't think I've changed any behavior in this respect. I always assumed that the scripts are 'linked' into the build-dir but in the version from Dominic there isn't even a subdir command for the python folder in the top level CMakeLists.txt so the python package remains completely in the source dir. How is it found so that it can be installed (editable) into the venv. The setup.py file contains packages=['dune.testtools', 'dune.testtools.wrapper', 'dune.testtools.parametertree', ], but those folders are only available in the source folder but pip install is called from the build folder. How did this work? can work since the 'python' subfolder is not copied into the builddir. So how was the 'make install_python' supposed to find it? Perhaps I completely missed something?
Well in dune-common 2.8 there is still the option for pure Python packages that are installed from the source directory: line 51ff: https://gitlab.dune-project.org/core/dune-common/-/blob/releases/2.8/cmake/modules/DunePythonInstallPackage.cmake
Edited by Timo KochI didn't know about that feature but can't remember removing it. That clears up part of my problem. Then there remains the question what was actually supposed to happen to dune-testtools/wrapper/*.py (and scripts). Is it correct that these are all supposed to be added into venv/bin?
The python/test folder does not seem to be referenced anywhere.
Running
setup.py
should install the scripts so that they are available in thePATH
, so I would guess they are installed tovenv/bin
.In the module in module approach, the python/test folder should be inside the sub/inner/downstream-module as I understood @dominic, maybe the commit just creates the sub-module but doesn't remove the redundant
python/test
?I think it's a very special case (with particular requirements: (1) module can't depend on dune python packages, (2) has to be ready to use at configure time right after calling the CMake function) that should probably be made clear by using a different CMake function. For "normal" packages (not used at configure time through CMake) it should usually be ok to be ready to use after building.
Edited by Timo KochI agree. Maybe options
PURE_PYTHON_SETUP
andINSTALL_IMMEDIATELY
. Pure Python setup would mean it can be directly installed without configuring anything and install immediately (an option which only works together withPURE_PYTHON_SETUP
) installs right away. IfINSTALL_IMMEDIATELY
is not specified then installation can be (should be) deferred to the build time (this would mean the entire module directly would need to be copied to the build directory). Would that work? (No idea what consequences this has for packaging)Edited by Timo KochDon't know if we need to have both options at least not if you don't have a specific use case in mind. I would have suggested something like 'PURE_PYTHON_PACKAGE' which should always be okay to install at configure time. One reason for the build time installation is that parts of the python package could require some compilation then installation has to be postpend. For a pure python package (without CMakeLists.txt files etc.) installing it at configure time (after the venv is setup) should always work I would think.
yes but I then have to set it up in the right structure and add stuff "manually" to the build directory. Well there is no used case right now, so the current mechanism works for that.
Anyway I wanted to make the point that PURE_PYTHON_PACKAGE is not descriptive enough because this pure Python module also can't depend on other non-pure modules. And important for the CMake stuff is that it is installed immediately when calling the CMake command and is available right after, while the default is that the isntallation is deferred.
Edited by Timo Kochand add stuff "manually" to the build directory.
You mean the cmake structure in the source directory? That can possibly be solved with a link from 'source to build' of the
python
folder. If everything is setup there anyway as a python package that would be enough. The install after build should then work fine.Perhaps the configure time setup should be done in a different command at first - the existing install python function is too complex anyway. Once we know what we need we could decide to combine the two into one at a later stage or leave things as is.
@andreas.dedner I just had a quick look at the pipeline and this is not how it's supposed to look like. See for example the last successful pipeline on master https://gitlab.dune-project.org/quality/dune-testtools/-/jobs/273117. There is 56 tests and
pytest
andpep8
shouldn't be skipped. No idea whatCOMMAND
(on this branch) is.Edited by Timo Koch
mentioned in issue core/dune-common#278 (closed)
mentioned in issue core/dune-common#279 (closed)
mentioned in issue #142
With the latest refactoring of the python package installation in core/dune-common!1148 (merged), I believe that bringing
dune-testtools
back to work withmaster
in the core modules is relatively trivial. Now, python packages are installed at configure time and are separated from the python bindings, just like before. I'll make another MR with the proposed changes.