search

NatronGitHub / Natron

Open-source video compositing software. Node-graph based. Similar in functionalities to Adobe After Effects and Nuke by The Foundry.
http://NatronGitHub.github.io
GNU General Public License v2.0
4.54k stars 332 forks source link

System libs for glog and Ceres in CMake #829

Closed YakoYakoYokuYoku closed 1 year ago

YakoYakoYokuYoku commented 1 year ago

PR Description

What type of PR is this? (Check one of the boxes below)

What does this pull request do?

Because a system installation of glog may conflict with the bundled version (see #807) and Ceres can take quite the time to build, I've decided to provide the option to use the system version for the CMake build. openMVG was ignored due to not many distributions provide a package for it and its lack of CMake config files for the build to discover its install, though this can be revisited in the future.

Show a few screenshots (if this is a visual change)

N/A.

Have you tested your changes (if applicable)? If so, how?

By building with those libraries and running Natron.

Futher details of this pull request

In the future, when Qt6 support arrives alongside the official HTTP server module, qhttpserver will receive the same treatment.

devernay commented 1 year ago

hum I don't think this is a good idea. We're using an older version of Ceres, and we may be lagging on glog too. Current versions are untested. It's better to stick with what we provide until the current versions are properly tested. The problem with a system installation of glog or ceres can be fixed by making sure the include path and the library path are correctly set. I would prefer that.

YakoYakoYokuYoku commented 1 year ago

We're using an older version of Ceres, and we may be lagging on glog too. Current versions are untested. It's better to stick with what we provide until the current versions are properly tested. The problem with a system installation of glog or ceres can be fixed by making sure the include path and the library path are correctly set. I would prefer that.

In my case I can say that current versions of Ceres and glog work with Natron in Arch Linux. But if issues raise from using the system installs then the bundled versions can comfortably be used instead, and due to the lack of testing in platforms different than Linux then it makes sense that the option is OFF by default as this PR does.

devernay commented 1 year ago

Ok to keep it OFF, but does this also fix #807 if there are system versions installed?

YakoYakoYokuYoku commented 1 year ago

I speculate that given the option to not use the bundled version of glog it would not pass any include or link flags that collide with the ones from a system install, but we have to take in consideration that QMake was being in use in that issue, which links glog as -lglog (as of time of writing of said issue CMake wasn't supported yet) and that CMake is a fix too, due to its nature that project libraries are linked using their path directly, i.e. glog would be linked with build/glog/libglog.a instead of -lglog, thus disambiguating between a bundled version and a system version of glog, same as openMVG or libmv.

YakoYakoYokuYoku commented 1 year ago

Merging this PR in the evening if there are no further objections.