FindPython3 broken for MAC.

added bug label

@gruenich: At this point I think we might consider to revert your FindPython3 changes since it keeps breaking things.

At this point I think we might consider to revert your FindPython3 changes since it keeps breaking things.

Robert, please stop whining about every single breakage on master! You are very quick proposing a revert. Give others the time to fix forward. You know it is impossible to test all circumstances and ways of using our code. As long as users don't try the code, we will never be able to iron the remaining bugs out.

If you can't tolerate these short periods of disruption, stay with the stable releases! You live on master, things are going to break - not because I offhandedly rush my changes. Alternatively, go back to a known working version for a couple of days until we fix it.

Why, things should more or less mature when merged into master. It is extremely disrespectful that you expect others to fix fallout from changes that you did. At this point I would say you should no longer be allowed to commit changes to dune-common unsupervised. It is simply not working. Your commits produce a higher than acceptable code breakage. It is simply not working. Furthermore, when things break I expect the responsible person (he/she who accepted the merge) to be around to fix things (actively). This has not been the case with you and this is not the first time. You leave the fix to other people, sometimes because of not having time and sometimes because of lack of knowledge. Unacceptable. If you don't have time to followup on things that break then don't merge. It as simple as that.

I agree with Christoph here. Of course we all try to only merge code that works, but it is never possible to avoid all breakage. Whoever runs master must live with short-term breakages.

This maybe a practical point of view for code in dune-common, but for the build system it is different. I would agree to this, if we had weekly stable snapshots. But we don't have this, so the master should be somewhat stable. If the build system breaks, this is very bad.

Apart from that I think it's extremely careless to take code from recent development branches in cmake and port it into the build system here.

@robert.kloefkorn: If code breaks on MAC, only Mac OS X users can fix it. Other people don't have access nor do we even have a CI system testing for Mac OS X.

Setting up a CI system for OS X / Windows / *BSD might be worth looking into by people using these systems.

Also I think your "[...]" comment is not really appropriate.

@ansgar: Thanks. I have rephrased it.

@core: I would like to propose to revert the FindPython3 changes. It breaks installing DUNE on MAC OS and it broke the dune_python_add_test. It is not clear what else will break.

Apparently, the offending cmake_path code was only in that module for a very short time frame of 1 month:

So up- or downgrading CMake might solve the problem. Hope this helps. Reverting will also create a mess in itself.

cmake was installed using

pip install cmake

3 different versions were tested: 3.18.4, 3.16.8, 3.14.4

All produce the same error using a pristine dune-common.

Is there any way in cmake to catch errors like that and then reverting back to the original way of finding Python. It seems that we introduced something a bit too 'cutting edge' with `FindPython3' if there are actually so fundamental changes to cmake related to this a recently as 2 months ago.

Oh, this is because we copied the relevant code in the buggy period: https://gitlab.dune-project.org/core/dune-common/-/blob/master/cmake/modules/FindPython3/Support.cmake

LOL.

@dominic @gruenich does this mean the solution would be as simple as updateing FindPython3.cmake?

I don't know. I used the "Dune-we", I was not actually involved.

I backported the FindPython3 code to CMake 3.13. It contained newer code constructs. Obviously, I missed either something specific to OS X or related to installation. I am going to have a look tonight.

mentioned in merge request !916 (merged)

It boils down to this change:

@@ -3043,12 +3043,13 @@ if (("Development.Module" IN_LIST ${_PYTHON_PREFIX}_FIND_COMPONENTS
       AND ${_PYTHON_PREFIX}_LIBRARY_RELEASE MATCHES "${CMAKE_SHARED_LIBRARY_SUFFIX}$")
     # rpath must be specified if python is part of a framework
     unset(_${_PYTHON_PREFIX}_is_prefix)
     foreach (_${_PYTHON_PREFIX}_implementation IN LISTS _${_PYTHON_PREFIX}_FIND_IMPLEMENTATIONS)
       foreach (_${_PYTHON_PREFIX}_framework IN LISTS _${_PYTHON_PREFIX}_${_${_PYTHON_PREFIX}_implementation}_FRAMEWORKS)
-        if (${_PYTHON_PREFIX}_LIBRARY_RELEASE MATCHES "^${_${_PYTHON_PREFIX}_framework}")
-          get_filename_component (_${_PYTHON_PREFIX}_framework "${_${_PYTHON_PREFIX}_framework}" DIRECTORY)
+        cmake_path (IS_PREFIX _${_PYTHON_PREFIX}_framework "${${_PYTHON_PREFIX}_LIBRARY_RELEASE}" _${_PYTHON_PREFIX}_is_prefix)
+        if (_${_PYTHON_PREFIX}_is_prefix)
+          cmake_path (GET _${_PYTHON_PREFIX}_framework PARENT_PATH _${_PYTHON_PREFIX}_framework)
           set (${_PYTHON_PREFIX}_LINK_OPTIONS "LINKER:-rpath,${_${_PYTHON_PREFIX}_framework}")
           break()
         endif()
       endforeach()
       if (_${_PYTHON_PREFIX}_is_prefix)

Nevertheless, I would go back to code from 3.19.3 and only back-port the unversioned Python support, see !916 (merged)

mentioned in issue #235 (closed)

changed the description

closed with merge request !916 (merged)

mentioned in commit 1b3700d8

FindPython3 broken for MAC.

Designs

Child items ...

Activity