search

martukas / nuclei

An Evaluated Nuclear Structure Data (ENSDF) parser, viewer and editor
GNU General Public License v3.0
20 stars 6 forks source link

Work on migrating to conan v2 #16

Closed php1ic closed 11 months ago

php1ic commented 1 year ago

Updates on the road to getting things working with conan v2.

This should NOT yet be merged as it doesn't compile. There are include errors relating to 1 of the packages pulled in by conan (nlohmann) but I can't see why that package is any different to the others so need some help.

ferdymercury commented 1 year ago

Maybe this helps? https://github.com/nlohmann/json/discussions/3483 https://json.nlohmann.me/integration/package_managers/#conan

php1ic commented 1 year ago

The branch now compiles and I can run the program :+1:

In the top level CMakeLists file I had to explicitly call find_package() on all the packages installed via conan then make sure they were linked against when building the main executable. Perhaps in conan v1 a lot of that was automagically done by the code used in cmake and the file it downloaded?

php1ic commented 1 year ago

Conan work has been moved into it's own cmake file and we are back to only needing to run cmake .. from the build directory.

Minor annoyance is that conan will always be called with cmake as I couldn't find a good way to check if packages are already installed thus stopping subsequent (unnecessary) calls to conan. If we don't want to do this, we can delete the new cmake file, remove the call from the top level cmake file and have a separate call to conan. The cmake call will still just be cmake .. as the variable is set externally to the conan specific file.

ferdymercury commented 1 year ago

Builds well with Ubuntu 22, thanks for this!!

php1ic commented 1 year ago

I've made a PR on the conan-Qt-Color-Widgets repo to update to the latest commit on Qt-Color-Widgets. Local testing of this repo with the latest commit doesn't raise any issues.

Should we wait for that PR to be merged then update the conan reference before accepting this merge? (assuming that PR is accepted of course :wink: )

ferdymercury commented 1 year ago

@php1ic is this PR also addressing https://github.com/martukas/nuclei/issues/1 ?

php1ic commented 1 year ago

@php1ic is this PR also addressing #1 ?

It wasn't the plan, but the build is now failing due to mismatches package dependencies, so I'll check things are the latest.

php1ic commented 1 year ago

The spdlog package is now at the latest version and I have removed the explicit dependency on fmt as it is pulled in as part of spdlog. Everything else uses the latest versions.

I'm not sure why, but if I tried to specify 10.0.0 for fmt (i.e. the same version as the dependency for spdlog/1.12.0), there were conflict errors with 10.1.1.

ferdymercury commented 1 year ago

The spdlog package is now at the latest version and I have removed the explicit dependency on fmt as it is pulled in as part of spdlog. Everything else uses the latest versions.

Thanks so much!!

I'm not sure why, but if I tried to specify 10.0.0 for fmt (i.e. the same version as the dependency for spdlog/1.12.0), there were conflict errors with 10.1.1.

When i run the CMake script on your current branch, it pulls 10.1.1 for fmt in my Ubuntu 22. So maybe spdlog depends on at least 10.1.1 and their docu is wrong?:

spdlog/1.12.0: Not found in local cache, looking in remotes...
spdlog/1.12.0: Checking remote: conancenter
spdlog/1.12.0: Downloaded recipe revision 0e390a2f5c3e96671d0857bc734e4731
fmt/10.1.1: Not found in local cache, looking in remotes...
fmt/10.1.1: Checking remote: conancenter
fmt/10.1.1: Downloaded recipe revision cd63809a79574a2f9eb73ca35f16a243
ferdymercury commented 1 year ago

The spdlog package is now at the latest version and I have removed the explicit dependency on fmt as it is pulled in as part of spdlog. Everything else uses the latest versions.

I'm not sure why, but if I tried to specify 10.0.0 for fmt (i.e. the same version as the dependency for spdlog/1.12.0), there were conflict errors with 10.1.1.

See https://github.com/conan-io/conan-center-index/commit/90e3ab49607d0f3ddaa9d5e53d87e367c45dbd69 so the webpage seems outdated?

php1ic commented 1 year ago

Good find! Looks like the webpage is wrong.

I'm happy to either put the explicit reference to fmt back in the conanfile.txt or leave it out as spdlog will pull it in.

Best practice is probably to have it explicitly, in case the project ever stops using spdlog?

php1ic commented 1 year ago

Hi @martukas, I believe all the changes you requested have been done. Can you please confirm/accept.

martukas commented 1 year ago

Thanks. Sorry for being so slow to respond. I will try to review this as soon as I can.