search

robotology / blockfactory

A tiny framework to wrap algorithms for dataflow programming
https://robotology.github.io/blockfactory
GNU Lesser General Public License v2.1
40 stars 16 forks source link

Support BLOCKFACTORY_PLUGIN_PATH environment variable #32

Closed diegoferigo closed 5 years ago

diegoferigo commented 5 years ago

P.s. for the time being I will keep the shlibpp changes local to this project. I will open a PR upstream in the future.

diegoferigo commented 5 years ago

What's still puzzling me that in Windows everything works only if BUILD_SHARED_LIBS is turned on. @traversaro any clue?

traversaro commented 5 years ago

What's still puzzling me that in Windows everything works only if BUILD_SHARED_LIBS is turned on. @traversaro any clue?

What is strange about that?

traversaro commented 5 years ago

You are exporting all symbols in https://github.com/robotology/blockfactory/pull/32/files#diff-af3b638bc2a3e6c650974192a53c7291L92 , so that should work fine with shared libraries unless you have some static variables to export.

traversaro commented 5 years ago

For all the file path stuff did you considered to use std:::filesystem/std::experimental::filesystem?

diegoferigo commented 5 years ago

For all the file path stuff did you considered to use std:::filesystem/std::experimental::filesystem?

Yes but I was not 100% sure that it would work on all compilers. Do you have any experience?

traversaro commented 5 years ago

Yet but I was not 100% sure that it would work on all compilers. Do you have any experience?

Yes, but just in Ubuntu, see https://github.com/robotology/gazebo-fmi/blob/master/.travis.yml . It works fine (with the proper flags, see https://github.com/robotology/gazebo-fmi/blob/733ef7c66cdf8241ab8c91c83fb34148f4716ed7/libraries/private-utils/CMakeLists.txt#L24). Apparently there could be problems on macOS, see https://stackoverflow.com/questions/49577343/filesystem-with-c17-doesnt-work-on-my-mac-os-x-high-sierra/49600206 . It is probably indeed better to avoid it for now.

diegoferigo commented 5 years ago

You are exporting all symbols in https://github.com/robotology/blockfactory/pull/32/files#diff-af3b638bc2a3e6c650974192a53c7291L92 , so that should work fine with shared libraries unless you have some static variables to export.

I didn't get it entirely. The factory test fails on Windows if BUILD_SHARED_LIBS=OFF, and this means that the plugin is not found / fails to load. But it is something not related to the plugin library itself since it is forced to get compiled as SHARED:

https://github.com/robotology/blockfactory/blob/7b9033c133488b648fc04c509c24c764a7c3cd64/cmake/BlockFactoryPlugin.cmake#L54-L57

I still have to figure out what can be the problem.

traversaro commented 5 years ago

I didn't get it entirely.

Sorry, I was confused and thought you were surprised that it worked with BUILD_SHARED_LIBS to ON, not that it was not working with BUILD_SHARED_LIBS set to OFF.

diegoferigo commented 5 years ago

Do you think if would be safe setting by default BUILD_SHARED_LIBS=ON as a non-cache variable, at least on windows?

traversaro commented 5 years ago

Do you think if would be safe setting by default BUILD_SHARED_LIBS=ON as a non-cache variable, at least on windows?

Not sure, I would probably just print a clear error if BUILD_SHARED_LIBS set to OFF is used with Windows, setting BUILD_SHARED_LIBS without the possibility to change it may be counterintuitive w.r.t. to the typical CMake user experience.

diegoferigo commented 5 years ago

@traversaro Addressed all the requested changes