Skip to content
This repository has been archived by the owner on Feb 3, 2025. It is now read-only.

[Gazebo9] Fixed fails for OSX: Added using namespace boost::placeholders #2809

Merged
merged 8 commits into from
Aug 11, 2020

Conversation

ahcorde
Copy link
Contributor

@ahcorde ahcorde commented Jul 30, 2020

Related with this issue #2808 (comment) and this error in the buildfarm https://build.osrfoundation.org/job/gazebo-ci-pr_any-homebrew-amd64/2057/console

Signed-off-by: Alejandro Hernández [email protected]

Signed-off-by: Alejandro Hernández <[email protected]>
@ahcorde ahcorde added the 9 Gazebo 9 label Jul 30, 2020
@ahcorde ahcorde requested review from j-rivero and chapulina July 30, 2020 13:15
@ahcorde ahcorde self-assigned this Jul 30, 2020
@j-rivero
Copy link
Contributor

I still see some problems on the Mac build. Are they related to this PR?

/Users/jenkins/workspace/gazebo-ci-pr_any-homebrew-amd64/gazebo/test/plugins/ForceTorqueModelRemovalTestPlugin.cc:56:72: error: use of undeclared identifier '_1'
       boost::bind(&ForceTorqueModelRemovalTestPlugin::onUpdate, this, _1));

@ahcorde
Copy link
Contributor Author

ahcorde commented Jul 30, 2020

The compilation of the tests are failing. I'm trying to find a better solution

ahcorde added 2 commits July 31, 2020 09:34
Signed-off-by: Alejandro Hernández <[email protected]>
Copy link
Member

@scpeters scpeters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. I think an alternative would be to use std::bind instead of boost::bind, but this is more concise

gazebo/gui/model/ModelTreeWidget.cc Outdated Show resolved Hide resolved
Signed-off-by: Louise Poubel <[email protected]>
@scpeters
Copy link
Member

scpeters commented Aug 8, 2020

ah, it looks like the Ubuntu builds are broken by the using namespace change.

I think we should either:

  • #if BOOST_VERSION > X instead of #idfef __APPLE__ (like in CommonIface.hh)
  • try using std::bind instead of boost::bind to see if that fixes it

@scpeters
Copy link
Member

scpeters commented Aug 8, 2020

  • try using std::bind instead of boost::bind to see if that fixes it

I'm going to take a quick look at this to see if it's an easy fix

boost::bind changed its syntax for the placeholders,
so just switch to std::bind and add a scoped
using namespace std::placeholders to reduce
the size of the diff.

Signed-off-by: Steve Peters <[email protected]>
@scpeters
Copy link
Member

scpeters commented Aug 8, 2020

  • try using std::bind instead of boost::bind to see if that fixes it

I'm going to take a quick look at this to see if it's an easy fix

I took this approach in 5d3d06e; it's more than 3 lines per file, but it reduces our boost usage slightly without an unwieldy diff

@scpeters
Copy link
Member

scpeters commented Aug 8, 2020

  • try using std::bind instead of boost::bind to see if that fixes it

I'm going to take a quick look at this to see if it's an easy fix

I took this approach in 5d3d06e; it's more than 3 lines per file, but it reduces our boost usage slightly without an unwieldy diff

it doesn't work for ubuntu though 🤭

Revert the previous change to std::bind.

Signed-off-by: Steve Peters <[email protected]>
@scpeters
Copy link
Member

scpeters commented Aug 8, 2020

I switched to the other approach in fd718ac (#if BOOST_VERSION > X) and reduced the scope of the using namespace statement

@ahcorde
Copy link
Contributor Author

ahcorde commented Aug 10, 2020

thank you @scpeters for rework this PR 👍

Copy link
Contributor

@j-rivero j-rivero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @scpeters , @ahcorde . Looks good to me.

Signed-off-by: Alejandro Hernández <[email protected]>

Co-authored-by: Jose Luis Rivero <[email protected]>
@chapulina chapulina merged commit c55942e into gazebo9 Aug 11, 2020
@chapulina chapulina deleted the ahcorde/fix/boost branch August 11, 2020 01:03
j-rivero added a commit that referenced this pull request Aug 11, 2020
…ers (#2809)

Signed-off-by: Alejandro Hernández <[email protected]>

Co-authored-by: Jose Luis Rivero <[email protected]>
Co-authored-by: Louise Poubel <[email protected]>
Co-authored-by: Steve Peters <[email protected]>
scpeters added a commit to scpeters/gazebo that referenced this pull request Aug 11, 2020
…ers (gazebosim#2809)

Signed-off-by: Alejandro Hernández <[email protected]>

Co-authored-by: Jose Luis Rivero <[email protected]>
Co-authored-by: Louise Poubel <[email protected]>
Co-authored-by: Steve Peters <[email protected]>
scpeters added a commit to scpeters/gazebo that referenced this pull request Aug 11, 2020
…ers (gazebosim#2809)

Signed-off-by: Alejandro Hernández <[email protected]>

Co-authored-by: Jose Luis Rivero <[email protected]>
Co-authored-by: Louise Poubel <[email protected]>
Co-authored-by: Steve Peters <[email protected]>
chapulina added a commit that referenced this pull request Aug 18, 2020
…ers (#2809)

Signed-off-by: Alejandro Hernández <[email protected]>

Co-authored-by: Jose Luis Rivero <[email protected]>
Co-authored-by: Louise Poubel <[email protected]>
Co-authored-by: Steve Peters <[email protected]>
scpeters added a commit that referenced this pull request Aug 19, 2020
…ers (#2809)

Signed-off-by: Alejandro Hernández <[email protected]>

Co-authored-by: Jose Luis Rivero <[email protected]>
Co-authored-by: Louise Poubel <[email protected]>
Co-authored-by: Steve Peters <[email protected]>
scpeters added a commit to scpeters/homebrew-simulation that referenced this pull request Sep 15, 2020
scpeters added a commit to osrf/homebrew-simulation that referenced this pull request Sep 15, 2020
@scpeters scpeters mentioned this pull request Jan 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
9 Gazebo 9
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants