Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to system installed Boost on Linux. #2007

Merged

Conversation

Projects
None yet
4 participants
@PetrosKataras
Copy link
Contributor

commented May 9, 2018

This PR switches the default behavior on Linux from linking with the precompiled Boost libs to system installed versions.

Requires libboost-filesystem-dev to be installed through the package manager of your platform/distribution.

This should resolves issues like this, #1915, #1968 and a few more of similar nature.

@richardeakin
Copy link
Collaborator

left a comment

Awesome, I think this will really simplify getting setup on Linux with various distros that we don't test day to day.

endif()
find_package( Boost 1.54 REQUIRED COMPONENTS system filesystem )
list( APPEND CINDER_LIBS_DEPENDS ${Boost_LIBRARIES} )
list( APPEND CINDER_INCLUDE_SYSTEM_PRIVATE ${Boost_INCLUDE_DIRS} )

This comment has been minimized.

Copy link
@richardeakin

richardeakin May 16, 2018

Collaborator

Does this mean that we no longer be shipping any boost libraries on linux, in other words the CINDER_BOOST_USE_SYSTEM option is the only option? If so I'm not against it, just thought we had discussed switching the default behavior but still shipping prebuilt binaries. Maybe that has shown itself to be more trouble than its worth, though.

This comment has been minimized.

Copy link
@PetrosKataras

PetrosKataras May 16, 2018

Author Contributor

My opinion would be to make the default and only behavior systemwide installed Boost. It has proven indeed problematic in the past even by just checking out the Boost submodule and then having mixed includes ( e.g. coming from Cinder's version and from a system wide installed version of Boost ) Not linking with the prebuilt binaries is not really enough in this case.

@PetrosKataras

This comment has been minimized.

Copy link
Contributor Author

commented May 16, 2018

Had forgotten that the binaries are not part of the submodule, so I just removed them and this should be complete now from my side I believe.

@rdtor

This comment has been minimized.

Copy link

commented Apr 29, 2019

Hi Petros how can i use this PR, do i need to build cinder again from your repo https://github.com/PetrosKataras/Cinder/tree/linux-boost-use-system-install?

@PetrosKataras

This comment has been minimized.

Copy link
Contributor Author

commented Apr 29, 2019

The simplest thing to try first would be to remove your previous Cinder build folder and re-compile Cinder with:

cd Cinder && mkdir build && cd build && cmake .. -DCINDER_BOOST_USE_SYSTEM=1 && make -j4

Make sure you have libboost-filesystem-dev installed and also make sure that you completely remove the Cinder/include/boost folder before trying this out.

If you want to apply this PR then you should do what @gaborpapp suggested in #2088 in order to checkout a branch locally in your machine with the changes applied and then recompile Cinder as usual.

i.e git fetch origin pull/2007/head:pr2007

@rdtor

This comment has been minimized.

Copy link

commented Apr 29, 2019

I applied this PR and followed what @gaborpapp suggested and the build was succesfull thanks

@andrewfb

This comment has been minimized.

Copy link
Collaborator

commented Apr 29, 2019

I'm intending to merge this in the next day or two unless anyone has any counterpoints

@richardeakin

This comment has been minimized.

Copy link
Collaborator

commented Apr 30, 2019

@PetrosKataras question:

The simplest thing to try first would be to remove your previous Cinder build folder and re-compile Cinder with:

cd Cinder && mkdir build && cd build && cmake .. -DCINDER_BOOST_USE_SYSTEM=1 && make -j4

Doesn't this PR remove the use of -DCINDER_BOOST_USE_SYSTEM=1? As in, after this PR the only option is to use system boost on linux.

@PetrosKataras

This comment has been minimized.

Copy link
Contributor Author

commented May 1, 2019

@richardeakin yeah this is correct - the part of the comment you quote was in the case that the user didn't want to apply this PR locally and instead try installed boost with the current Cinder setup ( i.e without this PR merged ).

@PetrosKataras

This comment has been minimized.

Copy link
Contributor Author

commented May 1, 2019

Just noticed the README mentioning a recursive git checkout. This should not be required for Linux actually after this PR is merged and in fact it should be discouraged since this would pull the include files for Boost which could still cause some weird clashes with system installed Boost ( ..it would also pull TinderBox which is also completely irrelevant to the Linux setup.. ).

I suppose we should add a note about this?

@richardeakin

This comment has been minimized.

Copy link
Collaborator

commented Jun 21, 2019

Just tested this with both the command line and Visual Studio Code on Ubuntu 19.04, looking good!

If it can cause header mismatches, I do think we should remove the --recursive flag from the git command for the linux docs pages. We may end up using other submodules later, but hopefully we'll have ditched the boost submodule by that point.

@richardeakin richardeakin merged commit e822387 into cinder:master Jun 21, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.