Skip to content
Snippets Groups Projects

python overhaul branch

Closed Andreas Dedner requested to merge feature/cmake-python-overhaul into master
3 unresolved threads

Use shebang from dunecontrol

Edited by Andreas Dedner

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Andreas Dedner changed title from use a different shebang in test-setup.sh since /usr/bin/bash fails on a ubuntu 20 machine. to python overhaul branch

    changed title from use a different shebang in test-setup.sh since /usr/bin/bash fails on a ubuntu 20 machine. to python overhaul branch

  • Andreas Dedner added 1 commit
  • Andreas Dedner added 1 commit

    added 1 commit

    • c2314af7 - need to add required python packages to dune.module to have them downloaded for the CI

    Compare with previous version

    • 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).

    • Please register or sign in to reply
    • For example: why is there a folder python/dune/testtools/wrapper and a folder python/wrapper? What is the package structure supposed to look like at the end?

    • In python/wrapper are Python scripts used by users of dune-testtools to write system tests. Internally, the scripts use functionality from the Python package dune.testtools and particularly from the dune.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 Koch
    • so how does the user get access to the pytjon/wrapper scripts if they are not installed?

    • if 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
    • I thought the new Python stuff also works that way that everything is installed in a virtual env now per default. So the situation that they are not installed shouldn't really exist anymore?

    • @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 Koch
    • I don't believe anything has changed in that respect. Doninic's last change to the overhaul branch in dune-testtools was

      39d4693e

      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 Koch
    • I 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 the PATH, so I would guess they are installed to venv/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?

    • Yes, dunecontrol configure would fail which makes the overall approach questionable

    • But if dune_python_install_package would allow to install pure Python packages at configure time everything would work again, right?

    • that would probably work if it was done together with the other requirements. Of course that python package couldn't use any dune bindings (at configure time)

    • 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 Koch
    • I was considering a good named parameter for the existing function - just relying on 'setup.py' being available is a bit to implicit.

    • I agree. Maybe options PURE_PYTHON_SETUP and INSTALL_IMMEDIATELY. Pure Python setup would mean it can be directly installed without configuring anything and install immediately (an option which only works together with PURE_PYTHON_SETUP) installs right away. If INSTALL_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 Koch
    • Don'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.

    • well it doesn't work if the pure python package depends on other non-pure python dune packages.

    • But then it needs to be installed at the same time as the dune time s the bindings and the current mechanism does the trick. That is what dune-testtools does now so that works.

    • 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 Koch
    • and 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.

    • Please register or sign in to reply
  • Andreas Dedner added 1 commit
  • Andreas Dedner added 2 commits

    added 2 commits

    • 4c8f2deb - get to run with merged python overhaul branch in dune-common
    • 0e7c3dca - use a different shebang in test-setup.sh since /usr/bin/bash fails on a ubuntu 20 machine.

    Compare with previous version

  • Andreas Dedner mentioned in issue #142

    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 with master 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.

  • closed

Please register or sign in to reply
Loading