search

optimad / bitpit

Open source library for scientific HPC
http://optimad.github.io/bitpit/
GNU Lesser General Public License v3.0
115 stars 34 forks source link

IO: use Boost library for reading/writing configuration files #442

Closed andrea-iob closed 4 months ago

andrea-iob commented 6 months ago

Using boost allows to drop libxml2 and RapidJOSN dependencies. These two dependencies are tricky to install on Windows, dropping them will make it easy to install bitpit in Windows.

roccoarpa commented 6 months ago

Hi, apologies, I prefer to leave the review to the others.

edoardolombardi commented 4 months ago

The pull modifications are fine. Anyway, as it is, it's compiling only for Linux, even if its goal is to simplify the Windows compilation. Thus, as the pull is related to issue affecting Windows version of the library, it should be rebased on a msvc compliant branch. The current branch rebased on top of https://github.com/optimad/bitpit/tree/general.msvc.compiling.for.AIob can be found in https://github.com/optimad/bitpit/tree/io.configuration.with.boost.msvc, @andrea-iob.

Furthermore, as libxml2 and rapidjson library are no more needed, the download of them through Nuget tool during project configuration, as done in the mscv compliant branches should be removed.

The pull should be merged together (or after, in any case rebased on) the complete support of Windows OS. Please, consider that the msvc related branch/es need continuous rebases on top of the master branch to be kept aligned and often conflicts happen, mostly in cmake project files.

andrea-iob commented 4 months ago

This pull request is a preparatory step for adding Windows support and should be merged before adding Windows support. The branch the will allow to compile bitpit on Windows will start from here and will introduce the needed changes for adding Windows support. The sole purpose of these changes is to avoid using Nuget in that future branch (libxml2 and RapidJSON were working fine on Linux).

edoardolombardi commented 4 months ago

In my opinion it's not a good practice coding preparatory developments for Windows without testing them on Windows. Indeed, in order to know if their introduction doesn't affect Windows compliant branch, we had to rebase it and test the library compiled on Windows for our purposes. The Windows porting should have higher priority than interventions aimed to the optimization of Windows compliancy as this one. As the branch on Linux and the Windows rebase are ok, I'm approving, but, in order to avoid external rework needed to test the pulls, I think the choice of the prioritization should be reviewed.