jacobwilliams / json-fortran

A Modern Fortran JSON API
https://jacobwilliams.github.io/json-fortran/
Other
333 stars 82 forks source link

Fixes compilation with Ninja and IntelLLVM when using OneAPI #544

Closed lmdiazangulo closed 3 months ago

lmdiazangulo commented 10 months ago

I have found some issues trying to compile with the recent Intel One API + Visual Studio 2022. Basically I got this error.

1> [CMake]   failed with:
1> [CMake] 
1> [CMake]    ninja: error: build.ninja:280: multiple rules generate jsonfortran.lib

I have implemented some changes in CMakeLists.txt to fix it by considering the case of IntelLLVM which is the compiler Id of the new compiler.

Best regards, Luis

codecov[bot] commented 8 months ago

Codecov Report

Merging #544 (9eaa63f) into master (7819919) will not change coverage. The diff coverage is n/a.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #544 +/- ## ======================================= Coverage 89.48% 89.48% ======================================= Files 7 7 Lines 5356 5356 ======================================= Hits 4793 4793 Misses 563 563 ```
jacobwilliams commented 8 months ago

It looks like the CI failed? (haven't looked into why)

jacobwilliams commented 3 months ago

What is the reason for both JSONFORTRAN_ENABLE_TESTS and ENABLE_TESTS? It's not clear to me what the user is supposed to set to get the tests...

(note: the CI is failing because the tests aren't being enabled)

jacobwilliams commented 3 months ago

It looks like -D ENABLE_TESTS=ON enables the tests?

lmdiazangulo commented 3 months ago

It looks like -D ENABLE_TESTS=ON enables the tests?

Yes. I added an option because at some point I was using it as a submodule which was imported with add_subdirectory from a parent cmake. It was a little bit confusing to me that the names in the variables defined by options do not include the name of the project they refer to.

Besides that, thank you a lot for this library. We are using it extensively in our FDTD solver project: https://github.com/OpenSEMBA/fdtd

jacobwilliams commented 3 months ago

I think these changes have broken the conda-forge package. The shared library doesn't seem to be being built anymore. Did that get disabled?

We should be getting:

-- Installing: $PREFIX/lib/libjsonfortran.so.8.4.0
-- Installing: $PREFIX/lib/libjsonfortran.so.8.4
-- Installing: $PREFIX/lib/libjsonfortran.so
-- Installing: $PREFIX/lib/libjsonfortran.a

Now we are only getting:

-- Installing: $PREFIX/lib/libjsonfortran.a

https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=943042&view=logs&j=656edd35-690f-5c53-9ba3-09c10d0bea97&t=986b1512-c876-5f92-0d81-ba851554a0a3

jacobwilliams commented 3 months ago

See #558

lmdiazangulo commented 3 months ago

Yes, maybe I disabled that while doing some tests. Maybe is better to revert the merge. Sorry about that.

jacobwilliams commented 3 months ago

I think https://github.com/jacobwilliams/json-fortran/commit/1784d4ee2b8cc08f95e86a87840bbaa75111f649 fixes it.