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

[Core] Drop build-system support for Qt4 and Python 2 #4575

Closed
wants to merge 12 commits into from

Conversation

chennes
Copy link
Member

@chennes chennes commented Mar 4, 2021

As discussed on the FreeCAD forums, this PR updates our CMake build system to require the following software versions (corresponding to Ubuntu 18.04 LTS):

  • GCC 7.3
  • Clang 6.0
  • CMake 3.10
  • Python 3.6
  • Qt - 5.9

It also sets the C++ standard to C++17 -- this PR is set as a Draft pending figuring out the problem with that standard version and Boost::Bind on one of the Travis builds.

Note that many of the changes in the diffs are indentation-only, since the Qt5 conditional was removed in several places. I left the BUILD_QT5 variable in place, but it defaults to ON and is ignored by the main build system (some Mods still refer to it -- those will be dealt with in later PRs).

Also increase minimum CMake, GCC, and Clang versions to match
Ubuntu 18.04 LTS. This sets our minimum supported version of the C++
standard to C++17.
@wwmayer
Copy link
Contributor

wwmayer commented Mar 4, 2021

As mentioned in the forum thread using C++17 leads to a build failure due to some boost stuff:
https://travis-ci.org/github/FreeCAD/FreeCAD/jobs/761384474#L5251

@luzpaz luzpaz added the Core Issue or PR touches core sections (App, Gui, Base) of FreeCAD label Mar 4, 2021
@wwmayer
Copy link
Contributor

wwmayer commented Mar 4, 2021

Btw with 19d8c75 I have disabled the builds for Qt4 and Py2 which now fail anyway

@chennes
Copy link
Member Author

chennes commented Mar 4, 2021

Thanks for that. I missed a change to SetupPython that I'll commit shortly. It appears that Boost::Bind does not like the function signatures of:

 void createdObject(const App::DocumentObject& obj) noexcept {
        // When undoing the removal
        if (object == &obj) {
            indocument = true;
        }
    }
    void deletedObject(const App::DocumentObject& obj) noexcept {
        if (object == &obj) {
            indocument = false;
        }
    }

The noexcept clause is causing a mismatch in the signature, I think. The compile hasn't finished yet so there may be more out there, but those were the first culprit.

@wwmayer
Copy link
Contributor

wwmayer commented Mar 4, 2021

OK but why doesn't it cause a problem with boost 1.67 and MSVC? Is it then compiler specific or a limitation of boost?

@chennes
Copy link
Member Author

chennes commented Mar 4, 2021

OK but why doesn't it cause a problem with boost 1.67 and MSVC? Is it then compiler specific or a limitation of boost?

I am not entirely sure, but noexcept changed in C++17 -- in earlier versions it was not actually a part of the function's type, but in C++17 they made it a part. So in a formal sense, trying to bind a noexcept function someplace that expects a potentially-throwing function is a mismatch now, and it wasn't before. Of course, it seems like the bind should still work (in a similar way to const-ness... if your function is more restrictive than required it's still OK), so it may be that some compilers are magic-ing it, and some are not. Or maybe Boost has some magic of their own. BTW, What is Boost::bind providing that we don't get from std::bind?

@chennes chennes force-pushed the libraryRequirementUpdate_0_20 branch 4 times, most recently from b46fbb5 to 148f32b Compare March 5, 2021 01:30
@chennes chennes force-pushed the libraryRequirementUpdate_0_20 branch from 148f32b to a2ff7f4 Compare March 5, 2021 01:54
@donovaly
Copy link
Member

donovaly commented Mar 5, 2021

sets the C++ standard to C++17

This works for me except for the build target "ReverseEngineering" For this build target I have to use C++14. (ReverseEngineeringGui is not affected)

@chennes
Copy link
Member Author

chennes commented Mar 5, 2021

This works for me except for the build target "ReverseEngineering" For this build target I have to use C++14. (ReverseEngineeringGui is not affected)

Thanks for testing. I have a patch for ReverseEngineering, it uses a library that has some deprecated stuff from before C++11 in the Windows LibPack. I started a discussion about it here: https://forum.freecadweb.org/viewtopic.php?f=10&t=56328

@luzpaz
Copy link
Contributor

luzpaz commented Mar 5, 2021

BTW, do we need to change anything in https://github.com/FreeCAD/FreeCAD/blob/master/requirements.txt ?

@wwmayer
Copy link
Contributor

wwmayer commented Mar 5, 2021

BTW, What is Boost::bind providing that we don't get from std::bind?

Wow, good catch! Using std::bind fixes the build failure. The reason for using boost::bind simply is that we already used it 15 years ago for boost's signal/slot mechanism and it's used by habit in new code. Since std::bind has been added with C++11 boost::bind was the only option at that time.

@wwmayer
Copy link
Contributor

wwmayer commented Mar 5, 2021

In 3779aad I have replaced the boost::bind with std::bind

@chennes chennes force-pushed the libraryRequirementUpdate_0_20 branch from 05bba05 to 06d64ba Compare March 5, 2021 14:33
PCL was already defaulted to OFF on Linux, this changes the default to OFF on MSVC
as well. This can be reverted once the primary LibPack for Windows includes a version
of FLANN that compiles under C++17 (the last official release of FLANN, 1.9.1, does
not, but the HEAD in their Git repository does).
@chennes chennes marked this pull request as ready for review March 5, 2021 17:43
@chennes
Copy link
Member Author

chennes commented Mar 5, 2021

OK, this PR should be ready (0.20 only, obviously). I didn't squash it because I'm afraid of screwing it up with all those merges from master, but obviously it probably should be: there is a change and reversion to DocumentObserver (@wwmayer fixed the issue on the trunk by switching to std::bind).

@wwmayer
Copy link
Contributor

wwmayer commented Mar 6, 2021

@wwmayer wwmayer closed this Mar 6, 2021
@chennes chennes deleted the libraryRequirementUpdate_0_20 branch March 6, 2021 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Issue or PR touches core sections (App, Gui, Base) of FreeCAD
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants