Replace setuptools by scikit-build-core to build wheels.

[bcoconni]

Historically JSBSim has used setuptools to build everything linked to Python. But since PR #866 has been merged, the Python module is built by CMake and setuptools is only used to build the Python wheels that are uploaded to PyPI.

As a consequence we are using 2 build systems: setuptools to build the wheels and CMake to build everything else. To avoid manual synchronization, the CMake scripts contain code to collect the source files, compilation flags and libraries to pass them to setuptools. This makes the CMake scripts more complex than they should be.

In addition, the management of compilers by setuptools has an historical limitation inherited from distutils: all flags are passed to all compilers, both C and C++. However on MacOSX the clang C compiler has a very strict policy and rejects pure C++ flags such as the flag that requests using C++17. To work around this limitation, setup.py has some code that overloads some classes in distutils to intercept the source file that is currently compiled and adapt the flags to the programming language. This is a brittle hack because it uses some undocumented distutils API that may be modified at any time. Fortunately, this API is so obscure that the developers of setuptools have never messed with it.

There is also some logic in setup.py to avoid building the JSBSim library if it is already existing. This logic was meant to reduce the duration of the CI job that is building all the wheels.

And there is also some homemade code in setup.py to find and include the MSVC runtime librairies such as msvcp140.dll (see #285 (comment) for more details about why msvcp140.dll is included in the wheels for Windows).

This PR is replacing setuptools by scikit-build-core to avoid all these complexities which should reduce the maintenance burden. scikit-build-core is a Python build backend that is using CMake to build wheels thus saving all the trouble mentioned above:

• JSBSim will only use one build system: CMake. There is no longer a need to pass compilation data (sources, flags, libraries) to another build system i.e. setup.py.
• Compilation flags and libraries are managed by CMake so we no longer need to address ourselves the clang strict policy.
• scikit-build-core uses ninja (an equivalent to make) to compile the wheels. ninja is able to detect the files and libraries that have already been compiled and save re-compiling them again.
• The compilation flags are now located in pyproject.toml rather than buried in the CI scripts. This should allow easier maintenance, especially as far as flags for optimization are concerned.
• The MSVC runtime DLLs are now found by CMake using include(InstallRequiredSystemLibraries) which is saving doing that ourselves.

Below is a description of additional changes that this PR brings:

• The CMake scripts are using the flag SKBUILD to detect that they have been called by scikit-build-core and avoid compiling code that is irrelevant to the Python wheels (unit tests, aeromatic, etc.).
• The script JSBSim.py has been modified to be in a main function. This is to allow an entry point being declared in pyproject.toml so that JSBSim.py is installed as a binary that can be invoked from the command line.
• The Python-Wheels job no longer needs to explicitly build the libJSBSim target before invoking cibuildwheel. This is now all taken care of by CMake. This also allows to avoid installing Doxygen at that stage since cibuildwheel is now using the source distribution package that is built by the MacOSX job and that already contains the documentation strings.
• CMake is now looking for the Development.Module component of Python because - according to scikit-build-core docs - "You do not want to find the entire Development package, as that include Embed component, which is not always present and is not related to making Python extension modules.". As a consequence of that requirement, the minimum version of CMake is now 3.18
• The files that must be included in the wheel are no longer copied but "installed": the CMake command install is now used with the target wheel. This allows a cleaner declaration of the files that are inserted in the wheel.
• The files setup.py and MANIFEST.in are deleted since they are no longer needed.
• The property TARGET_DIRECTORY is no longer needed in the CMake scripts. It was used to auto-discover the source files that would be passed to setup.py.
• Header files are no longer inserted in CMake targets as this was used to auto-discover source files for setup.py.
  • As a result, CMake targets that were containing only headers (Structure and Simgear) have been removed since they were only used to auto-discover source files for setup.py
• The logic in src/CMakeLists.txt that was used to store the compilation flags and libraries in JSBSIM_COMPILE_DEFINITIONS and JSBSIM_LINK_LIBRARIES and pass them to setup.py has been removed.
• The CMake instructions add_coverage are now guarded by if(CXXTEST_FOUND) to avoid coverage steps to be executed during the compilation of the wheels.

This allows to greatly simplify the CMake files and the CI workflow and save approximately 250 lines of build scripts.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 25.30%. Comparing base (f2ff1a3) to head (f8137db).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1400   +/-   ##
=======================================
  Coverage   25.30%   25.30%           
=======================================
  Files         169      169           
  Lines       18574    18574           
=======================================
  Hits         4701     4701           
  Misses      13873    13873           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[seanmcleod70]

Looks good.

[bcoconni]

Merged. Thanks.

Oh and by the way, you are welcome to submit better compilation flags for the Windows wheels as I am not so comfortable with the default values I have set (both the build type and compilation flags):

jsbsim/python/pyproject.toml.in

Lines 72 to 77 in 4a434e0

	[[tool.scikit-build.overrides]]
	if.platform-system = "windows"
	inherit.cmake.define = "append"
	cmake.build-type = "RelWithDebInfo"
	cmake.define.CMAKE_CXX_FLAGS="/DNDEBUG"
	cmake.define.CMAKE_C_FLAGS="/DNDEBUG"

[seanmcleod70]

I'm assuming CMAKE includes a standard set of C++ flags for a MSVC release build in terms of optimization flags etc. and the ones you've listed above are additional flags?

[bcoconni]

I'm assuming CMAKE includes a standard set of C++ flags for a MSVC release build in terms of optimization flags etc. and the ones you've listed above are additional flags?

You already did this investigation 😉 and found that:

C:\source\jsbsim\src>cmake -LAH
...
...
-- The CXX compiler identification is MSVC 19.44.35213.0
....
-- Check for working CXX compiler: C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.44.35207/bin/Hostx64/x64/cl.exe - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
....
// Flags used by the CXX compiler during MINSIZEREL builds.
CMAKE_CXX_FLAGS_MINSIZEREL:STRING=/MD /O1 /Ob1 /DNDEBUG

// Flags used by the CXX compiler during RELEASE builds.
CMAKE_CXX_FLAGS_RELEASE:STRING=/MD /O2 /Ob2 /DNDEBUG

// Flags used by the CXX compiler during RELWITHDEBINFO builds.
CMAKE_CXX_FLAGS_RELWITHDEBINFO:STRING=/MD /Zi /O2 /Ob1 /DNDEBUG

Which is making me realize that the /DNDEBUG flag needs not to be specified since it is a standard flag.

However we might want to pick a Release build rather than RelWithDebInfo ?

[seanmcleod70]

Ah, yes, thanks for reminding me 😉

Now just need to double-check the differences between Release and RelWithDebInfo.

I'd always been used to Debug versus Release builds when using MSVC projects, but had noticed on a couple of projects using CMake, including JSBSim that the default for a release build was RelWithDebInfo. Which stands for 'Release with Debug Info'.

/O2 - The /O1 and /O2 compiler options are a quick way to set several specific optimization options at once. The /O1 option sets the individual optimization options that create the smallest code in the majority of cases. The /O2 option sets the options that create the fastest code in the majority of cases. The /O2 option is the default for release builds.

/Zi - Generate full symbolic debugging information in a Program Database (PDB) file instead of embedding it in object files. This PDB file contains function names, variable types, and line number mappings for use by debuggers.

/Ob1 /Ob2 - Controls inline expansion of functions.

/Ob1 - Allows expansion only of functions marked inline, __inline, or __forceinline, or in a C++ member function defined in a class declaration.

/Ob2 - The default value under /O1 and /O2. Allows the compiler to expand any function not explicitly marked for no inlining.

So over and above the difference in terms of producing symbolic debugging information, not sure we've ever made use of it to debug an issue, there is a slight difference in aggressiveness with regards to inlining methods.

I'm pretty sure all our Windows/MSVC builds in terms of JSBSim.exe and the lib built for the Python bindings have been RelWithDebInfo for years. So safe option would be to stick with that. But if we switch to Release we may eek out slightly more performance with the more aggressive inlining.