search

pothosware / PothosCore

The Pothos data-flow framework
https://github.com/pothosware/PothosCore/wiki
Boost Software License 1.0
300 stars 48 forks source link

Update cmake #237

Closed guruofquality closed 3 years ago

guruofquality commented 3 years ago
ncorgan commented 3 years ago

Neat. I've confirmed that all my local modules clean-build fine against this.

Two questions:

ncorgan commented 3 years ago

In running self-tests, I'm seeing a couple random crashes. I haven't looked deep into this yet, but I notice this change removes the enforcement of C++11 for modules. My local g++ (9.3) uses C++14 by default. Could type differences between the standards cause this?

guruofquality commented 3 years ago

In running self-tests, I'm seeing a couple random crashes. I haven't looked deep into this yet, but I notice this change removes the enforcement of C++11 for modules. My local g++ (9.3) uses C++14 by default. Could type differences between the standards cause this?

Maybe the removal of set(CMAKE_CXX_STANDARD 11) in the common config. Does adding set_property(TARGET ${POTHOS_MODULE_UTIL_TARGET} PROPERTY CXX_STANDARD 11) in the module util fix this? I dont think its the right idea going forward, just quick experiment.

guruofquality commented 3 years ago

Are there any similar changes needed to the PothosPython CMake config stuff?

Theres various places we could cleanup toolkits in general to use the target stuff and also to stop using the Pothos_* variables. But everything should keep building fine. Regardless, PothosPython in general looks like there isnt much to change at all.

Do the modules' minimum CMake versions need to be updated to support this?

maybe, but not has high. It needs cmake with the target feature, but the complication might be in the generation of the export, but the include side for the modules might be simpler.

in reality, its going to be the same cmake version that the core library was built with, so it shouldnt matter. But to be complete, the check should just be the exact same version (or higher if need be for that toolkit).

ncorgan commented 3 years ago

Scratch that, I'm seeing the same problems with the CXX_STANDARD addition. Not sure what's going on, digging into it. No more objections.

guruofquality commented 3 years ago

Scratch that, I'm seeing the same problems with the CXX_STANDARD addition. Not sure what's going on, digging into it. No more objections.

Theres a few flags that went from being set on all modules to basically private, just for the library build -- as to not be overly aggressive with forcing build flags into sub-projects. Given all of the function pointers and registries floating around, could always be something related to visibility for example: 


    target_compile_options(Pothos PRIVATE -fvisibility=hidden)
    target_compile_options(Pothos PRIVATE -fvisibility-inlines-hidden)
ncorgan commented 3 years ago

That did it. Visibility's bitten me before, I always assumed those flags were there in modules.

I think that's a reasonable one to export. It can easily be overridden if needed by subprojects.

ncorgan commented 3 years ago

It's a bit odd to push the GCC/Clang file change in this PR instead of the other one, but I won't raise too much of a fuss about it.

guruofquality commented 3 years ago

It's a bit odd to push the GCC/Clang file change in this PR instead of the other one, but I won't raise too much of a fuss about it.

im just testing with clang on osx if homebrew ever finishes updating

guruofquality commented 3 years ago

Looks like I havent made anything worse. Merging, can fix it in post.